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
parent a1c0d99298
commit 4aab0d9d6a
2 changed files with 279 additions and 5 deletions
@@ -377,11 +377,11 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand
 			return authFailure, nil, err
 		}

-		// If authentication succeeds or the list of available methods does not
-		// contain the "publickey" method, do not attempt to authenticate with any
-		// other keys.  According to RFC 4252 Section 7, the latter can occur when
-		// additional authentication methods are required.
-		if success == authSuccess || !slices.Contains(methods, cb.method()) {
+		// If authentication succeeds or partially succeeds, return immediately
+		// so the caller can select the next auth method. According to RFC 4252
+		// Section 7, if the server no longer lists "publickey" among its
+		// allowed methods, do not attempt to authenticate with any other keys.
+		if success == authSuccess || success == authPartialSuccess || !slices.Contains(methods, cb.method()) {
 			return success, methods, err
 		}
 	}
@@ -169,6 +169,280 @@ func TestClientAuthThirdKey(t *testing.T) {
 	}
 }

+// 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
+// tracks state so that the same key only triggers partial success once,
+// avoiding an impossible auth loop.
+func partialSuccessPublicKeyAndKbdInteractiveServer() *ServerConfig {
+	rsaDone := false
+	serverConf := &ServerConfig{
+		PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+			if bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				return nil, nil
+			}
+			return nil, errors.New("invalid credentials")
+		},
+		VerifiedPublicKeyCallback: func(conn ConnMetadata, key PublicKey, permissions *Permissions, signatureAlgorithm string) (*Permissions, error) {
+			if !rsaDone && bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				rsaDone = true
+				return nil, &PartialSuccessError{
+					Next: ServerAuthCallbacks{
+						PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+							if bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+								return nil, nil
+							}
+							return nil, errors.New("invalid credentials")
+						},
+						KeyboardInteractiveCallback: func(conn ConnMetadata, client KeyboardInteractiveChallenge) (*Permissions, error) {
+							answers, err := client("", "", []string{"token: "}, []bool{true})
+							if err != nil {
+								return nil, err
+							}
+							if len(answers) == 1 && answers[0] == "correct-token" {
+								return nil, nil
+							}
+							return nil, errors.New("invalid token")
+						},
+					},
+				}
+			}
+			return nil, errors.New("invalid credentials")
+		},
+	}
+	serverConf.AddHostKey(testSigners["rsa"])
+	return serverConf
+}
+
+// partialSuccessMultiKeyServer returns a server config that requires two
+// public keys in sequence: the first key (rsa) triggers partial success
+// requiring a second key (ecdsa) to complete authentication.
+func partialSuccessMultiKeyServer() *ServerConfig {
+	firstKeyDone := false
+	serverConf := &ServerConfig{
+		PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+			if bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				return nil, nil
+			}
+			if bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+				return nil, nil
+			}
+			return nil, errors.New("invalid credentials")
+		},
+		VerifiedPublicKeyCallback: func(conn ConnMetadata, key PublicKey, permissions *Permissions, signatureAlgorithm string) (*Permissions, error) {
+			if !firstKeyDone && bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				firstKeyDone = true
+				return nil, &PartialSuccessError{
+					Next: ServerAuthCallbacks{
+						PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+							if bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+								return nil, nil
+							}
+							return nil, errors.New("invalid credentials")
+						},
+					},
+				}
+			}
+			if firstKeyDone && bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+				return nil, nil
+			}
+			return nil, errors.New("invalid credentials")
+		},
+	}
+	serverConf.AddHostKey(testSigners["rsa"])
+	return serverConf
+}
+
+func TestStaticMultiKeyPartialSuccess(t *testing.T) {
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	clientConf := &ClientConfig{
+		User: "user",
+		Auth: []AuthMethod{
+			PublicKeys(testSigners["rsa"], testSigners["ecdsa"]),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+
+	go NewServerConn(c1, partialSuccessMultiKeyServer())
+
+	_, _, _, err = NewClientConn(c2, "", clientConf)
+	if err != nil {
+		t.Fatalf("client login error: %s", err)
+	}
+}
+
+func TestStaticPartialSuccess(t *testing.T) {
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	clientConf := &ClientConfig{
+		User: "user",
+		Auth: []AuthMethod{
+			PublicKeys(testSigners["rsa"]),
+			KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) {
+				return []string{"correct-token"}, nil
+			}),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+
+	go NewServerConn(c1, partialSuccessPublicKeyAndKbdInteractiveServer())
+
+	_, _, _, err = NewClientConn(c2, "", clientConf)
+	if err != nil {
+		t.Fatalf("client login error: %s", err)
+	}
+}
+
+// partialSuccessKeyAThenKeyBServer returns a server config where keyA (rsa)
+// gets partial success and the server offers BOTH publickey and
+// keyboard-interactive. keyB (ecdsa) then completes auth successfully.
+func partialSuccessKeyAThenKeyBServer() *ServerConfig {
+	keyADone := false
+	serverConf := &ServerConfig{
+		PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+			if bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				return nil, nil
+			}
+			if bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+				return nil, nil
+			}
+			return nil, errors.New("invalid credentials")
+		},
+		VerifiedPublicKeyCallback: func(conn ConnMetadata, key PublicKey, permissions *Permissions, signatureAlgorithm string) (*Permissions, error) {
+			if !keyADone && bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				keyADone = true
+				return nil, &PartialSuccessError{
+					Next: ServerAuthCallbacks{
+						PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+							if bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+								return nil, nil
+							}
+							return nil, errors.New("invalid credentials")
+						},
+						KeyboardInteractiveCallback: func(conn ConnMetadata, client KeyboardInteractiveChallenge) (*Permissions, error) {
+							answers, err := client("", "", []string{"token: "}, []bool{true})
+							if err != nil {
+								return nil, err
+							}
+							if len(answers) == 1 && answers[0] == "correct-token" {
+								return nil, nil
+							}
+							return nil, errors.New("invalid token")
+						},
+					},
+				}
+			}
+			if keyADone && bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+				return nil, nil
+			}
+			return nil, errors.New("invalid credentials")
+		},
+	}
+	serverConf.AddHostKey(testSigners["rsa"])
+	return serverConf
+}
+
+func TestStaticPartialSuccessKeyAThenKeyB(t *testing.T) {
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	clientConf := &ClientConfig{
+		User: "user",
+		Auth: []AuthMethod{
+			PublicKeys(testSigners["rsa"], testSigners["ecdsa"]),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+
+	go NewServerConn(c1, partialSuccessKeyAThenKeyBServer())
+
+	_, _, _, err = NewClientConn(c2, "", clientConf)
+	if err != nil {
+		t.Fatalf("client login error: %s", err)
+	}
+}
+
+// partialSuccessMultiKeyThenKbdInteractiveServer returns a server config
+// that strictly requires keyA (rsa) AND keyB (ecdsa) as public keys before
+// allowing keyboard-interactive. After keyA, ONLY publickey is offered
+// (for keyB). After keyB, ONLY keyboard-interactive is offered.
+func partialSuccessMultiKeyThenKbdInteractiveServer() *ServerConfig {
+	serverConf := &ServerConfig{
+		PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+			if bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				return nil, &PartialSuccessError{
+					Next: ServerAuthCallbacks{
+						PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+							if bytes.Equal(key.Marshal(), testPublicKeys["ecdsa"].Marshal()) {
+								return nil, &PartialSuccessError{
+									Next: ServerAuthCallbacks{
+										KeyboardInteractiveCallback: func(conn ConnMetadata, client KeyboardInteractiveChallenge) (*Permissions, error) {
+											answers, err := client("", "", []string{"token: "}, []bool{true})
+											if err != nil {
+												return nil, err
+											}
+											if len(answers) == 1 && answers[0] == "correct-token" {
+												return nil, nil
+											}
+											return nil, errors.New("invalid token")
+										},
+									},
+								}
+							}
+							return nil, errors.New("invalid credentials")
+						},
+					},
+				}
+			}
+			return nil, errors.New("invalid credentials")
+		},
+	}
+	serverConf.AddHostKey(testSigners["rsa"])
+	return serverConf
+}
+
+func TestStaticMultiKeyPartialThenKbdInteractive(t *testing.T) {
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	clientConf := &ClientConfig{
+		User: "user",
+		Auth: []AuthMethod{
+			PublicKeys(testSigners["rsa"], testSigners["ecdsa"]),
+			KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) {
+				return []string{"correct-token"}, nil
+			}),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+
+	go NewServerConn(c1, partialSuccessMultiKeyThenKbdInteractiveServer())
+
+	_, _, _, err = NewClientConn(c2, "", clientConf)
+	if err != nil {
+		t.Fatalf("client login error: %s", err)
+	}
+}
+
 func TestAuthMethodPassword(t *testing.T) {
 	config := &ClientConfig{
 		User: "testuser",