ssh: enforce nil Permissions when returning PartialSuccessError

In serverAuthenticate, the permissions variable is reset to nil at the
beginning of the authentication loop. If an authentication callback
returns a PartialSuccessError along with non-nil Permissions, those
permissions are currently silently discarded before the next
authentication step.

This change returns an error if a callback returns both a PartialSuccessError
and non-nil Permissions, preventing API misuse where the user might
erroneously expect those permissions to be preserved or merged into the
final session permissions.

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

Fixes golang/go#79562
Fixes CVE-2026-39828

Change-Id: I632c9e46e2b5e8804ef88081063a3612a2462f9f
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781621
Reviewed-by: Neal Patel <nealpatel@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>

ssh/keys_test.go

@@ -272,18 +272,18 @@ func TestParseEncryptedPrivateKeysWithPassphrase(t *testing.T) {
 }
 
 func TestParseEncryptedPrivateKeysWithUnsupportedCiphers(t *testing.T) {
-    for _, tt := range testdata.UnsupportedCipherData {
-        t.Run(tt.Name, func(t *testing.T){
-            _, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte(tt.EncryptionKey))
-            if err == nil {
-                t.Fatalf("expected 'unknown cipher' error for %q, got nil", tt.Name)
-                // If this cipher is now supported, remove it from testdata.UnsupportedCipherData
-            }
-            if !strings.Contains(err.Error(), "unknown cipher") {
-                t.Errorf("wanted 'unknown cipher' error, got %v", err.Error())
-            }
-        })
-    }
+	for _, tt := range testdata.UnsupportedCipherData {
+		t.Run(tt.Name, func(t *testing.T) {
+			_, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte(tt.EncryptionKey))
+			if err == nil {
+				t.Fatalf("expected 'unknown cipher' error for %q, got nil", tt.Name)
+				// If this cipher is now supported, remove it from testdata.UnsupportedCipherData
+			}
+			if !strings.Contains(err.Error(), "unknown cipher") {
+				t.Errorf("wanted 'unknown cipher' error, got %v", err.Error())
+			}
+		})
+	}
 }
 
 func TestParseEncryptedPrivateKeysWithIncorrectPassphrase(t *testing.T) {

ssh/server.go

@@ -242,8 +242,10 @@ func (c *pubKeyCache) add(candidate cachedPubKey) {
 type ServerConn struct {
 	Conn
 
-	// If the succeeding authentication callback returned a
-	// non-nil Permissions pointer, it is stored here.
+	// If the succeeding authentication callback returned a non-nil Permissions
+	// pointer, it is stored here. These are the permissions from the final,
+	// successful authentication method. Permissions returned by callbacks that
+	// return PartialSuccessError are not preserved and must be nil.
 	Permissions *Permissions
 }
 
@@ -824,6 +826,13 @@ userAuthLoop:
 		var failureMsg userAuthFailureMsg
 
 		if partialSuccess, ok := authErr.(*PartialSuccessError); ok {
+			// Permissions are not preserved between authentication steps. To
+			// avoid confusion about the final state of the connection, we
+			// disallow returning non-nil Permissions combined with
+			// PartialSuccessError.
+			if perms != nil {
+				return nil, errors.New("ssh: permissions must be nil when returning PartialSuccessError")
+			}
 			// After a partial success error we don't allow changing the user
 			// name and execute the NoClientAuthCallback.
 			partialSuccessReturned = true

ssh/server_test.go

@@ -808,6 +808,60 @@ func TestVerifiedPublicKeyCallbackOnly(t *testing.T) {
 	<-done
 }
 
+func TestPartialSuccessWithNonNilPerms(t *testing.T) {
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	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 bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
+				// Intentionally return non-nil Permissions along with a
+				// PartialSuccessError. Since permissions are reset between
+				// authentication steps, this constitutes invalid library usage
+				// and the server is expected to reject the connection.
+				return &Permissions{Extensions: map[string]string{"permit-port-forwarding": ""}}, &PartialSuccessError{
+					Next: ServerAuthCallbacks{
+						PasswordCallback: func(conn ConnMetadata, password []byte) (*Permissions, error) {
+							if string(password) == clientPassword {
+								return nil, nil
+							}
+							return nil, nil
+						},
+					},
+				}
+			}
+			return nil, errors.New("invalid credentials")
+		},
+	}
+	serverConf.AddHostKey(testSigners["rsa"])
+
+	clientConf := ClientConfig{
+		User: "user",
+		Auth: []AuthMethod{
+			PublicKeys(testSigners["rsa"]),
+			Password(clientPassword),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+
+	go NewServerConn(c1, serverConf)
+
+	_, _, _, err = NewClientConn(c2, "", &clientConf)
+	if err == nil {
+		t.Fatal("authentication succeeded unexpectedly; server should have rejected non-nil Permissions combined with PartialSuccessError")
+	}
+}
+
 type markerConn struct {
 	closed uint32
 	used   uint32