mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(sync): harden encryption batching and SuperSync abort timer
- super-sync: keep the 75s abort timer alive across response.text() on non-OK responses so a stalled error body still triggers AbortError. - decryptBatch: hold derived keys in a batch-local map. Previously Phase 3 read from the LRU session cache, which could evict entries mid-batch when the input contained more unique salts than the cache could hold (SESSION_DECRYPT_CACHE_MAX_SIZE = 100), crashing on the non-null assertion. Adds a 120-item regression test. - session cache: replace the 32-bit djb2 password identifier with a length-prefixed full-password key (injective). A djb2 collision silently returned a key derived from a different password, producing undecryptable ciphertext on subsequent encrypts. - docs: bless salt-per-session-per-password semantics explicitly in the encryption.ts JSDoc and the sync-core README so future readers and tests know IV uniqueness — not salt uniqueness — is the AES-GCM invariant being preserved.
This commit is contained in:
parent
4e1ace0786
commit
18cae275f5
6 changed files with 75 additions and 32 deletions
|
|
@ -29,6 +29,11 @@ const plain = await decrypt(cipher, password);
|
|||
|
||||
All ciphertexts are base64-encoded for transport. The format is discriminated by length: `< 28` bytes is invalid, `< 44` bytes is unambiguously legacy, `>= 44` bytes is treated as Argon2id with a legacy fallback on auth failure. Do not change this without a versioning migration.
|
||||
|
||||
### Salt and IV semantics
|
||||
|
||||
- The **IV** (12 bytes) is freshly random per call. AES-GCM security under a fixed key reduces to IV uniqueness, which this guarantees.
|
||||
- The **salt** (16 bytes) is derived once per `(process session, password)` pair and reused across every `encrypt`/`encryptWithDerivedKey`/`encryptBatch` call in that session. This is intentional — it lets the session cache amortize the ~500 ms–2 s Argon2id derivation. Two encryptions of the same plaintext within a session therefore share the salt prefix and differ only in IV and ciphertext. Do not assert per-call salt uniqueness in tests.
|
||||
|
||||
### Session key caching
|
||||
|
||||
`encrypt`/`decrypt`/`encryptBatch`/`decryptBatch` all share three in-memory caches (encrypt key, decrypt key by salt, legacy PBKDF2 key) that survive across sync cycles. Argon2id derivation is expensive (~500–2000 ms on mobile with the default 64 MiB / 3 iterations); the cache turns repeated syncs from minutes into seconds.
|
||||
|
|
|
|||
|
|
@ -17,6 +17,21 @@
|
|||
* by length: < 28 bytes is invalid, < 44 bytes is unambiguously legacy,
|
||||
* >= 44 bytes is treated as Argon2id with a legacy fallback on auth failure.
|
||||
*
|
||||
* ## Salt and IV semantics
|
||||
*
|
||||
* - **IV** is freshly random per call (12 bytes from CSPRNG). AES-GCM
|
||||
* confidentiality and integrity depend on IV uniqueness under a given key,
|
||||
* which this guarantees.
|
||||
* - **Salt** is derived once per (process session, password) and reused
|
||||
* across every `encrypt()` / `encryptWithDerivedKey()` / `encryptBatch()`
|
||||
* call in that session. This is intentional: it lets us amortize the
|
||||
* ~500ms-2s Argon2id derivation across many calls via the session cache.
|
||||
* A session-stable salt is safe because the derived key never changes for
|
||||
* the same (salt, password) pair, and AES-GCM security under a fixed key
|
||||
* reduces to IV uniqueness. Two encryptions of the same plaintext within
|
||||
* a session therefore share the salt prefix and differ only in IV +
|
||||
* ciphertext — do not assert otherwise in tests.
|
||||
*
|
||||
* ## Legacy-decrypt diagnostics
|
||||
*
|
||||
* Two complementary mechanisms surface legacy ciphertext to callers; both are
|
||||
|
|
@ -49,7 +64,6 @@ import { type DerivedKey, deriveKeyFromPassword } from './encryption/argon2';
|
|||
import {
|
||||
getDecryptCache,
|
||||
getOrDeriveEncryptKey,
|
||||
hasDecryptCache,
|
||||
setDecryptCache,
|
||||
} from './encryption/session-cache';
|
||||
import { decryptLegacy } from './encryption/legacy';
|
||||
|
|
@ -266,8 +280,7 @@ export const decryptBatch = async (
|
|||
interface ArgonItem {
|
||||
index: number;
|
||||
format: 'argon2';
|
||||
cacheKey: string;
|
||||
salt: Uint8Array;
|
||||
saltBase64: string;
|
||||
buffer: ArrayBuffer;
|
||||
}
|
||||
interface LegacyItem {
|
||||
|
|
@ -276,9 +289,9 @@ export const decryptBatch = async (
|
|||
data: string;
|
||||
}
|
||||
|
||||
// Phase 1: analyze items, decode once, collect unique salts needing derivation.
|
||||
// Phase 1: analyze items, decode once, collect every unique salt.
|
||||
const itemAnalysis: Array<ArgonItem | LegacyItem> = [];
|
||||
const saltsNeedingDerivation = new Map<string, Uint8Array>();
|
||||
const uniqueSalts = new Map<string, Uint8Array>();
|
||||
|
||||
for (let i = 0; i < dataItems.length; i++) {
|
||||
const data = dataItems[i];
|
||||
|
|
@ -296,36 +309,40 @@ export const decryptBatch = async (
|
|||
|
||||
const salt = new Uint8Array(buffer, 0, SALT_LENGTH);
|
||||
const saltBase64 = encodeBase64(salt);
|
||||
const cacheKey = `${passwordHash}:${saltBase64}`;
|
||||
|
||||
itemAnalysis.push({ index: i, format: 'argon2', cacheKey, salt, buffer });
|
||||
itemAnalysis.push({ index: i, format: 'argon2', saltBase64, buffer });
|
||||
|
||||
if (!hasDecryptCache(cacheKey) && !saltsNeedingDerivation.has(saltBase64)) {
|
||||
saltsNeedingDerivation.set(saltBase64, salt);
|
||||
if (!uniqueSalts.has(saltBase64)) {
|
||||
uniqueSalts.set(saltBase64, salt);
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 2: derive keys for unique salts SERIALLY.
|
||||
// Argon2id is single-threaded and allocates 64MB per derivation; parallel
|
||||
// derivations via Promise.all only interleave microtasks (no real parallelism)
|
||||
// and risk OOM on mobile.
|
||||
for (const [saltBase64, salt] of saltsNeedingDerivation) {
|
||||
// Phase 2: derive keys for unique salts SERIALLY and keep them in a
|
||||
// batch-local map. Argon2id is single-threaded and allocates 64MB per
|
||||
// derivation; parallel derivations via Promise.all only interleave microtasks
|
||||
// (no real parallelism) and risk OOM on mobile. Holding keys locally also
|
||||
// ensures Phase 3 cannot see an entry evicted by the LRU session cache
|
||||
// (capped at SESSION_DECRYPT_CACHE_MAX_SIZE) when a batch contains more
|
||||
// unique salts than the cache can hold.
|
||||
const batchKeys = new Map<string, DerivedKey>();
|
||||
for (const [saltBase64, salt] of uniqueSalts) {
|
||||
const cacheKey = `${passwordHash}:${saltBase64}`;
|
||||
if (hasDecryptCache(cacheKey)) {
|
||||
continue;
|
||||
let key = getDecryptCache(cacheKey);
|
||||
if (!key) {
|
||||
key = await deriveKeyFromPassword(password, salt);
|
||||
setDecryptCache(cacheKey, key);
|
||||
}
|
||||
const key = await deriveKeyFromPassword(password, salt);
|
||||
setDecryptCache(cacheKey, key);
|
||||
batchKeys.set(saltBase64, key);
|
||||
}
|
||||
|
||||
// Phase 3: decrypt all items in parallel using cached keys, reusing the
|
||||
// buffer decoded in phase 1 to avoid a second base64 decode.
|
||||
// Phase 3: decrypt all items in parallel using batch-local keys, reusing
|
||||
// the buffer decoded in phase 1 to avoid a second base64 decode.
|
||||
const decryptionPromises = itemAnalysis.map(async (item) => {
|
||||
if (item.format === 'legacy') {
|
||||
return { index: item.index, result: await decryptLegacy(item.data, password) };
|
||||
}
|
||||
|
||||
const key = getDecryptCache(item.cacheKey)!;
|
||||
const key = batchKeys.get(item.saltBase64)!;
|
||||
try {
|
||||
return {
|
||||
index: item.index,
|
||||
|
|
|
|||
|
|
@ -63,9 +63,6 @@ export const getOrDeriveEncryptKey = async (password: string): Promise<DerivedKe
|
|||
export const getDecryptCache = (cacheKey: string): DerivedKey | undefined =>
|
||||
sessionDecryptKeyCache.get(cacheKey);
|
||||
|
||||
export const hasDecryptCache = (cacheKey: string): boolean =>
|
||||
sessionDecryptKeyCache.has(cacheKey);
|
||||
|
||||
export const setDecryptCache = (cacheKey: string, key: DerivedKey): void => {
|
||||
if (sessionDecryptKeyCache.size >= SESSION_DECRYPT_CACHE_MAX_SIZE) {
|
||||
const firstKey = sessionDecryptKeyCache.keys().next().value;
|
||||
|
|
|
|||
|
|
@ -136,13 +136,17 @@ export const detectFormat = (
|
|||
};
|
||||
|
||||
/**
|
||||
* Simple hash of password for cache key comparison (djb2).
|
||||
* NOT for security — just for cache invalidation when password changes.
|
||||
* Returns an injective string identifier for a password, used as a cache-map
|
||||
* key (and as a prefix in decrypt-cache keys like `${id}:${saltBase64}`).
|
||||
*
|
||||
* Length-prefixed so concatenation stays unambiguous even if the password
|
||||
* contains the `:` separator used downstream. Holding the password verbatim
|
||||
* is safe: these caches live in-memory in the same process that already holds
|
||||
* the plaintext password.
|
||||
*
|
||||
* Earlier versions used a 32-bit djb2 hash; collisions would silently return
|
||||
* a key derived from a different password, producing undecryptable ciphertext.
|
||||
*/
|
||||
export const hashPasswordForCache = (password: string): string => {
|
||||
let hash = 5381;
|
||||
for (let i = 0; i < password.length; i++) {
|
||||
hash = (hash * 33) ^ password.charCodeAt(i);
|
||||
}
|
||||
return (hash >>> 0).toString(16);
|
||||
return `${password.length}:${password}`;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -284,6 +284,24 @@ describe('encryption', () => {
|
|||
expect(decrypted[0]).toBe('item-0');
|
||||
expect(decrypted[49]).toBe('item-49');
|
||||
});
|
||||
|
||||
// SESSION_DECRYPT_CACHE_MAX_SIZE is 100; if a batch contains more unique
|
||||
// salts than the LRU cache can hold, early derivations get evicted before
|
||||
// Phase 3 reads them. The implementation keeps derived keys in a
|
||||
// batch-local map to keep this safe.
|
||||
it('decrypts batches with more unique salts than the LRU cache holds', async () => {
|
||||
clearSessionKeyCache();
|
||||
const COUNT = 120;
|
||||
const items = Array.from({ length: COUNT }, (_, i) => `item-${i}`);
|
||||
const encrypted: string[] = [];
|
||||
for (let i = 0; i < COUNT; i++) {
|
||||
// Fresh derivation per item → unique salt per ciphertext
|
||||
const key = await deriveKeyFromPassword(PASSWORD);
|
||||
encrypted.push(await encryptWithDerivedKey(items[i], key));
|
||||
}
|
||||
const decrypted = await decryptBatch(encrypted, PASSWORD);
|
||||
expect(decrypted).toEqual(items);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Session key caching', () => {
|
||||
|
|
|
|||
|
|
@ -853,8 +853,10 @@ export class SuperSyncProvider
|
|||
});
|
||||
|
||||
if (!response.ok) {
|
||||
clearTimeout(timeoutId);
|
||||
// Keep the abort timer running across response.text() — a server/proxy
|
||||
// that sends an error status and stalls the body must still be aborted.
|
||||
const errorText = await response.text().catch(() => '');
|
||||
clearTimeout(timeoutId);
|
||||
// Check for auth failure FIRST before throwing generic error
|
||||
this._checkHttpStatus(response.status, errorText);
|
||||
const reason = this._extractServerErrorReason(errorText, response.status);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue