Seems the OpenSSH server running on windows fails keyboard-interactive
auth this way without sending any prompt to client. In such case the
golang ssh client should not retry keyboard-interactive auth when the
auth method is wrapped in a RetryableAuthMethod(). Rather the auth
method should be immediately marked as tried&failed and the client auth
process should move on to next available and acceptable auth method.
Fixesgolang/go#67855
Change-Id: I6c64ae58ff8325774e37af716601b112f8833d8f
GitHub-Last-Rev: 7fafc4d1c81284b31000d7d6ccadd934dda26d24
GitHub-Pull-Request: golang/crypto#297
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/590956
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
CL 220037 had implemented support for host authentication using
rsa-sha2-256/512, but not client public key authentication. OpenSSH
disabled the SHA-1 based ssh-rsa by default in version 8.8 (after
pre-announcing it in versions 8.2, 8.3, 8.4, 8.5, 8.6, and 8.7) although
some distributions re-enable it. GitHub will start rejecting ssh-rsa for
keys uploaded before November 2, 2021 on March 15, 2022.
https://github.blog/2021-09-01-improving-git-protocol-security-github/
The server side already worked, as long as the client selected one of
the SHA-2 algorithms, because the signature flowed freely to Verify.
There was however nothing verifying that the signature algorithm matched
the advertised one. The comment suggested the check was being performed,
but it got lost back in CL 86190043. Not a security issue because the
signature had to pass the callback's Verify method regardless, and both
values were checked to be acceptable.
Tested with OpenSSH 8.8 configured with "PubkeyAcceptedKeyTypes -ssh-rsa"
and no application-side changes.
The Signers returned by ssh/agent (when backed by an agent client)
didn't actually implement AlgorithmSigner but ParameterizedSigner, an
interface defined in an earlier version of CL 123955.
Updates golang/go#49269Fixesgolang/go#39885
For golang/go#49952
Change-Id: I13b41db8041f1112a70f106c55f077b904b12cb8
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392394
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
The server side implementation was not actually populating the
SSH_MSG_USERAUTH_INFO_REQUEST field with the
KeyboardInteractiveChallenge argument, although the client side was
deserializing it and passing it to the KeyboardInteractiveChallenge
callback.
Anyway, the first field of SSH_MSG_USERAUTH_INFO_REQUEST is "name", not
"user". Maybe the confusion was due to the first field of
SSH_MSG_USERAUTH_REQUEST being the user.
RFC 4256, Section 3.3, says this about it
One possibility is to use the name field (possibly
prefixed with the application's name) as the title of a dialog window
in which the prompt(s) are presented.
and examples include "CRYPTOCard Authentication", "Password
Authentication", and "Password Expired".
Co-authored-by: Kevin Wallace <kevin@pentabarf.net>
Change-Id: Ic6ec0dfea2122704603c44f42898a980689a15c9
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/372234
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
By using a slice instead of a map for tried authentication methods the order is always the same. Small slices are also faster than maps.
Before the change sometimes I get the error:
ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain
and sometimes:
ssh: handshake failed: ssh: unable to authenticate, attempted methods [publickey none], no supported methods remain
Change-Id: I06507d57e9eef497ff05bce088d52607e69dde3e
GitHub-Last-Rev: 3a46aae4c6e3e5f52ca4b04384b3cc4efc039aa6
GitHub-Pull-Request: golang/crypto#142
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/239171
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixesgolang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
Reviewed-on: https://go-review.googlesource.com/88035
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
According to RFC 4252 section 5.4, the banner is sent between the
ssh-connection request and responding to user authentication.
Original support for server sending banner by joshua stein <jcs@jcs.org>
Fixesgolang/go#19567
Change-Id: I729b3c8e5fd2c0068609d1590b61e92f40d87ea4
Reviewed-on: https://go-review.googlesource.com/71790
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
According to RFC 4252 section 5.4, the banner is sent between the
ssh-connection request and responding to user authentication.
Original support for server sending banner by joshua stein <jcs@jcs.org>
Fixesgolang/go#19567
Change-Id: I68944a7f4711c0623759f6a59023e8e45a8781aa
Reviewed-on: https://go-review.googlesource.com/65271
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
Previously, the public key authentication for clients would send an
enquiry to the remote for every key specified before attempting to
authenticate with the server.
Now, we immediately try to authenticate once a valid key is found.
This results in exchanging fewer packets if the valid key is near the
top of the list. If all keys fail, then the number of packets exchanged
by the client and server is unaffected.
For OpenSSH daemon, an enquiry into the validity of a key without
authentication is still recorded as an authentication attempt, so any
clients with more than MaxAuthTries public keys would not be able to
authenticate using the previous implementation. This change will allow
clients to succeed authentication if the successful key is at the start
of the list of keys.
Change-Id: I8ea42caf40c0864752218c3f6934e86b12f5b81a
Reviewed-on: https://go-review.googlesource.com/38890
Reviewed-by: Adam Langley <agl@golang.org>
Use channels and a dedicated write loop for managing the rekeying
process. This lets us collect packets to be written while a key
exchange is in progress.
Previously, the read loop ran the key exchange, and writers would
block if a key exchange was going on. If a reader wrote back a packet
while processing a read packet, it could block, stopping the read
loop, thus causing a deadlock. Such coupled read/writes are inherent
with handling requests that want a response (eg. keepalive,
opening/closing channels etc.). The buffered channels (most channels
have capacity 16) papered over these problems, but under load SSH
connections would occasionally deadlock.
Fixes#18439.
Change-Id: I7c14ff4991fa3100a5d36025125d0cf1119c471d
Reviewed-on: https://go-review.googlesource.com/35012
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Adds a new AuthMethod called "RetryableAuthMethod" which decorates any
other authmethod, allowing it to be retried up to maxTries before
aborting.
Fixes#16077
Change-Id: Ie310c24643e53dca4fa452750a69936674906484
Reviewed-on: https://go-review.googlesource.com/24156
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Validating a public key doesn't return any remaining methods so, if all public keys were rejected, a nil slice would be returned for the remaining methods and authentication would stop.
We could have validateKey return methods, but that wouldn't solve the problem of what to do if the callback returns no keys. In that case we don't have any keys to test.
So this change makes it possible for an AuthMethod to return a nil slice for the remaining methods (meaning "reuse the last list"). It also fixes a scoping bug.
Fixesgolang/go#7787.
LGTM=hanwen
R=hanwen
CC=golang-codereviews
https://golang.org/cl/94350043
Transport should not be a ReadWriter. It can only write packets, i.e. no partial reads or writes. Furthermore, you can currently do ClientConn.Write() while the connection is live, which sends raw bytes over the connection. Doing so will confuse the transports because the data is not encrypted.
As a consequence, ClientConn and ServerConn stop being a net.Conn
Finally, ensure that {Server,Client}Conn implement LocalAddr and RemoteAddr methods that previously were exposed by an embedded net.Conn field.
R=hanwen
CC=golang-dev
https://golang.org/cl/16610043
Adds readPacket() to conn, and renames conn to packetConn.
Key exchanges operate on packetConn, so they can be
unittested.
R=agl, jpsugar, dave
CC=golang-dev
https://golang.org/cl/13352055
I ran go vet over all of go.crypto and removed lots of panic("unreachable") that are no longer needed.
R=golang-dev, r, dgryski
CC=golang-dev
https://golang.org/cl/10113043
These are the cosmetic cleanups from the bits of code that I
rereviewed.
1) stringLength now takes a int; the length of the string. Too many
callers were allocating with stringLength([]byte(s)) and
stringLength only needs to call len().
2) agent.go now has sendAndReceive to remove logic that was
duplicated.
3) We now reject negative DH values
4) We now reject empty packets rather than crashing.
R=dave, jonathan.mark.pittman
CC=golang-dev
https://golang.org/cl/6061052
Everywhere else in the code base, when we have an interface{} which is
a stand in for a public key, we use *foo.PublicKey rather than
foo.PublicKey. This change makes ssh reflect that.
R=dave, r
CC=golang-dev
https://golang.org/cl/5686067
Manual edits to README.
Moved from main Go repository, deleted Makefiles, ran gofix -r go1rename.
Tested with: go test code.google.com/p/go.crypto/...
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/5564059
Nicola Murino
samiponkanen
Filippo Valsorda
Aidan Coyle
Julian Kornberger
yanweizhi
Tugdual Saunier
Han-Wen Nienhuys
namusyaka
James Myers
Jamie Beverly
Adam Langley
Dave Cheney
Jonathan Pittman
JP Sugarbroad
Mikio Hara
Gary Burd
Russ Cox