diff --git a/README.md b/README.md index cd41938f..aa90e3e2 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,16 @@ The library supports a wide range of modern SSH algorithms, including: For a complete list of supported algorithms and their respective RFCs, see [docs/ALGORITHMS.md](docs/ALGORITHMS.md). +The defaults intentionally exclude SHA-1 key exchange and MACs, CBC/3DES +ciphers, and `ssh-rsa` host-key signatures. These legacy algorithms remain +available only through the explicit `kexAlgorithms`, `hostKeyAlgorithms`, +`encryptionAlgorithms`, and `macAlgorithms` settings in `SshClientConfig`. +RSA user authentication normally requires the server to advertise +`rsa-sha2-256` or `rsa-sha2-512` through `server-sig-algs`. Explicitly including +`ssh-rsa` in the configured host-key algorithm wishlist also permits the legacy +RSA/SHA-1 signature when advertised, or as the base-key algorithm when that +extension is absent. + ## Quick Start ### Build @@ -125,23 +135,23 @@ Enable SSH agent forwarding to allow remote servers to use your keys: ```kotlin // Implement an agent provider class MyAgentProvider : AgentProvider { - override suspend fun getIdentities(): List { + override suspend fun getIdentities(): AgentResult> { val keyBlob = loadPublicKeyBlob() - return listOf(AgentIdentity(keyBlob, "my-key")) + return AgentResult.Success(listOf(AgentIdentity(keyBlob, "my-key"))) } - override suspend fun signData(context: AgentSigningContext): ByteArray? { + override suspend fun signData(context: AgentSigningContext): AgentResult { // Show approval UI to user with session context val approved = showSigningPrompt( "Remote server ${context.serverHostKey.toHex()} wants to use your key", "Session bound: ${context.isBound}" ) - return if (approved) { + return AgentResult.Success(if (approved) { signWithPrivateKey(context.publicKeyBlob, context.dataToSign) } else { null // Deny the request - } + }) } } diff --git a/docs/ALGORITHMS.md b/docs/ALGORITHMS.md index 4ab452d9..247a15d7 100644 --- a/docs/ALGORITHMS.md +++ b/docs/ALGORITHMS.md @@ -2,6 +2,10 @@ This document lists the cryptographic algorithms supported by the ConnectBot SSH library. +Algorithms labeled **legacy opt-in** are implemented for compatibility but are +not offered by the default `SshClientConfig`. Applications must add them +explicitly to the corresponding algorithm string. + ## Authentication - `keyboard-interactive` - `password` @@ -15,7 +19,7 @@ This document lists the cryptographic algorithms supported by the ConnectBot SSH - `ecdsa-sha2-nistp521` ([RFC 5656](https://tools.ietf.org/html/rfc5656#section-3)) - `rsa-sha2-512` ([RFC 8332](https://tools.ietf.org/html/rfc8332#section-3)) - `rsa-sha2-256` ([RFC 8332](https://tools.ietf.org/html/rfc8332#section-3)) -- `ssh-rsa` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-8.1)) +- `ssh-rsa` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-8.1)) — **legacy opt-in** ## Key Exchange - `mlkem768x25519-sha256` ([draft-ietf-sshm-mlkem-hybrid-kex](https://datatracker.ietf.org/doc/draft-ietf-sshm-mlkem-hybrid-kex/)) (depends on JEP-496 support) @@ -27,9 +31,9 @@ This document lists the cryptographic algorithms supported by the ConnectBot SSH - `diffie-hellman-group16-sha512` ([RFC 8268](https://tools.ietf.org/html/rfc8268)) - `diffie-hellman-group14-sha256` ([RFC 8268](https://tools.ietf.org/html/rfc8268)) - `diffie-hellman-group-exchange-sha256` ([RFC 4419](https://tools.ietf.org/html/rfc4419)) -- `diffie-hellman-group14-sha1` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-8.1)) -- `diffie-hellman-group-exchange-sha1` ([RFC 4419](https://tools.ietf.org/html/rfc4419)) -- `diffie-hellman-group1-sha1` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-8.1)) +- `diffie-hellman-group14-sha1` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-8.1)) — **legacy opt-in** +- `diffie-hellman-group-exchange-sha1` ([RFC 4419](https://tools.ietf.org/html/rfc4419)) — **legacy opt-in** +- `diffie-hellman-group1-sha1` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-8.1)) — **legacy opt-in** ## Encryption - `chacha20-poly1305@openssh.com` ([draft-ietf-sshm-chacha20-poly1305](https://datatracker.ietf.org/doc/html/draft-ietf-sshm-chacha20-poly1305)) @@ -37,14 +41,14 @@ This document lists the cryptographic algorithms supported by the ConnectBot SSH - `aes128-gcm@openssh.com` ([draft-miller-sshm-aes-gcm](https://datatracker.ietf.org/doc/html/draft-miller-sshm-aes-gcm)) - `aes256-ctr` ([RFC 4344](https://tools.ietf.org/html/rfc4344#section-4)) - `aes128-ctr` ([RFC 4344](https://tools.ietf.org/html/rfc4344#section-4)) -- `aes256-cbc` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-6.3)) -- `aes128-cbc` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-6.3)) -- `3des-cbc` ([RFC 4253](https://datatracker.ietf.org/doc/html/rfc4253#section-6.3)) +- `aes256-cbc` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-6.3)) — **legacy opt-in** +- `aes128-cbc` ([RFC 4253](https://tools.ietf.org/html/rfc4253#section-6.3)) — **legacy opt-in** +- `3des-cbc` ([RFC 4253](https://datatracker.ietf.org/doc/html/rfc4253#section-6.3)) — **legacy opt-in** ## MACs - `hmac-sha2-512-etm@openssh.com` ([OpenSSH PROTOCOL](https://github.com/openssh/openssh-portable/blob/60b909fb110f77c1ffd15cceb5d09b8e3f79b27e/PROTOCOL#L50)) - `hmac-sha2-256-etm@openssh.com` ([OpenSSH PROTOCOL](https://github.com/openssh/openssh-portable/blob/60b909fb110f77c1ffd15cceb5d09b8e3f79b27e/PROTOCOL#L50)) -- `hmac-sha1-etm@openssh.com` ([OpenSSH PROTOCOL](https://github.com/openssh/openssh-portable/blob/60b909fb110f77c1ffd15cceb5d09b8e3f79b27e/PROTOCOL#L50)) -- `hmac-sha2-512` ([RFC 4868](https://tools.ietf.org/html/rfc4868)) -- `hmac-sha2-256` ([RFC 4868](https://tools.ietf.org/html/rfc4868)) -- `hmac-sha1` ([RFC 4253](https://tools.ietf.org/html/rfc4253)) +- `hmac-sha1-etm@openssh.com` ([OpenSSH PROTOCOL](https://github.com/openssh/openssh-portable/blob/60b909fb110f77c1ffd15cceb5d09b8e3f79b27e/PROTOCOL#L50)) — **legacy opt-in** +- `hmac-sha2-512` ([RFC 4868](https://tools.ietf.org/html/rfc4868)) — **legacy opt-in** +- `hmac-sha2-256` ([RFC 4868](https://tools.ietf.org/html/rfc4868)) — **legacy opt-in** +- `hmac-sha1` ([RFC 4253](https://tools.ietf.org/html/rfc4253)) — **legacy opt-in** diff --git a/protocol/src/main/resources/kaitai/ssh_agentc_session_bind.ksy b/protocol/src/main/resources/kaitai/ssh_agentc_session_bind.ksy index ef8ba74b..9b45606f 100644 --- a/protocol/src/main/resources/kaitai/ssh_agentc_session_bind.ksy +++ b/protocol/src/main/resources/kaitai/ssh_agentc_session_bind.ksy @@ -14,6 +14,8 @@ seq: - id: session_identifier type: byte_string doc: SSH session identifier (H from key exchange) + valid: + expr: _.data.size <= 128 - id: signature type: byte_string doc: Signature of (hostkey || session_identifier) by host key diff --git a/protocol/src/main/resources/kaitai/userauth_publickey_signature_data_any.ksy b/protocol/src/main/resources/kaitai/userauth_publickey_signature_data_any.ksy index 891affa1..65924ff9 100644 --- a/protocol/src/main/resources/kaitai/userauth_publickey_signature_data_any.ksy +++ b/protocol/src/main/resources/kaitai/userauth_publickey_signature_data_any.ksy @@ -22,9 +22,13 @@ seq: - id: service_name type: ascii_string doc: Service name + valid: + expr: _.value == "ssh-connection" - id: method_name type: ascii_string doc: Authentication method name + valid: + expr: _.value == "publickey" or _.value == "publickey-hostbound-v00@openssh.com" - id: has_signature contents: [1] doc: TRUE (1) diff --git a/sshlib/api.txt b/sshlib/api.txt index 9fde5030..f2dbb366 100644 --- a/sshlib/api.txt +++ b/sshlib/api.txt @@ -33,8 +33,36 @@ package org.connectbot.sshlib { } public interface AgentProvider { - method public suspend java.lang.Object? getIdentities(kotlin.coroutines.Continuation>); - method public suspend java.lang.Object? signData(org.connectbot.sshlib.AgentSigningContext context, kotlin.coroutines.Continuation); + method public suspend java.lang.Object? getIdentities(kotlin.coroutines.Continuation>>); + method public suspend java.lang.Object? signData(org.connectbot.sshlib.AgentSigningContext context, kotlin.coroutines.Continuation>); + } + + public sealed exhaustive interface AgentResult { + } + + public static final class AgentResult.Failure implements org.connectbot.sshlib.AgentResult { + ctor public AgentResult.Failure(java.lang.String message, optional java.lang.Throwable? cause); + method public java.lang.String component1(); + method public java.lang.Throwable? component2(); + method public org.connectbot.sshlib.AgentResult.Failure copy(optional java.lang.String message, optional java.lang.Throwable? cause); + method public boolean equals(java.lang.Object? other); + method @InaccessibleFromKotlin public java.lang.Throwable? getCause(); + method @InaccessibleFromKotlin public java.lang.String getMessage(); + method public int hashCode(); + method public java.lang.String toString(); + property public Throwable? cause; + property public String message; + } + + public static final class AgentResult.Success implements org.connectbot.sshlib.AgentResult { + ctor public AgentResult.Success(T value); + method public T component1(); + method public org.connectbot.sshlib.AgentResult.Success copy(optional T value); + method public boolean equals(java.lang.Object? other); + method @InaccessibleFromKotlin public T getValue(); + method public int hashCode(); + method public java.lang.String toString(); + property public T value; } public final class AgentSignatureFlags { diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentProvider.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentProvider.kt index d4a2afae..e8df3288 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentProvider.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentProvider.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,15 +69,18 @@ data class DestinationConstraint( * * Implement this interface to handle agent requests from remote servers * when agent forwarding is enabled. The library will call these methods - * when a remote server requests identities or signatures. + * when a remote server requests identities or signatures. Implementations + * should return [AgentResult.Failure] instead of throwing when a backend + * operation fails. */ interface AgentProvider { /** * Return the list of identities available for forwarding. * * Called when a remote server requests the list of available keys. + * Return [AgentResult.Failure] if the identities cannot be loaded. */ - suspend fun getIdentities(): List + suspend fun getIdentities(): AgentResult> /** * Sign data with the specified key. @@ -85,12 +88,13 @@ interface AgentProvider { * Called when a remote server requests a signature. The provider should: * 1. Verify the request is legitimate (check context) * 2. Optionally prompt the user for approval - * 3. Return the signature, or null to deny the request + * 3. Return the signature, or a successful null value to deny the request * * @param context Rich context about the signing request - * @return SSH-encoded signature blob, or null to refuse + * @return Successful SSH-encoded signature blob, successful null to refuse, + * or [AgentResult.Failure] if the provider could not process the request */ - suspend fun signData(context: AgentSigningContext): ByteArray? + suspend fun signData(context: AgentSigningContext): AgentResult } /** @@ -145,7 +149,7 @@ data class AgentSigningContext( /** Server's host key from key exchange */ val serverHostKey: ByteArray, - /** Whether session-bind extension was used */ + /** Whether a trusted session binding is active */ val isBound: Boolean, ) { override fun equals(other: Any?): Boolean { diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentResult.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentResult.kt new file mode 100644 index 00000000..4b96e6fe --- /dev/null +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/AgentResult.kt @@ -0,0 +1,35 @@ +/* + * ConnectBot SSH Library + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.connectbot.sshlib + +/** + * Result of an [AgentProvider] callback. + * + * Provider failures are values rather than thrown exceptions so an agent + * backend failure cannot terminate the SSH connection's packet loop. + */ +sealed interface AgentResult { + /** The provider completed successfully with [value]. */ + data class Success(val value: T) : AgentResult + + /** The provider could not complete the request. */ + data class Failure( + val message: String, + val cause: Throwable? = null, + ) : AgentResult +} diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/SshKeys.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/SshKeys.kt index fc7fa57c..cb171841 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/SshKeys.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/SshKeys.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,7 +57,8 @@ object SshKeys { * - Ed25519 keys are encoded as PKCS#8 (`BEGIN PRIVATE KEY`) * * @param keyPair JCA key pair to encode - * @param password Optional passphrase to encrypt the key (AES-256-CBC) + * @param password Optional passphrase to encrypt the key. RSA and EC use legacy + * PEM AES-256-CBC encryption; Ed25519 uses PBES2 encrypted PKCS#8. * @return PEM-encoded private key string * @throws SshException if the key type is unsupported */ diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/SshSession.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/SshSession.kt index a710573f..ef34d15d 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/SshSession.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/SshSession.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -78,6 +78,10 @@ interface SshSession : AutoCloseable { suspend fun read(): ByteArray? + /** + * Read non-stderr extended data. RFC 4254 data type 1 is exposed exclusively + * through [stderr] so the same remote bytes are not buffered twice. + */ suspend fun readExtended(): Pair? suspend fun sendEof() diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentChannel.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentChannel.kt index 3b07b138..aa4fcbe3 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentChannel.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentChannel.kt @@ -17,12 +17,17 @@ package org.connectbot.sshlib.client +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.launch import org.slf4j.LoggerFactory internal class AgentChannel( private val handler: AgentProtocolHandler, private val connection: SshConnection, + scope: CoroutineScope, private val localChannelNumber: Int, private var remoteChannelNumber: Int, private val maxPacketSize: Int, @@ -38,6 +43,16 @@ internal class AgentChannel( private val window = LocalChannelWindow(initialWindowSize, remoteInitial = remoteWindowSizeInitial) private val windowAvailable = Channel(Channel.CONFLATED) + private val requests = Channel(Channel.UNLIMITED) + private val requestWorker: Job = scope.launch { + for (data in requests) { + val response = handler.handleRequest(data) + connection.sendWindowAdjust(remoteChannelNumber, window.releaseLocal(data.size)) + + logger.debug("Sending agent response (${response.size} bytes)") + sendData(response) + } + } val isOpen: Boolean get() = _isOpen @@ -46,17 +61,13 @@ internal class AgentChannel( logger.warn("Received data on closed agent channel") return } - val adjust = window.consumeLocal(data.size) + window.consumeLocal(data.size) - logger.debug("Agent channel received ${data.size} bytes") - if (adjust > 0) { - connection.sendWindowAdjust(remoteChannelNumber, adjust) + logger.debug("Queueing ${data.size} bytes received on agent channel") + if (requests.trySend(data).isFailure) { + window.releaseLocal(data.size) + logger.warn("Discarding data received while agent channel is closing") } - - val response = handler.handleRequest(data) - - logger.debug("Sending agent response (${response.size} bytes)") - sendData(response) } fun onWindowAdjust(bytesToAdd: Long) { @@ -82,6 +93,8 @@ internal class AgentChannel( } } _isOpen = false + requests.close() + requestWorker.cancelAndJoin() windowAvailable.close() } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentProtocolHandler.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentProtocolHandler.kt index b36e9b08..cbdc44cb 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentProtocolHandler.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/AgentProtocolHandler.kt @@ -23,10 +23,10 @@ import io.kaitai.struct.KaitaiStruct import org.connectbot.sshlib.AgentIdentity import org.connectbot.sshlib.AgentKeySpec import org.connectbot.sshlib.AgentProvider +import org.connectbot.sshlib.AgentResult import org.connectbot.sshlib.AgentSigningContext import org.connectbot.sshlib.DestinationConstraint import org.connectbot.sshlib.crypto.SignatureVerifier -import org.connectbot.sshlib.kaitaiParseFailureOrNull import org.connectbot.sshlib.protocol.SshAgentIdentitiesAnswer import org.connectbot.sshlib.protocol.SshAgentMessage import org.connectbot.sshlib.protocol.SshAgentSignResponse @@ -37,24 +37,42 @@ import org.connectbot.sshlib.protocol.UserauthPublickeySignatureDataAny import org.connectbot.sshlib.protocol.createByteString import org.connectbot.sshlib.protocol.toByteArray import org.slf4j.LoggerFactory +import java.util.concurrent.CancellationException internal fun interface SessionBindVerifier { fun verify(hostKeyBlob: ByteArray, signature: ByteArray, data: ByteArray): Boolean } -internal data class BindingEntry(val hostKeyBlob: ByteArray, val sessionId: ByteArray) { +internal const val AGENT_MAX_SESSION_ID_LENGTH = 128 + +internal sealed interface AgentParseResult { + data class Success(val value: T) : AgentParseResult + + data class Failure( + val message: String, + val cause: Throwable? = null, + ) : AgentParseResult +} + +internal data class BindingEntry( + val hostKeyBlob: ByteArray, + val sessionId: ByteArray, + val isForwarding: Boolean = false, +) { override fun equals(other: Any?): Boolean { if (this === other) return true if (javaClass != other?.javaClass) return false other as BindingEntry if (!hostKeyBlob.contentEquals(other.hostKeyBlob)) return false if (!sessionId.contentEquals(other.sessionId)) return false + if (isForwarding != other.isForwarding) return false return true } override fun hashCode(): Int { var result = hostKeyBlob.contentHashCode() result = 31 * result + sessionId.contentHashCode() + result = 31 * result + isForwarding.hashCode() return result } } @@ -63,6 +81,7 @@ internal data class SignedDataComponents( val methodName: String, val destUsername: String, val serverHostKeyBlob: ByteArray?, + val sessionId: ByteArray = ByteArray(0), ) { override fun equals(other: Any?): Boolean { if (this === other) return true @@ -70,6 +89,7 @@ internal data class SignedDataComponents( other as SignedDataComponents if (methodName != other.methodName) return false if (destUsername != other.destUsername) return false + if (!sessionId.contentEquals(other.sessionId)) return false if (!(serverHostKeyBlob == null && other.serverHostKeyBlob == null) && ( serverHostKeyBlob == null || other.serverHostKeyBlob == null || @@ -85,6 +105,7 @@ internal data class SignedDataComponents( var result = methodName.hashCode() result = 31 * result + destUsername.hashCode() result = 31 * result + (serverHostKeyBlob?.contentHashCode() ?: 0) + result = 31 * result + sessionId.contentHashCode() return result } } @@ -101,35 +122,56 @@ internal fun buildAgentMessage(messageType: Int, payload: ByteArray): ByteArray internal fun isConstraintSatisfied( constraints: List, - components: SignedDataComponents, bindingList: List, + destinationUsername: String?, ): Boolean { - val isForwarding = bindingList.isNotEmpty() - val forwardingHopKey = if (bindingList.size >= 2) { - bindingList[bindingList.size - 2].hostKeyBlob - } else if (bindingList.size == 1) { - bindingList[0].hostKeyBlob - } else { - null - } + if (bindingList.isEmpty()) return false + + fun keyMatches(specs: List, key: ByteArray): Boolean = specs.any { !it.isCa && it.keyBlob.contentEquals(key) } - return constraints.any { c -> - val fromMatches = if (!isForwarding) { - c.fromHostname.isEmpty() && c.fromKeyspecs.isEmpty() + fun edgeMatches( + fromKey: ByteArray?, + toKey: ByteArray?, + username: String?, + ): Boolean = constraints.any { constraint -> + val fromMatches = if (fromKey == null) { + constraint.fromHostname.isEmpty() && constraint.fromKeyspecs.isEmpty() } else { - forwardingHopKey != null && c.fromKeyspecs.any { spec -> - spec.keyBlob.contentEquals(forwardingHopKey) - } + keyMatches(constraint.fromKeyspecs, fromKey) } if (!fromMatches) return@any false - val usernameMatches = c.toUsername.isEmpty() || c.toUsername == components.destUsername - if (!usernameMatches) return@any false + if (username != null && + constraint.toUsername.isNotEmpty() && + constraint.toUsername != username + ) { + return@any false + } + + toKey == null || keyMatches(constraint.toHostspecs, toKey) + } + + for (index in bindingList.indices) { + val binding = bindingList[index] + val isLast = index == bindingList.lastIndex + + if (!isLast && !binding.isForwarding) return false + if (isLast && destinationUsername != null && binding.isForwarding) return false - val hostKeyMatches = components.serverHostKeyBlob != null && - c.toHostspecs.any { spec -> spec.keyBlob.contentEquals(components.serverHostKeyBlob) } - hostKeyMatches + val fromKey = bindingList.getOrNull(index - 1)?.hostKeyBlob + val username = destinationUsername.takeIf { isLast } + if (!edgeMatches(fromKey, binding.hostKeyBlob, username)) return false } + + // A forwarding-only final binding may list a constrained identity only if + // at least one permitted next hop starts at that host. + if (destinationUsername == null && bindingList.last().isForwarding) { + if (!edgeMatches(bindingList.last().hostKeyBlob, null, null)) { + return false + } + } + + return true } internal class AgentProtocolHandler( @@ -150,47 +192,74 @@ internal class AgentProtocolHandler( const val SSH_AGENTC_EXTENSION: Int = 27 const val SSH_AGENT_SUCCESS: Int = 6 + private const val MAX_BINDINGS = 16 + private const val SERVICE_CONNECTION = "ssh-connection" + private const val METHOD_PUBLICKEY = "publickey" private const val METHOD_PUBLICKEY_HOSTBOUND = "publickey-hostbound-v00@openssh.com" + private const val KEY_TYPE_SSH_RSA = "ssh-rsa" + private const val KEY_TYPE_SSH_RSA_CERT = "ssh-rsa-cert-v01@openssh.com" } - private val bindingList: MutableList = mutableListOf() + private val bindingList: MutableList = mutableListOf().apply { + if (sessionInfo.sessionId.isNotEmpty() && + sessionInfo.sessionId.size <= AGENT_MAX_SESSION_ID_LENGTH && + sessionInfo.serverHostKey.isNotEmpty() + ) { + add( + BindingEntry( + hostKeyBlob = sessionInfo.serverHostKey.copyOf(), + sessionId = sessionInfo.sessionId.copyOf(), + isForwarding = true, + ), + ) + } + } - suspend fun handleRequest(requestBytes: ByteArray): ByteArray { + suspend fun handleRequest(requestBytes: ByteArray): ByteArray = try { logger.debug("Handling agent request (${requestBytes.size} bytes)") - val message = try { - parseAgentMessage(requestBytes) - } catch (e: MalformedAgentRequestException) { - logger.warn("Malformed SSH agent request", e.cause) - return createFailureResponse() + val message = when (val result = parseAgentMessage(requestBytes)) { + is AgentParseResult.Success -> result.value + + is AgentParseResult.Failure -> { + logParseFailure(result) + return createFailureResponse() + } } val messageType = message.messageType() logger.debug("Agent message type: $messageType") - return try { - when (messageType) { - SSH_AGENTC_REQUEST_IDENTITIES -> handleRequestIdentities() + when (messageType) { + SSH_AGENTC_REQUEST_IDENTITIES -> handleRequestIdentities() - SSH_AGENTC_SIGN_REQUEST -> handleSignRequest(message) + SSH_AGENTC_SIGN_REQUEST -> handleSignRequest(message) - SSH_AGENTC_EXTENSION -> handleExtension(message) + SSH_AGENTC_EXTENSION -> handleExtension(message) - else -> { - logger.warn("Unknown agent message type: $messageType") - createFailureResponse() - } + else -> { + logger.warn("Unknown agent message type: $messageType") + createFailureResponse() } - } catch (e: MalformedAgentRequestException) { - logger.warn("Malformed SSH agent request", e.cause) - return createFailureResponse() } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + logger.error("Agent request failed without a protocol response", e) + createFailureResponse() } private suspend fun handleRequestIdentities(): ByteArray { logger.debug("Handling REQUEST_IDENTITIES") - val allIdentities = provider.getIdentities() + val allIdentities = when (val result = getProviderIdentities()) { + is AgentResult.Success -> result.value + + is AgentResult.Failure -> { + logProviderFailure("get identities", result) + return createFailureResponse() + } + } val visibleIdentities = filterVisibleIdentities(allIdentities) logger.debug("Provider returned ${allIdentities.size} identities, ${visibleIdentities.size} visible for current path") @@ -214,46 +283,75 @@ internal class AgentProtocolHandler( } private fun filterVisibleIdentities(identities: List): List { - if (bindingList.isEmpty()) return identities - val lastHopKey = bindingList.last().hostKeyBlob return identities.filter { identity -> val constraints = identity.destinationConstraints if (constraints == null) return@filter true - constraints.any { c -> - c.fromKeyspecs.isNotEmpty() && c.fromKeyspecs.any { spec -> - spec.keyBlob.contentEquals(lastHopKey) - } - } + isConstraintSatisfied(constraints, bindingList, destinationUsername = null) } } private suspend fun handleSignRequest(message: SshAgentMessage): ByteArray { logger.debug("Handling SIGN_REQUEST") - val payload = parsePayload(message) + val payload = when (val result = parsePayload(message)) { + is AgentParseResult.Success -> result.value + + is AgentParseResult.Failure -> { + logParseFailure(result) + return createFailureResponse() + } + } val keyBlob = payload.keyBlob().data() val dataToSign = payload.data().data() - val identity = provider.getIdentities().find { it.publicKeyBlob.contentEquals(keyBlob) } - val constraints = identity?.destinationConstraints + val identities = when (val result = getProviderIdentities()) { + is AgentResult.Success -> result.value + + is AgentResult.Failure -> { + logProviderFailure("get identities for signing", result) + return createFailureResponse() + } + } + val identity = identities.find { it.publicKeyBlob.contentEquals(keyBlob) } + if (identity == null) { + logger.warn("Refusing signing request for an identity not exposed by the provider") + return createFailureResponse() + } + val constraints = identity.destinationConstraints if (constraints != null) { - var components = parseSignedDataComponents(dataToSign) - if (components == null) { - logger.warn("Failed to parse signed data for constraint check") + val components = when (val result = parseSignedDataComponents(dataToSign, keyBlob)) { + is AgentParseResult.Success -> result.value + + is AgentParseResult.Failure -> { + logParseFailure(result) + return createFailureResponse() + } + } + + val latestBinding = bindingList.lastOrNull() + if (latestBinding == null) { + logger.warn("Refusing constrained signing without a trusted session binding") + return createFailureResponse() + } + if (!components.sessionId.contentEquals(latestBinding.sessionId)) { + logger.warn("Signed session ID does not match the active session binding") return createFailureResponse() } - if (bindingList.isNotEmpty() && components.methodName != METHOD_PUBLICKEY_HOSTBOUND) { + if (bindingList.size > 1 && components.methodName != METHOD_PUBLICKEY_HOSTBOUND) { logger.warn("Forwarded connection requires publickey-hostbound method, got: ${components.methodName}") return createFailureResponse() } - if (bindingList.isEmpty() && components.serverHostKeyBlob == null) { - components = components.copy(serverHostKeyBlob = sessionInfo.serverHostKey) + if (components.serverHostKeyBlob == null || + !components.serverHostKeyBlob.contentEquals(latestBinding.hostKeyBlob) + ) { + logger.warn("Signed server host key does not match the active session binding") + return createFailureResponse() } - if (!isConstraintSatisfied(constraints, components, bindingList)) { + if (!isConstraintSatisfied(constraints, bindingList, components.destUsername)) { logger.warn("Destination constraint not satisfied for key") return createFailureResponse() } @@ -261,19 +359,27 @@ internal class AgentProtocolHandler( val isBound = bindingList.isNotEmpty() val effectiveSessionId = bindingList.lastOrNull()?.sessionId ?: sessionInfo.sessionId + val effectiveServerHostKey = bindingList.lastOrNull()?.hostKeyBlob ?: sessionInfo.serverHostKey val context = AgentSigningContext( - publicKeyBlob = keyBlob, - dataToSign = dataToSign, + publicKeyBlob = keyBlob.copyOf(), + dataToSign = dataToSign.copyOf(), flags = payload.flags().toInt(), - sessionId = effectiveSessionId, - serverHostKey = sessionInfo.serverHostKey, + sessionId = effectiveSessionId.copyOf(), + serverHostKey = effectiveServerHostKey.copyOf(), isBound = isBound, ) logger.debug("Requesting signature from provider (bound=$isBound, flags=${context.flags})") - val signature = provider.signData(context) + val signature = when (val result = signWithProvider(context)) { + is AgentResult.Success -> result.value + + is AgentResult.Failure -> { + logProviderFailure("sign data", result) + return createFailureResponse() + } + } return if (signature != null) { logger.debug("Provider approved signing request") @@ -287,25 +393,72 @@ internal class AgentProtocolHandler( } } - private fun parseSignedDataComponents(data: ByteArray): SignedDataComponents? = try { - val stream = ByteBufferKaitaiStream(data) - val sigData = UserauthPublickeySignatureDataAny(stream) - sigData._read() + private fun parseSignedDataComponents( + data: ByteArray, + expectedPublicKeyBlob: ByteArray, + ): AgentParseResult { + val parsed = parseAgentInput("Malformed public-key authentication data") { + val stream = ByteBufferKaitaiStream(data) + val sigData = UserauthPublickeySignatureDataAny(stream) + sigData._read() + sigData to stream.isEof + } + val (sigData, isFullyConsumed) = when (parsed) { + is AgentParseResult.Success -> parsed.value + is AgentParseResult.Failure -> return parsed + } + + val serviceName = sigData.serviceName().value() + val methodName = sigData.methodName().value() + val algorithmName = sigData.publicKeyAlgorithmName().value() + val embeddedPublicKey = sigData.publicKeyBlob().data() + val keyType = keyBlobAlgorithmName(expectedPublicKeyBlob) + val algorithmMatches = when (keyType) { + KEY_TYPE_SSH_RSA -> + algorithmName == KEY_TYPE_SSH_RSA || + algorithmName == "rsa-sha2-256" || + algorithmName == "rsa-sha2-512" + + KEY_TYPE_SSH_RSA_CERT -> + algorithmName == KEY_TYPE_SSH_RSA_CERT || + algorithmName == "rsa-sha2-256-cert-v01@openssh.com" || + algorithmName == "rsa-sha2-512-cert-v01@openssh.com" + + null -> false + + else -> algorithmName == keyType + } - SignedDataComponents( - methodName = sigData.methodName().value(), - destUsername = sigData.userName().value(), - serverHostKeyBlob = sigData.serverHostKey()?.data(), + if (!isFullyConsumed || + serviceName != SERVICE_CONNECTION || + (methodName != METHOD_PUBLICKEY && methodName != METHOD_PUBLICKEY_HOSTBOUND) || + !embeddedPublicKey.contentEquals(expectedPublicKeyBlob) || + !algorithmMatches + ) { + return AgentParseResult.Failure("Inconsistent public-key authentication data") + } + + return AgentParseResult.Success( + SignedDataComponents( + methodName = methodName, + destUsername = sigData.userName().value(), + serverHostKeyBlob = sigData.serverHostKey()?.data(), + sessionId = sigData.sessionIdentifier().data(), + ), ) - } catch (e: Exception) { - logger.debug("Failed to parse signed data components: ${e.message}") - null } private suspend fun handleExtension(message: SshAgentMessage): ByteArray { logger.debug("Handling EXTENSION") - val ext = parsePayload(message) + val ext = when (val result = parsePayload(message)) { + is AgentParseResult.Success -> result.value + + is AgentParseResult.Failure -> { + logParseFailure(result) + return createFailureResponse() + } + } val extensionName = String(ext.extensionName().data(), Charsets.UTF_8) logger.debug("Extension name: $extensionName") @@ -320,63 +473,118 @@ internal class AgentProtocolHandler( private fun handleSessionBind(extensionData: ByteArray): ByteArray { logger.debug("Handling session-bind@openssh.com extension") + val parsed = parseAgentInput("Malformed session-bind extension") { + val stream = ByteBufferKaitaiStream(extensionData) + val bind = SshAgentcSessionBind(stream) + bind._read() + bind to stream.isEof + } + val (bind, isFullyConsumed) = when (parsed) { + is AgentParseResult.Success -> parsed.value - val stream = ByteBufferKaitaiStream(extensionData) - val bind = SshAgentcSessionBind(stream) - bind._read() + is AgentParseResult.Failure -> { + logParseFailure(parsed) + return createFailureResponse() + } + } val hostKeyBlob = bind.hostkey().data() val sessionId = bind.sessionIdentifier().data() val isForwarding = bind.isForwarding() != 0 + if (!isFullyConsumed) { + logger.warn("Session bind request contains trailing data") + return createFailureResponse() + } + if (sessionId.isEmpty() || sessionId.size > AGENT_MAX_SESSION_ID_LENGTH) { + logger.warn("Session bind identifier has invalid length: ${sessionId.size}") + return createFailureResponse() + } + if (bindingList.size >= MAX_BINDINGS) { + logger.warn("Too many session bindings") + return createFailureResponse() + } + if (bindingList.lastOrNull()?.isForwarding == false) { + logger.warn("Session binding already finalized for authentication") + return createFailureResponse() + } if (bindingList.any { it.sessionId.contentEquals(sessionId) }) { logger.warn("Session bind replay: session ID already recorded") return createFailureResponse() } - - if (!isForwarding && !hostKeyBlob.contentEquals(sessionInfo.serverHostKey)) { - logger.error("Session bind hostkey mismatch for non-forwarding bind") - return createFailureResponse() + val verified = try { + bindVerifier.verify(hostKeyBlob, bind.signature().data(), sessionId) + } catch (e: Exception) { + logger.warn("Session bind verifier failed", e) + false } - - if (!bindVerifier.verify(hostKeyBlob, bind.signature().data(), sessionId)) { + if (!verified) { logger.error("Session bind signature verification failed") return createFailureResponse() } - bindingList.add(BindingEntry(hostKeyBlob, sessionId)) + bindingList.add( + BindingEntry(hostKeyBlob.copyOf(), sessionId.copyOf(), isForwarding), + ) logger.info("Session binding successful (isForwarding=$isForwarding, total bindings=${bindingList.size})") return createSuccessResponse() } - private fun parseAgentMessage(requestBytes: ByteArray): SshAgentMessage = try { + private fun parseAgentMessage(requestBytes: ByteArray): AgentParseResult = parseAgentInput("Malformed SSH agent request") { val stream = ByteBufferKaitaiStream(requestBytes) val message = SshAgentMessage(stream) message._read() message - } catch (e: RuntimeException) { - throwMalformedAgentRequest(e) } - private inline fun parsePayload(message: SshAgentMessage): T = try { + private inline fun parsePayload( + message: SshAgentMessage, + ): AgentParseResult = parseAgentInput("Malformed SSH agent payload") { val stream = ByteBufferKaitaiStream(message._raw_payload()) val payload = T::class.java.getConstructor(KaitaiStream::class.java).newInstance(stream) payload._read() payload + } + + private inline fun parseAgentInput( + description: String, + parse: () -> T, + ): AgentParseResult = try { + AgentParseResult.Success(parse()) } catch (e: Exception) { - throwMalformedAgentRequest(e) + AgentParseResult.Failure(description, e) } - private fun createFailureResponse(): ByteArray = buildAgentMessage(SSH_AGENT_FAILURE, ByteArray(0)) + private suspend fun getProviderIdentities(): AgentResult> = callProvider("Agent provider threw while retrieving identities") { + provider.getIdentities() + } - private fun createSuccessResponse(): ByteArray = buildAgentMessage(SSH_AGENT_SUCCESS, ByteArray(0)) + private suspend fun signWithProvider(context: AgentSigningContext): AgentResult = callProvider("Agent provider threw while signing") { + provider.signData(context) + } + + private suspend inline fun callProvider( + failureMessage: String, + callback: suspend () -> AgentResult, + ): AgentResult = try { + callback() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + AgentResult.Failure(failureMessage, e) + } + + private fun logParseFailure(failure: AgentParseResult.Failure) { + logger.warn(failure.message, failure.cause) + } - private fun throwMalformedAgentRequest(e: Exception): Nothing { - val parseFailure = e.kaitaiParseFailureOrNull() ?: throw e - throw MalformedAgentRequestException(parseFailure) + private fun logProviderFailure(operation: String, failure: AgentResult.Failure) { + logger.warn("Agent provider failed to $operation: ${failure.message}", failure.cause) } - private class MalformedAgentRequestException(cause: Throwable) : Exception(cause) + private fun createFailureResponse(): ByteArray = buildAgentMessage(SSH_AGENT_FAILURE, ByteArray(0)) + + private fun createSuccessResponse(): ByteArray = buildAgentMessage(SSH_AGENT_SUCCESS, ByteArray(0)) } internal data class AgentSessionInfo( diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/ForwardingChannel.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/ForwardingChannel.kt index 4899de3e..15d0112d 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/ForwardingChannel.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/ForwardingChannel.kt @@ -17,12 +17,15 @@ package org.connectbot.sshlib.client +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.ReceiveChannel +import kotlinx.coroutines.launch import org.slf4j.LoggerFactory internal class ForwardingChannel( private val connection: SshConnection, + private val connectionScope: CoroutineScope, val localChannelNumber: Int, var remoteChannelNumber: Int, private val maxPacketSize: Int, @@ -31,28 +34,37 @@ internal class ForwardingChannel( ) { companion object { private val logger = LoggerFactory.getLogger(ForwardingChannel::class.java) - private const val WINDOW_ADJUST_THRESHOLD = 64 * 1024 } private var _isOpen = true private var closeSent = false private val window = LocalChannelWindow( initialWindowSize, - adjustThreshold = WINDOW_ADJUST_THRESHOLD, remoteInitial = remoteWindowSizeInitial, ) private val windowAvailable = Channel(Channel.CONFLATED) - private val _incomingData = Channel(Channel.UNLIMITED) + private val incomingIngress = Channel(Channel.UNLIMITED) + private val _incomingData = Channel(Channel.RENDEZVOUS) val incomingData: ReceiveChannel get() = _incomingData + private val incomingDeliveryJob = connectionScope.launch { + try { + for (data in incomingIngress) { + _incomingData.send(data) + val adjust = window.releaseLocal(data.size) + connection.sendWindowAdjust(remoteChannelNumber, adjust) + } + } finally { + _incomingData.close() + } + } val isOpen: Boolean get() = _isOpen internal suspend fun onData(data: ByteArray) { - val adjust = window.consumeLocal(data.size) - _incomingData.trySend(data) - if (adjust > 0) { - connection.sendWindowAdjust(remoteChannelNumber, adjust) + window.consumeLocal(data.size) + if (incomingIngress.trySend(data).isFailure) { + throw org.connectbot.sshlib.SshException("Received data for a closed forwarding stream") } } @@ -66,7 +78,7 @@ internal class ForwardingChannel( internal fun onEof() { logger.debug("Forwarding channel $localChannelNumber received EOF") - _incomingData.close() + incomingIngress.close() } internal suspend fun onClose() { @@ -80,6 +92,8 @@ internal class ForwardingChannel( } } _isOpen = false + incomingIngress.close() + incomingDeliveryJob.cancel() _incomingData.close() windowAvailable.close() } @@ -106,6 +120,8 @@ internal class ForwardingChannel( if (!_isOpen) return closeSent = true _isOpen = false + incomingIngress.close() + incomingDeliveryJob.cancel() _incomingData.close() connection.sendChannelClose(remoteChannelNumber) } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/LocalChannelWindow.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/LocalChannelWindow.kt index 2b680ad4..dd11a62a 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/LocalChannelWindow.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/LocalChannelWindow.kt @@ -23,14 +23,13 @@ import org.connectbot.sshlib.SshException * Tracks SSH channel flow control windows per RFC 4254 §5.2. * * Local window: bytes we are willing to receive. Enforces that the server never - * exceeds it; auto-refills when below [adjustThreshold]. + * exceeds it; callers explicitly release capacity after consuming buffered data. * * Remote window: bytes the server is willing to receive. Enforces uint32 max * and positive-only adjustments per the protocol. */ internal class LocalChannelWindow( private val initialSize: Int, - private val adjustThreshold: Int = 16 * 1024, remoteInitial: Long = 0, ) { companion object { @@ -44,20 +43,27 @@ internal class LocalChannelWindow( /** * Consume [size] bytes from the local window, rejecting excess data. - * Returns the window-adjust amount to send back (0 if no adjust needed). */ - fun consumeLocal(size: Int): Int { + @Synchronized + fun consumeLocal(size: Int) { + if (size < 0) throw SshException("Cannot consume a negative local window size: $size") if (size > localRemaining) { throw SshException("Server sent $size bytes exceeding local window ($localRemaining)") } localRemaining -= size - return if (localRemaining < adjustThreshold) { - val adjust = initialSize - localRemaining.toInt() - localRemaining += adjust - adjust - } else { - 0 + } + + /** + * Release locally consumed bytes and return the window credit to advertise. + */ + @Synchronized + fun releaseLocal(size: Int): Int { + if (size <= 0) throw SshException("Local window release must be positive: $size") + if (localRemaining + size > initialSize) { + throw SshException("Local window release exceeds initial size") } + localRemaining += size + return size } /** diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/RemotePortForwarder.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/RemotePortForwarder.kt index cc120dc2..bbef815d 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/RemotePortForwarder.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/RemotePortForwarder.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -94,6 +94,7 @@ internal class RemotePortForwarder( val fwdChannel = ForwardingChannel( connection, + scope, localChannelNumber, senderChannel, maxPacketSize, 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 65e377f5..1b0ec3de 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SessionChannel.kt @@ -58,9 +58,22 @@ class SessionChannel internal constructor( private val window = LocalChannelWindow(initialWindowSize, remoteInitial = remoteWindowSizeInitial) private val windowAvailable = Channel(Channel.CONFLATED) - private val _stdout = Channel(Channel.UNLIMITED) - private val _stderr = Channel(Channel.UNLIMITED) - private val _extendedData = Channel>(Channel.UNLIMITED) + private val stdoutIngress = Channel(Channel.UNLIMITED) + private val stderrIngress = Channel(Channel.UNLIMITED) + private val extendedDataIngress = Channel>(Channel.UNLIMITED) + private val _stdout = Channel(Channel.RENDEZVOUS) + private val _stderr = Channel(Channel.RENDEZVOUS) + private val _extendedData = Channel>(Channel.RENDEZVOUS) + + private val stdoutDeliveryJob = connectionScope.launch { + deliverData(stdoutIngress, _stdout) { it.size } + } + private val stderrDeliveryJob = connectionScope.launch { + deliverData(stderrIngress, _stderr) { it.size } + } + private val extendedDeliveryJob = connectionScope.launch { + deliverData(extendedDataIngress, _extendedData) { it.second.size } + } override val isOpen: Boolean get() = _isOpen override val remoteChannelNumber: Int get() = _remoteChannelNumber @@ -83,21 +96,36 @@ class SessionChannel internal constructor( get() = ptyGranted && canSendChaff && obscureKeystrokeTimingIntervalMs > 0 internal suspend fun onData(data: ByteArray) { - val adjust = window.consumeLocal(data.size) - _stdout.trySend(data) - if (adjust > 0) { - connection.sendWindowAdjust(_remoteChannelNumber, adjust) + window.consumeLocal(data.size) + if (stdoutIngress.trySend(data).isFailure) { + throw org.connectbot.sshlib.SshException("Received data for a closed stdout stream") } } internal suspend fun onExtendedData(dataType: Int, data: ByteArray) { - val adjust = window.consumeLocal(data.size) - _extendedData.trySend(dataType to data) + window.consumeLocal(data.size) if (dataType == 1) { - _stderr.trySend(data) + if (stderrIngress.trySend(data).isFailure) { + throw org.connectbot.sshlib.SshException("Received data for a closed stderr stream") + } + } else if (extendedDataIngress.trySend(dataType to data).isFailure) { + throw org.connectbot.sshlib.SshException("Received data for a closed extended-data stream") } - if (adjust > 0) { - connection.sendWindowAdjust(_remoteChannelNumber, adjust) + } + + private suspend fun deliverData( + ingress: ReceiveChannel, + output: Channel, + sizeOf: (T) -> Int, + ) { + try { + for (value in ingress) { + output.send(value) + val adjust = window.releaseLocal(sizeOf(value)) + connection.sendWindowAdjust(_remoteChannelNumber, adjust) + } + } finally { + output.close() } } @@ -111,9 +139,9 @@ class SessionChannel internal constructor( internal fun onEof() { logger.debug("Received EOF on channel $localChannelNumber") - _stdout.close() - _stderr.close() - _extendedData.close() + stdoutIngress.close() + stderrIngress.close() + extendedDataIngress.close() } internal suspend fun onClose() { @@ -131,6 +159,12 @@ class SessionChannel internal constructor( } } _isOpen = false + stdoutIngress.close() + stderrIngress.close() + extendedDataIngress.close() + stdoutDeliveryJob.cancel() + stderrDeliveryJob.cancel() + extendedDeliveryJob.cancel() _stdout.close() _stderr.close() _extendedData.close() @@ -350,6 +384,12 @@ class SessionChannel internal constructor( chaffJob?.cancel() closeSent = true _isOpen = false + stdoutIngress.close() + stderrIngress.close() + extendedDataIngress.close() + stdoutDeliveryJob.cancel() + stderrDeliveryJob.cancel() + extendedDeliveryJob.cancel() _stdout.close() _stderr.close() _extendedData.close() diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt index 38cc78b2..b1ec62b0 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt @@ -64,6 +64,10 @@ import org.connectbot.sshlib.crypto.KexEntry import org.connectbot.sshlib.crypto.KexType import org.connectbot.sshlib.crypto.KeyDerivation import org.connectbot.sshlib.crypto.MacEntry +import org.connectbot.sshlib.crypto.PacketAead +import org.connectbot.sshlib.crypto.PacketCipher +import org.connectbot.sshlib.crypto.PacketCompressor +import org.connectbot.sshlib.crypto.PacketMac import org.connectbot.sshlib.crypto.SignatureEntry import org.connectbot.sshlib.crypto.SignatureVerifier import org.connectbot.sshlib.crypto.SshPrivateKey @@ -87,10 +91,12 @@ import org.connectbot.sshlib.protocol.SshMsgChannelClose import org.connectbot.sshlib.protocol.SshMsgChannelData import org.connectbot.sshlib.protocol.SshMsgChannelEof import org.connectbot.sshlib.protocol.SshMsgChannelExtendedData +import org.connectbot.sshlib.protocol.SshMsgChannelFailure import org.connectbot.sshlib.protocol.SshMsgChannelOpen import org.connectbot.sshlib.protocol.SshMsgChannelOpenConfirmation import org.connectbot.sshlib.protocol.SshMsgChannelOpenFailure import org.connectbot.sshlib.protocol.SshMsgChannelRequest +import org.connectbot.sshlib.protocol.SshMsgChannelSuccess import org.connectbot.sshlib.protocol.SshMsgChannelWindowAdjust import org.connectbot.sshlib.protocol.SshMsgDebug import org.connectbot.sshlib.protocol.SshMsgDisconnect @@ -142,6 +148,78 @@ import java.util.concurrent.atomic.AtomicLong import kotlin.coroutines.CoroutineContext import org.connectbot.sshlib.AuthResult as PublicAuthResult +internal fun boundRemotePacketSize(packetSize: Long): Int? = when { + packetSize <= 0L -> null + packetSize > 0xFFFFFFFFL -> null + else -> minOf(packetSize, 32L * 1024L).toInt() +} + +private sealed interface PendingProtection { + fun installOutbound(packetIO: PacketIO) + fun installInbound(packetIO: PacketIO) + fun destroy() +} + +private class PendingAeadProtection( + private var outbound: PacketAead?, + private var inbound: PacketAead?, +) : PendingProtection { + override fun installOutbound(packetIO: PacketIO) { + val aead = outbound ?: throw SshException("Outbound AEAD protection already installed") + packetIO.enableSendAead(aead) + outbound = null + } + + override fun installInbound(packetIO: PacketIO) { + val aead = inbound ?: throw SshException("Inbound AEAD protection already installed") + packetIO.enableReceiveAead(aead) + inbound = null + } + + override fun destroy() { + outbound?.destroy() + outbound = null + inbound?.destroy() + inbound = null + } +} + +private class PendingCipherProtection( + private var outboundCipher: PacketCipher?, + private var outboundMac: PacketMac?, + private val outboundEtm: Boolean, + private var inboundCipher: PacketCipher?, + private var inboundMac: PacketMac?, + private val inboundEtm: Boolean, +) : PendingProtection { + override fun installOutbound(packetIO: PacketIO) { + val cipher = outboundCipher ?: throw SshException("Outbound cipher protection already installed") + val mac = outboundMac ?: throw SshException("Outbound MAC protection already installed") + packetIO.enableSendEncryption(cipher, mac, outboundEtm) + outboundCipher = null + outboundMac = null + } + + override fun installInbound(packetIO: PacketIO) { + val cipher = inboundCipher ?: throw SshException("Inbound cipher protection already installed") + val mac = inboundMac ?: throw SshException("Inbound MAC protection already installed") + packetIO.enableReceiveEncryption(cipher, mac, inboundEtm) + inboundCipher = null + inboundMac = null + } + + override fun destroy() { + outboundCipher?.destroy() + outboundCipher = null + outboundMac?.destroy() + outboundMac = null + inboundCipher?.destroy() + inboundCipher = null + inboundMac?.destroy() + inboundMac = null + } +} + /** * SSH connection handler that manages the protocol flow. * @@ -202,6 +280,8 @@ class SshConnection( private class HostKeyRejectedException(val key: PublicKey) : Exception("Host key rejected") + private class ProtocolViolationException(message: String, cause: Throwable? = null) : SshException(message, cause) + private val packetIO = PacketIO(transport) private val callbacks = object : SshClientCallbacks { @@ -219,7 +299,7 @@ class SshConnection( override suspend fun sendKexDhGexInit() = this@SshConnection.sendKexDhGexInit() override suspend fun sendNewKeys() = this@SshConnection.sendNewKeys() override fun receiveNewKeys() = this@SshConnection.receiveNewKeys() - override suspend fun activateEncryption() = this@SshConnection.activateEncryption() + override fun activateEncryption() = this@SshConnection.activateEncryption() override suspend fun sendClientExtInfo() = this@SshConnection.sendClientExtInfo() override suspend fun sendServiceRequest(service: String) = this@SshConnection.sendServiceRequest(service) override fun receiveServiceAccept(service: String) = this@SshConnection.receiveServiceAccept(service) @@ -232,8 +312,8 @@ class SshConnection( override fun receiveChannelOpenConfirmation(msg: SshMsgChannelOpenConfirmation) = this@SshConnection.receiveChannelOpenConfirmation(msg) override fun receiveChannelOpenFailure(msg: SshMsgChannelOpenFailure) = this@SshConnection.receiveChannelOpenFailure(msg) override suspend fun sendChannelRequest(recipientChannel: Int, requestType: String, wantReply: Boolean, message: SshMsgChannelRequest) = this@SshConnection.sendChannelRequest(recipientChannel, requestType, wantReply, message) - override fun receiveChannelSuccess() = this@SshConnection.receiveChannelSuccess() - override fun receiveChannelFailure() = this@SshConnection.receiveChannelFailure() + override fun receiveChannelSuccess(recipientChannel: Int) = this@SshConnection.receiveChannelSuccess(recipientChannel) + override fun receiveChannelFailure(recipientChannel: Int) = this@SshConnection.receiveChannelFailure(recipientChannel) override suspend fun receiveGlobalRequest(msg: SshMsgGlobalRequest) = this@SshConnection.receiveGlobalRequest(msg) override fun debug(msg: SshMsgDebug) = this@SshConnection.debug(msg) override fun ignore() = this@SshConnection.ignore() @@ -243,6 +323,7 @@ class SshConnection( } private val stateMachine = SshClientStateMachine(callbacks) + private val inboundPacketController = InboundPacketController() internal val connectionScope = CoroutineScope(SupervisorJob() + coroutineDispatcher) private val writeMutex = Mutex() @@ -308,6 +389,7 @@ class SshConnection( suspend fun set(value: CompletableDeferred) { withContext(stateMachineDispatcher) { + check(deferred == null) { "Operation already has a pending reply" } deferred = value } } @@ -316,6 +398,7 @@ class SshConnection( * Set the deferred directly. Must only be called from within [stateMachineDispatcher]. */ fun setDirect(value: CompletableDeferred) { + check(deferred == null) { "Operation already has a pending reply" } deferred = value } @@ -352,14 +435,14 @@ class SshConnection( // Pending async operations - completed by callbacks private val pendingAuth = PendingValue() - private val pendingChannelOpen = PendingValue() + private val pendingSessionChannelOpens = HashMap>() private class PendingChannelOpen( val deferred: CompletableDeferred, val maxPacketSize: Int, val initialWindowSize: Int, ) private val pendingChannelOpens = ConcurrentHashMap() - private val pendingChannelRequest = PendingValue() + private val pendingChannelRequests = HashMap>() private val pendingGlobalRequest = PendingValue() private val remoteForwarders = ConcurrentHashMap Unit>() @@ -382,15 +465,19 @@ class SshConnection( @Volatile private var pendingConnect: CompletableDeferred? = null private var dhGexGroup: SshMsgKexDhGexGroup? = null + private var pendingProtection: PendingProtection? = null + private var pendingSendCompressor: PacketCompressor? = null + private var pendingReceiveCompressor: PacketCompressor? = null + private var pendingCompressionImmediate = false - private suspend fun dispatchEvent(event: SshClientStateMachine.SshEvent) { - logger.debug("Dispatching event: $event") + private suspend fun dispatchCommand(name: String, command: suspend SshClientStateMachine.() -> Boolean) { + logger.debug("Dispatching command: $name") try { withContext(stateMachineDispatcher) { - stateMachine.processEvent(event) + check(stateMachine.command()) { "Command $name is not valid in the current SSH state" } } } catch (e: Exception) { - logger.error("State machine failed to process event: $event", e) + logger.error("State machine failed to process command: $name", e) throw e } } @@ -414,12 +501,12 @@ class SshConnection( pendingConnect = deferred return try { - dispatchEvent(SshClientStateMachine.SshEvent.Connect) + dispatchCommand("Connect") { connect() } // Version exchange — text protocol, handled inline packetIO.writeBanner(clientVersion) val banner = packetIO.readBanner() - dispatchEvent(SshClientStateMachine.SshEvent.ReceiveVersion(banner)) + dispatchCommand("ReceiveVersion") { receiveVersion(banner) } // Start packet loop — handles all binary SSH packets from here startPacketLoop() @@ -472,6 +559,7 @@ class SshConnection( } val deferred = CompletableDeferred() + dispatchCommand("BeginPasswordAuthentication") { beginAuthentication() } pendingAuth.set(deferred) writePacket( @@ -522,6 +610,7 @@ class SshConnection( val deferred = CompletableDeferred() val channel = Channel(Channel.UNLIMITED) + dispatchCommand("BeginKeyboardInteractiveAuthentication") { beginAuthentication() } pendingAuth.set(deferred) withContext(stateMachineDispatcher) { infoRequestChannel = channel @@ -600,6 +689,7 @@ class SshConnection( val sigAlgorithmName = if (privateKey.keyType == KEY_TYPE_SSH_RSA) { negotiateRsaAlgorithm() + ?: return PublicAuthResult.Failure(allowedAuthentications ?: setOf("publickey")) } else { privateKey.signatureAlgorithm } @@ -650,6 +740,7 @@ class SshConnection( } val deferred = CompletableDeferred() + dispatchCommand("BeginPublicKeyAuthentication") { beginAuthentication() } pendingAuth.set(deferred) writePacket( @@ -717,7 +808,10 @@ class SshConnection( return data.toByteArray() } - private fun negotiateRsaAlgorithm(): String = SignatureEntry.negotiateRsaAlgorithm(serverSigAlgs) + private fun negotiateRsaAlgorithm(): String? = SignatureEntry.negotiateRsaAlgorithm( + serverSigAlgs, + hostKeyAlgorithms.split(',').filter { it.isNotEmpty() }.toSet(), + ) /** * Authenticate using the strategy-based [AuthHandler] flow. @@ -816,6 +910,7 @@ class SshConnection( ): InternalAuthResult { val effectiveAlgorithmName = if (keyBlobAlgorithmName(key.publicKeyBlob) == KEY_TYPE_SSH_RSA) { negotiateRsaAlgorithm() + ?: return InternalAuthResult.Failure(allowedAuthentications ?: setOf("publickey"), partialSuccess = false) } else { key.algorithmName } @@ -841,7 +936,7 @@ class SshConnection( val hostBoundKeyBlob = serverHostKeyBlob.takeIf { serverAdvertisesHostBound } val effectiveAlgorithmName = if (keyBlobAlgorithmName(key.publicKeyBlob) == KEY_TYPE_SSH_RSA) { - negotiateRsaAlgorithm() + negotiateRsaAlgorithm() ?: return false } else { key.algorithmName } @@ -990,6 +1085,7 @@ class SshConnection( method: String, configure: SshMsgUserauthRequest.() -> Unit, ) { + dispatchCommand("BeginAuthenticationRequest") { beginAuthentication() } val req = SshMsgUserauthRequest().apply { setUserName(createAsciiString(username)) setServiceName(createAsciiString(SERVICE_SSH_CONNECTION)) @@ -1228,9 +1324,9 @@ class SshConnection( rekeyTimerJob?.cancel() rekeyTimerJob = connectionScope.launch { delay(rekeyIntervalMs) - if (!isRekeying && stateMachine.isInState("PostAuthenticated")) { + if (!isRekeying && stateMachine.isPostAuthenticated()) { connectionScope.launch { - dispatchEvent(SshClientStateMachine.SshEvent.RekeyStarted) + dispatchCommand("TimerRekey") { requestRekey() } } } } @@ -1300,9 +1396,19 @@ class SshConnection( val hash = exchangeHash ?: throw SshException("Exchange hash computation failed") - if (sessionId == null) { - sessionId = hash.copyOf() + val existingKey = serverHostKeyBlob + if (existingKey != null && !serverHostKey.contentEquals(existingKey)) { + logger.error("Host key changed during re-key — possible MITM attack") + throw SshException("Host key changed during re-key") + } + + val negotiatedAlg = negotiatedHostKeyAlgorithm + ?: throw SshException("No host key algorithm negotiated") + if (!SignatureVerifier.verify(serverHostKey, signature, hash, negotiatedAlg)) { + logger.error("Server signature verification failed") + throw SshException("Server signature verification failed") } + logger.info("Server signature verified") val keyType = try { val stream = ByteBufferKaitaiStream(serverHostKey) @@ -1324,20 +1430,10 @@ class SshConnection( } logger.info("Host key verified") - val existingKey = serverHostKeyBlob - if (existingKey != null && !serverHostKey.contentEquals(existingKey)) { - logger.error("Host key changed during re-key — possible MITM attack") - throw SshException("Host key changed during re-key") - } serverHostKeyBlob = serverHostKey - - val negotiatedAlg = negotiatedHostKeyAlgorithm - ?: throw SshException("No host key algorithm negotiated") - if (!SignatureVerifier.verify(serverHostKey, signature, hash, negotiatedAlg)) { - logger.error("Server signature verification failed") - throw SshException("Server signature verification failed") + if (sessionId == null) { + sessionId = hash.copyOf() } - logger.info("Server signature verified") clientKexInit?.fill(0) clientKexInit = null @@ -1358,11 +1454,22 @@ class SshConnection( private suspend fun sendNewKeys() { logger.info("Sending NEW_KEYS") - writeMutex.withLock { - packetIO.writePacket(SshEnums.MessageType.SSH_MSG_NEWKEYS.id().toInt()) - if (strictKexEnabled) { - packetIO.resetSendSequenceNumber() + prepareEncryption() + try { + writeMutex.withLock { + packetIO.writePacket(SshEnums.MessageType.SSH_MSG_NEWKEYS.id().toInt()) + val protection = pendingProtection + ?: throw SshException("No staged packet protection") + protection.installOutbound(packetIO) + packetIO.enableSendCompression(pendingSendCompressor, pendingCompressionImmediate) + pendingSendCompressor = null + if (strictKexEnabled) { + packetIO.resetSendSequenceNumber() + } } + } catch (e: Exception) { + discardPendingEncryption() + throw e } } @@ -1446,14 +1553,33 @@ class SshConnection( private fun receiveNewKeys() { logger.info("Received NEW_KEYS from server") + val protection = pendingProtection + ?: throw SshException("No staged packet protection") + protection.installInbound(packetIO) + packetIO.enableReceiveCompression(pendingReceiveCompressor, pendingCompressionImmediate) + pendingReceiveCompressor = null if (strictKexEnabled) { packetIO.resetReceiveSequenceNumber() } } - private suspend fun activateEncryption() { - logger.info("Activating encryption") + private fun activateEncryption() { + logger.info("Encryption active") + pendingProtection?.destroy() + pendingProtection = null + pendingSendCompressor = null + pendingReceiveCompressor = null + + kex?.zeroize() + kex = null + sharedSecret?.fill(0) + sharedSecret = null + exchangeHash?.fill(0) + exchangeHash = null + } + private fun prepareEncryption() { + discardPendingEncryption() val encC2S = negotiatedEncryptionC2S ?: throw SshException("No encryption algorithm negotiated for client-to-server") val encS2C = negotiatedEncryptionS2C @@ -1463,36 +1589,26 @@ class SshConnection( ?: throw SshException("Unknown cipher: $encC2S") if (cipherC2S.isAead) { - activateAeadEncryption(encC2S, encS2C) + pendingProtection = prepareAeadEncryption(encC2S, encS2C) } else { - activateNonAeadEncryption(encC2S, encS2C) + pendingProtection = prepareNonAeadEncryption(encC2S, encS2C) } - logger.info("Encryption active") - - kex?.zeroize() - kex = null - sharedSecret?.fill(0) - sharedSecret = null - exchangeHash?.fill(0) - exchangeHash = null - val compC2SName = negotiatedCompressionC2S val compS2CName = negotiatedCompressionS2C if (compC2SName != null && compS2CName != null) { val compC2S = CompressionEntry.fromSshName(compC2SName) val compS2C = CompressionEntry.fromSshName(compS2CName) if (compC2S != null && compS2C != null) { - val immediateActivation = !compC2S.delayedActivation && !compS2C.delayedActivation - writeMutex.withLock { - packetIO.enableCompression(compC2S.create(), compS2C.create(), immediateActivation) - } - logger.info("Compression configured: c2s=$compC2SName, s2c=$compS2CName, immediate=$immediateActivation") + pendingCompressionImmediate = isRekeying || (!compC2S.delayedActivation && !compS2C.delayedActivation) + pendingSendCompressor = compC2S.create() + pendingReceiveCompressor = compS2C.create() + logger.info("Compression staged: c2s=$compC2SName, s2c=$compS2CName, immediate=$pendingCompressionImmediate") } } } - private suspend fun activateAeadEncryption(encC2S: String, encS2C: String) { + private fun prepareAeadEncryption(encC2S: String, encS2C: String): PendingProtection { val entryC2S = CipherEntry.fromSshName(encC2S) ?: throw SshException("Unknown AEAD cipher: $encC2S") val entryS2C = CipherEntry.fromSshName(encS2C) @@ -1526,11 +1642,13 @@ class SshConnection( try { val c2sAead = (entryC2S.create(c2sKey, c2sIv, true) as EncryptionInstance.Aead).aead - val s2cAead = (entryS2C.create(s2cKey, s2cIv, false) as EncryptionInstance.Aead).aead - - writeMutex.withLock { - packetIO.enableAead(c2sAead, s2cAead) + val s2cAead = try { + (entryS2C.create(s2cKey, s2cIv, false) as EncryptionInstance.Aead).aead + } catch (e: Exception) { + c2sAead.destroy() + throw e } + return PendingAeadProtection(c2sAead, s2cAead) } finally { keys.zeroize() c2sKey.fill(0) @@ -1540,7 +1658,7 @@ class SshConnection( } } - private suspend fun activateNonAeadEncryption(encC2S: String, encS2C: String) { + private fun prepareNonAeadEncryption(encC2S: String, encS2C: String): PendingProtection { val cipherEntryC2S = CipherEntry.fromSshName(encC2S) ?: throw SshException("Unknown cipher: $encC2S") val cipherEntryS2C = CipherEntry.fromSshName(encS2C) @@ -1585,20 +1703,25 @@ class SshConnection( try { val clientToServerCipher = (cipherEntryC2S.create(c2sCipherKey, c2sIv, true) as EncryptionInstance.Cipher).cipher - val serverToClientCipher = (cipherEntryS2C.create(s2cCipherKey, s2cIv, false) as EncryptionInstance.Cipher).cipher - - val clientToServerMac = macEntryC2S.create(keys.integrityKeyClientToServer) - val serverToClientMac = macEntryS2C.create(keys.integrityKeyServerToClient) - - writeMutex.withLock { - packetIO.enableEncryption( + var serverToClientCipher: PacketCipher? = null + var clientToServerMac: PacketMac? = null + try { + serverToClientCipher = (cipherEntryS2C.create(s2cCipherKey, s2cIv, false) as EncryptionInstance.Cipher).cipher + clientToServerMac = macEntryC2S.create(keys.integrityKeyClientToServer) + val serverToClientMac = macEntryS2C.create(keys.integrityKeyServerToClient) + return PendingCipherProtection( clientToServerCipher, clientToServerMac, + macEntryC2S.isEtm, serverToClientCipher, serverToClientMac, - clientToServerEtm = macEntryC2S.isEtm, - serverToClientEtm = macEntryS2C.isEtm, + macEntryS2C.isEtm, ) + } catch (e: Exception) { + clientToServerCipher.destroy() + serverToClientCipher?.destroy() + clientToServerMac?.destroy() + throw e } } finally { keys.zeroize() @@ -1609,6 +1732,14 @@ class SshConnection( } } + private fun discardPendingEncryption() { + pendingProtection?.destroy() + pendingProtection = null + pendingSendCompressor = null + pendingReceiveCompressor = null + pendingCompressionImmediate = false + } + private suspend fun sendServiceRequest(service: String) { logger.info("Requesting service: $service") @@ -1704,7 +1835,12 @@ class SshConnection( val channelType = msg.channelType().value() val senderChannel = msg.senderChannel().toInt() val initialWindow = msg.initialWindowSize() - val maxPacketSize = msg.maximumPacketSize().toInt() + val maxPacketSize = boundRemotePacketSize(msg.maximumPacketSize()) + if (maxPacketSize == null) { + logger.warn("Rejecting channel with invalid maximum packet size: ${msg.maximumPacketSize()}") + rejectChannelOpen(senderChannel, channelType) + return + } logger.info("Received CHANNEL_OPEN: type=$channelType, sender=$senderChannel") @@ -1714,16 +1850,27 @@ class SshConnection( rejectChannelOpen(senderChannel, channelType) return } + val verifiedSessionId = sessionId + val verifiedServerHostKey = serverHostKeyBlob + if (verifiedSessionId == null || verifiedSessionId.isEmpty() || + verifiedSessionId.size > AGENT_MAX_SESSION_ID_LENGTH || + verifiedServerHostKey == null || verifiedServerHostKey.isEmpty() + ) { + logger.error("Rejecting agent channel without a verified SSH session binding") + rejectChannelOpen(senderChannel, channelType) + return + } val localChannelNumber = allocateChannelNumber() val sessionInfo = AgentSessionInfo( - sessionId = sessionId ?: ByteArray(0), - serverHostKey = serverHostKeyBlob ?: ByteArray(0), + sessionId = verifiedSessionId.copyOf(), + serverHostKey = verifiedServerHostKey.copyOf(), ) val handler = AgentProtocolHandler(agentProvider!!, sessionInfo) val agentChannel = AgentChannel( handler, this, + connectionScope, localChannelNumber, senderChannel, maxPacketSize, @@ -1888,12 +2035,18 @@ class SshConnection( private fun receiveChannelOpenConfirmation(msg: SshMsgChannelOpenConfirmation) { logger.info("Channel open confirmed") - pendingChannelOpen.complete(msg) + val recipientChannel = msg.recipientChannel().toInt() + val pending = pendingSessionChannelOpens.remove(recipientChannel) + ?: throw ProtocolViolationException("Channel confirmation with no pending open for channel $recipientChannel") + pending.complete(msg) } private fun receiveChannelOpenFailure(msg: SshMsgChannelOpenFailure) { logger.error("Channel open failed: ${msg.reasonCode()}") - pendingChannelOpen.complete(null) + val recipientChannel = msg.recipientChannel().toInt() + val pending = pendingSessionChannelOpens.remove(recipientChannel) + ?: throw ProtocolViolationException("Channel failure with no pending open for channel $recipientChannel") + pending.complete(null) } private suspend fun sendChannelRequest(recipientChannel: Int, requestType: String, wantReply: Boolean, message: SshMsgChannelRequest) { @@ -1904,14 +2057,18 @@ class SshConnection( ) } - private fun receiveChannelSuccess() { + private fun receiveChannelSuccess(recipientChannel: Int) { logger.debug("Channel request succeeded") - pendingChannelRequest.complete(true) + val pending = pendingChannelRequests.remove(recipientChannel) + ?: throw ProtocolViolationException("Channel success with no pending request for channel $recipientChannel") + pending.complete(true) } - private fun receiveChannelFailure() { + private fun receiveChannelFailure(recipientChannel: Int) { logger.warn("Channel request failed") - pendingChannelRequest.complete(false) + val pending = pendingChannelRequests.remove(recipientChannel) + ?: throw ProtocolViolationException("Channel failure with no pending request for channel $recipientChannel") + pending.complete(false) } private suspend fun receiveGlobalRequest(msg: SshMsgGlobalRequest) { @@ -2084,349 +2241,400 @@ class SshConnection( * Read and dispatch the next packet through the state machine. * This is the central packet processing loop that converts packets to events. */ - private suspend fun processNextPacket() { - val packet = packetIO.readPacket() - val msgType = packet.messageType() - logger.debug("Received packet: $msgType") + private inner class InboundPacketController { + private fun requireAccepted(accepted: Boolean, messageType: Any) { + if (!accepted) { + throw ProtocolViolationException("Unexpected SSH packet $messageType in the current protocol state") + } + } - withContext(stateMachineDispatcher) { - when (msgType) { - SshEnums.MessageType.SSH_MSG_KEXINIT -> { - val kexInitMsgType = msgType.id().toByte() - serverKexInit = byteArrayOf(kexInitMsgType) + packet._raw_body() - val kexInit = packet.body() as SshMsgKexinit - if (stateMachine.isInState("PostAuthenticated")) { - stateMachine.processEvent(SshClientStateMachine.SshEvent.RekeyStarted) - } - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveKexInit(kexInit)) - } + suspend fun processNextPacket() { + val packet = packetIO.readPacket() + val msgType = packet.messageType() + logger.debug("Received packet: $msgType") - SshEnums.MessageType.SSH_MSG_NEWKEYS -> { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveNewKeys) - } + withContext(stateMachineDispatcher) { + when (msgType) { + SshEnums.MessageType.SSH_MSG_KEXINIT -> { + val kexInitMsgType = msgType.id().toByte() + serverKexInit = byteArrayOf(kexInitMsgType) + packet._raw_body() + val kexInit = packet.body() as SshMsgKexinit + if (stateMachine.isPostAuthenticated()) { + requireAccepted(stateMachine.requestRekey(), msgType) + } + requireAccepted(stateMachine.receiveKexInit(kexInit), msgType) + } - SshEnums.MessageType.SSH_MSG_SERVICE_ACCEPT -> { - val msg = parseBody(packet) - stateMachine.processEvent( - SshClientStateMachine.SshEvent.ReceiveServiceAccept(msg.serviceName().value()), - ) - } + SshEnums.MessageType.SSH_MSG_NEWKEYS -> { + requireAccepted(stateMachine.receiveNewKeys(), msgType) + } - SshEnums.MessageType.SSH_MSG_IGNORE -> { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveIgnore) - } + SshEnums.MessageType.SSH_MSG_SERVICE_ACCEPT -> { + val msg = parseBody(packet) + requireAccepted(stateMachine.receiveServiceAccept(msg.serviceName().value()), msgType) + } - SshEnums.MessageType.SSH_MSG_DEBUG -> { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveDebug(packet.body() as SshMsgDebug)) - } + SshEnums.MessageType.SSH_MSG_IGNORE -> { + if (strictKexEnabled && stateMachine.isKexInProgress()) { + throw ProtocolViolationException("SSH_MSG_IGNORE is forbidden during strict key exchange") + } + requireAccepted(stateMachine.receiveIgnore(), msgType) + } - SshEnums.MessageType.SSH_MSG_GLOBAL_REQUEST -> { - try { - val rawBody = packet._raw_body() - val stream = ByteBufferKaitaiStream(rawBody) - val globalRequestMsg = SshMsgGlobalRequest(stream) - globalRequestMsg._read() - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveGlobalRequest(globalRequestMsg)) - } catch (e: Exception) { - logger.error("Failed to parse SSH_MSG_GLOBAL_REQUEST", e) + SshEnums.MessageType.SSH_MSG_DEBUG -> { + if (strictKexEnabled && stateMachine.isKexInProgress()) { + throw ProtocolViolationException("SSH_MSG_DEBUG is forbidden during strict key exchange") + } + requireAccepted(stateMachine.receiveDebug(packet.body() as SshMsgDebug), msgType) } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_OPEN -> { - handleIncomingChannelOpen(packet) - } + SshEnums.MessageType.SSH_MSG_GLOBAL_REQUEST -> { + try { + val rawBody = packet._raw_body() + val stream = ByteBufferKaitaiStream(rawBody) + val globalRequestMsg = SshMsgGlobalRequest(stream) + globalRequestMsg._read() + requireAccepted(stateMachine.receiveGlobalRequest(globalRequestMsg), msgType) + } catch (e: ProtocolViolationException) { + throw e + } catch (e: Exception) { + throw ProtocolViolationException("Malformed SSH_MSG_GLOBAL_REQUEST", e) + } + } - SshEnums.MessageType.SSH_MSG_CHANNEL_OPEN_CONFIRMATION -> { - val confirmationMsg = parseBody(packet) - val recipientChannel = confirmationMsg.recipientChannel().toInt() - val pending = pendingChannelOpens.remove(recipientChannel) - if (pending != null) { - val remoteChannelNumber = confirmationMsg.senderChannel().toInt() - val remoteWindow = confirmationMsg.initialWindowSize() - logger.info("Direct-tcpip channel opened: local=$recipientChannel, remote=$remoteChannelNumber") - val channel = ForwardingChannel( - this@SshConnection, - recipientChannel, - remoteChannelNumber, - pending.maxPacketSize, - remoteWindowSizeInitial = remoteWindow, - initialWindowSize = pending.initialWindowSize, - ) - registerForwardingChannel(channel) - pending.deferred.complete(channel) - } else { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveChannelOpenConfirmation(confirmationMsg)) + SshEnums.MessageType.SSH_MSG_CHANNEL_OPEN -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + handleIncomingChannelOpen(packet) } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_OPEN_FAILURE -> { - val failureMsg = parseBody(packet) - val recipientChannel = failureMsg.recipientChannel().toInt() - val pending = pendingChannelOpens.remove(recipientChannel) - if (pending != null) { - pending.deferred.complete(null) - } else { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveChannelOpenFailure(failureMsg)) + SshEnums.MessageType.SSH_MSG_CHANNEL_OPEN_CONFIRMATION -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val confirmationMsg = parseBody(packet) + val recipientChannel = confirmationMsg.recipientChannel().toInt() + val pending = pendingChannelOpens.remove(recipientChannel) + if (pending != null) { + val remoteChannelNumber = confirmationMsg.senderChannel().toInt() + val remoteWindow = confirmationMsg.initialWindowSize() + val remoteMaxPacketSize = boundRemotePacketSize(confirmationMsg.maximumPacketSize()) + if (remoteMaxPacketSize == null) { + logger.warn("Rejecting channel confirmation with invalid maximum packet size: ${confirmationMsg.maximumPacketSize()}") + pending.deferred.complete(null) + sendChannelClose(remoteChannelNumber) + } else { + logger.info("Direct-tcpip channel opened: local=$recipientChannel, remote=$remoteChannelNumber") + val channel = ForwardingChannel( + this@SshConnection, + connectionScope, + recipientChannel, + remoteChannelNumber, + remoteMaxPacketSize, + remoteWindowSizeInitial = remoteWindow, + initialWindowSize = pending.initialWindowSize, + ) + registerForwardingChannel(channel) + pending.deferred.complete(channel) + } + } else { + requireAccepted(stateMachine.receiveChannelOpenConfirmation(confirmationMsg), msgType) + } } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_DATA -> { - val msg = parseBody(packet) - val recipientChannel = msg.recipientChannel().toInt() - val channel = channelsByRemote[recipientChannel] - val agentChannel = agentChannelsByRemote[recipientChannel] - val fwdChannel = forwardingChannelsByRemote[recipientChannel] - when { - channel != null -> channel.onData(msg.data().data()) - agentChannel != null -> agentChannel.handleData(msg.data().data()) - fwdChannel != null -> fwdChannel.onData(msg.data().data()) - else -> logger.warn("Data for unknown channel $recipientChannel") + SshEnums.MessageType.SSH_MSG_CHANNEL_OPEN_FAILURE -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val failureMsg = parseBody(packet) + val recipientChannel = failureMsg.recipientChannel().toInt() + val pending = pendingChannelOpens.remove(recipientChannel) + if (pending != null) { + pending.deferred.complete(null) + } else { + requireAccepted(stateMachine.receiveChannelOpenFailure(failureMsg), msgType) + } } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_EXTENDED_DATA -> { - val msg = parseBody(packet) - val channel = channelsByRemote[msg.recipientChannel().toInt()] - if (channel != null) { - channel.onExtendedData(msg.dataTypeCode().toInt(), msg.data().data()) - } else { - logger.warn("Extended data for unknown channel ${msg.recipientChannel()}") + SshEnums.MessageType.SSH_MSG_CHANNEL_DATA -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val msg = parseBody(packet) + val recipientChannel = msg.recipientChannel().toInt() + val channel = channelsByRemote[recipientChannel] + val agentChannel = agentChannelsByRemote[recipientChannel] + val fwdChannel = forwardingChannelsByRemote[recipientChannel] + when { + channel != null -> channel.onData(msg.data().data()) + agentChannel != null -> agentChannel.handleData(msg.data().data()) + fwdChannel != null -> fwdChannel.onData(msg.data().data()) + else -> throw ProtocolViolationException("Channel data for unknown channel $recipientChannel") + } } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_WINDOW_ADJUST -> { - val msg = parseBody(packet) - val recipientChannel = msg.recipientChannel().toInt() - val channel = channelsByRemote[recipientChannel] - val agentChannel = agentChannelsByRemote[recipientChannel] - val fwdChannel = forwardingChannelsByRemote[recipientChannel] - when { - channel != null -> channel.onWindowAdjust(msg.bytesToAdd()) - agentChannel != null -> agentChannel.onWindowAdjust(msg.bytesToAdd()) - fwdChannel != null -> fwdChannel.onWindowAdjust(msg.bytesToAdd()) - else -> logger.warn("Window adjust for unknown channel $recipientChannel") + SshEnums.MessageType.SSH_MSG_CHANNEL_EXTENDED_DATA -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val msg = parseBody(packet) + val channel = channelsByRemote[msg.recipientChannel().toInt()] + if (channel != null) { + channel.onExtendedData(msg.dataTypeCode().toInt(), msg.data().data()) + } else { + throw ProtocolViolationException("Extended data for unknown channel ${msg.recipientChannel()}") + } } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_EOF -> { - val msg = parseBody(packet) - val recipientChannel = msg.recipientChannel().toInt() - val channel = channelsByRemote[recipientChannel] - val agentChannel = agentChannelsByRemote[recipientChannel] - val fwdChannel = forwardingChannelsByRemote[recipientChannel] - when { - channel != null -> channel.onEof() - agentChannel != null -> agentChannel.onEof() - fwdChannel != null -> fwdChannel.onEof() - else -> logger.warn("EOF for unknown channel $recipientChannel") + SshEnums.MessageType.SSH_MSG_CHANNEL_WINDOW_ADJUST -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val msg = parseBody(packet) + val recipientChannel = msg.recipientChannel().toInt() + val channel = channelsByRemote[recipientChannel] + val agentChannel = agentChannelsByRemote[recipientChannel] + val fwdChannel = forwardingChannelsByRemote[recipientChannel] + when { + channel != null -> channel.onWindowAdjust(msg.bytesToAdd()) + agentChannel != null -> agentChannel.onWindowAdjust(msg.bytesToAdd()) + fwdChannel != null -> fwdChannel.onWindowAdjust(msg.bytesToAdd()) + else -> throw ProtocolViolationException("Window adjust for unknown channel $recipientChannel") + } } - } - SshEnums.MessageType.SSH_MSG_CHANNEL_CLOSE -> { - val msg = parseBody(packet) - val recipientChannel = msg.recipientChannel().toInt() - logger.debug("Received CHANNEL_CLOSE for remote channel $recipientChannel (channels: ${channels.size}, forwardingChannels: ${forwardingChannels.size})") - val channel = channelsByRemote[recipientChannel] - val agentChannel = agentChannelsByRemote[recipientChannel] - val fwdChannel = forwardingChannelsByRemote[recipientChannel] - when { - channel != null -> channel.onClose() - - agentChannel != null -> agentChannel.onClose() - - fwdChannel != null -> { - fwdChannel.onClose() - unregisterForwardingChannel(fwdChannel) + SshEnums.MessageType.SSH_MSG_CHANNEL_EOF -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val msg = parseBody(packet) + val recipientChannel = msg.recipientChannel().toInt() + val channel = channelsByRemote[recipientChannel] + val agentChannel = agentChannelsByRemote[recipientChannel] + val fwdChannel = forwardingChannelsByRemote[recipientChannel] + when { + channel != null -> channel.onEof() + agentChannel != null -> agentChannel.onEof() + fwdChannel != null -> fwdChannel.onEof() + else -> throw ProtocolViolationException("EOF for unknown channel $recipientChannel") } + } - else -> logger.warn("Close for unknown channel $recipientChannel") + SshEnums.MessageType.SSH_MSG_CHANNEL_CLOSE -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val msg = parseBody(packet) + val recipientChannel = msg.recipientChannel().toInt() + logger.debug("Received CHANNEL_CLOSE for remote channel $recipientChannel (channels: ${channels.size}, forwardingChannels: ${forwardingChannels.size})") + val channel = channelsByRemote[recipientChannel] + val agentChannel = agentChannelsByRemote[recipientChannel] + val fwdChannel = forwardingChannelsByRemote[recipientChannel] + when { + channel != null -> channel.onClose() + + agentChannel != null -> agentChannel.onClose() + + fwdChannel != null -> { + fwdChannel.onClose() + unregisterForwardingChannel(fwdChannel) + } + + else -> throw ProtocolViolationException("Close for unknown channel $recipientChannel") + } + checkAllChannelsClosed() } - checkAllChannelsClosed() - } - SshEnums.MessageType.SSH_MSG_CHANNEL_REQUEST -> { - val rawBody = packet._raw_body() - val stream = ByteBufferKaitaiStream(rawBody) - val msg = SshMsgChannelRequest(stream) - msg._read() - logger.debug("Received channel request: ${msg.requestType().value()} (want_reply=${msg.wantReply() != 0})") - } + SshEnums.MessageType.SSH_MSG_CHANNEL_REQUEST -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val rawBody = packet._raw_body() + val stream = ByteBufferKaitaiStream(rawBody) + val msg = SshMsgChannelRequest(stream) + msg._read() + val recipientChannel = msg.recipientChannel().toInt() + if ( + !channelsByRemote.containsKey(recipientChannel) && + !agentChannelsByRemote.containsKey(recipientChannel) && + !forwardingChannelsByRemote.containsKey(recipientChannel) + ) { + throw ProtocolViolationException("Channel request for unknown channel $recipientChannel") + } + logger.debug("Received channel request: ${msg.requestType().value()} (want_reply=${msg.wantReply() != 0})") + } - SshEnums.MessageType.SSH_MSG_CHANNEL_SUCCESS -> { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveChannelSuccess) - } + SshEnums.MessageType.SSH_MSG_CHANNEL_SUCCESS -> { + val msg = parseBody(packet) + requireAccepted(stateMachine.receiveChannelSuccess(msg.recipientChannel().toInt()), msgType) + } - SshEnums.MessageType.SSH_MSG_CHANNEL_FAILURE -> { - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveChannelFailure) - } + SshEnums.MessageType.SSH_MSG_CHANNEL_FAILURE -> { + val msg = parseBody(packet) + requireAccepted(stateMachine.receiveChannelFailure(msg.recipientChannel().toInt()), msgType) + } - SshEnums.MessageType.SSH_MSG_USERAUTH_SUCCESS -> { - stateMachine.processEvent(SshClientStateMachine.SshEvent.AuthenticationSuccess) - val ch = authResultChannel - if (ch != null) { - ch.trySend(InternalAuthResult.Success) + SshEnums.MessageType.SSH_MSG_USERAUTH_SUCCESS -> { + requireAccepted(stateMachine.authenticationSuccess(), msgType) + val ch = authResultChannel + if (ch != null) { + ch.trySend(InternalAuthResult.Success) + } } - } - SshEnums.MessageType.SSH_MSG_USERAUTH_FAILURE -> { - val ch = authResultChannel - if (ch != null) { + SshEnums.MessageType.SSH_MSG_USERAUTH_FAILURE -> { + val ch = authResultChannel val msg = parseBody(packet) - val methods = msg.validAuthentications().entries().data().toSet() - val partial = msg.partialSuccess() != 0 - ch.trySend(InternalAuthResult.Failure(methods, partial)) - } else { - stateMachine.processEvent(SshClientStateMachine.SshEvent.AuthenticationFailure) + requireAccepted(stateMachine.authenticationFailure(), msgType) + if (ch != null) { + val methods = msg.validAuthentications().entries().data().toSet() + val partial = msg.partialSuccess() != 0 + ch.trySend(InternalAuthResult.Failure(methods, partial)) + } } - } - SshEnums.MessageType.SSH_MSG_USERAUTH_BANNER -> { - val msg = parseBody(packet) - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveUserauthBanner(msg)) - } + SshEnums.MessageType.SSH_MSG_USERAUTH_BANNER -> { + val msg = parseBody(packet) + requireAccepted(stateMachine.receiveUserauthBanner(msg), msgType) + } - SshEnums.MessageType.SSH_MSG_USERAUTH_METHOD_SPECIFIC_60 -> { - val ch = authResultChannel - if (ch != null && currentAuthMethod is AuthMethod.PublicKey) { - val msg = parseBody(packet) - ch.trySend( - InternalAuthResult.PkOk( - msg.publicKeyAlgorithmName().value(), - msg.publicKeyBlob().data(), - ), - ) - } else if (ch != null && currentAuthMethod is AuthMethod.KeyboardInteractive) { - val msg = parseBody(packet) - val name = String(msg.name().data(), Charsets.UTF_8) - val instruction = String(msg.instruction().data(), Charsets.UTF_8) - val prompts = msg.prompts().map { prompt -> - KeyboardInteractiveCallback.Prompt( - text = String(prompt.prompt().data(), Charsets.UTF_8), - echo = prompt.echo() != 0, + SshEnums.MessageType.SSH_MSG_USERAUTH_METHOD_SPECIFIC_60 -> { + requireAccepted(stateMachine.authorizeAuthenticationPacket(), msgType) + val ch = authResultChannel + if (ch != null && currentAuthMethod is AuthMethod.PublicKey) { + val msg = parseBody(packet) + ch.trySend( + InternalAuthResult.PkOk( + msg.publicKeyAlgorithmName().value(), + msg.publicKeyBlob().data(), + ), ) + } else if (ch != null && currentAuthMethod is AuthMethod.KeyboardInteractive) { + val msg = parseBody(packet) + val name = String(msg.name().data(), Charsets.UTF_8) + val instruction = String(msg.instruction().data(), Charsets.UTF_8) + val prompts = msg.prompts().map { prompt -> + KeyboardInteractiveCallback.Prompt( + text = String(prompt.prompt().data(), Charsets.UTF_8), + echo = prompt.echo() != 0, + ) + } + ch.trySend(InternalAuthResult.InfoRequest(name, instruction, prompts)) + } else { + val msg = parseBody(packet) + requireAccepted(stateMachine.receiveUserauthInfoRequest(msg), msgType) } - ch.trySend(InternalAuthResult.InfoRequest(name, instruction, prompts)) - } else { - val msg = parseBody(packet) - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveUserauthInfoRequest(msg)) } - } - SshEnums.MessageType.SSH_MSG_REQUEST_SUCCESS -> { - val rawBody = packet._raw_body() - if (!pendingGlobalRequest.complete(rawBody)) { - logger.warn("Received SSH_MSG_REQUEST_SUCCESS with no pending global request") + SshEnums.MessageType.SSH_MSG_REQUEST_SUCCESS -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + val rawBody = packet._raw_body() + if (!pendingGlobalRequest.complete(rawBody)) { + throw ProtocolViolationException("Received SSH_MSG_REQUEST_SUCCESS with no pending global request") + } } - } - SshEnums.MessageType.SSH_MSG_REQUEST_FAILURE -> { - if (!pendingGlobalRequest.complete(null)) { - logger.warn("Received SSH_MSG_REQUEST_FAILURE with no pending global request") + SshEnums.MessageType.SSH_MSG_REQUEST_FAILURE -> { + requireAccepted(stateMachine.authorizeAuthenticatedPacket(), msgType) + if (!pendingGlobalRequest.complete(null)) { + throw ProtocolViolationException("Received SSH_MSG_REQUEST_FAILURE with no pending global request") + } } - } - - SshEnums.MessageType.SSH_MSG_DISCONNECT -> { - val msg = parseBody(packet) - logger.info("Received SSH_MSG_DISCONNECT from server: reason=${msg.reasonCode()}, description=${msg.description().value()}") - stateMachine.processEvent(SshClientStateMachine.SshEvent.Disconnect) - } - SshEnums.MessageType.SSH_MSG_EXT_INFO -> { - processServerExtInfo(parseBody(packet)) - } + SshEnums.MessageType.SSH_MSG_DISCONNECT -> { + val msg = parseBody(packet) + logger.info("Received SSH_MSG_DISCONNECT from server: reason=${msg.reasonCode()}, description=${msg.description().value()}") + requireAccepted(stateMachine.disconnect(), msgType) + } - SshEnums.MessageType.SSH_MSG_PING -> { - val msg = parseBody(packet) - val pongSend: suspend () -> Unit = { - val pong = SshMsgPong() - pong.setData(createByteString(msg.data().data())) - pong._check() - writePacket(SshEnums.MessageType.SSH_MSG_PONG.id().toInt(), pong.toByteArray()) + SshEnums.MessageType.SSH_MSG_EXT_INFO -> { + requireAccepted(stateMachine.authorizeExtInfo(), msgType) + processServerExtInfo(parseBody(packet)) } - val sendNow = withContext(stateMachineDispatcher) { - if (isRekeying) { - pendingPingQueue.addLast(pongSend) - false - } else { - true + + SshEnums.MessageType.SSH_MSG_PING -> { + requireAccepted(stateMachine.authorizeConnectionPacket(), msgType) + val msg = parseBody(packet) + val pongSend: suspend () -> Unit = { + val pong = SshMsgPong() + pong.setData(createByteString(msg.data().data())) + pong._check() + writePacket(SshEnums.MessageType.SSH_MSG_PONG.id().toInt(), pong.toByteArray()) + } + val sendNow = withContext(stateMachineDispatcher) { + if (isRekeying) { + pendingPingQueue.addLast(pongSend) + false + } else { + true + } + } + if (sendNow) { + pongSend() } } - if (sendNow) { - pongSend() - } - } - SshEnums.MessageType.SSH_MSG_PONG -> { - val msg = parseBody(packet) - val seqBytes = msg.data().data() - if (seqBytes.size == 8) { - val seq = ByteBuffer.wrap(seqBytes).getLong() - withContext(stateMachineDispatcher) { - val pending = pendingPings.remove(seq) - if (pending != null) { - val sentTimeNs = pending.sentTimeNs - if (sentTimeNs != null) { - pending.deferred.complete(PingResult.Success(System.nanoTime() - sentTimeNs)) + SshEnums.MessageType.SSH_MSG_PONG -> { + requireAccepted(stateMachine.authorizeConnectionPacket(), msgType) + val msg = parseBody(packet) + val seqBytes = msg.data().data() + if (seqBytes.size == 8) { + val seq = ByteBuffer.wrap(seqBytes).getLong() + withContext(stateMachineDispatcher) { + val pending = pendingPings.remove(seq) + if (pending != null) { + val sentTimeNs = pending.sentTimeNs + if (sentTimeNs != null) { + pending.deferred.complete(PingResult.Success(System.nanoTime() - sentTimeNs)) + } else { + pendingPings[seq] = pending + logger.warn("Received SSH_MSG_PONG before ping send timestamp was recorded: $seq") + } } else { - pendingPings[seq] = pending - logger.warn("Received SSH_MSG_PONG before ping send timestamp was recorded: $seq") + logger.debug("Ignoring SSH_MSG_PONG with no pending latency probe: $seq") } - } else { - logger.warn("Received SSH_MSG_PONG with unknown sequence: $seq") } + } else { + logger.trace("Ignoring opaque SSH_MSG_PONG payload (${seqBytes.size} bytes)") } - } else { - logger.warn("Received SSH_MSG_PONG with unexpected data length: ${seqBytes.size}") } - } - else -> { - // KEX-specific messages 30-49 are not in MessageType enum. - // Disambiguate by negotiated KEX type since ECDH reply, DH reply, - // and DH-GEX group all share message ID 31. - val msgId = msgType.id().toInt() - val rawBody = byteArrayOf(msgType.id().toByte()) + packet._raw_body() - val kexEntry = negotiatedKex?.let { KexEntry.fromSshName(it) } - when { - kexEntry?.type == KexType.ECDH && - msgId == SshEnums.KexEcdh.SSH_MSG_KEX_ECDH_REPLY.id().toInt() -> { - val stream = ByteBufferKaitaiStream(rawBody) - val ecdhPayload = KexEcdhPayload(stream) - ecdhPayload._read() - val ecdhReply = ecdhPayload.body() as SshMsgKexEcdhReply - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveKex.EcdhReply(ecdhReply)) - } + else -> { + // KEX-specific messages 30-49 are not in MessageType enum. + // Disambiguate by negotiated KEX type since ECDH reply, DH reply, + // and DH-GEX group all share message ID 31. + val msgId = msgType.id().toInt() + val rawBody = byteArrayOf(msgType.id().toByte()) + packet._raw_body() + val kexEntry = negotiatedKex?.let { KexEntry.fromSshName(it) } + when { + kexEntry?.type == KexType.ECDH && + msgId == SshEnums.KexEcdh.SSH_MSG_KEX_ECDH_REPLY.id().toInt() -> { + val stream = ByteBufferKaitaiStream(rawBody) + val ecdhPayload = KexEcdhPayload(stream) + ecdhPayload._read() + val ecdhReply = ecdhPayload.body() as SshMsgKexEcdhReply + requireAccepted(stateMachine.receiveKexEcdhReply(ecdhReply), msgType) + } - kexEntry?.type == KexType.DH && - msgId == SshEnums.KexDh.SSH_MSG_KEXDH_REPLY.id().toInt() -> { - val stream = ByteBufferKaitaiStream(rawBody) - val kexdhPayload = KexdhPayload(stream) - kexdhPayload._read() - val dhReply = kexdhPayload.body() as SshMsgKexdhReply - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveKex.DhReply(dhReply)) - } + kexEntry?.type == KexType.DH && + msgId == SshEnums.KexDh.SSH_MSG_KEXDH_REPLY.id().toInt() -> { + val stream = ByteBufferKaitaiStream(rawBody) + val kexdhPayload = KexdhPayload(stream) + kexdhPayload._read() + val dhReply = kexdhPayload.body() as SshMsgKexdhReply + requireAccepted(stateMachine.receiveKexDhReply(dhReply), msgType) + } - kexEntry?.type == KexType.DH_GEX && - msgId == SshEnums.KexDhGex.SSH_MSG_KEX_DH_GEX_GROUP.id().toInt() -> { - val stream = ByteBufferKaitaiStream(rawBody) - val payload = KexDhGexPayload(stream) - payload._read() - val group = payload.body() as SshMsgKexDhGexGroup - dhGexGroup = group - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveKex.DhGexGroup(group)) - } + kexEntry?.type == KexType.DH_GEX && + msgId == SshEnums.KexDhGex.SSH_MSG_KEX_DH_GEX_GROUP.id().toInt() -> { + val stream = ByteBufferKaitaiStream(rawBody) + val payload = KexDhGexPayload(stream) + payload._read() + val group = payload.body() as SshMsgKexDhGexGroup + dhGexGroup = group + requireAccepted(stateMachine.receiveKexDhGexGroup(group), msgType) + } - kexEntry?.type == KexType.DH_GEX && - msgId == SshEnums.KexDhGex.SSH_MSG_KEX_DH_GEX_REPLY.id().toInt() -> { - val stream = ByteBufferKaitaiStream(rawBody) - val replyPayload = KexDhGexPayload(stream) - replyPayload._read() - val reply = replyPayload.body() as SshMsgKexDhGexReply - stateMachine.processEvent(SshClientStateMachine.SshEvent.ReceiveKex.DhGexReply(reply)) - } + kexEntry?.type == KexType.DH_GEX && + msgId == SshEnums.KexDhGex.SSH_MSG_KEX_DH_GEX_REPLY.id().toInt() -> { + val stream = ByteBufferKaitaiStream(rawBody) + val replyPayload = KexDhGexPayload(stream) + replyPayload._read() + val reply = replyPayload.body() as SshMsgKexDhGexReply + requireAccepted(stateMachine.receiveKexDhGexReply(reply), msgType) + } - else -> { - logger.warn("Unhandled message type: ${packet.messageType()}") + else -> { + if (msgId in 30..49) { + throw ProtocolViolationException( + "Unexpected key-exchange packet ${packet.messageType()} for negotiated algorithm $negotiatedKex", + ) + } + logger.warn("Unhandled message type: ${packet.messageType()}") + } } } } @@ -2528,6 +2736,19 @@ class SshConnection( transport.close() } + private suspend fun sendProtocolError(description: String) { + val msg = SshMsgDisconnect().apply { + setReasonCode(SshEnums.DisconnectReason.SSH_DISCONNECT_PROTOCOL_ERROR) + setDescription(createUtf8String(description)) + setLanguage(createAsciiString("")) + _check() + } + writePacket( + SshEnums.MessageType.SSH_MSG_DISCONNECT.id().toInt(), + msg.toByteArray(), + ) + } + private fun startPacketLoop() { if (packetLoopJob != null) return packetLoopJob = connectionScope.launch { @@ -2535,18 +2756,25 @@ class SshConnection( try { while (isActive) { logger.debug("Packet loop: waiting for next packet") - processNextPacket() - if (!isRekeying && stateMachine.isInState("PostAuthenticated") && ( + inboundPacketController.processNextPacket() + if (!isRekeying && stateMachine.isPostAuthenticated() && ( packetIO.bytesSentOnWire >= rekeyBytesLimit || packetIO.bytesReceivedOnWire >= rekeyBytesLimit ) ) { - dispatchEvent(SshClientStateMachine.SshEvent.RekeyStarted) + dispatchCommand("ByteLimitRekey") { requestRekey() } } } } catch (_: CancellationException) { logger.debug("Packet loop cancelled") } catch (e: Exception) { + if (e is ProtocolViolationException) { + try { + sendProtocolError(e.message ?: "Unexpected SSH packet") + } catch (sendFailure: Exception) { + logger.debug("Failed to send protocol-error disconnect", sendFailure) + } + } val packetFailure = e.kaitaiParseFailureOrNull() val loopFailure = when { e is SshException -> e @@ -2572,8 +2800,10 @@ class SshConnection( val loopError = loopException ?: Exception("Packet loop terminated") withContext(stateMachineDispatcher) { pendingAuth.completeExceptionally(loopError) - pendingChannelOpen.completeExceptionally(loopError) - pendingChannelRequest.completeExceptionally(loopError) + pendingSessionChannelOpens.values.forEach { it.completeExceptionally(loopError) } + pendingSessionChannelOpens.clear() + pendingChannelRequests.values.forEach { it.completeExceptionally(loopError) } + pendingChannelRequests.clear() pendingGlobalRequest.completeExceptionally(loopError) for ((_, pending) in pendingChannelOpens) { pending.deferred.completeExceptionally(loopError) @@ -2615,13 +2845,13 @@ class SshConnection( val deferred = CompletableDeferred() withContext(stateMachineDispatcher) { - pendingChannelOpen.setDirect(deferred) - stateMachine.processEvent( - SshClientStateMachine.SshEvent.OpenChannel( - channelType = "session", - localChannelNumber = localChannelNumber, - initialWindowSize = initialWindowSize, - maxPacketSize = maxPacketSize, + check(pendingSessionChannelOpens.put(localChannelNumber, deferred) == null) + check( + stateMachine.openChannel( + "session", + localChannelNumber, + initialWindowSize, + maxPacketSize, ), ) } @@ -2629,11 +2859,19 @@ class SshConnection( val confirmationMsg = try { deferred.await() } finally { - pendingChannelOpen.clearIfSame(deferred) + withContext(stateMachineDispatcher) { + pendingSessionChannelOpens.remove(localChannelNumber, deferred) + } } ?: return null val remoteChannelNumber = confirmationMsg.senderChannel().toInt() val remoteWindow = confirmationMsg.initialWindowSize() + val remoteMaxPacketSize = boundRemotePacketSize(confirmationMsg.maximumPacketSize()) + if (remoteMaxPacketSize == null) { + logger.warn("Rejecting session channel confirmation with invalid maximum packet size: ${confirmationMsg.maximumPacketSize()}") + sendChannelClose(remoteChannelNumber) + return null + } logger.info("Channel opened: local=$localChannelNumber, remote=$remoteChannelNumber, remoteWindow=$remoteWindow") val channel = SessionChannel( @@ -2641,7 +2879,7 @@ class SshConnection( connectionScope, localChannelNumber, remoteChannelNumber, - maxPacketSize, + remoteMaxPacketSize, remoteWindowSizeInitial = remoteWindow, initialWindowSize = initialWindowSize, canSendChaff = serverSupportsPing, @@ -2681,14 +2919,18 @@ class SshConnection( val deferred = if (wantReply) CompletableDeferred() else null withContext(stateMachineDispatcher) { if (deferred != null) { - pendingChannelRequest.setDirect(deferred) + val localChannelNumber = channels.values.singleOrNull { it.remoteChannelNumber == recipientChannel }?.localChannelNumber + ?: throw IllegalStateException("No session channel has remote channel number $recipientChannel") + check(pendingChannelRequests.put(localChannelNumber, deferred) == null) { + "Channel $localChannelNumber already has a pending request" + } } - stateMachine.processEvent( - SshClientStateMachine.SshEvent.SendChannelRequest( - recipientChannel = recipientChannel, - requestType = requestType, - wantReply = wantReply, - message = msg, + check( + stateMachine.sendChannelRequest( + recipientChannel, + requestType, + wantReply, + msg, ), ) } @@ -2700,7 +2942,9 @@ class SshConnection( return try { deferred.await() } finally { - pendingChannelRequest.clearIfSame(deferred) + withContext(stateMachineDispatcher) { + pendingChannelRequests.entries.removeIf { it.value === deferred } + } } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImpl.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImpl.kt index 6d267e84..426bfead 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImpl.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImpl.kt @@ -319,9 +319,16 @@ internal class SftpClientImpl private constructor( payload: ByteArray, map: (SftpRawPacket) -> SftpResult, ): SftpResult = when (val result = dispatcher.request(type, payload)) { - is SftpResult.Success -> map(result.value) + is SftpResult.Success -> try { + map(result.value) + } catch (e: SftpDecodeException) { + SftpResult.ProtocolError(e.message ?: "Malformed SFTP response") + } + is SftpResult.ServerError -> result + is SftpResult.ProtocolError -> result + is SftpResult.IoError -> result } @@ -435,12 +442,7 @@ internal class SftpClientImpl private constructor( } /** Read a length-prefixed string/byte array from a ByteBuffer. */ - private fun extractString(buf: ByteBuffer): ByteArray { - val len = buf.int - val data = ByteArray(len) - buf.get(data) - return data - } + private fun extractString(buf: ByteBuffer): ByteArray = SftpDecoder.readString(buf, "SFTP string") /** Decode a STATUS response to get the status code. */ private fun decodeStatus(payload: ByteArray): SftpStatusCode { @@ -452,7 +454,7 @@ internal class SftpClientImpl private constructor( /** Decode a STATUS response into an [SftpResult.ServerError]. */ private fun decodeStatusError(payload: ByteArray): SftpResult.ServerError { val buf = ByteBuffer.wrap(payload) - val code = if (buf.remaining() >= 4) buf.int else 4 + val code = if (buf.remaining() >= 4) SftpDecoder.readInt(buf, "status code") else 4 val statusCode = SftpStatusCode.fromCode(code) val message = if (buf.remaining() >= 4) { val msgBytes = extractString(buf) @@ -466,8 +468,8 @@ internal class SftpClientImpl private constructor( /** Decode a NAME response (used by readdir, realpath, readlink). */ private fun decodeName(payload: ByteArray): List { val buf = ByteBuffer.wrap(payload) - val count = buf.int - val entries = mutableListOf() + val count = SftpDecoder.readCount(buf, "NAME entry count", minimumElementSize = 12) + val entries = ArrayList(count) repeat(count) { val filename = String(extractString(buf), StandardCharsets.UTF_8) val longname = String(extractString(buf), StandardCharsets.UTF_8) diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpDecoder.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpDecoder.kt new file mode 100644 index 00000000..23ca6f97 --- /dev/null +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpDecoder.kt @@ -0,0 +1,61 @@ +/* + * ConnectBot SSH Library + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.connectbot.sshlib.client.sftp + +import java.nio.ByteBuffer + +internal class SftpDecodeException(message: String) : Exception(message) + +/** Checked readers for server-controlled SFTP response fields. */ +internal object SftpDecoder { + private const val MAX_FIELD_SIZE = 256 * 1024 + + fun readInt(buf: ByteBuffer, field: String): Int { + requireRemaining(buf, Int.SIZE_BYTES, field) + return buf.int + } + + fun readLong(buf: ByteBuffer, field: String): Long { + requireRemaining(buf, Long.SIZE_BYTES, field) + return buf.long + } + + fun readString(buf: ByteBuffer, field: String): ByteArray { + val length = readInt(buf, "$field length") + if (length < 0) throw SftpDecodeException("Negative $field length: $length") + if (length > MAX_FIELD_SIZE) throw SftpDecodeException("$field length exceeds limit: $length") + requireRemaining(buf, length, field) + return ByteArray(length).also(buf::get) + } + + fun readCount(buf: ByteBuffer, field: String, minimumElementSize: Int): Int { + val count = readInt(buf, field) + if (count < 0) throw SftpDecodeException("Negative $field: $count") + val maximumCount = buf.remaining() / minimumElementSize + if (count > maximumCount) { + throw SftpDecodeException("$field $count exceeds remaining packet capacity $maximumCount") + } + return count + } + + private fun requireRemaining(buf: ByteBuffer, length: Int, field: String) { + if (length > buf.remaining()) { + throw SftpDecodeException("$field length $length exceeds remaining response data ${buf.remaining()}") + } + } +} diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpFileAttributes.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpFileAttributes.kt index 352902fc..68676fa1 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpFileAttributes.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/sftp/SftpFileAttributes.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -44,22 +44,20 @@ internal object SftpFileAttributes { * Parse ATTRS from a ByteBuffer at its current position. */ fun decode(buf: ByteBuffer): SftpAttributes { - val flags = buf.int - val size = if (flags and SSH_FILEXFER_ATTR_SIZE != 0) buf.long else null - val uid = if (flags and SSH_FILEXFER_ATTR_UIDGID != 0) buf.int else null - val gid = if (flags and SSH_FILEXFER_ATTR_UIDGID != 0) buf.int else null - val permissions = if (flags and SSH_FILEXFER_ATTR_PERMISSIONS != 0) buf.int else null - val atime = if (flags and SSH_FILEXFER_ATTR_ACMODTIME != 0) buf.int else null - val mtime = if (flags and SSH_FILEXFER_ATTR_ACMODTIME != 0) buf.int else null + val flags = SftpDecoder.readInt(buf, "attribute flags") + val size = if (flags and SSH_FILEXFER_ATTR_SIZE != 0) SftpDecoder.readLong(buf, "file size") else null + val uid = if (flags and SSH_FILEXFER_ATTR_UIDGID != 0) SftpDecoder.readInt(buf, "uid") else null + val gid = if (flags and SSH_FILEXFER_ATTR_UIDGID != 0) SftpDecoder.readInt(buf, "gid") else null + val permissions = if (flags and SSH_FILEXFER_ATTR_PERMISSIONS != 0) SftpDecoder.readInt(buf, "permissions") else null + val atime = if (flags and SSH_FILEXFER_ATTR_ACMODTIME != 0) SftpDecoder.readInt(buf, "access time") else null + val mtime = if (flags and SSH_FILEXFER_ATTR_ACMODTIME != 0) SftpDecoder.readInt(buf, "modification time") else null // Skip extended attributes if present if (flags and SSH_FILEXFER_ATTR_EXTENDED != 0) { - val extCount = buf.int + val extCount = SftpDecoder.readCount(buf, "extended attribute count", minimumElementSize = 8) repeat(extCount) { - val typeLen = buf.int - buf.position(buf.position() + typeLen) // skip type string - val dataLen = buf.int - buf.position(buf.position() + dataLen) // skip data string + SftpDecoder.readString(buf, "extended attribute type") + SftpDecoder.readString(buf, "extended attribute data") } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Algorithms.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Algorithms.kt index d7c8c0b7..5986ea56 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Algorithms.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Algorithms.kt @@ -103,7 +103,13 @@ internal enum class CipherEntry( internal fun create(key: ByteArray, iv: ByteArray, forEncryption: Boolean): EncryptionInstance = factory(key, iv, forEncryption) companion object { - val defaults: List = entries.toList() + val defaults: List = listOf( + CHACHA20_POLY1305, + AES128_GCM, + AES256_GCM, + AES128_CTR, + AES256_CTR, + ) val defaultString: String = defaults.joinToString(",") { it.sshName } @@ -165,7 +171,7 @@ internal enum class MacEntry( internal fun create(key: ByteArray): PacketMac = factory(key) companion object { - val defaults: List = entries.toList() + val defaults: List = listOf(HMAC_SHA2_256_ETM, HMAC_SHA2_512_ETM) val defaultString: String = defaults.joinToString(",") { it.sshName } @@ -271,7 +277,9 @@ internal enum class KexEntry( internal fun create(): KexAlgorithm = factory() companion object { - val defaults: List = entries.filter { it != DH_GROUP1_SHA1 } + val defaults: List = entries.filter { + it !in setOf(DH_GROUP14_SHA1, DH_GROUP_EXCHANGE_SHA1, DH_GROUP1_SHA1) + } val defaultString: String = defaults.joinToString(",") { it.sshName } + ",kex-strict-c-v00@openssh.com,ext-info-c" @@ -295,7 +303,7 @@ internal enum class SignatureEntry( ; companion object { - val defaults: List = entries.toList() + val defaults: List = entries.filter { it != SSH_RSA } val defaultString: String = defaults.joinToString(",") { it.sshName } @@ -304,13 +312,16 @@ internal enum class SignatureEntry( private val rsaPreferenceOrder = listOf("rsa-sha2-512", "rsa-sha2-256", KEY_TYPE_SSH_RSA) /** - * Picks the best RSA signing algorithm given the server's advertised list. - * Returns "ssh-rsa" if [serverSigAlgs] is null (server didn't send the extension) - * or if no supported RSA algorithms were advertised. + * Picks the best RSA signing algorithm allowed by the client's configured + * algorithm wishlist and the server's advertised list. When RFC 8308 + * server-sig-algs is absent, only an explicitly allowed ssh-rsa is usable, + * matching OpenSSH's base-key fallback behavior. */ - fun negotiateRsaAlgorithm(serverSigAlgs: Set?): String { - if (serverSigAlgs == null) return KEY_TYPE_SSH_RSA - return rsaPreferenceOrder.firstOrNull { it in serverSigAlgs } ?: KEY_TYPE_SSH_RSA + fun negotiateRsaAlgorithm(serverSigAlgs: Set?, allowedAlgorithms: Set): String? { + if (serverSigAlgs == null) { + return KEY_TYPE_SSH_RSA.takeIf { it in allowedAlgorithms } + } + return rsaPreferenceOrder.firstOrNull { it in allowedAlgorithms && it in serverSigAlgs } } } } 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 b48d2fb1..7f140fde 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt @@ -33,6 +33,20 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String) : 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(GROUP_CACHE_LIMIT, 0.75f, true) { + override fun removeEldestEntry(eldest: MutableMap.MutableEntry?): 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 min = 2048 @@ -59,6 +73,9 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String) : 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") + } this.p = p this.g = g } @@ -77,17 +94,21 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String) : val x = privateKey ?: throw SshException("Client keys not generated; call generateClientKeys() first") val p = this.p ?: throw SshException(ERROR_GROUP_NOT_SET) - val f = BigInteger(1, serverPublicKey) - - if (f <= BigInteger.ONE || f >= p - BigInteger.ONE) { - throw SshException("Invalid server public key") + try { + val f = BigInteger(1, serverPublicKey) + + if (f <= BigInteger.ONE || f >= p - BigInteger.ONE) { + throw SshException("Invalid server public key") + } + + val secret = f.modPow(x, p) + if (secret <= BigInteger.ONE || secret >= p - BigInteger.ONE) { + throw SshException("Invalid DH shared secret") + } + return encodeMpint(secret.toByteArray()) + } finally { + privateKey = null } - - val sharedSecret = encodeMpint(f.modPow(x, p).toByteArray()) - - privateKey = null - - return sharedSecret } /** diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyReader.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyReader.kt index 08c2d6f3..917eae72 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyReader.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyReader.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,7 +40,7 @@ import java.security.spec.RSAPublicKeySpec internal object PemKeyReader { - private enum class PemType { RSA, EC, PKCS8 } + private enum class PemType { RSA, EC, PKCS8, ENCRYPTED_PKCS8 } private class PemStructure( val type: PemType, @@ -51,7 +51,8 @@ internal object PemKeyReader { fun isEncrypted(text: String): Boolean { val ps = parsePem(text) - return ps.procType != null && ps.procType!!.size == 2 && ps.procType!![1] == "ENCRYPTED" + return ps.type == PemType.ENCRYPTED_PKCS8 || + (ps.procType != null && ps.procType!!.size == 2 && ps.procType!![1] == "ENCRYPTED") } fun read(text: String, passphrase: String?): SshPrivateKey { @@ -66,8 +67,15 @@ internal object PemKeyReader { return when (ps.type) { PemType.RSA -> readPkcs1Rsa(ps.data) + PemType.EC -> readSec1Ec(ps.data) + PemType.PKCS8 -> readPkcs8(ps.data) + + PemType.ENCRYPTED_PKCS8 -> { + val password = passphrase ?: throw SshException("PKCS#8 key is encrypted, but no passphrase was specified") + readPkcs8(Pkcs8Encryption.decrypt(ps.data, password)) + } } } @@ -94,6 +102,11 @@ internal object PemKeyReader { type = PemType.PKCS8 endMarker = "-----END PRIVATE KEY-----" } + + line.startsWith("-----BEGIN ENCRYPTED PRIVATE KEY-----") -> { + type = PemType.ENCRYPTED_PKCS8 + endMarker = "-----END ENCRYPTED PRIVATE KEY-----" + } } i++ if (type != null) break diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyWriter.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyWriter.kt index a3d0c7f0..2c50b5c9 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyWriter.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PemKeyWriter.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -102,7 +102,12 @@ internal object PemKeyWriter { private fun writeEd25519(keyPair: KeyPair, password: String?): String { val der = keyPair.private.encoded - return wrapPem("PRIVATE KEY", der, password) + return if (password == null) { + wrapPem("PRIVATE KEY", der, null) + } else { + val encrypted = Pkcs8Encryption.encrypt(der, password) + wrapPem("ENCRYPTED PRIVATE KEY", encrypted, null) + } } private fun wrapPem(label: String, data: ByteArray, password: String?): String { diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Pkcs8Encryption.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Pkcs8Encryption.kt new file mode 100644 index 00000000..05d21eea --- /dev/null +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Pkcs8Encryption.kt @@ -0,0 +1,165 @@ +/* + * ConnectBot SSH Library + * Copyright 2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.connectbot.sshlib.crypto + +import org.connectbot.sshlib.SshException +import java.math.BigInteger +import java.security.GeneralSecurityException +import java.security.SecureRandom +import javax.crypto.Cipher +import javax.crypto.SecretKeyFactory +import javax.crypto.spec.IvParameterSpec +import javax.crypto.spec.PBEKeySpec +import javax.crypto.spec.SecretKeySpec + +/** PBES2 encrypted PKCS#8 using PBKDF2-HMAC-SHA256 and AES-256-CBC. */ +internal object Pkcs8Encryption { + private const val ITERATIONS = 210_000 + private const val MAX_ITERATIONS = 10_000_000 + private const val KEY_BITS = 256 + private const val KEY_BYTES = KEY_BITS / Byte.SIZE_BITS + private const val SALT_BYTES = 16 + private const val IV_BYTES = 16 + + private val oidPbes2 = oid(0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x05, 0x0D) + private val oidPbkdf2 = oid(0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x05, 0x0C) + private val oidHmacSha256 = oid(0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x02, 0x09) + private val oidAes256Cbc = oid(0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x01, 0x2A) + + fun encrypt(plaintext: ByteArray, password: String, random: SecureRandom = SecureRandom()): ByteArray { + val salt = ByteArray(SALT_BYTES).also(random::nextBytes) + val iv = ByteArray(IV_BYTES).also(random::nextBytes) + val ciphertext = crypt(Cipher.ENCRYPT_MODE, plaintext, password, salt, ITERATIONS, iv) + + return encodeDer { + sequence { + sequence { + objectIdentifier(oidPbes2) + sequence { + sequence { + objectIdentifier(oidPbkdf2) + sequence { + octetString(salt) + integer(BigInteger.valueOf(ITERATIONS.toLong())) + integer(BigInteger.valueOf(KEY_BYTES.toLong())) + sequence { + objectIdentifier(oidHmacSha256) + nullValue() + } + } + } + sequence { + objectIdentifier(oidAes256Cbc) + octetString(iv) + } + } + } + octetString(ciphertext) + } + } + } + + fun decrypt(encoded: ByteArray, password: String): ByteArray { + val reader = DerReader(encoded) + val parsed = reader.readSequence { outer -> + val parameters = outer.readSequence { algorithm -> + requireOid(algorithm.readObjectIdentifier(), oidPbes2, "PBES2") + algorithm.readSequence { pbes2 -> + val kdf = pbes2.readSequence { keyDerivation -> + requireOid(keyDerivation.readObjectIdentifier(), oidPbkdf2, "PBKDF2") + keyDerivation.readSequence { pbkdf2 -> + val salt = pbkdf2.readOctetString() + val iterations = positiveInt(pbkdf2.readInteger(), "PBKDF2 iteration count") + if (iterations > MAX_ITERATIONS) { + throw SshException("PBKDF2 iteration count exceeds supported maximum") + } + val keyLength = if (pbkdf2.peekTag() == 0x02) { + positiveInt(pbkdf2.readInteger(), "PBKDF2 key length") + } else { + KEY_BYTES + } + if (keyLength != KEY_BYTES) { + throw SshException("Unsupported PBKDF2 key length: $keyLength") + } + pbkdf2.readSequence { prf -> + requireOid(prf.readObjectIdentifier(), oidHmacSha256, "HMAC-SHA256") + if (prf.hasRemaining()) prf.skipTag() + } + KdfParameters(salt, iterations) + } + } + val iv = pbes2.readSequence { encryptionScheme -> + requireOid(encryptionScheme.readObjectIdentifier(), oidAes256Cbc, "AES-256-CBC") + encryptionScheme.readOctetString() + } + if (iv.size != IV_BYTES) throw SshException("Invalid AES-256-CBC IV length: ${iv.size}") + EncryptionParameters(kdf.salt, kdf.iterations, iv) + } + } + ParsedEncryptedKey(parameters, outer.readOctetString()) + } + reader.ensureFullyConsumed() + + return crypt( + Cipher.DECRYPT_MODE, + parsed.ciphertext, + password, + parsed.parameters.salt, + parsed.parameters.iterations, + parsed.parameters.iv, + ) + } + + private fun crypt( + mode: Int, + input: ByteArray, + password: String, + salt: ByteArray, + iterations: Int, + iv: ByteArray, + ): ByteArray { + val spec = PBEKeySpec(password.toCharArray(), salt, iterations, KEY_BITS) + var keyBytes: ByteArray? = null + try { + keyBytes = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256").generateSecret(spec).encoded + val cipher = Cipher.getInstance("AES/CBC/PKCS5Padding") + cipher.init(mode, SecretKeySpec(keyBytes, "AES"), IvParameterSpec(iv)) + return cipher.doFinal(input) + } catch (e: GeneralSecurityException) { + throw SshException("Unable to process encrypted PKCS#8 private key", e) + } finally { + spec.clearPassword() + keyBytes?.fill(0) + } + } + + private fun positiveInt(value: BigInteger, name: String): Int { + if (value.signum() <= 0 || value.bitLength() > 31) throw SshException("Invalid $name") + return value.toInt() + } + + private fun requireOid(actual: ByteArray, expected: ByteArray, name: String) { + if (!actual.contentEquals(expected)) throw SshException("Unsupported encrypted PKCS#8 algorithm; expected $name") + } + + private fun oid(vararg bytes: Int): ByteArray = ByteArray(bytes.size) { bytes[it].toByte() } + + private data class KdfParameters(val salt: ByteArray, val iterations: Int) + private data class EncryptionParameters(val salt: ByteArray, val iterations: Int, val iv: ByteArray) + private data class ParsedEncryptedKey(val parameters: EncryptionParameters, val ciphertext: ByteArray) +} diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PrivateKeyReader.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PrivateKeyReader.kt index 9153480e..355d3e54 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PrivateKeyReader.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PrivateKeyReader.kt @@ -37,7 +37,8 @@ internal object PrivateKeyReader { trimmed.startsWith("-----BEGIN RSA PRIVATE KEY-----") || trimmed.startsWith("-----BEGIN EC PRIVATE KEY-----") || - trimmed.startsWith("-----BEGIN PRIVATE KEY-----") -> { + trimmed.startsWith("-----BEGIN PRIVATE KEY-----") || + trimmed.startsWith("-----BEGIN ENCRYPTED PRIVATE KEY-----") -> { PemKeyReader.read(trimmed, passphrase) } @@ -63,6 +64,8 @@ internal object PrivateKeyReader { trimmed.startsWith("-----BEGIN PRIVATE KEY-----") -> false + trimmed.startsWith("-----BEGIN ENCRYPTED PRIVATE KEY-----") -> true + // PKCS#8 unencrypted else -> throw SshException("Unrecognized private key format") } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/SignatureVerifier.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/SignatureVerifier.kt index aaf73087..ec3a8b63 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/SignatureVerifier.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/SignatureVerifier.kt @@ -18,6 +18,7 @@ package org.connectbot.sshlib.crypto import io.kaitai.struct.ByteBufferKaitaiStream +import org.connectbot.sshlib.protocol.EcdsaPublicKeyBlob import org.connectbot.sshlib.protocol.SshPublicKey import org.connectbot.sshlib.protocol.SshSignature @@ -42,6 +43,15 @@ internal object SignatureVerifier { val pubKey = SshPublicKey(ByteBufferKaitaiStream(serverHostKey)) pubKey._read() + if (!isAlgorithmCompatibleWithKeyType(expectedAlgorithm, pubKey.algorithmName())) { + return false + } + if (expectedAlgorithm.startsWith("ecdsa-sha2-") && + (pubKey.keyBlob() as EcdsaPublicKeyBlob).curveIdentifier().value() != expectedAlgorithm.removePrefix("ecdsa-sha2-") + ) { + return false + } + val algorithm = SignatureEntry.fromSshName(sig.algorithmName())?.algorithm ?: return false @@ -72,8 +82,8 @@ internal object SignatureVerifier { return algorithm.verify(pubKey, sig, data) } - private fun isAlgorithmCompatibleWithKeyType(sigAlgorithm: String, keyType: String): Boolean = when (keyType) { - "ssh-rsa" -> sigAlgorithm in setOf("ssh-rsa", "rsa-sha2-256", "rsa-sha2-512") - else -> sigAlgorithm == keyType + private fun isAlgorithmCompatibleWithKeyType(algorithm: String, keyType: String): Boolean = when (algorithm) { + "ssh-rsa", "rsa-sha2-256", "rsa-sha2-512" -> keyType == "ssh-rsa" + else -> algorithm == keyType } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachine.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachine.kt index 12492224..aebe2b9d 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachine.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachine.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ import ru.nsk.kstatemachine.state.onEntry import ru.nsk.kstatemachine.state.onExit import ru.nsk.kstatemachine.state.state import ru.nsk.kstatemachine.state.transition +import ru.nsk.kstatemachine.statemachine.ProcessingResult import ru.nsk.kstatemachine.statemachine.StateMachine import ru.nsk.kstatemachine.statemachine.createStdLibStateMachine import ru.nsk.kstatemachine.transition.onTriggered @@ -56,7 +57,7 @@ import ru.nsk.kstatemachine.transition.onTriggered internal class SshClientStateMachine( private val callbacks: SshClientCallbacks, ) { - sealed class SshEvent : Event { + private sealed class SshEvent : Event { object Connect : SshEvent() data class ReceiveVersion(val banner: IdBanner) : SshEvent() data class ReceiveKexInit(val msg: SshMsgKexinit) : SshEvent() @@ -68,6 +69,7 @@ internal class SshClientStateMachine( } object ReceiveNewKeys : SshEvent() data class ReceiveServiceAccept(val service: String) : SshEvent() + object BeginAuthentication : SshEvent() object AuthenticationSuccess : SshEvent() object AuthenticationFailure : SshEvent() data class ReceiveUserauthInfoRequest(val msg: SshMsgUserauthInfoRequest) : SshEvent() @@ -86,16 +88,20 @@ internal class SshClientStateMachine( val wantReply: Boolean, val message: SshMsgChannelRequest, ) : SshEvent() - object ReceiveChannelSuccess : SshEvent() - object ReceiveChannelFailure : SshEvent() + data class ReceiveChannelSuccess(val recipientChannel: Int) : SshEvent() + data class ReceiveChannelFailure(val recipientChannel: Int) : SshEvent() data class ReceiveGlobalRequest(val msg: SshMsgGlobalRequest) : SshEvent() data class ReceiveDebug(val msg: SshMsgDebug) : SshEvent() object ReceiveIgnore : SshEvent() + object AuthorizeAuthenticationPacket : SshEvent() + object AuthorizeAuthenticatedPacket : SshEvent() + object AuthorizeConnectionPacket : SshEvent() + object AuthorizeExtInfo : SshEvent() object Disconnect : SshEvent() object RekeyStarted : SshEvent() } - val stateMachine: StateMachine = createStdLibStateMachine { + private val stateMachine: StateMachine = createStdLibStateMachine { val waitVersion = state("WaitVersion") val waitKexInit = state("WaitKexInit") val waitKex = state("WaitKex") @@ -107,17 +113,53 @@ internal class SshClientStateMachine( lateinit var postAuthHistory: HistoryState val postAuthenticated = state("PostAuthenticated") { - val authenticated = initialState("Authenticated") - val waitChannelOpenConfirmation = state("WaitChannelOpenConfirmation") - val channelOpen = state("ChannelOpen") - val waitChannelRequestReply = state("WaitChannelRequestReply") + val authenticationReady = initialState("AuthenticationReady") + val authenticating = state("Authenticating") + val authenticated = state("Authenticated") + + authenticationReady { + onEntry { callbacks.onStateEnter("AuthenticationReady") } + onExit { callbacks.onStateExit("AuthenticationReady") } + + transition { + targetState = authenticating + } + transition { + onTriggered { callbacks.receiveUserauthBanner(it.event.msg) } + } + } + + authenticating { + onEntry { callbacks.onStateEnter("Authenticating") } + onExit { callbacks.onStateExit("Authenticating") } + + transition {} + transition { + targetState = authenticated + onTriggered { callbacks.authenticationSuccess() } + } + transition { + targetState = authenticationReady + onTriggered { callbacks.authenticationFailure() } + } + transition { + onTriggered { callbacks.receiveUserauthInfoRequest(it.event.msg) } + } + transition { + onTriggered { callbacks.receiveUserauthBanner(it.event.msg) } + } + transition {} + } authenticated { onEntry { callbacks.onStateEnter("Authenticated") } onExit { callbacks.onStateExit("Authenticated") } + transition {} + transition { + onTriggered { callbacks.receiveGlobalRequest(it.event.msg) } + } transition { - targetState = waitChannelOpenConfirmation onTriggered { callbacks.sendChannelOpen( it.event.channelType, @@ -127,28 +169,13 @@ internal class SshClientStateMachine( ) } } - } - - waitChannelOpenConfirmation { - onEntry { callbacks.onStateEnter("WaitChannelOpenConfirmation") } - onExit { callbacks.onStateExit("WaitChannelOpenConfirmation") } - transition { - targetState = channelOpen onTriggered { callbacks.receiveChannelOpenConfirmation(it.event.msg) } } transition { - targetState = authenticated onTriggered { callbacks.receiveChannelOpenFailure(it.event.msg) } } - } - - channelOpen { - onEntry { callbacks.onStateEnter("ChannelOpen") } - onExit { callbacks.onStateExit("ChannelOpen") } - transition { - targetState = waitChannelRequestReply onTriggered { callbacks.sendChannelRequest( it.event.recipientChannel, @@ -158,26 +185,18 @@ internal class SshClientStateMachine( ) } } - } - - waitChannelRequestReply { - onEntry { callbacks.onStateEnter("WaitChannelRequestReply") } - onExit { callbacks.onStateExit("WaitChannelRequestReply") } - transition { - targetState = channelOpen - onTriggered { callbacks.receiveChannelSuccess() } + onTriggered { callbacks.receiveChannelSuccess(it.event.recipientChannel) } } transition { - targetState = channelOpen - onTriggered { callbacks.receiveChannelFailure() } + onTriggered { callbacks.receiveChannelFailure(it.event.recipientChannel) } } } postAuthHistory = historyState( name = "PostAuthHistory", - defaultState = authenticated, - historyType = HistoryType.SHALLOW, + defaultState = authenticationReady, + historyType = HistoryType.DEEP, ) transition { @@ -187,6 +206,8 @@ internal class SshClientStateMachine( callbacks.sendKexInit() } } + transition {} + transition {} } initialState("Unconnected") { @@ -298,43 +319,91 @@ internal class SshClientStateMachine( callbacks.startAuthentication() } } + transition {} } - transition { - onTriggered { callbacks.authenticationSuccess() } - } - transition { - onTriggered { callbacks.authenticationFailure() } - } - transition { - onTriggered { callbacks.receiveUserauthInfoRequest(it.event.msg) } - } - transition { - onTriggered { callbacks.receiveUserauthBanner(it.event.msg) } - } transition { onTriggered { callbacks.debug(it.event.msg) } } transition { onTriggered { callbacks.ignore() } } - transition { - onTriggered { callbacks.receiveGlobalRequest(it.event.msg) } - } transition { targetState = disconnected onTriggered { callbacks.disconnect() } } } - suspend fun processEvent(event: SshEvent) { - stateMachine.processEvent(event) - } + suspend fun connect(): Boolean = process(SshEvent.Connect) - val currentState: String - get() = stateMachine.activeStates().firstOrNull()?.name ?: "Unknown" + suspend fun receiveVersion(banner: IdBanner): Boolean = process(SshEvent.ReceiveVersion(banner)) + + suspend fun receiveKexInit(msg: SshMsgKexinit): Boolean = process(SshEvent.ReceiveKexInit(msg)) + + suspend fun receiveKexDhReply(msg: SshMsgKexdhReply): Boolean = process(SshEvent.ReceiveKex.DhReply(msg)) + + suspend fun receiveKexEcdhReply(msg: SshMsgKexEcdhReply): Boolean = process(SshEvent.ReceiveKex.EcdhReply(msg)) + + suspend fun receiveKexDhGexGroup(msg: SshMsgKexDhGexGroup): Boolean = process(SshEvent.ReceiveKex.DhGexGroup(msg)) + + suspend fun receiveKexDhGexReply(msg: SshMsgKexDhGexReply): Boolean = process(SshEvent.ReceiveKex.DhGexReply(msg)) + + suspend fun receiveNewKeys(): Boolean = process(SshEvent.ReceiveNewKeys) + + suspend fun receiveServiceAccept(service: String): Boolean = service == "ssh-userauth" && process(SshEvent.ReceiveServiceAccept(service)) + + suspend fun beginAuthentication(): Boolean = process(SshEvent.BeginAuthentication) + + suspend fun authenticationSuccess(): Boolean = process(SshEvent.AuthenticationSuccess) + + suspend fun authenticationFailure(): Boolean = process(SshEvent.AuthenticationFailure) + + suspend fun receiveUserauthInfoRequest(msg: SshMsgUserauthInfoRequest): Boolean = process(SshEvent.ReceiveUserauthInfoRequest(msg)) + + suspend fun receiveUserauthBanner(msg: SshMsgUserauthBanner): Boolean = process(SshEvent.ReceiveUserauthBanner(msg)) + + suspend fun openChannel(channelType: String, localChannelNumber: Int, initialWindowSize: Int, maxPacketSize: Int): Boolean = process(SshEvent.OpenChannel(channelType, localChannelNumber, initialWindowSize, maxPacketSize)) + + suspend fun receiveChannelOpenConfirmation(msg: SshMsgChannelOpenConfirmation): Boolean = process(SshEvent.ReceiveChannelOpenConfirmation(msg)) + + suspend fun receiveChannelOpenFailure(msg: SshMsgChannelOpenFailure): Boolean = process(SshEvent.ReceiveChannelOpenFailure(msg)) + + suspend fun sendChannelRequest( + recipientChannel: Int, + requestType: String, + wantReply: Boolean, + message: SshMsgChannelRequest, + ): Boolean = process(SshEvent.SendChannelRequest(recipientChannel, requestType, wantReply, message)) + + suspend fun receiveChannelSuccess(recipientChannel: Int): Boolean = process(SshEvent.ReceiveChannelSuccess(recipientChannel)) + + suspend fun receiveChannelFailure(recipientChannel: Int): Boolean = process(SshEvent.ReceiveChannelFailure(recipientChannel)) + + suspend fun receiveGlobalRequest(msg: SshMsgGlobalRequest): Boolean = process(SshEvent.ReceiveGlobalRequest(msg)) + + suspend fun receiveDebug(msg: SshMsgDebug): Boolean = process(SshEvent.ReceiveDebug(msg)) + + suspend fun receiveIgnore(): Boolean = process(SshEvent.ReceiveIgnore) + + suspend fun authorizeAuthenticationPacket(): Boolean = process(SshEvent.AuthorizeAuthenticationPacket) + + suspend fun authorizeAuthenticatedPacket(): Boolean = process(SshEvent.AuthorizeAuthenticatedPacket) + + suspend fun authorizeConnectionPacket(): Boolean = process(SshEvent.AuthorizeConnectionPacket) + + suspend fun authorizeExtInfo(): Boolean = process(SshEvent.AuthorizeExtInfo) + + suspend fun disconnect(): Boolean = process(SshEvent.Disconnect) + + suspend fun requestRekey(): Boolean = process(SshEvent.RekeyStarted) + + fun isPostAuthenticated(): Boolean = stateMachine.activeStates().any { it.name == "PostAuthenticated" } + + fun isKexInProgress(): Boolean = stateMachine.activeStates().any { + it.name == "WaitKexInit" || it.name == "WaitKex" || it.name == "WaitKexDhGexInit" || it.name == "WaitNewKeys" + } - fun isInState(stateName: String): Boolean = stateMachine.activeStates().any { it.name == stateName } + private suspend fun process(event: SshEvent): Boolean = stateMachine.processEvent(event) == ProcessingResult.PROCESSED } internal interface SshClientCallbacks { @@ -352,7 +421,7 @@ internal interface SshClientCallbacks { suspend fun sendKexDhGexInit() suspend fun sendNewKeys() fun receiveNewKeys() - suspend fun activateEncryption() + fun activateEncryption() suspend fun sendClientExtInfo() suspend fun sendServiceRequest(service: String) fun receiveServiceAccept(service: String) @@ -365,8 +434,8 @@ internal interface SshClientCallbacks { fun receiveChannelOpenConfirmation(msg: SshMsgChannelOpenConfirmation) fun receiveChannelOpenFailure(msg: SshMsgChannelOpenFailure) suspend fun sendChannelRequest(recipientChannel: Int, requestType: String, wantReply: Boolean, message: SshMsgChannelRequest) - fun receiveChannelSuccess() - fun receiveChannelFailure() + fun receiveChannelSuccess(recipientChannel: Int) + fun receiveChannelFailure(recipientChannel: Int) suspend fun receiveGlobalRequest(msg: SshMsgGlobalRequest) fun debug(msg: SshMsgDebug) fun ignore() diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/transport/PacketIO.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/transport/PacketIO.kt index e0538b6c..235f26f1 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/transport/PacketIO.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/transport/PacketIO.kt @@ -29,7 +29,7 @@ import org.slf4j.LoggerFactory import java.io.ByteArrayOutputStream import java.nio.ByteBuffer import java.security.MessageDigest -import kotlin.random.Random +import java.security.SecureRandom /** * Handles SSH packet framing and unframing according to RFC 4253. @@ -45,7 +45,10 @@ import kotlin.random.Random * * @param transport Underlying transport layer */ -internal class PacketIO(private val transport: Transport) { +internal class PacketIO( + private val transport: Transport, + private val secureRandom: SecureRandom = SecureRandom(), +) { companion object { private val logger = LoggerFactory.getLogger(PacketIO::class.java) @@ -100,20 +103,30 @@ internal class PacketIO(private val transport: Transport) { clientToServerEtm: Boolean = false, serverToClientEtm: Boolean = false, ) { + enableSendEncryption(clientToServerCipher, clientToServerMac, clientToServerEtm) + enableReceiveEncryption(serverToClientCipher, serverToClientMac, serverToClientEtm) + } + + /** Install cipher and MAC protection for subsequent outbound packets. */ + fun enableSendEncryption(cipher: PacketCipher, mac: PacketMac, etm: Boolean = false) { sendAead?.destroy() sendAead = null - receiveAead?.destroy() - receiveAead = null sendCipher?.destroy() sendMac?.destroy() + sendCipher = cipher + sendMac = mac + sendEtm = etm + } + + /** Install cipher and MAC protection for subsequent inbound packets. */ + fun enableReceiveEncryption(cipher: PacketCipher, mac: PacketMac, etm: Boolean = false) { + receiveAead?.destroy() + receiveAead = null receiveCipher?.destroy() receiveMac?.destroy() - this.sendCipher = clientToServerCipher - this.sendMac = clientToServerMac - this.receiveCipher = serverToClientCipher - this.receiveMac = serverToClientMac - this.sendEtm = clientToServerEtm - this.receiveEtm = serverToClientEtm + receiveCipher = cipher + receiveMac = mac + receiveEtm = etm } /** @@ -126,20 +139,30 @@ internal class PacketIO(private val transport: Transport) { clientToServerAead: PacketAead, serverToClientAead: PacketAead, ) { + enableSendAead(clientToServerAead) + enableReceiveAead(serverToClientAead) + } + + /** Install AEAD protection for subsequent outbound packets. */ + fun enableSendAead(aead: PacketAead) { sendCipher?.destroy() sendCipher = null sendMac?.destroy() sendMac = null + sendEtm = false + sendAead?.destroy() + sendAead = aead + } + + /** Install AEAD protection for subsequent inbound packets. */ + fun enableReceiveAead(aead: PacketAead) { receiveCipher?.destroy() receiveCipher = null receiveMac?.destroy() receiveMac = null - sendEtm = false receiveEtm = false - sendAead?.destroy() receiveAead?.destroy() - this.sendAead = clientToServerAead - this.receiveAead = serverToClientAead + receiveAead = aead } /** @@ -171,12 +194,20 @@ internal class PacketIO(private val transport: Transport) { serverToClient: PacketCompressor?, immediateActivation: Boolean, ) { - this.sendCompressor = clientToServer - this.receiveCompressor = serverToClient - if (immediateActivation) { - this.sendCompressionActive = clientToServer != null - this.receiveCompressionActive = serverToClient != null - } + enableSendCompression(clientToServer, immediateActivation) + enableReceiveCompression(serverToClient, immediateActivation) + } + + /** Install the compressor used for subsequent outbound packets. */ + fun enableSendCompression(compressor: PacketCompressor?, immediateActivation: Boolean) { + sendCompressor = compressor + sendCompressionActive = immediateActivation && compressor != null + } + + /** Install the compressor used for subsequent inbound packets. */ + fun enableReceiveCompression(compressor: PacketCompressor?, immediateActivation: Boolean) { + receiveCompressor = compressor + receiveCompressionActive = immediateActivation && compressor != null } /** @@ -486,7 +517,7 @@ internal class PacketIO(private val transport: Transport) { buffer.write(payload) // padding (random bytes) - val padding = Random.nextBytes(paddingLength) + val padding = securePadding(paddingLength) buffer.write(padding) val data = buffer.toByteArray() @@ -524,7 +555,7 @@ internal class PacketIO(private val transport: Transport) { buffer.write(payload) // padding (random bytes) - val padding = Random.nextBytes(paddingLength) + val padding = securePadding(paddingLength) buffer.write(padding) val unencryptedPacket = buffer.toByteArray() @@ -566,7 +597,7 @@ internal class PacketIO(private val transport: Transport) { payloadBuffer.write(paddingLength) payloadBuffer.write(messageType) payloadBuffer.write(payload) - val padding = Random.nextBytes(paddingLength) + val padding = securePadding(paddingLength) payloadBuffer.write(padding) val payloadToEncrypt = payloadBuffer.toByteArray() @@ -611,7 +642,7 @@ internal class PacketIO(private val transport: Transport) { plaintextBuffer.write(paddingLength) plaintextBuffer.write(messageType) plaintextBuffer.write(payload) - plaintextBuffer.write(Random.nextBytes(paddingLength)) + plaintextBuffer.write(securePadding(paddingLength)) val plaintext = plaintextBuffer.toByteArray() val lengthBytes = ByteBuffer.allocate(4).putInt(packetLength).array() @@ -657,6 +688,8 @@ internal class PacketIO(private val transport: Transport) { return paddingLength } + private fun securePadding(length: Int): ByteArray = ByteArray(length).also(secureRandom::nextBytes) + /** * Calculate padding length for ETM/AEAD packets. * diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentDestinationConstraintTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentDestinationConstraintTest.kt index 585bc9ad..10330df3 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentDestinationConstraintTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentDestinationConstraintTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,21 +56,24 @@ class AgentDestinationConstraintTest { private fun buildSignedData( sessionId: ByteArray = byteArrayOf(0xAA.toByte()), username: String = "user", + serviceName: String = "ssh-connection", methodName: String = "publickey", algorithmName: String = "ssh-ed25519", - publicKeyBlob: ByteArray = byteArrayOf(1, 2, 3), + publicKeyBlob: ByteArray = keyBlob, serverHostKey: ByteArray? = null, + trailingData: ByteArray = ByteArray(0), ): ByteArray { val parts = mutableListOf() parts += sshString(sessionId) parts += byteArrayOf(50) // SSH_MSG_USERAUTH_REQUEST parts += sshString(username) - parts += sshString("ssh-connection", Charsets.US_ASCII) + parts += sshString(serviceName, Charsets.US_ASCII) parts += sshString(methodName, Charsets.US_ASCII) parts += byteArrayOf(1) // has_signature = TRUE parts += sshString(algorithmName, Charsets.US_ASCII) parts += sshString(publicKeyBlob) if (serverHostKey != null) parts += sshString(serverHostKey) + parts += trailingData return parts.fold(ByteArray(0)) { acc, b -> acc + b } } @@ -115,6 +118,21 @@ class AgentDestinationConstraintTest { return buildAgentMessage(27, extBytes) } + private fun buildRawSessionBindRequest( + hostKey: ByteArray, + sessionId: ByteArray, + isForwarding: Int, + trailingData: ByteArray = ByteArray(0), + ): ByteArray { + val bindBytes = sshString(hostKey) + + sshString(sessionId) + + sshString(byteArrayOf(0x01)) + + byteArrayOf(isForwarding.toByte()) + + trailingData + val extensionPayload = sshString("session-bind@openssh.com") + bindBytes + return buildAgentMessage(27, extensionPayload) + } + private fun buildSignRequest(keyBlob: ByteArray, dataToSign: ByteArray): ByteArray { val signRequest = SshAgentcSignRequest() signRequest.setKeyBlob(createByteString(keyBlob)) @@ -126,13 +144,33 @@ class AgentDestinationConstraintTest { private val hostKeyA = byteArrayOf(0x10, 0x11, 0x12) private val hostKeyB = byteArrayOf(0x20, 0x21, 0x22) - private val keyBlob = byteArrayOf(0x01, 0x02, 0x03) + private val keyBlob = sshString("ssh-ed25519", Charsets.US_ASCII) + ByteArray(32) { 0x42 } + + private fun destinationConstraints( + fromHostKey: ByteArray = hostKeyA, + toHostKey: ByteArray = hostKeyB, + ) = listOf( + DestinationConstraint( + fromHostname = "", + fromKeyspecs = emptyList(), + toUsername = "", + toHostname = "hop-a", + toHostspecs = listOf(AgentKeySpec(fromHostKey, isCa = false)), + ), + DestinationConstraint( + fromHostname = "hop-a", + fromKeyspecs = listOf(AgentKeySpec(fromHostKey, isCa = false)), + toUsername = "user", + toHostname = "host-b", + toHostspecs = listOf(AgentKeySpec(toHostKey, isCa = false)), + ), + ) @Test fun `unconstrained key is always allowed`() = runTest { val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf(AgentIdentity(keyBlob, "test")) - override suspend fun signData(context: AgentSigningContext) = byteArrayOf(0xFF.toByte()) + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test"))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(0xFF.toByte())) } val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) @@ -144,7 +182,7 @@ class AgentDestinationConstraintTest { } @Test - fun `constrained key direct connection matching destination is allowed`() = runTest { + fun `constrained key cannot sign before destination authenticates its session`() = runTest { val constraints = listOf( DestinationConstraint( fromHostname = "", @@ -155,16 +193,20 @@ class AgentDestinationConstraintTest { ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf(AgentIdentity(keyBlob, "test", constraints)) - override suspend fun signData(context: AgentSigningContext) = byteArrayOf(0xFF.toByte()) + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(0xFF.toByte())) } val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) - val signedData = buildSignedData(username = "user", serverHostKey = hostKeyA) + val signedData = buildSignedData( + sessionId = byteArrayOf(1), + username = "user", + serverHostKey = hostKeyA, + ) val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) val (msgType, _) = parseAgentMessage(response) - assertEquals(14, msgType) // SSH_AGENT_SIGN_RESPONSE + assertEquals(5, msgType) // SSH_AGENT_FAILURE } @Test @@ -179,13 +221,13 @@ class AgentDestinationConstraintTest { ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf(AgentIdentity(keyBlob, "test", constraints)) - override suspend fun signData(context: AgentSigningContext) = byteArrayOf(0xFF.toByte()) + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(0xFF.toByte())) } // sessionInfo has hostKeyB as the server key — not permitted by the constraint (which requires hostKeyA) val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyB), noopVerifier) - val signedData = buildSignedData(username = "user") + val signedData = buildSignedData(sessionId = byteArrayOf(1), username = "user") val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) val (msgType, _) = parseAgentMessage(response) @@ -204,17 +246,20 @@ class AgentDestinationConstraintTest { ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf(AgentIdentity(keyBlob, "test", constraints)) - override suspend fun signData(context: AgentSigningContext) = byteArrayOf(0xFF.toByte()) + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(0xFF.toByte())) } val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) - // Simulate forwarding: bind via hostKeyA, then attempt to sign without hostbound method - handler.handleRequest(buildSessionBindRequest(hostKeyA, byteArrayOf(1), isForwarding = 0)) - handler.handleRequest(buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 1)) + // The handler already has the trusted hostKeyA forwarding binding. + handler.handleRequest(buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0)) // Standard "publickey" method (not hostbound) should be rejected when forwarding - val signedData = buildSignedData(methodName = "publickey", username = "user", serverHostKey = hostKeyB) + val signedData = buildSignedData( + sessionId = byteArrayOf(2), + methodName = "publickey", + username = "user", + ) val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) val (msgType, _) = parseAgentMessage(response) @@ -223,26 +268,135 @@ class AgentDestinationConstraintTest { @Test fun `constrained key forwarding with correct path is allowed`() = runTest { + val constraints = destinationConstraints() + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(0xFF.toByte())) + } + val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) + + val destinationSessionId = byteArrayOf(2) + handler.handleRequest(buildSessionBindRequest(hostKeyB, destinationSessionId, isForwarding = 0)) + + val signedData = buildSignedData( + sessionId = destinationSessionId, + methodName = "publickey-hostbound-v00@openssh.com", + username = "user", + serverHostKey = hostKeyB, + ) + val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) + + val (msgType, _) = parseAgentMessage(response) + assertEquals(14, msgType) // SSH_AGENT_SIGN_RESPONSE + } + + @Test + fun `constrained key direct connection with mismatched signed session id is rejected`() = runTest { + var signCalled = false val constraints = listOf( DestinationConstraint( - fromHostname = "hop-a", - fromKeyspecs = listOf(AgentKeySpec(hostKeyA, isCa = false)), + fromHostname = "", + fromKeyspecs = emptyList(), toUsername = "user", - toHostname = "host-b", - toHostspecs = listOf(AgentKeySpec(hostKeyB, isCa = false)), + toHostname = "host-a", + toHostspecs = listOf(AgentKeySpec(hostKeyA, isCa = false)), ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf(AgentIdentity(keyBlob, "test", constraints)) - override suspend fun signData(context: AgentSigningContext) = byteArrayOf(0xFF.toByte()) + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalled = true + return AgentResult.Success(byteArrayOf(0xFF.toByte())) + } + } + val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) + + val signedData = buildSignedData( + sessionId = byteArrayOf(9), + username = "user", + serverHostKey = hostKeyA, + ) + val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) + + val (msgType, _) = parseAgentMessage(response) + assertEquals(5, msgType) + assertEquals(false, signCalled) + } + + @Test + fun `constrained key forwarded connection with mismatched signed session id is rejected`() = runTest { + var signCalled = false + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", destinationConstraints()))) + + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalled = true + return AgentResult.Success(byteArrayOf(0xFF.toByte())) + } + } + val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) + + handler.handleRequest(buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0)) + + val signedData = buildSignedData( + sessionId = byteArrayOf(9), + methodName = "publickey-hostbound-v00@openssh.com", + username = "user", + serverHostKey = hostKeyB, + ) + val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) + + val (msgType, _) = parseAgentMessage(response) + assertEquals(5, msgType) + assertEquals(false, signCalled) + } + + @Test + fun `constrained key forwarded connection with signed host key not matching latest binding is rejected`() = runTest { + var signCalled = false + val hostKeyC = byteArrayOf(0x30, 0x31, 0x32) + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", destinationConstraints(toHostKey = hostKeyB)))) + + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalled = true + return AgentResult.Success(byteArrayOf(0xFF.toByte())) + } + } + val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) + + handler.handleRequest(buildSessionBindRequest(hostKeyC, byteArrayOf(2), isForwarding = 0)) + + val signedData = buildSignedData( + sessionId = byteArrayOf(2), + methodName = "publickey-hostbound-v00@openssh.com", + username = "user", + serverHostKey = hostKeyB, + ) + val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) + + val (msgType, _) = parseAgentMessage(response) + assertEquals(5, msgType) + assertEquals(false, signCalled) + } + + @Test + fun `constrained key cannot sign when latest binding is for forwarding`() = runTest { + var signCalled = false + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", destinationConstraints()))) + + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalled = true + return AgentResult.Success(byteArrayOf(0xFF.toByte())) + } } val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) - // Origin bind (non-forwarding) through hostKeyA, then forwarding bind through hostKeyA → hostKeyB - handler.handleRequest(buildSessionBindRequest(hostKeyA, byteArrayOf(1), isForwarding = 0)) handler.handleRequest(buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 1)) val signedData = buildSignedData( + sessionId = byteArrayOf(2), methodName = "publickey-hostbound-v00@openssh.com", username = "user", serverHostKey = hostKeyB, @@ -250,7 +404,61 @@ class AgentDestinationConstraintTest { val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) val (msgType, _) = parseAgentMessage(response) - assertEquals(14, msgType) // SSH_AGENT_SIGN_RESPONSE + assertEquals(5, msgType) + assertEquals(false, signCalled) + } + + @Test + fun `authentication binding cannot be extended with another binding`() = runTest { + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) + } + val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) + + val hostKeyC = byteArrayOf(0x30, 0x31, 0x32) + val authResponse = handler.handleRequest( + buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0), + ) + val forwardingResponse = handler.handleRequest( + buildSessionBindRequest(hostKeyC, byteArrayOf(3), isForwarding = 1), + ) + + assertEquals(6, parseAgentMessage(authResponse).first) + assertEquals(5, parseAgentMessage(forwardingResponse).first) + } + + @Test + fun `failed binding attempt prevents constrained signing on same agent connection`() = runTest { + var signCalled = false + val constraints = destinationConstraints() + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalled = true + return AgentResult.Success(byteArrayOf(0xFF.toByte())) + } + } + val rejectingVerifier = SessionBindVerifier { _, _, _ -> false } + val handler = AgentProtocolHandler( + provider, + AgentSessionInfo(byteArrayOf(1), hostKeyA), + rejectingVerifier, + ) + + val bindResponse = handler.handleRequest( + buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0), + ) + val signedData = buildSignedData( + sessionId = byteArrayOf(1), + methodName = "publickey-hostbound-v00@openssh.com", + serverHostKey = hostKeyA, + ) + val signResponse = handler.handleRequest(buildSignRequest(keyBlob, signedData)) + + assertEquals(5, parseAgentMessage(bindResponse).first) + assertEquals(5, parseAgentMessage(signResponse).first) + assertEquals(false, signCalled) } @Test @@ -265,17 +473,17 @@ class AgentDestinationConstraintTest { ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf(AgentIdentity(keyBlob, "test", constraints)) - override suspend fun signData(context: AgentSigningContext) = byteArrayOf(0xFF.toByte()) + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(keyBlob, "test", constraints))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(0xFF.toByte())) } val hostKeyC = byteArrayOf(0x30, 0x31, 0x32) val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyC), noopVerifier) // Forwarding through hostKeyC (not in constraints) - handler.handleRequest(buildSessionBindRequest(hostKeyC, byteArrayOf(1), isForwarding = 0)) - handler.handleRequest(buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 1)) + handler.handleRequest(buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0)) val signedData = buildSignedData( + sessionId = byteArrayOf(2), methodName = "publickey-hostbound-v00@openssh.com", username = "user", serverHostKey = hostKeyB, @@ -292,6 +500,13 @@ class AgentDestinationConstraintTest { val unconstrainedKey = byteArrayOf(0xCC.toByte(), 0xDD.toByte()) val constraints = listOf( + DestinationConstraint( + fromHostname = "", + fromKeyspecs = emptyList(), + toUsername = "", + toHostname = "hop-a", + toHostspecs = listOf(AgentKeySpec(hostKeyA, isCa = false)), + ), DestinationConstraint( fromHostname = "hop-a", fromKeyspecs = listOf(AgentKeySpec(hostKeyA, isCa = false)), @@ -301,17 +516,16 @@ class AgentDestinationConstraintTest { ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf( - AgentIdentity(constrainedKey, "constrained", constraints), - AgentIdentity(unconstrainedKey, "unconstrained"), + override suspend fun getIdentities() = AgentResult.Success( + listOf( + AgentIdentity(constrainedKey, "constrained", constraints), + AgentIdentity(unconstrainedKey, "unconstrained"), + ), ) - override suspend fun signData(context: AgentSigningContext) = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyA), noopVerifier) - // Add a forwarding binding through hostKeyA - handler.handleRequest(buildSessionBindRequest(hostKeyA, byteArrayOf(1), isForwarding = 0)) - val response = handler.handleRequest(buildAgentMessage(11, ByteArray(0))) val (msgType, payload) = parseAgentMessage(response) assertEquals(12, msgType) // SSH_AGENT_IDENTITIES_ANSWER @@ -333,6 +547,13 @@ class AgentDestinationConstraintTest { val hostKeyC = byteArrayOf(0x30, 0x31, 0x32) val constraints = listOf( + DestinationConstraint( + fromHostname = "", + fromKeyspecs = emptyList(), + toUsername = "", + toHostname = "hop-a", + toHostspecs = listOf(AgentKeySpec(hostKeyA, isCa = false)), + ), DestinationConstraint( fromHostname = "hop-a", fromKeyspecs = listOf(AgentKeySpec(hostKeyA, isCa = false)), @@ -342,15 +563,16 @@ class AgentDestinationConstraintTest { ), ) val provider = object : AgentProvider { - override suspend fun getIdentities() = listOf( - AgentIdentity(constrainedKey, "constrained", constraints), - AgentIdentity(unconstrainedKey, "unconstrained"), + override suspend fun getIdentities() = AgentResult.Success( + listOf( + AgentIdentity(constrainedKey, "constrained", constraints), + AgentIdentity(unconstrainedKey, "unconstrained"), + ), ) - override suspend fun signData(context: AgentSigningContext) = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } // Forwarding through hostKeyC (not hostKeyA, so constrained key not reachable) val handler = AgentProtocolHandler(provider, AgentSessionInfo(byteArrayOf(1), hostKeyC), noopVerifier) - handler.handleRequest(buildSessionBindRequest(hostKeyC, byteArrayOf(1), isForwarding = 0)) val response = handler.handleRequest(buildAgentMessage(11, ByteArray(0))) val (msgType, payload) = parseAgentMessage(response) @@ -364,4 +586,132 @@ class AgentDestinationConstraintTest { assertEquals(1, answer.nkeys().toInt()) assert(answer.identities()[0].keyBlob().data().contentEquals(unconstrainedKey)) } + + @Test + fun `constrained signing rejects malformed or inconsistent userauth data`() = runTest { + var signCalls = 0 + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success( + listOf( + AgentIdentity(keyBlob, "test", destinationConstraints()), + ), + ) + + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalls++ + return AgentResult.Success(byteArrayOf(0x7F)) + } + } + val invalidSignedData = listOf( + buildSignedData( + sessionId = byteArrayOf(2), + serviceName = "not-ssh-connection", + methodName = "publickey-hostbound-v00@openssh.com", + serverHostKey = hostKeyB, + ), + buildSignedData( + sessionId = byteArrayOf(2), + methodName = "keyboard-interactive", + serverHostKey = hostKeyB, + ), + buildSignedData( + sessionId = byteArrayOf(2), + methodName = "publickey-hostbound-v00@openssh.com", + publicKeyBlob = keyBlob + byteArrayOf(0), + serverHostKey = hostKeyB, + ), + buildSignedData( + sessionId = byteArrayOf(2), + methodName = "publickey-hostbound-v00@openssh.com", + algorithmName = "ssh-rsa", + serverHostKey = hostKeyB, + ), + buildSignedData( + sessionId = byteArrayOf(2), + methodName = "publickey-hostbound-v00@openssh.com", + serverHostKey = hostKeyB, + trailingData = byteArrayOf(0), + ), + ) + + for (signedData in invalidSignedData) { + val handler = AgentProtocolHandler( + provider, + AgentSessionInfo(byteArrayOf(1), hostKeyA), + noopVerifier, + ) + assertEquals( + 6, + parseAgentMessage( + handler.handleRequest( + buildSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0), + ), + ).first, + ) + + val response = handler.handleRequest(buildSignRequest(keyBlob, signedData)) + assertEquals(5, parseAgentMessage(response).first) + } + assertEquals(0, signCalls) + } + + @Test + fun `session binding enforces length count and trailing data limits`() = runTest { + var verifyCalls = 0 + val verifier = SessionBindVerifier { _, _, _ -> + verifyCalls++ + true + } + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) + } + val handler = AgentProtocolHandler( + provider, + AgentSessionInfo(byteArrayOf(1), hostKeyA), + verifier, + ) + + val oversized = handler.handleRequest( + buildRawSessionBindRequest(hostKeyB, ByteArray(129), isForwarding = 1), + ) + val trailing = handler.handleRequest( + buildRawSessionBindRequest(hostKeyB, byteArrayOf(99), isForwarding = 1, trailingData = byteArrayOf(0)), + ) + assertEquals(5, parseAgentMessage(oversized).first) + assertEquals(5, parseAgentMessage(trailing).first) + assertEquals(0, verifyCalls) + + for (value in 2..16) { + val response = handler.handleRequest( + buildRawSessionBindRequest(hostKeyB, byteArrayOf(value.toByte()), isForwarding = 1), + ) + assertEquals(6, parseAgentMessage(response).first) + } + assertEquals(15, verifyCalls) + + val overLimit = handler.handleRequest( + buildRawSessionBindRequest(hostKeyB, byteArrayOf(17), isForwarding = 1), + ) + assertEquals(5, parseAgentMessage(overLimit).first) + assertEquals(15, verifyCalls) + } + + @Test + fun `session bind verifier exceptions fail closed`() = runTest { + val provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) + } + val handler = AgentProtocolHandler( + provider, + AgentSessionInfo(byteArrayOf(1), hostKeyA), + SessionBindVerifier { _, _, _ -> error("verifier failure") }, + ) + + val response = handler.handleRequest( + buildRawSessionBindRequest(hostKeyB, byteArrayOf(2), isForwarding = 0), + ) + assertEquals(5, parseAgentMessage(response).first) + } } diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProtocolTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProtocolTest.kt index 7779c741..adfabc3f 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProtocolTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProtocolTest.kt @@ -41,7 +41,6 @@ import org.junit.jupiter.api.Assertions.assertArrayEquals import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNotNull -import org.junit.jupiter.api.Assertions.assertSame import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.nio.BufferUnderflowException @@ -150,9 +149,9 @@ class AgentProtocolTest { fun `handler returns failure for malformed agent frame`() = runTest { val handler = AgentProtocolHandler( provider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() + override suspend fun getIdentities() = AgentResult.Success(emptyList()) - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) }, sessionInfo = AgentSessionInfo(ByteArray(0), ByteArray(0)), ) @@ -175,9 +174,9 @@ class AgentProtocolTest { fun `handler returns failure for malformed sign request payload`() = runTest { val handler = AgentProtocolHandler( provider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() + override suspend fun getIdentities() = AgentResult.Success(emptyList()) - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) }, sessionInfo = AgentSessionInfo(ByteArray(0), ByteArray(0)), ) @@ -201,33 +200,76 @@ class AgentProtocolTest { } @Test - fun `handler does not mistake provider exception for malformed input`() = runTest { + fun `handler converts provider exception to agent failure`() = runTest { val providerFailure = BufferUnderflowException() val handler = AgentProtocolHandler( provider = object : AgentProvider { - override suspend fun getIdentities(): List = throw providerFailure + override suspend fun getIdentities(): AgentResult> = throw providerFailure - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) }, sessionInfo = AgentSessionInfo(ByteArray(0), ByteArray(0)), ) - val error = assertFailsWith { - handler.handleRequest(buildAgentMessage(11, ByteArray(0))) + val response = handler.handleRequest(buildAgentMessage(11, ByteArray(0))) + + assertEquals(5, parseAgentMessage(response).first) + } + + @Test + fun `handler converts explicit provider failure to agent failure`() = runTest { + val handler = AgentProtocolHandler( + provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Failure("agent backend unavailable") + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) + }, + sessionInfo = AgentSessionInfo(ByteArray(0), ByteArray(0)), + ) + + val response = handler.handleRequest(buildAgentMessage(11, ByteArray(0))) + + assertEquals(5, parseAgentMessage(response).first) + } + + @Test + fun `handler never throws across deterministic agent input sweeps`() = runTest { + val handler = AgentProtocolHandler( + provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) + }, + sessionInfo = AgentSessionInfo(ByteArray(0), ByteArray(0)), + ) + suspend fun assertHandled(request: ByteArray) { + val response = handler.handleRequest(request) + val messageType = parseAgentMessage(response).first + assertTrue(messageType == 5 || messageType == 12) } - assertSame(providerFailure, error) + for (messageType in 0..255) { + assertHandled(buildAgentMessage(messageType, ByteArray(0))) + } + for (value in 0..255) { + assertHandled(ByteArray(1) { value.toByte() }) + assertHandled(ByteArray(5) { value.toByte() }) + assertHandled(ByteArray(16) { value.toByte() }) + } + for (size in 0..256) { + assertHandled(ByteArray(size) { index -> ((size + index) and 0xFF).toByte() }) + } } @Test fun `handler returns identities answer`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = listOf( - AgentIdentity(byteArrayOf(1, 2, 3), "key1"), - AgentIdentity(byteArrayOf(4, 5, 6), "key2"), + override suspend fun getIdentities() = AgentResult.Success( + listOf( + AgentIdentity(byteArrayOf(1, 2, 3), "key1"), + AgentIdentity(byteArrayOf(4, 5, 6), "key2"), + ), ) - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val sessionInfo = AgentSessionInfo( @@ -255,10 +297,11 @@ class AgentProtocolTest { @Test fun `handler returns sign response when provider approves`() = runTest { + val signingKey = byteArrayOf(1, 2, 3, 4) val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(signingKey, "test"))) - override suspend fun signData(context: AgentSigningContext): ByteArray? = byteArrayOf(9, 8, 7, 6, 5) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(byteArrayOf(9, 8, 7, 6, 5)) } val sessionInfo = AgentSessionInfo( @@ -268,7 +311,7 @@ class AgentProtocolTest { val handler = AgentProtocolHandler(testProvider, sessionInfo) - val keyBlob = createByteString(byteArrayOf(1, 2, 3, 4)) + val keyBlob = createByteString(signingKey) val dataToSign = createByteString(byteArrayOf(5, 6, 7, 8)) val signRequest = SshAgentcSignRequest() signRequest.setKeyBlob(keyBlob) @@ -291,10 +334,11 @@ class AgentProtocolTest { @Test fun `handler returns failure when provider denies`() = runTest { + val signingKey = byteArrayOf(1, 2, 3, 4) val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(signingKey, "test"))) - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val sessionInfo = AgentSessionInfo( @@ -304,7 +348,7 @@ class AgentProtocolTest { val handler = AgentProtocolHandler(testProvider, sessionInfo) - val keyBlob = createByteString(byteArrayOf(1, 2, 3, 4)) + val keyBlob = createByteString(signingKey) val dataToSign = createByteString(byteArrayOf(5, 6, 7, 8)) val signRequest = SshAgentcSignRequest() signRequest.setKeyBlob(keyBlob) @@ -319,16 +363,67 @@ class AgentProtocolTest { assertEquals(5, messageType) // SSH_AGENT_FAILURE } + @Test + fun `handler converts signing provider failure to agent failure`() = runTest { + val signingKey = byteArrayOf(1, 2, 3, 4) + val handler = AgentProtocolHandler( + provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(signingKey, "test"))) + + override suspend fun signData(context: AgentSigningContext) = AgentResult.Failure("signing device unavailable") + }, + sessionInfo = AgentSessionInfo(byteArrayOf(1), byteArrayOf(2)), + ) + val signRequest = SshAgentcSignRequest().apply { + setKeyBlob(createByteString(signingKey)) + setData(createByteString(byteArrayOf(5, 6, 7, 8))) + setFlags(0) + _check() + } + + val response = handler.handleRequest(buildAgentMessage(13, signRequest.toByteArray())) + + assertEquals(5, parseAgentMessage(response).first) + } + + @Test + fun `handler does not invoke provider for an identity it did not expose`() = runTest { + var signCalled = false + val handler = AgentProtocolHandler( + provider = object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + + override suspend fun signData(context: AgentSigningContext): AgentResult { + signCalled = true + return AgentResult.Success(byteArrayOf(1)) + } + }, + sessionInfo = AgentSessionInfo(byteArrayOf(1), byteArrayOf(2)), + ) + val signRequest = SshAgentcSignRequest().apply { + setKeyBlob(createByteString(byteArrayOf(3))) + setData(createByteString(byteArrayOf(4))) + setFlags(0) + _check() + } + + val response = handler.handleRequest(buildAgentMessage(13, signRequest.toByteArray())) + + assertEquals(5, parseAgentMessage(response).first) + assertFalse(signCalled) + } + @Test fun `provider receives correct context`() = runTest { var capturedContext: AgentSigningContext? = null + val signingKey = byteArrayOf(1, 2, 3, 4) val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(signingKey, "test"))) - override suspend fun signData(context: AgentSigningContext): ByteArray? { + override suspend fun signData(context: AgentSigningContext): AgentResult { capturedContext = context - return byteArrayOf(1, 2, 3) + return AgentResult.Success(byteArrayOf(1, 2, 3)) } } @@ -339,7 +434,7 @@ class AgentProtocolTest { val handler = AgentProtocolHandler(testProvider, sessionInfo) - val keyBlob = createByteString(byteArrayOf(1, 2, 3, 4)) + val keyBlob = createByteString(signingKey) val dataToSign = createByteString(byteArrayOf(5, 6, 7, 8)) val signRequest = SshAgentcSignRequest() signRequest.setKeyBlob(keyBlob) @@ -356,7 +451,7 @@ class AgentProtocolTest { assertEquals(2, capturedContext.flags) assertArrayEquals(byteArrayOf(10, 11, 12), capturedContext.sessionId) assertArrayEquals(byteArrayOf(13, 14, 15), capturedContext.serverHostKey) - assertFalse(capturedContext.isBound) + assertTrue(capturedContext.isBound) } private fun buildSessionBindRequest( @@ -383,10 +478,10 @@ class AgentProtocolTest { private val rejectingVerifier: SessionBindVerifier = SessionBindVerifier { _, _, _ -> false } @Test - fun `handler handles session bind extension`() = runTest { + fun `handler rejects peer replay of trusted initial binding`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val sessionId = byteArrayOf(1, 2, 3) @@ -397,14 +492,14 @@ class AgentProtocolTest { val response = handler.handleRequest(buildSessionBindRequest(hostKey, sessionId, isForwarding = 1)) val (messageType, _) = parseAgentMessage(response) - assertEquals(6, messageType) // SSH_AGENT_SUCCESS + assertEquals(5, messageType) // SSH_AGENT_FAILURE } @Test fun `handler rejects session bind when signature verification fails`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val sessionId = byteArrayOf(1, 2, 3) @@ -412,7 +507,9 @@ class AgentProtocolTest { val sessionInfo = AgentSessionInfo(sessionId, hostKey) val handler = AgentProtocolHandler(testProvider, sessionInfo, rejectingVerifier) - val response = handler.handleRequest(buildSessionBindRequest(hostKey, sessionId, isForwarding = 1)) + val response = handler.handleRequest( + buildSessionBindRequest(byteArrayOf(7, 8, 9), byteArrayOf(4, 5, 6), isForwarding = 1), + ) val (messageType, _) = parseAgentMessage(response) assertEquals(5, messageType) // SSH_AGENT_FAILURE @@ -421,8 +518,8 @@ class AgentProtocolTest { @Test fun `handler rejects duplicate session bind`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val sessionId = byteArrayOf(1, 2, 3) @@ -430,7 +527,7 @@ class AgentProtocolTest { val sessionInfo = AgentSessionInfo(sessionId, hostKey) val handler = AgentProtocolHandler(testProvider, sessionInfo, noopVerifier) - val requestMessage = buildSessionBindRequest(hostKey, sessionId, isForwarding = 1) + val requestMessage = buildSessionBindRequest(byteArrayOf(7, 8, 9), byteArrayOf(4, 5, 6), isForwarding = 1) handler.handleRequest(requestMessage) val response = handler.handleRequest(requestMessage) @@ -439,16 +536,15 @@ class AgentProtocolTest { } @Test - fun `handler rejects non-forwarding session bind with mismatched hostkey`() = runTest { + fun `handler rejects replay of trusted session as non-forwarding bind`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val sessionInfo = AgentSessionInfo(byteArrayOf(1, 2, 3), byteArrayOf(4, 5, 6)) val handler = AgentProtocolHandler(testProvider, sessionInfo, noopVerifier) - // is_forwarding = 0 means origin bind — hostkey must match sessionInfo.serverHostKey val response = handler.handleRequest( buildSessionBindRequest(byteArrayOf(9, 9, 9), byteArrayOf(1, 2, 3), isForwarding = 0), ) @@ -460,19 +556,23 @@ class AgentProtocolTest { @Test fun `handler accumulates multiple forwarding binds`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val hostKey = byteArrayOf(4, 5, 6) val sessionInfo = AgentSessionInfo(byteArrayOf(1, 2, 3), hostKey) val handler = AgentProtocolHandler(testProvider, sessionInfo, noopVerifier) - val response1 = handler.handleRequest(buildSessionBindRequest(hostKey, byteArrayOf(1, 2, 3), isForwarding = 0)) + val response1 = handler.handleRequest( + buildSessionBindRequest(byteArrayOf(7, 8, 9), byteArrayOf(4, 5, 6), isForwarding = 1), + ) val (type1, _) = parseAgentMessage(response1) assertEquals(6, type1) - val response2 = handler.handleRequest(buildSessionBindRequest(byteArrayOf(7, 8, 9), byteArrayOf(4, 5, 6), isForwarding = 1)) + val response2 = handler.handleRequest( + buildSessionBindRequest(byteArrayOf(10, 11, 12), byteArrayOf(7, 8, 9), isForwarding = 1), + ) val (type2, _) = parseAgentMessage(response2) assertEquals(6, type2) } @@ -480,8 +580,8 @@ class AgentProtocolTest { @Test fun `handler returns failure for unknown extension`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val handler = AgentProtocolHandler(testProvider, AgentSessionInfo(byteArrayOf(), byteArrayOf())) @@ -501,8 +601,8 @@ class AgentProtocolTest { @Test fun `handler returns failure for unknown message type`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) } val handler = AgentProtocolHandler(testProvider, AgentSessionInfo(byteArrayOf(), byteArrayOf())) @@ -525,13 +625,14 @@ class AgentProtocolTest { // A 255-byte signature makes the SIGN_RESPONSE payload large enough that byte[2] of the // length field is non-zero. Routes through the production buildAgentMessage (not the local helper). val largeSignature = ByteArray(255) { it.toByte() } + val signingKey = byteArrayOf(1) val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray = largeSignature + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(signingKey, "test"))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(largeSignature) } val handler = AgentProtocolHandler(testProvider, AgentSessionInfo(byteArrayOf(1), byteArrayOf(2))) val signRequest = SshAgentcSignRequest() - signRequest.setKeyBlob(createByteString(byteArrayOf(1))) + signRequest.setKeyBlob(createByteString(signingKey)) signRequest.setData(createByteString(byteArrayOf(2))) signRequest.setFlags(0) signRequest._check() @@ -548,13 +649,14 @@ class AgentProtocolTest { // A 65535-byte signature forces byte[1] of the length field to be non-zero. // Routes through the production buildAgentMessage. val hugeSignature = ByteArray(65535) { it.toByte() } + val signingKey = byteArrayOf(1) val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray = hugeSignature + override suspend fun getIdentities() = AgentResult.Success(listOf(AgentIdentity(signingKey, "test"))) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(hugeSignature) } val handler = AgentProtocolHandler(testProvider, AgentSessionInfo(byteArrayOf(1), byteArrayOf(2))) val signRequest = SshAgentcSignRequest() - signRequest.setKeyBlob(createByteString(byteArrayOf(1))) + signRequest.setKeyBlob(createByteString(signingKey)) signRequest.setData(createByteString(byteArrayOf(2))) signRequest.setFlags(0) signRequest._check() @@ -628,80 +730,105 @@ class IsConstraintSatisfiedTest { private fun keyspec(blob: ByteArray) = AgentKeySpec(blob, false) - private val hostKey = byteArrayOf(0x01, 0x02, 0x03) - private val sessionId = byteArrayOf(0x04, 0x05, 0x06) + private val hostKeyA = byteArrayOf(0x01) + private val hostKeyB = byteArrayOf(0x02) + private val hostKeyC = byteArrayOf(0x03) @Test - fun `direct connection - no constraints - always rejected`() { - val components = SignedDataComponents("publickey", "user", hostKey) - assertFalse(isConstraintSatisfied(emptyList(), components, emptyList())) + fun `empty path is rejected`() { + assertFalse(isConstraintSatisfied(emptyList(), emptyList(), "user")) } @Test - fun `direct connection - empty from constraint with matching toHostspec - satisfied`() { - val c = constraint(toHostspecs = listOf(keyspec(hostKey))) - val components = SignedDataComponents("publickey", "user", hostKey) - assertTrue(isConstraintSatisfied(listOf(c), components, emptyList())) - } + fun `trusted first binding must match an origin edge`() { + val bindings = listOf(BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true)) + val constraints = listOf( + constraint(toHostspecs = listOf(keyspec(hostKeyA))), + constraint(fromKeyspecs = listOf(keyspec(hostKeyA)), toHostspecs = listOf(keyspec(hostKeyB))), + ) - @Test - fun `direct connection - empty from constraint with non-matching toHostspec - rejected`() { - val c = constraint(toHostspecs = listOf(keyspec(byteArrayOf(0x99.toByte())))) - val components = SignedDataComponents("publickey", "user", hostKey) - assertFalse(isConstraintSatisfied(listOf(c), components, emptyList())) + assertTrue(isConstraintSatisfied(constraints, bindings, destinationUsername = null)) } @Test - fun `direct connection - toUsername mismatch - rejected`() { - val c = constraint(toHostspecs = listOf(keyspec(hostKey)), toUsername = "alice") - val components = SignedDataComponents("publickey", "bob", hostKey) - assertFalse(isConstraintSatisfied(listOf(c), components, emptyList())) + fun `missing origin edge rejects an otherwise matching final edge`() { + val bindings = listOf( + BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true), + BindingEntry(hostKeyB, byteArrayOf(2), isForwarding = false), + ) + val constraints = listOf( + constraint(fromKeyspecs = listOf(keyspec(hostKeyA)), toHostspecs = listOf(keyspec(hostKeyB))), + ) + + assertFalse(isConstraintSatisfied(constraints, bindings, "user")) } @Test - fun `direct connection - toUsername matches - satisfied`() { - val c = constraint(toHostspecs = listOf(keyspec(hostKey)), toUsername = "alice") - val components = SignedDataComponents("publickey", "alice", hostKey) - assertTrue(isConstraintSatisfied(listOf(c), components, emptyList())) + fun `all edges in a multi-hop path must be permitted`() { + val bindings = listOf( + BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true), + BindingEntry(hostKeyB, byteArrayOf(2), isForwarding = true), + BindingEntry(hostKeyC, byteArrayOf(3), isForwarding = false), + ) + val complete = listOf( + constraint(toHostspecs = listOf(keyspec(hostKeyA))), + constraint(fromKeyspecs = listOf(keyspec(hostKeyA)), toHostspecs = listOf(keyspec(hostKeyB))), + constraint(fromKeyspecs = listOf(keyspec(hostKeyB)), toHostspecs = listOf(keyspec(hostKeyC))), + ) + val missingMiddle = listOf(complete[0], complete[2]) + + assertTrue(isConstraintSatisfied(complete, bindings, "user")) + assertFalse(isConstraintSatisfied(missingMiddle, bindings, "user")) } @Test - fun `direct connection - null serverHostKeyBlob - rejected`() { - val c = constraint(toHostspecs = listOf(keyspec(hostKey))) - val components = SignedDataComponents("publickey", "user", null) - assertFalse(isConstraintSatisfied(listOf(c), components, emptyList())) + fun `username is enforced on final edge only`() { + val bindings = listOf( + BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true), + BindingEntry(hostKeyB, byteArrayOf(2), isForwarding = false), + ) + val constraints = listOf( + constraint(toUsername = "ignored", toHostspecs = listOf(keyspec(hostKeyA))), + constraint( + fromKeyspecs = listOf(keyspec(hostKeyA)), + toUsername = "alice", + toHostspecs = listOf(keyspec(hostKeyB)), + ), + ) + + assertTrue(isConstraintSatisfied(constraints, bindings, "alice")) + assertFalse(isConstraintSatisfied(constraints, bindings, "bob")) } @Test - fun `forwarded connection - single binding - fromKeyspec matches hop - satisfied`() { - val hopKey = byteArrayOf(0xAA.toByte()) - val destKey = byteArrayOf(0xBB.toByte()) - val binding = BindingEntry(hopKey, sessionId) - val c = constraint(fromKeyspecs = listOf(keyspec(hopKey)), toHostspecs = listOf(keyspec(destKey))) - val components = SignedDataComponents("publickey-hostbound-v00@openssh.com", "user", destKey) - assertTrue(isConstraintSatisfied(listOf(c), components, listOf(binding))) + fun `signing rejects a forwarding-only final binding`() { + val bindings = listOf(BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true)) + val constraints = listOf(constraint(toHostspecs = listOf(keyspec(hostKeyA)))) + + assertFalse(isConstraintSatisfied(constraints, bindings, "user")) } @Test - fun `forwarded connection - single binding - fromKeyspec does not match - rejected`() { - val hopKey = byteArrayOf(0xAA.toByte()) - val wrongKey = byteArrayOf(0xCC.toByte()) - val destKey = byteArrayOf(0xBB.toByte()) - val binding = BindingEntry(hopKey, sessionId) - val c = constraint(fromKeyspecs = listOf(keyspec(wrongKey)), toHostspecs = listOf(keyspec(destKey))) - val components = SignedDataComponents("publickey-hostbound-v00@openssh.com", "user", destKey) - assertFalse(isConstraintSatisfied(listOf(c), components, listOf(binding))) + fun `listing on a forwarding binding requires a permitted outgoing edge`() { + val bindings = listOf(BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true)) + val originOnly = listOf(constraint(toHostspecs = listOf(keyspec(hostKeyA)))) + val reachable = originOnly + constraint( + fromKeyspecs = listOf(keyspec(hostKeyA)), + toHostspecs = listOf(keyspec(hostKeyB)), + ) + + assertFalse(isConstraintSatisfied(originOnly, bindings, destinationUsername = null)) + assertTrue(isConstraintSatisfied(reachable, bindings, destinationUsername = null)) } @Test - fun `forwarded connection - two bindings - uses second-to-last as hop key`() { - val hop1Key = byteArrayOf(0xAA.toByte()) - val hop2Key = byteArrayOf(0xBB.toByte()) - val destKey = byteArrayOf(0xCC.toByte()) - val bindings = listOf(BindingEntry(hop1Key, byteArrayOf(1)), BindingEntry(hop2Key, byteArrayOf(2))) - val c = constraint(fromKeyspecs = listOf(keyspec(hop1Key)), toHostspecs = listOf(keyspec(destKey))) - val components = SignedDataComponents("publickey-hostbound-v00@openssh.com", "user", destKey) - assertTrue(isConstraintSatisfied(listOf(c), components, bindings)) + fun `CA keyspecs fail closed until host certificates are parsed`() { + val bindings = listOf(BindingEntry(hostKeyA, byteArrayOf(1), isForwarding = true)) + val constraints = listOf( + constraint(toHostspecs = listOf(AgentKeySpec(hostKeyA, isCa = true))), + ) + + assertFalse(isConstraintSatisfied(constraints, bindings, destinationUsername = null)) } } diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProviderTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProviderTest.kt index b755d0a1..cdbd35d1 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProviderTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/AgentProviderTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Test +import kotlin.test.assertIs class AgentProviderTest { @@ -50,25 +51,27 @@ class AgentProviderTest { @Test fun `AgentProvider can be implemented with custom logic`() = runTest { val testProvider = object : AgentProvider { - override suspend fun getIdentities(): List = listOf( - AgentIdentity( - publicKeyBlob = byteArrayOf(1, 2, 3), - comment = "test-key-1", - ), - AgentIdentity( - publicKeyBlob = byteArrayOf(4, 5, 6), - comment = "test-key-2", + override suspend fun getIdentities() = AgentResult.Success( + listOf( + AgentIdentity( + publicKeyBlob = byteArrayOf(1, 2, 3), + comment = "test-key-1", + ), + AgentIdentity( + publicKeyBlob = byteArrayOf(4, 5, 6), + comment = "test-key-2", + ), ), ) - override suspend fun signData(context: AgentSigningContext): ByteArray? = if (context.isBound) { - byteArrayOf(1, 2, 3, 4) + override suspend fun signData(context: AgentSigningContext) = if (context.isBound) { + AgentResult.Success(byteArrayOf(1, 2, 3, 4)) } else { - null + AgentResult.Success(null) } } - val identities = testProvider.getIdentities() + val identities = assertIs>>(testProvider.getIdentities()).value assertEquals(2, identities.size) assertEquals("test-key-1", identities[0].comment) assertEquals("test-key-2", identities[1].comment) @@ -81,10 +84,10 @@ class AgentProviderTest { serverHostKey = byteArrayOf(10, 11, 12), isBound = true, ) - assertNotNull(testProvider.signData(boundContext)) + assertNotNull(assertIs>(testProvider.signData(boundContext)).value) val unboundContext = boundContext.copy(isBound = false) - assertNull(testProvider.signData(unboundContext)) + assertNull(assertIs>(testProvider.signData(unboundContext)).value) } @Test diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/SshClientTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/SshClientTest.kt index 03f839fc..6ba01eec 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/SshClientTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/SshClientTest.kt @@ -481,7 +481,7 @@ class SshClientTest { assertNull(client.forwardStream(ByteChannel(autoFlush = true), ByteChannel(autoFlush = true), "remote", 22)) - val channel = ForwardingChannel(connection, 1, 2, 32768, 32768) + val channel = ForwardingChannel(connection, backgroundScope, 1, 2, 32768, 32768) coEvery { connection.openDirectTcpipChannel("remote", 22, "127.0.0.1", 0) } returns channel val forwarder = client.forwardStream(ByteChannel(autoFlush = true), ByteChannel(autoFlush = true), "remote", 22) @@ -500,7 +500,7 @@ class SshClientTest { @Test fun `openDirectTcpipTransport opens direct tcpip channel from factory`() = runTest { val connection = mockk(relaxed = true) - val channel = ForwardingChannel(connection, 1, 2, 32768, 32768) + val channel = ForwardingChannel(connection, backgroundScope, 1, 2, 32768, 32768) coEvery { connection.openDirectTcpipChannel("remote", 22, "127.0.0.1", 0) } returns channel val factory = connectedClient(connection, authenticated = true) diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/AgentChannelTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/AgentChannelTest.kt index 43d022a1..224d3a4f 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/AgentChannelTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/AgentChannelTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,9 +21,11 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.launch +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test @@ -36,26 +38,20 @@ class AgentChannelTest { private val maxPacketSize = 1024 private val initialWindowSize = 4096L - private val agentChannel = AgentChannel( - handler, - connection, - localChannel, - remoteChannel, - maxPacketSize, - initialWindowSize, - ) - @Test fun `handleData calls handler and sends response`() = runTest { + val agentChannel = newChannel(this) val request = byteArrayOf(1, 2, 3) val response = byteArrayOf(4, 5, 6) coEvery { handler.handleRequest(request) } returns response agentChannel.handleData(request) + advanceUntilIdle() coVerify { handler.handleRequest(request) } coVerify { connection.sendChannelData(remoteChannel, response) } + agentChannel.onClose() } @Test @@ -63,6 +59,7 @@ class AgentChannelTest { val smallWindowChannel = AgentChannel( handler, connection, + this, localChannel, remoteChannel, maxPacketSize, @@ -73,19 +70,21 @@ class AgentChannelTest { val response = byteArrayOf(4, 5, 6) coEvery { handler.handleRequest(request) } returns response - // This would suspend if we don't adjust window - val job = launch { - smallWindowChannel.handleData(request) - } + // Packet dispatch must return even while the response is blocked by the + // peer's zero-sized send window. + smallWindowChannel.handleData(request) + coVerify(exactly = 0) { connection.sendChannelData(any(), any()) } smallWindowChannel.onWindowAdjust(10L) - job.join() + advanceUntilIdle() coVerify { connection.sendChannelData(remoteChannel, response) } + smallWindowChannel.onClose() } @Test fun `onClose sends CHANNEL_CLOSE and closes channel`() = runTest { + val agentChannel = newChannel(backgroundScope) assertTrue(agentChannel.isOpen) agentChannel.onClose() @@ -96,6 +95,7 @@ class AgentChannelTest { @Test fun `onClose only sends CHANNEL_CLOSE once`() = runTest { + val agentChannel = newChannel(backgroundScope) agentChannel.onClose() agentChannel.onClose() @@ -104,6 +104,7 @@ class AgentChannelTest { @Test fun `handleData does nothing if channel is closed`() = runTest { + val agentChannel = newChannel(backgroundScope) agentChannel.onClose() agentChannel.handleData(byteArrayOf(1, 2, 3)) @@ -112,6 +113,26 @@ class AgentChannelTest { @Test fun `onEof does not throw`() { + val agentChannel = newChannel(kotlinx.coroutines.CoroutineScope(kotlinx.coroutines.Dispatchers.Unconfined)) agentChannel.onEof() } + + @Test + fun `remote packet size is rejected or safely bounded`() { + assertNull(boundRemotePacketSize(0)) + assertNull(boundRemotePacketSize(-1)) + assertNull(boundRemotePacketSize(0x1_0000_0000L)) + assertEquals(1, boundRemotePacketSize(1)) + assertEquals(32 * 1024, boundRemotePacketSize(0xFFFFFFFFL)) + } + + private fun newChannel(scope: kotlinx.coroutines.CoroutineScope) = AgentChannel( + handler, + connection, + scope, + localChannel, + remoteChannel, + maxPacketSize, + initialWindowSize, + ) } diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/DropbearCompatibilityTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/DropbearCompatibilityTest.kt index 7617ab41..c002cc29 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/DropbearCompatibilityTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/DropbearCompatibilityTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -219,6 +219,7 @@ class DropbearCompatibilityTest { fun `can connect with encryption algorithm`(algorithm: String) { authenticateWithPassword { encryptionAlgorithms = algorithm + macAlgorithms = "hmac-sha2-256" } } 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 8c84c6ce..03edf784 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/FakeSshServer.kt @@ -36,6 +36,7 @@ import org.connectbot.sshlib.crypto.SshPublicKeyEncoder import org.connectbot.sshlib.crypto.X25519ProviderFactory import org.connectbot.sshlib.crypto.encodeMpint import org.connectbot.sshlib.protocol.SshEnums +import org.connectbot.sshlib.protocol.SshMsgChannelData import org.connectbot.sshlib.protocol.SshMsgChannelFailure import org.connectbot.sshlib.protocol.SshMsgChannelOpen import org.connectbot.sshlib.protocol.SshMsgChannelOpenConfirmation @@ -85,6 +86,8 @@ class FakeSshServer( private val hostKeyPair: KeyPair = KeyPairGenerator.getInstance("Ed25519").generateKeyPair() private val hostKeyBlob: ByteArray = SshPublicKeyEncoder.encode(hostKeyPair.public, "ssh-ed25519") + val serverHostKeyBlob: ByteArray + get() = hostKeyBlob.copyOf() private lateinit var serverIo: PacketIO private lateinit var clientVersionStr: String @@ -100,6 +103,7 @@ class FakeSshServer( var advertisePing: Boolean = false var advertiseExtInfo: Boolean = false var kexAlgorithms: String? = null + var corruptKexSignature: Boolean = false private val receivedPongs = Channel(Channel.UNLIMITED) private val receivedExtInfo = Channel(Channel.UNLIMITED) private val receivedUserauthRequests = Channel(Channel.UNLIMITED) @@ -108,6 +112,7 @@ class FakeSshServer( private val receivedChannelRequests = Channel(Channel.UNLIMITED) private val receivedChannelOpenConfirmations = Channel(Channel.UNLIMITED) private val receivedChannelOpenFailures = Channel(Channel.UNLIMITED) + private val receivedChannelData = Channel(Channel.UNLIMITED) fun start(ignoreTransportErrors: Boolean = false) { scope.launch(coroutineContext) { @@ -252,6 +257,13 @@ class FakeSshServer( receivedChannelOpenFailures.trySend(failure) } + SshEnums.MessageType.SSH_MSG_CHANNEL_DATA -> { + val bodyBytes = rawBytes.copyOfRange(1, rawBytes.size) + val data = SshMsgChannelData(ByteBufferKaitaiStream(bodyBytes)) + data._read() + receivedChannelData.trySend(data) + } + SshEnums.MessageType.SSH_MSG_PING -> { val bodyBytes = rawBytes.copyOfRange(1, rawBytes.size) val pingMsg = SshMsgPing(ByteBufferKaitaiStream(bodyBytes)) @@ -352,8 +364,8 @@ class FakeSshServer( setServerHostKeyAlgorithms(createNameList("ssh-ed25519")) setEncryptionAlgorithmsClientToServer(createNameList("aes128-ctr")) setEncryptionAlgorithmsServerToClient(createNameList("aes128-ctr")) - setMacAlgorithmsClientToServer(createNameList("hmac-sha2-256")) - setMacAlgorithmsServerToClient(createNameList("hmac-sha2-256")) + setMacAlgorithmsClientToServer(createNameList("hmac-sha2-256-etm@openssh.com")) + setMacAlgorithmsServerToClient(createNameList("hmac-sha2-256-etm@openssh.com")) setCompressionAlgorithmsClientToServer(createNameList("none")) setCompressionAlgorithmsServerToClient(createNameList("none")) setLanguagesClientToServer(createNameList("")) @@ -399,7 +411,9 @@ class FakeSshServer( sessionId = exchangeHash } - val signature = signExchangeHash(exchangeHash) + val signature = signExchangeHash(exchangeHash).also { + if (corruptKexSignature) it[0] = (it[0].toInt() xor 1).toByte() + } val signatureBlob = buildSignatureBlob(signature) val reply = SshMsgKexEcdhReply().apply { @@ -419,7 +433,7 @@ class FakeSshServer( val keyDerivation = KeyDerivation(sharedSecret, exchangeHash, sid, "SHA-256") val cipherEntry = CipherEntry.fromSshName("aes128-ctr") ?: return - val macEntry = MacEntry.fromSshName("hmac-sha2-256") ?: return + val macEntry = MacEntry.fromSshName("hmac-sha2-256-etm@openssh.com") ?: return val keys = keyDerivation.deriveKeys( ivLength = cipherEntry.ivLength, @@ -449,6 +463,8 @@ class FakeSshServer( clientToServerMac = sendMac, serverToClientCipher = receiveCipher, serverToClientMac = receiveMac, + clientToServerEtm = true, + serverToClientEtm = true, ) } @@ -854,4 +870,6 @@ class FakeSshServer( suspend fun awaitChannelOpenConfirmation(): SshMsgChannelOpenConfirmation = receivedChannelOpenConfirmations.receive() suspend fun awaitChannelOpenFailure(): SshMsgChannelOpenFailure = receivedChannelOpenFailures.receive() + + suspend fun awaitChannelData(): SshMsgChannelData = receivedChannelData.receive() } diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ForwardingChannelTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ForwardingChannelTest.kt index 6dfe0850..055d8837 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ForwardingChannelTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ForwardingChannelTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ package org.connectbot.sshlib.client import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertArrayEquals import org.junit.jupiter.api.Assertions.assertEquals @@ -37,6 +39,7 @@ class ForwardingChannelTest { ): Pair { val channel = ForwardingChannel( connection = connection, + connectionScope = CoroutineScope(UnconfinedTestDispatcher()), localChannelNumber = 0, remoteChannelNumber = 1, maxPacketSize = maxPacketSize, @@ -63,6 +66,8 @@ class ForwardingChannelTest { val data = ByteArray(100) channel.onData(data) + coVerify(exactly = 0) { conn.sendWindowAdjust(any(), any()) } + channel.incomingData.receive() coVerify { conn.sendWindowAdjust(1, any()) } } @@ -109,6 +114,7 @@ class ForwardingChannelTest { val channel = ForwardingChannel( connection = conn, + connectionScope = CoroutineScope(UnconfinedTestDispatcher()), localChannelNumber = 0, remoteChannelNumber = 1, maxPacketSize = 10, diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/LocalChannelWindowTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/LocalChannelWindowTest.kt index 7452f164..6510e8e9 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/LocalChannelWindowTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/LocalChannelWindowTest.kt @@ -26,29 +26,23 @@ class LocalChannelWindowTest { @Test fun `consumeLocal rejects data exceeding window`() { - val w = LocalChannelWindow(initialSize = 1024, adjustThreshold = 0) + val w = LocalChannelWindow(initialSize = 1024) assertFailsWith { w.consumeLocal(1025) } } @Test fun `consumeLocal accepts data exactly filling window`() { - val w = LocalChannelWindow(initialSize = 1024, adjustThreshold = 0) - val adjust = w.consumeLocal(1024) - assertEquals(0, adjust) - } - - @Test - fun `consumeLocal returns adjust when below threshold`() { - val w = LocalChannelWindow(initialSize = 1024, adjustThreshold = 512) - val adjust = w.consumeLocal(600) - assertEquals(1024 - (1024 - 600), adjust) + val w = LocalChannelWindow(initialSize = 1024) + w.consumeLocal(1024) + assertFailsWith { w.consumeLocal(1) } } @Test - fun `consumeLocal returns 0 when above threshold`() { - val w = LocalChannelWindow(initialSize = 1024, adjustThreshold = 64) - val adjust = w.consumeLocal(100) - assertEquals(0, adjust) + fun `releaseLocal restores consumed capacity`() { + val w = LocalChannelWindow(initialSize = 1024) + w.consumeLocal(600) + assertEquals(600, w.releaseLocal(600)) + w.consumeLocal(1024) } @Test diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/PingConnectionTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/PingConnectionTest.kt index 6a176f1e..8610701a 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/PingConnectionTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/PingConnectionTest.kt @@ -114,6 +114,35 @@ class PingConnectionTest { } } + @Test + fun `opaque chaff pong does not terminate packet processing`() = 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, + rekeyIntervalMs = Long.MAX_VALUE, + rekeyBytesLimit = Long.MAX_VALUE, + coroutineDispatcher = dispatcher, + ) + + try { + assertIs(connectInBackground(connection, backgroundScope, dispatcher)) + + server.sendServerPong("PING!".encodeToByteArray()) + yield() + + assertIs(connection.ping()) + } finally { + connection.close() + } + } + @Test fun `client responds to server ping with pong containing same data`() = runTest { val dispatcher = StandardTestDispatcher(testScheduler) diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ProtocolIngressArchitectureTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ProtocolIngressArchitectureTest.kt new file mode 100644 index 00000000..241bd41d --- /dev/null +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/ProtocolIngressArchitectureTest.kt @@ -0,0 +1,34 @@ +/* + * ConnectBot SSH Library + * Copyright 2025-2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.connectbot.sshlib.client + +import java.lang.reflect.Modifier +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class ProtocolIngressArchitectureTest { + @Test + fun `packet receive entry point belongs to a private controller`() { + val controller = SshConnection::class.java.declaredClasses.single { it.simpleName == "InboundPacketController" } + + assertTrue(Modifier.isPrivate(controller.modifiers)) + assertTrue(controller.declaredMethods.any { it.name == "processNextPacket" }) + assertFalse(SshConnection::class.java.declaredMethods.any { it.name == "processNextPacket" }) + } +} diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SessionChannelTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SessionChannelTest.kt index c4df211d..9effc854 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SessionChannelTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SessionChannelTest.kt @@ -124,10 +124,23 @@ class SessionChannelTest { val (channel, conn) = createChannel(initialWindowSize = 128) channel.onData(ByteArray(100)) + coVerify(exactly = 0) { conn.sendWindowAdjust(any(), any()) } + channel.stdout.receive() coVerify { conn.sendWindowAdjust(1, any()) } } + @Test + fun `unconsumed data does not refill local window`() = runTest { + val (channel, _) = createChannel(initialWindowSize = 128) + + channel.onData(ByteArray(100)) + + assertFailsWith { + channel.onData(ByteArray(29)) + } + } + @Test fun `onExtendedData delivers data to stderr for type 1`() = runTest { val (channel, _) = createChannel() diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshClientIntegrationTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshClientIntegrationTest.kt index 0d13d799..4d41c937 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshClientIntegrationTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshClientIntegrationTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -492,7 +492,7 @@ class SshClientIntegrationTest { .bufferedReader().readText() @Test - fun `should connect to server only supporting ssh-rsa`() = runBlocking { + fun `explicit ssh-rsa wishlist enables SHA-1 RSA authentication`() = runBlocking { val host = opensshRsaOnlyContainer.host val port = opensshRsaOnlyContainer.getMappedPort(22) @@ -526,7 +526,7 @@ class SshClientIntegrationTest { } val result = withTimeout(10_000) { client.authenticate(USERNAME, handler) } - assertTrue(result is AuthResult.Success, "Should authenticate with ssh-rsa") + assertTrue(result is AuthResult.Success, "Should authenticate with explicitly enabled ssh-rsa") assertTrue(client.isAuthenticated, "Client should be authenticated") } finally { client.disconnect() diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshConnectionFlowTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshConnectionFlowTest.kt index 1b67a6ff..058f663c 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshConnectionFlowTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/SshConnectionFlowTest.kt @@ -30,12 +30,15 @@ import kotlinx.coroutines.test.runTest import kotlinx.coroutines.withTimeout import kotlinx.coroutines.yield import org.connectbot.sshlib.AgentIdentity +import org.connectbot.sshlib.AgentKeySpec import org.connectbot.sshlib.AgentProvider +import org.connectbot.sshlib.AgentResult import org.connectbot.sshlib.AgentSigningContext import org.connectbot.sshlib.AuthHandler import org.connectbot.sshlib.AuthPublicKey import org.connectbot.sshlib.AuthResult import org.connectbot.sshlib.ConnectResult +import org.connectbot.sshlib.DestinationConstraint import org.connectbot.sshlib.HostKeyVerifier import org.connectbot.sshlib.KeyboardInteractiveCallback import org.connectbot.sshlib.PublicKey @@ -48,6 +51,7 @@ import java.nio.file.Files import java.nio.file.Paths import kotlin.test.assertContentEquals import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertIs import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -60,6 +64,17 @@ class SshConnectionFlowTest { override suspend fun verify(key: PublicKey): Boolean = true } + @Test + fun `unsolicited authentication success is rejected as a protocol error`() = runTest { + connectedFixture { connection, server, dispatcher -> + val disconnected = async(dispatcher) { connection.disconnectedFlow.first() } + server.sendUserauthSuccess() + + val failure = assertNotNull(withTimeout(5_000) { disconnected.await() }) + assertTrue(failure.message.orEmpty().contains("Unexpected SSH packet SSH_MSG_USERAUTH_SUCCESS")) + } + } + @Test fun `connect returns host key rejected when verifier rejects server key`() = runTest { val dispatcher = StandardTestDispatcher(testScheduler) @@ -83,6 +98,35 @@ class SshConnectionFlowTest { } } + @Test + fun `host key verifier is not called before server proves key possession`() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val (clientTransport, serverTransport) = PipedTransport.create() + val server = FakeSshServer(serverTransport, backgroundScope, dispatcher).apply { + corruptKexSignature = true + } + server.start(ignoreTransportErrors = true) + var verifierCalled = false + val connection = SshConnection( + transport = clientTransport, + hostKeyVerifier = object : HostKeyVerifier { + override suspend fun verify(key: PublicKey): Boolean { + verifierCalled = true + return true + } + }, + coroutineDispatcher = dispatcher, + ) + + try { + val result = connectInBackground(connection, backgroundScope, dispatcher) + assertFalse(result is ConnectResult.Success) + assertFalse(verifierCalled) + } finally { + connection.close() + } + } + @Test fun `connect returns algorithm mismatch when kex negotiation has no match`() = runTest { val dispatcher = StandardTestDispatcher(testScheduler) @@ -327,13 +371,13 @@ class SshConnectionFlowTest { val exec = async(dispatcher) { session.requestExec("true") } val request = withTimeout(5_000) { server.awaitChannelRequest() } assertEquals("exec", request.requestType().value()) - server.sendChannelSuccess(request.recipientChannel().toInt()) + server.sendChannelSuccess(session.localChannelNumber) assertTrue(withTimeout(5_000) { exec.await() }) val shell = async(dispatcher) { session.requestShell() } val shellRequest = withTimeout(5_000) { server.awaitChannelRequest() } assertEquals("shell", shellRequest.requestType().value()) - server.sendChannelFailure(shellRequest.recipientChannel().toInt()) + server.sendChannelFailure(session.localChannelNumber) assertEquals(false, withTimeout(5_000) { shell.await() }) } } @@ -422,8 +466,8 @@ class SshConnectionFlowTest { connection.enableAgentForwarding( object : AgentProvider { - override suspend fun getIdentities(): List = emptyList() - override suspend fun signData(context: AgentSigningContext): ByteArray? = null + override suspend fun getIdentities() = AgentResult.Success(emptyList()) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) }, ) @@ -440,6 +484,56 @@ class SshConnectionFlowTest { } } + @Test + fun `incoming agent channel starts with the verified connection binding`() = runTest { + connectedFixture { connection, server, dispatcher -> + authenticate(connection, server, dispatcher) + + val currentHostKey = server.serverHostKeyBlob + val nextHostKey = byteArrayOf(9, 8, 7) + val constrainedIdentity = AgentIdentity( + publicKeyBlob = byteArrayOf(1, 2, 3), + comment = "constrained", + destinationConstraints = listOf( + DestinationConstraint( + fromHostname = "", + fromKeyspecs = emptyList(), + toUsername = "", + toHostname = "current", + toHostspecs = listOf(AgentKeySpec(currentHostKey, isCa = false)), + ), + DestinationConstraint( + fromHostname = "current", + fromKeyspecs = listOf(AgentKeySpec(currentHostKey, isCa = false)), + toUsername = "", + toHostname = "next", + toHostspecs = listOf(AgentKeySpec(nextHostKey, isCa = false)), + ), + ), + ) + connection.enableAgentForwarding( + object : AgentProvider { + override suspend fun getIdentities() = AgentResult.Success(listOf(constrainedIdentity)) + override suspend fun signData(context: AgentSigningContext) = AgentResult.Success(null) + }, + ) + + server.sendChannelOpen("auth-agent@openssh.com", senderChannel = 79) + val confirmation = withTimeout(5_000) { server.awaitChannelOpenConfirmation() } + val agentLocalChannel = confirmation.senderChannel().toInt() + server.sendChannelData(agentLocalChannel, ByteBuffer.allocate(5).putInt(1).put(11).array()) + + val responsePacket = withTimeout(5_000) { server.awaitChannelData() } + assertEquals(79, responsePacket.recipientChannel().toInt()) + val response = ByteBuffer.wrap(responsePacket.data().data()) + assertEquals(12, response.get(4).toInt() and 0xFF) + assertEquals(1, response.getInt(5)) + + server.sendChannelClose(agentLocalChannel) + yield() + } + } + @Test fun `tcpip forward request handles assigned port success fixed port success and failure`() = runTest { connectedFixture { connection, server, dispatcher -> diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/StreamForwarderTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/StreamForwarderTest.kt index 797de634..509780ca 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/StreamForwarderTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/StreamForwarderTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,7 +42,7 @@ class StreamForwarderTest { fun `start forwards data from read channel to ssh`() = runTest { val connection = mockk(relaxed = true) coEvery { connection.openDirectTcpipChannel("example.com", 80, "127.0.0.1", 0) } returns - ForwardingChannel(connection, 0, 1, 32 * 1024, 256L * 1024) + ForwardingChannel(connection, backgroundScope, 0, 1, 32 * 1024, 256L * 1024) val input = ByteChannel(autoFlush = true) val output = ByteChannel(autoFlush = true) @@ -87,7 +87,7 @@ class StreamForwarderTest { fun `stop is idempotent`() = runTest { val connection = mockk(relaxed = true) coEvery { connection.openDirectTcpipChannel("example.com", 80, "127.0.0.1", 0) } returns - ForwardingChannel(connection, 0, 1, 32 * 1024, 256L * 1024) + ForwardingChannel(connection, backgroundScope, 0, 1, 32 * 1024, 256L * 1024) val input = ByteChannel(autoFlush = true) val output = ByteChannel(autoFlush = true) diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImplTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImplTest.kt index b7a0f19a..b9732e90 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImplTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/client/sftp/SftpClientImplTest.kt @@ -282,6 +282,63 @@ class SftpClientImplTest { } } + @Test + fun `oversized HANDLE and DATA strings return protocol errors`() = runBlocking { + val maliciousLength = ByteBuffer.allocate(4).putInt(Int.MAX_VALUE).array() + val client = createClient( + FakeSshSession(responseFor = { type, _ -> + when (type) { + SSH_FXP_OPEN -> response(SSH_FXP_HANDLE, maliciousLength) + SSH_FXP_READ -> response(SSH_FXP_DATA, maliciousLength) + else -> response(SSH_FXP_STATUS, statusPayload(SftpStatusCode.OK)) + } + }), + ) + + assertIs(client.open("/file", setOf(SftpOpenFlag.READ))) + assertIs(client.read(SftpFileHandle(byteArrayOf(1)), 0, 1)) + Unit + } + + @Test + fun `oversized STATUS message returns protocol error`() = runBlocking { + val payload = ByteBuffer.allocate(8) + .putInt(SftpStatusCode.FAILURE.code) + .putInt(Int.MAX_VALUE) + .array() + val client = createClient( + FakeSshSession(responseFor = { _, _ -> response(SSH_FXP_STATUS, payload) }), + ) + + assertIs(client.remove("/file")) + Unit + } + + @Test + fun `excessive NAME count returns protocol error`() = runBlocking { + val payload = ByteBuffer.allocate(4).putInt(Int.MAX_VALUE).array() + val client = createClient( + FakeSshSession(responseFor = { _, _ -> response(SSH_FXP_NAME, payload) }), + ) + + assertIs(client.readdir(SftpFileHandle(byteArrayOf(1)))) + Unit + } + + @Test + fun `malformed extended attributes return protocol error`() = runBlocking { + val payload = ByteBuffer.allocate(8) + .putInt(0x80000000.toInt()) + .putInt(Int.MAX_VALUE) + .array() + val client = createClient( + FakeSshSession(responseFor = { _, _ -> response(SSH_FXP_ATTRS, payload) }), + ) + + assertIs(client.stat("/file")) + Unit + } + private suspend fun createClient(session: FakeSshSession): SftpClient { session.enqueueRead(packet(SSH_FXP_VERSION, ByteBuffer.allocate(4).putInt(3).array())) return assertSuccess(SftpClientImpl.create(session)) diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/AlgorithmsTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/AlgorithmsTest.kt index 8b8e5da8..17744f48 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/AlgorithmsTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/AlgorithmsTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -122,13 +122,17 @@ class AlgorithmsTest { } @Test - fun `CipherEntry defaults contain all entries`() { - for (entry in CipherEntry.entries) { - assertTrue( - CipherEntry.defaultString.contains(entry.sshName), - "${entry.sshName} not found in defaultString", - ) - } + fun `CipherEntry defaults contain only modern ciphers`() { + assertEquals( + listOf( + CipherEntry.CHACHA20_POLY1305, + CipherEntry.AES128_GCM, + CipherEntry.AES256_GCM, + CipherEntry.AES128_CTR, + CipherEntry.AES256_CTR, + ), + CipherEntry.defaults, + ) } // MacEntry lookup @@ -187,13 +191,11 @@ class AlgorithmsTest { } @Test - fun `MacEntry defaults contain all entries`() { - for (entry in MacEntry.entries) { - assertTrue( - MacEntry.defaultString.contains(entry.sshName), - "${entry.sshName} not found in defaultString", - ) - } + fun `MacEntry defaults contain only SHA2 ETM`() { + assertEquals( + listOf(MacEntry.HMAC_SHA2_256_ETM, MacEntry.HMAC_SHA2_512_ETM), + MacEntry.defaults, + ) } // KexEntry lookup @@ -258,9 +260,11 @@ class AlgorithmsTest { } @Test - fun `KexEntry defaults exclude DH_GROUP1_SHA1`() { + fun `KexEntry defaults exclude SHA1`() { + assertFalse(KexEntry.defaults.contains(KexEntry.DH_GROUP14_SHA1)) + assertFalse(KexEntry.defaults.contains(KexEntry.DH_GROUP_EXCHANGE_SHA1)) assertFalse(KexEntry.defaults.contains(KexEntry.DH_GROUP1_SHA1)) - assertFalse(KexEntry.defaultString.contains("diffie-hellman-group1-sha1")) + assertTrue(KexEntry.defaults.none { it.hashAlgorithm == "SHA-1" }) } @Test @@ -324,46 +328,53 @@ class AlgorithmsTest { } @Test - fun `SignatureEntry defaults contain all entries`() { - for (entry in SignatureEntry.entries) { - assertTrue( - SignatureEntry.defaultString.contains(entry.sshName), - "${entry.sshName} not found in defaultString", - ) - } + fun `SignatureEntry defaults exclude SHA1 ssh-rsa`() { + assertFalse(SignatureEntry.defaults.contains(SignatureEntry.SSH_RSA)) + assertFalse(SignatureEntry.defaultString.split(",").contains("ssh-rsa")) + } + + @Test + fun `negotiateRsaAlgorithm skips RSA when serverSigAlgs is null`() { + assertNull(SignatureEntry.negotiateRsaAlgorithm(null, setOf("rsa-sha2-512", "rsa-sha2-256"))) } @Test - fun `negotiateRsaAlgorithm returns ssh-rsa when serverSigAlgs is null`() { - assertEquals("ssh-rsa", SignatureEntry.negotiateRsaAlgorithm(null)) + fun `negotiateRsaAlgorithm uses explicitly allowed ssh-rsa when serverSigAlgs is null`() { + assertEquals("ssh-rsa", SignatureEntry.negotiateRsaAlgorithm(null, setOf("ssh-rsa"))) } @Test fun `negotiateRsaAlgorithm prefers rsa-sha2-512 when server supports it`() { val serverAlgs = setOf("rsa-sha2-256", "rsa-sha2-512", "ssh-ed25519") - assertEquals("rsa-sha2-512", SignatureEntry.negotiateRsaAlgorithm(serverAlgs)) + assertEquals("rsa-sha2-512", SignatureEntry.negotiateRsaAlgorithm(serverAlgs, setOf("rsa-sha2-256", "rsa-sha2-512"))) } @Test fun `negotiateRsaAlgorithm falls back to rsa-sha2-256 when 512 not supported`() { val serverAlgs = setOf("rsa-sha2-256", "ssh-ed25519") - assertEquals("rsa-sha2-256", SignatureEntry.negotiateRsaAlgorithm(serverAlgs)) + assertEquals("rsa-sha2-256", SignatureEntry.negotiateRsaAlgorithm(serverAlgs, setOf("rsa-sha2-256", "rsa-sha2-512"))) + } + + @Test + fun `negotiateRsaAlgorithm accepts explicitly allowed ssh-rsa advertisement`() { + val serverAlgs = setOf("ssh-rsa", "ssh-ed25519") + assertEquals("ssh-rsa", SignatureEntry.negotiateRsaAlgorithm(serverAlgs, setOf("ssh-rsa"))) } @Test - fun `negotiateRsaAlgorithm falls back to ssh-rsa as last resort`() { + fun `negotiateRsaAlgorithm skips ssh-rsa when it is not allowed`() { val serverAlgs = setOf("ssh-rsa", "ssh-ed25519") - assertEquals("ssh-rsa", SignatureEntry.negotiateRsaAlgorithm(serverAlgs)) + assertNull(SignatureEntry.negotiateRsaAlgorithm(serverAlgs, setOf("rsa-sha2-512", "rsa-sha2-256"))) } @Test - fun `negotiateRsaAlgorithm returns ssh-rsa when no RSA algorithm in server list`() { + fun `negotiateRsaAlgorithm skips when no RSA algorithm is advertised`() { val serverAlgs = setOf("ssh-ed25519", "ecdsa-sha2-nistp256") - assertEquals("ssh-rsa", SignatureEntry.negotiateRsaAlgorithm(serverAlgs)) + assertNull(SignatureEntry.negotiateRsaAlgorithm(serverAlgs, setOf("rsa-sha2-512", "rsa-sha2-256"))) } @Test - fun `negotiateRsaAlgorithm returns ssh-rsa when serverSigAlgs is empty`() { - assertEquals("ssh-rsa", SignatureEntry.negotiateRsaAlgorithm(emptySet())) + fun `negotiateRsaAlgorithm skips when serverSigAlgs is empty`() { + assertNull(SignatureEntry.negotiateRsaAlgorithm(emptySet(), setOf("rsa-sha2-512", "rsa-sha2-256", "ssh-rsa"))) } } 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 6d7457cd..4f9ae8ab 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchangeTest.kt @@ -88,6 +88,16 @@ class DiffieHellmanGroupExchangeTest { } } + @Test + fun `setGroup rejects odd composite modulus`() { + val composite = BigInteger.ONE.shiftLeft(2047).add(BigInteger.ONE).multiply(BigInteger.valueOf(3)) + val gex = DiffieHellmanGroupExchange("SHA-256") + + assertFailsWith { + gex.setGroup(composite, DhGroups.GENERATOR) + } + } + @Test fun `setGroup accepts valid group 14 parameters`() { val gex = DiffieHellmanGroupExchange("SHA-256") @@ -147,6 +157,29 @@ class DiffieHellmanGroupExchangeTest { assertContentEquals(serverSecret, clientSecret) } + @Test + fun `failed server public value discards ephemeral private exponent`() { + val gex = DiffieHellmanGroupExchange("SHA-256") + gex.setGroup(DhGroups.GROUP14_P, DhGroups.GENERATOR) + gex.generateClientKeys() + + assertFailsWith { gex.computeSharedSecret(BigInteger.ONE.toByteArray()) } + assertFailsWith { gex.computeSharedSecret(BigInteger.TWO.toByteArray()) } + } + + @Test + fun `validated group cache remains bounded`() { + 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 { gex.setGroup(composite, DhGroups.GENERATOR) } + } + + assertTrue(DiffieHellmanGroupExchange.cachedGroupValidationCount() <= 16) + } + @Test fun `exchange hash includes min n max p g`() { val p = DhGroups.GROUP14_P diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/PemKeyWriterTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/PemKeyWriterTest.kt index 6fcd72b5..3f0fbb42 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/PemKeyWriterTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/PemKeyWriterTest.kt @@ -1,6 +1,6 @@ /* * ConnectBot SSH Library - * Copyright 2025 Kenny Root + * Copyright 2025-2026 Kenny Root * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,11 +17,13 @@ package org.connectbot.sshlib.crypto +import org.connectbot.sshlib.SshException import org.junit.jupiter.api.Test import java.security.interfaces.ECPublicKey import java.security.interfaces.RSAPublicKey import java.util.Base64 import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertTrue class PemKeyWriterTest { @@ -66,6 +68,36 @@ class PemKeyWriterTest { roundTrip("ed25519_unencrypted") } + @Test + fun `round-trip Ed25519 encrypted PKCS8`() { + val original = PrivateKeyReader.read(readKey("ed25519_unencrypted")).jcaKeyPair + val encoded = PemKeyWriter.write(original, "testpass") + + assertTrue(encoded.contains("-----BEGIN ENCRYPTED PRIVATE KEY-----")) + assertTrue(PrivateKeyReader.isEncrypted(encoded)) + val decoded = PrivateKeyReader.read(encoded, "testpass").jcaKeyPair + assertEquals(original.public, decoded.public) + } + + @Test + fun `encrypted Ed25519 PKCS8 rejects missing and incorrect passwords`() { + val original = PrivateKeyReader.read(readKey("ed25519_unencrypted")).jcaKeyPair + val encoded = PemKeyWriter.write(original, "testpass") + + assertFailsWith { PrivateKeyReader.read(encoded) } + assertFailsWith { PrivateKeyReader.read(encoded, "wrongpass") } + } + + @Test + fun `encrypted Ed25519 PKCS8 uses fresh salt and IV`() { + val original = PrivateKeyReader.read(readKey("ed25519_unencrypted")).jcaKeyPair + + val first = PemKeyWriter.write(original, "testpass") + val second = PemKeyWriter.write(original, "testpass") + + assertTrue(first != second) + } + @Test fun `round-trip RSA encrypted`() { val original = PrivateKeyReader.read(readKey("rsa_unencrypted")).jcaKeyPair diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/SignatureVerifierTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/SignatureVerifierTest.kt index bac83a21..b18b60a8 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/SignatureVerifierTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/crypto/SignatureVerifierTest.kt @@ -165,6 +165,30 @@ class SignatureVerifierTest { assertFalse(SignatureVerifier.verify(hostKey, sigBlob, data, "unknown-algo")) } + @Test + fun `rejects RSA key blob for a negotiated Ed25519 algorithm`() { + val kp = KeyPairGenerator.getInstance("RSA").apply { initialize(2048) }.generateKeyPair() + val data = "exchange hash".toByteArray() + val hostKey = buildRsaHostKey(kp.public as RSAPublicKey) + val sigBlob = buildSignatureBlob("ssh-ed25519", ByteArray(64)) + + assertFalse(SignatureVerifier.verify(hostKey, sigBlob, data, "ssh-ed25519")) + } + + @Test + fun `rejects ECDSA key whose inner curve disagrees with negotiated algorithm`() { + val privateKey = readKey("ecdsa256_unencrypted") + val data = "exchange hash".toByteArray() + val hostKey = SshPublicKeyEncoder.encode(privateKey.jcaKeyPair, "ecdsa-sha2-nistp256") + val mismatchedOuterName = hostKey.copyOf().also { + val name = "ecdsa-sha2-nistp384".toByteArray(Charsets.US_ASCII) + name.copyInto(it, 4) + } + val sigBlob = signWithSshAlgorithm(privateKey, "ecdsa-sha2-nistp384", data) + + assertFalse(SignatureVerifier.verify(mismatchedOuterName, sigBlob, data, "ecdsa-sha2-nistp384")) + } + @Test fun `verifyWithKeyType accepts RSA-compatible signature algorithms`() { val privateKey = readKey("rsa_unencrypted") diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachineTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachineTest.kt new file mode 100644 index 00000000..fa933336 --- /dev/null +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/protocol/SshClientStateMachineTest.kt @@ -0,0 +1,189 @@ +/* + * ConnectBot SSH Library + * Copyright 2025-2026 Kenny Root + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.connectbot.sshlib.protocol + +import kotlinx.coroutines.test.runTest +import java.lang.reflect.Modifier +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class SshClientStateMachineTest { + @Test + fun `network events cannot bypass authentication guards`() = runTest { + val callbacks = RecordingCallbacks() + val machine = SshClientStateMachine(callbacks) + + assertFalse(machine.authenticationSuccess()) + assertFalse(machine.authenticationFailure()) + assertFalse(machine.authorizeAuthenticationPacket()) + assertFalse(machine.authorizeAuthenticatedPacket()) + assertFalse(machine.openChannel("session", 0, 1024, 1024)) + assertFalse("authenticationSuccess" in callbacks.actions) + assertFalse("sendChannelOpen" in callbacks.actions) + } + + @Test + fun `authentication success is an explicit state transition`() = runTest { + val callbacks = RecordingCallbacks() + val machine = SshClientStateMachine(callbacks) + + assertTrue(machine.connect()) + assertTrue(machine.receiveVersion(IdBanner())) + assertTrue(machine.receiveKexInit(SshMsgKexinit())) + assertTrue(machine.receiveKexDhReply(SshMsgKexdhReply())) + assertTrue(machine.receiveNewKeys()) + assertTrue(machine.receiveServiceAccept("ssh-userauth")) + + assertFalse(machine.authorizeAuthenticationPacket()) + assertFalse(machine.authorizeAuthenticatedPacket()) + assertFalse(machine.authenticationSuccess()) + assertTrue(machine.beginAuthentication()) + assertTrue(machine.authorizeAuthenticationPacket()) + assertTrue(machine.authenticationSuccess()) + assertFalse(machine.authenticationSuccess()) + assertFalse(machine.authorizeAuthenticationPacket()) + assertTrue(machine.authorizeAuthenticatedPacket()) + assertTrue(machine.openChannel("session", 0, 1024, 1024)) + } + + @Test + fun `raw KStateMachine and event hierarchy are private`() { + val machineField = SshClientStateMachine::class.java.getDeclaredField("stateMachine") + assertTrue(Modifier.isPrivate(machineField.modifiers)) + + val eventClass = SshClientStateMachine::class.java.declaredClasses.single { it.simpleName == "SshEvent" } + assertTrue(Modifier.isPrivate(eventClass.modifiers)) + val processMethod = SshClientStateMachine::class.java.declaredMethods.single { it.name == "process" } + assertTrue(Modifier.isPrivate(processMethod.modifiers)) + } + + private class RecordingCallbacks : SshClientCallbacks { + val actions = mutableListOf() + + override fun sendVersion() { + actions += "sendVersion" + } + override fun receiveVersion(banner: IdBanner) { + actions += "receiveVersion" + } + override suspend fun sendKexInit() { + actions += "sendKexInit" + } + override fun receiveKexInit(msg: SshMsgKexinit) { + actions += "receiveKexInit" + } + override suspend fun sendKexExchangeInit() { + actions += "sendKexExchangeInit" + } + override suspend fun receiveKexDhReply(msg: SshMsgKexdhReply) { + actions += "receiveKexDhReply" + } + override suspend fun receiveKexEcdhReply(msg: SshMsgKexEcdhReply) { + actions += "receiveKexEcdhReply" + } + override suspend fun receiveKexDhGexReply(msg: SshMsgKexDhGexReply) { + actions += "receiveKexDhGexReply" + } + override fun isRekeying(): Boolean = false + override fun rekeyStarted() { + actions += "rekeyStarted" + } + override fun rekeyComplete() { + actions += "rekeyComplete" + } + override suspend fun sendKexDhGexInit() { + actions += "sendKexDhGexInit" + } + override suspend fun sendNewKeys() { + actions += "sendNewKeys" + } + override fun receiveNewKeys() { + actions += "receiveNewKeys" + } + override fun activateEncryption() { + actions += "activateEncryption" + } + override suspend fun sendClientExtInfo() { + actions += "sendClientExtInfo" + } + override suspend fun sendServiceRequest(service: String) { + actions += "sendServiceRequest" + } + override fun receiveServiceAccept(service: String) { + actions += "receiveServiceAccept" + } + override fun startAuthentication() { + actions += "startAuthentication" + } + override fun authenticationSuccess() { + actions += "authenticationSuccess" + } + override fun authenticationFailure() { + actions += "authenticationFailure" + } + override fun receiveUserauthInfoRequest(msg: SshMsgUserauthInfoRequest) { + actions += "receiveUserauthInfoRequest" + } + override fun receiveUserauthBanner(msg: SshMsgUserauthBanner) { + actions += "receiveUserauthBanner" + } + override suspend fun sendChannelOpen( + channelType: String, + localChannelNumber: Int, + initialWindowSize: Int, + maxPacketSize: Int, + ) { + actions += "sendChannelOpen" + } + override fun receiveChannelOpenConfirmation(msg: SshMsgChannelOpenConfirmation) { + actions += "receiveChannelOpenConfirmation" + } + override fun receiveChannelOpenFailure(msg: SshMsgChannelOpenFailure) { + actions += "receiveChannelOpenFailure" + } + override suspend fun sendChannelRequest( + recipientChannel: Int, + requestType: String, + wantReply: Boolean, + message: SshMsgChannelRequest, + ) { + actions += "sendChannelRequest" + } + override fun receiveChannelSuccess(recipientChannel: Int) { + actions += "receiveChannelSuccess" + } + override fun receiveChannelFailure(recipientChannel: Int) { + actions += "receiveChannelFailure" + } + override suspend fun receiveGlobalRequest(msg: SshMsgGlobalRequest) { + actions += "receiveGlobalRequest" + } + override fun debug(msg: SshMsgDebug) { + actions += "debug" + } + override fun ignore() { + actions += "ignore" + } + override suspend fun disconnect() { + actions += "disconnect" + } + override fun onStateEnter(stateName: String) = Unit + override fun onStateExit(stateName: String) = Unit + } +} diff --git a/sshlib/src/test/kotlin/org/connectbot/sshlib/transport/PacketIOTest.kt b/sshlib/src/test/kotlin/org/connectbot/sshlib/transport/PacketIOTest.kt index 32126727..24cb12a9 100644 --- a/sshlib/src/test/kotlin/org/connectbot/sshlib/transport/PacketIOTest.kt +++ b/sshlib/src/test/kotlin/org/connectbot/sshlib/transport/PacketIOTest.kt @@ -18,17 +18,46 @@ package org.connectbot.sshlib.transport import kotlinx.coroutines.runBlocking +import org.connectbot.sshlib.crypto.AeadResult import org.connectbot.sshlib.crypto.AesCbcCipher import org.connectbot.sshlib.crypto.HmacSha256 +import org.connectbot.sshlib.crypto.PacketAead import org.connectbot.sshlib.protocol.SshEnums import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.nio.ByteBuffer +import java.security.SecureRandom class PacketIOTest { + private class TrackingSecureRandom : SecureRandom() { + var requestedBytes = 0 + + override fun nextBytes(bytes: ByteArray) { + requestedBytes += bytes.size + bytes.fill(0x5a) + } + } + + private class TrackingAead : PacketAead { + var destroyed = false + + override val tagLength: Int = 16 + + override fun encrypt(packetLength: ByteArray, plaintext: ByteArray): AeadResult = AeadResult(plaintext.copyOf(), ByteArray(tagLength)) + + override fun decrypt(packetLength: ByteArray, ciphertext: ByteArray, tag: ByteArray): ByteArray = ciphertext.copyOf() + + override fun destroy() { + destroyed = true + } + + override fun isDestroyed(): Boolean = destroyed + } + @Test fun `unencrypted round trip works`() = runBlocking { val transport = ByteArrayTransport() @@ -141,6 +170,27 @@ class PacketIOTest { io.resetReceiveSequenceNumber() } + @Test + fun `outbound and inbound AEAD keys are replaced independently`() { + val io = PacketIO(ByteArrayTransport()) + val oldOutbound = TrackingAead() + val oldInbound = TrackingAead() + val newOutbound = TrackingAead() + val newInbound = TrackingAead() + io.enableAead(oldOutbound, oldInbound) + + io.enableSendAead(newOutbound) + + assertTrue(oldOutbound.destroyed) + assertFalse(oldInbound.destroyed) + + io.enableReceiveAead(newInbound) + + assertTrue(oldInbound.destroyed) + assertFalse(newOutbound.destroyed) + assertFalse(newInbound.destroyed) + } + @Test fun `bytesSentOnWire tracks unencrypted write`() = runBlocking { val transport = ByteArrayTransport() @@ -243,6 +293,20 @@ class PacketIOTest { } } + @Test + fun `packet padding comes from cryptographically secure random source`() = runBlocking { + val transport = ByteArrayTransport() + val random = TrackingSecureRandom() + val io = PacketIO(transport, random) + + io.writePacket(SshEnums.MessageType.SSH_MSG_IGNORE.id().toInt()) + + val wire = transport.getWrittenData() + val paddingLength = wire[4].toInt() and 0xFF + assertEquals(paddingLength, random.requestedBytes) + assertTrue(wire.takeLast(paddingLength).all { it == 0x5a.toByte() }) + } + @Test fun `calculatePaddingLength result aligns total wire packet to multiple of 8`() { // totalLength in RFC 4253 = 4(length field) + 1(padding_length) + 1(msg_type) + payloadSize + padding