fix(passkeys): validate base64url WebAuthn fields at the route boundary#20658
Open
vpomerleau wants to merge 1 commit into
Open
fix(passkeys): validate base64url WebAuthn fields at the route boundary#20658vpomerleau wants to merge 1 commit into
vpomerleau wants to merge 1 commit into
Conversation
Because: Malformed base64url values in passkey route payloads were caught only at the DB/SimpleWebAuthn boundary, throwing Errors that Hapi surfaced as 500s and pollute Sentry — e.g. FXA-AUTH-2T4, an OAST callback probe on POST /v1/passkey/authentication/finish. This commit: - adds local base64url Joi factories (BASE64URL_PATTERN, base64urlString, base64urlCredentialId, base64urlChallenge) in passkeys.ts - applies them to response.id/rawId/challenge and inner-response base64url fields (clientDataJSON, attestationObject, authenticatorData, signature, userHandle, publicKey) on registration/finish and authentication/finish; and to params.credentialId on DELETE/PATCH - tightens the challenge validator on registration/finish to match authentication/finish - adds Joi-direct schema-validation tests asserting on error.details.type Closes #FXA-13808
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
POST /v1/passkey/authentication/finishwith a non-base64urlresponse.id. Validation was deferred tobase64urlToBufferat the DB boundary, which throws anErrorand surfaces as a 500 — polluting Sentry with what should be a 400.registration/finish, DELETE, PATCH); future probes there would reproduce the pattern.This pull request
base64urlString(maxLen),base64urlCredentialId(),base64urlChallenge()) inpackages/fxa-auth-server/lib/routes/passkeys.ts. Credential-ID.max(1364)matches the WebAuthn-L2 ceiling and thepasskeys.credentialId VARBINARY(1023)column.response.id,rawId,challenge, and the innerresponse.response.*base64url fields (clientDataJSON,attestationObject,authenticatorData,signature,userHandle,publicKey) onregistration/finish+authentication/finish, and toparams.credentialIdon DELETE/PATCH.challengeonregistration/finishto matchauthentication/finish.packages/fxa-auth-server/lib/routes/passkeys.spec.tscovering positive, alphabet/length rejection, and missing-required-field cases. Assertions pin toerror.details[].path+error.details[].type(Joi-version-resilient).Issue that this pull request solves
Closes: FXA-13808
Checklist
Put an
xin the boxes that applyHow to review (Optional)
packages/fxa-auth-server/lib/routes/passkeys.ts— factories at top + four route schemas.@simplewebauthn/browseremit spec-strict, and the priorbase64urlToBufferalready enforced strict onresponse.idwith no real-client 500s in Sentry — so empirical risk is very low.Screenshots (Optional)
N/A — server-only change.
Other information (Optional)
response.id/challenge/params.credentialId. During/fxa-review, the same defense-in-depth was extended to the innerresponse.response.*WebAuthn fields, which SimpleWebAuthn decodes internally and could throw to 500 the same way. Folded into this PR rather than a follow-up.lib/routes/validators.jsfor cross-route reuse; rate-limit tuning forpasskeyAuthFinish(separate, lives inwebservices-infra).