Skip to content

fix(passkeys): validate base64url WebAuthn fields at the route boundary #20658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vpomerleau wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(passkeys): validate base64url WebAuthn fields at the route boundary #20658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
292 changes: 292 additions & 0 deletions packages/fxa-auth-server/lib/routes/passkeys.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import type { Schema } from 'joi';
import { Container } from 'typedi';
import { PasskeyService } from '@fxa/accounts/passkey';
import { AppError } from '@fxa/accounts/errors';
Expand Down Expand Up @@ -1327,4 +1328,295 @@ describe('passkeys routes', () => {
).rejects.toThrow('DB unavailable');
});
});

describe('credentialId payload validation', () => {
const VALID_CRED_ID = 'A_z-09Aa';
const VALID_CHALLENGE = 'A_z-09';
const VALID_AUTH_INNER = {
clientDataJSON: 'eyJ0eXBlIjoid2ViYXV0aG4uZ2V0In0',
authenticatorData: 'SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAAAQ',
signature: 'MEUCIQCx',
};
const VALID_REG_INNER = {
clientDataJSON: 'eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIn0',
attestationObject: 'o2NmbXRkbm9uZQ',
};

function getSchema(
path: string,
method: string,
kind: 'payload' | 'params'
): Schema {
const all = passkeyRoutes(customs, db, config, statsd, glean, log);
const route = all.find(
(r: any) => r.path === path && r.method === method
);
if (!route) {
throw new Error(`Route not found: ${method} ${path}`);
}
return route.options.validate[kind];
}

const authPayload = (
responseOverride: Record<string, unknown> = {},
innerOverride: Record<string, unknown> = {},
challenge: string = VALID_CHALLENGE
) => ({
response: {
id: VALID_CRED_ID,
type: 'public-key',
response: { ...VALID_AUTH_INNER, ...innerOverride },
...responseOverride,
},
challenge,
});

const regPayload = (
responseOverride: Record<string, unknown> = {},
innerOverride: Record<string, unknown> = {},
challenge: string = VALID_CHALLENGE
) => ({
response: {
id: VALID_CRED_ID,
type: 'public-key',
response: { ...VALID_REG_INNER, ...innerOverride },
...responseOverride,
},
challenge,
});

describe('POST /passkey/authentication/finish', () => {
let schema: Schema;
beforeEach(() => {
schema = getSchema('/passkey/authentication/finish', 'POST', 'payload');
});

it('accepts a well-formed assertion payload', () => {
const { error } = schema.validate(authPayload());
expect(error).toBeUndefined();
});

it.each([
[
'shell-injection probe shape',
'(nslookup x.example.com||curl x.example.com)',
'string.pattern.base',
],
['contains slash', 'A/B', 'string.pattern.base'],
['contains plus', 'A+B', 'string.pattern.base'],
['contains equals padding', 'AA==', 'string.pattern.base'],
['empty string', '', 'string.empty'],
])('rejects response.id (%s)', (_label, badId, expectedType) => {
const { error } = schema.validate(authPayload({ id: badId }));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'id'],
type: expectedType,
}),
]);
});

it('rejects response.id that exceeds the max length', () => {
const { error } = schema.validate(
authPayload({ id: 'A'.repeat(1365) })
);
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'id'],
type: 'string.max',
}),
]);
});

it('rejects a challenge longer than 64 chars', () => {
const { error } = schema.validate(authPayload({}, {}, 'A'.repeat(65)));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['challenge'],
type: 'string.max',
}),
]);
});

it('rejects a non-base64url challenge', () => {
const { error } = schema.validate(authPayload({}, {}, 'has/slash'));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['challenge'],
type: 'string.pattern.base',
}),
]);
});

it.each<[string, Record<string, string>]>([
['clientDataJSON', { clientDataJSON: 'has/slash' }],
['authenticatorData', { authenticatorData: 'has/slash' }],
['signature', { signature: 'has/slash' }],
['userHandle', { userHandle: 'has/slash' }],
])(
'rejects a non-base64url response.response.%s',
(field: string, innerOverride: Record<string, string>) => {
const { error } = schema.validate(authPayload({}, innerOverride));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'response', field],
type: 'string.pattern.base',
}),
]);
}
);

it.each<[string]>([
['clientDataJSON'],
['authenticatorData'],
['signature'],
])(
'rejects when required response.response.%s is missing',
(field: string) => {
const inner: Record<string, string> = { ...VALID_AUTH_INNER };
delete inner[field];
const { error } = schema.validate({
response: {
id: VALID_CRED_ID,
type: 'public-key',
response: inner,
},
challenge: VALID_CHALLENGE,
});
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'response', field],
type: 'any.required',
}),
]);
}
);
});

describe('POST /passkey/registration/finish', () => {
let schema: Schema;
beforeEach(() => {
schema = getSchema('/passkey/registration/finish', 'POST', 'payload');
});

it('accepts a well-formed attestation payload', () => {
const { error } = schema.validate(regPayload());
expect(error).toBeUndefined();
});

it('rejects a non-base64url response.id', () => {
const { error } = schema.validate(regPayload({ id: 'has/slash' }));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'id'],
type: 'string.pattern.base',
}),
]);
});

it('rejects a challenge longer than 64 chars', () => {
const { error } = schema.validate(regPayload({}, {}, 'A'.repeat(65)));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['challenge'],
type: 'string.max',
}),
]);
});

it('rejects a non-base64url challenge', () => {
const { error } = schema.validate(regPayload({}, {}, 'has/slash'));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['challenge'],
type: 'string.pattern.base',
}),
]);
});

it.each<[string, Record<string, string>]>([
['clientDataJSON', { clientDataJSON: 'has/slash' }],
['attestationObject', { attestationObject: 'has/slash' }],
['authenticatorData', { authenticatorData: 'has/slash' }],
['publicKey', { publicKey: 'has/slash' }],
])(
'rejects a non-base64url response.response.%s',
(field: string, innerOverride: Record<string, string>) => {
const { error } = schema.validate(regPayload({}, innerOverride));
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'response', field],
type: 'string.pattern.base',
}),
]);
}
);

it.each<[string]>([['clientDataJSON'], ['attestationObject']])(
'rejects when required response.response.%s is missing',
(field: string) => {
const inner: Record<string, string> = { ...VALID_REG_INNER };
delete inner[field];
const { error } = schema.validate({
response: {
id: VALID_CRED_ID,
type: 'public-key',
response: inner,
},
challenge: VALID_CHALLENGE,
});
expect(error?.details).toEqual([
expect.objectContaining({
path: ['response', 'response', field],
type: 'any.required',
}),
]);
}
);
});

describe('DELETE /passkey/{credentialId}', () => {
let schema: Schema;
beforeEach(() => {
schema = getSchema('/passkey/{credentialId}', 'DELETE', 'params');
});

it('accepts a base64url credentialId', () => {
const { error } = schema.validate({ credentialId: VALID_CRED_ID });
expect(error).toBeUndefined();
});

it('rejects a non-base64url credentialId', () => {
const { error } = schema.validate({ credentialId: 'has/slash' });
expect(error?.details).toEqual([
expect.objectContaining({
path: ['credentialId'],
type: 'string.pattern.base',
}),
]);
});
});

describe('PATCH /passkey/{credentialId}', () => {
let schema: Schema;
beforeEach(() => {
schema = getSchema('/passkey/{credentialId}', 'PATCH', 'params');
});

it('accepts a base64url credentialId', () => {
const { error } = schema.validate({ credentialId: VALID_CRED_ID });
expect(error).toBeUndefined();
});

it('rejects a non-base64url credentialId', () => {
const { error } = schema.validate({ credentialId: 'has/slash' });
expect(error?.details).toEqual([
expect.objectContaining({
path: ['credentialId'],
type: 'string.pattern.base',
}),
]);
});
});
});
});
53 changes: 43 additions & 10 deletions packages/fxa-auth-server/lib/routes/passkeys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ import { FxaMailer } from '../senders/fxa-mailer';
import { FxaMailerFormat } from '../senders/fxa-mailer-format';
import { reportSentryError } from '../sentry';

// Without these, malformed base64url values throw inside SimpleWebAuthn /
// `base64urlToBuffer` and Hapi surfaces a 500. Route-boundary validation
// turns probes into 400s and keeps them out of Sentry. CredentialId max
// matches the WebAuthn L2 ceiling (1023 bytes → 1364 base64url chars),
// aligning with `passkeys.credentialId VARBINARY(1023)`.
const BASE64URL_PATTERN = /^[A-Za-z0-9_-]+$/;
const base64urlString = (maxLen: number) =>
isA.string().max(maxLen).regex(BASE64URL_PATTERN);
const base64urlCredentialId = () => base64urlString(1364);
const base64urlChallenge = () => base64urlString(64);

/** Subset of the Customs service used by passkey routes. */
interface Customs {
/**
Expand Down Expand Up @@ -661,8 +672,24 @@ export const passkeyRoutes = (
},
validate: {
payload: isA.object({
response: isA.object().required(),
challenge: isA.string().required(),
response: isA
.object({
id: base64urlCredentialId().required(),
rawId: base64urlCredentialId().optional(),
type: isA.string().valid('public-key').required(),
response: isA
.object({
clientDataJSON: base64urlString(2048).required(),
attestationObject: base64urlString(32768).required(),
authenticatorData: base64urlString(16384).optional(),
publicKey: base64urlString(1024).optional(),
})
.unknown(true)
.required(),
})
.unknown(true)
.required(),
challenge: base64urlChallenge().required(),
}),
},
response: {
Expand Down Expand Up @@ -729,7 +756,7 @@ export const passkeyRoutes = (
},
validate: {
params: isA.object({
credentialId: isA.string().required(),
credentialId: base64urlCredentialId().required(),
}),
},
response: {
Expand Down Expand Up @@ -784,16 +811,22 @@ export const passkeyRoutes = (
payload: isA.object({
response: isA
.object({
id: isA.string().required(),
id: base64urlCredentialId().required(),
rawId: base64urlCredentialId().optional(),
type: isA.string().valid('public-key').required(),
response: isA
.object({
clientDataJSON: base64urlString(2048).required(),
authenticatorData: base64urlString(16384).required(),
signature: base64urlString(1024).required(),
userHandle: base64urlString(128).optional(),
})
.unknown(true)
.required(),
})
.unknown(true)
.required(),
challenge: isA
.string()
.max(64)
.regex(/^[A-Za-z0-9_-]+$/)
.required(),
challenge: base64urlChallenge().required(),
service: validators.service.optional(),
}),
},
Expand Down Expand Up @@ -825,7 +858,7 @@ export const passkeyRoutes = (
},
validate: {
params: isA.object({
credentialId: isA.string().required(),
credentialId: base64urlCredentialId().required(),
}),
payload: isA.object({
name: isA.string().min(1).max(255).required(),
Expand Down
Loading