matrix-rust-sdk icon indicating copy to clipboard operation
matrix-rust-sdk copied to clipboard

KeyBackup | Improve `verify_backup`API to return more details on why backup is trusted

Open BillCarsonFr opened this issue 2 years ago • 6 comments

Currently verify_backup is only returning a boolean if the backup is trusted. Current EA API returns more info, it returns if trusted as well as the list of signature and which of them are actually trusted.

Also It appears that rust sdk doesn't support using a backup when it's signed by the current user MSK.

Current android API returns

data class KeysBackupVersionTrust(
        /**
         * Flag to indicate if the backup is trusted.
         * true if there is a signature that is valid & from a trusted device.
         */
        val usable: Boolean,

        /**
         * Signatures found in the backup version.
         */
        val signatures: List<KeysBackupVersionTrustSignature> = emptyList()
)
sealed class KeysBackupVersionTrustSignature {

    data class DeviceSignature(
            /**
             * The id of the device that signed the backup version.
             */
            val deviceId: String?,

            /**
             * The device that signed the backup version.
             * Can be null if the device is not known.
             */
            val device: CryptoDeviceInfo?,

            /**
             * Flag to indicate the signature from this device is valid.
             */
            val valid: Boolean) : KeysBackupVersionTrustSignature()

    data class UserSignature(
            val keyId: String?,
            val cryptoCrossSigningKey: CryptoCrossSigningKey?,
            val valid: Boolean
    ) : KeysBackupVersionTrustSignature()
}

It allows to display some advanced UX like that: image

BillCarsonFr avatar May 24 '22 12:05 BillCarsonFr

That API seems to be a bit heavy for what the suggested use-case in the screenshot is.

Should we really return all the signatures? Even the invalid ones? Even of other users?

Why is deviceId in DeviceSignature optional? Same for keyId in UserSignature? Do we really want to transfer all the devices and user identities as part of this to essentially answer the question "Does our device trust this backup" and "Does our user identity trust this backup".

The screenshot has the sentence "The backup has a valid signature from FOO", does this mean that we're just checking if it's a valid signature or if it's valid and trusted?

Would the following be enough to get the desired UI? This would make sure we don't transfer potentially huge amounts of data over FFI to just discard it in the end.

KeysBackupVersionTrust {
    usable: bool,
    device_signature: SignatureState,
    user_signature: SignatureState
}
pub enum SignatureState {
    Missing,
    Invalid,
    Valid,
    Trusted,
}

poljar avatar May 26 '22 13:05 poljar

Yes it doesn't have to match what was in EA. The screenshot I sent is not complete, but you can find it on web also. You can see if a signature is valid/unknown/invalid/trusted, exactly like your proposed enum.

Could we still get a list? and deviceId/userId to match with device/users?

BillCarsonFr avatar May 27 '22 11:05 BillCarsonFr

We could, though that would mean we check N signatures even though 2 signature checks might be enough for the functionality.

Do we want to check every signature?

poljar avatar May 27 '22 11:05 poljar

Well not sure. We were using it during tests, and the UI looks a bit like a devtool thing. Do I have API now to verify the signature manually from the MegolmBackup Auth Data ? could use that for test or devtools.

BillCarsonFr avatar May 27 '22 12:05 BillCarsonFr

Oh this is only for tests? Perhaps a flag that determines if we should check all signatures would be the way forward then?

Something like fn verify_backup(auth_data: Value, check_all: bool) -> KeysBackupVersionTrust. The flag would modify the behavior.

If the flag is false:

  1. Check if we have a valid and trusted signature from our own device and our own user identity
  2. If we don't have the above, check all signatures until you find one from a trusted device

If the flag is true:

  1. Same as for when the flag is false
  2. Go through all the other signatures and collect the results for each signature and put it into a map BTreeMap<UserId, BTreeMap<DeviceId, SignatureState>>

Does this sound sensible?

poljar avatar May 27 '22 12:05 poljar

yes would be perfect

BillCarsonFr avatar May 27 '22 12:05 BillCarsonFr