matrix/matrix-js-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-08-07 23:02:56 +03:00
Code Activity

Convert secret storage to new account data API

Browse Source 
This converts all secret storage to use a newer account data API which uses
cached data in stored when available, but also knows how to ask the homeserver
in case it's invoked during early client startup before the initial sync.

As a consequence, it means most secret storage APIs are now async.

Part of https://github.com/vector-im/riot-web/issues/11901
This commit is contained in:
J. Ryan Stinnett
2020-01-17 17:56:01 +00:00
parent 8217c0f05f
commit ceb4581f91
5 changed files with 84 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
spec/unit/crypto/secrets.spec.js
View File

@@ -26,6 +26,12 @@ async function makeTestClient(userInfo, options) {
userInfo.userId, userInfo.deviceId, undefined, undefined, options, userInfo.userId, userInfo.deviceId, undefined, undefined, options,
)).client; )).client;
// Make it seem as if we've synced and thus the store can be trusted to
// contain valid account data.
client.isInitialSyncComplete = function() {
return true;
};
await client.initCrypto(); await client.initCrypto();
return client; return client;
@@ -103,11 +109,11 @@ describe("Secrets", function() {
}), }),
]); ]);
expect(secretStorage.isStored("foo")).toBe(false); expect(await secretStorage.isStored("foo")).toBe(false);
await secretStorage.store("foo", "bar", ["abc"]); await secretStorage.store("foo", "bar", ["abc"]);
expect(secretStorage.isStored("foo")).toBe(true); expect(await secretStorage.isStored("foo")).toBe(true);
expect(await secretStorage.get("foo")).toBe("bar"); expect(await secretStorage.get("foo")).toBe("bar");
expect(getKey).toHaveBeenCalled(); expect(getKey).toHaveBeenCalled();
@@ -268,8 +274,8 @@ describe("Secrets", function() {
const secretStorage = bob._crypto._secretStorage; const secretStorage = bob._crypto._secretStorage;
expect(crossSigning.getId()).toBeTruthy(); expect(crossSigning.getId()).toBeTruthy();
expect(crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy(); expect(await crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy();
expect(secretStorage.hasKey()).toBeTruthy(); expect(await secretStorage.hasKey()).toBeTruthy();
}); });
it("bootstraps when cross-signing keys in secret storage", async function() { it("bootstraps when cross-signing keys in secret storage", async function() {
@@ -284,8 +290,8 @@ describe("Secrets", function() {
}, },
{ {
cryptoCallbacks: { cryptoCallbacks: {
getSecretStorageKey: request => { getSecretStorageKey: async request => {
const defaultKeyId = bob.getDefaultSecretStorageKeyId(); const defaultKeyId = await bob.getDefaultSecretStorageKeyId();
expect(Object.keys(request.keys)).toEqual([defaultKeyId]); expect(Object.keys(request.keys)).toEqual([defaultKeyId]);
return [defaultKeyId, storagePrivateKey]; return [defaultKeyId, storagePrivateKey];
}, },
@@ -324,7 +330,7 @@ describe("Secrets", function() {
await bob.bootstrapSecretStorage(); await bob.bootstrapSecretStorage();
expect(crossSigning.getId()).toBeTruthy(); expect(crossSigning.getId()).toBeTruthy();
expect(crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy(); expect(await crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy();
expect(secretStorage.hasKey()).toBeTruthy(); expect(await secretStorage.hasKey()).toBeTruthy();
}); });
}); });

27
src/client.js
View File

@@ -498,6 +498,18 @@ MatrixClient.prototype.getSyncStateData = function() {
return this._syncApi.getSyncStateData(); return this._syncApi.getSyncStateData();
}; };
/**
* Whether the initial sync has completed.
* @return {boolean} True if at least on sync has happened.
*/
MatrixClient.prototype.isInitialSyncComplete = function() {
const state = this.getSyncState();
if (!state) {
return false;
}
return state === "PREPAED" || state === "SYNCING";
};
/** /**
* Return whether the client is configured for a guest account. * Return whether the client is configured for a guest account.
* @return {boolean} True if this is a guest access_token (or no token is supplied). * @return {boolean} True if this is a guest access_token (or no token is supplied).
@@ -1944,14 +1956,23 @@ MatrixClient.prototype.getAccountData = function(eventType) {
/** /**
* Get account data event of given type for the current user. This variant * Get account data event of given type for the current user. This variant
* bypasses the local store and gets account data directly from the homeserver, * gets account data directly from the homeserver if the local store is not
* which can be useful very early in startup before the initial sync. * ready, which can be useful very early in startup before the initial sync.
* @param {string} eventType The event type * @param {string} eventType The event type
* @return {module:client.Promise} Resolves: The contents of the given account * @return {module:client.Promise} Resolves: The contents of the given account
* data event. * data event.
* @return {module:http-api.MatrixError} Rejects: with an error response. * @return {module:http-api.MatrixError} Rejects: with an error response.
*/ */
MatrixClient.prototype.getAccountDataFromServer = function(eventType) { MatrixClient.prototype.getAccountDataFromServer = async function(eventType) {
if (this.isInitialSyncComplete()) {
const event = this.store.getAccountData(eventType);
if (!event) {
return null;
}
// The network version below returns just the content, so this branch
// does the same to match.
return event.getContent();
}
const path = utils.encodeUri("/user/$userId/account_data/$type", { const path = utils.encodeUri("/user/$userId/account_data/$type", {
$userId: this.credentials.userId, $userId: this.credentials.userId,
$type: eventType, $type: eventType,

4
src/crypto/CrossSigning.js
View File

@@ -113,10 +113,10 @@ export class CrossSigningInfo extends EventEmitter {
* @param {SecretStorage} secretStorage The secret store using account data * @param {SecretStorage} secretStorage The secret store using account data
* @returns {boolean} Whether all private keys were found in storage * @returns {boolean} Whether all private keys were found in storage
*/ */
isStoredInSecretStorage(secretStorage) { async isStoredInSecretStorage(secretStorage) {
let stored = true; let stored = true;
for (const type of ["master", "self_signing", "user_signing"]) { for (const type of ["master", "self_signing", "user_signing"]) {
stored &= secretStorage.isStored(`m.cross_signing.${type}`, false); stored &= await secretStorage.isStored(`m.cross_signing.${type}`, false);
} }
return stored; return stored;
} }

82
src/crypto/SecretStorage.js
View File

@@ -36,12 +36,12 @@ export class SecretStorage extends EventEmitter {
this._incomingRequests = {}; this._incomingRequests = {};
} }
getDefaultKeyId() { async getDefaultKeyId() {
const defaultKeyEvent = this._baseApis.getAccountData( const defaultKey = await this._baseApis.getAccountDataFromServer(
'm.secret_storage.default_key', 'm.secret_storage.default_key',
); );
if (!defaultKeyEvent) return null; if (!defaultKey) return null;
return defaultKeyEvent.getContent().key; return defaultKey.key;
} }
setDefaultKeyId(keyId) { setDefaultKeyId(keyId) {
@@ -112,7 +112,11 @@ export class SecretStorage extends EventEmitter {
if (!keyId) { if (!keyId) {
do { do {
keyId = randomString(32); keyId = randomString(32);
} while (this._baseApis.getAccountData(`m.secret_storage.key.${keyId}`)); } while (
await this._baseApis.getAccountDataFromServer(
`m.secret_storage.key.${keyId}`,
)
);
} }
await this._crossSigningInfo.signObject(keyData, 'master'); await this._crossSigningInfo.signObject(keyData, 'master');
@@ -130,18 +134,20 @@ export class SecretStorage extends EventEmitter {
* @param {string} [keyId = default key's ID] The ID of the key to sign. * @param {string} [keyId = default key's ID] The ID of the key to sign.
* Defaults to the default key ID if not provided. * Defaults to the default key ID if not provided.
*/ */
async signKey(keyId = this.getDefaultKeyId()) { async signKey(keyId) {
if (!keyId) {
keyId = await this.getDefaultKeyId();
}
if (!keyId) { if (!keyId) {
throw new Error("signKey requires a key ID"); throw new Error("signKey requires a key ID");
} }
const keyInfoEvent = this._baseApis.getAccountData( const keyInfo = await this._baseApis.getAccountDataFromServer(
`m.secret_storage.key.${keyId}`, `m.secret_storage.key.${keyId}`,
); );
if (!keyInfoEvent) { if (!keyInfo) {
throw new Error(`Key ${keyId} does not exist in account data`); throw new Error(`Key ${keyId} does not exist in account data`);
} }
const keyInfo = keyInfoEvent.getContent();
await this._crossSigningInfo.signObject(keyInfo, 'master'); await this._crossSigningInfo.signObject(keyInfo, 'master');
await this._baseApis.setAccountData( await this._baseApis.setAccountData(
@@ -156,15 +162,17 @@ export class SecretStorage extends EventEmitter {
* for. Defaults to the default key ID if not provided. * for. Defaults to the default key ID if not provided.
* @return {boolean} Whether we have the key. * @return {boolean} Whether we have the key.
*/ */
hasKey(keyId = this.getDefaultKeyId()) { async hasKey(keyId) {
if (!keyId) {
keyId = await this.getDefaultKeyId();
}
if (!keyId) { if (!keyId) {
return false; return false;
} }
const keyInfo = this._baseApis.getAccountData( return !!this._baseApis.getAccountDataFromServer(
"m.secret_storage.key." + keyId, "m.secret_storage.key." + keyId,
); );
return keyInfo && keyInfo.getContent();
} }
/** /**
@@ -179,7 +187,7 @@ export class SecretStorage extends EventEmitter {
const encrypted = {}; const encrypted = {};
if (!keys) { if (!keys) {
const defaultKeyId = this.getDefaultKeyId(); const defaultKeyId = await this.getDefaultKeyId();
if (!defaultKeyId) { if (!defaultKeyId) {
throw new Error("No keys specified and no default key present"); throw new Error("No keys specified and no default key present");
} }
@@ -192,28 +200,27 @@ export class SecretStorage extends EventEmitter {
for (const keyId of keys) { for (const keyId of keys) {
// get key information from key storage // get key information from key storage
const keyInfo = this._baseApis.getAccountData( const keyInfo = await this._baseApis.getAccountDataFromServer(
"m.secret_storage.key." + keyId, "m.secret_storage.key." + keyId,
); );
if (!keyInfo) { if (!keyInfo) {
throw new Error("Unknown key: " + keyId); throw new Error("Unknown key: " + keyId);
} }
const keyInfoContent = keyInfo.getContent();
// check signature of key info // check signature of key info
pkVerify( pkVerify(
keyInfoContent, keyInfo,
this._crossSigningInfo.getId('master'), this._crossSigningInfo.getId('master'),
this._crossSigningInfo.userId, this._crossSigningInfo.userId,
); );
// encrypt secret, based on the algorithm // encrypt secret, based on the algorithm
switch (keyInfoContent.algorithm) { switch (keyInfo.algorithm) {
case SECRET_STORAGE_ALGORITHM_V1: case SECRET_STORAGE_ALGORITHM_V1:
{ {
const encryption = new global.Olm.PkEncryption(); const encryption = new global.Olm.PkEncryption();
try { try {
encryption.set_recipient_key(keyInfoContent.pubkey); encryption.set_recipient_key(keyInfo.pubkey);
encrypted[keyId] = encryption.encrypt(secret); encrypted[keyId] = encryption.encrypt(secret);
} finally { } finally {
encryption.free(); encryption.free();
@@ -222,7 +229,7 @@ export class SecretStorage extends EventEmitter {
} }
default: default:
logger.warn("unknown algorithm for secret storage key " + keyId logger.warn("unknown algorithm for secret storage key " + keyId
+ ": " + keyInfoContent.algorithm); + ": " + keyInfo.algorithm);
// do nothing if we don't understand the encryption algorithm // do nothing if we don't understand the encryption algorithm
} }
} }
@@ -260,25 +267,22 @@ export class SecretStorage extends EventEmitter {
* @return {string} the contents of the secret * @return {string} the contents of the secret
*/ */
async get(name) { async get(name) {
const secretInfo = this._baseApis.getAccountData(name); const secretInfo = await this._baseApis.getAccountDataFromServer(name);
if (!secretInfo) { if (!secretInfo) {
return; return;
} }
if (!secretInfo.encrypted) {
const secretContent = secretInfo.getContent();
if (!secretContent.encrypted) {
throw new Error("Content is not encrypted!"); throw new Error("Content is not encrypted!");
} }
// get possible keys to decrypt // get possible keys to decrypt
const keys = {}; const keys = {};
for (const keyId of Object.keys(secretContent.encrypted)) { for (const keyId of Object.keys(secretInfo.encrypted)) {
// get key information from key storage // get key information from key storage
const keyInfo = this._baseApis.getAccountData( const keyInfo = await this._baseApis.getAccountDataFromServer(
"m.secret_storage.key." + keyId, "m.secret_storage.key." + keyId,
).getContent(); );
const encInfo = secretContent.encrypted[keyId]; const encInfo = secretInfo.encrypted[keyId];
switch (keyInfo.algorithm) { switch (keyInfo.algorithm) {
case SECRET_STORAGE_ALGORITHM_V1: case SECRET_STORAGE_ALGORITHM_V1:
if (keyInfo.pubkey && encInfo.ciphertext && encInfo.mac if (keyInfo.pubkey && encInfo.ciphertext && encInfo.mac
@@ -297,7 +301,7 @@ export class SecretStorage extends EventEmitter {
// fetch private key from app // fetch private key from app
[keyId, decryption] = await this._getSecretStorageKey(keys); [keyId, decryption] = await this._getSecretStorageKey(keys);
const encInfo = secretContent.encrypted[keyId]; const encInfo = secretInfo.encrypted[keyId];
// We don't actually need the decryption object if it's a passthrough // We don't actually need the decryption object if it's a passthrough
// since we just want to return the key itself. // since we just want to return the key itself.
@@ -323,32 +327,24 @@ export class SecretStorage extends EventEmitter {
* *
* @return {boolean} whether or not the secret is stored * @return {boolean} whether or not the secret is stored
*/ */
isStored(name, checkKey) { async isStored(name, checkKey) {
// check if secret exists // check if secret exists
const secretInfo = this._baseApis.getAccountData(name); const secretInfo = await this._baseApis.getAccountDataFromServer(name);
if (!secretInfo) { if (!secretInfo || !secretInfo.encrypted) {
return false; return false;
} }
if (checkKey === undefined) checkKey = true; if (checkKey === undefined) checkKey = true;
const secretContent = secretInfo.getContent();
if (!secretContent.encrypted) {
return false;
}
// check if secret is encrypted by a known/trusted secret and // check if secret is encrypted by a known/trusted secret and
// encryption looks sane // encryption looks sane
for (const keyId of Object.keys(secretContent.encrypted)) { for (const keyId of Object.keys(secretInfo.encrypted)) {
// get key information from key storage // get key information from key storage
const keyEvent = this._baseApis.getAccountData( const keyInfo = await this._baseApis.getAccountDataFromServer(
"m.secret_storage.key." + keyId, "m.secret_storage.key." + keyId,
); );
if (!keyEvent) return false;
const keyInfo = keyEvent.getContent();
if (!keyInfo) return false; if (!keyInfo) return false;
const encInfo = secretContent.encrypted[keyId]; const encInfo = secretInfo.encrypted[keyId];
if (checkKey) { if (checkKey) {
pkVerify( pkVerify(
keyInfo, keyInfo,

11
src/crypto/index.js
View File

@@ -359,15 +359,14 @@ Crypto.prototype.bootstrapSecretStorage = async function({
const appCallbacks = Object.assign({}, this._baseApis._cryptoCallbacks); const appCallbacks = Object.assign({}, this._baseApis._cryptoCallbacks);
try { try {
if ( const inStorage =
!this._crossSigningInfo.getId() || await this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage);
!this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage) if (!this._crossSigningInfo.getId() || !inStorage) {
) {
logger.log( logger.log(
"Cross-signing public and/or private keys not found, " + "Cross-signing public and/or private keys not found, " +
"checking secret storage for private keys", "checking secret storage for private keys",
); );
if (this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage)) { if (inStorage) {
logger.log("Cross-signing private keys found in secret storage"); logger.log("Cross-signing private keys found in secret storage");
await this.checkOwnCrossSigningTrust(); await this.checkOwnCrossSigningTrust();
} else { } else {
@@ -390,7 +389,7 @@ Crypto.prototype.bootstrapSecretStorage = async function({
// Check if Secure Secret Storage has a default key. If we don't have one, create // Check if Secure Secret Storage has a default key. If we don't have one, create
// the default key (which will also be signed by the cross-signing master key). // the default key (which will also be signed by the cross-signing master key).
if (!this.hasSecretStorageKey()) { if (!await this.hasSecretStorageKey()) {
let newKeyId; let newKeyId;
if (keyBackupInfo) { if (keyBackupInfo) {
logger.log("Secret storage default key not found, using key backup key"); logger.log("Secret storage default key not found, using key backup key");