Skip to content

Commit d49f084

Browse files
DelmoreCooperDelmoreCooper
committed
crypto: optimize key share
1 parent 43e48d1 commit d49f084

File tree

11 files changed

+143
-178
lines changed

11 files changed

+143
-178
lines changed

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/api/session/crypto/model/CryptoDeviceInfo.kt‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ data class CryptoDeviceInfo(
2323
val deviceId: String,
2424
override val userId: String,
2525
val algorithms: List<String>? = null,
26-
override val keys: Map<String, String>? = null,
26+
override var keys: Map<String, String>? = null,
2727
override val signatures: Map<String, Map<String, String>>? = null,
2828
val unsigned: UnsignedDeviceInfo? = null,
2929
var trustLevel: DeviceTrustLevel? = null,
@@ -55,6 +55,18 @@ data class CryptoDeviceInfo(
5555
?.get("curve25519:$deviceId")
5656
}
5757

58+
fun fallbackKey(): String? {
59+
return keys
60+
?.takeIf { deviceId.isNotBlank() }
61+
?.get("fbk25519:$deviceId")
62+
}
63+
64+
fun setFallbackKey(fbk: String) {
65+
val keyMap = keys?.toMutableMap()
66+
keyMap?.put("fbk25519:$deviceId", fbk)
67+
keys = keyMap
68+
}
69+
5870
/**
5971
* @return the display name
6072
*/

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/internal/crypto/DefaultCryptoService.kt‎

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -214,56 +214,15 @@ internal class DefaultCryptoService @Inject constructor(
214214
}
215215

216216
fun onLiveEvent(roomId: String, event: Event, isInitialSync: Boolean) {
217-
// handle key require
218-
if (event.type == EventType.ROOM_KEY_REQUIRE) {
219-
if (event.originServerTs == null || event.originServerTs < System.currentTimeMillis() - 24 * 3600 * 1000) {
220-
Timber.tag(loggerTag.value).w("skip reply to old key require ${event.eventId}")
221-
return
217+
val senderUserId = event.senderId
218+
val senderDeviceId = event.content?.get("device_id")?.toString()
219+
val senderDeviceKey = event.getSenderKey()
220+
if (!senderUserId.isNullOrEmpty() && !senderDeviceId.isNullOrEmpty() && !senderDeviceKey.isNullOrEmpty()) {
221+
val deviceInfo = this.deviceListManager.getUserDevice(senderUserId, senderDeviceId)
222+
if (deviceInfo == null || deviceInfo.identityKey() != senderDeviceKey) {
223+
Timber.tag(loggerTag.value).w("unknown sender $senderUserId | $senderDeviceId from event ${event.eventId}")
224+
this.deviceListManager.handleDeviceListsChanges(listOf(senderUserId), emptyList())
222225
}
223-
val keyShareRequest = event.content.toModel<RoomKeyShareRequest>()
224-
if (event.senderId == null || keyShareRequest == null) {
225-
return
226-
}
227-
val recipients = keyShareRequest.recipients
228-
recipients?.let {
229-
for (recipient in recipients) {
230-
if (this.userId == recipient.userId && this.deviceId == recipient.deviceId) {
231-
Timber.tag(loggerTag.value).i("received ${EventType.ROOM_KEY_REQUIRE} eventId: ${event.eventId}")
232-
this.incomingKeyRequestManager.addNewIncomingRequest(event.senderId, keyShareRequest)
233-
}
234-
}
235-
}
236-
return
237-
}
238-
// handle key reply
239-
if (event.type == EventType.ROOM_KEY_REPLY) {
240-
val encryptedContent = event.content.toModel<EncryptedMessage>() ?: return
241-
if (encryptedContent.cipherText?.contains(this.olmDevice.deviceCurve25519Key) != true) {
242-
// not recipient
243-
Timber.tag(loggerTag.value).i("skip handling ${EventType.ROOM_KEY_REPLY} as not in recipient ${event.eventId}")
244-
return
245-
}
246-
247-
Timber.tag(loggerTag.value).i("start decrypting ${EventType.ROOM_KEY_REPLY} eventId: ${event.eventId}")
248-
try {
249-
val result = runBlocking { decryptEvent(event.copy(roomId = roomId), "") }
250-
event.mxDecryptionResult = OlmDecryptionResult(
251-
payload = result.clearEvent,
252-
senderKey = result.senderCurve25519Key,
253-
keysClaimed = result.claimedEd25519Key?.let { k -> mapOf("ed25519" to k) },
254-
forwardingCurve25519KeyChain = result.forwardingCurve25519KeyChain,
255-
isSafe = result.isSafe
256-
)
257-
} catch (e: MXCryptoError) {
258-
if (e is MXCryptoError.Base) {
259-
event.mCryptoError = e.errorType
260-
event.mCryptoErrorReason = e.technicalMessage.takeIf { it.isNotEmpty() } ?: e.detailedErrorDescription
261-
}
262-
return
263-
}
264-
265-
Timber.tag(loggerTag.value).i("received ${EventType.ROOM_KEY_REPLY} eventId: ${event.eventId}")
266-
this.onRoomKeyEvent(event, true)
267226
}
268227

269228
// handle state events
@@ -1117,10 +1076,6 @@ internal class DefaultCryptoService @Inject constructor(
11171076
* Upload my user's device keys.
11181077
*/
11191078
private suspend fun uploadDeviceKeys() {
1120-
if (cryptoStore.areDeviceKeysUploaded()) {
1121-
Timber.tag(loggerTag.value).d("Keys already uploaded, nothing to do")
1122-
return
1123-
}
11241079
// Prepare the device keys data to send
11251080
// Sign it
11261081
val canonicalJson = JsonCanonicalizer.getCanonicalJson(Map::class.java, getMyDevice().signalableJSONDictionary())

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/internal/crypto/DeviceListManager.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -461,8 +461,8 @@ internal class DeviceListManager @Inject constructor(
461461
Timber.e("## CRYPTO | validateDeviceKeys() : deviceKeys is null from $userId:$deviceId")
462462
return false
463463
}
464-
465-
if (null == deviceKeys.keys) {
464+
val keys = deviceKeys.keys
465+
if (null == keys) {
466466
Timber.e("## CRYPTO | validateDeviceKeys() : deviceKeys.keys is null from $userId:$deviceId")
467467
return false
468468
}
@@ -484,7 +484,7 @@ internal class DeviceListManager @Inject constructor(
484484
}
485485

486486
val signKeyId = "ed25519:" + deviceKeys.deviceId
487-
val signKey = deviceKeys.keys[signKeyId]
487+
val signKey = keys[signKeyId]
488488

489489
if (null == signKey) {
490490
Timber.e("## CRYPTO | validateDeviceKeys() : Device $userId:${deviceKeys.deviceId} has no ed25519 key")

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/internal/crypto/IncomingKeyRequestManager.kt‎

Lines changed: 11 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -213,61 +213,28 @@ internal class IncomingKeyRequestManager @Inject constructor(
213213
}
214214

215215
private suspend fun handleIncomingRequest(request: ValidMegolmRequestBody) {
216-
// We don't want to download keys, if we don't know the device yet we won't share any how?
217-
val requestingDevice = cryptoStore.getUserDevice(request.requestingUserId, request.requestingDeviceId)
218-
if (requestingDevice == null || requestingDevice.identityKey() != request.requestingDeviceKey) {
219-
directShareMegolmKey(request)
216+
if (request.requestingUserId == credentials.userId && request.requestingDeviceId == credentials.deviceId) {
217+
Timber.tag(loggerTag.value).w("ignoring key request from own user")
220218
return
221219
}
222-
223-
cryptoStore.saveIncomingKeyRequestAuditTrail(
224-
request.requestId,
225-
request.roomId,
226-
request.sessionId,
227-
request.senderKey,
228-
request.algorithm,
229-
request.requestingUserId,
230-
request.requestingDeviceId
231-
)
232-
233220
if (!arrayOf(MXCRYPTO_ALGORITHM_MEGOLM, MXCRYPTO_ALGORITHM_RATCHET).contains(request.algorithm)) {
234221
// strange we received a request for a room that is not encrypted
235222
// maybe a broken state?
236223
Timber.tag(loggerTag.value).w("Received a key request in a room with unsupported alg:${request.algorithm} , req:${request.shortDbgString()}")
237224
return
238225
}
239-
240-
// Is it for one of our sessions?
241-
if (request.requestingUserId == credentials.userId) {
242-
Timber.tag(loggerTag.value).v("handling request from own user: megolm session ${request.sessionId}")
243-
244-
if (request.requestingDeviceId == credentials.deviceId) {
245-
// ignore it's a remote echo
246-
return
247-
}
248-
// If it's verified we share from the early index we know
249-
// if not we check if it was originaly shared or not
250-
if (requestingDevice.isVerified) {
251-
// we share from the earliest known chain index
252-
shareMegolmKey(request, requestingDevice, null)
253-
} else {
254-
Timber.tag(loggerTag.value).w("requesting device is not verified, still share the key")
255-
shareMegolmKey(request, requestingDevice, null)
256-
}
257-
} else {
258-
if (cryptoConfig.limitRoomKeyRequestsToMyDevices) {
259-
Timber.tag(loggerTag.value).v("Ignore request from other user as per crypto config: ${request.shortDbgString()}")
260-
return
226+
val requestingDevice = cryptoStore.getUserDevice(request.requestingUserId, request.requestingDeviceId)
227+
if (requestingDevice != null) {
228+
if (requestingDevice.fallbackKey().isNullOrEmpty() && !request.requestingDeviceOtk.isNullOrEmpty()) {
229+
requestingDevice.setFallbackKey(request.requestingDeviceOtk)
261230
}
262-
Timber.tag(loggerTag.value).v("handling request from other user: megolm session ${request.sessionId}")
263-
if (requestingDevice.isBlocked) {
264-
// it's blocked, so send a withheld code
265-
Timber.tag(loggerTag.value).w("requesting device is blocked, will not share the key")
266-
sendWithheldForRequest(request, WithHeldCode.BLACKLISTED)
267-
} else {
268-
shareMegolmKey(request, requestingDevice, null)
231+
if (!requestingDevice.fallbackKey().isNullOrEmpty()) {
232+
if (shareMegolmKey(request, requestingDevice, null)){
233+
return
234+
}
269235
}
270236
}
237+
directShareMegolmKey(request)
271238
}
272239

273240
private suspend fun shareIfItWasPreviouslyShared(request: ValidMegolmRequestBody, requestingDevice: CryptoDeviceInfo) {

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/internal/crypto/MXOlmDevice.kt‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.sdn.android.sdk.internal.crypto
1818

1919
import androidx.annotation.VisibleForTesting
20+
import com.squareup.moshi.Types
2021
import kotlinx.coroutines.sync.Mutex
2122
import kotlinx.coroutines.sync.withLock
2223
import org.sdn.android.sdk.api.extensions.orFalse
@@ -156,10 +157,26 @@ internal class MXOlmDevice @Inject constructor(
156157
* Returns an unpublished fallback key.
157158
* A call to markKeysAsPublished will mark it as published and this
158159
* call will return null (until a call to generateFallbackKey is made).
160+
* { "curve25519":
161+
* {
162+
* "AAAABQ":"qefVZd8qvjOpsFzoKSAdfUnJVkIreyxWFlipCHjSQQg"
163+
* }
164+
* }
159165
*/
160166
fun getFallbackKey(): MutableMap<String, MutableMap<String, String>>? {
167+
val mapType = Types.newParameterizedType(MutableMap::class.java, String::class.java, String::class.java)
168+
val adapterType = Types.newParameterizedType(MutableMap::class.java, String::class.java, mapType)
169+
val adapter = MoshiProvider.providesMoshi().adapter<MutableMap<String, MutableMap<String, String>>>(adapterType)
161170
try {
162-
return store.doWithOlmAccount { it.fallbackKey() }
171+
val fbk = store.getFbk25519()
172+
if(fbk.isNotEmpty()) {
173+
return adapter.fromJson(fbk)
174+
}
175+
val fbkMap = store.doWithOlmAccount { it.fallbackKey() }
176+
if (!fbkMap[OlmAccount.JSON_KEY_ONE_TIME_KEY].isNullOrEmpty()) {
177+
store.storeFbk25519(adapter.toJson(fbkMap))
178+
}
179+
return fbkMap
163180
} catch (e: Exception) {
164181
Timber.tag(loggerTag.value).e("## getFallbackKey() : failed")
165182
}

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/internal/crypto/MyDeviceInfoHolder.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.sdn.android.sdk.internal.crypto
1818

19+
import org.matrix.olm.OlmAccount
1920
import org.sdn.android.sdk.api.auth.data.Credentials
2021
import org.sdn.android.sdk.api.session.crypto.crosssigning.DeviceTrustLevel
2122
import org.sdn.android.sdk.api.session.crypto.model.CryptoDeviceInfo
@@ -51,6 +52,15 @@ internal class MyDeviceInfoHolder @Inject constructor(
5152
keys["curve25519:" + credentials.deviceId] = olmDevice.deviceCurve25519Key!!
5253
}
5354

55+
olmDevice.generateFallbackKeyIfNeeded()
56+
olmDevice.getFallbackKey()?.let { fbkMap ->
57+
fbkMap[OlmAccount.JSON_KEY_ONE_TIME_KEY]?.let {
58+
for (item in it) {
59+
keys["fbk25519:" + credentials.deviceId] = item.value
60+
}
61+
}
62+
}
63+
5464
// myDevice.keys = keys
5565
//
5666
// myDevice.algorithms = MXCryptoAlgorithms.supportedAlgorithms()

‎sdn-sdk-android/src/main/java/org/sdn/android/sdk/internal/crypto/OneTimeKeysUploader.kt‎

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ internal class OneTimeKeysUploader @Inject constructor(
9494
Timber.w("maybeUploadOneTimeKeys: Failed to get otk count from server")
9595
}
9696

97-
Timber.d("maybeUploadOneTimeKeys: otk count $oneTimeKeyCountFromSync , unpublished fallback key ${olmDevice.hasUnpublishedFallbackKey()}")
97+
Timber.d("maybeUploadOneTimeKeys: otk count $oneTimeKeyCountFromSync")
9898

9999
lastOneTimeKeyCheck = clock.epochMillis()
100100

@@ -125,15 +125,6 @@ internal class OneTimeKeysUploader @Inject constructor(
125125
Timber.v("## uploadKeys() : success, $uploadedKeys key(s) sent")
126126
}
127127
oneTimeKeyCheckInProgress = false
128-
129-
// Check if we need to forget a fallback key
130-
val latestPublishedTime = getLastFallbackKeyPublishTime()
131-
if (latestPublishedTime != 0L && clock.epochMillis() - latestPublishedTime > FALLBACK_KEY_FORGET_DELAY) {
132-
// This should be called once you are reasonably certain that you will not receive any more messages
133-
// that use the old fallback key
134-
Timber.d("## forgetFallbackKey()")
135-
olmDevice.forgetFallbackKey()
136-
}
137128
}
138129

139130
private suspend fun fetchOtkCount(): Int? {
@@ -151,33 +142,19 @@ internal class OneTimeKeysUploader @Inject constructor(
151142
* @return the number of uploaded keys
152143
*/
153144
private suspend fun uploadOTK(keyCount: Int, keyLimit: Int): Int {
154-
if (keyLimit <= keyCount && !olmDevice.hasUnpublishedFallbackKey()) {
145+
if (keyLimit <= keyCount) {
155146
// If we don't need to generate any more keys then we are done.
156147
return 0
157148
}
158-
var keysThisLoop = 0
159-
if (keyLimit > keyCount) {
160-
// Creating keys can be an expensive operation so we limit the
161-
// number we generate in one go to avoid blocking the application
162-
// for too long.
163-
keysThisLoop = min(keyLimit - keyCount, ONE_TIME_KEY_GENERATION_MAX_NUMBER)
164-
olmDevice.generateOneTimeKeys(keysThisLoop)
165-
}
166-
167-
// We check before sending if there is an unpublished key in order to saveLastFallbackKeyPublishTime if needed
168-
val hadUnpublishedFallbackKey = olmDevice.hasUnpublishedFallbackKey()
149+
val keysThisLoop = min(keyLimit - keyCount, ONE_TIME_KEY_GENERATION_MAX_NUMBER)
150+
olmDevice.generateOneTimeKeys(keysThisLoop)
169151
val response = uploadOneTimeKeys(olmDevice.getOneTimeKeys())
170152
olmDevice.markKeysAsPublished()
171-
if (hadUnpublishedFallbackKey) {
172-
// It had an unpublished fallback key that was published just now
173-
saveLastFallbackKeyPublishTime(clock.epochMillis())
174-
}
175153

176154
if (response.hasOneTimeKeyCountsForAlgorithm(MXKey.KEY_SIGNED_CURVE_25519_TYPE)) {
177155
// Maybe upload other keys
178156
return keysThisLoop +
179-
uploadOTK(response.oneTimeKeyCountsForAlgorithm(MXKey.KEY_SIGNED_CURVE_25519_TYPE), keyLimit) +
180-
(if (hadUnpublishedFallbackKey) 1 else 0)
157+
uploadOTK(response.oneTimeKeyCountsForAlgorithm(MXKey.KEY_SIGNED_CURVE_25519_TYPE), keyLimit) + 1
181158
} else {
182159
Timber.e("## uploadOTK() : response for uploading keys does not contain one_time_key_counts.signed_curve25519")
183160
throw Exception("response for uploading keys does not contain one_time_key_counts.signed_curve25519")
@@ -213,6 +190,7 @@ internal class OneTimeKeysUploader @Inject constructor(
213190
}
214191

215192
val fallbackJson = mutableMapOf<String, Any>()
193+
olmDevice.generateFallbackKeyIfNeeded()
216194
val fallbackCurve25519Map = olmDevice.getFallbackKey()?.get(OlmAccount.JSON_KEY_ONE_TIME_KEY).orEmpty()
217195
fallbackCurve25519Map.forEach { (key_id, key) ->
218196
val k = mutableMapOf<String, Any>()

0 commit comments

Comments
 (0)