Skip to content

Commit b63ab91

Browse files
NiKrauseclaude
andcommitted
fix: actually write the secret key into largeBlob
The keystore record carried: secretKey: sk, // Will be moved to largeBlob Nothing ever moved it. That was the only occurrence of the field in all of src/, and storeEncryptedKeystore() serialises a fixed whitelist which does not include it — so the key was dropped at persist time. Choosing largeBlob produced a keystore that worked for exactly the session that created it and could never be unlocked afterwards: the next load called retrieveSKFromLargeBlob(), read a blob nobody had written, and failed with "No largeBlob data found in credential". Worth stating plainly, because the first suspicion was worse: the key never reached disk in the clear. The whitelist that dropped it also kept it out of localStorage. The feature was inert, not leaky. writeSKToLargeBlob() now performs the assertion that stores it — a blob can only be written during an assertion, never at registration, which is why addLargeBlobToCredentialOptions() alone persists nothing and why this costs one extra prompt. It throws unless the authenticator reports `written: true`. Failing there beats persisting a record that cannot be opened, and it is the same lesson as the rest of this branch: read what happened, do not trust what was requested. Tests: 5 new cases covering confirmed write, declined write, absent result, a truthy-but-not-true value, and a refused assertion. 63 node tests pass. One wrinkle worth recording: Node defines globalThis.navigator as a getter-only accessor, so the obvious `globalThis.navigator = stub` throws TypeError: Cannot set property navigator. The stub swaps the property descriptor and restores the original afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2590e14 commit b63ab91

5 files changed

Lines changed: 178 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,23 @@
44

55
### Fixed
66

7+
- Actually write the secret key into `largeBlob`. The keystore record carried
8+
`secretKey: sk, // Will be moved to largeBlob` — and nothing ever moved it.
9+
That was the only occurrence of the field in `src/`, and
10+
`storeEncryptedKeystore()` serialises a fixed whitelist that does not include
11+
it, so the key was dropped at persist time. Choosing largeBlob therefore
12+
produced a keystore that worked for the session that created it and could
13+
never be unlocked again: on the next load `retrieveSKFromLargeBlob()` read a
14+
blob nobody had written and failed with "No largeBlob data found in
15+
credential".
16+
17+
The key never reached disk in the clear — the whitelist saw to that — but the
18+
feature was inert. `writeSKToLargeBlob()` now performs the assertion that
19+
stores it (a blob can only be written during an assertion, never at
20+
registration, so this costs one extra prompt) and **throws unless the
21+
authenticator reports `written: true`**. Failing there beats persisting a
22+
record that cannot be opened.
23+
724
- Carry the authenticator's own answer on the credential. `createCredential()`
825
now records `extensionSupport` — what the authenticator agreed to during the
926
ceremony — because the raw `PublicKeyCredential` does not survive past that

src/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ export {
277277
decryptWithAESGCM,
278278
addLargeBlobToCredentialOptions,
279279
addPRFToCredentialOptions,
280+
writeSKToLargeBlob,
280281
retrieveSKFromLargeBlob,
281282
addHmacSecretToCredentialOptions,
282283
wrapSKWithPRF,

src/keystore/encryption.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,67 @@ export function addPRFToCredentialOptions(
213213
};
214214
}
215215

216+
/**
217+
* Write the secret key into the credential's largeBlob.
218+
*
219+
* A blob can only be written during an assertion, never at registration — so
220+
* this costs one extra WebAuthn prompt after the credential exists. That is
221+
* why `addLargeBlobToCredentialOptions()` alone does not persist anything.
222+
*
223+
* Throws unless the authenticator reports `written: true`. Silence here would
224+
* leave a keystore whose key exists only in memory: it works for the current
225+
* session and can never be unlocked again, because
226+
* {@link retrieveSKFromLargeBlob} would read a blob nobody wrote.
227+
*
228+
* @param {Uint8Array} credentialId - WebAuthn credential ID (raw bytes).
229+
* @param {Uint8Array} sk - Secret key to store.
230+
* @param {string} rpId - Relying party ID (domain).
231+
* @returns {Promise<void>}
232+
*/
233+
export async function writeSKToLargeBlob(credentialId, sk, rpId) {
234+
log('Writing secret key to largeBlob');
235+
236+
let assertion;
237+
try {
238+
assertion = await navigator.credentials.get(
239+
buildCredentialRequestOptions({
240+
challenge: crypto.getRandomValues(new Uint8Array(32)),
241+
credentialId,
242+
rpId,
243+
userVerification: 'required',
244+
extensions: {
245+
largeBlob: {
246+
write: sk,
247+
},
248+
},
249+
})
250+
);
251+
} catch (error) {
252+
log.error('largeBlob write assertion failed: %s', error.message);
253+
throw new KeystoreEncryptionError(
254+
`Failed to write secret key to largeBlob: ${error.message}`,
255+
{ cause: error }
256+
);
257+
}
258+
259+
// The authenticator answers whether it took the blob. Accepting the
260+
// assertion without reading this is how a write that never happened passes
261+
// for a success.
262+
const written = assertion?.getClientExtensionResults?.()?.largeBlob?.written;
263+
264+
if (written !== true) {
265+
log.error(
266+
'Authenticator did not write the largeBlob (written: %o)',
267+
written
268+
);
269+
throw new KeystoreEncryptionError(
270+
'Authenticator did not store the secret key in largeBlob'
271+
);
272+
}
273+
274+
log('Secret key written to largeBlob');
275+
}
276+
216277
/**
217278
* Retrieve secret key from WebAuthn credential using largeBlob extension
218279
* @param {Uint8Array} credentialId - WebAuthn credential ID
@@ -734,6 +795,7 @@ export default {
734795
encryptWithAESGCM,
735796
decryptWithAESGCM,
736797
addLargeBlobToCredentialOptions,
798+
writeSKToLargeBlob,
737799
retrieveSKFromLargeBlob,
738800
addHmacSecretToCredentialOptions,
739801
wrapSKWithHmacSecret,

src/keystore/provider.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,15 +408,24 @@ export class OrbitDBWebAuthnIdentityProvider {
408408
} else if (
409409
this.keystoreEncryptionMethod === KEYSTORE_ENCRYPTION_METHODS.LARGE_BLOB
410410
) {
411-
// For largeBlob, we need to store SK during next authentication
412-
// Store it temporarily for wrapping
411+
// Write the key into the authenticator before recording anything.
412+
// This used to be `secretKey: sk, // Will be moved to largeBlob` — and
413+
// nothing ever moved it: the field was dropped at serialization, so
414+
// the key survived only in memory and the keystore could never be
415+
// unlocked again. Throws if the authenticator refuses, which is better
416+
// than persisting a record that cannot be opened.
417+
await KeystoreEncryption.writeSKToLargeBlob(
418+
this.credential.rawCredentialId,
419+
sk,
420+
window.location.hostname
421+
);
422+
413423
encryptedData = {
414424
ciphertext,
415425
iv,
416426
credentialId: this.credential.credentialId,
417427
publicKey: publicKeyBytes,
418428
keyType,
419-
secretKey: sk, // Will be moved to largeBlob
420429
encryptionMethod: KEYSTORE_ENCRYPTION_METHODS.LARGE_BLOB,
421430
};
422431
} else if (

tests/webauthn-extension-support.test.js

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,46 @@ import { test, expect } from '@playwright/test';
1414
import {
1515
checkExtensionSupport,
1616
extensionSupportFromCredential,
17+
writeSKToLargeBlob,
1718
} from '../src/keystore/encryption.js';
1819

20+
const CREDENTIAL_ID = new Uint8Array(32).fill(0x11);
21+
const SECRET_KEY = new Uint8Array(32).fill(0x22);
22+
23+
// Node defines globalThis.navigator as a getter-only accessor, so a plain
24+
// assignment throws. Swap the property descriptor and put the original back.
25+
const ORIGINAL_NAVIGATOR = Object.getOwnPropertyDescriptor(
26+
globalThis,
27+
'navigator'
28+
);
29+
30+
/**
31+
* Stand in for navigator.credentials.get, answering with whatever the
32+
* authenticator is meant to have reported back.
33+
*/
34+
function installCredentialsGet(extensionResults, { rejectWith } = {}) {
35+
Object.defineProperty(globalThis, 'navigator', {
36+
configurable: true,
37+
writable: true,
38+
value: {
39+
credentials: {
40+
get: async () => {
41+
if (rejectWith) throw rejectWith;
42+
return { getClientExtensionResults: () => extensionResults };
43+
},
44+
},
45+
},
46+
});
47+
}
48+
49+
function restoreNavigator() {
50+
if (ORIGINAL_NAVIGATOR) {
51+
Object.defineProperty(globalThis, 'navigator', ORIGINAL_NAVIGATOR);
52+
} else {
53+
delete globalThis.navigator;
54+
}
55+
}
56+
1957
/**
2058
* The real interface: extensions do not appear here, only these six members.
2159
*/
@@ -237,3 +275,51 @@ test.describe('per-credential extension support', () => {
237275
});
238276
});
239277
});
278+
279+
test.describe('writing the secret key to largeBlob', () => {
280+
test.afterEach(() => {
281+
restoreNavigator();
282+
});
283+
284+
test('resolves when the authenticator confirms the write', async () => {
285+
installCredentialsGet({ largeBlob: { written: true } });
286+
287+
await expect(
288+
writeSKToLargeBlob(CREDENTIAL_ID, SECRET_KEY, 'example.com')
289+
).resolves.toBeUndefined();
290+
});
291+
292+
test('throws when the authenticator declines the write', async () => {
293+
// The regression this exists for. Ignoring `written` leaves a keystore
294+
// whose key lives only in memory: usable this session, unopenable after.
295+
installCredentialsGet({ largeBlob: { written: false } });
296+
297+
await expect(
298+
writeSKToLargeBlob(CREDENTIAL_ID, SECRET_KEY, 'example.com')
299+
).rejects.toThrow(/did not store the secret key/i);
300+
});
301+
302+
test('throws when there is no largeBlob result at all', async () => {
303+
installCredentialsGet({});
304+
305+
await expect(
306+
writeSKToLargeBlob(CREDENTIAL_ID, SECRET_KEY, 'example.com')
307+
).rejects.toThrow(/did not store the secret key/i);
308+
});
309+
310+
test('does not accept a truthy non-true value as a write', async () => {
311+
installCredentialsGet({ largeBlob: { written: 'yes' } });
312+
313+
await expect(
314+
writeSKToLargeBlob(CREDENTIAL_ID, SECRET_KEY, 'example.com')
315+
).rejects.toThrow(/did not store the secret key/i);
316+
});
317+
318+
test('surfaces a refused or failed assertion', async () => {
319+
installCredentialsGet(null, { rejectWith: new Error('user cancelled') });
320+
321+
await expect(
322+
writeSKToLargeBlob(CREDENTIAL_ID, SECRET_KEY, 'example.com')
323+
).rejects.toThrow(/Failed to write secret key to largeBlob/i);
324+
});
325+
});

0 commit comments

Comments
 (0)