From 13018879d582a1c6f71888d7872a2fe3a0c3fb67 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 9 Dec 2023 14:25:29 +0100 Subject: [PATCH] ssh: add server side support for Diffie Hellman Group Exchange We add this support for the following reasons: - We are planning to expose recommended (secure) vs. supported (works, not necessarily recommended) algorithms. The DHGEX kex is currently only exposed as a client-side kex. To simplify the calling convention for this follow-on, we expose the server side too. - Some clients are quite inflexible with reference to kex algorithms choice, for example they offer: diffie-hellman-group-exchange-sha256, diffie-hellman-group-exchange-sha1, diffie-hellman-group14-sha1, diffie-hellman-group1-sha1 therefore DHGEX helps interoperability. We do not recommend the DHGEX kex as a whole: - the negotiation requires an extra round trip - the server must generate parameters (slow) or hardcode them, which defeats the security benefit over traditional DH. In this implementation we hardcode sending Oakley Group 14 or Oakley Group 16. Users that are concerned with security of classical DH kex should migrate to kex based on EC or Ed25519. Fixes golang/go#54743 Change-Id: I127822e90efc36821af4aca679931f40a2023021 --- ssh/common.go | 7 ---- ssh/kex.go | 69 +++++++++++++++++++++++++++--------- ssh/messages.go | 6 ++-- ssh/server.go | 7 ---- ssh/test/sshcli_test.go | 77 +++++++++++++++++++++++++++++++++++------ 5 files changed, 121 insertions(+), 45 deletions(-) diff --git a/ssh/common.go b/ssh/common.go index 14b1b7d37f..1cdd008934 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -211,13 +211,6 @@ func InsecureAlgorithms() Algorithms { } } -// serverForbiddenKexAlgos contains key exchange algorithms, that are forbidden -// for the server half. -var serverForbiddenKexAlgos = map[string]struct{}{ - InsecureKeyExchangeDHGEXSHA1: {}, // server half implementation is only minimal to satisfy the automated tests - KeyExchangeDHGEXSHA256: {}, // server half implementation is only minimal to satisfy the automated tests -} - var supportedCompressions = []string{compressionNone} // hashFuncs keeps the mapping of supported signature algorithms to their diff --git a/ssh/kex.go b/ssh/kex.go index 0becc7c2f5..1f5546aced 100644 --- a/ssh/kex.go +++ b/ssh/kex.go @@ -19,6 +19,15 @@ import ( "golang.org/x/crypto/curve25519" ) +const ( + // Oakley Group 2 defined in RFC 2409. + oakleyGroup2 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF" + // Oakley Group 14 defined in RFC 3526. + oakleyGroup14 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF" + // Oakley Group 16 defined in RFC 3526. + oakleyGroup16 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AAAC42DAD33170D04507A33A85521ABDF1CBA64ECFB850458DBEF0A8AEA71575D060C7DB3970F85A6E1E4C7ABF5AE8CDB0933D71E8C94E04A25619DCEE3D2261AD2EE6BF12FFA06D98A0864D87602733EC86A64521F2B18177B200CBBE117577A615D6C770988C0BAD946E208E24FA074E5AB3143DB5BFCE0FD108E4B82D120A92108011A723C12A787E6D788719A10BDBA5B2699C327186AF4E23C1A946834B6150BDA2583E9CA2AD44CE8DBBBC2DB04DE8EF92E8EFC141FBECAA6287C59474E6BC05D99B2964FA090C3A2233BA186515BE7ED1F612970CEE2D7AFB81BDD762170481CD0069127D5B05AA993B4EA988D8FDDC186FFB7DC90A6C08F4DF435C934063199FFFFFFFFFFFFFFFF" +) + // kexResult captures the outcome of a key exchange. type kexResult struct { // Session hash. See also RFC 4253, section 8. @@ -386,7 +395,7 @@ var kexAlgoMap = map[string]kexAlgorithm{} func init() { // This is the group called diffie-hellman-group1-sha1 in // RFC 4253 and Oakley Group 2 in RFC 2409. - p, _ := new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF", 16) + p, _ := new(big.Int).SetString(oakleyGroup2, 16) kexAlgoMap[InsecureKeyExchangeDH1SHA1] = &dhGroup{ g: new(big.Int).SetInt64(2), p: p, @@ -397,7 +406,7 @@ func init() { // This are the groups called diffie-hellman-group14-sha1 and // diffie-hellman-group14-sha256 in RFC 4253 and RFC 8268, // and Oakley Group 14 in RFC 3526. - p, _ = new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF", 16) + p, _ = new(big.Int).SetString(oakleyGroup14, 16) group14 := &dhGroup{ g: new(big.Int).SetInt64(2), p: p, @@ -415,7 +424,7 @@ func init() { // This is the group called diffie-hellman-group16-sha512 in RFC // 8268 and Oakley Group 16 in RFC 3526. - p, _ = new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AAAC42DAD33170D04507A33A85521ABDF1CBA64ECFB850458DBEF0A8AEA71575D060C7DB3970F85A6E1E4C7ABF5AE8CDB0933D71E8C94E04A25619DCEE3D2261AD2EE6BF12FFA06D98A0864D87602733EC86A64521F2B18177B200CBBE117577A615D6C770988C0BAD946E208E24FA074E5AB3143DB5BFCE0FD108E4B82D120A92108011A723C12A787E6D788719A10BDBA5B2699C327186AF4E23C1A946834B6150BDA2583E9CA2AD44CE8DBBBC2DB04DE8EF92E8EFC141FBECAA6287C59474E6BC05D99B2964FA090C3A2233BA186515BE7ED1F612970CEE2D7AFB81BDD762170481CD0069127D5B05AA993B4EA988D8FDDC186FFB7DC90A6C08F4DF435C934063199FFFFFFFFFFFFFFFF", 16) + p, _ = new(big.Int).SetString(oakleyGroup16, 16) kexAlgoMap[KeyExchangeDH16SHA512] = &dhGroup{ g: new(big.Int).SetInt64(2), @@ -583,9 +592,9 @@ const ( func (gex *dhGEXSHA) Client(c packetConn, randSource io.Reader, magics *handshakeMagics) (*kexResult, error) { // Send GexRequest kexDHGexRequest := kexDHGexRequestMsg{ - MinBits: dhGroupExchangeMinimumBits, - PreferedBits: dhGroupExchangePreferredBits, - MaxBits: dhGroupExchangeMaximumBits, + MinBits: dhGroupExchangeMinimumBits, + PreferredBits: dhGroupExchangePreferredBits, + MaxBits: dhGroupExchangeMaximumBits, } if err := c.writePacket(Marshal(&kexDHGexRequest)); err != nil { return nil, err @@ -672,9 +681,7 @@ func (gex *dhGEXSHA) Client(c packetConn, randSource io.Reader, magics *handshak } // Server half implementation of the Diffie Hellman Key Exchange with SHA1 and SHA256. -// -// This is a minimal implementation to satisfy the automated tests. -func (gex dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshakeMagics, priv AlgorithmSigner, algo string) (result *kexResult, err error) { +func (gex *dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshakeMagics, priv AlgorithmSigner, algo string) (result *kexResult, err error) { // Receive GexRequest packet, err := c.readPacket() if err != nil { @@ -684,13 +691,41 @@ func (gex dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshake if err = Unmarshal(packet, &kexDHGexRequest); err != nil { return } + // We check that the request received is valid and that the MaxBits + // requested are at least equal to our supported minimum. This is the same + // check done in OpenSSH: + // https://github.com/openssh/openssh-portable/blob/80a2f64b8c1d27383cc83d182b73920d1e6a91f1/kexgexs.c#L94 + if kexDHGexRequest.MaxBits < kexDHGexRequest.MinBits || kexDHGexRequest.PreferredBits < kexDHGexRequest.MinBits || + kexDHGexRequest.MaxBits < kexDHGexRequest.PreferredBits || kexDHGexRequest.MaxBits < dhGroupExchangeMinimumBits { + return nil, fmt.Errorf("ssh: DH GEX request out of range, min: %d, max: %d, preferred: %d", kexDHGexRequest.MinBits, + kexDHGexRequest.MaxBits, kexDHGexRequest.PreferredBits) + } + minBits := uint32(dhGroupExchangeMinimumBits) + // We limit the maximum number of bits supported to 4096 because Oakley + // groups with multiple bits are too slow to be useful in our + // implementation. + maxBits := uint32(4096) + preferred := kexDHGexRequest.PreferredBits + // Adjust the preferred bits requested by the client so that they are within + // our range. + if preferred > maxBits { + preferred = maxBits + } + if preferred < minBits { + preferred = minBits + } + + var p *big.Int + // We hardcode sending Oakley Group 14 (2048 bits) or Oakley Group 16 (4096 + // bits), so we choose Group14 if the preferred bits are less than or equal + // to 3072, otherwise Group 16. + if preferred <= 3072 { + p, _ = new(big.Int).SetString(oakleyGroup14, 16) + } else { + p, _ = new(big.Int).SetString(oakleyGroup16, 16) + } - // Send GexGroup - // This is the group called diffie-hellman-group14-sha1 in RFC - // 4253 and Oakley Group 14 in RFC 3526. - p, _ := new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF", 16) g := big.NewInt(2) - msg := &kexDHGexGroupMsg{ P: p, G: g, @@ -728,9 +763,9 @@ func (gex dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshake h := gex.hashFunc.New() magics.write(h) writeString(h, hostKeyBytes) - binary.Write(h, binary.BigEndian, uint32(dhGroupExchangeMinimumBits)) - binary.Write(h, binary.BigEndian, uint32(dhGroupExchangePreferredBits)) - binary.Write(h, binary.BigEndian, uint32(dhGroupExchangeMaximumBits)) + binary.Write(h, binary.BigEndian, kexDHGexRequest.MinBits) + binary.Write(h, binary.BigEndian, kexDHGexRequest.PreferredBits) + binary.Write(h, binary.BigEndian, kexDHGexRequest.MaxBits) writeInt(h, p) writeInt(h, g) writeInt(h, kexDHGexInit.X) diff --git a/ssh/messages.go b/ssh/messages.go index b55f860564..419bab2b63 100644 --- a/ssh/messages.go +++ b/ssh/messages.go @@ -122,9 +122,9 @@ type kexDHGexReplyMsg struct { const msgKexDHGexRequest = 34 type kexDHGexRequestMsg struct { - MinBits uint32 `sshtype:"34"` - PreferedBits uint32 - MaxBits uint32 + MinBits uint32 `sshtype:"34"` + PreferredBits uint32 + MaxBits uint32 } // See RFC 4253, section 10. diff --git a/ssh/server.go b/ssh/server.go index ae211e6a98..03983772eb 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -218,13 +218,6 @@ func NewServerConn(c net.Conn, config *ServerConfig) (*ServerConn, <-chan NewCha } } } - // Check if the config contains any unsupported key exchanges - for _, kex := range fullConf.KeyExchanges { - if _, ok := serverForbiddenKexAlgos[kex]; ok { - c.Close() - return nil, nil, nil, fmt.Errorf("ssh: unsupported key exchange %s for server", kex) - } - } s := &connection{ sshConn: sshConn{conn: c}, diff --git a/ssh/test/sshcli_test.go b/ssh/test/sshcli_test.go index ac2f7c10a9..6648067459 100644 --- a/ssh/test/sshcli_test.go +++ b/ssh/test/sshcli_test.go @@ -34,23 +34,29 @@ func sshClient(t *testing.T) string { return sshCLI } +// setupSSHCLIKeys writes the provided key files to a temporary directory and +// returns the path to the private key. +func setupSSHCLIKeys(t *testing.T, keyFiles map[string][]byte, privKeyName string) string { + tmpDir := t.TempDir() + for fn, content := range keyFiles { + if err := os.WriteFile(filepath.Join(tmpDir, fn), content, 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", fn, err) + } + } + return filepath.Join(tmpDir, privKeyName) +} + func TestSSHCLIAuth(t *testing.T) { if runtime.GOOS == "windows" { t.Skipf("always fails on Windows, see #64403") } sshCLI := sshClient(t) - dir := t.TempDir() - keyPrivPath := filepath.Join(dir, "rsa") - - for fn, content := range map[string][]byte{ - keyPrivPath: testdata.PEMBytes["rsa"], - keyPrivPath + ".pub": ssh.MarshalAuthorizedKey(testPublicKeys["rsa"]), - filepath.Join(dir, "rsa-cert.pub"): testdata.SSHCertificates["rsa-user-testcertificate"], - } { - if err := os.WriteFile(fn, content, 0600); err != nil { - t.Fatalf("WriteFile(%q): %v", fn, err) - } + keyFiles := map[string][]byte{ + "rsa": testdata.PEMBytes["rsa"], + "rsa.pub": ssh.MarshalAuthorizedKey(testPublicKeys["rsa"]), + "rsa-cert.pub": testdata.SSHCertificates["rsa-user-testcertificate"], } + keyPrivPath := setupSSHCLIKeys(t, keyFiles, "rsa") certChecker := ssh.CertChecker{ IsUserAuthority: func(k ssh.PublicKey) bool { @@ -98,3 +104,52 @@ func TestSSHCLIAuth(t *testing.T) { t.Fatalf("user certificate authentication failed, error: %v, command output %q", err, string(out)) } } + +func TestSSHCLIKeyExchanges(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skipf("always fails on Windows, see #64403") + } + sshCLI := sshClient(t) + keyFiles := map[string][]byte{ + "rsa": testdata.PEMBytes["rsa"], + "rsa.pub": ssh.MarshalAuthorizedKey(testPublicKeys["rsa"]), + } + keyPrivPath := setupSSHCLIKeys(t, keyFiles, "rsa") + + keyExchanges := append(ssh.SupportedAlgorithms().KeyExchanges, ssh.InsecureAlgorithms().KeyExchanges...) + for _, kex := range keyExchanges { + t.Run(kex, func(t *testing.T) { + config := &ssh.ServerConfig{ + Config: ssh.Config{ + KeyExchanges: []string{kex}, + }, + PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { + if conn.User() == "testpubkey" && bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) { + return nil, nil + } + + return nil, fmt.Errorf("pubkey for %q not acceptable", conn.User()) + }, + } + config.AddHostKey(testSigners["rsa"]) + + server, err := newTestServer(config) + if err != nil { + t.Fatalf("unable to start test server: %v", err) + } + defer server.Close() + + port, err := server.port() + if err != nil { + t.Fatalf("unable to get server port: %v", err) + } + + cmd := testenv.Command(t, sshCLI, "-vvv", "-i", keyPrivPath, "-o", "StrictHostKeyChecking=no", + "-o", fmt.Sprintf("KexAlgorithms=%s", kex), "-p", port, "testpubkey@127.0.0.1", "true") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%s failed, error: %v, command output %q", kex, err, string(out)) + } + }) + } +}