Skip to content

Reject a non-empty attStmt in the "none" attestation statement format - #538

Open
choksi2212 wants to merge 1 commit into
Yubico:mainfrom
choksi2212:fix/none-attestation-empty-attstmt-526
Open

choksi2212 wants to merge 1 commit into
Yubico:mainfrom
choksi2212:fix/none-attestation-empty-attstmt-526

Conversation

@choksi2212

Copy link
Copy Markdown

The "none" attestation statement format defined in WebAuthn Level 3, Section 8.7 requires the attStmt member to be an empty map:

noneStmtFormat = {
    "fmt": "none",
    "attStmt": { },
    "authData": bytes
}

NoneAttestationStatementVerifier returned true unconditionally, so a registration response with fmt: "none" and a non-empty attStmt map passed Step 19 with the extra properties silently ignored, instead of being rejected as non-conforming. This is the (b) half of #526.

The verifier now throws IllegalArgumentException when the attStmt map has any properties, in line with how the other attestation statement verifiers reject malformed statements, which fails the registration ceremony in FinishRegistrationSteps.Step19. A new NoneAttestationStatementVerifierSpec pins both directions, run with TestWithEachProvider like the other verifier specs: it accepts an empty attStmt and rejects an attStmt with properties.

Before the fix, the new "rejects an attStmt with properties" test fails on every provider; with the fix the spec passes on every provider, the full :webauthn-server-core:test suite is green on JDK 21, and spotlessCheck (including scalafix) is clean.

The (a) half of #526 (rejecting crossOrigin: true unless the Relying Party expects cross-origin use) needs a new RelyingParty setting to express that expectation, so I've left it out of this PR rather than pick the shape of a public API on my own.

Partially addresses #526.

The "none" attestation statement format defined in WebAuthn Level 3,
Section 8.7 requires the attStmt member to be an empty map. The verifier
accepted any map, so a registration response with fmt "none" and a
non-empty attStmt passed validation with the extra properties silently
ignored.

verifyAttestationSignature now throws IllegalArgumentException when the
attStmt map has any properties, in line with how the other attestation
statement verifiers reject malformed statements, which fails the
registration ceremony in FinishRegistrationSteps.Step19.

Partially addresses Yubico#526.

Signed-off-by: choksi2212 <choksi2212@users.noreply.github.com>
@choksi2212

Copy link
Copy Markdown
Author

Quick note on the checks section: the workflow runs on this PR are sitting in the awaiting-approval state (first PR from this fork), so nothing has executed on CI yet. For what it is worth, the branch was verified locally before opening — :webauthn-server-core:test plus a root spotlessCheck (which chains checkScalafix) were green on the final head 2d423d0.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant