Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Lifecycle: bail out if picklekey is missing
Browse files Browse the repository at this point in the history
Currently, if we have an accesstoken which is encrypted with a picklekey, but
the picklekey has gone missing, we carry on with no access token at all. This
is sure to blow up in some way or other later on, but in a rather cryptic way.

Instead, let's bail out early.

(This will produce a "can't restore session" error, but we normally see one of
those anyway because we can't initialise the crypto store.)
  • Loading branch information
richvdh committed Sep 11, 2024
1 parent b13655b commit ab50038
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 19 deletions.
7 changes: 4 additions & 3 deletions src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,10 +592,11 @@ export async function restoreSessionFromStorage(opts?: { ignoreGuest?: boolean }
if (pickleKey) {
logger.log(`Got pickle key for ${userId}|${deviceId}`);
} else {
logger.log("No pickle key available");
logger.log(`No pickle key available for ${userId}|${deviceId}`);
}
const decryptedAccessToken = await tryDecryptToken(pickleKey, accessToken, ACCESS_TOKEN_IV);
const decryptedRefreshToken = await tryDecryptToken(pickleKey, refreshToken, REFRESH_TOKEN_IV);
const decryptedRefreshToken =
refreshToken && (await tryDecryptToken(pickleKey, refreshToken, REFRESH_TOKEN_IV));

const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true";
sessionStorage.removeItem("mx_fresh_login");
Expand All @@ -605,7 +606,7 @@ export async function restoreSessionFromStorage(opts?: { ignoreGuest?: boolean }
{
userId: userId,
deviceId: deviceId,
accessToken: decryptedAccessToken!,
accessToken: decryptedAccessToken,
refreshToken: decryptedRefreshToken,
homeserverUrl: hsUrl,
identityServerUrl: isUrl,
Expand Down
29 changes: 14 additions & 15 deletions src/utils/tokens/tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,6 @@ async function pickleKeyToAesKey(pickleKey: string): Promise<Uint8Array> {
);
}

const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): token is IEncryptedPayload => {
return !!token && typeof token !== "string";
};

/**
* Try to decrypt a token retrieved from storage
*
Expand All @@ -86,24 +82,27 @@ const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): tok
* @param tokenName Name of the token. Used in logging, but also used as an input when generating the actual AES key,
* so the same value must be provided to {@link persistTokenInStorage}.
*
* @returns the decrypted token, or the plain text token. Returns undefined when token cannot be decrypted
* @returns the decrypted token, or the plain text token.
*/
export async function tryDecryptToken(
pickleKey: string | undefined,
token: IEncryptedPayload | string | undefined,
token: IEncryptedPayload | string,
tokenName: string,
): Promise<string | undefined> {
if (pickleKey && isEncryptedPayload(token)) {
const encrKey = await pickleKeyToAesKey(pickleKey);
const decryptedToken = await decryptAES(token, encrKey, tokenName);
encrKey.fill(0);
return decryptedToken;
}
// if the token wasn't encrypted (plain string) just return it back
): Promise<string> {
if (typeof token === "string") {
// Looks like an unencrypted token
return token;
}
// otherwise return undefined

// Otherwise, it must be an encrypted token.
if (!pickleKey) {
throw new Error(`Error decrypting secret ${tokenName}: no pickle key found.`);

Check failure on line 99 in src/utils/tokens/tokens.ts

View workflow job for this annotation

GitHub Actions / Jest (1)

Lifecycle › restoreSessionFromStorage() › when session is found in storage › should throw if the token was persisted with a pickle key but there is no pickle key available now

Error decrypting secret access_token: no pickle key found. at tryDecryptToken (src/utils/tokens/tokens.ts:99:15) at restoreSessionFromStorage (src/Lifecycle.ts:597:59) at Object.<anonymous> (test/Lifecycle-test.ts:543:24)
}

const encrKey = await pickleKeyToAesKey(pickleKey);
const decryptedToken = await decryptAES(token, encrKey, tokenName);
encrKey.fill(0);
return decryptedToken;
}

/**
Expand Down
23 changes: 22 additions & 1 deletion test/Lifecycle-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,12 @@ describe("Lifecycle", () => {
mockStore[tableKey] = table;
},
);
jest.spyOn(StorageAccess, "idbDelete").mockClear().mockResolvedValue(undefined);
jest.spyOn(StorageAccess, "idbDelete")
.mockClear()
.mockImplementation(async (tableKey: string, key: string | string[]) => {
const table = mockStore[tableKey];
delete table?.[key as string];
});
};

const homeserverUrl = "https://server.org";
Expand Down Expand Up @@ -521,6 +526,22 @@ describe("Lifecycle", () => {

expect(await restoreSessionFromStorage()).toEqual(true);
});

it("should throw if the token was persisted with a pickle key but there is no pickle key available now", async () => {
initLocalStorageMock(localStorageSession);
initIdbMock({});

// Create a pickle key, and store it, encrypted, in IDB.
const pickleKey = (await PlatformPeg.get()!.createPickleKey(credentials.userId, credentials.deviceId))!;
localStorage.setItem("mx_has_pickle_key", "true");
await persistAccessTokenInStorage(credentials.accessToken, pickleKey);

// Now destroy the pickle key
await PlatformPeg.get()!.destroyPickleKey(credentials.userId, credentials.deviceId);

console.log("10");
expect(await restoreSessionFromStorage()).toEqual(true);
});
});
});

Expand Down

0 comments on commit ab50038

Please sign in to comment.