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>

2 files changed, 52 insertions, 2 deletions

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 @@ 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 f0ffd59..0fd284d 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()