From 5437ef88025e69e07c6d6085297fb7c7d42c991c Mon Sep 17 00:00:00 2001 From: Kenny Root Date: Sat, 9 May 2026 02:48:13 -0700 Subject: [PATCH] chore: add more test coverage Add test coverage for EXT_INFO and keystroke obfuscation. This will kill some mutations discovered by pitest. --- .../sshlib/client/ExtInfoNegotiationTest.kt | 236 ++++++++++++++++++ .../connectbot/sshlib/client/FakeSshServer.kt | 6 + .../sshlib/client/KeystrokeObfuscatorTest.kt | 144 +++++++++++ 3 files changed, 386 insertions(+) diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ExtInfoNegotiationTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ExtInfoNegotiationTest.kt index c560da5..79aa661 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ExtInfoNegotiationTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ExtInfoNegotiationTest.kt @@ -4,8 +4,10 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import kotlinx.coroutines.withTimeout import kotlinx.coroutines.withTimeoutOrNull @@ -15,6 +17,7 @@ import org.connectbot.sshlib.HostKeyVerifier import org.connectbot.sshlib.PublicKey import org.connectbot.sshlib.protocol.SshMsgExtInfo import org.connectbot.sshlib.transport.PipedTransport +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Test @@ -166,6 +169,239 @@ class ExtInfoNegotiationTest { } } + // --- stripExtInfoC / appendExtInfoC boundary tests --- + + @Test + fun `client strips ext-info-c from kexAlgorithms before re-advertising it`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = true + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + kexAlgorithms = "curve25519-sha256,ext-info-c", + coroutineDispatcher = dispatcher, + ) + + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + val kexInit = withTimeout(1000) { server.awaitClientKexInit() } + val kexAlgs = kexInit.kexAlgorithms().entries().data() + // ext-info-c must appear exactly once (appended), not duplicated + assertEquals(1, kexAlgs.count { it == "ext-info-c" }, "ext-info-c should appear exactly once, got: $kexAlgs") + } finally { + connection.close() + } + } + + @Test + fun `client appends ext-info-c when not present in custom kex list`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = true + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + kexAlgorithms = "curve25519-sha256", + coroutineDispatcher = dispatcher, + ) + + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + val kexInit = withTimeout(1000) { server.awaitClientKexInit() } + val kexAlgs = kexInit.kexAlgorithms().entries().data() + assertTrue("ext-info-c" in kexAlgs, "ext-info-c should be appended to kex list: $kexAlgs") + } finally { + connection.close() + } + } + + // --- sendClientExtInfo guard: not sent twice --- + + @Test + fun `client sends EXT_INFO at most once even after rekey`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = true + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + coroutineDispatcher = dispatcher, + ) + + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + + // Consume the initial EXT_INFO + val first = withTimeout(1000) { server.awaitExtInfo() } + assertNotNull(first) + + // Trigger a server-initiated rekey; the client must NOT send EXT_INFO again + server.initiateRekey() + server.sendIgnore() + withTimeout(1000) { server.rekeyCount.first { it >= 1 } } + + val second = withTimeoutOrNull(300) { server.awaitExtInfo() } + assertNull(second, "Client must not send EXT_INFO again after rekey") + } finally { + connection.close() + } + } + + // --- processServerExtInfo: count guard (>= 2 rejects third message) --- + + @Test + fun `client ignores third EXT_INFO from server`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = true + server.advertisePing = true + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + coroutineDispatcher = dispatcher, + ) + + var authJob: kotlinx.coroutines.Job? = null + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + assertTrue(connection.serverSupportsPing, "Initial EXT_INFO should set ping support") + + // Second EXT_INFO (during auth) — allowed, but must not clear ping + authJob = backgroundScope.launch(dispatcher) { + connection.authenticatePassword("user", "pass") + } + withTimeout(1000) { server.awaitUserauthRequest() } + server.sendCustomExtInfo(mapOf("server-sig-algs" to "rsa-sha2-256".toByteArray(Charsets.US_ASCII))) + awaitServerSigAlgs(connection, setOf("rsa-sha2-256")) + + // Third EXT_INFO — must be ignored; ping support and sig-algs must remain unchanged + server.sendCustomExtInfo(mapOf("server-sig-algs" to "ssh-ed25519".toByteArray(Charsets.US_ASCII))) + // Drain only currently scheduled packet handling. advanceUntilIdle would also + // advance virtual time far enough to fire the connection's rekey timer. + runCurrent() + assertEquals(setOf("rsa-sha2-256"), serverSigAlgs(connection), "Third EXT_INFO must be ignored") + assertTrue(connection.serverSupportsPing, "Ping support must survive ignored third EXT_INFO") + } finally { + authJob?.cancel() + connection.close() + } + } + + // --- processServerExtInfo: initialExtInfo flag (== 0 check) --- + + @Test + fun `second EXT_INFO does not clear initial-only fields like ping and hostbound`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = true + server.advertisePing = true + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + coroutineDispatcher = dispatcher, + ) + + var authJob: kotlinx.coroutines.Job? = null + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + assertTrue(connection.serverSupportsPing, "Ping should be set by initial EXT_INFO") + + authJob = backgroundScope.launch(dispatcher) { + connection.authenticatePassword("user", "pass") + } + withTimeout(1000) { server.awaitUserauthRequest() } + + // Second EXT_INFO without ping — ping must remain true (initial-only) + server.sendCustomExtInfo(mapOf("server-sig-algs" to "rsa-sha2-256".toByteArray(Charsets.US_ASCII))) + awaitServerSigAlgs(connection, setOf("rsa-sha2-256")) + assertTrue(connection.serverSupportsPing, "Ping support must not be cleared by second EXT_INFO") + } finally { + authJob?.cancel() + connection.close() + } + } + + @Test + fun `initial EXT_INFO with no ping extension leaves serverSupportsPing false`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = true + server.advertisePing = false // no ping in initial EXT_INFO + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + coroutineDispatcher = dispatcher, + ) + + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + assertFalse(connection.serverSupportsPing, "Ping should be false when not in initial EXT_INFO") + } finally { + connection.close() + } + } + + // --- processServerExtInfo: ignored when server did not advertise ext-info-s --- + + @Test + fun `EXT_INFO from server is ignored when server did not advertise ext-info-s`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher) + server.advertiseExtInfo = false + server.start() + + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = acceptAllVerifier, + coroutineDispatcher = dispatcher, + ) + + var authJob: kotlinx.coroutines.Job? = null + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertIs(result) + + authJob = backgroundScope.launch(dispatcher) { + connection.authenticatePassword("user", "pass") + } + withTimeout(1000) { server.awaitUserauthRequest() } + + // Send a rogue EXT_INFO even though server never advertised ext-info-s + server.sendCustomExtInfo(mapOf("server-sig-algs" to "ssh-ed25519".toByteArray(Charsets.US_ASCII))) + runCurrent() + assertNull(serverSigAlgs(connection), "server-sig-algs must remain null when EXT_INFO is ignored") + } finally { + authJob?.cancel() + connection.close() + } + } + @Test fun `server may update server-sig-algs during user authentication`() = runTest { val dispatcher = StandardTestDispatcher(testScheduler) diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt index 2c1c905..6f03941 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt @@ -89,6 +89,7 @@ class FakeSshServer( private val receivedPongs = Channel(Channel.UNLIMITED) private val receivedExtInfo = Channel(Channel.UNLIMITED) private val receivedUserauthRequests = Channel(Channel.UNLIMITED) + private val receivedClientKexInits = Channel(Channel.UNLIMITED) fun start() { scope.launch(coroutineContext) { serve() } @@ -231,6 +232,9 @@ class FakeSshServer( private suspend fun doFullKex(io: PacketIO) { val serverKexInitBytes = sendKexInit(io) val clientKexInitRaw = readPacketRaw(io) + val clientKexInit = SshMsgKexinit(ByteBufferKaitaiStream(clientKexInitRaw.copyOfRange(1, clientKexInitRaw.size))) + clientKexInit._read() + receivedClientKexInits.trySend(clientKexInit) val ecdhInitRaw = readPacketRaw(io) val clientPublic = parseEcdhInit(ecdhInitRaw) sendEcdhReply(io, clientKexInitRaw, serverKexInitBytes, clientPublic) @@ -546,4 +550,6 @@ class FakeSshServer( suspend fun awaitExtInfo(): SshMsgExtInfo = receivedExtInfo.receive() suspend fun awaitUserauthRequest(): SshMsgUserauthRequest = receivedUserauthRequests.receive() + + suspend fun awaitClientKexInit(): SshMsgKexinit = receivedClientKexInits.receive() } diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscatorTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscatorTest.kt index effdd56..c43ac06 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscatorTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscatorTest.kt @@ -20,6 +20,7 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test +import kotlin.random.Random import kotlin.test.assertFailsWith class KeystrokeObfuscatorTest { @@ -149,4 +150,147 @@ class KeystrokeObfuscatorTest { ) } } + + // --- delayUntilNextSendMs boundary tests --- + + @Test + fun `delayUntilNextSendMs returns 0 exactly when now equals nextIntervalMs`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + val boundary = obfuscator.nextIntervalMs() + now = boundary + assertEquals(0L, obfuscator.delayUntilNextSendMs()) + } + + @Test + fun `delayUntilNextSendMs returns 1 when one ms before nextIntervalMs`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + val boundary = obfuscator.nextIntervalMs() + now = boundary - 1 + assertEquals(1L, obfuscator.delayUntilNextSendMs()) + } + + // --- advanceInterval math and clamp tests --- + + @Test + fun `advanceInterval skips exactly one interval when just past boundary`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + val first = obfuscator.nextIntervalMs() + now = first + 1 // just past the first boundary + obfuscator.advanceInterval() + val second = obfuscator.nextIntervalMs() + // Must be strictly after 'now', not before it + assertTrue(second > now, "nextIntervalMs $second should be after now $now") + } + + @Test + fun `advanceInterval skips multiple missed intervals`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + val first = obfuscator.nextIntervalMs() + // Advance 5 full intervals past the boundary + now = first + intervalMs * 5 + obfuscator.advanceInterval() + val second = obfuscator.nextIntervalMs() + assertTrue(second > now, "nextIntervalMs $second should be after now $now after skipping intervals") + } + + @Test + fun `advanceInterval clamps negative missed-interval count to zero`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + // Don't advance clock — now is before nextIntervalMs so missedIntervals would be negative + val before = obfuscator.nextIntervalMs() + obfuscator.advanceInterval() + val after = obfuscator.nextIntervalMs() + // Result should still be in the future (clamped to 0 missed, advances by 1 interval) + assertTrue(after > now, "nextIntervalMs $after should be positive after clamp") + // The boundary should move forward even though we didn't cross it. + assertTrue(after > before, "nextIntervalMs $after should be after previous boundary $before") + assertTrue(after >= 1L, "nextIntervalMs must be at least 1") + } + + // --- computeFuzzMs boundary tests --- + + @Test + fun `fuzz floor is 1 for tiny intervals`() { + // With intervalMs=1 and FUZZ_PERCENT=10, raw fuzz = 0 → should clamp to 1 + var now = 0L + val obfuscator = KeystrokeObfuscator(1L, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + // nextIntervalMs must be at least 1 (coerceAtLeast(1)) + assertTrue(obfuscator.nextIntervalMs() >= 1L) + } + + @Test + fun `interval close to Long MAX does not overflow fuzz computation`() { + // Int.MAX_VALUE cap prevents fuzz from exceeding random's int range + val hugeInterval = Int.MAX_VALUE.toLong() * 100L + var now = 0L + val obfuscator = KeystrokeObfuscator(hugeInterval, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + assertTrue(obfuscator.nextIntervalMs() >= 1L) + } + + // --- setNextInterval: sessionRate only set on starting=true --- + + @Test + fun `sessionRate is fixed per session and does not change on subsequent advances`() { + // Use a seeded Random so results are deterministic. + // First keystroke sets sessionRate (starting=true). Subsequent advanceInterval calls + // should NOT change sessionRate (starting=false), so the per-advance fuzz is different + // from what it would be if sessionRate were reset each time. + var now = 0L + val seededRandom = Random(42) + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }, random = seededRandom) + obfuscator.recordKeystroke() + + // Record nextIntervalMs values across several advances + val intervals = mutableListOf() + repeat(5) { + val prev = obfuscator.nextIntervalMs() + now = prev + obfuscator.advanceInterval() + intervals.add(obfuscator.nextIntervalMs() - prev) + } + + // All intervals must be positive (sessionRate never causes a negative adjusted value) + assertTrue(intervals.all { it >= 1L }, "All intervals must be >= 1, got: $intervals") + } + + @Test + fun `recordKeystroke returns true on first call and false when window still active`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }) + assertTrue(obfuscator.recordKeystroke(), "First keystroke should return true (just started)") + // Still within chaff window + assertFalse(obfuscator.recordKeystroke(), "Second keystroke within window should return false") + } + + @Test + fun `recordKeystroke returns true again after chaff window expires`() { + var now = 0L + val obfuscator = KeystrokeObfuscator(intervalMs, clockMs = { now }) + obfuscator.recordKeystroke() + now = obfuscator.chaffUntilMs() + 1 + assertTrue(obfuscator.recordKeystroke(), "Keystroke after expired window should return true") + } + + @Test + fun `setNextInterval adjusted value is at least 1 when fuzz exceeds interval`() { + // With intervalMs=1 fuzz=1, adjusted = 1 - 1 + random(1) + sessionRate + // random(1) is always 0, sessionRate is random(1) which is also 0 + // → adjusted = 0, coerceAtLeast(1) = 1 + var now = 1000L + val obfuscator = KeystrokeObfuscator(1L, clockMs = { now }, random = Random(0)) + obfuscator.recordKeystroke() + assertTrue(obfuscator.nextIntervalMs() >= now + 1L) + } }