From da373b8be6ce3270caa340bea1d142234066cba3 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 31 Aug 2025 20:07:32 +0200 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Pratt Reviewed-by: Jakub Ciolek --- ssh/agent/client.go | 6 +++-- ssh/agent/client_test.go | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/ssh/agent/client.go b/ssh/agent/client.go index 37525e1a18..b357e18b0a 100644 --- a/ssh/agent/client.go +++ b/ssh/agent/client.go @@ -430,8 +430,9 @@ func (c *client) List() ([]*Key, error) { return keys, nil case *failureAgentMsg: return nil, errors.New("agent: failed to list keys") + default: + return nil, fmt.Errorf("agent: failed to list keys, unexpected message type %T", msg) } - panic("unreachable") } // 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 return &sig, nil case *failureAgentMsg: return nil, errors.New("agent: failed to sign challenge") + default: + return nil, fmt.Errorf("agent: failed to sign challenge, unexpected message type %T", msg) } - panic("unreachable") } // unmarshal parses an agent message in packet, returning the parsed diff --git a/ssh/agent/client_test.go b/ssh/agent/client_test.go index f0ffd59592..0fd284d786 100644 --- a/ssh/agent/client_test.go +++ b/ssh/agent/client_test.go @@ -7,6 +7,7 @@ package agent import ( "bytes" "crypto/rand" + "encoding/binary" "errors" "io" "net" @@ -347,6 +348,53 @@ func TestServerResponseTooLarge(t *testing.T) { } } +func TestInvalidResponses(t *testing.T) { + a, b, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + done := make(chan struct{}) + defer func() { <-done }() + + defer a.Close() + defer b.Close() + + agent := NewClient(a) + go func() { + defer close(done) + + resp := []byte{agentSuccess} + msg := make([]byte, 4+len(resp)) + binary.BigEndian.PutUint32(msg[:4], uint32(len(resp))) + copy(msg[4:], resp) + + if _, err := b.Write(msg); err != nil { + t.Errorf("unexpected error sending agent reply: %v", err) + b.Close() + return + } + + if _, err := b.Write(msg); err != nil { + t.Errorf("unexpected error sending agent reply: %v", err) + b.Close() + } + }() + _, err = agent.List() + if err == nil { + t.Fatal("error expected") + } + if !strings.Contains(err.Error(), "failed to list keys, unexpected message type") { + t.Fatalf("unexpected error: %v", err) + } + _, err = agent.Sign(testPublicKeys["rsa"], []byte("message")) + if err == nil { + t.Fatal("error expected") + } + if !strings.Contains(err.Error(), "failed to sign challenge, unexpected message type") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestAuth(t *testing.T) { agent, _, cleanup := startOpenSSHAgent(t) defer cleanup()