code review

This commit is contained in:
Valere 2021-11-23 11:58:01 +01:00
parent feda53bfb7
commit 77454c8ae9
3 changed files with 16 additions and 13 deletions

View File

@ -16,6 +16,7 @@
package org.matrix.android.sdk.internal.crypto.actions package org.matrix.android.sdk.internal.crypto.actions
import org.matrix.android.sdk.api.logger.LoggerTag
import org.matrix.android.sdk.internal.crypto.MXOlmDevice import org.matrix.android.sdk.internal.crypto.MXOlmDevice
import org.matrix.android.sdk.internal.crypto.model.CryptoDeviceInfo import org.matrix.android.sdk.internal.crypto.model.CryptoDeviceInfo
import org.matrix.android.sdk.internal.crypto.model.MXKey import org.matrix.android.sdk.internal.crypto.model.MXKey
@ -28,6 +29,8 @@ import javax.inject.Inject
private const val ONE_TIME_KEYS_RETRY_COUNT = 3 private const val ONE_TIME_KEYS_RETRY_COUNT = 3
private val loggerTag = LoggerTag("EnsureOlmSessionsForDevicesAction", LoggerTag.CRYPTO)
internal class EnsureOlmSessionsForDevicesAction @Inject constructor( internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
private val olmDevice: MXOlmDevice, private val olmDevice: MXOlmDevice,
private val oneTimeKeysForUsersDeviceTask: ClaimOneTimeKeysForUsersDeviceTask) { private val oneTimeKeysForUsersDeviceTask: ClaimOneTimeKeysForUsersDeviceTask) {
@ -49,10 +52,10 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
val sessionId = olmDevice.getSessionId(key) val sessionId = olmDevice.getSessionId(key)
if (sessionId.isNullOrEmpty() || force) { if (sessionId.isNullOrEmpty() || force) {
Timber.d("## CRYPTO | Found no existing olm session (${deviceInfo.userId}|$deviceId) (force=$force)") Timber.tag(loggerTag.value).d("Found no existing olm session (${deviceInfo.userId}|$deviceId) (force=$force)")
devicesWithoutSession.add(deviceInfo) devicesWithoutSession.add(deviceInfo)
} else { } else {
Timber.d("## CRYPTO | using olm session $sessionId for (${deviceInfo.userId}|$deviceId)") Timber.tag(loggerTag.value).d("using olm session $sessionId for (${deviceInfo.userId}|$deviceId)")
} }
val olmSessionResult = MXOlmSessionResult(deviceInfo, sessionId) val olmSessionResult = MXOlmSessionResult(deviceInfo, sessionId)
@ -60,7 +63,7 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
} }
} }
Timber.d("## CRYPTO | Devices without olm session (count:${devicesWithoutSession.size}) :" + Timber.tag(loggerTag.value).d("Devices without olm session (count:${devicesWithoutSession.size}) :" +
" ${devicesWithoutSession.joinToString { "${it.userId}|${it.deviceId}" }}") " ${devicesWithoutSession.joinToString { "${it.userId}|${it.deviceId}" }}")
if (devicesWithoutSession.size == 0) { if (devicesWithoutSession.size == 0) {
return results return results
@ -81,11 +84,11 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
// //
// That should eventually resolve itself, but it's poor form. // That should eventually resolve itself, but it's poor form.
Timber.i("## CRYPTO | claimOneTimeKeysForUsersDevices() : ${usersDevicesToClaim.toDebugString()}") Timber.tag(loggerTag.value).i("claimOneTimeKeysForUsersDevices() : ${usersDevicesToClaim.toDebugString()}")
val claimParams = ClaimOneTimeKeysForUsersDeviceTask.Params(usersDevicesToClaim) val claimParams = ClaimOneTimeKeysForUsersDeviceTask.Params(usersDevicesToClaim)
val oneTimeKeys = oneTimeKeysForUsersDeviceTask.executeRetry(claimParams, remainingRetry = ONE_TIME_KEYS_RETRY_COUNT) val oneTimeKeys = oneTimeKeysForUsersDeviceTask.executeRetry(claimParams, remainingRetry = ONE_TIME_KEYS_RETRY_COUNT)
Timber.v("## CRYPTO | claimOneTimeKeysForUsersDevices() : keysClaimResponse.oneTimeKeys: $oneTimeKeys") Timber.tag(loggerTag.value).v("claimOneTimeKeysForUsersDevices() : keysClaimResponse.oneTimeKeys: $oneTimeKeys")
for ((userId, deviceInfos) in devicesByUser) { for ((userId, deviceInfos) in devicesByUser) {
for (deviceInfo in deviceInfos) { for (deviceInfo in deviceInfos) {
var oneTimeKey: MXKey? = null var oneTimeKey: MXKey? = null
@ -102,7 +105,7 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
oneTimeKey = key oneTimeKey = key
} }
if (oneTimeKey == null) { if (oneTimeKey == null) {
Timber.d("## CRYPTO: No one time key for $userId|$deviceId") Timber.tag(loggerTag.value).d("No one time key for $userId|$deviceId")
continue continue
} }
// Update the result for this device in results // Update the result for this device in results
@ -130,9 +133,9 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
olmDevice.verifySignature(fingerprint, oneTimeKey.signalableJSONDictionary(), signature) olmDevice.verifySignature(fingerprint, oneTimeKey.signalableJSONDictionary(), signature)
isVerified = true isVerified = true
} catch (e: Exception) { } catch (e: Exception) {
Timber.d(e, "## CRYPTO | verifyKeyAndStartSession() : Verify error for otk: ${oneTimeKey.signalableJSONDictionary()}," + Timber.tag(loggerTag.value).d(e, "verifyKeyAndStartSession() : Verify error for otk: ${oneTimeKey.signalableJSONDictionary()}," +
" signature:$signature fingerprint:$fingerprint") " signature:$signature fingerprint:$fingerprint")
Timber.e("## CRYPTO | verifyKeyAndStartSession() : Verify error for ${deviceInfo.userId}|${deviceInfo.deviceId} " + Timber.tag(loggerTag.value).e("verifyKeyAndStartSession() : Verify error for ${deviceInfo.userId}|${deviceInfo.deviceId} " +
" - signable json ${oneTimeKey.signalableJSONDictionary()}") " - signable json ${oneTimeKey.signalableJSONDictionary()}")
errorMessage = e.message errorMessage = e.message
} }
@ -145,12 +148,12 @@ internal class EnsureOlmSessionsForDevicesAction @Inject constructor(
if (sessionId.isNullOrEmpty()) { if (sessionId.isNullOrEmpty()) {
// Possibly a bad key // Possibly a bad key
Timber.e("## CRYPTO | verifyKeyAndStartSession() : Error starting session with device $userId:$deviceId") Timber.tag(loggerTag.value).e("verifyKeyAndStartSession() : Error starting session with device $userId:$deviceId")
} else { } else {
Timber.d("## CRYPTO | verifyKeyAndStartSession() : Started new sessionId $sessionId for device $userId:$deviceId") Timber.tag(loggerTag.value).d("verifyKeyAndStartSession() : Started new sessionId $sessionId for device $userId:$deviceId")
} }
} else { } else {
Timber.e("## CRYPTO | verifyKeyAndStartSession() : Unable to verify signature on one-time key for device " + userId + Timber.tag(loggerTag.value).e("verifyKeyAndStartSession() : Unable to verify signature on one-time key for device " + userId +
":" + deviceId + " Error " + errorMessage) ":" + deviceId + " Error " + errorMessage)
} }
} }

View File

@ -285,7 +285,7 @@ internal class MXMegolmEncryption(
gossipingEventBuffer.add( gossipingEventBuffer.add(
Event( Event(
type = EventType.ROOM_KEY, type = EventType.ROOM_KEY,
senderId = this.myUserId, senderId = myUserId,
content = submap.apply { content = submap.apply {
this["session_key"] = "" this["session_key"] = ""
// we add a fake key for trail // we add a fake key for trail

View File

@ -33,7 +33,7 @@ import org.matrix.android.sdk.internal.session.initsync.ProgressReporter
import timber.log.Timber import timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
private val loggerTag = LoggerTag("MXMegolmEncryption", LoggerTag.CRYPTO) private val loggerTag = LoggerTag("CryptoSyncHandler", LoggerTag.CRYPTO)
internal class CryptoSyncHandler @Inject constructor(private val cryptoService: DefaultCryptoService, internal class CryptoSyncHandler @Inject constructor(private val cryptoService: DefaultCryptoService,
private val verificationService: DefaultVerificationService) { private val verificationService: DefaultVerificationService) {