From 7fe3443040f2a633741b8ea76a8d0205c88cadaa Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 30 Mar 2022 12:29:44 +0200 Subject: [PATCH 1/8] ssh: add support for extension negotiation (rfc 8308) This is a rebase of the following PR https://github.com/golang/crypto/pull/197 with some changes and improvements: - added support for client certificate authentication - removed read loop from server handshake - adapted extInfoMsg to upstream changes Signed-off-by: Nicola Murino --- ssh/client_auth_test.go | 4 +--- ssh/common.go | 33 +++++++++++++++++++++++++++++++++ ssh/handshake.go | 37 ++++++++++++++++++++++++++++++++++++- ssh/server.go | 33 ++++++++++++++++++++++++++++++++- 4 files changed, 102 insertions(+), 5 deletions(-) diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index a6bedbfcaa..35b62e3311 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -132,9 +132,7 @@ func TestClientAuthPublicKey(t *testing.T) { if err := tryAuth(t, config); err != nil { t.Fatalf("unable to dial remote side: %s", err) } - // Once the server implements the server-sig-algs extension, this will turn - // into KeyAlgoRSASHA256. - if len(signer.used) != 1 || signer.used[0] != KeyAlgoRSA { + if len(signer.used) != 1 || signer.used[0] != KeyAlgoRSASHA256 { t.Errorf("unexpected Sign/SignWithAlgorithm calls: %q", signer.used) } } diff --git a/ssh/common.go b/ssh/common.go index 2a47a61ded..9e09180495 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -24,6 +24,18 @@ const ( serviceSSH = "ssh-connection" ) +// These are string constants related to extensions and extension negotiation +const ( + extInfoServer = "ext-info-s" + extInfoClient = "ext-info-c" + ExtServerSigAlgs = "server-sig-algs" +) + +// defaultExtensions lists extensions enabled by default. +var defaultExtensions = []string{ + ExtServerSigAlgs, +} + // supportedCiphers lists ciphers we support but might not recommend. var supportedCiphers = []string{ "aes128-ctr", "aes192-ctr", "aes256-ctr", @@ -89,6 +101,15 @@ var supportedMACs = []string{ var supportedCompressions = []string{compressionNone} +// supportedServerSigAlgs defines the algorithms supported for pubkey authentication +// in no particular order. +var supportedServerSigAlgs = []string{KeyAlgoRSASHA256, + KeyAlgoRSASHA512, KeyAlgoRSA, + KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, + KeyAlgoSKECDSA256, KeyAlgoED25519, KeyAlgoSKED25519, + KeyAlgoDSA, +} + // hashFuncs keeps the mapping of supported signature algorithms to their // respective hashes needed for signing and verification. var hashFuncs = map[string]crypto.Hash{ @@ -180,6 +201,10 @@ func findAgreedAlgorithms(isClient bool, clientKexInit, serverKexInit *kexInitMs result.kex, err = findCommon("key exchange", clientKexInit.KexAlgos, serverKexInit.KexAlgos) if err != nil { return + } else if result.kex == extInfoClient || result.kex == extInfoServer { + // According to RFC8308 section 2.2 if either the client or server extension signal + // is chosen as the kex algorithm the parties must disconnect. + return result, fmt.Errorf("ssh: invalid kex algorithm chosen: %s", result.kex) } result.hostKey, err = findCommon("host key", clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos) @@ -257,6 +282,10 @@ type Config struct { // The allowed MAC algorithms. If unspecified then a sensible default // is used. MACs []string + + // A list of enabled extensions. If unspecified then a sensible + // default is used + Extensions []string } // SetDefaults sets sensible values for unset fields in config. This is @@ -286,6 +315,10 @@ func (c *Config) SetDefaults() { c.MACs = supportedMACs } + if c.Extensions == nil { + c.Extensions = defaultExtensions + } + if c.RekeyThreshold == 0 { // cipher specific default } else if c.RekeyThreshold < minRekeyThreshold { diff --git a/ssh/handshake.go b/ssh/handshake.go index 653dc4d2cf..7e6e52ec7e 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -11,6 +11,7 @@ import ( "io" "log" "net" + "strings" "sync" ) @@ -94,6 +95,10 @@ type handshakeTransport struct { // The session ID or nil if first kex did not complete yet. sessionID []byte + + // True if the first ext info message has been sent immediately following + // SSH_MSG_NEWKEYS, false otherwise. + extInfoSent bool } type pendingKex struct { @@ -474,6 +479,9 @@ func (t *handshakeTransport) sendKexInit() error { msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) } } + if contains(t.config.Extensions, ExtServerSigAlgs) { + msg.KexAlgos = append(msg.KexAlgos, extInfoServer) + } } else { msg.ServerHostKeyAlgos = t.hostKeyAlgorithms @@ -483,7 +491,7 @@ func (t *handshakeTransport) sendKexInit() error { if firstKeyExchange := t.sessionID == nil; firstKeyExchange { msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) - msg.KexAlgos = append(msg.KexAlgos, "ext-info-c") + msg.KexAlgos = append(msg.KexAlgos, extInfoClient) } } @@ -632,6 +640,33 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { return unexpectedMessageError(msgNewKeys, packet[0]) } + if !isClient { + // We're on the server side, see if the client sent the extension signal + if !t.extInfoSent && contains(clientInit.KexAlgos, extInfoClient) && contains(t.config.Extensions, ExtServerSigAlgs) { + // The other side supports ext info, an ext info message hasn't been sent this session, + // and we have at least one extension enabled, so send an SSH_MSG_EXT_INFO message. + extensions := map[string][]byte{} + // We're the server, the client supports SSH_MSG_EXT_INFO and server-sig-algs + // is enabled. Prepare the server-sig-algos extension message to send. + extensions[ExtServerSigAlgs] = []byte(strings.Join(supportedServerSigAlgs, ",")) + var payload []byte + for k, v := range extensions { + payload = appendInt(payload, len(k)) + payload = append(payload, k...) + payload = appendInt(payload, len(v)) + payload = append(payload, v...) + } + extInfo := extInfoMsg{ + NumExtensions: uint32(len(extensions)), + Payload: payload, + } + if err := t.conn.writePacket(Marshal(&extInfo)); err != nil { + return err + } + t.extInfoSent = true + } + } + return nil } diff --git a/ssh/server.go b/ssh/server.go index 70045bdfd8..0dd93bc11f 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -256,11 +256,41 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error) // We just did the key change, so the session ID is established. s.sessionID = s.transport.getSessionID() + // the client could send a SSH_MSG_EXT_INFO before SSH_MSG_SERVICE_REQUEST var packet []byte if packet, err = s.transport.readPacket(); err != nil { return nil, err } + // be permissive and don't add contains(s.transport.config.Extensions, ExtServerSigAlgs) + if len(packet) > 0 && packet[0] == msgExtInfo { + // read SSH_MSG_EXT_INFO + var extInfo extInfoMsg + extensions := make(map[string][]byte) + if err := Unmarshal(packet, &extInfo); err != nil { + return nil, err + } + payload := extInfo.Payload + for i := uint32(0); i < extInfo.NumExtensions; i++ { + name, rest, ok := parseString(payload) + if !ok { + return nil, parseError(msgExtInfo) + } + value, rest, ok := parseString(rest) + if !ok { + return nil, parseError(msgExtInfo) + } + extensions[string(name)] = value + payload = rest + } + + // read the next packet + packet = nil + if packet, err = s.transport.readPacket(); err != nil { + return nil, err + } + } + var serviceRequest serviceRequestMsg if err = Unmarshal(packet, &serviceRequest); err != nil { return nil, err @@ -286,7 +316,8 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error) func isAcceptableAlgo(algo string) bool { switch algo { case KeyAlgoRSA, KeyAlgoRSASHA256, KeyAlgoRSASHA512, KeyAlgoDSA, KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, KeyAlgoSKECDSA256, KeyAlgoED25519, KeyAlgoSKED25519, - CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoSKECDSA256v01, CertAlgoED25519v01, CertAlgoSKED25519v01: + CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoSKECDSA256v01, CertAlgoED25519v01, CertAlgoSKED25519v01, + CertAlgoRSASHA256v01, CertAlgoRSASHA512v01: return true } return false From 0196e38d30c462b7e1eab4a786363939a39fb4c7 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 30 Mar 2022 13:50:30 +0200 Subject: [PATCH 2/8] remove the Extension setting always add ext-info-s to KEX and send the SSH_MSG_EXT_INFO message if we received ext-info-c from the client Signed-off-by: Nicola Murino --- ssh/common.go | 15 +-------------- ssh/handshake.go | 8 +++----- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/ssh/common.go b/ssh/common.go index 9e09180495..27343d4afa 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -28,14 +28,9 @@ const ( const ( extInfoServer = "ext-info-s" extInfoClient = "ext-info-c" - ExtServerSigAlgs = "server-sig-algs" + extServerSigAlgs = "server-sig-algs" ) -// defaultExtensions lists extensions enabled by default. -var defaultExtensions = []string{ - ExtServerSigAlgs, -} - // supportedCiphers lists ciphers we support but might not recommend. var supportedCiphers = []string{ "aes128-ctr", "aes192-ctr", "aes256-ctr", @@ -282,10 +277,6 @@ type Config struct { // The allowed MAC algorithms. If unspecified then a sensible default // is used. MACs []string - - // A list of enabled extensions. If unspecified then a sensible - // default is used - Extensions []string } // SetDefaults sets sensible values for unset fields in config. This is @@ -315,10 +306,6 @@ func (c *Config) SetDefaults() { c.MACs = supportedMACs } - if c.Extensions == nil { - c.Extensions = defaultExtensions - } - if c.RekeyThreshold == 0 { // cipher specific default } else if c.RekeyThreshold < minRekeyThreshold { diff --git a/ssh/handshake.go b/ssh/handshake.go index 7e6e52ec7e..9fdb1e95e3 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -479,9 +479,7 @@ func (t *handshakeTransport) sendKexInit() error { msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) } } - if contains(t.config.Extensions, ExtServerSigAlgs) { - msg.KexAlgos = append(msg.KexAlgos, extInfoServer) - } + msg.KexAlgos = append(msg.KexAlgos, extInfoServer) } else { msg.ServerHostKeyAlgos = t.hostKeyAlgorithms @@ -642,13 +640,13 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { if !isClient { // We're on the server side, see if the client sent the extension signal - if !t.extInfoSent && contains(clientInit.KexAlgos, extInfoClient) && contains(t.config.Extensions, ExtServerSigAlgs) { + if !t.extInfoSent && contains(clientInit.KexAlgos, extInfoClient) { // The other side supports ext info, an ext info message hasn't been sent this session, // and we have at least one extension enabled, so send an SSH_MSG_EXT_INFO message. extensions := map[string][]byte{} // We're the server, the client supports SSH_MSG_EXT_INFO and server-sig-algs // is enabled. Prepare the server-sig-algos extension message to send. - extensions[ExtServerSigAlgs] = []byte(strings.Join(supportedServerSigAlgs, ",")) + extensions[extServerSigAlgs] = []byte(strings.Join(supportedServerSigAlgs, ",")) var payload []byte for k, v := range extensions { payload = appendInt(payload, len(k)) From 2b413598b393b29479f367197daae83c8eb42907 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 30 Mar 2022 16:02:13 +0200 Subject: [PATCH 3/8] add ext-info-s to KEX algorithms only in the first key exchange Signed-off-by: Nicola Murino --- ssh/handshake.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ssh/handshake.go b/ssh/handshake.go index 9fdb1e95e3..689301516c 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -479,7 +479,11 @@ func (t *handshakeTransport) sendKexInit() error { msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) } } - msg.KexAlgos = append(msg.KexAlgos, extInfoServer) + if firstKeyExchange := t.sessionID == nil; firstKeyExchange { + msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) + msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) + msg.KexAlgos = append(msg.KexAlgos, extInfoServer) + } } else { msg.ServerHostKeyAlgos = t.hostKeyAlgorithms From 4929d4a7625667bfd8bc8b92a6ca2bbfdc7b70b0 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 30 Mar 2022 16:58:50 +0200 Subject: [PATCH 4/8] remove extInfoSent field we already know if this is the first key exchange Signed-off-by: Nicola Murino --- ssh/handshake.go | 39 +++++++++++++++++---------------------- ssh/server.go | 1 - 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/ssh/handshake.go b/ssh/handshake.go index 689301516c..150d53eccb 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -95,10 +95,6 @@ type handshakeTransport struct { // The session ID or nil if first kex did not complete yet. sessionID []byte - - // True if the first ext info message has been sent immediately following - // SSH_MSG_NEWKEYS, false otherwise. - extInfoSent bool } type pendingKex struct { @@ -625,7 +621,8 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { return err } - if t.sessionID == nil { + firstKeyExchange := t.sessionID == nil + if firstKeyExchange { t.sessionID = result.H } result.SessionID = t.sessionID @@ -643,29 +640,27 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { } if !isClient { - // We're on the server side, see if the client sent the extension signal - if !t.extInfoSent && contains(clientInit.KexAlgos, extInfoClient) { - // The other side supports ext info, an ext info message hasn't been sent this session, - // and we have at least one extension enabled, so send an SSH_MSG_EXT_INFO message. + // We're on the server side, if this is the first key exchange + // see if the client sent the extension signal + if firstKeyExchange && contains(clientInit.KexAlgos, extInfoClient) { + // The other side supports ext info, and this is the first key exchange, + // so send an SSH_MSG_EXT_INFO message. extensions := map[string][]byte{} - // We're the server, the client supports SSH_MSG_EXT_INFO and server-sig-algs - // is enabled. Prepare the server-sig-algos extension message to send. + // Prepare the server-sig-algos extension message to send. extensions[extServerSigAlgs] = []byte(strings.Join(supportedServerSigAlgs, ",")) - var payload []byte - for k, v := range extensions { - payload = appendInt(payload, len(k)) - payload = append(payload, k...) - payload = appendInt(payload, len(v)) - payload = append(payload, v...) - } - extInfo := extInfoMsg{ + + extInfo := &extInfoMsg{ NumExtensions: uint32(len(extensions)), - Payload: payload, } - if err := t.conn.writePacket(Marshal(&extInfo)); err != nil { + for k, v := range extensions { + extInfo.Payload = appendInt(extInfo.Payload, len(k)) + extInfo.Payload = append(extInfo.Payload, k...) + extInfo.Payload = appendInt(extInfo.Payload, len(v)) + extInfo.Payload = append(extInfo.Payload, v...) + } + if err := t.conn.writePacket(Marshal(extInfo)); err != nil { return err } - t.extInfoSent = true } } diff --git a/ssh/server.go b/ssh/server.go index 0dd93bc11f..eee631d437 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -285,7 +285,6 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error) } // read the next packet - packet = nil if packet, err = s.transport.readPacket(); err != nil { return nil, err } From 76c940069807150cbdc239f40f72f84deb3eb21c Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 30 Mar 2022 17:02:43 +0200 Subject: [PATCH 5/8] sendKexInit: evaluate first key exchange only once we need it for both client and server Signed-off-by: Nicola Murino --- ssh/handshake.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ssh/handshake.go b/ssh/handshake.go index 150d53eccb..eb3197fdcb 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -457,6 +457,7 @@ func (t *handshakeTransport) sendKexInit() error { io.ReadFull(rand.Reader, msg.Cookie[:]) isServer := len(t.hostKeys) > 0 + firstKeyExchange := t.sessionID == nil if isServer { for _, k := range t.hostKeys { // If k is an AlgorithmSigner, presume it supports all signature algorithms @@ -475,7 +476,7 @@ func (t *handshakeTransport) sendKexInit() error { msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) } } - if firstKeyExchange := t.sessionID == nil; firstKeyExchange { + if firstKeyExchange { msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) msg.KexAlgos = append(msg.KexAlgos, extInfoServer) @@ -486,7 +487,7 @@ func (t *handshakeTransport) sendKexInit() error { // As a client we opt in to receiving SSH_MSG_EXT_INFO so we know what // algorithms the server supports for public key authentication. See RFC // 8308, Section 2.1. - if firstKeyExchange := t.sessionID == nil; firstKeyExchange { + if firstKeyExchange { msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) msg.KexAlgos = append(msg.KexAlgos, extInfoClient) From 5835f04acddd50f994fe0ac81f9a4024e743b59a Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Thu, 31 Mar 2022 11:01:55 +0200 Subject: [PATCH 6/8] improve code comments Signed-off-by: Nicola Murino --- ssh/common.go | 5 +++-- ssh/handshake.go | 9 +++++---- ssh/server.go | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ssh/common.go b/ssh/common.go index 27343d4afa..33a88e1518 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -24,7 +24,8 @@ const ( serviceSSH = "ssh-connection" ) -// These are string constants related to extensions and extension negotiation +// These are string constants related to extensions and extension negotiation. +// See RFC 8308 const ( extInfoServer = "ext-info-s" extInfoClient = "ext-info-c" @@ -97,7 +98,7 @@ var supportedMACs = []string{ var supportedCompressions = []string{compressionNone} // supportedServerSigAlgs defines the algorithms supported for pubkey authentication -// in no particular order. +// in no particular order. See RFC 8308, Section 3.1. var supportedServerSigAlgs = []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512, KeyAlgoRSA, KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, diff --git a/ssh/handshake.go b/ssh/handshake.go index eb3197fdcb..694b853d60 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -476,6 +476,9 @@ func (t *handshakeTransport) sendKexInit() error { msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) } } + // As a server we add ext-info-s to the KEX algorithms to indicate that we support + // the Extension Negotiation Mechanism. The ext-info-s indicator must be added only + // in the first key exchange. See RFC 8308, Section 2.1. if firstKeyExchange { msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) @@ -642,12 +645,10 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { if !isClient { // We're on the server side, if this is the first key exchange - // see if the client sent the extension signal + // and the client sent the ext-info-c indicator, we send an SSH_MSG_EXT_INFO + // message with the server-sig-algs extension. See RFC 8308, Section 3.1. if firstKeyExchange && contains(clientInit.KexAlgos, extInfoClient) { - // The other side supports ext info, and this is the first key exchange, - // so send an SSH_MSG_EXT_INFO message. extensions := map[string][]byte{} - // Prepare the server-sig-algos extension message to send. extensions[extServerSigAlgs] = []byte(strings.Join(supportedServerSigAlgs, ",")) extInfo := &extInfoMsg{ diff --git a/ssh/server.go b/ssh/server.go index eee631d437..700657746f 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -256,13 +256,13 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error) // We just did the key change, so the session ID is established. s.sessionID = s.transport.getSessionID() - // the client could send a SSH_MSG_EXT_INFO before SSH_MSG_SERVICE_REQUEST + // the client could send a SSH_MSG_EXT_INFO after the first SSH_MSG_NEWKEYS + // and so before SSH_MSG_SERVICE_REQUEST. See RFC 8308, Section 2.4. var packet []byte if packet, err = s.transport.readPacket(); err != nil { return nil, err } - // be permissive and don't add contains(s.transport.config.Extensions, ExtServerSigAlgs) if len(packet) > 0 && packet[0] == msgExtInfo { // read SSH_MSG_EXT_INFO var extInfo extInfoMsg From 92ea34e7eab8f96a776aba5e50761e82a8fd42f7 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Thu, 19 May 2022 17:03:39 +0200 Subject: [PATCH 7/8] added a shared method to parse extInfoMsgs both client and server side need to parse this message Signed-off-by: Nicola Murino --- ssh/client_auth.go | 17 ++--------------- ssh/common.go | 25 +++++++++++++++++++++++++ ssh/server.go | 18 +----------------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/ssh/client_auth.go b/ssh/client_auth.go index 409b5ea1d4..a531b4c81e 100644 --- a/ssh/client_auth.go +++ b/ssh/client_auth.go @@ -35,23 +35,10 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error { // RFC 8308, Section 2.4. extensions := make(map[string][]byte) if len(packet) > 0 && packet[0] == msgExtInfo { - var extInfo extInfoMsg - if err := Unmarshal(packet, &extInfo); err != nil { + extensions, err = parseExtInfoMsg(packet) + if err != nil { return err } - payload := extInfo.Payload - for i := uint32(0); i < extInfo.NumExtensions; i++ { - name, rest, ok := parseString(payload) - if !ok { - return parseError(msgExtInfo) - } - value, rest, ok := parseString(rest) - if !ok { - return parseError(msgExtInfo) - } - extensions[string(name)] = value - payload = rest - } packet, err = c.transport.readPacket() if err != nil { return err diff --git a/ssh/common.go b/ssh/common.go index 33a88e1518..39cbbf734e 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -146,6 +146,31 @@ func parseError(tag uint8) error { return fmt.Errorf("ssh: parse error in message type %d", tag) } +// parseExtInfoMsg returns the extensions from an extInfoMsg packet. +// packet must be an already validated extInfoMsg +func parseExtInfoMsg(packet []byte) (map[string][]byte, error) { + extensions := make(map[string][]byte) + var extInfo extInfoMsg + + if err := Unmarshal(packet, &extInfo); err != nil { + return nil, err + } + payload := extInfo.Payload + for i := uint32(0); i < extInfo.NumExtensions; i++ { + name, rest, ok := parseString(payload) + if !ok { + return nil, parseError(msgExtInfo) + } + value, rest, ok := parseString(rest) + if !ok { + return nil, parseError(msgExtInfo) + } + extensions[string(name)] = value + payload = rest + } + return extensions, nil +} + func findCommon(what string, client []string, server []string) (common string, err error) { for _, c := range client { for _, s := range server { diff --git a/ssh/server.go b/ssh/server.go index 700657746f..20c09e58b4 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -265,25 +265,9 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error) if len(packet) > 0 && packet[0] == msgExtInfo { // read SSH_MSG_EXT_INFO - var extInfo extInfoMsg - extensions := make(map[string][]byte) - if err := Unmarshal(packet, &extInfo); err != nil { + if _, err := parseExtInfoMsg(packet); err != nil { return nil, err } - payload := extInfo.Payload - for i := uint32(0); i < extInfo.NumExtensions; i++ { - name, rest, ok := parseString(payload) - if !ok { - return nil, parseError(msgExtInfo) - } - value, rest, ok := parseString(rest) - if !ok { - return nil, parseError(msgExtInfo) - } - extensions[string(name)] = value - payload = rest - } - // read the next packet if packet, err = s.transport.readPacket(); err != nil { return nil, err From 8cff98973996ea82734de3db273cf47ae2b2e3bf Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 15 Jun 2022 10:02:07 +0200 Subject: [PATCH 8/8] send server-sig-algs using the same order as OpenSSH Signed-off-by: Nicola Murino --- ssh/common.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ssh/common.go b/ssh/common.go index 39cbbf734e..ba781a5303 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -97,13 +97,13 @@ var supportedMACs = []string{ var supportedCompressions = []string{compressionNone} -// supportedServerSigAlgs defines the algorithms supported for pubkey authentication -// in no particular order. See RFC 8308, Section 3.1. -var supportedServerSigAlgs = []string{KeyAlgoRSASHA256, - KeyAlgoRSASHA512, KeyAlgoRSA, - KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, - KeyAlgoSKECDSA256, KeyAlgoED25519, KeyAlgoSKED25519, - KeyAlgoDSA, +// supportedServerSigAlgs defines the algorithms supported for pubkey authentication. +// Order should not matter, but to avoid any issues we use the same order as OpenSSH. +// See RFC 8308, Section 3.1. +var supportedServerSigAlgs = []string{KeyAlgoED25519, KeyAlgoSKED25519, + KeyAlgoRSA, KeyAlgoRSASHA256, KeyAlgoRSASHA512, + KeyAlgoDSA, KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, + KeyAlgoSKECDSA256, } // hashFuncs keeps the mapping of supported signature algorithms to their