Skip to content

Commit

Permalink
Log clearer errors when picklekey goes missing (#27)
Browse files Browse the repository at this point in the history 
* tokens.ts: improve documentation

Improve variable naming and documentation on the methods in `tokens.ts`.

* rename restoreFromLocalStorage

Since the session data isn't actually stored in localstorage, this feels like a
misleading name.

* Lifecycle: bail out if picklekey is missing

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
richvdh authored Sep 11, 2024
1 parent d337fba commit 433c14e
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 77 deletions.
33 changes: 18 additions & 15 deletions src/Lifecycle.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,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 @@ -548,17 +548,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 @@ -582,10 +584,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 @@ -595,7 +598,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
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,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
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import * as StorageAccess from "../StorageAccess";
*/

/*
* 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 @@ -60,50 +60,63 @@ 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
* 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.`);
}

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 @@ -122,12 +135,12 @@ export async function persistTokenInStorage(
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 @@ -159,9 +172,11 @@ export async function persistTokenInStorage(
}

/**
* 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 @@ -177,9 +192,11 @@ export async function persistAccessTokenInStorage(
}

/**
* 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

0 comments on commit 433c14e

Please sign in to comment.