Avoid injecting credentials. Inject userId and deviceId instead

And cleanup API
This commit is contained in:
Benoit Marty 2020-01-25 12:08:50 +01:00
parent 962b85b041
commit be77017209
5 changed files with 64 additions and 69 deletions

View file

@ -39,20 +39,20 @@ interface VerificationService {
*/
fun markedLocallyAsManuallyVerified(userId: String, deviceID: String)
fun getExistingTransaction(otherUser: String, tid: String): VerificationTransaction?
fun getExistingTransaction(otherUserId: String, tid: String): VerificationTransaction?
fun getExistingVerificationRequest(otherUser: String): List<PendingVerificationRequest>?
fun getExistingVerificationRequest(otherUserId: String): List<PendingVerificationRequest>?
fun getExistingVerificationRequest(otherUser: String, tid: String?): PendingVerificationRequest?
fun getExistingVerificationRequest(otherUserId: String, tid: String?): PendingVerificationRequest?
fun getExistingVerificationRequestInRoom(roomId: String, tid: String?): PendingVerificationRequest?
fun beginKeyVerification(method: VerificationMethod, userId: String, deviceID: String): String?
fun beginKeyVerification(method: VerificationMethod, otherUserId: String, otherDeviceID: String): String?
/**
* Request a key verification from another user using toDevice events.
*/
fun requestKeyVerificationInDMs(methods: List<VerificationMethod>, userId: String, roomId: String, localId: String? = LocalEcho.createLocalEchoId()): PendingVerificationRequest
fun requestKeyVerificationInDMs(methods: List<VerificationMethod>, otherUserId: String, roomId: String, localId: String? = LocalEcho.createLocalEchoId()): PendingVerificationRequest
fun declineVerificationRequestInDMs(otherUserId: String, otherDeviceId: String, transactionId: String, roomId: String)

View file

@ -17,7 +17,6 @@ package im.vector.matrix.android.internal.crypto.verification
import android.util.Base64
import im.vector.matrix.android.BuildConfig
import im.vector.matrix.android.api.auth.data.Credentials
import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService
import im.vector.matrix.android.api.session.crypto.sas.CancelCode
import im.vector.matrix.android.api.session.crypto.sas.IncomingSasVerificationTransaction
@ -30,16 +29,18 @@ import timber.log.Timber
internal class DefaultIncomingSASDefaultVerificationTransaction(
setDeviceVerificationAction: SetDeviceVerificationAction,
override val credentials: Credentials,
override val userId: String,
override val deviceId: String?,
private val cryptoStore: IMXCryptoStore,
crossSigningService: CrossSigningService,
deviceFingerprint: String,
transactionId: String,
otherUserID: String,
val autoAccept: Boolean = false
private val autoAccept: Boolean = false
) : SASDefaultVerificationTransaction(
setDeviceVerificationAction,
credentials,
userId,
deviceId,
cryptoStore,
crossSigningService,
deviceFingerprint,
@ -157,7 +158,7 @@ internal class DefaultIncomingSASDefaultVerificationTransaction(
cancel(CancelCode.UnexpectedMessage)
}
override fun onKeyVerificationKey(userId: String, vKey: VerificationInfoKey) {
override fun onKeyVerificationKey(vKey: VerificationInfoKey) {
Timber.v("## SAS received key for request id:$transactionId")
if (state != VerificationTxState.SendingAccept && state != VerificationTxState.Accepted) {
Timber.e("## SAS received key from invalid state $state")
@ -194,10 +195,7 @@ internal class DefaultIncomingSASDefaultVerificationTransaction(
// - the Matrix ID of the user who sent the m.key.verification.accept message,
// - he device ID of the device that sent the m.key.verification.accept message
// - the transaction ID.
val sasInfo = "MATRIX_KEY_VERIFICATION_SAS" +
"$otherUserId$otherDeviceId" +
"${credentials.userId}${credentials.deviceId}" +
transactionId
val sasInfo = "MATRIX_KEY_VERIFICATION_SAS$otherUserId$otherDeviceId$userId$deviceId$transactionId"
// decimal: generate five bytes by using HKDF.
// emoji: generate six bytes by using HKDF.
shortCodeBytes = getSAS().generateShortCode(sasInfo, 6)

View file

@ -15,7 +15,6 @@
*/
package im.vector.matrix.android.internal.crypto.verification
import im.vector.matrix.android.api.auth.data.Credentials
import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService
import im.vector.matrix.android.api.session.crypto.sas.CancelCode
import im.vector.matrix.android.api.session.crypto.sas.OutgoingSasVerificationRequest
@ -27,7 +26,8 @@ import timber.log.Timber
internal class DefaultOutgoingSASDefaultVerificationRequest(
setDeviceVerificationAction: SetDeviceVerificationAction,
credentials: Credentials,
userId: String,
deviceId: String?,
cryptoStore: IMXCryptoStore,
crossSigningService: CrossSigningService,
deviceFingerprint: String,
@ -36,7 +36,8 @@ internal class DefaultOutgoingSASDefaultVerificationRequest(
otherDeviceId: String
) : SASDefaultVerificationTransaction(
setDeviceVerificationAction,
credentials,
userId,
deviceId,
cryptoStore,
crossSigningService,
deviceFingerprint,
@ -81,7 +82,7 @@ internal class DefaultOutgoingSASDefaultVerificationRequest(
}
val startMessage = transport.createStartForSas(
credentials.deviceId ?: "",
deviceId ?: "",
transactionId,
KNOWN_AGREEMENT_PROTOCOLS,
KNOWN_HASHES,
@ -161,7 +162,7 @@ internal class DefaultOutgoingSASDefaultVerificationRequest(
}
}
override fun onKeyVerificationKey(userId: String, vKey: VerificationInfoKey) {
override fun onKeyVerificationKey(vKey: VerificationInfoKey) {
Timber.v("## SAS O: onKeyVerificationKey id:$transactionId")
if (state != VerificationTxState.SendingKey && state != VerificationTxState.KeySent) {
Timber.e("## received key from invalid state $state")
@ -189,16 +190,13 @@ internal class DefaultOutgoingSASDefaultVerificationRequest(
// - the Matrix ID of the user who sent the m.key.verification.accept message,
// - he device ID of the device that sent the m.key.verification.accept message
// - the transaction ID.
val sasInfo = "MATRIX_KEY_VERIFICATION_SAS" +
"${credentials.userId}${credentials.deviceId}" +
"$otherUserId$otherDeviceId" +
transactionId
val sasInfo = "MATRIX_KEY_VERIFICATION_SAS$userId$deviceId$otherUserId$otherDeviceId$transactionId"
// decimal: generate five bytes by using HKDF.
// emoji: generate six bytes by using HKDF.
shortCodeBytes = getSAS().generateShortCode(sasInfo, 6)
state = VerificationTxState.ShortCodeReady
} else {
// bad commitement
// bad commitment
cancel(CancelCode.MismatchedCommitment)
}
}

View file

@ -20,7 +20,6 @@ import android.os.Handler
import android.os.Looper
import dagger.Lazy
import im.vector.matrix.android.api.MatrixCallback
import im.vector.matrix.android.api.auth.data.Credentials
import im.vector.matrix.android.api.session.crypto.CryptoService
import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService
import im.vector.matrix.android.api.session.crypto.sas.CancelCode
@ -64,6 +63,8 @@ import im.vector.matrix.android.internal.crypto.store.IMXCryptoStore
import im.vector.matrix.android.internal.crypto.verification.qrcode.QrCodeData
import im.vector.matrix.android.internal.crypto.verification.qrcode.generateSharedSecret
import im.vector.matrix.android.internal.crypto.verification.qrcode.toUrl
import im.vector.matrix.android.internal.di.DeviceId
import im.vector.matrix.android.internal.di.UserId
import im.vector.matrix.android.internal.session.SessionScope
import im.vector.matrix.android.internal.util.MatrixCoroutineDispatchers
import kotlinx.coroutines.GlobalScope
@ -76,10 +77,8 @@ import kotlin.collections.set
@SessionScope
internal class DefaultVerificationService @Inject constructor(
// TODO @UserId private val userId: String,
// TODO @DeviceId private val deviceId: String,
// TODO Do not use credential (do it in a dedicated commit)
private val credentials: Credentials,
@UserId private val userId: String,
@DeviceId private val deviceId: String?,
private val cryptoStore: IMXCryptoStore,
private val myDeviceInfoHolder: Lazy<MyDeviceInfoHolder>,
private val deviceListManager: DeviceListManager,
@ -264,7 +263,7 @@ internal class DefaultVerificationService @Inject constructor(
?: return
val senderId = event.senderId ?: return
if (requestInfo.toUserId != credentials.userId) {
if (requestInfo.toUserId != userId) {
// I should ignore this, it's not for me
Timber.w("## SAS Verification ignoring request from ${event.senderId}, not sent to me")
return
@ -404,7 +403,8 @@ internal class DefaultVerificationService @Inject constructor(
val tx = DefaultIncomingSASDefaultVerificationTransaction(
// this,
setDeviceVerificationAction,
credentials,
userId,
deviceId,
cryptoStore,
crossSigningService,
myDeviceInfoHolder.get().myDevice.fingerprint()!!,
@ -685,7 +685,7 @@ internal class DefaultVerificationService @Inject constructor(
return@let null
}
val myDeviceId = credentials.deviceId
val myDeviceId = deviceId
?: run {
Timber.w("Unable to get my deviceId")
return@let null
@ -697,7 +697,7 @@ internal class DefaultVerificationService @Inject constructor(
val generatedSharedSecret = generateSharedSecret()
.also { myGeneratedSharedSecret = it }
QrCodeData(
userId = credentials.userId,
userId = userId,
requestEventId = requestEventId,
action = QrCodeData.ACTION_VERIFY,
keys = hashMapOf(
@ -725,21 +725,22 @@ internal class DefaultVerificationService @Inject constructor(
updatePendingRequest(existingRequest.copy(isSuccessful = true))
}
override fun getExistingTransaction(otherUser: String, tid: String): VerificationTransaction? {
// TODO All this methods should be delegated to a TransactionStore
override fun getExistingTransaction(otherUserId: String, tid: String): VerificationTransaction? {
synchronized(lock = txMap) {
return txMap[otherUser]?.get(tid)
return txMap[otherUserId]?.get(tid)
}
}
override fun getExistingVerificationRequest(otherUser: String): List<PendingVerificationRequest>? {
override fun getExistingVerificationRequest(otherUserId: String): List<PendingVerificationRequest>? {
synchronized(lock = pendingRequests) {
return pendingRequests[otherUser]
return pendingRequests[otherUserId]
}
}
override fun getExistingVerificationRequest(otherUser: String, tid: String?): PendingVerificationRequest? {
override fun getExistingVerificationRequest(otherUserId: String, tid: String?): PendingVerificationRequest? {
synchronized(lock = pendingRequests) {
return tid?.let { tid -> pendingRequests[otherUser]?.firstOrNull { it.transactionId == tid } }
return tid?.let { tid -> pendingRequests[otherUserId]?.firstOrNull { it.transactionId == tid } }
}
}
@ -776,19 +777,20 @@ internal class DefaultVerificationService @Inject constructor(
}
}
override fun beginKeyVerification(method: VerificationMethod, userId: String, deviceID: String): String? {
val txID = createUniqueIDForTransaction(userId, deviceID)
override fun beginKeyVerification(method: VerificationMethod, otherUserId: String, otherDeviceID: String): String? {
val txID = createUniqueIDForTransaction(otherUserId, otherDeviceID)
// should check if already one (and cancel it)
if (method == VerificationMethod.SAS) {
val tx = DefaultOutgoingSASDefaultVerificationRequest(
setDeviceVerificationAction,
credentials,
userId,
deviceId,
cryptoStore,
crossSigningService,
myDeviceInfoHolder.get().myDevice.fingerprint()!!,
txID,
userId,
deviceID)
otherUserId,
otherDeviceID)
tx.transport = verificationTransportToDeviceFactory.createTransport(tx)
addTransaction(tx)
@ -799,13 +801,13 @@ internal class DefaultVerificationService @Inject constructor(
}
}
override fun requestKeyVerificationInDMs(methods: List<VerificationMethod>, userId: String, roomId: String, localId: String?)
override fun requestKeyVerificationInDMs(methods: List<VerificationMethod>, otherUserId: String, roomId: String, localId: String?)
: PendingVerificationRequest {
Timber.i("## SAS Requesting verification to user: $userId in room $roomId")
Timber.i("## SAS Requesting verification to user: $otherUserId in room $roomId")
val requestsForUser = pendingRequests[userId]
val requestsForUser = pendingRequests[otherUserId]
?: ArrayList<PendingVerificationRequest>().also {
pendingRequests[userId] = it
pendingRequests[otherUserId] = it
}
val transport = verificationTransportRoomMessageFactory.createTransport(roomId, null)
@ -828,7 +830,7 @@ internal class DefaultVerificationService @Inject constructor(
isIncoming = false,
roomId = roomId,
localID = localID,
otherUserId = userId
otherUserId = otherUserId
)
// Add reciprocate method if application declares it can scan or show QR codes
@ -836,7 +838,7 @@ internal class DefaultVerificationService @Inject constructor(
val reciprocateMethod = methods.firstOrNull { it == VerificationMethod.QR_CODE_SCAN || it == VerificationMethod.QR_CODE_SHOW }?.let { listOf(VERIFICATION_METHOD_RECIPROCATE) }.orEmpty()
val methodValues = (methods.map { it.toValue() } + reciprocateMethod).distinct()
transport.sendVerificationRequest(methodValues, localID, userId, roomId) { syncedId, info ->
transport.sendVerificationRequest(methodValues, localID, otherUserId, roomId) { syncedId, info ->
// We need to update with the syncedID
updatePendingRequest(verificationRequest.copy(
transactionId = syncedId,
@ -886,7 +888,8 @@ internal class DefaultVerificationService @Inject constructor(
if (method == VerificationMethod.SAS) {
val tx = DefaultOutgoingSASDefaultVerificationRequest(
setDeviceVerificationAction,
credentials,
userId,
deviceId,
cryptoStore,
crossSigningService,
myDeviceInfoHolder.get().myDevice.fingerprint()!!,
@ -918,7 +921,7 @@ internal class DefaultVerificationService @Inject constructor(
return false
}
// TODO this is not yet related to a transaction, maybe we should use another method like for cancel?
val readyMsg = transport.createReady(transactionId, credentials.deviceId ?: "", methods)
val readyMsg = transport.createReady(transactionId, deviceId ?: "", methods)
transport.sendToOther(EventType.KEY_VERIFICATION_READY, readyMsg,
VerificationTxState.None,
CancelCode.User,
@ -936,12 +939,12 @@ internal class DefaultVerificationService @Inject constructor(
/**
* This string must be unique for the pair of users performing verification for the duration that the transaction is valid
*/
private fun createUniqueIDForTransaction(userId: String, deviceID: String): String {
private fun createUniqueIDForTransaction(otherUserId: String, otherDeviceID: String): String {
return buildString {
append(credentials.userId).append("|")
append(credentials.deviceId).append("|")
append(userId).append("|")
append(deviceID).append("|")
append(deviceId).append("|")
append(otherUserId).append("|")
append(otherDeviceID).append("|")
append(UUID.randomUUID().toString())
}
}

View file

@ -17,7 +17,6 @@ package im.vector.matrix.android.internal.crypto.verification
import android.os.Build
import im.vector.matrix.android.api.MatrixCallback
import im.vector.matrix.android.api.auth.data.Credentials
import im.vector.matrix.android.api.session.crypto.crosssigning.CrossSigningService
import im.vector.matrix.android.api.session.crypto.sas.CancelCode
import im.vector.matrix.android.api.session.crypto.sas.EmojiRepresentation
@ -41,7 +40,8 @@ import kotlin.properties.Delegates
*/
internal abstract class SASDefaultVerificationTransaction(
private val setDeviceVerificationAction: SetDeviceVerificationAction,
open val credentials: Credentials,
open val userId: String,
open val deviceId: String?,
private val cryptoStore: IMXCryptoStore,
private val crossSigningService: CrossSigningService,
private val deviceFingerprint: String,
@ -139,11 +139,7 @@ internal abstract class SASDefaultVerificationTransaction(
// - the device ID of the device receiving the MAC,
// - the transaction ID, and
// - the key ID of the key being MAC-ed, or the string “KEY_IDS” if the item being MAC-ed is the list of key IDs.
val baseInfo = "MATRIX_KEY_VERIFICATION_MAC" +
credentials.userId + credentials.deviceId +
otherUserId + otherDeviceId +
transactionId
val baseInfo = "MATRIX_KEY_VERIFICATION_MAC$userId$deviceId$otherUserId$otherDeviceId$transactionId"
// Previously, with SAS verification, the m.key.verification.mac message only contained the user's device key.
// It should now contain both the device key and the MSK.
@ -151,7 +147,7 @@ internal abstract class SASDefaultVerificationTransaction(
val keyMap = HashMap<String, String>()
val keyId = "ed25519:${credentials.deviceId}"
val keyId = "ed25519:$deviceId"
val macString = macUsingAgreedMethod(deviceFingerprint, baseInfo + keyId)
if (macString.isNullOrBlank()) {
@ -211,7 +207,7 @@ internal abstract class SASDefaultVerificationTransaction(
when (info) {
is VerificationInfoStart -> onVerificationStart(info)
is VerificationInfoAccept -> onVerificationAccept(info)
is VerificationInfoKey -> onKeyVerificationKey(senderId, info)
is VerificationInfoKey -> onKeyVerificationKey(info)
is VerificationInfoMac -> onKeyVerificationMac(info)
else -> {
// nop
@ -223,7 +219,7 @@ internal abstract class SASDefaultVerificationTransaction(
abstract fun onVerificationAccept(accept: VerificationInfoAccept)
abstract fun onKeyVerificationKey(userId: String, vKey: VerificationInfoKey)
abstract fun onKeyVerificationKey(vKey: VerificationInfoKey)
abstract fun onKeyVerificationMac(vKey: VerificationInfoMac)
@ -241,7 +237,7 @@ internal abstract class SASDefaultVerificationTransaction(
val baseInfo = "MATRIX_KEY_VERIFICATION_MAC" +
otherUserId + otherDeviceId +
credentials.userId + credentials.deviceId +
userId + deviceId +
transactionId
val commaSeparatedListOfKeyIds = theirMac!!.mac!!.keys.sorted().joinToString(",")
@ -305,7 +301,7 @@ internal abstract class SASDefaultVerificationTransaction(
}
// If not me sign his MSK and upload the signature
if (otherMasterKeyIsVerified && otherUserId != credentials.userId) {
if (otherMasterKeyIsVerified && otherUserId != userId) {
// we should trust this master key
// And check verification MSK -> SSK?
crossSigningService.trustUser(otherUserId, object : MatrixCallback<Unit> {
@ -315,7 +311,7 @@ internal abstract class SASDefaultVerificationTransaction(
})
}
if (otherUserId == credentials.userId) {
if (otherUserId == userId) {
// If me it's reasonable to sign and upload the device signature
// Notice that i might not have the private keys, so may ot be able to do it
crossSigningService.signDevice(otherDeviceId!!, object : MatrixCallback<Unit> {