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

Log clearer errors when picklekey goes missing #12971

Closed
wants to merge 3 commits into from
Closed

Log clearer errors when picklekey goes missing #12971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
642058d
tokens.ts: improve documentation
richvdh Sep 11, 2024
b13655b
rename restoreFromLocalStorage
richvdh Sep 11, 2024
ab50038
Lifecycle: bail out if picklekey is missing
richvdh Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/Lifecycle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise<boolean>
false,
).then(() => true);
}
const success = await restoreFromLocalStorage({
const success = await restoreSessionFromStorage({
ignoreGuest: Boolean(opts.ignoreGuest),
});
if (success) {
Expand Down Expand Up @@ -556,17 +556,19 @@ async function abortLogin(): Promise<void> {
}
}

// returns a promise which resolves to true if a session is found in
// localstorage
//
// N.B. Lifecycle.js should not maintain any further localStorage state, we
// are moving towards using SessionStore to keep track of state related
// to the current session (which is typically backed by localStorage).
//
// The plan is to gradually move the localStorage access done here into
// SessionStore to avoid bugs where the view becomes out-of-sync with
// localStorage (e.g. isGuest etc.)
export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): Promise<boolean> {
/** Attempt to restore the session from localStorage or indexeddb.
*
* @returns true if a session was found; false if no existing session was found.
*
* N.B. Lifecycle.js should not maintain any further localStorage state, we
* are moving towards using SessionStore to keep track of state related
* to the current session (which is typically backed by localStorage).
*
* The plan is to gradually move the localStorage access done here into
* SessionStore to avoid bugs where the view becomes out-of-sync with
* localStorage (e.g. isGuest etc.)
*/
export async function restoreSessionFromStorage(opts?: { ignoreGuest?: boolean }): Promise<boolean> {
const ignoreGuest = opts?.ignoreGuest;

if (!localStorage) {
Expand All @@ -590,10 +592,11 @@ export async function restoreFromLocalStorage(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 @@ -603,7 +606,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }):
{
userId: userId,
deviceId: deviceId,
accessToken: decryptedAccessToken!,
accessToken: decryptedAccessToken,
refreshToken: decryptedRefreshToken,
homeserverUrl: hsUrl,
identityServerUrl: isUrl,
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/MatrixChat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
// Create and start the client
// accesses the new credentials just set in storage during attemptDelegatedAuthLogin
// and sets logged in state
await Lifecycle.restoreFromLocalStorage({ ignoreGuest: true });
await Lifecycle.restoreSessionFromStorage({ ignoreGuest: true });
await this.postLoginSetup();
return;
}
Expand Down
95 changes: 56 additions & 39 deletions src/utils/tokens/tokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
*/

/*
* Keys used when storing the tokens in indexeddb or localstorage
* Names used when storing the tokens in indexeddb or localstorage
*/
export const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token";
export const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token";
/*
* Used as initialization vector during encryption in persistTokenInStorage
* And decryption in restoreFromLocalStorage
* Names of the tokens. Used as part of the calculation to derive AES keys during encryption in persistTokenInStorage,
* and decryption in restoreSessionFromStorage.
*/
export const ACCESS_TOKEN_IV = "access_token";
export const REFRESH_TOKEN_IV = "refresh_token";
Expand Down Expand Up @@ -68,50 +68,63 @@
);
}

const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): token is IEncryptedPayload => {
return !!token && typeof token !== "string";
};
/**
* Try to decrypt a token retrieved from storage
* Where token is not encrypted (plain text) returns the plain text token
* Where token is encrypted, attempts decryption. Returns successfully decrypted token, else undefined.
* @param pickleKey pickle key used during encryption of token, or undefined
* @param token
* @param tokenIv initialization vector used during encryption of token eg ACCESS_TOKEN_IV
* @returns the decrypted token, or the plain text token. Returns undefined when token cannot be decrypted
*
* Where token is not encrypted (plain text) returns the plain text token.
*
* Where token is encrypted, attempts decryption. Returns successfully decrypted token, or throws if
* decryption failed.
*
* @param pickleKey Pickle key: used to derive the encryption key, or undefined if the token is not encrypted.
* Must be the same as provided to {@link persistTokenInStorage}.
* @param token token to be decrypted.
* @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.
*/
export async function tryDecryptToken(
pickleKey: string | undefined,
token: IEncryptedPayload | string | undefined,
tokenIv: string,
): Promise<string | undefined> {
if (pickleKey && isEncryptedPayload(token)) {
const encrKey = await pickleKeyToAesKey(pickleKey);
const decryptedToken = await decryptAES(token, encrKey, tokenIv);
encrKey.fill(0);
return decryptedToken;
}
// if the token wasn't encrypted (plain string) just return it back
token: IEncryptedPayload | string,
tokenName: string,
): 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;
}

/**
* Persist a token in storage
* When pickle key is present, will attempt to encrypt the token
* Stores in idb, falling back to localStorage
*
* @param storageKey key used to store the token
* @param initializationVector Initialization vector for encrypting the token. Only used when `pickleKey` is present
* @param token the token to store, when undefined any existing token at the storageKey is removed from storage
* @param pickleKey optional pickle key used to encrypt token
* @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb, eg "mx_has_access_token".
* When pickle key is present, will attempt to encrypt the token. If encryption fails (typically because
* WebCrypto is unavailable), the key will be stored unencrypted.
*
* Stores in IndexedDB, falling back to localStorage.
*
* @param storageKey key used to store the token. Note: not an encryption key; rather a localstorage or indexeddb key.
* @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 tryDecryptToken} when decrypting.
* @param token the token to store. When undefined, any existing token at the `storageKey` is removed from storage.
* @param pickleKey Pickle key: used to derive the key used to encrypt token. If `undefined`, the token will be stored
* unencrypted.
* @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb,
* eg "mx_has_access_token".
*/
export async function persistTokenInStorage(
storageKey: string,
initializationVector: string,
tokenName: string,
token: string | undefined,
pickleKey: string | undefined,
hasTokenStorageKey: string,
Expand All @@ -130,12 +143,12 @@
try {
// try to encrypt the access token using the pickle key
const encrKey = await pickleKeyToAesKey(pickleKey);
encryptedToken = await encryptAES(token, encrKey, initializationVector);
encryptedToken = await encryptAES(token, encrKey, tokenName);
encrKey.fill(0);
} catch (e) {
// This is likely due to the browser not having WebCrypto or somesuch.
// Warn about it, but fall back to storing the unencrypted token.
logger.warn(`Could not encrypt token for ${storageKey}`, e);
logger.warn(`Could not encrypt token for ${tokenName}`, e);
}
}
try {
Expand Down Expand Up @@ -167,9 +180,11 @@
}

/**
* Wraps persistTokenInStorage with accessToken storage keys
* @param token the token to store, when undefined any existing accessToken is removed from storage
* @param pickleKey optional pickle key used to encrypt token
* Wraps {@link persistTokenInStorage} with accessToken storage keys
*
* @param token - The token to store. When undefined, any existing accessToken is removed from storage.
* @param pickleKey - Pickle key: used to derive the key used to encrypt token. If `undefined`, the token will be stored
* unencrypted.
*/
export async function persistAccessTokenInStorage(
token: string | undefined,
Expand All @@ -185,9 +200,11 @@
}

/**
* Wraps persistTokenInStorage with refreshToken storage keys
* @param token the token to store, when undefined any existing refreshToken is removed from storage
* @param pickleKey optional pickle key used to encrypt token
* Wraps {@link persistTokenInStorage} with refreshToken storage keys.
*
* @param token - The token to store. When undefined, any existing refreshToken is removed from storage.
* @param pickleKey - Pickle key: used to derive the key used to encrypt token. If `undefined`, the token will be stored
* unencrypted.
*/
export async function persistRefreshTokenInStorage(
token: string | undefined,
Expand Down
Loading
Loading