From f67314fb7735a8b587d2e4628b61ba1f16eea0cf Mon Sep 17 00:00:00 2001 From: prashanOS <100228930+prashanOS@users.noreply.github.com> Date: Thu, 29 Sep 2022 15:34:10 -0400 Subject: [PATCH] Verify HMAC when decrypting received payloads. (#11) * Verify HMAC when decrypting received payloads. --- .../impls/MoshiPayloadAdapter.kt | 7 -- .../impls/MoshiPayloadEncryption.kt | 37 +++++-- .../org/walletconnect/impls/WCSession.kt | 10 +- .../java/org/walletconnect/TheUriParser.kt | 10 +- ...tConnectBridgeRepositoryIntegrationTest.kt | 13 +-- .../impls/MoshiPayloadEncryptionTest.kt | 96 +++++++++++++++++++ 6 files changed, 139 insertions(+), 34 deletions(-) create mode 100644 lib/src/test/java/org/walletconnect/impls/MoshiPayloadEncryptionTest.kt diff --git a/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadAdapter.kt b/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadAdapter.kt index 7e9a650..8c0869d 100644 --- a/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadAdapter.kt +++ b/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadAdapter.kt @@ -155,11 +155,4 @@ class MoshiPayloadAdapter(moshi: Moshi) : Session.PayloadAdapter { "method" to method, "params" to params ) - - @JsonClass(generateAdapter = true) - data class EncryptedPayload( - @Json(name = "data") val data: String, - @Json(name = "iv") val iv: String, - @Json(name = "hmac") val hmac: String - ) } diff --git a/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadEncryption.kt b/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadEncryption.kt index e9d30a3..a284736 100644 --- a/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadEncryption.kt +++ b/lib/src/main/kotlin/org/walletconnect/impls/MoshiPayloadEncryption.kt @@ -1,5 +1,7 @@ package org.walletconnect.impls +import com.squareup.moshi.Json +import com.squareup.moshi.JsonClass import com.squareup.moshi.Moshi import org.bouncycastle.crypto.digests.SHA256Digest import org.bouncycastle.crypto.engines.AESEngine @@ -16,7 +18,7 @@ import java.security.SecureRandom class MoshiPayloadEncryption(moshi: Moshi) : Session.PayloadEncryption { - private val encryptedPayloadAdapter = moshi.adapter(MoshiPayloadAdapter.EncryptedPayload::class.java) + private val encryptedPayloadAdapter = moshi.adapter(EncryptedPayload::class.java) override fun encrypt(unencryptedPayloadJson: String, key: String): String { val bytesData = unencryptedPayloadJson.toByteArray() @@ -45,7 +47,7 @@ class MoshiPayloadEncryption(moshi: Moshi) : Session.PayloadEncryption { hmac.doFinal(hmacResult, 0) return encryptedPayloadAdapter.toJson( - MoshiPayloadAdapter.EncryptedPayload( + EncryptedPayload( outBuf.toNoPrefixHexString(), hmac = hmacResult.toNoPrefixHexString(), iv = iv.toNoPrefixHexString() @@ -56,20 +58,34 @@ class MoshiPayloadEncryption(moshi: Moshi) : Session.PayloadEncryption { override fun decrypt(encryptedPayloadJson: String, key: String): String { val encryptedPayload = encryptedPayloadAdapter.fromJson(encryptedPayloadJson) ?: throw IllegalArgumentException("Invalid json payload!") - // TODO verify hmac + val hexKey = decode(key) + val iv = decode(encryptedPayload.iv) + val encryptedData = decode(encryptedPayload.data) + val providedHmac = decode(encryptedPayload.hmac) + + // verify hmac + with(HMac(SHA256Digest())) { + val hmacResult = ByteArray(macSize) + init(KeyParameter(hexKey)) + update(encryptedData, 0, encryptedData.size) + update(iv, 0, iv.size) + doFinal(hmacResult, 0) + require(hmacResult.contentEquals(providedHmac)) { "HMAC does not match - expected: $hmacResult received: $providedHmac" } + } + + // decrypt payload val padding = PKCS7Padding() val aes = PaddedBufferedBlockCipher( CBCBlockCipher(AESEngine()), padding ) val ivAndKey = ParametersWithIV( - KeyParameter(decode(key)), - decode(encryptedPayload.iv) + KeyParameter(hexKey), + iv ) aes.init(false, ivAndKey) - val encryptedData = decode(encryptedPayload.data) val minSize = aes.getOutputSize(encryptedData.size) val outBuf = ByteArray(minSize) var len = aes.processBytes(encryptedData, 0, encryptedData.size, outBuf, 0) @@ -79,4 +95,11 @@ class MoshiPayloadEncryption(moshi: Moshi) : Session.PayloadEncryption { } private fun createRandomBytes(i: Int) = ByteArray(i).also { SecureRandom().nextBytes(it) } -} \ No newline at end of file +} + +@JsonClass(generateAdapter = true) +data class EncryptedPayload( + @Json(name = "data") val data: String, + @Json(name = "iv") val iv: String, + @Json(name = "hmac") val hmac: String +) \ No newline at end of file diff --git a/lib/src/main/kotlin/org/walletconnect/impls/WCSession.kt b/lib/src/main/kotlin/org/walletconnect/impls/WCSession.kt index f2cdead..3028160 100644 --- a/lib/src/main/kotlin/org/walletconnect/impls/WCSession.kt +++ b/lib/src/main/kotlin/org/walletconnect/impls/WCSession.kt @@ -13,7 +13,7 @@ class WCSession( private val payloadAdapter: Session.PayloadAdapter, private val payloadEncryption: Session.PayloadEncryption, private val sessionStore: WCSessionStore, - private val messageLogger: Session.MessageLogger, + private val messageLogger: Session.MessageLogger? = null, transportBuilder: Session.Transport.Builder, clientMeta: Session.PeerMeta, clientId: String? = null @@ -97,7 +97,7 @@ class WCSession( config.handshakeTopic, "sub", "" ) transport.send(message) - messageLogger.log(message, isOwnMessage = true) + messageLogger?.log(message, isOwnMessage = true) } } @@ -173,7 +173,7 @@ class WCSession( clientData.id, "sub", "" ) transport.send(message) - messageLogger.log(message, isOwnMessage = true) + messageLogger?.log(message, isOwnMessage = true) } Session.Transport.Status.Disconnected -> { // no-op @@ -198,7 +198,7 @@ class WCSession( try { val decryptedPayload = payloadEncryption.decrypt(message.payload, decryptionKey) data = payloadAdapter.parse(decryptedPayload) - messageLogger.log(message.copy(payload = decryptedPayload), isOwnMessage = false) + messageLogger?.log(message.copy(payload = decryptedPayload), isOwnMessage = false) } catch (e: Exception) { handlePayloadError(e) return @@ -299,7 +299,7 @@ class WCSession( } val message = Session.Transport.Message(topic, "pub", payload) transport.send(message) - messageLogger.log(message.copy(payload = unencryptedPayload), isOwnMessage = true) + messageLogger?.log(message.copy(payload = unencryptedPayload), isOwnMessage = true) return true } diff --git a/lib/src/test/java/org/walletconnect/TheUriParser.kt b/lib/src/test/java/org/walletconnect/TheUriParser.kt index b3e7612..3a57e29 100644 --- a/lib/src/test/java/org/walletconnect/TheUriParser.kt +++ b/lib/src/test/java/org/walletconnect/TheUriParser.kt @@ -1,15 +1,7 @@ package org.walletconnect -import com.squareup.moshi.Moshi -import okhttp3.OkHttpClient import org.assertj.core.api.Assertions.assertThat -import org.junit.Test -import org.walletconnect.impls.FileWCSessionStore -import org.walletconnect.impls.MoshiPayloadAdapter -import org.walletconnect.impls.OkHttpTransport -import org.walletconnect.impls.WCSession -import java.io.File -import java.util.concurrent.TimeUnit +import org.junit.jupiter.api.Test class TheUriParser { diff --git a/lib/src/test/java/org/walletconnect/WalletConnectBridgeRepositoryIntegrationTest.kt b/lib/src/test/java/org/walletconnect/WalletConnectBridgeRepositoryIntegrationTest.kt index c027031..9d19e1d 100644 --- a/lib/src/test/java/org/walletconnect/WalletConnectBridgeRepositoryIntegrationTest.kt +++ b/lib/src/test/java/org/walletconnect/WalletConnectBridgeRepositoryIntegrationTest.kt @@ -2,9 +2,9 @@ package org.walletconnect import com.squareup.moshi.Moshi import okhttp3.OkHttpClient -import org.junit.Test import org.walletconnect.impls.FileWCSessionStore import org.walletconnect.impls.MoshiPayloadAdapter +import org.walletconnect.impls.MoshiPayloadEncryption import org.walletconnect.impls.OkHttpTransport import org.walletconnect.impls.WCSession import java.io.File @@ -27,11 +27,12 @@ class WalletConnectBridgeRepositoryIntegrationTest { val config = Session.Config.fromWCUri(uri).toFullyQualifiedConfig() val session = WCSession( - config, - MoshiPayloadAdapter(moshi), - sessionStore, - OkHttpTransport.Builder(client, moshi), - Session.PeerMeta(name = "WC Unit Test") + config = config, + payloadAdapter = MoshiPayloadAdapter(moshi), + payloadEncryption = MoshiPayloadEncryption(moshi), + sessionStore = sessionStore, + transportBuilder = OkHttpTransport.Builder(client, moshi), + clientMeta = Session.PeerMeta(name = "WC Unit Test"), ) session.addCallback(object : Session.Callback { diff --git a/lib/src/test/java/org/walletconnect/impls/MoshiPayloadEncryptionTest.kt b/lib/src/test/java/org/walletconnect/impls/MoshiPayloadEncryptionTest.kt new file mode 100644 index 0000000..0ab65a0 --- /dev/null +++ b/lib/src/test/java/org/walletconnect/impls/MoshiPayloadEncryptionTest.kt @@ -0,0 +1,96 @@ +package org.walletconnect.impls + +import com.squareup.moshi.Moshi +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.security.SecureRandom + +class MoshiPayloadEncryptionTest { + + private val moshi = Moshi.Builder().build() + private val payloadEncryption = MoshiPayloadEncryption(Moshi.Builder().build()) + private val encryptedPayloadAdapter = moshi.adapter(EncryptedPayload::class.java) + + @Test + fun `happy path encryption + decryption`() { + val key = generateKey() + val payloadJson = + """ + { + "id" : 123456, + "method" : "wc_sessionRequest", + } + """.trimIndent() + val encryptedPayload = payloadEncryption.encrypt(payloadJson, key) + + val result = payloadEncryption.decrypt(encryptedPayload, key) + + assertThat(result).isEqualTo(payloadJson) + } + + @Test + fun `invalid hmac throws exception`() { + val key = generateKey() + // this is the original payload + val payloadJson = + """ + { + "id" : 123456, + "method" : "wc_sessionRequest", + } + """.trimIndent() + val mutatedPayload = with(encryptedPayloadAdapter.fromJson(payloadEncryption.encrypt(payloadJson, key))) { + requireNotNull(this) + encryptedPayloadAdapter.toJson(copy(hmac = hmac.dropLast(4)+"1234")) + } + + assertThrows { + payloadEncryption.decrypt(mutatedPayload, key) + } + } + + @Test + fun `invalid iv throws exception`() { + val key = generateKey() + // this is the original payload + val payloadJson = + """ + { + "id" : 123456, + "method" : "wc_sessionRequest", + } + """.trimIndent() + val mutatedPayload = with(encryptedPayloadAdapter.fromJson(payloadEncryption.encrypt(payloadJson, key))) { + requireNotNull(this) + encryptedPayloadAdapter.toJson(copy(iv = iv.dropLast(4)+"1234")) + } + + assertThrows { + payloadEncryption.decrypt(mutatedPayload, key) + } + } + + @Test + fun `invalid data throws exception`() { + val key = generateKey() + // this is the original payload + val payloadJson = + """ + { + "id" : 123456, + "method" : "wc_sessionRequest", + } + """.trimIndent() + val mutatedPayload = with(encryptedPayloadAdapter.fromJson(payloadEncryption.encrypt(payloadJson, key))) { + requireNotNull(this) + encryptedPayloadAdapter.toJson(copy(data = data.dropLast(4)+"1234")) + } + + assertThrows { + payloadEncryption.decrypt(mutatedPayload, key) + } + } + + private fun generateKey() = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString(separator = "") { "%02x".format(it) } +} \ No newline at end of file