Skip to content

Commit

Permalink
Element R: fix isCrossSigningReady not checking identity trust (#4156)
Browse files Browse the repository at this point in the history
* Fix inconsistency between rust and legacy

* Add tests

* Review: better comment

Co-authored-by: Richard van der Hoff <[email protected]>

* review: Better doc

Co-authored-by: Richard van der Hoff <[email protected]>

* Simplify test data and some comments

---------

Co-authored-by: Richard van der Hoff <[email protected]>
  • Loading branch information
BillCarsonFr and richvdh authored Apr 17, 2024
1 parent 4fc6ba8 commit d22a39f
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
61 changes: 61 additions & 0 deletions spec/integ/crypto/cross-signing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,67 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s

expect(isCrossSigningReady).toBeTruthy();
});

it("should return false if identity is not trusted, even if the secrets are in 4S", async () => {
e2eKeyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA);

// Complete initial sync, to get the 4S account_data events stored
mockInitialApiRequests(aliceClient.getHomeserverUrl());

// For this test we need to have a well-formed 4S setup.
const mockSecretInfo = {
encrypted: {
// Don't care about the actual values here, just need to be present for validation
KeyId: {
iv: "IVIVIVIVIVIVIV",
ciphertext: "CIPHERTEXTB64",
mac: "MACMACMAC",
},
},
};
syncResponder.sendOrQueueSyncResponse({
next_batch: 1,
account_data: {
events: [
{
type: "m.secret_storage.key.KeyId",
content: {
algorithm: "m.secret_storage.v1.aes-hmac-sha2",
// iv and mac not relevant for this test
},
},
{
type: "m.secret_storage.default_key",
content: {
key: "KeyId",
},
},
{
type: "m.cross_signing.master",
content: mockSecretInfo,
},
{
type: "m.cross_signing.user_signing",
content: mockSecretInfo,
},
{
type: "m.cross_signing.self_signing",
content: mockSecretInfo,
},
],
},
});
await aliceClient.startClient();
await syncPromise(aliceClient);

// Sanity: ensure that the secrets are in 4S
const status = await aliceClient.getCrypto()!.getCrossSigningStatus();
expect(status.privateKeysInSecretStorage).toBeTruthy();

const isCrossSigningReady = await aliceClient.getCrypto()!.isCrossSigningReady();

expect(isCrossSigningReady).toBeFalsy();
});
});

describe("getCrossSigningKeyId", () => {
Expand Down
10 changes: 6 additions & 4 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -610,15 +610,17 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
* Implementation of {@link CryptoApi#isCrossSigningReady}
*/
public async isCrossSigningReady(): Promise<boolean> {
const { publicKeysOnDevice, privateKeysInSecretStorage, privateKeysCachedLocally } =
await this.getCrossSigningStatus();
const { privateKeysInSecretStorage, privateKeysCachedLocally } = await this.getCrossSigningStatus();
const hasKeysInCache =
Boolean(privateKeysCachedLocally.masterKey) &&
Boolean(privateKeysCachedLocally.selfSigningKey) &&
Boolean(privateKeysCachedLocally.userSigningKey);

// The cross signing is ready if the public and private keys are available
return publicKeysOnDevice && (hasKeysInCache || privateKeysInSecretStorage);
const identity = await this.getOwnIdentity();

// Cross-signing is ready if the public identity is trusted, and the private keys
// are either cached, or accessible via secret-storage.
return !!identity?.isVerified() && (hasKeysInCache || privateKeysInSecretStorage);
}

/**
Expand Down

0 comments on commit d22a39f

Please sign in to comment.