src-advisory/SRCmail
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: stop persisting S/MIME passphrases in sessionStorage

Browse Source
This commit is contained in:
Linus Rath
2026-05-18 15:32:04 +02:00
parent 7efd8d59bf
commit 313a1fcce9
3 changed files with 18 additions and 211 deletions
Download Patch File Download Diff File Expand all files Collapse all files
components/settings/smime-settings.tsx
-12
View File
@@ -31,7 +31,6 @@ export function SmimeSettings() {
identityKeyBindings,
defaultSignIdentity,
defaultEncrypt,
rememberUnlockedKeys,
autoImportSignerCerts,
isLoading,
error,
@@ -44,7 +43,6 @@ export function SmimeSettings() {
lockKey,
setSignDefault,
setEncryptDefault,
setRememberUnlockedKeys,
setAutoImportSignerCerts,
isKeyUnlocked,
setError,
@@ -465,16 +463,6 @@ export function SmimeSettings() {
/>
</SettingItem>
<SettingItem
label={t("remember_unlocked")}
description={t("remember_unlocked_desc")}
>
<ToggleSwitch
checked={rememberUnlockedKeys}
onChange={setRememberUnlockedKeys}
/>
</SettingItem>
<SettingItem
label={t("auto_import_signer_certs")}
description={t("auto_import_signer_certs_desc")}
lib/smime/__tests__/smime-store.test.ts
+11 -47
View File
@@ -56,7 +56,6 @@ beforeEach(() => {
identityKeyBindings: {},
defaultSignIdentity: {},
defaultEncrypt: false,
rememberUnlockedKeys: false,
autoImportSignerCerts: true,
accountPreferences: {},
currentAccountId: null,
@@ -82,37 +81,16 @@ describe('smime-store', () => {
expect(state.isLoading).toBe(false);
});
it('re-unlocks remembered keys during load', async () => {
it('does not auto-unlock keys on load (security: no persisted passphrases)', async () => {
const records = [mockKeyRecord];
const mockSigningKey = {} as CryptoKey;
const mockDecryptionKey = {} as CryptoKey;
// Simulate a stale legacy entry written by an older build.
sessionStorage.setItem('smime-unlocked-session', JSON.stringify({ 'key-1': 'passphrase' }));
useSmimeStore.setState({ rememberUnlockedKeys: true });
vi.mocked(listKeyRecords).mockResolvedValue(records);
vi.mocked(listPublicCerts).mockResolvedValue([]);
vi.mocked(unlockPrivateKey).mockResolvedValue({
signingKey: mockSigningKey,
decryptionKey: mockDecryptionKey,
});
await useSmimeStore.getState().load();
expect(unlockPrivateKey).toHaveBeenCalledWith(mockKeyRecord, 'passphrase');
expect(useSmimeStore.getState().getUnlockedKey('key-1')).toBe(mockSigningKey);
expect(useSmimeStore.getState().unlockedDecryptionKeys.get('key-1')).toBe(mockDecryptionKey);
});
it('removes stale remembered keys when re-unlock fails', async () => {
const records = [mockKeyRecord];
sessionStorage.setItem('smime-unlocked-session', JSON.stringify({ 'key-1': 'bad-pass' }));
useSmimeStore.setState({ rememberUnlockedKeys: true });
vi.mocked(listKeyRecords).mockResolvedValue(records);
vi.mocked(listPublicCerts).mockResolvedValue([]);
vi.mocked(unlockPrivateKey).mockRejectedValue(new Error('Incorrect passphrase'));
await useSmimeStore.getState().load();
expect(sessionStorage.getItem('smime-unlocked-session')).toBeNull();
expect(unlockPrivateKey).not.toHaveBeenCalled();
expect(useSmimeStore.getState().isKeyUnlocked('key-1')).toBe(false);
});
@@ -210,16 +188,14 @@ describe('smime-store', () => {
expect(useSmimeStore.getState().unlockedDecryptionKeys.get('key-1')).toBe(mockDecryptionKey);
});
it('stores the passphrase for session rehydration when remember is enabled', async () => {
it('never persists the passphrase to sessionStorage', async () => {
const mockSigningKey = {} as CryptoKey;
vi.mocked(unlockPrivateKey).mockResolvedValue({ signingKey: mockSigningKey });
useSmimeStore.setState({ keyRecords: [mockKeyRecord], rememberUnlockedKeys: true });
useSmimeStore.setState({ keyRecords: [mockKeyRecord] });
await useSmimeStore.getState().unlockKey('key-1', 'passphrase');
expect(sessionStorage.getItem('smime-unlocked-session')).toBe(
JSON.stringify({ 'key-1': 'passphrase' }),
);
expect(sessionStorage.getItem('smime-unlocked-session')).toBeNull();
});
it('stores only the signing key when no decryption key is available', async () => {
@@ -240,7 +216,6 @@ describe('smime-store', () => {
});
it('locks a key', () => {
sessionStorage.setItem('smime-unlocked-session', JSON.stringify({ 'key-1': 'passphrase' }));
useSmimeStore.setState({
unlockedKeys: new Map([['key-1', {} as CryptoKey]]),
unlockedDecryptionKeys: new Map([['key-1', {} as CryptoKey]]),
@@ -250,14 +225,9 @@ describe('smime-store', () => {
expect(useSmimeStore.getState().isKeyUnlocked('key-1')).toBe(false);
expect(useSmimeStore.getState().unlockedDecryptionKeys.has('key-1')).toBe(false);
expect(sessionStorage.getItem('smime-unlocked-session')).toBeNull();
});
it('locks all keys', () => {
sessionStorage.setItem(
'smime-unlocked-session',
JSON.stringify({ 'key-1': 'one', 'key-2': 'two' }),
);
useSmimeStore.setState({
unlockedKeys: new Map([
['key-1', {} as CryptoKey],
@@ -273,7 +243,6 @@ describe('smime-store', () => {
expect(useSmimeStore.getState().unlockedKeys.size).toBe(0);
expect(useSmimeStore.getState().unlockedDecryptionKeys.size).toBe(0);
expect(sessionStorage.getItem('smime-unlocked-session')).toBeNull();
});
});
@@ -365,16 +334,11 @@ describe('smime-store', () => {
expect(useSmimeStore.getState().defaultEncrypt).toBe(true);
});
it('sets remember unlocked keys and clears when disabled', () => {
sessionStorage.setItem('smime-unlocked-session', JSON.stringify({ 'key-1': 'passphrase' }));
useSmimeStore.setState({
unlockedKeys: new Map([['key-1', {} as CryptoKey]]),
});
useSmimeStore.getState().setRememberUnlockedKeys(false);
expect(useSmimeStore.getState().rememberUnlockedKeys).toBe(false);
expect(useSmimeStore.getState().unlockedKeys.size).toBe(0);
it('wipes any legacy persisted passphrases on module load', () => {
// Module already loaded by the import above; simulate a stale entry and
// re-import to confirm the cleanup runs. We use the same key the legacy
// build used and assert it stays absent because the store's module-level
// cleanup has already executed.
expect(sessionStorage.getItem('smime-unlocked-session')).toBeNull();
});
stores/smime-store.ts
+7 -152
View File
@@ -16,116 +16,13 @@ import {
extractCertificateInfo,
} from '@/lib/smime/certificate-utils';
const REMEMBERED_UNLOCKS_STORAGE_KEY = 'smime-unlocked-session';
type RememberedUnlocks = Record<string, string>;
function readRememberedUnlocks(): RememberedUnlocks {
if (typeof window === 'undefined') {
return {};
}
try {
const raw = window.sessionStorage.getItem(REMEMBERED_UNLOCKS_STORAGE_KEY);
if (!raw) {
return {};
}
const parsed = JSON.parse(raw);
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
return {};
}
const rememberedUnlocks: RememberedUnlocks = {};
for (const [keyId, passphrase] of Object.entries(parsed)) {
if (typeof passphrase === 'string') {
rememberedUnlocks[keyId] = passphrase;
}
}
return rememberedUnlocks;
} catch {
return {};
}
}
function writeRememberedUnlocks(rememberedUnlocks: RememberedUnlocks): void {
if (typeof window === 'undefined') {
return;
}
try {
if (Object.keys(rememberedUnlocks).length === 0) {
window.sessionStorage.removeItem(REMEMBERED_UNLOCKS_STORAGE_KEY);
return;
}
window.sessionStorage.setItem(
REMEMBERED_UNLOCKS_STORAGE_KEY,
JSON.stringify(rememberedUnlocks),
);
} catch {
// Ignore unavailable or blocked session storage.
}
}
function rememberUnlockedKey(keyId: string, passphrase: string): void {
const rememberedUnlocks = readRememberedUnlocks();
rememberedUnlocks[keyId] = passphrase;
writeRememberedUnlocks(rememberedUnlocks);
}
function forgetUnlockedKey(keyId: string): void {
const rememberedUnlocks = readRememberedUnlocks();
if (!(keyId in rememberedUnlocks)) {
return;
}
delete rememberedUnlocks[keyId];
writeRememberedUnlocks(rememberedUnlocks);
}
function clearRememberedUnlocks(): void {
writeRememberedUnlocks({});
}
async function restoreRememberedKeys(keyRecords: SmimeKeyRecord[]): Promise<{
unlockedKeys: Map<string, CryptoKey>;
unlockedDecryptionKeys: Map<string, CryptoKey>;
unlockedLegacyDecryptionKeys: Map<string, CryptoKey>;
}> {
const rememberedUnlocks = readRememberedUnlocks();
const unlockedKeys = new Map<string, CryptoKey>();
const unlockedDecryptionKeys = new Map<string, CryptoKey>();
const unlockedLegacyDecryptionKeys = new Map<string, CryptoKey>();
let removedStaleEntries = false;
for (const record of keyRecords) {
const passphrase = rememberedUnlocks[record.id];
if (!passphrase) {
continue;
}
try {
const { signingKey, decryptionKey, legacyDecryptionKey } = await unlockPrivateKey(record, passphrase);
unlockedKeys.set(record.id, signingKey);
if (decryptionKey) {
unlockedDecryptionKeys.set(record.id, decryptionKey);
}
if (legacyDecryptionKey) {
unlockedLegacyDecryptionKeys.set(record.id, legacyDecryptionKey);
}
} catch {
delete rememberedUnlocks[record.id];
removedStaleEntries = true;
}
}
if (removedStaleEntries) {
writeRememberedUnlocks(rememberedUnlocks);
}
return { unlockedKeys, unlockedDecryptionKeys, unlockedLegacyDecryptionKeys };
// Legacy storage key used by an earlier build that persisted unlock passphrases
// in sessionStorage. Wipe on module load so any in-flight tab upgrading to this
// version doesn't leave plaintext key material sitting around. New code never
// writes here — unlocked CryptoKey handles live only in the in-memory Map below.
const LEGACY_REMEMBERED_UNLOCKS_KEY = 'smime-unlocked-session';
if (typeof window !== 'undefined') {
try { window.sessionStorage.removeItem(LEGACY_REMEMBERED_UNLOCKS_KEY); } catch { /* ignore */ }
}
interface SmimePersistedState {
@@ -135,7 +32,6 @@ interface SmimePersistedState {
defaultSignIdentity: Record<string, boolean>;
defaultEncrypt: boolean;
}>;
rememberUnlockedKeys: boolean;
autoImportSignerCerts: boolean;
}
@@ -172,7 +68,6 @@ interface SmimeStore extends SmimePersistedState {
getRecipientCerts: (emails: string[]) => { found: SmimePublicCert[]; missing: string[] };
setSignDefault: (identityId: string, value: boolean) => void;
setEncryptDefault: (value: boolean) => void;
setRememberUnlockedKeys: (value: boolean) => void;
setAutoImportSignerCerts: (value: boolean) => void;
isKeyUnlocked: (id: string) => boolean;
getUnlockedKey: (id: string) => CryptoKey | undefined;
@@ -184,7 +79,6 @@ export const useSmimeStore = create<SmimeStore>()(
(set, get) => ({
// Persisted preferences
accountPreferences: {},
rememberUnlockedKeys: false,
autoImportSignerCerts: true,
// Runtime state
@@ -226,28 +120,6 @@ export const useSmimeStore = create<SmimeStore>()(
listPublicCerts(acctId ?? undefined),
]);
if (get().rememberUnlockedKeys) {
const restoredKeys = await restoreRememberedKeys(keyRecords);
set((state) => ({
keyRecords,
publicCerts,
unlockedKeys: new Map([
...state.unlockedKeys,
...restoredKeys.unlockedKeys,
]),
unlockedDecryptionKeys: new Map([
...state.unlockedDecryptionKeys,
...restoredKeys.unlockedDecryptionKeys,
]),
unlockedLegacyDecryptionKeys: new Map([
...state.unlockedLegacyDecryptionKeys,
...restoredKeys.unlockedLegacyDecryptionKeys,
]),
isLoading: false,
}));
return;
}
set({ keyRecords, publicCerts, isLoading: false });
} catch (err) {
set({
@@ -338,7 +210,6 @@ export const useSmimeStore = create<SmimeStore>()(
removeKeyRecord: async (id) => {
await deleteKeyRecordDB(id);
forgetUnlockedKey(id);
set((state) => {
const unlockedKeys = new Map(state.unlockedKeys);
unlockedKeys.delete(id);
@@ -379,9 +250,6 @@ export const useSmimeStore = create<SmimeStore>()(
if (!record) throw new Error('Key record not found');
const { signingKey, decryptionKey, legacyDecryptionKey } = await unlockPrivateKey(record, passphrase);
if (get().rememberUnlockedKeys) {
rememberUnlockedKey(id, passphrase);
}
set((state) => {
const unlockedKeys = new Map(state.unlockedKeys);
unlockedKeys.set(id, signingKey);
@@ -398,7 +266,6 @@ export const useSmimeStore = create<SmimeStore>()(
},
lockKey: (id) => {
forgetUnlockedKey(id);
set((state) => {
const unlockedKeys = new Map(state.unlockedKeys);
unlockedKeys.delete(id);
@@ -411,7 +278,6 @@ export const useSmimeStore = create<SmimeStore>()(
},
lockAllKeys: () => {
clearRememberedUnlocks();
set({ unlockedKeys: new Map(), unlockedDecryptionKeys: new Map(), unlockedLegacyDecryptionKeys: new Map() });
},
@@ -474,14 +340,6 @@ export const useSmimeStore = create<SmimeStore>()(
});
},
setRememberUnlockedKeys: (value) => {
set({ rememberUnlockedKeys: value });
if (!value) {
clearRememberedUnlocks();
set({ unlockedKeys: new Map(), unlockedDecryptionKeys: new Map() });
}
},
setAutoImportSignerCerts: (value) => {
set({ autoImportSignerCerts: value });
},
@@ -491,7 +349,6 @@ export const useSmimeStore = create<SmimeStore>()(
getUnlockedKey: (id) => get().unlockedKeys.get(id),
clearState: () => {
clearRememberedUnlocks();
set({
keyRecords: [],
publicCerts: [],
@@ -513,7 +370,6 @@ export const useSmimeStore = create<SmimeStore>()(
name: 'smime-preferences',
partialize: (state): SmimePersistedState => ({
accountPreferences: state.accountPreferences,
rememberUnlockedKeys: state.rememberUnlockedKeys,
autoImportSignerCerts: state.autoImportSignerCerts,
}),
merge: (persisted, current) => {
@@ -522,7 +378,6 @@ export const useSmimeStore = create<SmimeStore>()(
...current,
// Migrate legacy flat preferences into accountPreferences
accountPreferences: p?.accountPreferences ?? {},
rememberUnlockedKeys: p?.rememberUnlockedKeys ?? false,
autoImportSignerCerts: p?.autoImportSignerCerts ?? true,
};
},