Skip to content

Commit ebc8e46

Browse files
drakkangopherbot
drakkan
authored and
gopherbot
committed
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, Oakley Group 15 or Oakley Group 16 based on the requested max size. 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 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/532415 Reviewed-by: Michael Knyszek <mknyszek@google.com> Auto-Submit: Nicola Murino <nicola.murino@gmail.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent e944286 commit ebc8e46

File tree

5 files changed

+118
-52
lines changed

5 files changed

+118
-52
lines changed

‎ssh/common.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,13 +279,6 @@ func InsecureAlgorithms() Algorithms {
279279
}
280280
}
281281

282-
// serverForbiddenKexAlgos contains key exchange algorithms, that are forbidden
283-
// for the server half.
284-
var serverForbiddenKexAlgos = map[string]struct{}{
285-
InsecureKeyExchangeDHGEXSHA1: {}, // server half implementation is only minimal to satisfy the automated tests
286-
KeyExchangeDHGEXSHA256: {}, // server half implementation is only minimal to satisfy the automated tests
287-
}
288-
289282
var supportedCompressions = []string{compressionNone}
290283

291284
// hashFuncs keeps the mapping of supported signature algorithms to their

‎ssh/kex.go‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ import (
1919
"golang.org/x/crypto/curve25519"
2020
)
2121

22+
const (
23+
// This is the group called diffie-hellman-group1-sha1 in RFC 4253 and
24+
// Oakley Group 2 in RFC 2409.
25+
oakleyGroup2 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF"
26+
// This is the group called diffie-hellman-group14-sha1 in RFC 4253 and
27+
// Oakley Group 14 in RFC 3526.
28+
oakleyGroup14 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF"
29+
// This is the group called diffie-hellman-group15-sha512 in RFC 8268 and
30+
// Oakley Group 15 in RFC 3526.
31+
oakleyGroup15 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AAAC42DAD33170D04507A33A85521ABDF1CBA64ECFB850458DBEF0A8AEA71575D060C7DB3970F85A6E1E4C7ABF5AE8CDB0933D71E8C94E04A25619DCEE3D2261AD2EE6BF12FFA06D98A0864D87602733EC86A64521F2B18177B200CBBE117577A615D6C770988C0BAD946E208E24FA074E5AB3143DB5BFCE0FD108E4B82D120A93AD2CAFFFFFFFFFFFFFFFF"
32+
// This is the group called diffie-hellman-group16-sha512 in RFC 8268 and
33+
// Oakley Group 16 in RFC 3526.
34+
oakleyGroup16 = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AAAC42DAD33170D04507A33A85521ABDF1CBA64ECFB850458DBEF0A8AEA71575D060C7DB3970F85A6E1E4C7ABF5AE8CDB0933D71E8C94E04A25619DCEE3D2261AD2EE6BF12FFA06D98A0864D87602733EC86A64521F2B18177B200CBBE117577A615D6C770988C0BAD946E208E24FA074E5AB3143DB5BFCE0FD108E4B82D120A92108011A723C12A787E6D788719A10BDBA5B2699C327186AF4E23C1A946834B6150BDA2583E9CA2AD44CE8DBBBC2DB04DE8EF92E8EFC141FBECAA6287C59474E6BC05D99B2964FA090C3A2233BA186515BE7ED1F612970CEE2D7AFB81BDD762170481CD0069127D5B05AA993B4EA988D8FDDC186FFB7DC90A6C08F4DF435C934063199FFFFFFFFFFFFFFFF"
35+
)
36+
2237
// kexResult captures the outcome of a key exchange.
2338
type kexResult struct {
2439
// Session hash. See also RFC 4253, section 8.
@@ -384,20 +399,15 @@ func ecHash(curve elliptic.Curve) crypto.Hash {
384399
var kexAlgoMap = map[string]kexAlgorithm{}
385400

386401
func init() {
387-
// This is the group called diffie-hellman-group1-sha1 in
388-
// RFC 4253 and Oakley Group 2 in RFC 2409.
389-
p, _ := new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE65381FFFFFFFFFFFFFFFF", 16)
402+
p, _ := new(big.Int).SetString(oakleyGroup2, 16)
390403
kexAlgoMap[InsecureKeyExchangeDH1SHA1] = &dhGroup{
391404
g: new(big.Int).SetInt64(2),
392405
p: p,
393406
pMinus1: new(big.Int).Sub(p, bigOne),
394407
hashFunc: crypto.SHA1,
395408
}
396409

397-
// This are the groups called diffie-hellman-group14-sha1 and
398-
// diffie-hellman-group14-sha256 in RFC 4253 and RFC 8268,
399-
// and Oakley Group 14 in RFC 3526.
400-
p, _ = new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF", 16)
410+
p, _ = new(big.Int).SetString(oakleyGroup14, 16)
401411
group14 := &dhGroup{
402412
g: new(big.Int).SetInt64(2),
403413
p: p,
@@ -413,9 +423,7 @@ func init() {
413423
hashFunc: crypto.SHA256,
414424
}
415425

416-
// This is the group called diffie-hellman-group16-sha512 in RFC
417-
// 8268 and Oakley Group 16 in RFC 3526.
418-
p, _ = new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AAAC42DAD33170D04507A33A85521ABDF1CBA64ECFB850458DBEF0A8AEA71575D060C7DB3970F85A6E1E4C7ABF5AE8CDB0933D71E8C94E04A25619DCEE3D2261AD2EE6BF12FFA06D98A0864D87602733EC86A64521F2B18177B200CBBE117577A615D6C770988C0BAD946E208E24FA074E5AB3143DB5BFCE0FD108E4B82D120A92108011A723C12A787E6D788719A10BDBA5B2699C327186AF4E23C1A946834B6150BDA2583E9CA2AD44CE8DBBBC2DB04DE8EF92E8EFC141FBECAA6287C59474E6BC05D99B2964FA090C3A2233BA186515BE7ED1F612970CEE2D7AFB81BDD762170481CD0069127D5B05AA993B4EA988D8FDDC186FFB7DC90A6C08F4DF435C934063199FFFFFFFFFFFFFFFF", 16)
426+
p, _ = new(big.Int).SetString(oakleyGroup16, 16)
419427

420428
kexAlgoMap[KeyExchangeDH16SHA512] = &dhGroup{
421429
g: new(big.Int).SetInt64(2),
@@ -583,9 +591,9 @@ const (
583591
func (gex *dhGEXSHA) Client(c packetConn, randSource io.Reader, magics *handshakeMagics) (*kexResult, error) {
584592
// Send GexRequest
585593
kexDHGexRequest := kexDHGexRequestMsg{
586-
MinBits: dhGroupExchangeMinimumBits,
587-
PreferedBits: dhGroupExchangePreferredBits,
588-
MaxBits: dhGroupExchangeMaximumBits,
594+
MinBits: dhGroupExchangeMinimumBits,
595+
PreferredBits: dhGroupExchangePreferredBits,
596+
MaxBits: dhGroupExchangeMaximumBits,
589597
}
590598
if err := c.writePacket(Marshal(&kexDHGexRequest)); err != nil {
591599
return nil, err
@@ -672,9 +680,7 @@ func (gex *dhGEXSHA) Client(c packetConn, randSource io.Reader, magics *handshak
672680
}
673681

674682
// Server half implementation of the Diffie Hellman Key Exchange with SHA1 and SHA256.
675-
//
676-
// This is a minimal implementation to satisfy the automated tests.
677-
func (gex dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshakeMagics, priv AlgorithmSigner, algo string) (result *kexResult, err error) {
683+
func (gex *dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshakeMagics, priv AlgorithmSigner, algo string) (result *kexResult, err error) {
678684
// Receive GexRequest
679685
packet, err := c.readPacket()
680686
if err != nil {
@@ -684,13 +690,32 @@ func (gex dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshake
684690
if err = Unmarshal(packet, &kexDHGexRequest); err != nil {
685691
return
686692
}
693+
// We check that the request received is valid and that the MaxBits
694+
// requested are at least equal to our supported minimum. This is the same
695+
// check done in OpenSSH:
696+
// https://github.com/openssh/openssh-portable/blob/80a2f64b/kexgexs.c#L94
697+
//
698+
// Furthermore, we also check that the required MinBits are less than or
699+
// equal to 4096 because we can use up to Oakley Group 16.
700+
if kexDHGexRequest.MaxBits < kexDHGexRequest.MinBits || kexDHGexRequest.PreferredBits < kexDHGexRequest.MinBits ||
701+
kexDHGexRequest.MaxBits < kexDHGexRequest.PreferredBits || kexDHGexRequest.MaxBits < dhGroupExchangeMinimumBits ||
702+
kexDHGexRequest.MinBits > 4096 {
703+
return nil, fmt.Errorf("ssh: DH GEX request out of range, min: %d, max: %d, preferred: %d", kexDHGexRequest.MinBits,
704+
kexDHGexRequest.MaxBits, kexDHGexRequest.PreferredBits)
705+
}
706+
707+
var p *big.Int
708+
// We hardcode sending Oakley Group 14 (2048 bits), Oakley Group 15 (3072
709+
// bits) or Oakley Group 16 (4096 bits), based on the requested max size.
710+
if kexDHGexRequest.MaxBits < 3072 {
711+
p, _ = new(big.Int).SetString(oakleyGroup14, 16)
712+
} else if kexDHGexRequest.MaxBits < 4096 {
713+
p, _ = new(big.Int).SetString(oakleyGroup15, 16)
714+
} else {
715+
p, _ = new(big.Int).SetString(oakleyGroup16, 16)
716+
}
687717

688-
// Send GexGroup
689-
// This is the group called diffie-hellman-group14-sha1 in RFC
690-
// 4253 and Oakley Group 14 in RFC 3526.
691-
p, _ := new(big.Int).SetString("FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE649286651ECE45B3DC2007CB8A163BF0598DA48361C55D39A69163FA8FD24CF5F83655D23DCA3AD961C62F356208552BB9ED529077096966D670C354E4ABC9804F1746C08CA18217C32905E462E36CE3BE39E772C180E86039B2783A2EC07A28FB5C55DF06F4C52C9DE2BCBF6955817183995497CEA956AE515D2261898FA051015728E5A8AACAA68FFFFFFFFFFFFFFFF", 16)
692718
g := big.NewInt(2)
693-
694719
msg := &kexDHGexGroupMsg{
695720
P: p,
696721
G: g,
@@ -728,9 +753,9 @@ func (gex dhGEXSHA) Server(c packetConn, randSource io.Reader, magics *handshake
728753
h := gex.hashFunc.New()
729754
magics.write(h)
730755
writeString(h, hostKeyBytes)
731-
binary.Write(h, binary.BigEndian, uint32(dhGroupExchangeMinimumBits))
732-
binary.Write(h, binary.BigEndian, uint32(dhGroupExchangePreferredBits))
733-
binary.Write(h, binary.BigEndian, uint32(dhGroupExchangeMaximumBits))
756+
binary.Write(h, binary.BigEndian, kexDHGexRequest.MinBits)
757+
binary.Write(h, binary.BigEndian, kexDHGexRequest.PreferredBits)
758+
binary.Write(h, binary.BigEndian, kexDHGexRequest.MaxBits)
734759
writeInt(h, p)
735760
writeInt(h, g)
736761
writeInt(h, kexDHGexInit.X)

‎ssh/messages.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ type kexDHGexReplyMsg struct {
122122
const msgKexDHGexRequest = 34
123123

124124
type kexDHGexRequestMsg struct {
125-
MinBits uint32 `sshtype:"34"`
126-
PreferedBits uint32
127-
MaxBits uint32
125+
MinBits uint32 `sshtype:"34"`
126+
PreferredBits uint32
127+
MaxBits uint32
128128
}
129129

130130
// See RFC 4253, section 10.

‎ssh/server.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,6 @@ func NewServerConn(c net.Conn, config *ServerConfig) (*ServerConn, <-chan NewCha
252252
}
253253
}
254254
}
255-
// Check if the config contains any unsupported key exchanges
256-
for _, kex := range fullConf.KeyExchanges {
257-
if _, ok := serverForbiddenKexAlgos[kex]; ok {
258-
c.Close()
259-
return nil, nil, nil, fmt.Errorf("ssh: unsupported key exchange %s for server", kex)
260-
}
261-
}
262255

263256
s := &connection{
264257
sshConn: sshConn{conn: c},

‎ssh/test/sshcli_test.go‎

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,23 +34,29 @@ func sshClient(t *testing.T) string {
3434
return sshCLI
3535
}
3636

37+
// setupSSHCLIKeys writes the provided key files to a temporary directory and
38+
// returns the path to the private key.
39+
func setupSSHCLIKeys(t *testing.T, keyFiles map[string][]byte, privKeyName string) string {
40+
tmpDir := t.TempDir()
41+
for fn, content := range keyFiles {
42+
if err := os.WriteFile(filepath.Join(tmpDir, fn), content, 0600); err != nil {
43+
t.Fatalf("WriteFile(%q): %v", fn, err)
44+
}
45+
}
46+
return filepath.Join(tmpDir, privKeyName)
47+
}
48+
3749
func TestSSHCLIAuth(t *testing.T) {
3850
if runtime.GOOS == "windows" {
3951
t.Skipf("always fails on Windows, see #64403")
4052
}
4153
sshCLI := sshClient(t)
42-
dir := t.TempDir()
43-
keyPrivPath := filepath.Join(dir, "rsa")
44-
45-
for fn, content := range map[string][]byte{
46-
keyPrivPath: testdata.PEMBytes["rsa"],
47-
keyPrivPath + ".pub": ssh.MarshalAuthorizedKey(testPublicKeys["rsa"]),
48-
filepath.Join(dir, "rsa-cert.pub"): testdata.SSHCertificates["rsa-user-testcertificate"],
49-
} {
50-
if err := os.WriteFile(fn, content, 0600); err != nil {
51-
t.Fatalf("WriteFile(%q): %v", fn, err)
52-
}
54+
keyFiles := map[string][]byte{
55+
"rsa": testdata.PEMBytes["rsa"],
56+
"rsa.pub": ssh.MarshalAuthorizedKey(testPublicKeys["rsa"]),
57+
"rsa-cert.pub": testdata.SSHCertificates["rsa-user-testcertificate"],
5358
}
59+
keyPrivPath := setupSSHCLIKeys(t, keyFiles, "rsa")
5460

5561
certChecker := ssh.CertChecker{
5662
IsUserAuthority: func(k ssh.PublicKey) bool {
@@ -98,3 +104,52 @@ func TestSSHCLIAuth(t *testing.T) {
98104
t.Fatalf("user certificate authentication failed, error: %v, command output %q", err, string(out))
99105
}
100106
}
107+
108+
func TestSSHCLIKeyExchanges(t *testing.T) {
109+
if runtime.GOOS == "windows" {
110+
t.Skipf("always fails on Windows, see #64403")
111+
}
112+
sshCLI := sshClient(t)
113+
keyFiles := map[string][]byte{
114+
"rsa": testdata.PEMBytes["rsa"],
115+
"rsa.pub": ssh.MarshalAuthorizedKey(testPublicKeys["rsa"]),
116+
}
117+
keyPrivPath := setupSSHCLIKeys(t, keyFiles, "rsa")
118+
119+
keyExchanges := append(ssh.SupportedAlgorithms().KeyExchanges, ssh.InsecureAlgorithms().KeyExchanges...)
120+
for _, kex := range keyExchanges {
121+
t.Run(kex, func(t *testing.T) {
122+
config := &ssh.ServerConfig{
123+
Config: ssh.Config{
124+
KeyExchanges: []string{kex},
125+
},
126+
PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) {
127+
if conn.User() == "testpubkey" && bytes.Equal(key.Marshal(), testPublicKeys["rsa"].Marshal()) {
128+
return nil, nil
129+
}
130+
131+
return nil, fmt.Errorf("pubkey for %q not acceptable", conn.User())
132+
},
133+
}
134+
config.AddHostKey(testSigners["rsa"])
135+
136+
server, err := newTestServer(config)
137+
if err != nil {
138+
t.Fatalf("unable to start test server: %v", err)
139+
}
140+
defer server.Close()
141+
142+
port, err := server.port()
143+
if err != nil {
144+
t.Fatalf("unable to get server port: %v", err)
145+
}
146+
147+
cmd := testenv.Command(t, sshCLI, "-vvv", "-i", keyPrivPath, "-o", "StrictHostKeyChecking=no",
148+
"-o", fmt.Sprintf("KexAlgorithms=%s", kex), "-p", port, "testpubkey@127.0.0.1", "true")
149+
out, err := cmd.CombinedOutput()
150+
if err != nil {
151+
t.Fatalf("%s failed, error: %v, command output %q", kex, err, string(out))
152+
}
153+
})
154+
}
155+
}

0 commit comments

Comments
 (0)