-
-
Notifications
You must be signed in to change notification settings - Fork 617
replace prepareKeyBackupVersion by resetKeyBackup #3662
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
Changes from all commits
46abd56
21b8f0f
f8d3a4f
49eaf4b
4582402
2c6037c
1d99a99
c95e90a
f6a8467
985b5e0
6c6c0df
511faa0
8f28611
319985c
80fce56
5a75462
50da327
646de15
05f80da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,16 +54,21 @@ import { | |
Room, | ||
RoomMember, | ||
RoomStateEvent, | ||
CryptoEvent, | ||
} from "../../../src/matrix"; | ||
import { DeviceInfo } from "../../../src/crypto/deviceinfo"; | ||
import { E2EKeyReceiver, IE2EKeyReceiver } from "../../test-utils/E2EKeyReceiver"; | ||
import { ISyncResponder, SyncResponder } from "../../test-utils/SyncResponder"; | ||
import { escapeRegExp } from "../../../src/utils"; | ||
import { downloadDeviceToJsDevice } from "../../../src/rust-crypto/device-converter"; | ||
import { flushPromises } from "../../test-utils/flushPromises"; | ||
import { mockInitialApiRequests, mockSetupCrossSigningRequests } from "../../test-utils/mockEndpoints"; | ||
import { | ||
mockInitialApiRequests, | ||
mockSetupCrossSigningRequests, | ||
mockSetupMegolmBackupRequests, | ||
} from "../../test-utils/mockEndpoints"; | ||
import { AddSecretStorageKeyOpts, SECRET_STORAGE_ALGORITHM_V1_AES } from "../../../src/secret-storage"; | ||
import { CryptoCallbacks } from "../../../src/crypto-api"; | ||
import { CryptoCallbacks, KeyBackupInfo } from "../../../src/crypto-api"; | ||
import { E2EKeyResponder } from "../../test-utils/E2EKeyResponder"; | ||
|
||
afterEach(() => { | ||
|
@@ -2228,6 +2233,65 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, | |
}); | ||
} | ||
|
||
function awaitMegolmBackupKeyUpload(): Promise<Record<string, {}>> { | ||
return new Promise((resolve) => { | ||
// Called when the megolm backup key is uploaded | ||
fetchMock.put( | ||
`express:/_matrix/client/v3/user/:userId/account_data/m.megolm_backup.v1`, | ||
(url: string, options: RequestInit) => { | ||
const content = JSON.parse(options.body as string); | ||
resolve(content.encrypted); | ||
return {}; | ||
}, | ||
{ overwriteRoutes: true }, | ||
); | ||
}); | ||
} | ||
|
||
async function bootstrapSecurity(backupVersion: string): Promise<void> { | ||
mockSetupCrossSigningRequests(); | ||
mockSetupMegolmBackupRequests(backupVersion); | ||
|
||
// promise which will resolve when a `KeyBackupStatus` event is emitted with `enabled: true` | ||
const backupStatusUpdate = new Promise<void>((resolve) => { | ||
aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { | ||
if (enabled) { | ||
resolve(); | ||
} | ||
}); | ||
}); | ||
|
||
const setupPromises = [ | ||
awaitCrossSigningKeyUpload("master"), | ||
awaitCrossSigningKeyUpload("user_signing"), | ||
awaitCrossSigningKeyUpload("self_signing"), | ||
awaitMegolmBackupKeyUpload(), | ||
]; | ||
|
||
// Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. | ||
await aliceClient.getCrypto()!.bootstrapCrossSigning({}); | ||
|
||
// Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. | ||
const bootstrapPromise = aliceClient.getCrypto()!.bootstrapSecretStorage({ | ||
setupNewSecretStorage: true, | ||
createSecretStorageKey, | ||
setupNewKeyBackup: true, | ||
}); | ||
|
||
// Wait for the key to be uploaded in the account data | ||
const secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); | ||
|
||
// Return the newly created key in the sync response | ||
sendSyncResponse(secretStorageKey); | ||
|
||
// Wait for the cross signing keys to be uploaded | ||
await Promise.all(setupPromises); | ||
|
||
// Finally, wait for bootstrapSecretStorage to finished | ||
await bootstrapPromise; | ||
await backupStatusUpdate; | ||
} | ||
|
||
/** | ||
* Send in the sync response the provided `secretStorageKey` into the account_data field | ||
* The key is set for the `m.secret_storage.default_key` and `m.secret_storage.key.${secretStorageKey}` events | ||
|
@@ -2275,7 +2339,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, | |
}, | ||
); | ||
|
||
newBackendOnly("should create a new key", async () => { | ||
it("should create a new key", async () => { | ||
const bootstrapPromise = aliceClient | ||
.getCrypto()! | ||
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); | ||
|
@@ -2318,46 +2382,43 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, | |
}, | ||
); | ||
|
||
newBackendOnly( | ||
"should create a new key if setupNewSecretStorage is at true even if an AES key is already in the secret storage", | ||
async () => { | ||
let bootstrapPromise = aliceClient | ||
.getCrypto()! | ||
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); | ||
it("should create a new key if setupNewSecretStorage is at true even if an AES key is already in the secret storage", async () => { | ||
let bootstrapPromise = aliceClient | ||
.getCrypto()! | ||
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); | ||
|
||
// Wait for the key to be uploaded in the account data | ||
let secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); | ||
// Wait for the key to be uploaded in the account data | ||
let secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); | ||
|
||
// Return the newly created key in the sync response | ||
sendSyncResponse(secretStorageKey); | ||
// Return the newly created key in the sync response | ||
sendSyncResponse(secretStorageKey); | ||
|
||
// Wait for bootstrapSecretStorage to finished | ||
await bootstrapPromise; | ||
// Wait for bootstrapSecretStorage to finished | ||
await bootstrapPromise; | ||
|
||
// Call again bootstrapSecretStorage | ||
bootstrapPromise = aliceClient | ||
.getCrypto()! | ||
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); | ||
// Call again bootstrapSecretStorage | ||
bootstrapPromise = aliceClient | ||
.getCrypto()! | ||
.bootstrapSecretStorage({ setupNewSecretStorage: true, createSecretStorageKey }); | ||
|
||
// Wait for the key to be uploaded in the account data | ||
secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); | ||
// Wait for the key to be uploaded in the account data | ||
secretStorageKey = await awaitSecretStorageKeyStoredInAccountData(); | ||
|
||
// Return the newly created key in the sync response | ||
sendSyncResponse(secretStorageKey); | ||
// Return the newly created key in the sync response | ||
sendSyncResponse(secretStorageKey); | ||
|
||
// Wait for bootstrapSecretStorage to finished | ||
await bootstrapPromise; | ||
// Wait for bootstrapSecretStorage to finished | ||
await bootstrapPromise; | ||
|
||
// createSecretStorageKey should have been called twice, one time every bootstrapSecretStorage call | ||
expect(createSecretStorageKey).toHaveBeenCalledTimes(2); | ||
}, | ||
); | ||
// createSecretStorageKey should have been called twice, one time every bootstrapSecretStorage call | ||
expect(createSecretStorageKey).toHaveBeenCalledTimes(2); | ||
}); | ||
|
||
newBackendOnly("should upload cross signing keys", async () => { | ||
it("should upload cross signing keys", async () => { | ||
mockSetupCrossSigningRequests(); | ||
|
||
// Before setting up secret-storage, bootstrap cross-signing, so that the client has cross-signing keys. | ||
await aliceClient.getCrypto()?.bootstrapCrossSigning({}); | ||
await aliceClient.getCrypto()!.bootstrapCrossSigning({}); | ||
|
||
// Now, when we bootstrap secret-storage, the cross-signing keys should be uploaded. | ||
const bootstrapPromise = aliceClient | ||
|
@@ -2385,6 +2446,89 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, | |
expect(userSigningKey[secretStorageKey]).toBeDefined(); | ||
expect(selfSigningKey[secretStorageKey]).toBeDefined(); | ||
}); | ||
|
||
newBackendOnly("should create a new megolm backup", async () => { | ||
const backupVersion = "abc"; | ||
await bootstrapSecurity(backupVersion); | ||
|
||
// Expect a backup to be available and used | ||
const activeBackup = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); | ||
expect(activeBackup).toStrictEqual(backupVersion); | ||
}); | ||
|
||
it("Reset key backup should create a new backup and update 4S", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like this isn't part of the tests for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. refactored. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't see any changes here. It's still inside the |
||
// First set up recovery | ||
const backupVersion = "1"; | ||
await bootstrapSecurity(backupVersion); | ||
|
||
const currentVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); | ||
const currentBackupKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey(); | ||
|
||
// we will call reset backup, it should delete the existing one, then setup a new one | ||
// Let's mock for that | ||
|
||
// Mock delete and replace the GET to return 404 as soon as called | ||
const awaitDeleteCalled = new Promise<void>((resolve) => { | ||
fetchMock.delete( | ||
"express:/_matrix/client/v3/room_keys/version/:version", | ||
(url: string, options: RequestInit) => { | ||
fetchMock.get( | ||
"path:/_matrix/client/v3/room_keys/version", | ||
{ | ||
status: 404, | ||
body: { errcode: "M_NOT_FOUND", error: "Account data not found." }, | ||
}, | ||
{ overwriteRoutes: true }, | ||
); | ||
resolve(); | ||
return {}; | ||
}, | ||
{ overwriteRoutes: true }, | ||
); | ||
}); | ||
|
||
const newVersion = "2"; | ||
fetchMock.post( | ||
"path:/_matrix/client/v3/room_keys/version", | ||
(url, request) => { | ||
const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}"); | ||
backupData.version = newVersion; | ||
backupData.count = 0; | ||
backupData.etag = "zer"; | ||
|
||
// update get call with new version | ||
fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, { | ||
overwriteRoutes: true, | ||
}); | ||
return { | ||
version: backupVersion, | ||
}; | ||
}, | ||
{ overwriteRoutes: true }, | ||
); | ||
|
||
const newBackupStatusUpdate = new Promise<void>((resolve) => { | ||
aliceClient.on(CryptoEvent.KeyBackupStatus, (enabled) => { | ||
if (enabled) { | ||
resolve(); | ||
} | ||
}); | ||
}); | ||
|
||
const new4SUpload = awaitMegolmBackupKeyUpload(); | ||
|
||
await aliceClient.getCrypto()!.resetKeyBackup(); | ||
await awaitDeleteCalled; | ||
await newBackupStatusUpdate; | ||
await new4SUpload; | ||
|
||
const nextVersion = await aliceClient.getCrypto()!.getActiveSessionBackupVersion(); | ||
const nextKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey(); | ||
|
||
expect(nextVersion).toBeDefined(); | ||
expect(nextVersion).not.toEqual(currentVersion); | ||
expect(nextKey).not.toEqual(currentBackupKey); | ||
}); | ||
}); | ||
|
||
describe("Incoming verification in a DM", () => { | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -16,6 +16,8 @@ limitations under the License. | |||||
|
||||||
import fetchMock from "fetch-mock-jest"; | ||||||
|
||||||
import { KeyBackupInfo } from "../../src/crypto-api"; | ||||||
|
||||||
/** | ||||||
* Mock out the endpoints that the js-sdk calls when we call `MatrixClient.start()`. | ||||||
* | ||||||
|
@@ -56,3 +58,35 @@ export function mockSetupCrossSigningRequests(): void { | |||||
{}, | ||||||
); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Mock out requests to `/room_keys/version`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* | ||||||
* Returns `404 M_NOT_FOUND` for GET requests until `POST room_keys/version` is called. | ||||||
* Once the POST is done, `GET /room_keys/version` will return the posted backup | ||||||
* instead of 404. | ||||||
* | ||||||
* @param backupVersion - The version of the backup to create | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this still needs an update, per #3662 (comment) |
||||||
*/ | ||||||
export function mockSetupMegolmBackupRequests(backupVersion: string): void { | ||||||
fetchMock.get("path:/_matrix/client/v3/room_keys/version", { | ||||||
status: 404, | ||||||
body: { | ||||||
errcode: "M_NOT_FOUND", | ||||||
error: "No current backup version", | ||||||
}, | ||||||
}); | ||||||
|
||||||
fetchMock.post("path:/_matrix/client/v3/room_keys/version", (url, request) => { | ||||||
const backupData: KeyBackupInfo = JSON.parse(request.body?.toString() ?? "{}"); | ||||||
backupData.version = backupVersion; | ||||||
backupData.count = 0; | ||||||
backupData.etag = "zer"; | ||||||
fetchMock.get("path:/_matrix/client/v3/room_keys/version", backupData, { | ||||||
overwriteRoutes: true, | ||||||
}); | ||||||
return { | ||||||
version: backupVersion, | ||||||
}; | ||||||
}); | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -233,6 +233,24 @@ export interface CryptoApi { | |||||||||||
*/ | ||||||||||||
bootstrapSecretStorage(opts: CreateSecretStorageOpts): Promise<void>; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Creates a new key backup version. | ||||||||||||
* | ||||||||||||
* If there are existing backups they will be replaced. | ||||||||||||
* | ||||||||||||
* The decryption key will be saved in Secret Storage (the `SecretStorageCallbacks.getSecretStorageKey` Crypto | ||||||||||||
* callback will be called) | ||||||||||||
* and the backup engine will be started. | ||||||||||||
Comment on lines
+241
to
+243
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
*/ | ||||||||||||
resetKeyBackup(): Promise<void>; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Deletes the given key backup. | ||||||||||||
* | ||||||||||||
* @param version - The backup version to delete. | ||||||||||||
*/ | ||||||||||||
deleteKeyBackupVersion(version: string): Promise<void>; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Get the status of our cross-signing keys. | ||||||||||||
* | ||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.