Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/crypto/ssh: Support Encrypted Private Keys #18692

Open
pquerna opened this Issue Jan 17, 2017 · 12 comments

Comments

Projects
None yet
9 participants
@pquerna
Copy link

pquerna commented Jan 17, 2017

What did you do?

SSH private keys are often stored on disk encrypted with a password. There are 2 major formats of these encrypted private keys. Today x/crypto/ssh just returns an error when encountering these keys.

What did you expect to see?

ssh.ParseRawPrivateKey(...) supports multiple private key formats, but only supports keys that are not encrypted with a passphrase.

For some PEM key formats, x509.DecryptPEMBlock works in, but for OpenSSH's newer openssh-key-v1 format, there is nothing in the stdlib that can decrypt the keys.

What did you see instead?

Two new methods, ParseRawPrivateKeyWithPassphrase and ParsePrivateKeyWithPassphrase:

func ParsePrivateKeyWithPassphrase(pemBytes []byte, passphrase []byte) (Signer, error)
func ParseRawPrivateKeyWithPassphrase(pemBytes []byte, passphrase []byte) (interface{}, error)

For keys that are PEM encrypted, decrypt them with x509.DecryptPEMBlock, and for the keys that are encrypted in OpenSSH's native key format, generate the key using the bcrypt-KDF, and decrypt with AES-256-CBC.

  • The function names are quite long, ideas?
@odeke-em

This comment has been minimized.

Copy link
Member

odeke-em commented Jan 17, 2017

Just a drive by comment so that if this proposal is accepted, we'll know what tests to update and the code to update: I added a proper error on encountering encrypted private keys with CL https://go-review.googlesource.com/29676, we'll need to update the tests from that CL.

The function names look gucci to me since they just add the WithPassphrase suffix and the return signatures are the same as the already existent ParsePrivateKey and ParseRawPrivateKey.
Alternatively, the names could be:

ParseEncryptedPrivateKey
ParseEncryptedRawPrivateKey

/cc @hanwen @agl

@odeke-em odeke-em added the Proposal label Jan 17, 2017

@rakyll rakyll added this to the Proposal milestone Jan 17, 2017

@pquerna

This comment has been minimized.

Copy link
Author

pquerna commented Jan 20, 2017

FYI, we had some code for decrypting & marshaling these encrypted keys from work code; hacked them over to the proposedParseEncryptedPrivateKey and ParseEncryptedRawPrivateKey APIs here:

https://github.com/ScaleFT/sshkeys

The hardest thing from making these an easy CR to gerrit from me is an external dependency on:

https://github.com/dchest/bcrypt_pbkdf

CC @dchest

@dchest

This comment has been minimized.

Copy link
Contributor

dchest commented Jan 20, 2017

@pquerna I'm not sure how useful bcrypt_pbkdf is for other uses, so probably there's no point in integrating it into x/crypto, it would be easier to just put it into internal/ package or right into the package without creating a new package (it's not a lot of lines of code). It already uses Go BSD-like license, I can just change my name in the copyright header to Go Authors (I signed CLA and have my name in AUTHORS file already) and whatever else I need to do for licensing purposes.

@odeke-em

This comment has been minimized.

Copy link
Member

odeke-em commented Feb 18, 2017

@pquerna and @dchest I just noticed that x/crypto already has https://godoc.org/golang.org/x/crypto/pbkdf2 and it has function:

func Key(password, salt []byte, iter, keyLen int, h func() hash.Hash) []byte

Perhaps this might ease the CL so that figuring out the extra dependency might no longer be needed. What do y'all think?

@dchest

This comment has been minimized.

Copy link
Contributor

dchest commented Feb 18, 2017

@odeke-em bcrypt_pbkdf and pbkdf2 are different and not compatible. OpenSSH uses the former.

@odeke-em

This comment has been minimized.

Copy link
Member

odeke-em commented Feb 18, 2017

Thank you for the clarification @dchest, I was just about to go blindly write a program that derived a key using the different packages :)

@jeffallen

This comment has been minimized.

Copy link
Contributor

jeffallen commented May 24, 2017

Decryption of PEM blocks is already in the stdlib (https://golang.org/pkg/crypto/x509/#DecryptPEMBlock), we should at least use that for encrypted PEM keys. The OpenSSH-format private keys are another story, and would need more code.

@mattn

This comment has been minimized.

Copy link
Member

mattn commented May 24, 2017

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Jun 13, 2017

CL https://golang.org/cl/36079 mentions this issue.

@dchest

This comment has been minimized.

Copy link
Contributor

dchest commented Jun 13, 2017

Shouldn't this issue be open until DecryptPEMBlock supports bcrypt_pbkdf? https://golang.org/cl/36079 doesn't fix it.

For some PEM key formats, x509.DecryptPEMBlock works in, but for OpenSSH's newer openssh-key-v1 format, there is nothing in the stdlib that can decrypt the keys.

@mattn

This comment has been minimized.

Copy link
Member

mattn commented Jun 13, 2017

Sorry, my mistake. I'll look into it in later. @hanwen could you please re-open this?

@hanwen hanwen reopened this Jun 13, 2017

tg123 added a commit to tg123/sshpiper that referenced this issue Jun 20, 2017

ssh: add ParsePrivateKeysWithPassphrase
ssh package doesn't provide way to parse private keys with passphrase.

Fixes golang/go#18692

Change-Id: Ic139f11b6dfe7ef61690d6125e0673d50a48db16
Reviewed-on: https://go-review.googlesource.com/36079
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
@Rondom

This comment has been minimized.

Copy link

Rondom commented Sep 16, 2018

OpenSSH 7.8 changed its default key format. It will likely enter distributions like Ubuntu or Fedora in their next release. Thus, new OpenSSH format keys with passphrase will become more common soon.

https://www.openssh.com/releasenotes.html

  • ssh-keygen(1): write OpenSSH format private keys by default
    instead of using OpenSSL's PEM format. The OpenSSH format,
    supported in OpenSSH releases since 2014 and described in the
    PROTOCOL.key file in the source distribution, offers substantially
    better protection against offline password guessing and supports
    key comments in private keys. If necessary, it is possible to write
    old PEM-style keys by adding "-m PEM" to ssh-keygen's arguments
    when generating or updating a key.

tg123 added a commit to tg123/sshpiper that referenced this issue Dec 29, 2018

introduce pickupstream tools
sync golang ssh

x/crypto/ssh: ParsePrivateKey errors out with encrypted private keys

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.

Fixes golang/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>

ssh/terminal: implement ReadPassword and IsTerminal

Fixes golang/go#13085.

Change-Id: I2fcdd60e5e8db032d6fa3ce76198bdc7a63f3cf6
Reviewed-on: https://go-review.googlesource.com/16722
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

ssh: Consistent error handling in examples

After discussion around an example SFTP implementation:
pkg/sftp#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>

x/crypto/ssh: public key authentication example

Fixes golang/go#13902.

Adds public key authentication to the
password authentication example.

Change-Id: I4af0ca627fb15b617cc1ba1c6e0954b013f4d94f
Reviewed-on: https://go-review.googlesource.com/29374
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: fix height/width order in RequestPty example

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>

ssh: add CryptoPublicKey interface, expose underlying crypto.PublicKey

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>

sync with golang/ssh upstream

Allow hyphen-characters within usernames (#9)

* Allow hyphen-characters within usernames.

* Allow up to 32 characters for usernames.

ssh: bound DH public values to [2, p-2].

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>

crypto/ssh: fix comment for ssh.NewPublicKey

Change-Id: I88bb7859259c82cd77ab2d26b728143281761def
Reviewed-on: https://go-review.googlesource.com/25232
Reviewed-by: Russ Cox <rsc@golang.org>

x/crypto/ssh: Add FingerprintLegacyMD5 and FingerprintSHA256 methods

Implement a standards-compliant fingerprint format method (RFC 4716 section 4)
and a newer SHA256 fingerprint format method.

Fixes golang/go#12292

Change-Id: I4f3f8fc1d0a263cb3b0964d0078e69006a39d1a5
Reviewed-on: https://go-review.googlesource.com/32814
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

x/crypto/ssh/terminal: replace \n with \r\n.

911fafb28f4 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.

Fixes golang/go#17364.

Change-Id: Ida15d3cf701a21eaa59161ab61b3ed4dee2ded46
Reviewed-on: https://go-review.googlesource.com/33902
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

crypto/ssh: use net.IP.Equal instead of bytes.Equal

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>

ssh/terminal: fix a typo

Change-Id: Iafe2ebb6d37afd2a64aa72750a722d4860bb735e
Reviewed-on: https://go-review.googlesource.com/34535
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

all: fix some vet warnings

Change-Id: I85c2912a6862c6c251450f2a0926ecd33a9fb8e7
Reviewed-on: https://go-review.googlesource.com/34815
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/terminal: consistent return value for Restore

This patch makes the Restore function return nil
on success to be consistent with other functions
like MakeRaw.

Change-Id: I81e63f568787dd88466a5bb30cb87c4c3be75a5c
Reviewed-on: https://go-review.googlesource.com/34952
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

x/ssh: filter debug and ignore messages in transport.readPacket.

This prevents these messages from confusing higher layers of the
protocol.

Fixes #16927.

Change-Id: If18d8d02bdde3c0470e29a7280cd355d3e55ad78
Reviewed-on: https://go-review.googlesource.com/34959
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>

ssh: make client auth tests less chatty.

Change-Id: Ib35ce0e7437e32a3fa24a9330c479306b7fa6880
Reviewed-on: https://go-review.googlesource.com/35011
Reviewed-by: Adam Langley <agl@golang.org>

ssh: rewrite (re)keying logic.

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>

ssh/terminal: fix line endings handling in ReadPassword

Fixes golang/go#16552

Change-Id: I18a9c9b42fe042c4871b3efb3f51bef7cca335d0
Reviewed-on: https://go-review.googlesource.com/25355
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: Adam Langley <agl@golang.org>

ssh/terminal: consume data before checking for an error.

According to the io.Reader docs, Alex had it right the first time. (See
discussion on https://golang.org/cl/25355.)

Change-Id: Ib6fb9dfb99009e034263574e82d7e9d4828df38f
Reviewed-on: https://go-review.googlesource.com/35242
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Adam Langley <agl@golang.org>

crypto/ssh: fix parsing order for ssh.ParseDSAPrivateKey

The inline struct has the wrong order for the public and private key parts.

Change-Id: Ib3a5d6846296a2300241331a2ad398579e042ca9
Reviewed-on: https://go-review.googlesource.com/35351
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: soft code internal channel size for testing purposes

Change-Id: I2ee0ed4ba82d2d156a7896551dea04b28cdeceb0
Reviewed-on: https://go-review.googlesource.com/35184
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

ssh: make sure we execute the initial key exchange only once

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>

ssh: rationalize rekeying decisions.

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>

ssh: add debug print at the lowest level

This is a simple minded, fast print, suitable for debugging timing
sensitive issues.

Change-Id: I9ae3df5fe86f1883c1fa9265b6f7f9a98d33747e
Reviewed-on: https://go-review.googlesource.com/36054
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

ssh: reset buffered packets after sending

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>

ssh: Support multiple source-addresses, don't require IPv4 in tests.

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>

ssh: prevent double kex at connection start, 2nd try

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>

ssh/agent: fix another test to not require IPv4.

Missed a copy/paste of netPipe in change 36110.

Change-Id: I1a850dd9273d71fadc0519cf4cb2a2de6ecae4c2
Reviewed-on: https://go-review.googlesource.com/36259
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>

ssh: Add the hmac-sha2-256-etm@openssh.com algorithm

Fixes golang/go#17676

Change-Id: I96c51431b174898a6bc0f6bec7f4561d5d64819f
Reviewed-on: https://go-review.googlesource.com/35513
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: require host key checking in the ClientConfig

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.

Fixes golang/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>

ssh: handle error from prepareKeyChange.

Fixes #18850.

Change-Id: Id3ae89233f9e95ec3238462bf2ecda3e0c515f88
Reviewed-on: https://go-review.googlesource.com/36051
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

ssh: fix typo in unexported comment

Thanks to Anisse Astier (@anisse) for noticing.

Change-Id: I1c282b2bb54601cf5649e194eafd5344c70331ca
Reviewed-on: https://go-review.googlesource.com/38916
Reviewed-by: dnv aps Sn <sndnvaps@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

ssh: improve client public key authentication

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>

ssh: reject RekeyThresholds over MaxInt64

This fixes weirdness when users use int64(-1) as sentinel value.

Also, really use cipher specific default thresholds. These were added
in a59c127441a8ae2ad9b0fb300ab36a6558bba697, but weren't taking
effect. Add a test.

Fixes golang/go#19639

Change-Id: Ie9518a0ff12fded2fca35465abb427d7a9f84340
Reviewed-on: https://go-review.googlesource.com/39431
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

ssh: fix format string in client_test.go

Change-Id: I92c3916b0b5628dc2079af82202d9bfef032c708
Reviewed-on: https://go-review.googlesource.com/39430
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>

ssh: Add support for RSA keys stored in OpenSSH's new format

Adds support for parsing RSA keys in the openssh-key-v1 private key format.

Change-Id: Iacdcbaadf72413e4067d146203604fb50b780083
Reviewed-on: https://go-review.googlesource.com/35244
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Paul Querna <paul@querna.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: support forwarding of Unix domain socket connections

This commit implements OpenSSH streamlocal extension, providing the equivalent
of `ssh -L local.sock:remote.sock`.

Change-Id: Idd6287d5a5669c643132bba770c3b4194615e84d
Reviewed-on: https://go-review.googlesource.com/38614
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: support MaxAuthTries on ServerConfig

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>

ssh: set rekeying thresholds on construction

The normal handshake kicks off with a waitSession(), which guarantees
that we never attempt to send data before the first kex is completed,
but ensuring readPacketsLeft > 0 and writePacketsLeft > 0 helps
understand that thresholds can never cause spurious rekeying at the
start of a connection.

Change-Id: If5bcafcda0c7d16fd21f22c664101ac5f5b487d7
Reviewed-on: https://go-review.googlesource.com/38696
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: fix reset{Read,Write}Thresholds for initial setup

Fixes a nil pointer dereference that slipped through buildbots because
it was introduced by the last two commits.

Change-Id: Ib269e910956cd8b3b46e217b03fde1b61572260a
Reviewed-on: https://go-review.googlesource.com/40530
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/knownhosts: a parser for the OpenSSH known_hosts file format

Change-Id: I271c90ff3a6d59e2e075c785a6bdb79e4b0849fa
Reviewed-on: https://go-review.googlesource.com/40354
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

ssh/knownhosts: fix variable reuse bug in checkAddrs

Consider the following code:
	var p *int
	a := []int{0, 1, 2, 3}
	for _, i := range a {
		if i == 1 {
			p = &i
		}
	}
	fmt.Println(*p) // Prints 3

This prints 3 because the variable i is the exact same variable across
all iterations of the loop. When the address is taken for some specific
iteration, the user's intent is to capture the value of i at that
given loop, but instead the value of i in the last loop is what remains.

A bug this sort occurs in the check logic since the address of the
knownKey is taken, but is changed upon subsequent iterations of the
loop (which happens when there are multiple lines).

Change-Id: Ic626778cdcde3968dcff4fa5e7206274957dcb04
Reviewed-on: https://go-review.googlesource.com/40937
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/knownhosts: support hashed hostnames

Change-Id: I855a6542a2eb2ae1d223f03892c0f19da81a4f8d
Reviewed-on: https://go-review.googlesource.com/40532
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

ssh/knownhosts: add file + linenumber for parse errors

Change-Id: Iddcb145ecd8a6b51c72ad3d77b242975baf4a5cf
Reviewed-on: https://go-review.googlesource.com/41210
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Sam Whited <sam@samwhited.com>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>

ssh/terminal: implement missing functions for Solaris/OmniOS

    terminal.MakeRaw
    terminal.Restore
    terminal.GetState
    terminal.GetSize

Fixes golang/go#20062

Change-Id: I9ccf194215998c5b80dbedc4f248b481f0ca57a6
Reviewed-on: https://go-review.googlesource.com/41297
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

ssh/knownhosts: add IsHostAuthority.

This is a breaking change.

This adds a new hostkey callback which takes the hostname field
restrictions into account when validating host certificates.

Prior to this, a known_hosts file with the following entry

  @cert-authority *.example.com ssh-rsa <example.com public key>

would, when passed to knownhosts.New() generate an ssh.HostKeyCallback
that would accept all host certificates signed by the example.com public
key, no matter what host the client was connecting to.

After this change, that known_hosts entry can only be used to validate
host certificates presented when connecting to hosts under *.example.com

This also renames IsAuthority to IsUserAuthority to make its intended
purpose more clear.

Change-Id: I7188a53fdd40a8c0bc21983105317b3498f567bb
Reviewed-on: https://go-review.googlesource.com/41751
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/knownhosts: test coverage for IsHostAuthority

Change-Id: Iad24fed7cec998e02620ec0eb61658786156ba41
Reviewed-on: https://go-review.googlesource.com/42530
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

crypto/ssh: fix tests on Go 1.7 on OpenBSD and Windows

Dialing the 0.0.0.0 address (as returned by net.Addr().String() for a
net.Listen("tcp", ":1") address) is not yet guaranteed to work. It's
currently OS-dependent.

For some reason it works on Go 1.8+, but it hasn't yet been defined to
work reliably.

Fix the tests for now (since we need to support older Go releases),
even if this might work in the future.

Updates golang/go#18806

Change-Id: I2f0476b1d4f2673ab64ffedfa733f2d92fceb6ff
Reviewed-on: https://go-review.googlesource.com/42496
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>

ssh: change the local copy of the ServerConfig passed to NewServerConn

Otherwise callers are forced to serialize access to the ServerConfig.

Change-Id: Id36f4d2877ea28b18447ef777d3839b21136c22f
Reviewed-on: https://go-review.googlesource.com/42821
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

x/crypto/ssh: fix host certificate principal evaluation to check for hostname only

SSH host certificates are expected to contain hostnames only,
not "host:port" format.

This change allows Go clients to connect to OpenSSH servers that
use host certificates.

Note, this change will break any clients that use ssh.NewClientConn()
with an `addr` that is not in `host:port` format (they will see a
"missing port in address" error).

Fixes bug 20273.

Change-Id: I5a306c6b7b419a737e1f0f9c5ca8c585e21a45a4
Reviewed-on: https://go-review.googlesource.com/43475
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh: fixing a small typo in connection.go

Change-Id: Iffbed7e16a8bb32c5ff7c393f3b6ad7dcffc69ac
Reviewed-on: https://go-review.googlesource.com/44340
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>

ssh: add ParsePrivateKeysWithPassphrase

ssh package doesn't provide way to parse private keys with passphrase.

Fixes golang/go#18692

Change-Id: Ic139f11b6dfe7ef61690d6125e0673d50a48db16
Reviewed-on: https://go-review.googlesource.com/36079
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>

ssh: clarify intended use of Permissions.

The Permissions struct should be used to pass information from
authentication callback to server application.

Fixes golang/go#20094.

Change-Id: I5542b657d053452327260707a24925286546bfdd
Reviewed-on: https://go-review.googlesource.com/45311
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

all: gofmt ./...

Change-Id: I8ffee4dc712091e424b83a9f5a3cc2a6724abefc
Reviewed-on: https://go-review.googlesource.com/46050
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.