This change breaks backwards compatibility.
MaxAuthTries specifies the maximum number of authentication attempts
permitted per connection. If set to a negative number, the server will
allow unlimited authentication attempts. MaxAuthTries defaults to 6 if
not specified, which is a backwards incompatible change. On exceeding
maximum authentication attempts, the server will send a disconnect
message to the client.
This configuration property mirrors a similar property in sshd_config
and prevents bad actors from continuously trying authentication.
Change-Id: Ic77d2c29ee2fd2ae5c764becf7df91d29d03131b
Reviewed-on: https://go-review.googlesource.com/35230
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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 previous attempt would fail in the following scenario:
* select picks "first" kex from requestKex
* read loop receives a remote kex, posts on requestKex (which is now
empty) [*] for sending out a response, and sends pendingKex on startKex.
* select picks pendingKex from startKex, and proceeds to run the key
exchange.
* the posting on requestKex in [*] now triggers a second key exchange.
Fixes#18861.
Change-Id: I443e82f1d04c7f17d1485fdb87072b9feec26aa8
Reviewed-on: https://go-review.googlesource.com/36055
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
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>
1) Always force a key exchange if we exchange 2^31 packets. In the past
this might not happen if RekeyThreshold was set to a very large
interval.
2) Follow recommendations from RFC 4344 for block ciphers. For AES, we
can encrypt 2^(blocksize/4) blocks under the same keys.
On modern hardware, the previous default of 1Gb could force a key
exchange within ~10 seconds. Since the key exchange takes 3 roundtrips
(send kex init, send DH init, send NEW_KEYS), this is relatively
expensive on high-latency links.
Change-Id: I1297124a307c541b7bf22d814d136ec0c6d8ed97
Reviewed-on: https://go-review.googlesource.com/35410
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
The initial kex is started from both sides simultaneously, and before,
we could consume the the incoming kex request before we consumed from
our internal channel. This would result in initiating a key exchange
just after completing the initial one, which is not only an extra
delay, but also an error when using OpenSSH (OpenSSH does not support
key exchanges during user authentication).
Change-Id: Ia7e0748ea2bca80ae97d187bcf2931ab6422276b
Reviewed-on: https://go-review.googlesource.com/35851
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
A net.IP may be represented by both by a 4 as well as a 16 byte long
byte slice. Because of this, it is not safe to compare IP addresses
using bytes.Equal as the same IP address using a different internal
representation will produce mismatches.
Change-Id: I0d228771cf363ccfb9532f8bc2a2fc8eff61f6e9
Reviewed-on: https://go-review.googlesource.com/34450
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
911fafb28f made MakeRaw match C's behaviour. This included clearing the
OPOST flag, which means that one now needs to write \r\n for a newline,
otherwise the cursor doesn't move back to the beginning and the terminal
prints a staircase.
(Dear god, we're still emulating line printers.)
This change causes the terminal package to do the required
transformation.
Fixesgolang/go#17364.
Change-Id: Ida15d3cf701a21eaa59161ab61b3ed4dee2ded46
Reviewed-on: https://go-review.googlesource.com/33902
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously this code bounded the values to [1, p-1]. This protects
against invalid values that could take lots of CPU time to calculate
with. But the standard bounding is [2, p-2] so mirror that.
Since the DH exchange is signed anyway, this is not a security fix.
Change-Id: Ibef01805a596a433b0699d7a09c076344fa8c070
Reviewed-on: https://go-review.googlesource.com/30590
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When implemented by ssh.PublicKey types, the new CryptoPublicKey
interface exposes the public key in the the crypto.PublicKey form via a
CryptoPublicKey() method.
This is useful for example in a custom ServerConfig.PublicKeyCallback
function to check or record additional details about the underlying
crypto.PublicKey
Change-Id: I4429df42c6fc5119f7c0023a539aaa9c59648bba
Reviewed-on: https://go-review.googlesource.com/23974
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The RequestPty function takes the size arguments in the order height,
then width, instead of the more common width, then height. 80 is a very
common width for a terminal, so when the example reads RequestPty(...,
80, 40, ...), it's easy to assume that the order is width-height.
Switching the order should make it more obvious what is going on.
Change-Id: I1d6266b1c0dcde5ee6e31a6d26d2dcaf14fec58a
Reviewed-on: https://go-review.googlesource.com/18290
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
After discussion around an example SFTP implementation:
https://github.com/pkg/sftp/pull/54
it has been suggested that errors should be handled using
log.Fatal rather than panic, and that the actual underlying error
should also be logged. In the existing SSH examples there
are several different styles of error handling using both panic
and log.Fatalf.
This patch uses log.Fatal consistently for all of these cases.
Change-Id: I2cebfae1821530dc3c5bbc46d451fe026bed582f
Reviewed-on: https://go-review.googlesource.com/16736
Reviewed-by: Russ Cox <rsc@golang.org>
RSA and DSA keys if encrypted have the
phrase ENCRYPTED in their Proc-Type block
header according to RFC 1421 Section 4.6.1.1.
This CL checks for that phrase and errors out
if we encounter it, since we don't yet have
decryption of encrypted private keys.
Fixesgolang/go#6650
Change-Id: I5b157716a2f93557d289af5f62994234a2e7a0ed
Reviewed-on: https://go-review.googlesource.com/29676
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If a key is added to an agent keyring with constraints, honor them.
This will remove keys when they've been on the keyring for
LifetimeSecs seconds or longer and will ask the user to confirm a
signing operation if ConfirmBeforeUse is set.
Change-Id: I633713c5f78b13a628a5d752f11b306b6e16a2ef
Reviewed-on: https://go-review.googlesource.com/28956
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the initial key exchange, a client has the option of sending a
guessed key exchange packet. This guess is considered wrong (RFC 4253
Section 7) if "the kex algorithm and/or the host key algorithm is
guessed wrong (server and client have different preferred algorithm),
or if any of the other algorithms cannot be agreed upon...".
The library should be checking the first algorithm in the supported
algorithms, not the agreed algorithm. It also needs to check both the
kex algorithm and the host key algorithm.
Fixesgolang/go#16962
Change-Id: I6b62b1f5b39731326280571d373635085135a2a1
Reviewed-on: https://go-review.googlesource.com/28750
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current behaviour of the crypto/ssh server implementation is to
remove the username from ConnMetadata if the connection is done without
authentication (NoClientAuth). This appears to be a bug.
This behaviour is different from other SSH server implementations like
for example Paramiko (Python) which keeps the username.
Additionally RFC4252 (https://www.ietf.org/rfc/rfc4252.txt) section
5 states the username has to be included in every USERAUTH message.
Change-Id: I27fa50db92eb535e90fe088453faa6f2a76ee31f
Reviewed-on: https://go-review.googlesource.com/27612
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Rather than guessing at which terminal flags should be set or cleared by
MakeRaw, this change tries to make it mirror the behaviour documented
for cfmakeraw() in the termios(3) manpage.
Fixesgolang/go#15625
Change-Id: Icd6b18ffb57ea332147c8c9b25eac5e41eb0863a
Reviewed-on: https://go-review.googlesource.com/22964
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Faiyaz Ahmed <ahmedf@vmware.com>
Reviewed-by: Adam Langley <agl@golang.org>
This failure is tracked already. Remove it from the dashboard
while it's fixed so it doesn't hide more interesting failures.
Updates golang/go#15198
Change-Id: Ib48d1e37ac97914ac082b2602c812151147393e4
Reviewed-on: https://go-review.googlesource.com/24986
Reviewed-by: Ian Lance Taylor <iant@golang.org>
According to RFC 4254 section 6.10, SSH server implementations may
omit the exit-status and exit-signal messages. If this happens, we
now return &ExitMissingError{}, so clients can handle this case
specifically.
This came up in the discussion of issue #16194.
Change-Id: Iae5e916b18aa5bd8e95618e9fcfcab8b19e147d9
Reviewed-on: https://go-review.googlesource.com/24727
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
James Myers
Akihiro Suda
Paul Querna
Han-Wen Nienhuys
Brad Fitzpatrick
MiLk
Heschi Kreinick
Adam Langley
Alex Brainman
Peter Morjan
Joe Tsai
Mikio Hara
Péter Szilágyi
Ryuzo Yamamoto
Dmitry Savintsev
Martin Garton
Henrik Hodne
Emmanuel Odeke
Mike Houston
Fazal Majid
Peter Moody
Kaleb Elwert
Sven Blumenstein
Faiyaz Ahmed
Martin Hamrle
Jamie Beverly