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

Add crypto mode setting for invisible crypto, and apply it to decrypting events #4407

Merged
merged 8 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
],
"dependencies": {
"@babel/runtime": "^7.12.5",
"@matrix-org/matrix-sdk-crypto-wasm": "^8.0.0",
"@matrix-org/matrix-sdk-crypto-wasm": "^9.0.0",
"@matrix-org/olm": "3.2.15",
"another-json": "^0.2.0",
"bs58": "^6.0.0",
Expand Down
82 changes: 82 additions & 0 deletions spec/integ/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ import { SecretStorageKeyDescription } from "../../../src/secret-storage";
import {
CrossSigningKey,
CryptoCallbacks,
CryptoMode,
DecryptionFailureCode,
EventShieldColour,
EventShieldReason,
Expand Down Expand Up @@ -746,6 +747,87 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
);
});

newBackendOnly(
"fails with an error when cross-signed sender is required but sender is not cross-signed",
async () => {
// This tests that a message will not be decrypted if the sender
// is not sufficiently trusted according to the selected crypto
// mode.
//
// This test is almost the same as the "Alice receives a megolm
// message" test, with the main difference that we set the
// crypto mode.
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });

// Start by using Invisible crypto mode
aliceClient.getCrypto()!.setCryptoMode(CryptoMode.Invisible);

await startClientAndAwaitFirstSync();

const p2pSession = await createOlmSession(testOlmAccount, keyReceiver);
const groupSession = new Olm.OutboundGroupSession();
groupSession.create();

// make the room_key event
const roomKeyEncrypted = encryptGroupSessionKey({
recipient: aliceClient.getUserId()!,
recipientCurve25519Key: keyReceiver.getDeviceKey(),
recipientEd25519Key: keyReceiver.getSigningKey(),
olmAccount: testOlmAccount,
p2pSession: p2pSession,
groupSession: groupSession,
room_id: ROOM_ID,
});

// encrypt a message with the group session
const messageEncrypted = encryptMegolmEvent({
senderKey: testSenderKey,
groupSession: groupSession,
room_id: ROOM_ID,
});

// Alice gets both the events in a single sync
const syncResponse = {
next_batch: 1,
to_device: {
events: [roomKeyEncrypted],
},
rooms: {
join: {
[ROOM_ID]: { timeline: { events: [messageEncrypted] } },
},
},
};

syncResponder.sendOrQueueSyncResponse(syncResponse);
await syncPromise(aliceClient);

const room = aliceClient.getRoom(ROOM_ID)!;
const event = room.getLiveTimeline().getEvents()[0];
expect(event.isEncrypted()).toBe(true);

// it probably won't be decrypted yet, because it takes a while to process the olm keys
const decryptedEvent = await testUtils.awaitDecryption(event);
// It will error as an unknown device because we haven't fetched
// the sender's device keys.
expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE);

// Next, try decrypting in transition mode, which should also
// fail for the same reason
aliceClient.getCrypto()!.setCryptoMode(CryptoMode.Transition);

await event.attemptDecryption(aliceClient["cryptoBackend"]!);
expect(decryptedEvent.decryptionFailureReason).toEqual(DecryptionFailureCode.UNKNOWN_SENDER_DEVICE);

// Decrypting in legacy mode should succeed since it doesn't
// care about device trust.
aliceClient.getCrypto()!.setCryptoMode(CryptoMode.Legacy);

await event.attemptDecryption(aliceClient["cryptoBackend"]!);
expect(decryptedEvent.decryptionFailureReason).toEqual(null);
},
);

it("Decryption fails with Unable to decrypt for other errors", async () => {
expectAliceKeyQuery({ device_keys: { "@alice:localhost": {} }, failures: {} });
await startClientAndAwaitFirstSync();
Expand Down
51 changes: 51 additions & 0 deletions src/crypto-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ export interface CryptoApi {
*/
globalBlacklistUnverifiedDevices: boolean;

/**
* The cryptography mode to use.
richvdh marked this conversation as resolved.
Show resolved Hide resolved
*/
setCryptoMode(cryptoMode: CryptoMode): void;

/**
* Return the current version of the crypto module.
* For example: `Rust SDK ${versions.matrix_sdk_crypto} (${versions.git_sha}), Vodozemac ${versions.vodozemac}`.
Expand Down Expand Up @@ -589,6 +594,24 @@ export enum DecryptionFailureCode {
*/
HISTORICAL_MESSAGE_USER_NOT_JOINED = "HISTORICAL_MESSAGE_USER_NOT_JOINED",

/**
* The sender's identity is not verified, but was previously verified.
*/
SENDER_IDENTITY_PREVIOUSLY_VERIFIED = "SENDER_IDENTITY_PREVIOUSLY_VERIFIED",

/**
* The sender device is not cross-signed. This will only be used if the
* crypto mode is set to `CryptoMode.Invisible` or `CryptoMode.Transition`.
*/
UNSIGNED_SENDER_DEVICE = "UNSIGNED_SENDER_DEVICE",

/**
* We weren't able to link the message back to any known device. This will
* only be used if the crypto mode is set to `CryptoMode.Invisible` or
* `CryptoMode.Transition`.
*/
UNKNOWN_SENDER_DEVICE = "UNKNOWN_SENDER_DEVICE",

/** Unknown or unclassified error. */
UNKNOWN_ERROR = "UNKNOWN_ERROR",

Expand Down Expand Up @@ -632,6 +655,34 @@ export enum DecryptionFailureCode {
UNKNOWN_ENCRYPTION_ALGORITHM = "UNKNOWN_ENCRYPTION_ALGORITHM",
}

/**
* The cryptography mode. Affects how messages are encrypted and decrypted.
* Only supported by Rust crypto.
*/
export enum CryptoMode {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure if we want to have a single knob that affects both encryption and decryption, or if we want to be able to tweak the encryption and decryption settings separately. Having a single setting is possibly easier to use, but gives less control.

Copy link
Member

Choose a reason for hiding this comment

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

We think this is fine.

/**
* Events are encrypted for all devices in the room, except for
richvdh marked this conversation as resolved.
Show resolved Hide resolved
* blacklisted devices, or unverified devices if
* `globalBlacklistUnverifiedDevices` is set. Events from all senders are
* decrypted.
*/
Legacy,
/**
richvdh marked this conversation as resolved.
Show resolved Hide resolved
* Events are encrypted as with `Legacy` mode, but will throw an error if a
richvdh marked this conversation as resolved.
Show resolved Hide resolved
* verified user has an unsigned device, or if a verified user replaces
* their identity. Events are decrypted only if they come from cross-signed
* devices, or devices that existed before the Rust crypto SDK started
* tracking device trust.
richvdh marked this conversation as resolved.
Show resolved Hide resolved
*/
Transition,
/**
richvdh marked this conversation as resolved.
Show resolved Hide resolved
* Events are only encrypted for cross-signed devices. Will throw an error
* when encrypting if a verified user replaces their identity. Events are
richvdh marked this conversation as resolved.
Show resolved Hide resolved
* decrypted only if they come from a cross-signed device.
richvdh marked this conversation as resolved.
Show resolved Hide resolved
*/
Invisible,
}

/**
* Options object for `CryptoApi.bootstrapCrossSigning`.
*/
Expand Down
8 changes: 8 additions & 0 deletions src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import {
BootstrapCrossSigningOpts,
CrossSigningKeyInfo,
CrossSigningStatus,
CryptoMode,
decodeRecoveryKey,
DecryptionFailureCode,
DeviceVerificationStatus,
Expand Down Expand Up @@ -648,6 +649,13 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
this.backupManager.checkAndStart();
}

/**
* Implementation of {@link Crypto.CryptoApi#setCryptoMode}.
*/
public setCryptoMode(cryptoMode: CryptoMode): void {
throw new Error("Not supported");
}

/**
* Implementation of {@link Crypto.CryptoApi#getVersion}.
*/
Expand Down
59 changes: 56 additions & 3 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
CrossSigningStatus,
CryptoApi,
CryptoCallbacks,
CryptoMode,
Curve25519AuthData,
DecryptionFailureCode,
DeviceVerificationStatus,
Expand Down Expand Up @@ -202,6 +203,15 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
//
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////

private cryptoMode = CryptoMode.Legacy;

/**
* Implementation of {@link Crypto.CryptoApi#setCryptoMode}.
*/
public setCryptoMode(cryptoMode: CryptoMode): void {
this.cryptoMode = cryptoMode;
}
richvdh marked this conversation as resolved.
Show resolved Hide resolved

public set globalErrorOnUnknownDevices(_v: boolean) {
// Not implemented for rust crypto.
}
Expand Down Expand Up @@ -252,7 +262,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
// through decryptEvent and hence get rid of this case.
throw new Error("to-device event was not decrypted in preprocessToDeviceMessages");
}
return await this.eventDecryptor.attemptEventDecryption(event);
return await this.eventDecryptor.attemptEventDecryption(event, this.cryptoMode);
}

/**
Expand Down Expand Up @@ -1730,18 +1740,31 @@ class EventDecryptor {
private readonly perSessionBackupDownloader: PerSessionKeyBackupDownloader,
) {}

public async attemptEventDecryption(event: MatrixEvent): Promise<IEventDecryptionResult> {
public async attemptEventDecryption(event: MatrixEvent, cryptoMode: CryptoMode): Promise<IEventDecryptionResult> {
// add the event to the pending list *before* attempting to decrypt.
// then, if the key turns up while decryption is in progress (and
// decryption fails), we will schedule a retry.
// (fixes https://github.com/vector-im/element-web/issues/5001)
this.addEventToPendingList(event);

let trustRequirement;
switch (cryptoMode) {
case CryptoMode.Legacy:
trustRequirement = RustSdkCryptoJs.TrustRequirement.Untrusted;
break;
case CryptoMode.Transition:
trustRequirement = RustSdkCryptoJs.TrustRequirement.CrossSignedOrLegacy;
break;
case CryptoMode.Invisible:
trustRequirement = RustSdkCryptoJs.TrustRequirement.CrossSigned;
break;
}

try {
const res = (await this.olmMachine.decryptRoomEvent(
stringifyEvent(event),
new RustSdkCryptoJs.RoomId(event.getRoomId()!),
new RustSdkCryptoJs.DecryptionSettings(RustSdkCryptoJs.TrustRequirement.Untrusted),
new RustSdkCryptoJs.DecryptionSettings(trustRequirement),
)) as RustSdkCryptoJs.DecryptedRoomEvent;

// Success. We can remove the event from the pending list, if
Expand Down Expand Up @@ -1849,6 +1872,36 @@ class EventDecryptor {
errorDetails,
);

case RustSdkCryptoJs.DecryptionErrorCode.SenderIdentityPreviouslyVerified:
// We're refusing to decrypt due to not trusting the sender,
// rather than failing to decrypt due to lack of keys, so we
// don't need to keep it on the pending list.
this.removeEventFromPendingList(event);
throw new DecryptionError(
DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED,
"The sender identity is unverified, but was previously verified.",
);

case RustSdkCryptoJs.DecryptionErrorCode.UnknownSenderDevice:
// We're refusing to decrypt due to not trusting the sender,
// rather than failing to decrypt due to lack of keys, so we
// don't need to keep it on the pending list.
this.removeEventFromPendingList(event);
throw new DecryptionError(
DecryptionFailureCode.UNKNOWN_SENDER_DEVICE,
"The sender device is not known.",
);

case RustSdkCryptoJs.DecryptionErrorCode.UnsignedSenderDevice:
// We're refusing to decrypt due to not trusting the sender,
// rather than failing to decrypt due to lack of keys, so we
// don't need to keep it on the pending list.
this.removeEventFromPendingList(event);
throw new DecryptionError(
DecryptionFailureCode.UNSIGNED_SENDER_DEVICE,
"The sender identity is not cross-signed.",
);

// We don't map MismatchedIdentityKeys for now, as there is no equivalent in legacy.
// Just put it on the `UNKNOWN_ERROR` bucket.
default:
Expand Down
39 changes: 7 additions & 32 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1453,10 +1453,10 @@
"@jridgewell/resolve-uri" "^3.1.0"
"@jridgewell/sourcemap-codec" "^1.4.14"

"@matrix-org/matrix-sdk-crypto-wasm@^8.0.0":
version "8.0.0"
resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-wasm/-/matrix-sdk-crypto-wasm-8.0.0.tgz#6ddc0e63538e821a2efbc5c1a2f0fa0f71d489ff"
integrity sha512-s0q3O2dK8b6hOJ+SZFz+s/IiMabmVsNue6r17sTwbrRD8liBkCrpjYnxoMYvtC01GggJ9TZLQbeqpt8hQSPHAg==
"@matrix-org/matrix-sdk-crypto-wasm@^9.0.0":
version "9.0.0"
resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-wasm/-/matrix-sdk-crypto-wasm-9.0.0.tgz#293fe8fcb9bc4d577c5f6cf2cbffa151c6e11329"
integrity sha512-dz4dkYXj6BeOQuw52XQj8dMuhi85pSFhfFeFlNRAO7JdRPhE9CHBrfK8knkZV5Zux5vvf3Ub4E7myoLeJgZoEw==

"@matrix-org/[email protected]":
version "3.2.15"
Expand Down Expand Up @@ -5794,16 +5794,7 @@ string-length@^4.0.1:
char-regex "^1.0.2"
strip-ansi "^6.0.0"

"string-width-cjs@npm:string-width@^4.2.0":
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"

string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand Down Expand Up @@ -5858,14 +5849,7 @@ string.prototype.trimstart@^1.0.8:
define-properties "^1.2.1"
es-object-atoms "^1.0.0"

"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"

strip-ansi@^6.0.0, strip-ansi@^6.0.1:
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
Expand Down Expand Up @@ -6397,16 +6381,7 @@ word-wrap@^1.2.5:
resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.5.tgz#d2c45c6dd4fbce621a66f136cbe328afd0410b34"
integrity sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^7.0.0:
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand Down