Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 31 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,37 @@

### Fixed

- The WebAuthn user handle is 64 random bytes instead of the UTF-8 of whatever
the caller passes as `userId`. An authenticator keeps one discoverable
credential per `(rp.id, user.id)` and **replaces** the previous one when both
match — silently, with no prompt and nothing to undo. Deriving the handle from
a typed name therefore meant two people registering as "anna" on a shared
device destroyed each other's passkey, and with it the DID and every entry
signed under it. The same happened to one person re-entering their usual name
after clearing storage. WebAuthn L2 §5.4.3 also forbids putting personally
identifying information in the handle, which a name or e-mail plainly is, and
the handle is stored in the authenticator indefinitely.

`userId` keeps its rightful place as `user.name`, the label the credential
picker shows. It is a label only: it identifies nothing, two credentials may
carry the same one, and nothing in this package looks a credential up by it.
Closes #45.

Nothing needs migrating. Recovery goes through discoverable credentials
(`readLargeBlobMetadata` calls `get()` with no `allowCredentials`), or through
an explicit credential ID where those are switched off — no path resolves a
credential by handle, so credentials registered under the old scheme keep
working untouched. The derived `did:key` comes from the credential's public
key and does not change.

Consumers should expect a behavioural difference: re-registering under a name
that was used before now **adds** a passkey instead of replacing one. That is
the point — a replaced passkey is data loss, a second entry is a choice — but
it means the picker can show several, so `user.name` and `displayName` should
be distinguishing enough to pick from. The new random handle is returned as
`credential.userHandle` (base64url); the authenticator keeps its own copy, so
storing it is optional.

- Actually write the secret key into `largeBlob`. The keystore record carried
`secretKey: sk, // Will be moved to largeBlob` — and nothing ever moved it.
That was the only occurrence of the field in `src/`, and
Expand Down
1 change: 1 addition & 0 deletions playwright.node.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default defineConfig({
'webauthn-extension-support.test.js',
'webauthn-varsig-challenge.test.js',
'webauthn-attestation-parsing.test.js',
'webauthn-user-handle.test.js',
'webauthn-two-peer-replication.test.js',
],
fullyParallel: false,
Expand Down
29 changes: 26 additions & 3 deletions src/webauthn/provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ export class WebAuthnDIDProvider {
* Create a WebAuthn credential for OrbitDB identity
* This triggers biometric authentication (Face ID, Touch ID, Windows Hello, etc.)
* @param {Object} options - Credential options
* @param {string} options.userId - User ID
* @param {string} options.userId - Account label shown in the credential
* picker (`user.name`). A label only: it does not identify the credential,
* two credentials may carry the same one, and it is never used to look one
* up. The handle the authenticator files the credential under is generated
* here and returned as `userHandle`.
* @param {string} options.displayName - Display name
* @param {string} options.domain - Domain/RP ID
* @param {boolean} options.encryptKeystore - Enable keystore encryption
Expand Down Expand Up @@ -117,7 +121,19 @@ export class WebAuthnDIDProvider {

// Generate challenge for credential creation
const challenge = crypto.getRandomValues(new Uint8Array(32));
const userIdBytes = new TextEncoder().encode(userId);

// The user handle is an opaque key, not a label. An authenticator stores
// one discoverable credential per (rp.id, user.id) pair and *replaces* the
// previous one when both match — silently, with no prompt and nothing to
// undo. Deriving the handle from a typed name therefore meant two people
// registering as "anna" on a shared device destroyed each other's passkey,
// and with it the DID and everything written under it (#45).
//
// 64 random bytes, as WebAuthn L2 §5.4.3 recommends. The same section
// forbids putting personally identifying information here, which a typed
// name or e-mail address plainly is. The name keeps its rightful place in
// `user.name` below, where the credential picker shows it.
const userHandle = crypto.getRandomValues(new Uint8Array(64));

webauthnLog('Calling navigator.credentials.create() for user: %s', userId);

Expand All @@ -130,7 +146,7 @@ export class WebAuthnDIDProvider {
id: domain,
},
user: {
id: userIdBytes,
id: userHandle,
name: userId,
displayName,
},
Expand Down Expand Up @@ -247,6 +263,13 @@ export class WebAuthnDIDProvider {
publicKey,
userId,
displayName,
// Nothing in this package looks a credential up by handle — recovery
// goes through discoverable credentials, or through an explicit
// credential ID when those are switched off. It is surfaced anyway
// because it is the only copy the caller will ever see, and a flow
// that one day passes `allowCredentials` needs it. Decode with
// `WebAuthnDIDProvider.base64urlToArrayBuffer()`.
userHandle: WebAuthnDIDProvider.arrayBufferToBase64url(userHandle),
attestationObject: new Uint8Array(
credential.response.attestationObject
),
Expand Down
56 changes: 47 additions & 9 deletions tests/helpers/mock-authenticator.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,23 @@ export async function createMockAuthenticator({
await crypto.subtle.exportKey('spki', keypair.publicKey)
);

const credentialId = crypto.getRandomValues(
// One resident credential per (rpId, user.id), keyed by the handle — the rule
// that makes a name-derived handle destructive (#45). A registration whose
// handle is already present replaces what was there; a new handle adds to it.
const residentByHandle = new Map();
let activeCredentialId = crypto.getRandomValues(
new Uint8Array(credentialIdLength)
);
const rpIdHash = await sha256(new TextEncoder().encode(rpId));

const state = { signCount: 0, assertions: 0, creations: 0, prfEvals: 0 };
const state = {
signCount: 0,
assertions: 0,
creations: 0,
prfEvals: 0,
/** Every `user` dictionary passed to create(), in order. */
createdUsers: [],
};

// Per-credential secret the PRF output is derived from.
const prfKey = prfSecret ?? crypto.getRandomValues(new Uint8Array(32));
Expand All @@ -107,7 +118,10 @@ export async function createMockAuthenticator({
)
);

const buildAuthData = ({ includeAttestedCredential }) => {
const buildAuthData = ({
includeAttestedCredential,
credentialId = activeCredentialId,
}) => {
const flagByte = includeAttestedCredential
? 0x01 | 0x04 | 0x40 // UP | UV | AT
: 0x01 | 0x04; // UP | UV
Expand Down Expand Up @@ -152,8 +166,23 @@ export async function createMockAuthenticator({
state.creations += 1;
state.signCount += 1;

const user = options?.publicKey?.user ?? {};
state.createdUsers.push(user);

const handleKey = bytesToBase64url(new Uint8Array(user.id ?? []));
const credentialId = crypto.getRandomValues(
new Uint8Array(credentialIdLength)
);
// set(), not a guard: a repeat handle silently drops the credential
// that was stored under it, exactly as an authenticator does.
residentByHandle.set(handleKey, credentialId);
activeCredentialId = credentialId;

const challenge = options?.publicKey?.challenge ?? new Uint8Array(32);
const authData = buildAuthData({ includeAttestedCredential: true });
const authData = buildAuthData({
includeAttestedCredential: true,
credentialId,
});
const attestationObject = new Uint8Array(
encode(
new Map([
Expand Down Expand Up @@ -222,10 +251,10 @@ export async function createMockAuthenticator({
);

return {
id: bytesToBase64url(credentialId),
rawId: credentialId.buffer.slice(
credentialId.byteOffset,
credentialId.byteOffset + credentialId.byteLength
id: bytesToBase64url(activeCredentialId),
rawId: activeCredentialId.buffer.slice(
activeCredentialId.byteOffset,
activeCredentialId.byteOffset + activeCredentialId.byteLength
),
type: 'public-key',
response: {
Expand All @@ -246,7 +275,16 @@ export async function createMockAuthenticator({
return {
navigator: navigatorShim,
publicKey: { algorithm: -7, x, y, keyType: 2, curve: 1 },
credentialId,
/** The credential an assertion would use: the most recently created one. */
get credentialId() {
return activeCredentialId;
},
/** What the authenticator still holds, one entry per distinct handle. */
residentCredentials: () =>
[...residentByHandle.entries()].map(([handle, id]) => ({
handle,
credentialId: bytesToBase64url(id),
})),
state,
};
}
Expand Down
124 changes: 124 additions & 0 deletions tests/webauthn-user-handle.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
/**
* The WebAuthn user handle must not be derived from anything a person types.
*
* An authenticator keeps one discoverable credential per (rp.id, user.id) and
* replaces the previous one when both match — no prompt, no undo. While the
* handle was `TextEncoder().encode(userId)`, two people registering under the
* same name on one device destroyed each other's passkey, and with it the DID
* and every entry signed under it (#45).
*
* The first test is the load-bearing one: it asserts what the library *sends*,
* so it holds regardless of how faithfully the mock models an authenticator.
* The rest describe the consequence.
*/
import { test, expect } from '@playwright/test';
import {
createMockAuthenticator,
installMockAuthenticator,
} from './helpers/mock-authenticator.js';
import { silenceWebAuthnDebugLogging } from './helpers/two-peer.js';

const NAME = 'anna';

test.describe('WebAuthn user handle', () => {
let restoreAuthenticator;
let restoreLogging;
let authenticator;
let WebAuthnDIDProvider;

test.beforeEach(async () => {
restoreLogging = silenceWebAuthnDebugLogging();
authenticator = await createMockAuthenticator();
restoreAuthenticator = installMockAuthenticator(authenticator);
({ WebAuthnDIDProvider } = await import('../src/webauthn/provider.js'));
});

test.afterEach(() => {
restoreAuthenticator?.();
restoreLogging?.();
});

test('is 64 random bytes, and carries nothing the user typed', async () => {
await WebAuthnDIDProvider.createCredential({
userId: NAME,
displayName: 'Anna',
});
await WebAuthnDIDProvider.createCredential({
userId: NAME,
displayName: 'Anna',
});

const [first, second] = authenticator.state.createdUsers;
const firstHandle = new Uint8Array(first.id);
const secondHandle = new Uint8Array(second.id);

expect(firstHandle.length).toBe(64);
expect(secondHandle.length).toBe(64);

// Same input, different handle — the property the whole fix rests on.
expect(Buffer.from(firstHandle).equals(Buffer.from(secondHandle))).toBe(
false
);

// And nothing recognisable in it: not the typed bytes, not the name as text.
const typed = new TextEncoder().encode(NAME);
expect(
Buffer.from(firstHandle.subarray(0, typed.length)).equals(
Buffer.from(typed)
)
).toBe(false);
expect(new TextDecoder().decode(firstHandle)).not.toContain(NAME);
});

test('keeps the typed value as the label the picker shows', async () => {
await WebAuthnDIDProvider.createCredential({
userId: NAME,
displayName: 'Anna at the front desk',
});

const [user] = authenticator.state.createdUsers;
expect(user.name).toBe(NAME);
expect(user.displayName).toBe('Anna at the front desk');
});

test('two people with the same name no longer overwrite each other', async () => {
const first = await WebAuthnDIDProvider.createCredential({
userId: NAME,
displayName: 'Anna',
});
const second = await WebAuthnDIDProvider.createCredential({
userId: NAME,
displayName: 'Anna',
});

expect(second.credentialId).not.toBe(first.credentialId);

// Both survive. Under a name-derived handle the second registration would
// have landed in the first one's slot and left a single credential here.
const resident = authenticator.residentCredentials();
expect(resident.length).toBe(2);
expect(resident.map((entry) => entry.credentialId).sort()).toEqual(
[first.credentialId, second.credentialId].sort()
);
});

test('returns the handle, base64url of 64 bytes', async () => {
const credential = await WebAuthnDIDProvider.createCredential({
userId: NAME,
displayName: 'Anna',
});

expect(credential.userHandle).toBeTruthy();
expect(credential.userHandle).toMatch(/^[A-Za-z0-9_-]+$/);

const decoded = new Uint8Array(
WebAuthnDIDProvider.base64urlToArrayBuffer(credential.userHandle)
);
expect(decoded.length).toBe(64);
expect(
Buffer.from(decoded).equals(
Buffer.from(new Uint8Array(authenticator.state.createdUsers[0].id))
)
).toBe(true);
});
});
3 changes: 3 additions & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export interface WebAuthnCredentialInfo {
publicKey: WebAuthnPublicKey | Uint8Array;
userId?: string;
displayName?: string;
/** Random 64-byte WebAuthn user handle, base64url. Not derived from userId. */
userHandle?: string;
attestationObject?: Uint8Array;
prfInput?: Uint8Array;
did?: string;
Expand All @@ -99,6 +101,7 @@ export interface WebAuthnCredentialInfo {
}

export interface CreateCredentialOptions {
/** Account label for the credential picker. Not an identifier. */
userId?: string;
displayName?: string;
domain?: string;
Expand Down
Loading