Skip to content

Commit b0e0ee1

Browse files
committed
crypto/ssh: return unexpected msg error when server fails keyboard-interactive auth early
1 parent d4e7c9c commit b0e0ee1

File tree

2 files changed

+87
-0
lines changed

2 files changed

+87
-0
lines changed

Diff for: ssh/client_auth.go

+5
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
555555
}
556556

557557
gotMsgExtInfo := false
558+
gotUserAuthInfoRequest := false
558559
for {
559560
packet, err := c.readPacket()
560561
if err != nil {
@@ -585,6 +586,9 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
585586
if msg.PartialSuccess {
586587
return authPartialSuccess, msg.Methods, nil
587588
}
589+
if !gotUserAuthInfoRequest {
590+
return authFailure, msg.Methods, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
591+
}
588592
return authFailure, msg.Methods, nil
589593
case msgUserAuthSuccess:
590594
return authSuccess, nil, nil
@@ -596,6 +600,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
596600
if err := Unmarshal(packet, &msg); err != nil {
597601
return authFailure, nil, err
598602
}
603+
gotUserAuthInfoRequest = true
599604

600605
// Manually unpack the prompt/echo pairs.
601606
rest := msg.Prompts

Diff for: ssh/client_auth_test.go

+82
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"os"
1616
"runtime"
1717
"strings"
18+
"sync"
1819
"testing"
1920
)
2021

@@ -1282,3 +1283,84 @@ func TestCertAuthOpenSSHCompat(t *testing.T) {
12821283
t.Fatalf("unable to dial remote side: %s", err)
12831284
}
12841285
}
1286+
1287+
func TestKeyboardInteractiveAuthEarlyFail(t *testing.T) {
1288+
const maxAuthTries = 2
1289+
1290+
c1, c2, err := netPipe()
1291+
if err != nil {
1292+
t.Fatalf("netPipe: %v", err)
1293+
}
1294+
defer c1.Close()
1295+
defer c2.Close()
1296+
1297+
// Start testserver
1298+
go func() {
1299+
config := &ServerConfig{
1300+
MaxAuthTries: maxAuthTries,
1301+
KeyboardInteractiveCallback: func(c ConnMetadata,
1302+
client KeyboardInteractiveChallenge) (*Permissions, error) {
1303+
// Fail keyboard-interactive authentication early before
1304+
// any prompt is sent to client.
1305+
return nil, errors.New("keyboard-interactive auth failed")
1306+
},
1307+
PasswordCallback: func(c ConnMetadata,
1308+
pass []byte) (*Permissions, error) {
1309+
if string(pass) == clientPassword {
1310+
return nil, nil
1311+
}
1312+
return nil, errors.New("password auth failed")
1313+
},
1314+
}
1315+
config.AddHostKey(testSigners["rsa"])
1316+
1317+
conn, chans, reqs, err := NewServerConn(c2, config)
1318+
if err != nil {
1319+
return
1320+
}
1321+
_ = conn.Close()
1322+
1323+
var wg sync.WaitGroup
1324+
wg.Add(1)
1325+
go func() {
1326+
defer wg.Done()
1327+
DiscardRequests(reqs)
1328+
}()
1329+
for newChannel := range chans {
1330+
newChannel.Reject(Prohibited,
1331+
"testserver not accepting requests")
1332+
}
1333+
wg.Wait()
1334+
}()
1335+
1336+
// Connect to testserver expect KeyboardInteractive() to be not called and
1337+
// PasswordCallback() to be called.
1338+
passwordCallbackCalled := false
1339+
cfg := &ClientConfig{
1340+
User: "testuser",
1341+
Auth: []AuthMethod{
1342+
RetryableAuthMethod(KeyboardInteractive(func(name,
1343+
instruction string, questions []string,
1344+
echos []bool) ([]string, error) {
1345+
t.Errorf("unexpected call to KeyboardInteractive()")
1346+
return []string{clientPassword}, nil
1347+
}), maxAuthTries),
1348+
RetryableAuthMethod(PasswordCallback(func() (secret string,
1349+
err error) {
1350+
t.Logf("PasswordCallback()")
1351+
passwordCallbackCalled = true
1352+
return clientPassword, nil
1353+
}), maxAuthTries),
1354+
},
1355+
HostKeyCallback: InsecureIgnoreHostKey(),
1356+
}
1357+
1358+
conn, _, _, _ := NewClientConn(c1, "", cfg)
1359+
if conn != nil {
1360+
conn.Close()
1361+
}
1362+
1363+
if !passwordCallbackCalled {
1364+
t.Errorf("expected PasswordCallback() to be called")
1365+
}
1366+
}

0 commit comments

Comments
 (0)