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>

ssh/client_auth_test.go

@@ -7,10 +7,12 @@ package ssh
 import (
 	"bytes"
 	"crypto/rand"
+	"crypto/rsa"
 	"errors"
 	"fmt"
 	"io"
 	"log"
+	"math/big"
 	"net"
 	"os"
 	"runtime"
@@ -169,6 +171,40 @@ func TestClientAuthThirdKey(t *testing.T) {
 	}
 }
 
+// invalidRSASigner offers an RSA public key the server cannot parse: its public
+// exponent is even, which parseRSA rejects regardless of any key size limit.
+// The key is intentionally invalid for a structural reason rather than for its
+// size, so the test stays meaningful even if the accepted modulus size changes.
+// Its Sign method is never reached, because the key is rejected during the
+// initial public key query.
+type invalidRSASigner struct{}
+
+func (invalidRSASigner) PublicKey() PublicKey {
+	n := new(big.Int).Lsh(big.NewInt(1), 2048)
+	pub, err := NewPublicKey(&rsa.PublicKey{N: n, E: 2}) // incorrect exponent.
+	if err != nil {
+		panic(err)
+	}
+	return pub
+}
+
+func (invalidRSASigner) Sign(rand io.Reader, data []byte) (*Signature, error) {
+	return nil, errors.New("ssh: invalid test key must not be used to sign")
+}
+
+func TestClientAuthInvalidPublicKey(t *testing.T) {
+	config := &ClientConfig{
+		User: "testuser",
+		Auth: []AuthMethod{
+			PublicKeys(invalidRSASigner{}, testSigners["rsa"]),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+	if err := tryAuth(t, config); err != nil {
+		t.Fatalf("client auth failed but should have fallen back to a valid key: %s", err)
+	}
+}
+
 // partialSuccessPublicKeyAndKbdInteractiveServer returns a server config
 // where the rsa key gets partial success and the server offers both publickey
 // and keyboard-interactive as next methods. The VerifiedPublicKeyCallback

ssh/server.go

@@ -796,7 +796,8 @@ userAuthLoop:
 
 			pubKey, err := ParsePublicKey(pubKeyData)
 			if err != nil {
-				return nil, err
+				authErr = err
+				break
 			}
 
 			candidate, ok := cache.get(s.user, pubKeyData)