Skip to content

Commit 3260bba

Browse files
krutonclaude
andcommitted
fix(security): validate DH-GEX group parameters from server
A malicious server could send SSH_MSG_KEX_DH_GEX_GROUP with a weak or degenerate prime (e.g., 16-bit p, g=1, or an even p) to trivially break the key exchange. Enforce that p falls within the client-requested [min, max] bit range, that p is odd, and that g satisfies 1 < g < p-1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f62dc3d commit 3260bba

2 files changed

Lines changed: 90 additions & 1 deletion

File tree

sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,21 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String) :
4444
private var privateKey: BigInteger? = null
4545

4646
fun setGroup(p: BigInteger, g: BigInteger) {
47+
val pBits = p.bitLength()
48+
if (pBits < min) {
49+
throw SshException("DH group prime too small: $pBits bits < $min bits minimum")
50+
}
51+
if (pBits > max) {
52+
throw SshException("DH group prime too large: $pBits bits > $max bits maximum")
53+
}
54+
// p must be odd (a basic primality sanity check: all primes > 2 are odd)
55+
if (!p.testBit(0)) {
56+
throw SshException("DH group prime is even")
57+
}
58+
// g must satisfy 1 < g < p-1 to be a valid generator candidate
59+
if (g <= BigInteger.ONE || g >= p - BigInteger.ONE) {
60+
throw SshException("DH group generator g is out of range: must be 1 < g < p-1")
61+
}
4762
this.p = p
4863
this.g = g
4964
}

sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* ConnectBot SSH Library
3-
* Copyright 2025 Kenny Root
3+
* Copyright 2025-2026 Kenny Root
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -34,6 +34,80 @@ import kotlin.test.assertTrue
3434

3535
class DiffieHellmanGroupExchangeTest {
3636

37+
// A tiny 16-bit prime — far below the min=2048 requested by the client
38+
private val tinyPrime = BigInteger("65537")
39+
40+
// A 2048-bit composite (GROUP14_P with the last bit flipped) — not a safe prime
41+
private val compositeLikePrime = DhGroups.GROUP14_P.subtract(BigInteger.ONE)
42+
43+
@Test
44+
fun `setGroup rejects p smaller than min bits`() {
45+
val gex = DiffieHellmanGroupExchange("SHA-256")
46+
assertFailsWith<SshException> {
47+
gex.setGroup(tinyPrime, DhGroups.GENERATOR)
48+
}
49+
}
50+
51+
@Test
52+
fun `setGroup rejects p larger than max bits`() {
53+
// Construct a number just over 8192 bits
54+
val tooBig = BigInteger.ONE.shiftLeft(8193)
55+
val gex = DiffieHellmanGroupExchange("SHA-256")
56+
assertFailsWith<SshException> {
57+
gex.setGroup(tooBig, DhGroups.GENERATOR)
58+
}
59+
}
60+
61+
@Test
62+
fun `setGroup rejects g equal to 1`() {
63+
val gex = DiffieHellmanGroupExchange("SHA-256")
64+
assertFailsWith<SshException> {
65+
gex.setGroup(DhGroups.GROUP14_P, BigInteger.ONE)
66+
}
67+
}
68+
69+
@Test
70+
fun `setGroup rejects g equal to 0`() {
71+
val gex = DiffieHellmanGroupExchange("SHA-256")
72+
assertFailsWith<SshException> {
73+
gex.setGroup(DhGroups.GROUP14_P, BigInteger.ZERO)
74+
}
75+
}
76+
77+
@Test
78+
fun `setGroup rejects g greater than or equal to p minus 1`() {
79+
val gex = DiffieHellmanGroupExchange("SHA-256")
80+
assertFailsWith<SshException> {
81+
gex.setGroup(DhGroups.GROUP14_P, DhGroups.GROUP14_P - BigInteger.ONE)
82+
}
83+
}
84+
85+
@Test
86+
fun `setGroup rejects even p`() {
87+
val evenP = DhGroups.GROUP14_P.subtract(BigInteger.ONE) // make it even
88+
val gex = DiffieHellmanGroupExchange("SHA-256")
89+
assertFailsWith<SshException> {
90+
gex.setGroup(evenP, DhGroups.GENERATOR)
91+
}
92+
}
93+
94+
@Test
95+
fun `setGroup accepts valid group 14 parameters`() {
96+
val gex = DiffieHellmanGroupExchange("SHA-256")
97+
gex.setGroup(DhGroups.GROUP14_P, DhGroups.GENERATOR)
98+
// Should not throw; verify we can generate keys
99+
val e = gex.generateClientKeys()
100+
assertTrue(e.isNotEmpty())
101+
}
102+
103+
@Test
104+
fun `setGroup accepts valid group 16 parameters`() {
105+
val gex = DiffieHellmanGroupExchange("SHA-256")
106+
gex.setGroup(DhGroups.GROUP16_P, DhGroups.GENERATOR)
107+
val e = gex.generateClientKeys()
108+
assertTrue(e.isNotEmpty())
109+
}
110+
37111
@Test
38112
fun `generateClientKeys throws if group not set`() {
39113
val gex = DiffieHellmanGroupExchange("SHA-256")

0 commit comments

Comments
 (0)