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>
diff --git a/ssh/server.go b/ssh/server.go index 3c0fcc9..e0e8890 100644 --- a/ssh/server.go +++ b/ssh/server.go
@@ -26,10 +26,12 @@ // 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 @@ 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 @@ 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 @@ // 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 @@ 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 {
diff --git a/ssh/server_test.go b/ssh/server_test.go index 01e262b..c44cdcf 100644 --- a/ssh/server_test.go +++ b/ssh/server_test.go
@@ -760,6 +760,94 @@ } } +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 {