From 05aba41f182087a987a1ee2017adfb5264d05e33 Mon Sep 17 00:00:00 2001
From: Kenny Root <kenny@the-b.org>
Date: Mon, 26 Oct 2026 20:25:07 -0700
Subject: [PATCH] fix: relax DH-GEX group checking

The safe prime check was taking 55 seconds on a user device which is way
too long. Let us assume the server is sending a safe prime and instead
focus on structural checks of the parameters.
---
 .../crypto/DiffieHellmanGroupExchange.kt      | 21 +++---------
 .../crypto/DiffieHellmanGroupExchangeTest.kt  | 32 +++++++++++--------
 2 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt
index eca25f8a4..2f74e913a 100644
--- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt
+++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt
@@ -33,20 +33,6 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String, va
     companion object {
         private val secureRandom = SecureRandom()
         private const val ERROR_GROUP_NOT_SET = "DH group not set; call setGroup() first"
-        private const val PRIMALITY_CERTAINTY = 80
-        private const val GROUP_CACHE_LIMIT = 16
-        private val groupValidationCache = object : LinkedHashMap<BigInteger, Boolean>(GROUP_CACHE_LIMIT, 0.75f, true) {
-            override fun removeEldestEntry(eldest: MutableMap.MutableEntry<BigInteger, Boolean>?): Boolean = size > GROUP_CACHE_LIMIT
-        }
-
-        @Synchronized
-        private fun isSafeGroup(p: BigInteger): Boolean = groupValidationCache.getOrPut(p) {
-            p.isProbablePrime(PRIMALITY_CERTAINTY) &&
-                p.subtract(BigInteger.ONE).shiftRight(1).isProbablePrime(PRIMALITY_CERTAINTY)
-        }
-
-        @Synchronized
-        internal fun cachedGroupValidationCount(): Int = groupValidationCache.size
     }
 
     val n = maxOf(4096, min)
@@ -72,9 +58,10 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String, va
         if (g <= BigInteger.ONE || g >= p - BigInteger.ONE) {
             throw SshException("DH group generator g is out of range: must be 1 < g < p-1")
         }
-        if (!isSafeGroup(p)) {
-            throw SshException("DH group modulus is not a safe prime")
-        }
+        // Just use client-side structural checks; the server supplies a safe prime.
+        // Re-testing p and (p-1)/2 took 55 seconds for a 4095-bit group on Android:
+        // https://github.com/connectbot/connectbot/issues/2550
+        // The host-key signature authenticates these parameters as part of the exchange hash.
         this.p = p
         this.g = g
     }
diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt
index 4f9ae8ab7..e30db0ecc 100644
--- a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt
+++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt
@@ -74,8 +74,8 @@ class DiffieHellmanGroupExchangeTest {
     @Test
     fun `setGroup rejects g greater than or equal to p minus 1`() {
         val gex = DiffieHellmanGroupExchange("SHA-256")
-        assertFailsWith<SshException> {
-            gex.setGroup(DhGroups.GROUP14_P, DhGroups.GROUP14_P - BigInteger.ONE)
+        for (g in listOf(DhGroups.GROUP14_P - BigInteger.ONE, DhGroups.GROUP14_P, DhGroups.GROUP14_P + BigInteger.ONE)) {
+            assertFailsWith<SshException> { gex.setGroup(DhGroups.GROUP14_P, g) }
         }
     }
 
@@ -89,13 +89,13 @@ class DiffieHellmanGroupExchangeTest {
     }
 
     @Test
-    fun `setGroup rejects odd composite modulus`() {
+    fun `setGroup leaves primality validation to the server`() {
         val composite = BigInteger.ONE.shiftLeft(2047).add(BigInteger.ONE).multiply(BigInteger.valueOf(3))
         val gex = DiffieHellmanGroupExchange("SHA-256")
 
-        assertFailsWith<SshException> {
-            gex.setGroup(composite, DhGroups.GENERATOR)
-        }
+        // OpenSSH-style checks accept in-range odd parameters without testing primality.
+        gex.setGroup(composite, DhGroups.GENERATOR)
+        assertTrue(gex.generateClientKeys().isNotEmpty())
     }
 
     @Test
@@ -168,16 +168,20 @@ class DiffieHellmanGroupExchangeTest {
     }
 
     @Test
-    fun `validated group cache remains bounded`() {
+    fun `setGroup rejects negative generator`() {
         val gex = DiffieHellmanGroupExchange("SHA-256")
-        repeat(20) { index ->
-            val composite = BigInteger.ONE.shiftLeft(2047)
-                .add(BigInteger.valueOf((index * 2L) + 1L))
-                .multiply(BigInteger.valueOf(3))
-            assertFailsWith<SshException> { gex.setGroup(composite, DhGroups.GENERATOR) }
-        }
+        assertFailsWith<SshException> { gex.setGroup(DhGroups.GROUP14_P, BigInteger.ONE.negate()) }
+    }
 
-        assertTrue(DiffieHellmanGroupExchange.cachedGroupValidationCount() <= 16)
+    @Test
+    fun `shared secret rejects out of range server public values`() {
+        val p = DhGroups.GROUP14_P
+        for (f in listOf(BigInteger.ZERO, BigInteger.ONE, p - BigInteger.ONE, p, p + BigInteger.ONE)) {
+            val gex = DiffieHellmanGroupExchange("SHA-256")
+            gex.setGroup(p, DhGroups.GENERATOR)
+            gex.generateClientKeys()
+            assertFailsWith<SshException> { gex.computeSharedSecret(f.toByteArray()) }
+        }
     }
 
     @Test