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 3cb9c244c2..716cbdaabb 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)) + } + }) + } +}