From d59570d1a3152c5998e3f9b4b757514412a1472e Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 25 Mar 2026 12:02:00 +0100 Subject: [PATCH] ssh: add AuthCallback to ClientConfig Add ClientAuthCallback, a hook invoked before each authentication attempt that allows the client to dynamically select an auth method based on server capabilities, partial successes, or previous failures. clientAuthenticate tracks partial successes in a slice and passes them to AuthCallback via ClientAuthContext.PartialSuccessMethods, along with the list of failed methods and currently allowed methods. When AuthCallback returns a non-nil AuthMethod, it is used for the next attempt, bypassing the static findNext selection. When it returns (nil, nil), the static config.Auth selection proceeds as before. If AuthCallback returns (nil, error) the handshake aborts immediately with that error. To bound resource use when AuthCallback keeps supplying methods, the client caps the total number of authentication attempts (failures and partial successes combined) at 64; exceeding the cap aborts the handshake with an error. Fixes golang/go#76146 Change-Id: I0d02bea7b9dd724e95e5d9d49d85306666c0df7a Reviewed-on: https://go-review.googlesource.com/c/crypto/+/717140 Reviewed-by: Cherry Mui Reviewed-by: Dmitri Shuralyov Reviewed-by: Filippo Valsorda LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com --- ssh/client.go | 59 ++++++ ssh/client_auth.go | 54 +++++- ssh/client_auth_test.go | 409 ++++++++++++++++++++++++++++++++++++++++ ssh/server_test.go | 17 +- 4 files changed, 525 insertions(+), 14 deletions(-) diff --git a/ssh/client.go b/ssh/client.go index 3307978..19ff9b3 100644 --- a/ssh/client.go +++ b/ssh/client.go @@ -197,6 +197,59 @@ type HostKeyCallback func(hostname string, remote net.Addr, key PublicKey) error // the server. A BannerCallback receives the message sent by the remote server. type BannerCallback func(message string) error +// ClientAuthContext contains information about the current state of the +// authentication process, passed to [ClientAuthCallback]. +type ClientAuthContext struct { + // Metadata contains the connection metadata. + Metadata ConnMetadata + + // Algorithms contains the negotiated algorithms. + Algorithms NegotiatedAlgorithms + + // AllowedMethods lists the authentication methods currently accepted + // by the server. These are the protocol-level names defined in RFC 4252 + // such as "publickey", "password". + AllowedMethods []string + + // PartialSuccessMethods lists the authentication methods that have already + // succeeded, indicating a multi-step authentication flow. This list + // represents the exact sequence of partial successes and may contain + // duplicates if the same method succeeded multiple times. + PartialSuccessMethods []string + + // TriedMethods lists the methods that have already been attempted and + // failed during this session. This list represents the exact sequence of + // failures and may contain duplicates. This allows the callback to also + // track the number of failed attempts for a specific method. + TriedMethods []string +} + +// ClientAuthCallback is a hook invoked before each authentication attempt. It +// allows the client to dynamically select an authentication method based on the +// current context, server capabilities, or previous failures. +// +// The callback is invoked after the initial "none" authentication method, once +// the server's supported authentication methods are known. +// +// Return values: +// - (AuthMethod, nil): The client will attempt this specific method next. +// The returned method does NOT need to be present in [ClientConfig.Auth]. +// This allows for dynamic authentication strategies (e.g., prompting +// for a password only if public key auth fails). Callers should inspect +// [ClientAuthContext.TriedMethods] to avoid repeatedly returning the +// same failing method. +// - (nil, nil): The client selects from [ClientConfig.Auth] the first +// instance of a method that has not been tried yet, or aborts if none +// are left. If authentication is not successful, the callback is invoked +// again before the following attempt. +// - (nil, error): The authentication process is aborted immediately, +// causing the ongoing SSH handshake to fail with the provided error. +// +// To bound resource use, the client caps the total number of authentication +// attempts (failures and partial successes combined) at 64. If the cap is +// exceeded the handshake aborts with an error. +type ClientAuthCallback func(ctx *ClientAuthContext) (AuthMethod, error) + // A ClientConfig structure is used to configure a Client. It must not be // modified after having been passed to an SSH function. type ClientConfig struct { @@ -210,6 +263,9 @@ type ClientConfig struct { // Auth contains possible authentication methods to use with the // server. Only the first instance of a particular RFC 4252 method will // be used during authentication. + // + // If AuthCallback is set, these AuthMethod are only used if the + // callback returns nil. Auth []AuthMethod // HostKeyCallback is called during the cryptographic @@ -240,6 +296,9 @@ type ClientConfig struct { // // A Timeout of zero means no timeout. Timeout time.Duration + + // AuthCallback, if non-nil, is invoked before each authentication attempt. + AuthCallback ClientAuthCallback } // InsecureIgnoreHostKey returns a function that can be used for diff --git a/ssh/client_auth.go b/ssh/client_auth.go index 1187ef9..60af2fc 100644 --- a/ssh/client_auth.go +++ b/ssh/client_auth.go @@ -21,6 +21,12 @@ const ( authSuccess ) +// maxAuthClientTried bounds the total number of authentication attempts +// (failures and partial successes combined) the client makes before +// aborting the loop, to prevent unbounded growth when an AuthCallback +// keeps supplying methods. +const maxAuthClientTried = 64 + // clientAuthenticate authenticates with the remote server. See RFC 4252. func (c *connection) clientAuthenticate(config *ClientConfig) error { // initiate user auth session @@ -67,32 +73,62 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error { // then any untried methods suggested by the server. var tried []string var lastMethods []string + var partialSuccess []string sessionID := c.transport.getSessionID() for auth := AuthMethod(new(noneAuth)); auth != nil; { ok, methods, err := auth.auth(sessionID, config.User, c.transport, config.Rand, extensions) if err != nil { // On disconnect, return error immediately - if _, ok := err.(*disconnectMsg); ok { + if _, isDisconnect := err.(*disconnectMsg); isDisconnect { return err } - // We return the error later if there is no other method left to - // try. + // We return the error later if there is no other method + // left to try. ok = authFailure } - if ok == authSuccess { - // success + + switch ok { + case authSuccess: return nil - } else if ok == authFailure { - if m := auth.method(); !slices.Contains(tried, m) { - tried = append(tried, m) - } + case authPartialSuccess: + partialSuccess = append(partialSuccess, auth.method()) + case authFailure: + tried = append(tried, auth.method()) } + if len(partialSuccess)+len(tried) > maxAuthClientTried { + return fmt.Errorf("ssh: too many authentication attempts (%d), aborting", + len(partialSuccess)+len(tried)) + } + if methods == nil { methods = lastMethods } lastMethods = methods + // If AuthCallback is set it takes precedence: it picks the next + // AuthMethod dynamically. The returned method need not be in + // config.Auth. If the callback returns (nil, nil) we fall back to + // selecting the next untried method from config.Auth below; on + // (nil, error) the handshake aborts. + if config.AuthCallback != nil { + ctx := &ClientAuthContext{ + Metadata: c, + Algorithms: c.Algorithms(), + AllowedMethods: slices.Clone(methods), + PartialSuccessMethods: slices.Clone(partialSuccess), + TriedMethods: slices.Clone(tried), + } + altAuth, cbErr := config.AuthCallback(ctx) + if cbErr != nil { + return cbErr + } + if altAuth != nil { + auth = altAuth + continue + } + } + auth = nil findNext: diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index e90fc0c..b994b78 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -443,6 +443,415 @@ func TestStaticMultiKeyPartialThenKbdInteractive(t *testing.T) { } } +func TestAuthMethodDynamic(t *testing.T) { + config := &ClientConfig{ + User: "testuser", + Auth: nil, + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if len(ctx.Metadata.SessionID()) == 0 { + t.Error("empty sessionID") + } + if ctx.Algorithms.HostKey == "" { + t.Error("empty Host Key") + } + if slices.Contains(ctx.AllowedMethods, "password") { + return Password(clientPassword), nil + } + return nil, errors.New("auth callback error") + }, + } + + if err := tryAuth(t, config); err != nil { + t.Fatalf("unable to dial remote side: %s", err) + } +} + +func TestAuthCallbackError(t *testing.T) { + config := &ClientConfig{ + User: "testuser", + Auth: []AuthMethod{ + Password(clientPassword), + }, + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + return nil, errors.New("auth callback error") + }, + } + + if err := tryAuth(t, config); err == nil { + t.Fatal("authentication unexpectedly succeeded after AuthCallback returned an error") + } +} + +func TestAuthCallbackPartialSuccess(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", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if slices.Contains(ctx.AllowedMethods, "publickey") && !slices.Contains(ctx.PartialSuccessMethods, "publickey") { + return PublicKeys(testSigners["rsa"]), nil + } + if slices.Contains(ctx.PartialSuccessMethods, "publickey") && slices.Contains(ctx.AllowedMethods, "keyboard-interactive") { + return KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) { + return []string{"correct-token"}, nil + }), nil + } + return nil, fmt.Errorf("unexpected auth state: allowed=%v partial=%v", ctx.AllowedMethods, ctx.PartialSuccessMethods) + }, + } + + go NewServerConn(c1, partialSuccessPublicKeyAndKbdInteractiveServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } +} + +func TestAuthCallbackMultiKeyPartialSuccess(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", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if slices.Contains(ctx.AllowedMethods, "publickey") && !slices.Contains(ctx.PartialSuccessMethods, "publickey") { + return PublicKeys(testSigners["rsa"]), nil + } + if slices.Contains(ctx.PartialSuccessMethods, "publickey") && slices.Contains(ctx.AllowedMethods, "publickey") { + return PublicKeys(testSigners["ecdsa"]), nil + } + return nil, fmt.Errorf("unexpected auth state: allowed=%v partial=%v", ctx.AllowedMethods, ctx.PartialSuccessMethods) + }, + } + + go NewServerConn(c1, partialSuccessMultiKeyServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } +} + +func TestAuthCallbackMultiKeyPartialThenKbdInteractive(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", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + partialCount := len(ctx.PartialSuccessMethods) + if partialCount == 0 && slices.Contains(ctx.AllowedMethods, "publickey") { + return PublicKeys(testSigners["rsa"]), nil + } + if partialCount == 1 && slices.Contains(ctx.AllowedMethods, "publickey") { + return PublicKeys(testSigners["ecdsa"]), nil + } + if partialCount >= 2 && slices.Contains(ctx.AllowedMethods, "keyboard-interactive") { + return KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) { + return []string{"correct-token"}, nil + }), nil + } + return nil, fmt.Errorf("unexpected auth state: allowed=%v partial=%v tried=%v", + ctx.AllowedMethods, ctx.PartialSuccessMethods, ctx.TriedMethods) + }, + } + + go NewServerConn(c1, partialSuccessMultiKeyThenKbdInteractiveServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } +} + +func TestAuthCallbackPartialSuccessKeyAThenKeyB(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", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if len(ctx.PartialSuccessMethods) == 0 && slices.Contains(ctx.AllowedMethods, "publickey") { + return PublicKeys(testSigners["rsa"]), nil + } + if len(ctx.PartialSuccessMethods) == 1 && slices.Contains(ctx.AllowedMethods, "publickey") { + return PublicKeys(testSigners["ecdsa"]), nil + } + return nil, fmt.Errorf("unexpected auth state: allowed=%v partial=%v", + ctx.AllowedMethods, ctx.PartialSuccessMethods) + }, + } + + go NewServerConn(c1, partialSuccessKeyAThenKeyBServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } +} + +func TestAuthCallbackPartialSuccessWithStaticAuth(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"]), + }, + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if slices.Contains(ctx.PartialSuccessMethods, "publickey") && slices.Contains(ctx.AllowedMethods, "keyboard-interactive") { + return KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) { + return []string{"correct-token"}, nil + }), nil + } + return nil, nil + }, + } + + go NewServerConn(c1, partialSuccessPublicKeyAndKbdInteractiveServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } +} + +func partialSuccessPubKeyForbiddenAfterPartialServer(maxTries int) *ServerConfig { + 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()) { + return nil, &PartialSuccessError{ + Next: ServerAuthCallbacks{ + PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) { + return nil, errors.New("legacy public key authentication is forbidden") + }, + 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") + }, + MaxAuthTries: maxTries, + } + serverConf.AddHostKey(testSigners["rsa"]) + return serverConf +} + +func TestAuthCallbackPartialSuccessWithStaticMultiKey(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"], + testSigners["ed25519"], + ), + }, + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if slices.Contains(ctx.PartialSuccessMethods, "publickey") && slices.Contains(ctx.AllowedMethods, "keyboard-interactive") { + return KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) { + return []string{"correct-token"}, nil + }), nil + } + return nil, nil + }, + } + + go NewServerConn(c1, partialSuccessPubKeyForbiddenAfterPartialServer(2)) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } +} + +func TestAuthCallbackPartialSuccessShortCircuit(t *testing.T) { + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + serverConf := partialSuccessPubKeyForbiddenAfterPartialServer(2) + + pubkeyAttempts := 0 + clientConf := &ClientConfig{ + User: "user", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if slices.Contains(ctx.AllowedMethods, "publickey") && !slices.Contains(ctx.PartialSuccessMethods, "publickey") { + pubkeyAttempts++ + return PublicKeys(testSigners["rsa"], testSigners["ecdsa"], testSigners["ed25519"]), nil + } + if slices.Contains(ctx.PartialSuccessMethods, "publickey") && slices.Contains(ctx.AllowedMethods, "keyboard-interactive") { + return KeyboardInteractive(func(name, instruction string, questions []string, echos []bool) ([]string, error) { + return []string{"correct-token"}, nil + }), nil + } + return nil, fmt.Errorf("unexpected auth state: allowed=%v partial=%v tried=%v", + ctx.AllowedMethods, ctx.PartialSuccessMethods, ctx.TriedMethods) + }, + } + + go NewServerConn(c1, serverConf) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err != nil { + t.Fatalf("client login error: %s", err) + } + if pubkeyAttempts != 1 { + t.Errorf("expected publickey to be attempted once before short-circuit, got %d", pubkeyAttempts) + } +} + +func TestAuthCallbackTriedLoopBound(t *testing.T) { + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + serverConf := &ServerConfig{ + MaxAuthTries: -1, + PasswordCallback: func(conn ConnMetadata, pass []byte) (*Permissions, error) { + return nil, errors.New("password rejected") + }, + } + serverConf.AddHostKey(testSigners["rsa"]) + + invocations := 0 + clientConf := &ClientConfig{ + User: "user", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + invocations++ + return Password("wrong"), nil + }, + } + + go NewServerConn(c1, serverConf) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err == nil { + t.Fatal("expected the client to abort after exceeding the attempts cap") + } + if !strings.Contains(err.Error(), "too many authentication attempts") { + t.Errorf("unexpected error: %v", err) + } + if invocations != maxAuthClientTried { + t.Errorf("AuthCallback invoked %d times; expected %d", + invocations, maxAuthClientTried) + } +} + +func alwaysPartialPubKeyServer() *ServerConfig { + var alwaysPartial func(ConnMetadata, PublicKey) (*Permissions, error) + alwaysPartial = func(conn ConnMetadata, key PublicKey) (*Permissions, error) { + if !bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) { + return nil, errors.New("invalid credentials") + } + return nil, &PartialSuccessError{ + Next: ServerAuthCallbacks{ + PublicKeyCallback: alwaysPartial, + }, + } + } + serverConf := &ServerConfig{ + MaxAuthTries: -1, + PublicKeyCallback: alwaysPartial, + } + serverConf.AddHostKey(testSigners["rsa"]) + return serverConf +} + +func TestAuthCallbackPartialSuccessLoopBound(t *testing.T) { + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + invocations := 0 + clientConf := &ClientConfig{ + User: "user", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + invocations++ + return PublicKeys(testSigners["rsa"]), nil + }, + } + + go NewServerConn(c1, alwaysPartialPubKeyServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err == nil { + t.Fatal("expected the client to abort after exceeding the attempts cap") + } + if !strings.Contains(err.Error(), "too many authentication attempts") { + t.Errorf("unexpected error: %v", err) + } + if invocations != maxAuthClientTried { + t.Errorf("AuthCallback invoked %d times; expected %d", + invocations, maxAuthClientTried) + } +} + func TestAuthMethodPassword(t *testing.T) { config := &ClientConfig{ User: "testuser", diff --git a/ssh/server_test.go b/ssh/server_test.go index 2900b61..502a25b 100644 --- a/ssh/server_test.go +++ b/ssh/server_test.go @@ -16,6 +16,7 @@ import ( "math/big" "net" "reflect" + "slices" "strings" "sync/atomic" "testing" @@ -531,12 +532,18 @@ func TestVerifiedPublicCallbackPartialSuccess(t *testing.T) { serverConf.AddHostKey(testSigners["rsa"]) clientConf := ClientConfig{ - User: "user", - Auth: []AuthMethod{ - PublicKeys(testSigners["rsa"]), - Password(clientPassword), - }, + User: "user", + Auth: nil, HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + if slices.Contains(ctx.AllowedMethods, "publickey") && !slices.Contains(ctx.PartialSuccessMethods, "publickey") { + return PublicKeys(testSigners["rsa"]), nil + } + if slices.Contains(ctx.AllowedMethods, "password") && slices.Contains(ctx.PartialSuccessMethods, "publickey") { + return Password(clientPassword), nil + } + return nil, nil + }, } go NewServerConn(c1, serverConf)