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 <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Jakub Ciolek <jakub@ciolek.dev>
diff --git a/ssh/agent/client.go b/ssh/agent/client.go
index 37525e1..b357e18 100644
--- a/ssh/agent/client.go
+++ b/ssh/agent/client.go
@@ -430,8 +430,9 @@
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 @@
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 f0ffd59..0fd284d 100644
--- a/ssh/agent/client_test.go
+++ b/ssh/agent/client_test.go
@@ -7,6 +7,7 @@
import (
"bytes"
"crypto/rand"
+ "encoding/binary"
"errors"
"io"
"net"
@@ -347,6 +348,53 @@
}
}
+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()
