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
parent 4044611da8
commit e557b08ec2
2 changed files with 126 additions and 18 deletions
@@ -26,10 +26,12 @@ type Permissions struct {
 	// defines "force-command" (only allow the given command to
 	// execute) and "source-address" (only allow connections from
 	// the given address). The SSH package currently only enforces
-	// the "source-address" critical option. It is up to server
-	// implementations to enforce other critical options, such as
-	// "force-command", by checking them after the SSH handshake
-	// is successful. In general, SSH servers should reject
+	// the "source-address" critical option: it is validated against
+	// the client's remote address whenever it is present in the
+	// Permissions returned by any authentication callback. It is up
+	// to server implementations to enforce other critical options,
+	// such as "force-command", by checking them after the SSH
+	// handshake is successful. In general, SSH servers should reject
 	// connections that specify critical options that are unknown
 	// or not supported.
 	CriticalOptions map[string]string
@@ -472,6 +474,19 @@ func checkSourceAddress(addr net.Addr, sourceAddrs string) error {
 	return fmt.Errorf("ssh: remote address %v is not allowed because of source-address restriction", addr)
 }

+// checkSourceAddressCriticalOption enforces the source-address critical
+// option, if present in perms, as documented in Permissions.CriticalOptions.
+func checkSourceAddressCriticalOption(addr net.Addr, perms *Permissions) error {
+	if perms == nil {
+		return nil
+	}
+	saco := perms.CriticalOptions[sourceAddressCriticalOption]
+	if saco == "" {
+		return nil
+	}
+	return checkSourceAddress(addr, saco)
+}
+
 func gssExchangeToken(gssapiConfig *GSSAPIWithMICConfig, token []byte, s *connection,
 	sessionID []byte, userAuthReq userAuthRequestMsg) (authErr error, perms *Permissions, err error) {
 	gssAPIServer := gssapiConfig.Server
@@ -784,13 +799,14 @@ userAuthLoop:
 					return nil, errors.New("ssh: invalid library usage: PublicKeyCallback must not return partial success when VerifiedPublicKeyCallback is defined")
 				}

-				if (candidate.result == nil || isPartialSuccessError) &&
-					candidate.perms != nil &&
-					candidate.perms.CriticalOptions != nil &&
-					candidate.perms.CriticalOptions[sourceAddressCriticalOption] != "" {
-					if err := checkSourceAddress(
-						s.RemoteAddr(),
-						candidate.perms.CriticalOptions[sourceAddressCriticalOption]); err != nil {
+				// This check is authoritative for the Permissions returned by
+				// PublicKeyCallback: the check at the end of the auth loop sees
+				// the final Permissions, which VerifiedPublicKeyCallback may
+				// have replaced, and is skipped on partial success. It also
+				// makes public key queries fail before the client signs when
+				// PublicKeyCallback supplies the restriction.
+				if candidate.result == nil || isPartialSuccessError {
+					if err := checkSourceAddressCriticalOption(s.RemoteAddr(), candidate.perms); err != nil {
 						candidate.result = err
 					}
 				}
@@ -866,13 +882,6 @@ userAuthLoop:
 					// considered verified and the callback must not run.
 					perms, authErr = config.VerifiedPublicKeyCallback(s, pubKey, perms, algo)
 				}
-				if authErr == nil && perms != nil && perms.CriticalOptions != nil {
-					if saco := perms.CriticalOptions[sourceAddressCriticalOption]; saco != "" {
-						if err := checkSourceAddress(s.RemoteAddr(), saco); err != nil {
-							authErr = err
-						}
-					}
-				}
 			}
 		case "gssapi-with-mic":
 			if !gssapiWithMICConfigured(authConfig.GSSAPIWithMICConfig) {
@@ -925,6 +934,17 @@ userAuthLoop:
 			authErr = fmt.Errorf("ssh: unknown method %q", userAuthReq.Method)
 		}

+		// The source-address critical option is enforced on the Permissions
+		// returned by any authentication callback. Permissions returned
+		// together with a PartialSuccessError skip this check: that is safe
+		// because they are required to be nil, as enforced in the partial
+		// success handling below.
+		if authErr == nil {
+			if err := checkSourceAddressCriticalOption(s.RemoteAddr(), perms); err != nil {
+				authErr = err
+			}
+		}
+
 		authErrs = append(authErrs, authErr)

 		if config.AuthLogCallback != nil {
@@ -760,6 +760,94 @@ func TestVerifiedPubKeyCallbackSourceAddress(t *testing.T) {
 	}
 }

+func TestAuthCallbacksSourceAddress(t *testing.T) {
+	permsWithSourceAddress := func(sourceAddress string) *Permissions {
+		return &Permissions{
+			CriticalOptions: map[string]string{
+				sourceAddressCriticalOption: sourceAddress,
+			},
+		}
+	}
+	methods := []struct {
+		name         string
+		serverConfig func(sourceAddress string) *ServerConfig
+		clientAuth   []AuthMethod
+	}{
+		{
+			name: "password",
+			serverConfig: func(sourceAddress string) *ServerConfig {
+				return &ServerConfig{
+					PasswordCallback: func(conn ConnMetadata, password []byte) (*Permissions, error) {
+						return permsWithSourceAddress(sourceAddress), nil
+					},
+				}
+			},
+			clientAuth: []AuthMethod{Password(clientPassword)},
+		},
+		{
+			name: "keyboard-interactive",
+			serverConfig: func(sourceAddress string) *ServerConfig {
+				return &ServerConfig{
+					KeyboardInteractiveCallback: func(conn ConnMetadata, challenge KeyboardInteractiveChallenge) (*Permissions, error) {
+						return permsWithSourceAddress(sourceAddress), nil
+					},
+				}
+			},
+			clientAuth: []AuthMethod{
+				KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) {
+					return nil, nil
+				}),
+			},
+		},
+		{
+			name: "none",
+			serverConfig: func(sourceAddress string) *ServerConfig {
+				return &ServerConfig{
+					NoClientAuth: true,
+					NoClientAuthCallback: func(conn ConnMetadata) (*Permissions, error) {
+						return permsWithSourceAddress(sourceAddress), nil
+					},
+				}
+			},
+		},
+	}
+	for _, method := range methods {
+		for _, tc := range []struct {
+			name          string
+			sourceAddress string
+			wantErr       bool
+		}{
+			{"mismatching", "192.168.99.99", true},
+			{"matching", "127.0.0.0/8,::1/128", false},
+		} {
+			t.Run(method.name+"/"+tc.name, func(t *testing.T) {
+				clientConf := &ClientConfig{
+					User:            "user",
+					Auth:            method.clientAuth,
+					HostKeyCallback: InsecureIgnoreHostKey(),
+				}
+				serverAuthErrors, err := doClientServerAuth(t, method.serverConfig(tc.sourceAddress), clientConf)
+				if tc.wantErr {
+					if err == nil {
+						t.Fatalf("client login succeeded with %s callback returning mismatching source-address", method.name)
+					}
+					var sourceAddressErrors int
+					for _, err := range serverAuthErrors {
+						if err != nil && strings.Contains(err.Error(), "source-address restriction") {
+							sourceAddressErrors++
+						}
+					}
+					if sourceAddressErrors == 0 {
+						t.Fatalf("no server auth error mentions the source-address restriction, got %v", serverAuthErrors)
+					}
+				} else if err != nil {
+					t.Fatalf("client login failed with matching source-address: %v", err)
+				}
+			})
+		}
+	}
+}
+
 func TestVerifiedPublicCallbackPartialSuccessBadUsage(t *testing.T) {
 	c1, c2, err := netPipe()
 	if err != nil {