Skip to content

Commit 559e062

Browse files
committed
ssh/agent: return an error for unexpected message types
Previously, receiving an unexpected message type in response to a key listing or a signing request could cause a panic due to a failed type assertion. This change adds a default case to the type switch in order to detect and explicitly handle unknown or invalid message types, returning a descriptive error instead of crashing. Fixes golang/go#75178 Change-Id: Icbc3432adc79fe3c56b1ff23c6724d7a6f710f3a Reviewed-on: https://go-review.googlesource.com/c/crypto/+/700295 Reviewed-by: Roland Shoemaker <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Reviewed-by: Jakub Ciolek <[email protected]>
1 parent 5307a0c commit 559e062

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

ssh/agent/client.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,9 @@ func (c *client) List() ([]*Key, error) {
430430
return keys, nil
431431
case *failureAgentMsg:
432432
return nil, errors.New("agent: failed to list keys")
433+
default:
434+
return nil, fmt.Errorf("agent: failed to list keys, unexpected message type %T", msg)
433435
}
434-
panic("unreachable")
435436
}
436437

437438
// Sign has the agent sign the data using a protocol 2 key as defined
@@ -462,8 +463,9 @@ func (c *client) SignWithFlags(key ssh.PublicKey, data []byte, flags SignatureFl
462463
return &sig, nil
463464
case *failureAgentMsg:
464465
return nil, errors.New("agent: failed to sign challenge")
466+
default:
467+
return nil, fmt.Errorf("agent: failed to sign challenge, unexpected message type %T", msg)
465468
}
466-
panic("unreachable")
467469
}
468470

469471
// unmarshal parses an agent message in packet, returning the parsed

ssh/agent/client_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package agent
77
import (
88
"bytes"
99
"crypto/rand"
10+
"encoding/binary"
1011
"errors"
1112
"io"
1213
"net"
@@ -347,6 +348,53 @@ func TestServerResponseTooLarge(t *testing.T) {
347348
}
348349
}
349350

351+
func TestInvalidResponses(t *testing.T) {
352+
a, b, err := netPipe()
353+
if err != nil {
354+
t.Fatalf("netPipe: %v", err)
355+
}
356+
done := make(chan struct{})
357+
defer func() { <-done }()
358+
359+
defer a.Close()
360+
defer b.Close()
361+
362+
agent := NewClient(a)
363+
go func() {
364+
defer close(done)
365+
366+
resp := []byte{agentSuccess}
367+
msg := make([]byte, 4+len(resp))
368+
binary.BigEndian.PutUint32(msg[:4], uint32(len(resp)))
369+
copy(msg[4:], resp)
370+
371+
if _, err := b.Write(msg); err != nil {
372+
t.Errorf("unexpected error sending agent reply: %v", err)
373+
b.Close()
374+
return
375+
}
376+
377+
if _, err := b.Write(msg); err != nil {
378+
t.Errorf("unexpected error sending agent reply: %v", err)
379+
b.Close()
380+
}
381+
}()
382+
_, err = agent.List()
383+
if err == nil {
384+
t.Fatal("error expected")
385+
}
386+
if !strings.Contains(err.Error(), "failed to list keys, unexpected message type") {
387+
t.Fatalf("unexpected error: %v", err)
388+
}
389+
_, err = agent.Sign(testPublicKeys["rsa"], []byte("message"))
390+
if err == nil {
391+
t.Fatal("error expected")
392+
}
393+
if !strings.Contains(err.Error(), "failed to sign challenge, unexpected message type") {
394+
t.Fatalf("unexpected error: %v", err)
395+
}
396+
}
397+
350398
func TestAuth(t *testing.T) {
351399
agent, _, cleanup := startOpenSSHAgent(t)
352400
defer cleanup()

0 commit comments

Comments
 (0)