Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: suppress buffer error for mls decrypt transaction [WPB-16233] 🍒 #3306

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: suppress buffer error for mls decrypt transaction [WPB-16233] 🍒 #3306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 36 additions & 13 deletions cryptography/src/commonJvmAndroid/kotlin/com.wire.kalium.cryptography/MLSClientImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@ import com.wire.crypto.BufferedDecryptedMessage
import com.wire.crypto.Ciphersuite
import com.wire.crypto.ConversationConfiguration
import com.wire.crypto.CoreCrypto
import com.wire.crypto.CoreCryptoCommand
import com.wire.crypto.CoreCryptoContext
import com.wire.crypto.CoreCryptoException
import com.wire.crypto.CustomConfiguration
import com.wire.crypto.DecryptedMessage
import com.wire.crypto.E2eiConversationState
import com.wire.crypto.MlsCredentialType
import com.wire.crypto.MlsException
import com.wire.crypto.MlsGroupInfoEncryptionType
import com.wire.crypto.MlsRatchetTreeType
import com.wire.crypto.MlsWirePolicy
Expand Down Expand Up @@ -137,22 +141,41 @@ class MLSClientImpl(
return applicationMessage
}

@Suppress("TooGenericExceptionCaught")
override suspend fun decryptMessage(groupId: MLSGroupId, message: ApplicationMessage): List<DecryptedMessageBundle> {
val decryptedMessage = coreCrypto.decryptMessage(
groupId.decodeBase64Bytes(),
message
)
var decryptedMessage: DecryptedMessage? = null

coreCrypto.transaction(object : CoreCryptoCommand {
override suspend fun execute(context: CoreCryptoContext) {
try {
val result = context.decryptMessage(
groupId.decodeBase64Bytes(),
message
)
decryptedMessage = result

} catch (throwable: Throwable) {
val isBufferedFutureError = (
throwable is CoreCryptoException.Mls && throwable.v1 is MlsException.BufferedFutureMessage
) || throwable.message
?.contains("Incoming message is a commit for which we have not yet received all the proposals") == true
if (!isBufferedFutureError) {
throw throwable
}
}
}
})

val messageBundle = listOf(
toDecryptedMessageBundle(
decryptedMessage
)
)
val bufferedMessages = decryptedMessage.bufferedMessages?.map {
toDecryptedMessageBundle(it)
} ?: emptyList()
if (decryptedMessage == null) {
return emptyList()
}

val mainMessageBundle = listOf(toDecryptedMessageBundle(decryptedMessage!!))
val bufferedBundles = decryptedMessage!!.bufferedMessages
?.map { toDecryptedMessageBundle(it) }
?: emptyList()

return messageBundle + bufferedMessages
return mainMessageBundle + bufferedBundles
}

override suspend fun commitAccepted(groupId: MLSGroupId) {
Expand Down
99 changes: 99 additions & 0 deletions cryptography/src/commonTest/kotlin/com/wire/kalium/cryptography/MLSClientTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,105 @@ class MLSClientTest : BaseMLSClientTest() {
assertNull(aliceClient.decryptMessage(welcomeBundle.groupId, commit).first().message)
}

@Test
fun givenThreeClients_whenProcessingCommitOutOfOrder_shouldCatchBufferedFutureMessageAndBuffer() = runTest {
// Bob creates a conversation.
val bobClient = createClient(BOB1)
bobClient.createConversation(MLS_CONVERSATION_ID, externalSenderKey)

// Bob adds Alice, Alice processes the welcome.
val aliceClient = createClient(ALICE1)
val welcomeAlice = bobClient.addMember(
MLS_CONVERSATION_ID,
listOf(aliceClient.generateKeyPackages(1).first())
)!!.welcome!!
bobClient.commitAccepted(MLS_CONVERSATION_ID)
aliceClient.processWelcomeMessage(welcomeAlice)

// Bob adds Carol but Alice does NOT process that commit => out of order for Alice later.
val carolClient = createClient(CAROL1)
val addCarolResult = bobClient.addMember(
MLS_CONVERSATION_ID,
listOf(carolClient.generateKeyPackages(1).first())
)
val commitAddCarol = addCarolResult!!.commit
bobClient.commitAccepted(MLS_CONVERSATION_ID)

// Bob immediately removes Carol => definitely out of order for Alice.
val removeCarolResult = bobClient.removeMember(MLS_CONVERSATION_ID, listOf(CAROL1.qualifiedClientId))
val commitRemoveCarol = removeCarolResult.commit
bobClient.commitAccepted(MLS_CONVERSATION_ID)

// Alice tries to decrypt the removeCarol commit, which references an epoch Alice hasn't seen yet.
// In normal MLS logic, this triggers a "buffering" error, typically thrown as MlsException.BufferedFutureMessage
// wrapped in CoreCryptoException.Mls. The client code is supposed to swallow that error in a transaction
// and return an empty DecryptedMessage list.

val decryptedBundlesResult = runCatching {
aliceClient.decryptMessage(MLS_CONVERSATION_ID, commitRemoveCarol)
}

// The exception should be caught internally, so from the caller's perspective we succeed with an empty result.
// That indicates the message was buffered instead of fully decrypted.
assertTrue(
decryptedBundlesResult.isSuccess,
"Out-of-order commit should not propagate BufferedFutureMessage as an unhandled exception."
)

val decryptedBundles = decryptedBundlesResult.getOrThrow()
assertTrue(
decryptedBundles.isEmpty(),
"Decryption result should be empty for a buffered out-of-order commit."
)
}

@Test
fun givenOutOfOrderCommits_whenProcessingMissingCommitLater_shouldAlsoProcessBufferedOne() = runTest {
val bobClient = createClient(BOB1)
bobClient.createConversation(MLS_CONVERSATION_ID, externalSenderKey)

val aliceClient = createClient(ALICE1)
// Bob adds Alice to the conversation
val welcomeForAlice = bobClient.addMember(
MLS_CONVERSATION_ID,
listOf(aliceClient.generateKeyPackages(1).first())
)!!.welcome!!
bobClient.commitAccepted(MLS_CONVERSATION_ID)
aliceClient.processWelcomeMessage(welcomeForAlice)

// Bob adds Carol, but Alice never sees this commit => out-of-order for Alice
val carolClient = createClient(CAROL1)
val addCarolResult = bobClient.addMember(
MLS_CONVERSATION_ID,
listOf(carolClient.generateKeyPackages(1).first())
)
val commitAddCarol = addCarolResult!!.commit
bobClient.commitAccepted(MLS_CONVERSATION_ID)

// Immediately Bob removes Carol => definitely out-of-order for Alice
val removeCarolResult = bobClient.removeMember(MLS_CONVERSATION_ID, listOf(CAROL1.qualifiedClientId))
val commitRemoveCarol = removeCarolResult.commit
bobClient.commitAccepted(MLS_CONVERSATION_ID)

// Alice tries to decrypt the removeCarol commit first => out-of-order => should buffer
val removeResult = aliceClient.decryptMessage(MLS_CONVERSATION_ID, commitRemoveCarol)
assertTrue(
removeResult.isEmpty(),
"Out-of-order remove commit should be buffered and return an empty list."
)

// Now Alice processes the missing 'addCarol' commit.
// By processing the addCarol commit, MLS should also flush any previously buffered commits (the removeCarol).
val addResult = aliceClient.decryptMessage(MLS_CONVERSATION_ID, commitAddCarol)

// We expect 2 total commits to be processed now: (1) addCarol, (2) removeCarol.
assertEquals(
2,
addResult.size,
"Processing the older 'addCarol' commit should also flush the buffered 'removeCarol' commit, resulting in 2 items."
)
}

companion object {
val externalSenderKey = ByteArray(32)
val DEFAULT_CIPHER_SUITES = 1.toUShort()
Expand Down