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
James Myers
Han-Wen Nienhuys
Jamie Beverly
Adam Langley
Dave Cheney
Jonathan Pittman
JP Sugarbroad
Mikio Hara
Gary Burd
Russ Cox