591 Commits
Author SHA1 Message Date
Nicola Murino 86efde54dc ssh: reject unexpected message types on established channels
ch.msg is only read while the channel open or a channel request with a
reply is pending, so anything the default arm of channel.handlePacket
delivered to it was never consumed. The blocking send there let a
misbehaving peer fill the buffer with well-formed but unexpected message
types carrying a valid channel id and stall the mux read loop,
deadlocking the whole connection.

No conforming peer sends such messages during the connection protocol.
Treat them as a protocol error and tear the connection down, as
handleUnknownChannelPacket already does for the same messages when the
channel id is not in use.

Fixes CVE-2026-56855
Fixes golang/go#81317

Change-Id: I87420dfe68fcb62a17df4b47dc5ffb6ccd72ba26
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/826524
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Neal Patel <nealpatel@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
2026-09-02 11:02:47 -07:00
Nicola Murino a6cdac6084 ssh: drop traffic on undecided channels
A channel in the mux's chanList is not usable until it is established:
an outbound channel has no confirmed remote id until the peer's open
confirmation, and an inbound channel is not serviced by the application
until it is accepted. handlePacket processed any channel message on it,
so a misbehaving peer could flood channel requests and block the mux
read loop on the send to incomingRequests, deadlocking the connection,
or close an outbound channel before confirming it, making the victim
tear down a half-initialized channel and emit a close for remote id 0,
an unrelated channel of the peer.

No such packet can be legitimate: the peer learns an inbound channel's
local id only from the confirmation we have not sent yet, and on an
outbound channel RFC 4254 lets it answer the open request only with a
confirmation or a failure.

Add an established flag, set when the channel becomes usable: for an
outbound channel when the open response is received, for an inbound
channel by Accept before the confirmation is sent. Until then
handlePacket drops every packet other than the open response. The flag
is separate from decided, which Reject also sets: a rejected channel is
decided but must never carry traffic.

Fixes CVE-2026-78662
Fixes golang/go#81316

Change-Id: Ib0983bb216a49808a2db1f4a4d92ee9fe38a3c51
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/826504
Auto-Submit: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
2026-09-02 10:48:31 -07:00
Nicola Murino 39dc44e69c ssh: don't skip the source-address critical option in CheckCert
CertChecker.CheckCert ignored source-address on the assumption that
serverAuthenticate would enforce it, but that only happens in the
server-side user authentication path. Nothing enforced it in the host key
path, so CheckHostKey accepted CA-signed host certificates carrying
source-address regardless of the server's address, and the same applied to
applications calling CheckCert directly.

Drop the special case: every critical option must be listed in
SupportedCriticalOptions, host and user certificates alike. Authenticate,
the only path where serverAuthenticate does enforce source-address, checks
against a copy of the CertChecker with that option appended.

Fixes golang/go#80872

Change-Id: I3c3554b71ea2a4ce4b17696a9596b973b7d74b00
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/816840
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
2026-08-30 20:04:51 -07:00
Nicola Murino 71488c48c2 ssh/knownhosts: compare only public key portions for revocation
Revocation matching compared full marshaled key blobs, so a @revoked
line containing a plain host key did not revoke a certificate
certifying that key: when the server presented such a certificate,
IsRevoked only matched the whole certificate blob or the CA key, and
verification succeeded even though the underlying host key was revoked.

OpenSSH's check_key_not_revoked compares keys with sshkey_equal_public,
which considers only the public portions and explicitly allows
comparisons between certificates and plain keys. Match that behavior by
normalizing both @revoked entries and lookups to the underlying public
key: for a certificate, the certified key rather than the certificate
blob. This also makes a @revoked line containing a certificate revoke
the plain key it certifies

Change-Id: I3029d59e68fb01f2340763e2eae25cac23c6193d
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/816841
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2026-08-23 06:53:19 -07:00
Daniel McCarney 82adefa711 ssh: synchronize unexpected response test
Wait for an ordered channel request after sending the unsolicited
response flood. This ensures that the client mux has discarded the
entire flood before SendRequest opens its response gate, avoiding
confusion between an in-flight unsolicited response and the expected
reply.

This fixes TestChannelUnexpectedResponsesDiscarded from flaking, where
otherwise running 1000000 iterations of the test with GOMAXPROCS=16
produced ~15 distinct failures of the form  "expected success reply".

Change-Id: Ic1125c1f44bd919a5cf32b889d331bbc94c06c25
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/817140
Reviewed-by: Neal Patel <neal@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
2026-08-19 13:42:46 -07:00
Damien Miller 593c81af8a ssh: correctly ignore pre-banner lines
Reading the peer's benner in readVersion() was incorrectly counting
lines sent by the server before the actual `SSH-2.0` banner against
the line limit for the banner itself.

This is incorrect per RFC 4253 section 4.2, where the limit of 255
characters is written to apply only to the `SSH-` banner string
itself.

This instead limits pre-banner lines to 255 characters each but
instead allows a maximum of 1024 lines.

Change-Id: Ief014bb3413e26e8d2077d3eb204ceaea9dc9444
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/811320
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
2026-08-17 08:34:06 -07:00
Nicola Murino b07833c067 ssh: return window credit for discarded extended data
handleData subtracts the length of every data packet from the receive
window, but extended data with a type code other than
SSH2_EXTENDED_DATA_STDERR is then silently discarded and, since no API
exists to read it, its window credit was never returned: adjustWindow
only runs as a side effect of reading a stream. A peer sending
extended data with an unknown type code could therefore permanently
shrink the channel window, down to a complete stall of the channel.

Credit the discarded bytes back through adjustWindow. adjustWindow
returns io.EOF if the local side has already sent a channel close;
ignore it like ReadExtended does, since an error returned here would
terminate the mux read loop and tear down the whole connection. For
comparison, OpenSSH rejects extended data with unexpected type codes
outright and never buffers it, so the dropped bytes are simply never
credited back to the misbehaving peer.

Fixes golang/go#80333

Change-Id: Ia311ce3ad008e7c89ce9752f1937c91374a4e74f
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/802901
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
2026-08-03 14:00:09 -07:00
Brad Fitzpatrick 999d053994 ssh: fix parsing of GSSAPI payloads offering multiple mechanisms
parseGSSAPIPayload overwrote the remaining payload with the trailing
bytes returned by asn1.Unmarshal after decoding each OID, so any
request offering more than one mechanism failed to parse after the
first one. Track the remaining payload across iterations instead, and
be strict: reject payloads with trailing bytes after an OID or after
the mechanism list.

Add a test with a two-mechanism payload (Kerberos V5 followed by
SPNEGO) that failed before this change.

Change-Id: If0984efcf103da76323d5d03ad7443d8ec9d3aed
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/803180
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-07-23 08:19:57 -07:00
Nicola Murino 90f76b8ffe ssh: reject certificate signature keys before recursing
parseCert parsed a certificate's SignatureKey by calling ParsePublicKey
again, and only afterwards rejected a certificate used as a signing key.
For a chain of nested certificates this recursed once per level, so a key
with enough nesting exhausted the goroutine stack and crashed with a fatal
stack overflow that recover cannot catch.

Reject the signature key by its declared algorithm before parsing it, which
bounds parsing.

Fixes golang/go#80414

Change-Id: Id51401aae15bfbec006c03ab97635a3b68bd6058
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/802145
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2026-07-22 10:37:06 -07:00
database64128 b53964a1ca ssh: permit empty but non-nil HostKeyAlgorithms, KeyExchanges, Ciphers, MACs
The documentation of ClientConfig.HostKeyAlgorithms says:

  If empty, a reasonable default is used.

However, if HostKeyAlgorithms is empty but non-nil, the reasonable
default won't be applied, and the SSH handshake will fail. Fix this by
changing the nil check to a length check.

While here, do the same for KeyExchanges, Ciphers, MACs in Config.

Change-Id: I1b3d2d2159f3c7d57a9a690ad04f1f6818b39a5c
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/800080
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
2026-07-22 10:37:01 -07:00
Nicola Murino 626e40fc98 ssh: drain stderr on forwarded TCP and Unix channels
tcpListener.Accept, unixListener.Accept, Client.dial and
Client.dialStreamLocal only read the main stream of the channels they
return. Data sent by the peer on the extended (stderr) stream
accumulates in the extPending buffer and its window credit is only
returned by ReadExtended, which is never called. Since the window is
shared between the two streams, a misbehaving peer can pin up to 2 MiB
per channel and eventually stall it; well-behaved peers never send
stderr on these channel types.

Drain the stderr stream into io.Discard, mirroring CL 783720 which
fixed the same issue in the agent forwarders.

The drain takes precedence over reading Stderr through a type assertion
on the returned net.Conn, which was never a supported use. For
comparison, OpenSSH refuses extended data on these channel types and
drops the payload without buffering it.

Updates golang/go#80333

Change-Id: I4f1445eee9ce1b56ca62cec342812d63cfdd3e13
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/802900
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-07-22 10:34:36 -07:00
Brad Fitzpatrick f2135b814c all: clean up minor issues found by staticcheck
Use bytes.Equal instead of comparing bytes.Compare results to zero,
avoid Yoda conditions, drop a redundant zero slice capacity, use
strings.ReplaceAll, build acme error strings with a strings.Builder,
and lowercase an error string in ocsp. No behavior changes.

Change-Id: I5d2b3fa5c241adc75e61be0f2d2d7af1eb9e8d66
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/803200
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
2026-07-21 14:30:59 -07:00
Akira ff03dafdb0 ssh: report verified signature format to callback
VerifiedPublicKeyCallback currently receives the public-key algorithm
declared in the outer authentication request. For RSA authentication,
that value can differ from the format of the signature that was
successfully verified because compatible RSA algorithm combinations are
accepted.

This can cause post-verification policy or audit decisions to use the
request algorithm instead of the signature format actually verified.

Pass sig.Format to VerifiedPublicKeyCallback so signatureAlgorithm
identifies the successfully verified signature format. Clarify the
callback documentation and add a regression test covering compatible
but different RSA request algorithms and signature formats.

Fixes golang/go#80411

Change-Id: Ib405378d75367a90536e0814c9d26d4dec9aaa25
GitHub-Last-Rev: 1a4acd4e639361be1f2948a4aef1013436a75ecf
GitHub-Pull-Request: golang/crypto#364
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/800740
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-07-17 15:41:46 -07:00
Nicola Murino 10b54ffa51 ssh: raise the RSA modulus limit to 16384 bits
the fix for CVE-2026-39829 capped the accepted RSA modulus at 8192 bits,
matching crypto/tls, to bound the CPU cost of verifying an
attacker-supplied key. That rejected keys OpenSSH generates: ssh-keygen
allows -b up to 16384. Raise the limit to 16384 so those keys interoperate,
both in the public key parser and the OpenSSH private key parser, and
correspondingly raise the prime-factor bound to 8192 bits (each prime is
about half the modulus). The agent's checkRSAKeyParams is updated to match.

Verifying a 16384-bit key costs roughly 3x an 8192-bit one.

Fixes golang/go#80075

Change-Id: Ib08ddd7b48552e692c48dbdf105bbc8579120bbc
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/795421
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2026-07-13 20:33:21 -07:00
Nicholas S. Husin 0f039f3a4a ssh: sanitize various error messages
Similar to CL 780180, sanitize other error messages that are controlled
by the remote peer, to avoid potential injection of control characters.

Also remove sanitizeString usage as it is not strictly needed to escape
error messages safely, and for consistency with how we sanitize other
error messages throughout the codebase.

BannerDisplayStderr is left unsanitized for now, as it is one instance
where we might actually want control characters for valid reasons (see
golang/go#80302).

Updates golang/go#80302
Fixes golang/go#80301

Change-Id: Ie40f4ac2f76afed740dceb548a4b0fa96a6a6964
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/798420
Auto-Submit: Nicholas Husin <nsh@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Nicholas Husin <husin@google.com>
2026-07-13 12:23:17 -07:00
Nicola Murino b27e1504e3 ssh: don't close the connection on an unparseable public key
In the server's publickey authentication path, a public key blob that
ParsePublicKey could not decode caused serverAuthenticate to return an
error, tearing down the whole connection. A key we cannot parse is part
of an individual authentication attempt, not a transport-level framing
error, so it must fail only that attempt. This lets the client fall
back to other keys or authentication methods.

Updates golang/go#80075

Change-Id: I8d882d08bb50eb33e0054d9d9c2bd9e0b3039443
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/795420
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
2026-07-13 12:20:29 -07:00
Nicola Murino c1a3b97d70 ssh: document and test source-address behavior for non-IP transports
The source-address critical option is a comma-separated list of IP
addresses and CIDR blocks, so it can never match a connection whose
remote address is not IP-based, such as a Unix domain socket.

Change-Id: I45e09fe5c8c13922cb0735278acf33320857322c
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797042
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-07-12 08:19:47 -07:00
Nicola Murino b9229ecc70 ssh: reject a present but empty source-address critical option
checkSourceAddressCriticalOption treated a source-address critical
option with an empty value as if the option was absent, allowing
authentication from any address. A callback (or certificate authority)
building the value dynamically from an empty allowlist would intend to
deny all sources, not to lift the restriction. This also contradicts
the documented contract that the option is validated whenever it is
present, and differs from OpenSSH, where an empty address list matches
nothing.

Change-Id: I1c0de27cbcab8817a7ff446019c25cd19957a7d7
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797041
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2026-07-12 08:19:43 -07:00
Nicola Murino e557b08ec2 ssh: enforce the source-address critical option for all auth callbacks
CVE-2026-46595 extended source-address validation, historically applied
only to the Permissions returned by PublicKeyCallback, to the ones
returned by VerifiedPublicKeyCallback. The documented contract of
Permissions.CriticalOptions does not restrict enforcement to a specific
authentication method, so move the check to a single point at the end of
each authentication attempt, where it covers the Permissions returned by
any callback (password, keyboard-interactive, none and GSSAPI included).

The check at public key cache insertion time is kept: it remains
authoritative for the Permissions returned by PublicKeyCallback, which
VerifiedPublicKeyCallback may replace before the check at the end of the
authentication attempt runs and which are not re-checked on partial
success. It also still makes public key queries fail before the client
produces a signature when PublicKeyCallback supplies the restriction.

Fixes CVE-2026-56854
Fixes golang/go#80213

Change-Id: Ibd612a8e4240bd710e33754f3ceb95bb29169d9a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797040
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2026-07-12 08:19:37 -07:00
Nicola Murino 4044611da8 ssh: fix data race in the doClientServerAuth test helper
The helper returned the auth errors collected via AuthLogCallback
without waiting for the server goroutine that appends to them. Close
the client side of the connection on handshake failure, to unblock a
server still reading from it, and join the server goroutine before
returning.

Change-Id: Ibe8c3b236491769f3433f7457d4f2b9a07a65be0
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797043
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2026-07-12 08:19:29 -07:00
Nicola Murino e8ad5cb7aa ssh: initialize new channels fully before adding them to chanList
newChannel published the channel into the mux's chanList (via
chanList.add) before assigning its localId, and openChannel/Accept set
maxIncomingPayload only after newChannel returned. For an outbound
channel, whose newChannel runs on an application goroutine, the mux
loop could observe the channel through getChan before those fields
were initialized, reading a zero localId or maxIncomingPayload with no
happens-before relationship.

Assign localId inside add while the chanList lock is held, and set
maxIncomingPayload (always channelMaxPacket) in newChannel itself, so
a channel returned by getChan is always fully initialized.

Change-Id: Ib54a5d10dcb5c1ad8ed4dabe5d8d2790778be298
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797061
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-07-12 08:19:12 -07:00
Nicola Murino fe1af3a418 ssh: make channel.close idempotent
channel.close closes the internal Go channels (msg, incomingRequests);
calling it twice panics with "close of closed channel". Both current
callers run on the mux loop and are serialized, so the panic is not
reachable today. Harden close by guarding it with sync.Once, so that
the connection does not become fragile to any future bug that reaches
close twice.

Change-Id: I5d8d948367538ea5561724033c4ee4daa668e221
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797060
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-07-12 08:19:06 -07:00
Nicola Murino 7626c50256 ssh: verify declared key type matches decoded key in authorized_keys
ParseAuthorizedKey and ParseKnownHosts previously ignored the key type
field (e.g. "ssh-rsa") in each entry, relying solely on the type
information embedded within the base64-encoded key blob.

For ParseAuthorizedKey this also caused a single-token option to be
silently dropped: a line such as "restrict <key>" (with the key type
omitted) was parsed as an unrestricted key, because the option token
landed in the key type position and was discarded together with its
meaning. The same happens for no-pty, no-port-forwarding and the other
single-word options. OpenSSH's sshkey_read rejects such lines outright,
as the option token cannot be read as a key type.

OpenSSH's sshkey_read also explicitly verifies that the key type
declared in the text matches the type of the parsed key, returning
SSH_ERR_KEY_TYPE_MISMATCH if they differ.

This change adds, in both functions, a check that the declared key type
matches the decoded key's type, returning an error for malformed lines
where they diverge. This mirrors the fix already applied to the
ssh/knownhosts package in CL 782427.

Change-Id: I9163108d964de4be8a415fe0efd68e6b71f77990
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/792840
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-06-30 10:24:32 -07:00
Nicola Murino 0471e7969e ssh/agent: enforce strict limits on DSA key parameters
The parseDSAKey function constructed a *dsa.PrivateKey directly from the
add-identity request without validating the key parameters. Unlike DSA
certificates, whose parameters are checked by the ssh package when the
certificate's public key is parsed, raw DSA keys added to the agent were
not validated at all.

Align the raw DSA key parsing with the validation already performed by
the main ssh package.

Fixes golang/go#79725

Change-Id: I537cc2175d35c19848c90c68739cf94ba7b50e10
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/795422
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-06-30 10:06:28 -07:00
Neal Patel 6435c37ab6 ssh: sanitize client disconnect messages
A malicious peer can send control characters
and escape sequences in SSH_MSG_DISCONNECT;
strip these characters and %q escape it.

Change-Id: I266f78630a7a816a79ce4dda568ce57d7c3f6fb0
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/780180
Commit-Queue: Nicola Murino <nicola.murino@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
2026-06-29 10:23:17 -07:00
Nicola Murino 7d695da948 ssh/agent: drain channel stderr in agent forwarders
ForwardToAgent and ForwardToRemote only read the main stream of the
auth-agent@openssh.com channels they accept. If a peer sends data on
the channel's extended (stderr) stream the bytes accumulate in the
client-side extPending buffer and the receive window is never
replenished, because the window is only adjusted as a side effect of
ReadExtended. That can pin up to channelWindowSize (2 MiB) of memory
per channel and silently stalls any stderr traffic once the window is
exhausted.

The auth-agent protocol does not use stderr, so a well-behaved peer
never sends anything on it. To stay tolerant of misbehaving peers
without leaving the channel half-stuck, drain the stderr stream into
io.Discard, mirroring the existing DiscardRequests pattern. The
goroutine exits when the channel is closed because Stderr().Read
returns io.EOF.

Add a regression test that opens an agent-forwarding channel and
writes more than the default window on the stderr stream from the
server side. Without the fix the write blocks once the remote window
is exhausted; with the fix the bytes are drained and the agent stream
remains usable.

Change-Id: Iadf8ea6ca726c058421bbc39f92e0100579fda17
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783720
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2026-06-29 06:36:26 -07:00
povcfe e2ffffe738 ssh: reject incomplete gssapi-with-mic configurations
Make the runtime gssapi-with-mic guard match the existing
configuration and method advertisement checks.

An incomplete GSSAPIWithMICConfig can be treated as unavailable when
building the advertised auth method list, while still remaining
reachable from the runtime auth dispatcher. Treat incomplete
configurations as not configured.

This change introduces a single internal completeness check for
GSSAPIWithMICConfig and uses it for the startup authentication
validation, the runtime gssapi-with-mic dispatch guard, and the
advertised authentication method list.

The change also adds a regression test. The test configures a server
with a normal PasswordCallback, a GSSAPIWithMICConfig with Server set,
and AllowLogin intentionally unset. It then uses a custom client auth
method that explicitly sends a USERAUTH_REQUEST with Method set to
gssapi-with-mic even though the server does not advertise that method,
and verifies that authentication fails cleanly with
"ssh: gssapi-with-mic auth not configured".

No golang/go issue reference is available yet.

Change-Id: I9a0c965d3a56192bd68309aa41e2c1f91952036c
GitHub-Last-Rev: 0267bda8e15e7c258ba3b92cd54f0941534c5fc9
GitHub-Pull-Request: golang/crypto#345
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/773460
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-06-02 00:25:39 -07:00
Mike Nolta 60e158ad30 ssh/test: isolate CLI tests from user SSH config and agent
Pass -F none and -o IdentityAgent=none to ssh(1) invocations in the
CLI tests. Without these flags, a user's ~/.ssh/config can enable
Control{Master,Persist}, which causes the ssh process to fork a
background daemon that holds stdout/stderr pipes open, preventing
CombinedOutput from ever returning. The agent flag prevents keys loaded
in the user's SSH agent from being offered to the server before the test
key, which would exhaust MaxAuthTries and cause a disconnect.

Fixes golang/go#79411

Change-Id: Id5b073e6df661006183d000928d7ef6e6096b933
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/777900
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-06-02 00:25:33 -07:00
Nicola Murino 1b77d23dc8 ssh/knownhosts: reject lines with multiple or unknown markers
Previously, parseLine would accept lines with multiple markers (e.g.,
"@cert-authority @revoked ...") or unknown markers (e.g., "@unknown ...").
In these cases, the second or unknown marker was incorrectly parsed as
the hostname. This often resulted in confusing downstream errors (like
"illegal base64 data") because field positions were shifted.

OpenSSH's hostfile.c strictly enforces a limit of one marker per line
and validates that markers must be exactly "@cert-authority" or
"@revoked".

This change adds a check to ensure that the parsed hostname does not
start with '@'. This effectively catches both multiple markers (where
the second marker becomes the host) and unknown markers (which are not
consumed as markers and thus become the host), enforcing strict
compliance with the OpenSSH format.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: I13bb04d1f2610483ad9c6d1020e5100b6feded90
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782428
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2026-06-02 00:24:36 -07:00
Nicola Murino 3872a2bc69 ssh/knownhosts: verify declared key type matches decoded key
The parseLine function previously ignored the key type field (e.g.,
"ssh-rsa") in known_hosts entries, relying solely on the type
information embedded within the base64-encoded key blob.

OpenSSH's sshkey_read implementation explicitly verifies that the key
type declared in the text matches the type of the parsed key, returning
SSH_ERR_KEY_TYPE_MISMATCH if they differ.

This change adds a check to ensure the declared key type matches
key.Type(), returning an error for malformed lines where they diverge.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: Id4f35c74055f5691088273630b50cdd02c81bfe9
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782427
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-06-02 00:24:24 -07:00
Nicola Murino 9f72eccdf6 ssh/knownhosts: treat only ASCII space and tab as whitespace
The previous implementation used bytes.TrimSpace, which strips all
Unicode whitespace categories (e.g., non-breaking spaces). However,
OpenSSH's known_hosts parser (hostfile.c) strictly treats only ASCII
space (0x20) and horizontal tab (0x09) as separators.

This discrepancy meant the Go parser might interpret fields differently
than OpenSSH, potentially treating parts of a key or hostname as
separators if they contained Unicode whitespace.

This change replaces bytes.TrimSpace with a local trimSpace helper that
only trims " \t", ensuring parsing behavior consistent with the
reference implementation.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: Ia536889636de2c167d2507c01e3f1b7c033c9a8f
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782426
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-06-02 00:24:19 -07:00
Nicola Murino 8f405a4109 ssh: validate ECDSA curve matches expected algorithm
Previously, parseECDSA determined the curve purely based on the key
blob content, ignoring the algorithm identifier passed to parsePubKey.

This allowed a mismatch where a key could be encoded with an algorithm
type of "ecdsa-sha2-nistp256" but contain a NIST P-384 or P-521 curve.
The parser would succeed, returning a key with a type different from
the one indicated by the caller/wire format.

This change updates parseECDSA to accept the expected algorithm type
and verify that it matches the curve specified in the key data. This
matches the behavior of OpenSSH's ssh_ecdsa_deserialize_public in
ssh-ecdsa.c, which rejects a curve identifier that does not correspond
to the key algorithm name.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: I9c748be948cca65e2f41089bb7510466d3bb316a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782425
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2026-06-02 00:24:14 -07:00
Nicola Murino bb41b3dbe6 ssh: improve DH GEX group selection using PreferredBits
Previously, the server selected the Diffie-Hellman group based solely on
the MaxBits value provided by the client. This resulted in suboptimal
modulus selection, often ignoring the client's PreferredBits or selecting
a larger-than-necessary group.

This change implements a "best fit" selection algorithm similar to
OpenSSH's choose_dh logic. It attempts to find the smallest available
group larger than or equal to the client's PreferredBits, falling back to
the largest available group within the accepted range if no group above
the preference is available.

Additionally, this commit caches the parsed Oakley groups using
sync.OnceValue, avoiding repeated big.Int parsing on every handshake
while keeping the cost out of package initialization.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: Idfa81bbcf354a7fb7b541cb4bbeb6e4a0181398a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782424
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
2026-06-02 00:23:03 -07:00
Nicola Murino e04e721997 ssh/agent: validate ed25519 private key length in Add
insertKey and insertCert assume an ed25519.PrivateKey is exactly
64 bytes long: a 32-byte seed followed by a 32-byte public key.
The expression []byte(k)[32:] panics with "slice bounds out of
range" when a shorter slice is passed, which is reachable through
the exported Add API and crashes the caller process.

Reject ed25519 keys whose length is not ed25519.PrivateKeySize in
all four branches (value and pointer variants of insertKey and
insertCert), using the same error wording already used by
parseEd25519Cert in server.go.

This issue was found during a security audit by NCC Group
Cryptography Services, sponsored by Teleport, and was assessed
and is being fixed as a non-security bug.

Change-Id: I4a053835588aad4c3482fe1ca8045542cd960500
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782423
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-06-02 00:22:54 -07:00
Nicola Murino b315afd69a ssh: limit bcrypt KDF rounds in OpenSSH private key decryption
passphraseProtectedOpenSSHKey decodes the bcrypt round count from
the key file and passes it directly to bcrypt_pbkdf.Key, whose
running time is linear in that count. Files specifying very large
round values cause the decryption to consume excessive CPU time:
a uint32 maximum is several months of work on commodity hardware.

Cap the accepted round count at 2048 (128x the default of 16, a
few seconds of CPU on a modern core). OpenSSH itself does not
impose an upper bound, but accepting arbitrary values turns key
loading into a resource-exhaustion footgun for any code that
processes files supplied by end users.

This issue was found during a security audit by NCC Group
Cryptography Services, sponsored by Teleport, and was assessed
and is being fixed as a non-security bug.

Change-Id: I01112bdf1b484ae4fab5dc8841d1a272f112df74
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782422
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-06-02 00:22:48 -07:00
Nicola Murino d4a85f4749 ssh/agent: limit RSA key size on Add requests
parseRSAKey and parseRSACert build an rsa.PrivateKey directly from
the Add request body and then call priv.Precompute(). The CRT
coefficient recomputation in Precompute() is cubic in the size of
the prime factors, so accepting RSA keys with arbitrary modulus
and prime sizes can consume excessive CPU resources during a
single Add operation.

Add a checkRSAKeyParams helper that enforces the same bounds as
parseRSA in the ssh package (modulus <= 8192 bits, exponent <= 24
bits, exponent >= 3 and odd) plus a cap of 4096 bits on each prime
factor, and call it from both parseRSAKey and parseRSACert before
priv.Precompute().

The previous inline check on the exponent (BitLen > 30) is
subsumed by the new helper, which tightens it to BitLen > 24 for
consistency with parseRSA.

Change-Id: I6f0bbbfdc1db13d62a1f85e1d0e6ae517f000443
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782421
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-06-02 00:22:42 -07:00
Nicola Murino 6f39c5226a ssh: limit RSA key size in OpenSSH private key parsing
parseOpenSSHPrivateKey builds an rsa.PrivateKey directly from the
unmarshalled OpenSSH key blob, bypassing parseRSA and its modulus
limit. Validate() and Precompute() then perform several modular
exponentiations whose cost grows with the size of the prime factors;
in particular Precompute() recomputes the CRT coefficient as
q^(p-2) mod p, which is cubic in |p|. A maliciously crafted key with
oversized N and P can keep the CPU busy for hours or days during a
single load.

Mirror the parseRSA validation here: cap the modulus at 8192 bits
(the same limit enforced by crypto/tls), reject exponents larger than
24 bits, and reject invalid exponent values (< 3 or even). In addition,
bound each prime factor at 4096 bits to prevent the CRT computation
from becoming the dominant cost.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport, and was assessed and is being fixed as
a non-security bug.

Change-Id: Ia5991f25dd41a22eddd7cf63a8fc5106de9e9663
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/782420
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-06-02 00:22:37 -07:00
Nicola Murino 4c4d20b72c ssh: fix spinloop in mux SendRequest drain on closed channel
The drain loop added in 4e7a738 ("ssh: fix deadlock on unexpected
global responses") receives from m.globalResponses without checking
the comma-ok flag. Once mux.loop closes globalResponses on connection
teardown, the loop spins forever on the zero value, pinning a CPU
core for every concurrent SendRequest caller.

Use the comma-ok idiom to detect the closed channel and break out.

Change-Id: Icf0a7419fe5b206b7a4f70429fa9f25d0b5af731
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783681
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Neal Patel <neal@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
2026-05-28 10:16:30 -07:00
Nicola Murino e3e62d9601 ssh: fix spinloop in channel SendRequest drain on closed channel
The drain loop added in 3c7c869 ("ssh: fix deadlock on unexpected
channel responses") receives from ch.msg without checking the comma-ok
flag. Once ch.msg is closed by channel.close(), receives succeed
immediately with the zero value, so the default arm is never taken
and the loop spins forever, pinning a CPU core for every concurrent
SendRequest caller on a torn-down channel.

Use the comma-ok idiom to detect the closed channel and break out.

Fixes golang/go#79658

Change-Id: Id80aa5d8429550c7a68e8759aed9ffba3aabb006
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783680
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Neal Patel <neal@golang.org>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
2026-05-28 10:16:26 -07:00
Nicola Murino 5adb68b3e6 ssh: cap total userauth attempts per server connection
serverAuthenticate only bounded real failures via MaxAuthTries.
PartialSuccessError responses and the publickey isQuery short-circuit
both kept the loop running without incrementing authFailures, so a
client could keep the server processing SSH_MSG_USERAUTH_REQUEST
messages indefinitely on a single connection.

Add an unconditional cap, maxAuthServerAttempts = 128, on the total
number of userauth requests handled per connection. The counter is
incremented at the top of the loop before method dispatch, so every
method and every isQuery / partial-success path counts. When the cap
is exceeded the server sends SSH_MSG_DISCONNECT with reason 2 ("too
many authentication attempts"), mirroring the MaxAuthTries handling.
The bound is well below OpenSSH's hard cap of 1024 but above any
realistic multi-step auth flow.

Change-Id: I56779fc55cd00ddfd32ec938f8de3a49be0145dc
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781903
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2026-05-27 09:40:09 -07:00
Neal Patel 9beb694f97 ssh: prevent malformed exit-status panic
A malicious server can crash any client by sending
a truncated exit-status payload, which triggers a
panic in binary.BigEndian.Uint32.

Change-Id: Iff4fe182b40210c99d46562f6c3b5ecf38bb98b4
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/780181
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: David Chase <drchase@google.com>
2026-05-25 19:49:21 -07:00
Nicola Murino e5306b2f3e ssh/agent: support parallel signing using request pipelining
Make NewClient automatically pipeline concurrent requests over its
connection when the supplied io.ReadWriter also implements io.Closer.
In this mode the client writes requests to the wire as soon as the
write path is available and dispatches responses back to callers in
FIFO order via a dedicated reader goroutine, instead of fully
serializing each call. Up to 32 requests may be in flight on a single
connection.

This lets an agent that load-balances signing across multiple backend
devices keep several of them busy concurrently without forcing callers
to maintain their own connection pool. Aggregate throughput scales
roughly linearly with the number of backend devices up to the
in-flight cap; the protocol's in-order response requirement means
slow requests still delay subsequent ones on the same connection,
which is unchanged.

The pipelined path requires io.Closer because, on a Write error, the
background reader goroutine must be unblocked by closing the
underlying connection; otherwise it would remain parked forever
waiting for a response that will never arrive, leaking the goroutine
and desynchronising the FIFO routing of responses for subsequent
successful writes. When the supplied transport does not implement
io.Closer, NewClient falls back to the previous fully-serialized
behavior: a single in-flight call at a time, with no background
goroutine.

Fixes golang/go#78473

Change-Id: Icf14e5e8ca897506d68fb32c14fc72c774cd97b2
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/768483
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-05-21 19:39:50 -07:00
dingandCyrus Katrak d2fe592df1 ssh: add openssh controlmaster socket support
Adds support for establishing SSH sessions over an existing "ControlMaster" [unix domain] socket in proxy mode.

Details of the protocol can be found here: https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.mux
More details about ControlMaster sockets can be found here: https://linux.die.net/man/5/ssh_config

Fixes golang/go#32958

Co-authored-by: Cyrus Katrak <ckatrak@slack-corp.com>
Change-Id: Ia3ae8893262f5060ed3fadfcbe97619c9659145b
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/733040
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
2026-05-21 19:39:36 -07:00
Nicola Murino d59570d1a3 ssh: add AuthCallback to ClientConfig
Add ClientAuthCallback, a hook invoked before each authentication
attempt that allows the client to dynamically select an auth method
based on server capabilities, partial successes, or previous failures.

clientAuthenticate tracks partial successes in a slice and passes
them to AuthCallback via ClientAuthContext.PartialSuccessMethods,
along with the list of failed methods and currently allowed methods.
When AuthCallback returns a non-nil AuthMethod, it is used for the
next attempt, bypassing the static findNext selection. When it
returns (nil, nil), the static config.Auth selection proceeds as
before. If AuthCallback returns (nil, error) the handshake aborts
immediately with that error.

To bound resource use when AuthCallback keeps supplying methods, the
client caps the total number of authentication attempts (failures and
partial successes combined) at 64; exceeding the cap aborts the
handshake with an error.

Fixes golang/go#76146

Change-Id: I0d02bea7b9dd724e95e5d9d49d85306666c0df7a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/717140
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-05-21 19:36:57 -07:00
Nicola Murino 4aab0d9d6a ssh: return partial success immediately from publickey auth
Change publicKeyCallback to return authPartialSuccess to the caller
immediately when a signer receives partial success, instead of
continuing to try remaining signers internally.

The caller (clientAuthenticate) already handles authPartialSuccess
by not adding the method to "tried", so findNext can re-select
publickey on the next iteration. When it does, all signers are
tried again from the beginning.

Change-Id: Icc87721aa56ef59a357e409aa046640a27e5fc46
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/757900
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
2026-05-21 19:36:47 -07:00
Nicola 3c7c86938f ssh: fix deadlock on unexpected channel responses
Previously, channel.handlePacket sent channelRequestSuccess and
channelRequestFailure messages to ch.msg unconditionally via the default
arm of its type switch. Because ch.msg is a bounded buffer (chanSize),
a peer that sends a burst of unsolicited channel request responses for
an open, idle channel fills the buffer and blocks the mux read loop on
the next send. That stalls all packet processing on the connection,
and because readLoop then backs up on t.incoming, closing the
underlying net.Conn does not unblock either goroutine: user code
observes Close() returning promptly while Wait() hangs and the mux,
readLoop, and kexLoop goroutines leak permanently.

This change mirrors the fix for the mux-level SendRequest path: a
sentRequestPending atomic gate is set while a SendRequest with
WantReply is in flight, handlePacket drops responses when the gate is
closed, and uses a non-blocking send otherwise. SendRequest drains
any spurious response that slipped through before discarding it, so
the caller always observes the reply to its own request.

This aligns with OpenSSH, which silently ignores channel confirm
messages that do not match a pending request.

Fixes golang/go#79564
Fixes CVE-2026-39830

Change-Id: I15e2add4bf7876bb0c6f921f8b57203d97e83f47
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781664
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Neal Patel <nealpatel@google.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2026-05-21 17:09:12 -07:00
Neal Patel 533fb3f7e4 ssh: fix source-address critical option bypass
Previously, CVE-2024-45337 fixed an authorization bypass
for misused ssh server configurations; if any other type
of callback is passed other than public key, then the
source-address validation would be skipped.

Fixes CVE-2026-46595
Fixes golang/go#79570

Change-Id: I08d86a961048a232c8672f23000e693ed5a0e2fd
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781642
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2026-05-21 17:03:26 -07:00
Neal Patel abbc44d451 ssh: fix incorrect operator order
Arithmetic is incorrectly applied to 'byte'
instead of 'int' resulting in a possible
overflow that allows for a panic.

Fixes CVE-2026-46597
Fixes golang/go#79561

Change-Id: I83edabeeda676f0209d29d5e2554890bbd0eef8f
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781620
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Gopher Robot <gobot@golang.org>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-05-21 17:03:25 -07:00
Nicola Murino e052873987 ssh: fix infinite loop on large channel writes due to integer overflow
The internal 'min' helper function in channel.go incorrectly cast the
input data length (int) to uint32 before comparing it with the
maximum packet size. On 64-bit systems, if the data length is a
multiple of 2^32 (approx. 4GB), this cast results in 0.

Consequently, the function returns 0, causing the WriteExtended loop
to spin indefinitely because it attempts to reserve 0 bytes while
the remaining data length is still positive.

This change renames the helper to 'minPayloadSize' to avoid confusion
with the Go 1.21 built-in 'min' and updates the logic to use int64
for comparisons, preventing truncation and the resulting infinite loop.

This issue was found during a security audit by NCC Group Cryptography
Services, sponsored by Teleport.

Fixes golang/go#79567
Fixes CVE-2026-39834

Change-Id: Id5bf81d9f06c7042452acffe1c76580ff878665e
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781663
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-05-21 17:03:23 -07:00
Nicola Murino b61cf853a8 ssh: enforce user presence verification for security keys
Previously the library did not verify the "User Presence" (UP) bit in
signatures generated by FIDO/U2F security keys
(sk-ecdsa-sha2-nistp256@openssh.com and sk-ssh-ed25519@openssh.com).
This allowed signatures without physical interaction to be accepted
if the underlying hardware produced them, deviating from the default
secure behavior expected by the FIDO standards and OpenSSH.

skECDSAPublicKey.Verify and skEd25519PublicKey.Verify now enforce
the user-presence bit (0x01, constant flagUserPresence) by default.
Signatures whose flags byte has UP clear fail with the sentinel
errSKMissingUserPresence.

The server public-key authentication path honors the OpenSSH
"no-touch-required" extension as an opt-out. noTouchAllowed reports
true when the extension is present either in the Permissions
returned by PublicKeyCallback (authorized_keys-level opt-out) or in
the certificate's own Extensions (CA-level opt-out); in that case
skKeyWithoutUP is used to derive a clone of the SK public key (and,
for certificates, a clone of the wrapping Certificate whose inner
Key is the cloned SK key) whose Verify accepts UP-clear signatures.
The originals are never mutated, so a per-session opt-out cannot
leak across authentication attempts or connections. Matching
OpenSSH, the opt-out is read only from Extensions, never from
CriticalOptions.

skKeyWithoutUP is iterative and unwraps at most one level of
*Certificate: the SSH cert format forbids Certificate.Key from being
another Certificate (parseCert rejects it) but callers can still
construct such a value directly in Go, so a recursive descent would
be driven to unbounded depth by malformed or cyclic input. Any such
pathological *Certificate is returned unchanged.

CertChecker.CheckCert applies skKeyWithoutUP unconditionally to the
certificate's CA key before verifying the CA signature, matching
OpenSSH, which calls sshkey_verify with detailsp==NULL in
sshkey.c:cert_parse and never extracts or enforces UP/UV flags on
CA signatures. The UP bit on a CA signature reflects the CA
operator's presence at cert-issuance time, which has no bearing on
whether the user being authenticated is present now, so enforcing it
here would only break interop with certificates issued by
non-interactive SK CAs without a corresponding security benefit. The
skKeyWithoutUP call is a no-op for non-SK CA keys (the common case).

This change breaks backward compatibility for clients or keys that
generate user-authentication signatures without the User Presence
flag set. Previously those signatures were accepted by the server.
They will now be rejected with "ssh: signature missing required user
presence flag" unless the "no-touch-required" extension is
explicitly granted to the session by the server callbacks, or
carried by the user certificate.

This issue was found during a security audit by NCC Group
Cryptography Services, sponsored by Teleport.

Fixes golang/go#79566
Fixes CVE-2026-39831

Change-Id: I74b6de3bb6a2d7a0f34d7fa36bbbbf06f0b3fc6b
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781662
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-05-21 17:03:20 -07:00