diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt index 52ce970a..80245e0a 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt @@ -422,28 +422,36 @@ class SessionChannel internal constructor( private fun startChaffLoopIfNeeded(obs: KeystrokeObfuscator) { if (chaffJob?.isActive == true) return chaffJob = connectionScope.launch { - while (obfuscatorMutex.withLock { obs.isActive() }) { - val delayMs = obfuscatorMutex.withLock { - obs.delayUntilNextSendMs() - } - if (delayMs > 0) { - delay(delayMs) - } - val shouldSendChaff = obfuscatorMutex.withLock { - if (!obs.isActive()) { - false - } else if (pendingObfuscatedWrites.get() > 0) { - null - } else { - obs.advanceInterval() - true + try { + while (obfuscatorMutex.withLock { obs.isActive() }) { + val delayMs = obfuscatorMutex.withLock { + obs.delayUntilNextSendMs() + } + if (delayMs > 0) { + delay(delayMs) + } + val shouldSendChaff = obfuscatorMutex.withLock { + if (!obs.isActive()) { + false + } else if (pendingObfuscatedWrites.get() > 0) { + null + } else { + obs.advanceInterval() + true + } + } + when (shouldSendChaff) { + true -> connection.sendChaff() + false -> break + null -> obfuscatedWritesIdle.receiveCatching() } } - when (shouldSendChaff) { - true -> connection.sendChaff() - false -> break - null -> obfuscatedWritesIdle.receiveCatching() - } + } catch (cancelled: CancellationException) { + throw cancelled + } catch (failure: Exception) { + // An admitted packet may fail with a transport error after cancellation. + // Chaff has no caller to receive it; use the normal connection failure path. + connection.transportFailed(failure) } } } diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscationTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscationTest.kt index 48c95dec..7d10aa50 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscationTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/KeystrokeObfuscationTest.kt @@ -20,15 +20,23 @@ package org.connectbot.sshlib.client import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.currentTime +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext import org.connectbot.sshlib.HostKeyVerifier import org.connectbot.sshlib.PublicKey import org.connectbot.sshlib.crypto.AesCtrCipher @@ -363,6 +371,126 @@ class KeystrokeObfuscationTest { coVerify(exactly = 0) { conn.sendChaff() } } + @Test + fun `failed chaff write reports transport failure without escaping background worker`() = runTest { + val uncaught = mutableListOf() + val scope = CoroutineScope( + SupervisorJob() + StandardTestDispatcher(testScheduler) + + CoroutineExceptionHandler { _, error -> uncaught += error }, + ) + val conn = mockk(relaxed = true) + val failure = TransportException("Connection closed: Socket closed") + coEvery { conn.sendChaff() } throws failure + val channel = createObfuscatingChannel( + conn, + canSendChaff = true, + intervalMs = 20L, + scope = scope, + obfuscatorClockMs = { testScheduler.currentTime }, + obfuscatorRandom = Random(0), + ) + try { + channel.markPtyGranted() + channel.write("a".toByteArray()) + testScheduler.advanceTimeBy(21) + runCurrent() + + assertTrue(uncaught.isEmpty(), "Chaff must not crash its caller's process: $uncaught") + coVerify(exactly = 1) { conn.sendChaff() } + coVerify(exactly = 1) { conn.transportFailed(failure) } + } finally { + scope.cancel() + runCurrent() + } + } + + @Test + fun `chaff write failing during shutdown does not escape cancelled worker`() = runTest { + val uncaught = mutableListOf() + val scope = CoroutineScope( + SupervisorJob() + StandardTestDispatcher(testScheduler) + + CoroutineExceptionHandler { _, error -> uncaught += error }, + ) + val conn = mockk(relaxed = true) + val writing = CompletableDeferred() + val finish = CompletableDeferred() + val failure = TransportException("Packet writer stopped", CancellationException("Connection closed")) + coEvery { conn.sendChaff() } coAnswers { + // An admitted SSH write can finish with a transport error even after cancellation. + withContext(NonCancellable) { + writing.complete(Unit) + finish.await() + throw failure + } + } + val channel = createObfuscatingChannel( + conn, + canSendChaff = true, + intervalMs = 20L, + scope = scope, + obfuscatorClockMs = { testScheduler.currentTime }, + obfuscatorRandom = Random(0), + ) + try { + channel.markPtyGranted() + channel.write("a".toByteArray()) + testScheduler.advanceTimeBy(21) + runCurrent() + assertTrue(writing.isCompleted) + + scope.cancel() + finish.complete(Unit) + runCurrent() + + assertTrue(uncaught.isEmpty(), "Shutdown must not crash its caller's process: $uncaught") + // Coroutine stack recovery may copy the exception and retain the original as cause. + coVerify(exactly = 1) { conn.transportFailed(match { it === failure || it.cause === failure }) } + } finally { + finish.complete(Unit) + scope.cancel() + runCurrent() + } + } + + @Test + fun `normal chaff cancellation does not report transport failure`() = runTest { + val uncaught = mutableListOf() + val scope = CoroutineScope( + SupervisorJob() + StandardTestDispatcher(testScheduler) + + CoroutineExceptionHandler { _, error -> uncaught += error }, + ) + val conn = mockk(relaxed = true) + val writing = CompletableDeferred() + coEvery { conn.sendChaff() } coAnswers { + writing.complete(Unit) + awaitCancellation() + } + val channel = createObfuscatingChannel( + conn, + canSendChaff = true, + intervalMs = 20L, + scope = scope, + obfuscatorClockMs = { testScheduler.currentTime }, + obfuscatorRandom = Random(0), + ) + try { + channel.markPtyGranted() + channel.write("a".toByteArray()) + testScheduler.advanceTimeBy(21) + runCurrent() + assertTrue(writing.isCompleted) + + scope.cancel() + runCurrent() + + assertTrue(uncaught.isEmpty()) + coVerify(exactly = 0) { conn.transportFailed(any()) } + } finally { + scope.cancel() + runCurrent() + } + } + private fun enableTestEncryption(connection: SshConnection) { val packetIoField = SshConnection::class.java.getDeclaredField("packetIO") packetIoField.isAccessible = true