Skip to content

Commit

Permalink
Rust-crypto: fix bootstrapCrossSigning on second call
Browse files Browse the repository at this point in the history 
Currently, `bootstrapCrossSigning` raises an exception if it is called a second
time before secret storage is set up. It is easily fixed by checking that 4S is
set up before trying to export to 4S.

Also a few logging fixes while we're in the area.
  • Loading branch information
@richvdh
richvdh committed Nov 28, 2023
1 parent 8ef2ca6 commit 4e5915e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 14 deletions.
13 changes: 13 additions & 0 deletions spec/integ/crypto/cross-signing.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,19 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("cross-signing (%s)", (backend: s
`[${TEST_USER_ID}].[${TEST_DEVICE_ID}].signatures.[${TEST_USER_ID}].[ed25519:${SELF_CROSS_SIGNING_PUBLIC_KEY_BASE64}]`,
);
});

it("can bootstrapCrossSigning twice", async () => {
mockSetupCrossSigningRequests();

const authDict = { type: "test" };
await bootstrapCrossSigning(authDict);

// a second call should do nothing except GET requests
fetchMock.mockClear();
await bootstrapCrossSigning(authDict);
const calls = fetchMock.calls((url, opts) => opts.method != "GET");
expect(calls.length).toEqual(0);
});
});

describe("getCrossSigningStatus()", () => {
Expand Down
42 changes: 28 additions & 14 deletions src/rust-crypto/CrossSigningIdentity.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class CrossSigningIdentity {
olmDeviceStatus.hasMaster && olmDeviceStatus.hasUserSigning && olmDeviceStatus.hasSelfSigning;

// Log all relevant state for easier parsing of debug logs.
logger.log("bootStrapCrossSigning: starting", {
logger.log("bootstrapCrossSigning: starting", {
setupNewCrossSigning: opts.setupNewCrossSigning,
olmDeviceHasMaster: olmDeviceStatus.hasMaster,
olmDeviceHasUserSigning: olmDeviceStatus.hasUserSigning,
Expand All @@ -66,18 +66,25 @@ export class CrossSigningIdentity {
});

if (olmDeviceHasKeys) {
if (!privateKeysInSecretStorage) {
if (!(await this.secretStorage.hasKey())) {
logger.warn(
"bootstrapCrossSigning: Olm device has private keys, but secret storage is not yet set up; doing nothing for now.",
);
// the keys should get uploaded to 4S once that is set up.
} else if (!privateKeysInSecretStorage) {
// the device has the keys but they are not in 4S, so update it
logger.log("bootStrapCrossSigning: Olm device has private keys: exporting to secret storage");
logger.log("bootstrapCrossSigning: Olm device has private keys: exporting to secret storage");
await this.exportCrossSigningKeysToStorage();
} else {
logger.log("bootStrapCrossSigning: Olm device has private keys and they are saved in 4S, do nothing");
logger.log(
"bootstrapCrossSigning: Olm device has private keys and they are saved in secret storage; doing nothing",
);
}
} /* (!olmDeviceHasKeys) */ else {
if (privateKeysInSecretStorage) {
// they are in 4S, so import from there
logger.log(
"bootStrapCrossSigning: Cross-signing private keys not found locally, but they are available " +
"bootstrapCrossSigning: Cross-signing private keys not found locally, but they are available " +
"in secret storage, reading storage and caching locally",
);
await this.olmMachine.importCrossSigningKeys(
Expand All @@ -97,15 +104,15 @@ export class CrossSigningIdentity {
await this.outgoingRequestProcessor.makeOutgoingRequest(request);
} else {
logger.log(
"bootStrapCrossSigning: Cross-signing private keys not found locally or in secret storage, creating new keys",
"bootstrapCrossSigning: Cross-signing private keys not found locally or in secret storage, creating new keys",
);
await this.resetCrossSigning(opts.authUploadDeviceSigningKeys);
}
}

// TODO: we might previously have bootstrapped cross-signing but not completed uploading the keys to the
// server -- in which case we should call OlmDevice.bootstrap_cross_signing. How do we know?
logger.log("bootStrapCrossSigning: complete");
logger.log("bootstrapCrossSigning: complete");
}

/** Reset our cross-signing keys
Expand All @@ -120,14 +127,21 @@ export class CrossSigningIdentity {
// or 4S passphrase/key the process will fail in a bad state, with keys rotated but not uploaded or saved in 4S.
const outgoingRequests: CrossSigningBootstrapRequests = await this.olmMachine.bootstrapCrossSigning(true);

// If 4S is configured we need to udpate it.
if (await this.secretStorage.hasKey()) {
// If 4S is configured we need to update it.
if (!(await this.secretStorage.hasKey())) {
logger.warn(
"resetCrossSigning: Secret storage is not yet set up; not exporting keys to secret storage yet.",
);
// the keys should get uploaded to 4S once that is set up.
} else {
// Update 4S before uploading cross-signing keys, to stay consistent with legacy that asks
// 4S passphrase before asking for account password.
// Ultimately should be made atomic and resistent to forgotten password/passphrase.
// Ultimately should be made atomic and resistant to forgotten password/passphrase.
logger.log("resetCrossSigning: exporting to secret storage");

await this.exportCrossSigningKeysToStorage();
}
logger.log("bootStrapCrossSigning: publishing keys to server");
logger.log("resetCrossSigning: publishing keys to server");
for (const req of [
outgoingRequests.uploadKeysRequest,
outgoingRequests.uploadSigningKeysRequest,
Expand All @@ -148,17 +162,17 @@ export class CrossSigningIdentity {
const exported: RustSdkCryptoJs.CrossSigningKeyExport | null = await this.olmMachine.exportCrossSigningKeys();
/* istanbul ignore else (this function is only called when we know the olm machine has keys) */
if (exported?.masterKey) {
this.secretStorage.store("m.cross_signing.master", exported.masterKey);
await this.secretStorage.store("m.cross_signing.master", exported.masterKey);
} else {
logger.error(`Cannot export MSK to secret storage, private key unknown`);
}
if (exported?.self_signing_key) {
this.secretStorage.store("m.cross_signing.self_signing", exported.self_signing_key);
await this.secretStorage.store("m.cross_signing.self_signing", exported.self_signing_key);
} else {
logger.error(`Cannot export SSK to secret storage, private key unknown`);
}
if (exported?.userSigningKey) {
this.secretStorage.store("m.cross_signing.user_signing", exported.userSigningKey);
await this.secretStorage.store("m.cross_signing.user_signing", exported.userSigningKey);
} else {
logger.error(`Cannot export USK to secret storage, private key unknown`);
}
Expand Down
3 changes: 3 additions & 0 deletions src/rust-crypto/rust-crypto.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
}

// Create a new storage key and add it to secret storage
this.logger.info("bootstrapSecretStorage: creating new secret storage key");
const recoveryKey = await createSecretStorageKey();
await this.addSecretStorageKeyToSecretStorage(recoveryKey);
}
Expand All @@ -718,6 +719,8 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
hasPrivateKeys &&
(isNewSecretStorageKeyNeeded || !(await secretStorageContainsCrossSigningKeys(this.secretStorage)))
) {
this.logger.info("bootstrapSecretStorage: cross-signing keys not yet exported; doing so now.");

const crossSigningPrivateKeys: RustSdkCryptoJs.CrossSigningKeyExport =
await this.olmMachine.exportCrossSigningKeys();

Expand Down

0 comments on commit 4e5915e

Please sign in to comment.