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 CryptoApi.pinCurrentUserIdentity and UserIdentity.needsUserApproval #4415

Merged
merged 4 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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();
Copy link
Member

Choose a reason for hiding this comment

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

There is another PR doing that #4411 . Ok for me to close it in favor of this one

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
Loading