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.

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

chore: Core crypto 2.0 migration [WPB-14635] #3141

Merged
merged 3 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ package com.wire.kalium.cryptography
import com.wire.crypto.ClientId
import com.wire.crypto.CoreCrypto
import com.wire.crypto.CoreCryptoCallbacks
import com.wire.crypto.CoreCryptoLogLevel
import com.wire.crypto.CoreCryptoLogger
import com.wire.crypto.coreCryptoDeferredInit
import com.wire.crypto.setLogger
import com.wire.kalium.cryptography.MLSClientImpl.Companion.toCrlRegistration
import com.wire.kalium.cryptography.exceptions.CryptographyException
import java.io.File
Expand All @@ -36,12 +39,19 @@ actual suspend fun coreCryptoCentral(
key = databaseKey
)
coreCrypto.setCallbacks(Callbacks())
setLogger(CoreCryptoLoggerImpl(), CoreCryptoLogLevel.INFO)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would we want to have some other log level for debug builds?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can set to the lowest level (CoreCryptoLogLevel.Trace), and within the CoreCryptoLoggerImpl we can map to different kaliumLogger functions, which alreadys implement log-level filtering within it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning ⚠️ this can have a negative performance impact since a lot of messages will go over the ffi boundary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@typfel should we then make it default to INFO ? or we should have it disabled for prod at all?

Copy link
Member

@typfel typfel Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want it enabled in Prod at the INFO level which will not generate large amounts of logs.

Setting the level TRACE will potentially generate a lot of logs so it should not be enabled in Prod.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted to INFO there

return CoreCryptoCentralImpl(
cc = coreCrypto,
rootDir = rootDir
)
}

private class CoreCryptoLoggerImpl : CoreCryptoLogger {
override fun log(level: CoreCryptoLogLevel, message: String, context: String?) {
kaliumLogger.i("CoreCrypto: $message. $context")
}
}
m-zagorski marked this conversation as resolved.
Show resolved Hide resolved

private class Callbacks : CoreCryptoCallbacks {
override suspend fun authorize(conversationId: ByteArray, clientId: ClientId): Boolean {
// We always return true because our BE is currently enforcing that this constraint is always true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import io.ktor.util.encodeBase64
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import java.io.File
import com.wire.crypto.ProteusException as ProteusExceptionNative

@Suppress("TooManyFunctions")
class ProteusClientCoreCryptoImpl private constructor(
Expand Down Expand Up @@ -145,13 +146,12 @@ class ProteusClientCoreCryptoImpl private constructor(
private inline fun <T> wrapException(b: () -> T): T {
try {
return b()
} catch (e: CoreCryptoException) {
val proteusLastErrorCode = coreCrypto.proteusLastErrorCode()
} catch (e: CoreCryptoException.Proteus) {
throw ProteusException(
e.message,
ProteusException.fromProteusCode(proteusLastErrorCode.toInt()),
proteusLastErrorCode.toInt(),
e
message = e.message,
code = mapProteusExceptionToErrorCode(e.v1),
intCode = mapProteusExceptionToRawIntErrorCode(e.v1),
cause = e
)
} catch (e: Exception) {
throw ProteusException(e.message, ProteusException.Code.UNKNOWN_ERROR, null, e)
Expand Down Expand Up @@ -187,11 +187,11 @@ class ProteusClientCoreCryptoImpl private constructor(
return ProteusClientCoreCryptoImpl(coreCrypto)
} catch (exception: ProteusStorageMigrationException) {
throw exception
} catch (e: CoreCryptoException) {
} catch (e: CoreCryptoException.Proteus) {
throw ProteusException(
message = e.message,
code = ProteusException.fromProteusCode(coreCrypto.proteusLastErrorCode().toInt()),
intCode = coreCrypto.proteusLastErrorCode().toInt(),
code = mapProteusExceptionToErrorCode(e.v1),
intCode = mapProteusExceptionToRawIntErrorCode(e.v1),
cause = e.cause
)
} catch (e: Exception) {
Expand All @@ -218,5 +218,24 @@ class ProteusClientCoreCryptoImpl private constructor(
throw ProteusStorageMigrationException("Failed to migrate from crypto box at $rootDir", exception)
}
}

private fun mapProteusExceptionToErrorCode(proteusException: ProteusExceptionNative): ProteusException.Code {
return when (proteusException) {
is ProteusExceptionNative.SessionNotFound -> ProteusException.Code.SESSION_NOT_FOUND
is ProteusExceptionNative.DuplicateMessage -> ProteusException.Code.DUPLICATE_MESSAGE
is ProteusExceptionNative.RemoteIdentityChanged -> ProteusException.Code.REMOTE_IDENTITY_CHANGED
is ProteusExceptionNative.Other -> ProteusException.fromProteusCode(proteusException.v1.toInt())
}
}

@Suppress("MagicNumber")
private fun mapProteusExceptionToRawIntErrorCode(proteusException: ProteusExceptionNative): Int {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those mappers are here because we've to rely on the native ProteusException type directly, and we're not having the right dependency in the ProteusException

Copy link
Member

@typfel typfel Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going forward I'd suggest aligning ProteusException with the ProteusExceptionNative. We want to go away from dealing Int numbers for error handling.

Doesn't have to be done right away but maybe create a TODO with a ticket?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to have it in a different task, there is also some corecryptobox mappings to the ProteusException - but indeed would be nice to also clean it up

return when (proteusException) {
is ProteusExceptionNative.SessionNotFound -> 102
is ProteusExceptionNative.DuplicateMessage -> 209
is ProteusExceptionNative.RemoteIdentityChanged -> 204
is ProteusExceptionNative.Other -> proteusException.v1.toInt()
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -176,19 +176,25 @@ open class ProteusException(message: String?, val code: Code, val intCode: Int?,
}
}

// Mapping source:
// https://github.com/wireapp/proteus/blob/2.x/crates/proteus-traits/src/lib.rs
// https://github.com/wireapp/wire-web-core/blob/7383e108f5e9d15d0b82c41ed504964667463cfc/packages/proteus/README.md
/**
* Those error codes are mapped directly from [com.wire.crypto.ProteusException]:
* - [Code.SESSION_NOT_FOUND]
* - [Code.REMOTE_IDENTITY_CHANGED]
* - [Code.DUPLICATE_MESSAGE]
*
* See the mapping: [com.wire.kalium.cryptography.ProteusClientCoreCryptoImpl.Companion.mapProteusExceptionToErrorCode]
*
* [Mapping sources](https://github.com/wireapp/proteus/blob/2.x/crates/proteus-traits/src/lib.rs)
*
* [Mapping source README](https://github.com/wireapp/wire-web-core/blob/7383e108f5e9d15d0b82c41ed504964667463cfc/packages/proteus/README.md)
*/
fun fromProteusCode(code: Int): Code {
@Suppress("MagicNumber")
return when (code) {
501 -> Code.STORAGE_ERROR
102 -> Code.SESSION_NOT_FOUND
3, 301, 302, 303 -> Code.DECODE_ERROR
204 -> Code.REMOTE_IDENTITY_CHANGED
206, 207, 210 -> Code.INVALID_SIGNATURE
200, 201, 202, 205, 213 -> Code.INVALID_MESSAGE
209 -> Code.DUPLICATE_MESSAGE
211, 212 -> Code.TOO_DISTANT_FUTURE
208 -> Code.OUTDATED_MESSAGE
300 -> Code.IDENTITY_ERROR
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pbandk = "0.14.2"
turbine = "1.1.0"
avs = "10.0.1"
jna = "5.14.0"
core-crypto = "1.0.2"
core-crypto = "2.0.0"
core-crypto-multiplatform = "0.6.0-rc.3-multiplatform-pre1"
completeKotlin = "1.1.0"
desugar-jdk = "2.1.3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,20 @@
package com.wire.kalium.logic

import com.wire.crypto.CoreCryptoException
import uniffi.core_crypto.CryptoError
import com.wire.crypto.MlsException

actual fun mapMLSException(exception: Exception): MLSFailure =
if (exception is CoreCryptoException.CryptoException) {
when (exception.error) {
is CryptoError.WrongEpoch -> MLSFailure.WrongEpoch
is CryptoError.DuplicateMessage -> MLSFailure.DuplicateMessage
is CryptoError.BufferedFutureMessage -> MLSFailure.BufferedFutureMessage
is CryptoError.SelfCommitIgnored -> MLSFailure.SelfCommitIgnored
is CryptoError.UnmergedPendingGroup -> MLSFailure.UnmergedPendingGroup
is CryptoError.StaleProposal -> MLSFailure.StaleProposal
is CryptoError.StaleCommit -> MLSFailure.StaleCommit
is CryptoError.ConversationAlreadyExists -> MLSFailure.ConversationAlreadyExists
is CryptoError.MessageEpochTooOld -> MLSFailure.MessageEpochTooOld
is CryptoError.MlsException -> MLSFailure.InternalErrors
if (exception is CoreCryptoException.Mls) {
when (exception.v1) {
is MlsException.WrongEpoch -> MLSFailure.WrongEpoch
is MlsException.DuplicateMessage -> MLSFailure.DuplicateMessage
is MlsException.BufferedFutureMessage -> MLSFailure.BufferedFutureMessage
is MlsException.SelfCommitIgnored -> MLSFailure.SelfCommitIgnored
is MlsException.UnmergedPendingGroup -> MLSFailure.UnmergedPendingGroup
is MlsException.StaleProposal -> MLSFailure.StaleProposal
is MlsException.StaleCommit -> MLSFailure.StaleCommit
is MlsException.ConversationAlreadyExists -> MLSFailure.ConversationAlreadyExists
is MlsException.MessageEpochTooOld -> MLSFailure.MessageEpochTooOld
else -> MLSFailure.Generic(exception)
}
} else {
Expand Down
Loading