This change breaks existing behavior.
Before, a missing ClientConfig.HostKeyCallback would cause host key
checking to be disabled. In this configuration, establishing a
connection to any host just works, so today, most SSH client code in
the wild does not perform any host key checks.
This makes it easy to perform a MITM attack:
* SSH installations that use keyboard-interactive or password
authentication can be attacked with MITM, thereby stealing
passwords.
* Clients that use public-key authentication with agent forwarding are
also vulnerable: the MITM server could allow the login to succeed, and
then immediately ask the agent to authenticate the login to the real
server.
* Clients that use public-key authentication without agent forwarding
are harder to attack unnoticedly: an attacker cannot authenticate the
login to the real server, so it cannot in general present a convincing
server to the victim.
Now, a missing HostKeyCallback will cause the handshake to fail. This
change also provides InsecureIgnoreHostKey() and FixedHostKey(key) as
ready made host checkers.
A simplistic parser for OpenSSH's known_hosts file is given as an
example. This change does not provide a full-fledged parser, as it
has complexity (wildcards, revocation, hashed addresses) that will
need further consideration.
When introduced, the host checking feature maintained backward
compatibility at the expense of security. We have decided this is not
the right tradeoff for the SSH library.
Fixesgolang/go#19767
Change-Id: I45fc7ba9bd1ea29c31ec23f115cdbab99913e814
Reviewed-on: https://go-review.googlesource.com/38701
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The ssh tests currently require 127.0.0.1 to work which isn't
necessarily available everywhere. To fix the source-address tests,
support comma-separated source-address values per the PROTOCOL.certkeys
file:
Comma-separated list of source addresses
from which this certificate is accepted
for authentication. Addresses are
specified in CIDR format (nn.nn.nn.nn/nn
or hhhh::hhhh/nn).
If this option is not present then
certificates may be presented from any
source address.
Change-Id: I87536ff81ffa005c073da103021ebc0dfb12b620
Reviewed-on: https://go-review.googlesource.com/36110
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Since encryption messes up the packets, the wrongly retained packets
look like noise and cause application protocol errors or panics in the
SSH library.
This normally triggers very rarely: the mandatory key exchange doesn't
have parallel writes, so this failure condition would be setup on the
first key exchange, take effect only after the second key exchange.
Fortunately, the tests against openssh exercise this. This change adds
also adds a unittest.
Fixes#18850.
Change-Id: I656c8b94bfb265831daa118f4d614a2f0c65d2af
Reviewed-on: https://go-review.googlesource.com/36056
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@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>
This is done by running the key exchange and setting the session ID
under mutex. If the first exchange encounters an already set session
ID, then do nothing.
This fixes a race condition:
On setting up the connection, both sides sent a kexInit to initiate
the first (mandatory) key exchange. If one side was faster, the
faster side might have completed the key exchange, before the slow
side had a chance to send a kexInit. The slow side would send a
kexInit which would trigger a second key exchange. The resulting
confirmation message (msgNewKeys) would confuse the authentication
loop.
This fix removes sessionID from the transport struct.
This fix also deletes the unused interface rekeyingTransport.
Fixes#15066
Change-Id: I7f303bce5d3214c9bdd58f52d21178a185871d90
Reviewed-on: https://go-review.googlesource.com/21606
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Unblock writers if a read error occurs while writers are blocked on a
pending key change.
Add test to check for deadlocks in error paths in handshake.go
Fixesgolang/go#11992.
Change-Id: Id098bd9fec3d4fe83daeb2b7f935e5647c19afd3
Reviewed-on: https://go-review.googlesource.com/13594
Reviewed-by: Adam Langley <agl@golang.org>
Han-Wen Nienhuys
Heschi Kreinick
David du Colombier
Adam Langley