diff --git a/.gitignore b/.gitignore index b0c9219..9f1237d 100644 --- a/.gitignore +++ b/.gitignore @@ -107,3 +107,6 @@ fabric.properties # Android projects local.properties + +# Git worktrees +.worktrees/ 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 bca18d9..3fde5a7 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/client/SshConnection.kt @@ -1143,7 +1143,7 @@ class SshConnection( ?: throw SshException("Exchange hash computation failed") if (sessionId == null) { - sessionId = hash + sessionId = hash.copyOf() } val keyType = try { @@ -1178,6 +1178,13 @@ class SshConnection( throw SshException("Server signature verification failed") } logger.info("Server signature verified") + + clientKexInit?.fill(0) + clientKexInit = null + serverKexInit?.fill(0) + serverKexInit = null + clientPublicKey?.fill(0) + clientPublicKey = null } private suspend fun receiveKexDhGexReply(msg: SshMsgKexDhGexReply) { @@ -1225,6 +1232,13 @@ class SshConnection( 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) { @@ -1272,11 +1286,19 @@ class SshConnection( val c2sIv = if (entryC2S.ivLength > 0) keys.initialIvClientToServer.copyOf(entryC2S.ivLength) else ByteArray(0) val s2cIv = if (entryS2C.ivLength > 0) keys.initialIvServerToClient.copyOf(entryS2C.ivLength) else ByteArray(0) - val c2sAead = (entryC2S.create(c2sKey, c2sIv, true) as EncryptionInstance.Aead).aead - val s2cAead = (entryS2C.create(s2cKey, s2cIv, false) as EncryptionInstance.Aead).aead + 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) + writeMutex.withLock { + packetIO.enableAead(c2sAead, s2cAead) + } + } finally { + keys.zeroize() + c2sKey.fill(0) + s2cKey.fill(0) + c2sIv.fill(0) + s2cIv.fill(0) } } @@ -1320,25 +1342,32 @@ class SshConnection( val c2sCipherKey = keys.encryptionKeyClientToServer.copyOf(cipherEntryC2S.keyLength) val s2cCipherKey = keys.encryptionKeyServerToClient.copyOf(cipherEntryS2C.keyLength) - val c2sIv = keys.initialIvClientToServer.copyOf(cipherEntryC2S.ivLength) val s2cIv = keys.initialIvServerToClient.copyOf(cipherEntryS2C.ivLength) - 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( - clientToServerCipher, - clientToServerMac, - serverToClientCipher, - serverToClientMac, - clientToServerEtm = macEntryC2S.isEtm, - serverToClientEtm = macEntryS2C.isEtm, - ) + 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( + clientToServerCipher, + clientToServerMac, + serverToClientCipher, + serverToClientMac, + clientToServerEtm = macEntryC2S.isEtm, + serverToClientEtm = macEntryS2C.isEtm, + ) + } + } finally { + keys.zeroize() + c2sCipherKey.fill(0) + s2cCipherKey.fill(0) + c2sIv.fill(0) + s2cIv.fill(0) } } @@ -1568,6 +1597,8 @@ class SshConnection( packetLoopJob?.join() packetLoopJob = null stateMachineDispatcher.close() + sessionId?.fill(0) + sessionId = null } private fun onStateEnter(stateName: String) { diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/AesGcmCipher.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/AesGcmCipher.kt index 65f4619..fe3c07c 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/AesGcmCipher.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/AesGcmCipher.kt @@ -35,7 +35,7 @@ import javax.crypto.spec.SecretKeySpec * @param iv Initial 12-byte IV from key derivation */ internal class AesGcmCipher( - key: ByteArray, + private val key: ByteArray, private val iv: ByteArray, ) : PacketAead { override val tagLength: Int = 16 @@ -93,4 +93,10 @@ internal class AesGcmCipher( throw TransportException("AEAD authentication failed", e) } } + + override fun destroy() { + key.fill(0) + iv.fill(0) + fixedField.fill(0) + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/ChaCha20Poly1305Cipher.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/ChaCha20Poly1305Cipher.kt index e098cac..b9cedd9 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/ChaCha20Poly1305Cipher.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/ChaCha20Poly1305Cipher.kt @@ -29,7 +29,7 @@ import javax.crypto.spec.SecretKeySpec * * The Poly1305 tag covers encrypted_length || ciphertext. */ -internal class ChaCha20Poly1305Cipher(key: ByteArray) : PacketAead { +internal class ChaCha20Poly1305Cipher(private val key: ByteArray) : PacketAead { companion object { const val KEY_SIZE = 64 const val TAG_SIZE = 16 @@ -160,6 +160,11 @@ internal class ChaCha20Poly1305Cipher(key: ByteArray) : PacketAead { return plaintext } + override fun destroy() { + key.fill(0) + nonce.fill(0) + } + private fun constantTimeEquals(a: ByteArray, b: ByteArray): Boolean { if (a.size != b.size) return false var result = 0 diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Curve25519KeyExchange.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Curve25519KeyExchange.kt index 53ad470..8f65380 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Curve25519KeyExchange.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/Curve25519KeyExchange.kt @@ -29,9 +29,12 @@ internal class Curve25519KeyExchange( private var clientPrivate: ByteArray? = null override fun generateClientKeys(): ByteArray { - clientPrivate = presetPrivateKey?.clone() ?: x25519Provider.generatePrivateKey() + val clientPrivate = presetPrivateKey?.clone() ?: x25519Provider.generatePrivateKey() + + this.clientPrivate = clientPrivate + return try { - x25519Provider.publicFromPrivate(clientPrivate!!) + x25519Provider.publicFromPrivate(clientPrivate) } catch (e: Exception) { throw SshException("Failed to generate X25519 public key", e) } @@ -68,4 +71,9 @@ internal class Curve25519KeyExchange( throw SshException("X25519 key agreement failed", e) } } + + override fun zeroize() { + clientPrivate?.fill(0) + clientPrivate = null + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellman.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellman.kt index 49f1811..6cb7977 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellman.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellman.kt @@ -52,6 +52,14 @@ internal class DiffieHellman( throw SshException("Invalid server public key") } - return encodeMpint(f.modPow(x, p).toByteArray()) + val sharedSecret = encodeMpint(f.modPow(x, p).toByteArray()) + + privateKey = null + + return sharedSecret + } + + override fun zeroize() { + privateKey = null } } 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 9ef087f..00736b1 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/DiffieHellmanGroupExchange.kt @@ -66,7 +66,11 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String) : throw SshException("Invalid server public key") } - return encodeMpint(f.modPow(x, p).toByteArray()) + val sharedSecret = encodeMpint(f.modPow(x, p).toByteArray()) + + privateKey = null + + return sharedSecret } /** @@ -130,4 +134,10 @@ internal class DiffieHellmanGroupExchange(override val hashAlgorithm: String) : val md = MessageDigest.getInstance(hashAlgorithm) return md.digest(transcript.toByteArray()) } + + override fun zeroize() { + privateKey = null + p = null + g = null + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/EcdhKeyExchange.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/EcdhKeyExchange.kt index dfe5874..0ba4bdc 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/EcdhKeyExchange.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/EcdhKeyExchange.kt @@ -27,6 +27,8 @@ import java.security.spec.ECParameterSpec import java.security.spec.ECPoint import java.security.spec.ECPublicKeySpec import javax.crypto.KeyAgreement +import javax.security.auth.DestroyFailedException +import javax.security.auth.Destroyable /** * Elliptic Curve Diffie-Hellman key exchange as specified in RFC 5656 Section 4. @@ -130,4 +132,13 @@ internal class EcdhKeyExchange(private val curveName: String) : KexAlgorithm { val y = BigInteger(1, encoded, 1 + fieldSize, fieldSize) return ECPoint(x, y) } + + override fun zeroize() { + val kp = clientKeyPair + clientKeyPair = null + try { + (kp?.private as? Destroyable)?.destroy() + } catch (_: DestroyFailedException) { + } + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha1.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha1.kt index 6a08505..c18fbe8 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha1.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha1.kt @@ -67,4 +67,8 @@ internal class HmacSha1(private val key: ByteArray) : PacketMac { mac.update(etmMac.toByteArray()) return mac.doFinal() } + + override fun destroy() { + key.fill(0) + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha256.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha256.kt index 3da7ddc..2a9dcec 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha256.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha256.kt @@ -67,4 +67,8 @@ internal class HmacSha256(private val key: ByteArray) : PacketMac { mac.update(etmMac.toByteArray()) return mac.doFinal() } + + override fun destroy() { + key.fill(0) + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha512.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha512.kt index df77747..952998c 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha512.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/HmacSha512.kt @@ -67,4 +67,8 @@ internal class HmacSha512(private val key: ByteArray) : PacketMac { mac.update(etmMac.toByteArray()) return mac.doFinal() } + + override fun destroy() { + key.fill(0) + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KexAlgorithm.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KexAlgorithm.kt index 96a1d1f..60a4105 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KexAlgorithm.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KexAlgorithm.kt @@ -41,6 +41,14 @@ internal interface KexAlgorithm { */ fun computeSharedSecret(serverPublicKey: ByteArray): ByteArray + /** + * Zeroize any private key material stored during key exchange. + * + * This method should be called after the key exchange is complete + * (after NEWKEYS) to ensure no secret material remains in memory. + */ + fun zeroize() + /** * Compute exchange hash H over the key exchange transcript. * diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyDerivation.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyDerivation.kt index 84123b6..7c5aa56 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyDerivation.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/KeyDerivation.kt @@ -43,7 +43,16 @@ internal class KeyDerivation( val encryptionKeyServerToClient: ByteArray, val integrityKeyClientToServer: ByteArray, val integrityKeyServerToClient: ByteArray, - ) + ) { + fun zeroize() { + initialIvClientToServer.fill(0) + initialIvServerToClient.fill(0) + encryptionKeyClientToServer.fill(0) + encryptionKeyServerToClient.fill(0) + integrityKeyClientToServer.fill(0) + integrityKeyServerToClient.fill(0) + } + } /** * Derive all keys needed for SSH connection. @@ -88,22 +97,27 @@ internal class KeyDerivation( md.update(sessionId) var key = md.digest() - val toCopy = minOf(key.size, length - offset) - System.arraycopy(key, 0, result, offset, toCopy) - offset += toCopy + try { + val toCopy = minOf(key.size, length - offset) + System.arraycopy(key, 0, result, offset, toCopy) + offset += toCopy - // Kn = HASH(K || H || K1 || K2 || ... || Kn-1) - while (offset < length) { - md.reset() - md.update(sharedSecret) - md.update(exchangeHash) - md.update(result, 0, offset) - key = md.digest() + // Kn = HASH(K || H || K1 || K2 || ... || Kn-1) + while (offset < length) { + key.fill(0) + md.reset() + md.update(sharedSecret) + md.update(exchangeHash) + md.update(result, 0, offset) + key = md.digest() - val remaining = length - offset - val copySize = minOf(key.size, remaining) - System.arraycopy(key, 0, result, offset, copySize) - offset += copySize + val remaining = length - offset + val copySize = minOf(key.size, remaining) + System.arraycopy(key, 0, result, offset, copySize) + offset += copySize + } + } finally { + key.fill(0) } return result diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/MlKemHybridKeyExchange.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/MlKemHybridKeyExchange.kt index b9ba377..5b12cdf 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/MlKemHybridKeyExchange.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/MlKemHybridKeyExchange.kt @@ -124,4 +124,11 @@ internal class MlKemHybridKeyExchange( throw SshException("Invalid X25519 shared secret; all zeroes") } } + + override fun zeroize() { + mlKemPrivateKey?.fill(0) + mlKemPrivateKey = null + x25519PrivateKey?.fill(0) + x25519PrivateKey = null + } } diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketAead.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketAead.kt index 8cf0b83..7435e8c 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketAead.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketAead.kt @@ -16,6 +16,8 @@ package org.connectbot.sshlib.crypto +import javax.security.auth.Destroyable + internal data class AeadResult(val ciphertext: ByteArray, val tag: ByteArray) /** @@ -25,7 +27,9 @@ internal data class AeadResult(val ciphertext: ByteArray, val tag: ByteArray) * unlike the separate cipher + MAC approach. The packet_length field serves * as AAD (authenticated but not encrypted). */ -internal interface PacketAead { +internal interface PacketAead : Destroyable { + override fun destroy() {} + override fun isDestroyed() = false val tagLength: Int val encryptsLength: Boolean get() = false diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketCipher.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketCipher.kt index cbe719b..169db27 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketCipher.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketCipher.kt @@ -16,10 +16,15 @@ package org.connectbot.sshlib.crypto +import javax.security.auth.Destroyable + /** * Interface for SSH packet encryption/decryption. */ -internal interface PacketCipher { +internal interface PacketCipher : Destroyable { + override fun destroy() {} + override fun isDestroyed() = false + /** * Block size in bytes for this cipher. */ diff --git a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketMac.kt b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketMac.kt index eb59cb9..920a9c8 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketMac.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/crypto/PacketMac.kt @@ -16,10 +16,15 @@ package org.connectbot.sshlib.crypto +import javax.security.auth.Destroyable + /** * Interface for SSH packet message authentication codes (MAC). */ -internal interface PacketMac { +internal interface PacketMac : Destroyable { + override fun destroy() {} + override fun isDestroyed() = false + /** * MAC length in bytes. */ 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 82c4cab..246e7f5 100644 --- a/sshlib/src/main/kotlin/org/connectbot/sshlib/transport/PacketIO.kt +++ b/sshlib/src/main/kotlin/org/connectbot/sshlib/transport/PacketIO.kt @@ -97,6 +97,14 @@ internal class PacketIO(private val transport: Transport) { clientToServerEtm: Boolean = false, serverToClientEtm: Boolean = false, ) { + sendAead?.destroy() + sendAead = null + receiveAead?.destroy() + receiveAead = null + sendCipher?.destroy() + sendMac?.destroy() + receiveCipher?.destroy() + receiveMac?.destroy() this.sendCipher = clientToServerCipher this.sendMac = clientToServerMac this.receiveCipher = serverToClientCipher @@ -115,6 +123,18 @@ internal class PacketIO(private val transport: Transport) { clientToServerAead: PacketAead, serverToClientAead: PacketAead, ) { + sendCipher?.destroy() + sendCipher = null + sendMac?.destroy() + sendMac = null + receiveCipher?.destroy() + receiveCipher = null + receiveMac?.destroy() + receiveMac = null + sendEtm = false + receiveEtm = false + sendAead?.destroy() + receiveAead?.destroy() this.sendAead = clientToServerAead this.receiveAead = serverToClientAead }