Skip to content

Commit

Permalink
ssh: add AlgorithmNegotiationError
Browse files Browse the repository at this point in the history
Fixes golang/go#61536

Change-Id: Id38cc6d46879dbe2bdea04dec061596387ec6cfe
  • Loading branch information
drakkan committed Jun 4, 2024
1 parent 5ec5df5 commit 797fc2b
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 11 deletions.
2 changes: 1 addition & 1 deletion ssh/client_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func pickSignatureAlgorithm(signer Signer, extensions map[string][]byte) (MultiA
}
}

algo, err := findCommon("public key signature algorithm", keyAlgos, serverAlgos)
algo, err := findCommon("public key signature algorithm", keyAlgos, serverAlgos, true)
if err != nil {
// If there is no overlap, return the fallback algorithm to support
// servers that fail to list all supported algorithms.
Expand Down
49 changes: 39 additions & 10 deletions ssh/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,15 +336,44 @@ func parseError(tag uint8) error {
return fmt.Errorf("ssh: parse error in message type %d", tag)
}

func findCommon(what string, client []string, server []string) (common string, err error) {
func findCommon(what string, client []string, server []string, isClient bool) (string, error) {
for _, c := range client {
for _, s := range server {
if c == s {
return c, nil
}
}
}
return "", fmt.Errorf("ssh: no common algorithm for %s; client offered: %v, server offered: %v", what, client, server)
err := &AlgorithmNegotiationError{
What: what,
isClient: isClient,
}
if isClient {
err.SupportedAlgorithms = server
err.RequestedAlgorithms = client
} else {
err.SupportedAlgorithms = client
err.RequestedAlgorithms = server
}
return "", err
}

// AlgorithmNegotiationError defines the error returned if the client and the
// server cannot agree on an algorithm for key exchange, host key, cipher, MAC.
type AlgorithmNegotiationError struct {
What string
RequestedAlgorithms []string
SupportedAlgorithms []string
isClient bool
}

func (a *AlgorithmNegotiationError) Error() string {
if a.isClient {
return fmt.Sprintf("ssh: no common algorithm for %s; client offered: %v, server offered: %v",
a.What, a.RequestedAlgorithms, a.SupportedAlgorithms)
}
return fmt.Sprintf("ssh: no common algorithm for %s; client offered: %v, server offered: %v",
a.What, a.SupportedAlgorithms, a.RequestedAlgorithms)
}

// DirectionAlgorithms defines the algorithms negotiated in one direction
Expand Down Expand Up @@ -379,12 +408,12 @@ var aeadCiphers = map[string]bool{
func findAgreedAlgorithms(isClient bool, clientKexInit, serverKexInit *kexInitMsg) (algs *NegotiatedAlgorithms, err error) {
result := &NegotiatedAlgorithms{}

result.KeyExchange, err = findCommon("key exchange", clientKexInit.KexAlgos, serverKexInit.KexAlgos)
result.KeyExchange, err = findCommon("key exchange", clientKexInit.KexAlgos, serverKexInit.KexAlgos, isClient)
if err != nil {
return
}

result.HostKey, err = findCommon("host key", clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos)
result.HostKey, err = findCommon("host key", clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos, isClient)
if err != nil {
return
}
Expand All @@ -394,36 +423,36 @@ func findAgreedAlgorithms(isClient bool, clientKexInit, serverKexInit *kexInitMs
ctos, stoc = stoc, ctos
}

ctos.Cipher, err = findCommon("client to server cipher", clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer)
ctos.Cipher, err = findCommon("client to server cipher", clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer, isClient)
if err != nil {
return
}

stoc.Cipher, err = findCommon("server to client cipher", clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient)
stoc.Cipher, err = findCommon("server to client cipher", clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient, isClient)
if err != nil {
return
}

if !aeadCiphers[ctos.Cipher] {
ctos.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer)
ctos.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer, isClient)
if err != nil {
return
}
}

if !aeadCiphers[stoc.Cipher] {
stoc.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient)
stoc.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient, isClient)
if err != nil {
return
}
}

ctos.compression, err = findCommon("client to server compression", clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer)
ctos.compression, err = findCommon("client to server compression", clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer, isClient)
if err != nil {
return
}

stoc.compression, err = findCommon("server to client compression", clientKexInit.CompressionServerClient, serverKexInit.CompressionServerClient)
stoc.compression, err = findCommon("server to client compression", clientKexInit.CompressionServerClient, serverKexInit.CompressionServerClient, isClient)
if err != nil {
return
}
Expand Down
49 changes: 49 additions & 0 deletions ssh/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1074,3 +1074,52 @@ func TestNegotiatedAlgorithms(t *testing.T) {
}
}
}

func TestAlgorithmNegotiationError(t *testing.T) {
c1, c2, err := netPipe()
if err != nil {
t.Fatalf("netPipe: %v", err)
}
defer c1.Close()
defer c2.Close()

serverConf := &ServerConfig{
Config: Config{
Ciphers: []string{CipherAES128CTR, CipherAES256CTR},
},
PasswordCallback: func(conn ConnMetadata, password []byte) (*Permissions, error) {
return &Permissions{}, nil
},
}
serverConf.AddHostKey(testSigners["rsa"])
go NewServerConn(c1, serverConf)

clientConf := &ClientConfig{
Config: Config{
Ciphers: []string{CipherAES128GCM, CipherAES256GCM},
},
User: "test",
Auth: []AuthMethod{Password("testpw")},
HostKeyCallback: FixedHostKey(testSigners["rsa"].PublicKey()),
}

_, _, _, err = NewClientConn(c2, "", clientConf)
if err == nil {
t.Fatal("client connection succeded expected algorithm negotiation error")
}
var negotiationError *AlgorithmNegotiationError
if !errors.As(err, &negotiationError) {
t.Fatalf("expected algorithm negotiation error, got %v", err)
}
expectedErrorString := fmt.Sprintf("ssh: handshake failed: ssh: no common algorithm for client to server cipher; client offered: %v, server offered: %v",
clientConf.Ciphers, serverConf.Ciphers)
if err.Error() != expectedErrorString {
t.Fatalf("expected error string %q, got %q", expectedErrorString, err.Error())
}
if !reflect.DeepEqual(negotiationError.RequestedAlgorithms, clientConf.Ciphers) {
t.Fatalf("expected requested algorithms %v, got %v", clientConf.Ciphers, negotiationError.RequestedAlgorithms)
}
if !reflect.DeepEqual(negotiationError.SupportedAlgorithms, serverConf.Ciphers) {
t.Fatalf("expected supported algorithms %v, got %v", serverConf.Ciphers, negotiationError.SupportedAlgorithms)
}
}

0 comments on commit 797fc2b

Please sign in to comment.