Merge pull request #1154 from matrix-org/jryans/complete-sec-confused
Convert secret storage to new account data API
This commit is contained in:
@@ -26,6 +26,12 @@ async function makeTestClient(userInfo, options) {
|
||||
userInfo.userId, userInfo.deviceId, undefined, undefined, options,
|
||||
)).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();
|
||||
|
||||
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"]);
|
||||
|
||||
expect(secretStorage.isStored("foo")).toBe(true);
|
||||
expect(await secretStorage.isStored("foo")).toBe(true);
|
||||
expect(await secretStorage.get("foo")).toBe("bar");
|
||||
|
||||
expect(getKey).toHaveBeenCalled();
|
||||
@@ -268,8 +274,8 @@ describe("Secrets", function() {
|
||||
const secretStorage = bob._crypto._secretStorage;
|
||||
|
||||
expect(crossSigning.getId()).toBeTruthy();
|
||||
expect(crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy();
|
||||
expect(secretStorage.hasKey()).toBeTruthy();
|
||||
expect(await crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy();
|
||||
expect(await secretStorage.hasKey()).toBeTruthy();
|
||||
});
|
||||
|
||||
it("bootstraps when cross-signing keys in secret storage", async function() {
|
||||
@@ -284,8 +290,8 @@ describe("Secrets", function() {
|
||||
},
|
||||
{
|
||||
cryptoCallbacks: {
|
||||
getSecretStorageKey: request => {
|
||||
const defaultKeyId = bob.getDefaultSecretStorageKeyId();
|
||||
getSecretStorageKey: async request => {
|
||||
const defaultKeyId = await bob.getDefaultSecretStorageKeyId();
|
||||
expect(Object.keys(request.keys)).toEqual([defaultKeyId]);
|
||||
return [defaultKeyId, storagePrivateKey];
|
||||
},
|
||||
@@ -324,7 +330,7 @@ describe("Secrets", function() {
|
||||
await bob.bootstrapSecretStorage();
|
||||
|
||||
expect(crossSigning.getId()).toBeTruthy();
|
||||
expect(crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy();
|
||||
expect(secretStorage.hasKey()).toBeTruthy();
|
||||
expect(await crossSigning.isStoredInSecretStorage(secretStorage)).toBeTruthy();
|
||||
expect(await secretStorage.hasKey()).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
+24
-3
@@ -498,6 +498,18 @@ MatrixClient.prototype.getSyncStateData = function() {
|
||||
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 {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
|
||||
* bypasses the local store and gets account data directly from the homeserver,
|
||||
* which can be useful very early in startup before the initial sync.
|
||||
* gets account data directly from the homeserver if the local store is not
|
||||
* ready, which can be useful very early in startup before the initial sync.
|
||||
* @param {string} eventType The event type
|
||||
* @return {module:client.Promise} Resolves: The contents of the given account
|
||||
* data event.
|
||||
* @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", {
|
||||
$userId: this.credentials.userId,
|
||||
$type: eventType,
|
||||
|
||||
@@ -113,10 +113,10 @@ export class CrossSigningInfo extends EventEmitter {
|
||||
* @param {SecretStorage} secretStorage The secret store using account data
|
||||
* @returns {boolean} Whether all private keys were found in storage
|
||||
*/
|
||||
isStoredInSecretStorage(secretStorage) {
|
||||
async isStoredInSecretStorage(secretStorage) {
|
||||
let stored = true;
|
||||
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;
|
||||
}
|
||||
|
||||
+39
-43
@@ -36,12 +36,12 @@ export class SecretStorage extends EventEmitter {
|
||||
this._incomingRequests = {};
|
||||
}
|
||||
|
||||
getDefaultKeyId() {
|
||||
const defaultKeyEvent = this._baseApis.getAccountData(
|
||||
async getDefaultKeyId() {
|
||||
const defaultKey = await this._baseApis.getAccountDataFromServer(
|
||||
'm.secret_storage.default_key',
|
||||
);
|
||||
if (!defaultKeyEvent) return null;
|
||||
return defaultKeyEvent.getContent().key;
|
||||
if (!defaultKey) return null;
|
||||
return defaultKey.key;
|
||||
}
|
||||
|
||||
setDefaultKeyId(keyId) {
|
||||
@@ -112,7 +112,11 @@ export class SecretStorage extends EventEmitter {
|
||||
if (!keyId) {
|
||||
do {
|
||||
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');
|
||||
@@ -130,18 +134,20 @@ export class SecretStorage extends EventEmitter {
|
||||
* @param {string} [keyId = default key's ID] The ID of the key to sign.
|
||||
* 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) {
|
||||
throw new Error("signKey requires a key ID");
|
||||
}
|
||||
|
||||
const keyInfoEvent = this._baseApis.getAccountData(
|
||||
const keyInfo = await this._baseApis.getAccountDataFromServer(
|
||||
`m.secret_storage.key.${keyId}`,
|
||||
);
|
||||
if (!keyInfoEvent) {
|
||||
if (!keyInfo) {
|
||||
throw new Error(`Key ${keyId} does not exist in account data`);
|
||||
}
|
||||
const keyInfo = keyInfoEvent.getContent();
|
||||
|
||||
await this._crossSigningInfo.signObject(keyInfo, 'master');
|
||||
await this._baseApis.setAccountData(
|
||||
@@ -156,15 +162,17 @@ export class SecretStorage extends EventEmitter {
|
||||
* for. Defaults to the default key ID if not provided.
|
||||
* @return {boolean} Whether we have the key.
|
||||
*/
|
||||
hasKey(keyId = this.getDefaultKeyId()) {
|
||||
async hasKey(keyId) {
|
||||
if (!keyId) {
|
||||
keyId = await this.getDefaultKeyId();
|
||||
}
|
||||
if (!keyId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const keyInfo = this._baseApis.getAccountData(
|
||||
return !!this._baseApis.getAccountDataFromServer(
|
||||
"m.secret_storage.key." + keyId,
|
||||
);
|
||||
return keyInfo && keyInfo.getContent();
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -179,7 +187,7 @@ export class SecretStorage extends EventEmitter {
|
||||
const encrypted = {};
|
||||
|
||||
if (!keys) {
|
||||
const defaultKeyId = this.getDefaultKeyId();
|
||||
const defaultKeyId = await this.getDefaultKeyId();
|
||||
if (!defaultKeyId) {
|
||||
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) {
|
||||
// get key information from key storage
|
||||
const keyInfo = this._baseApis.getAccountData(
|
||||
const keyInfo = await this._baseApis.getAccountDataFromServer(
|
||||
"m.secret_storage.key." + keyId,
|
||||
);
|
||||
if (!keyInfo) {
|
||||
throw new Error("Unknown key: " + keyId);
|
||||
}
|
||||
const keyInfoContent = keyInfo.getContent();
|
||||
|
||||
// check signature of key info
|
||||
pkVerify(
|
||||
keyInfoContent,
|
||||
keyInfo,
|
||||
this._crossSigningInfo.getId('master'),
|
||||
this._crossSigningInfo.userId,
|
||||
);
|
||||
|
||||
// encrypt secret, based on the algorithm
|
||||
switch (keyInfoContent.algorithm) {
|
||||
switch (keyInfo.algorithm) {
|
||||
case SECRET_STORAGE_ALGORITHM_V1:
|
||||
{
|
||||
const encryption = new global.Olm.PkEncryption();
|
||||
try {
|
||||
encryption.set_recipient_key(keyInfoContent.pubkey);
|
||||
encryption.set_recipient_key(keyInfo.pubkey);
|
||||
encrypted[keyId] = encryption.encrypt(secret);
|
||||
} finally {
|
||||
encryption.free();
|
||||
@@ -222,7 +229,7 @@ export class SecretStorage extends EventEmitter {
|
||||
}
|
||||
default:
|
||||
logger.warn("unknown algorithm for secret storage key " + keyId
|
||||
+ ": " + keyInfoContent.algorithm);
|
||||
+ ": " + keyInfo.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
|
||||
*/
|
||||
async get(name) {
|
||||
const secretInfo = this._baseApis.getAccountData(name);
|
||||
const secretInfo = await this._baseApis.getAccountDataFromServer(name);
|
||||
if (!secretInfo) {
|
||||
return;
|
||||
}
|
||||
|
||||
const secretContent = secretInfo.getContent();
|
||||
|
||||
if (!secretContent.encrypted) {
|
||||
if (!secretInfo.encrypted) {
|
||||
throw new Error("Content is not encrypted!");
|
||||
}
|
||||
|
||||
// get possible keys to decrypt
|
||||
const keys = {};
|
||||
for (const keyId of Object.keys(secretContent.encrypted)) {
|
||||
for (const keyId of Object.keys(secretInfo.encrypted)) {
|
||||
// get key information from key storage
|
||||
const keyInfo = this._baseApis.getAccountData(
|
||||
const keyInfo = await this._baseApis.getAccountDataFromServer(
|
||||
"m.secret_storage.key." + keyId,
|
||||
).getContent();
|
||||
const encInfo = secretContent.encrypted[keyId];
|
||||
);
|
||||
const encInfo = secretInfo.encrypted[keyId];
|
||||
switch (keyInfo.algorithm) {
|
||||
case SECRET_STORAGE_ALGORITHM_V1:
|
||||
if (keyInfo.pubkey && encInfo.ciphertext && encInfo.mac
|
||||
@@ -297,7 +301,7 @@ export class SecretStorage extends EventEmitter {
|
||||
// fetch private key from app
|
||||
[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
|
||||
// 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
|
||||
*/
|
||||
isStored(name, checkKey) {
|
||||
async isStored(name, checkKey) {
|
||||
// check if secret exists
|
||||
const secretInfo = this._baseApis.getAccountData(name);
|
||||
if (!secretInfo) {
|
||||
const secretInfo = await this._baseApis.getAccountDataFromServer(name);
|
||||
if (!secretInfo || !secretInfo.encrypted) {
|
||||
return false;
|
||||
}
|
||||
|
||||
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
|
||||
// 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
|
||||
const keyEvent = this._baseApis.getAccountData(
|
||||
const keyInfo = await this._baseApis.getAccountDataFromServer(
|
||||
"m.secret_storage.key." + keyId,
|
||||
);
|
||||
if (!keyEvent) return false;
|
||||
const keyInfo = keyEvent.getContent();
|
||||
if (!keyInfo) return false;
|
||||
const encInfo = secretContent.encrypted[keyId];
|
||||
const encInfo = secretInfo.encrypted[keyId];
|
||||
if (checkKey) {
|
||||
pkVerify(
|
||||
keyInfo,
|
||||
|
||||
+5
-6
@@ -359,15 +359,14 @@ Crypto.prototype.bootstrapSecretStorage = async function({
|
||||
const appCallbacks = Object.assign({}, this._baseApis._cryptoCallbacks);
|
||||
|
||||
try {
|
||||
if (
|
||||
!this._crossSigningInfo.getId() ||
|
||||
!this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage)
|
||||
) {
|
||||
const inStorage =
|
||||
await this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage);
|
||||
if (!this._crossSigningInfo.getId() || !inStorage) {
|
||||
logger.log(
|
||||
"Cross-signing public and/or private keys not found, " +
|
||||
"checking secret storage for private keys",
|
||||
);
|
||||
if (this._crossSigningInfo.isStoredInSecretStorage(this._secretStorage)) {
|
||||
if (inStorage) {
|
||||
logger.log("Cross-signing private keys found in secret storage");
|
||||
await this.checkOwnCrossSigningTrust();
|
||||
} 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
|
||||
// the default key (which will also be signed by the cross-signing master key).
|
||||
if (!this.hasSecretStorageKey()) {
|
||||
if (!await this.hasSecretStorageKey()) {
|
||||
let newKeyId;
|
||||
if (keyBackupInfo) {
|
||||
logger.log("Secret storage default key not found, using key backup key");
|
||||
|
||||
Reference in New Issue
Block a user