Skip to content

Commit

Permalink
Add CryptoApi.pinCurrentUserIdentity and `UserIdentity.needsUserApp…
Browse files Browse the repository at this point in the history
…roval` (#4415)

* Implement `UserVerificationStatus.needsUserApproval`

Expose the `identityNeedsUserApproval` flag from the rust crypto crate.

* Add CryptoApi.pinCurrentUserIdentity

Expose `pinCurrentMasterKey` from the rust crypto api.

* Test data: add second cross-signing key for Bob

* Add tests for verification status
  • Loading branch information
richvdh authored Sep 24, 2024
1 parent d0890d9 commit 1a8ea3d
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 16 deletions.
65 changes: 59 additions & 6 deletions spec/integ/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3261,15 +3261,13 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
});
});

describe("Check if the cross signing keys are available for a user", () => {
describe("User identity", () => {
let keyResponder: E2EKeyResponder;
beforeEach(async () => {
// anything that we don't have a specific matcher for silently returns a 404
fetchMock.catch(404);

const keyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl());
keyResponder = new E2EKeyResponder(aliceClient.getHomeserverUrl());
keyResponder.addCrossSigningData(SIGNED_CROSS_SIGNING_KEYS_DATA);
keyResponder.addDeviceKeys(SIGNED_TEST_DEVICE_DATA);
keyResponder.addKeyReceiver(BOB_TEST_USER_ID, keyReceiver);
keyResponder.addKeyReceiver(TEST_USER_ID, keyReceiver);
keyResponder.addCrossSigningData(BOB_SIGNED_CROSS_SIGNING_KEYS_DATA);
keyResponder.addDeviceKeys(BOB_SIGNED_TEST_DEVICE_DATA);

Expand All @@ -3285,6 +3283,12 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
.getCrypto()!
.userHasCrossSigningKeys(BOB_TEST_USER_ID, true);
expect(hasCrossSigningKeysForUser).toBe(true);

const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID);
expect(verificationStatus.isVerified()).toBe(false);
expect(verificationStatus.isCrossSigningVerified()).toBe(false);
expect(verificationStatus.wasCrossSigningVerified()).toBe(false);
expect(verificationStatus.needsUserApproval).toBe(false);
});

it("Cross signing keys are available for a tracked user", async () => {
Expand All @@ -3295,11 +3299,60 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
// Alice is the local user and should be tracked !
const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys(TEST_USER_ID);
expect(hasCrossSigningKeysForUser).toBe(true);

const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID);
expect(verificationStatus.isVerified()).toBe(false);
expect(verificationStatus.isCrossSigningVerified()).toBe(false);
expect(verificationStatus.wasCrossSigningVerified()).toBe(false);
expect(verificationStatus.needsUserApproval).toBe(false);
});

it("Cross signing keys are not available for an unknown user", async () => {
const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys("@unknown:xyz");
expect(hasCrossSigningKeysForUser).toBe(false);

const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID);
expect(verificationStatus.isVerified()).toBe(false);
expect(verificationStatus.isCrossSigningVerified()).toBe(false);
expect(verificationStatus.wasCrossSigningVerified()).toBe(false);
expect(verificationStatus.needsUserApproval).toBe(false);
});

newBackendOnly("An unverified user changes identity", async () => {
// We have to be tracking Bob's keys, which means we need to share a room with him
syncResponder.sendOrQueueSyncResponse({
...getSyncResponse([BOB_TEST_USER_ID]),
device_lists: { changed: [BOB_TEST_USER_ID] },
});
await syncPromise(aliceClient);

const hasCrossSigningKeysForUser = await aliceClient.getCrypto()!.userHasCrossSigningKeys(BOB_TEST_USER_ID);
expect(hasCrossSigningKeysForUser).toBe(true);

// Bob changes his cross-signing keys
keyResponder.addCrossSigningData(testData.BOB_ALT_SIGNED_CROSS_SIGNING_KEYS_DATA);
syncResponder.sendOrQueueSyncResponse({
next_batch: "2",
device_lists: { changed: [BOB_TEST_USER_ID] },
});
await syncPromise(aliceClient);

await aliceClient.getCrypto()!.userHasCrossSigningKeys(BOB_TEST_USER_ID, true);

{
const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID);
expect(verificationStatus.isVerified()).toBe(false);
expect(verificationStatus.isCrossSigningVerified()).toBe(false);
expect(verificationStatus.wasCrossSigningVerified()).toBe(false);
expect(verificationStatus.needsUserApproval).toBe(true);
}

// Pinning the new identity should clear the needsUserApproval flag.
await aliceClient.getCrypto()!.pinCurrentUserIdentity(BOB_TEST_USER_ID);
{
const verificationStatus = await aliceClient.getCrypto()!.getUserVerificationStatus(BOB_TEST_USER_ID);
expect(verificationStatus.needsUserApproval).toBe(false);
}
});
});

Expand Down
21 changes: 15 additions & 6 deletions spec/test-utils/test-data/generate-test-data.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"TEST_DEVICE_CURVE_PRIVATE_KEY_BYTES": b"Deadmuledeadmuledeadmuledeadmule",

"MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"Doyouspeakwhaaaaaaaaaaaaaaaaaale",
"ALT_MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"DoYouSpeakWhaaaaaaaaaaaaaaaaaale",
"USER_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"Useruseruseruseruseruseruseruser",
"SELF_CROSS_SIGNING_PRIVATE_KEY_BYTES": b"Selfselfselfselfselfselfselfself",

Expand Down Expand Up @@ -208,7 +209,7 @@ def build_test_data(user_data, prefix = "") -> str:

backup_recovery_key = export_recovery_key(user_data["B64_BACKUP_DECRYPTION_KEY"])

return f"""\
result = f"""\
export const {prefix}TEST_USER_ID = "{user_data['TEST_USER_ID']}";
export const {prefix}TEST_DEVICE_ID = "{user_data['TEST_DEVICE_ID']}";
export const {prefix}TEST_ROOM_ID = "{user_data['TEST_ROOM_ID']}";
Expand Down Expand Up @@ -239,7 +240,7 @@ def build_test_data(user_data, prefix = "") -> str:
/** Signed cross-signing keys data, also suitable for returning from a `/keys/query` call */
export const {prefix}SIGNED_CROSS_SIGNING_KEYS_DATA: Partial<IDownloadKeyResult> = {
json.dumps(build_cross_signing_keys_data(user_data), indent=4)
json.dumps(build_cross_signing_keys_data(user_data, user_data["MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES"]), indent=4)
};
/** Signed OTKs, returned by `POST /keys/claim` */
Expand Down Expand Up @@ -279,12 +280,20 @@ def build_test_data(user_data, prefix = "") -> str:
export const {prefix}ENCRYPTED_EVENT: Partial<IEvent> = {json.dumps(encrypted_event, indent=4)};
"""

alt_master_key = user_data.get("ALT_MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES")
if alt_master_key is not None:
result += f"""
/** A second set of signed cross-signing keys data, also suitable for returning from a `/keys/query` call */
export const {prefix}ALT_SIGNED_CROSS_SIGNING_KEYS_DATA: Partial<IDownloadKeyResult> = {
json.dumps(build_cross_signing_keys_data(user_data, alt_master_key), indent=4)
};
"""

return result

def build_cross_signing_keys_data(user_data) -> dict:
def build_cross_signing_keys_data(user_data, master_key_bytes) -> dict:
"""Build the signed cross-signing-keys data for return from /keys/query"""
master_private_key = ed25519.Ed25519PrivateKey.from_private_bytes(
user_data["MASTER_CROSS_SIGNING_PRIVATE_KEY_BYTES"]
)
master_private_key = ed25519.Ed25519PrivateKey.from_private_bytes(master_key_bytes)
b64_master_public_key = encode_base64(
master_private_key.public_key().public_bytes(Encoding.Raw, PublicFormat.Raw)
)
Expand Down
47 changes: 47 additions & 0 deletions spec/test-utils/test-data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,3 +449,50 @@ export const BOB_ENCRYPTED_EVENT: Partial<IEvent> = {
"origin_server_ts": 1507753886000
};

/** A second set of signed cross-signing keys data, also suitable for returning from a `/keys/query` call */
export const BOB_ALT_SIGNED_CROSS_SIGNING_KEYS_DATA: Partial<IDownloadKeyResult> = {
"master_keys": {
"@bob:xyz": {
"keys": {
"ed25519:MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E": "MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E"
},
"user_id": "@bob:xyz",
"usage": [
"master"
]
}
},
"self_signing_keys": {
"@bob:xyz": {
"keys": {
"ed25519:DaScI3WulBvDjf/d2vdyP5Cgjdypn1c/PSDX23MgN+A": "DaScI3WulBvDjf/d2vdyP5Cgjdypn1c/PSDX23MgN+A"
},
"user_id": "@bob:xyz",
"usage": [
"self_signing"
],
"signatures": {
"@bob:xyz": {
"ed25519:MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E": "eDZETBRUw9yW0WJnBZ7vxo12TW09Yb7/47qBPKZzPZzZEvs9M82dnAOtWUv00mcTdp2K9GpeFYDQJ6qLQgxaCA"
}
}
}
},
"user_signing_keys": {
"@bob:xyz": {
"keys": {
"ed25519:lXP89FP6zvFH9TSbU1S8uSdAsVawm1NmV6z+Rfr3lEw": "lXP89FP6zvFH9TSbU1S8uSdAsVawm1NmV6z+Rfr3lEw"
},
"user_id": "@bob:xyz",
"usage": [
"user_signing"
],
"signatures": {
"@bob:xyz": {
"ed25519:MCYxU7myKVkoQ55VYw/rXdg5cEupRfDdHmFPJUmR5+E": "Q1CbIXvp2BxBsu3F/eZ1ZpuR5rXIt0+FrrA/l6itskpW748xwMoIKxQRVQqs87kh7pCsWEoTy6FzIL8nV+P6BQ"
}
}
}
}
};

43 changes: 41 additions & 2 deletions spec/unit/rust-crypto/rust-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1362,13 +1362,52 @@ describe("RustCrypto", () => {
});

it("returns a verified UserVerificationStatus when the UserIdentity is verified", async () => {
olmMachine.getIdentity.mockResolvedValue({ free: jest.fn(), isVerified: jest.fn().mockReturnValue(true) });
olmMachine.getIdentity.mockResolvedValue({
free: jest.fn(),
isVerified: jest.fn().mockReturnValue(true),
wasPreviouslyVerified: jest.fn().mockReturnValue(true),
});

const userVerificationStatus = await rustCrypto.getUserVerificationStatus(testData.TEST_USER_ID);
expect(userVerificationStatus.isVerified()).toBeTruthy();
expect(userVerificationStatus.isTofu()).toBeFalsy();
expect(userVerificationStatus.isCrossSigningVerified()).toBeTruthy();
expect(userVerificationStatus.wasCrossSigningVerified()).toBeFalsy();
expect(userVerificationStatus.wasCrossSigningVerified()).toBeTruthy();
});
});

describe("pinCurrentIdentity", () => {
let rustCrypto: RustCrypto;
let olmMachine: Mocked<RustSdkCryptoJs.OlmMachine>;

beforeEach(() => {
olmMachine = {
getIdentity: jest.fn(),
} as unknown as Mocked<RustSdkCryptoJs.OlmMachine>;
rustCrypto = new RustCrypto(
logger,
olmMachine,
{} as MatrixClient["http"],
TEST_USER,
TEST_DEVICE_ID,
{} as ServerSideSecretStorage,
{} as CryptoCallbacks,
);
});

it("throws an error for an unknown user", async () => {
await expect(rustCrypto.pinCurrentUserIdentity("@alice:example.com")).rejects.toThrow(
"Cannot pin identity of unknown user",
);
});

it("throws an error for our own user", async () => {
const ownIdentity = new RustSdkCryptoJs.OwnUserIdentity();
olmMachine.getIdentity.mockResolvedValue(ownIdentity);

await expect(rustCrypto.pinCurrentUserIdentity("@alice:example.com")).rejects.toThrow(
"Cannot pin identity of own user",
);
});
});

Expand Down
32 changes: 31 additions & 1 deletion src/crypto-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,16 @@ export interface CryptoApi {
*/
getUserVerificationStatus(userId: string): Promise<UserVerificationStatus>;

/**
* "Pin" the current identity of the given user, accepting it as genuine.
*
* This is useful if the user has changed identity since we first saw them (leading to
* {@link UserVerificationStatus.needsUserApproval}), and we are now accepting their new identity.
*
* Throws an error if called on our own user ID, or on a user ID that we don't have an identity for.
*/
pinCurrentUserIdentity(userId: string): Promise<void>;

/**
* Get the verification status of a given device.
*
Expand Down Expand Up @@ -707,11 +717,29 @@ export interface BootstrapCrossSigningOpts {
* Represents the ways in which we trust a user
*/
export class UserVerificationStatus {
/**
* Indicates if the identity has changed in a way that needs user approval.
*
* This happens if the identity has changed since we first saw it, *unless* the new identity has also been verified
* by our user (eg via an interactive verification).
*
* To rectify this, either:
*
* * Conduct a verification of the new identity via {@link CryptoApi.requestVerificationDM}.
* * Pin the new identity, via {@link CryptoApi.pinCurrentUserIdentity}.
*
* @returns true if the identity has changed in a way that needs user approval.
*/
public readonly needsUserApproval: boolean;

public constructor(
private readonly crossSigningVerified: boolean,
private readonly crossSigningVerifiedBefore: boolean,
private readonly tofu: boolean,
) {}
needsUserApproval: boolean = false,
) {
this.needsUserApproval = needsUserApproval;
}

/**
* @returns true if this user is verified via any means
Expand All @@ -737,6 +765,8 @@ export class UserVerificationStatus {

/**
* @returns true if this user's key is trusted on first use
*
* @deprecated No longer supported, with the Rust crypto stack.
*/
public isTofu(): boolean {
return this.tofu;
Expand Down
7 changes: 7 additions & 0 deletions src/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,13 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
return this.checkUserTrust(userId);
}

/**
* Implementation of {@link Crypto.CryptoApi.pinCurrentUserIdentity}.
*/
public async pinCurrentUserIdentity(userId: string): Promise<void> {
throw new Error("not implemented");
}

/**
* Check whether a given device is trusted.
*
Expand Down
24 changes: 23 additions & 1 deletion src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,9 +654,31 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
if (userIdentity === undefined) {
return new UserVerificationStatus(false, false, false);
}

const verified = userIdentity.isVerified();
const wasVerified = userIdentity.wasPreviouslyVerified();
const needsUserApproval =
userIdentity instanceof RustSdkCryptoJs.UserIdentity ? userIdentity.identityNeedsUserApproval() : false;
userIdentity.free();
return new UserVerificationStatus(verified, false, false);
return new UserVerificationStatus(verified, wasVerified, false, needsUserApproval);
}

/**
* Implementation of {@link CryptoApi#pinCurrentUserIdentity}.
*/
public async pinCurrentUserIdentity(userId: string): Promise<void> {
const userIdentity: RustSdkCryptoJs.UserIdentity | RustSdkCryptoJs.OwnUserIdentity | undefined =
await this.getOlmMachineOrThrow().getIdentity(new RustSdkCryptoJs.UserId(userId));

if (userIdentity === undefined) {
throw new Error("Cannot pin identity of unknown user");
}

if (userIdentity instanceof RustSdkCryptoJs.OwnUserIdentity) {
throw new Error("Cannot pin identity of own user");
}

await userIdentity.pinCurrentMasterKey();
}

/**
Expand Down

0 comments on commit 1a8ea3d

Please sign in to comment.