crypto/ssh: return unexpected msg error when server fails keyboard-in…

…teractive auth early
golang · Sep 29, 2024 · f2a8ba2 · f2a8ba2
1 parent 42ee18b
commit f2a8ba2
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 0 deletions.
diff --git a/ssh/client_auth.go b/ssh/client_auth.go
@@ -555,6 +555,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
 	}
 
 	gotMsgExtInfo := false
+	gotUserAuthInfoRequest := false
 	for {
 		packet, err := c.readPacket()
 		if err != nil {
@@ -585,6 +586,9 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
 			if msg.PartialSuccess {
 				return authPartialSuccess, msg.Methods, nil
 			}
+			if !gotUserAuthInfoRequest {
+				return authFailure, msg.Methods, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
+			}
 			return authFailure, msg.Methods, nil
 		case msgUserAuthSuccess:
 			return authSuccess, nil, nil
@@ -596,6 +600,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
 		if err := Unmarshal(packet, &msg); err != nil {
 			return authFailure, nil, err
 		}
+		gotUserAuthInfoRequest = true
 
 		// Manually unpack the prompt/echo pairs.
 		rest := msg.Prompts

diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go
@@ -15,6 +15,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 )
 
@@ -1293,3 +1294,84 @@ func TestCertAuthOpenSSHCompat(t *testing.T) {
 		t.Fatalf("unable to dial remote side: %s", err)
 	}
 }
+
+func TestKeyboardInteractiveAuthEarlyFail(t *testing.T) {
+	const maxAuthTries = 2
+
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	// Start testserver
+	go func() {
+		config := &ServerConfig{
+			MaxAuthTries: maxAuthTries,
+			KeyboardInteractiveCallback: func(c ConnMetadata,
+				client KeyboardInteractiveChallenge) (*Permissions, error) {
+				// Fail keyboard-interactive authentication early before
+				// any prompt is sent to client.
+				return nil, errors.New("keyboard-interactive auth failed")
+			},
+			PasswordCallback: func(c ConnMetadata,
+				pass []byte) (*Permissions, error) {
+				if string(pass) == clientPassword {
+					return nil, nil
+				}
+				return nil, errors.New("password auth failed")
+			},
+		}
+		config.AddHostKey(testSigners["rsa"])
+
+		conn, chans, reqs, err := NewServerConn(c2, config)
+		if err != nil {
+			return
+		}
+		_ = conn.Close()
+
+		var wg sync.WaitGroup
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			DiscardRequests(reqs)
+		}()
+		for newChannel := range chans {
+			newChannel.Reject(Prohibited,
+				"testserver not accepting requests")
+		}
+		wg.Wait()
+	}()
+
+	// Connect to testserver expect KeyboardInteractive() to be not called and
+	// PasswordCallback() to be called.
+	passwordCallbackCalled := false
+	cfg := &ClientConfig{
+		User: "testuser",
+		Auth: []AuthMethod{
+			RetryableAuthMethod(KeyboardInteractive(func(name,
+				instruction string, questions []string,
+				echos []bool) ([]string, error) {
+				t.Errorf("unexpected call to KeyboardInteractive()")
+				return []string{clientPassword}, nil
+			}), maxAuthTries),
+			RetryableAuthMethod(PasswordCallback(func() (secret string,
+				err error) {
+				t.Logf("PasswordCallback()")
+				passwordCallbackCalled = true
+				return clientPassword, nil
+			}), maxAuthTries),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+
+	conn, _, _, _ := NewClientConn(c1, "", cfg)
+	if conn != nil {
+		conn.Close()
+	}
+
+	if !passwordCallbackCalled {
+		t.Errorf("expected PasswordCallback() to be called")
+	}
+}