Repository navigation
Security hardening batch: trust-chain validation, attestation bypasses, malformed-input DoS - #671
Merged
aseigler merged 32 commits intoSep 18, 2026
Merged
Conversation
An authenticator's metadata may name an intermediate CA, or the attestation certificate itself, as a trust anchor. X509Chain only honours a non-self-signed CustomTrustStore entry as an anchor on Windows; OpenSSL and Apple treat it as just another issuer and report the chain as partial, so such registrations failed on Linux and macOS and three tests were skipped there to keep CI green (passwordless-lib#293). ValidateTrustChain now builds the chain once with partial chains permitted -- the engine still verifies every signature -- and accepts the attestation certificate when a declared anchor appears anywhere above it in the verified path. Anchors are matched by DER bytes, per MDS3. Revocation moved out of the engine too. With an intermediate anchor the chain is partial on Linux, and OpenSSL never consults a CRL for a partial chain (confirmed: zero fetches against a local CRL server), so relying on RevocationMode.Online there would have silently skipped the check the moment the anchor fix made the chain acceptable; macOS never fetches under CustomRootTrust at all. If the attestation certificate names an HTTP(S) CRL distribution point, the CRL is fetched (kept in memory until its next update), decoded by the new RFC 5280 CertificateRevocationList, verified against the issuer the engine established, and the serial looked up. Unfetchable, forged, wrong-CA and stale CRLs all fail the registration, on every platform alike. IsCertInCRL gains a verifying overload on the same decoder; the metadata repositories keep the membership-only one for now. The CRL distribution point is read by a proper ASN.1 walk instead of indexing into the first name, so a malformed or LDAP-only extension no longer throws. The fetch is bounded at 32 MiB through the client's response buffer, so its 15-second timeout covers the whole body. Tests: a generated PKI fixture (root, intermediate, look-alike intermediate with the real one's key identifier, leaves with chosen distribution points, hand-assembled minimal CRLs) and a loopback CRL server cover intermediate anchors, anchor look-alikes, expired leaves, revoked/forged/stale/wrong-issuer CRLs, unreachable and oversized (declared and chunked) distribution points, the per-URL cache and its bound, and the CRL decoder's edge cases. The three OS-guarded tests run everywhere: HID's intermediate anchor (with a live fetch of its IdenTrust CRL), the ATKey attestation, and the root-in-x5c rejection, which had only ever passed on Windows because it fed the loader base64 text instead of DER. Fixes passwordless-lib#293.
Malformed, attacker-controlled input on the parsing paths threw raw runtime exceptions (ArgumentOutOfRangeException, KeyNotFoundException, InvalidCastException, NullReferenceException, CryptographicException) rather than Fido2VerificationException. Relying parties that catch only Fido2VerificationException would surface 500s / stack traces, and an unauthenticated caller had a cheap DoS on both the registration and login paths. - AttestedCredentialData.Parse: bound-check the declared credentialId length against the remaining buffer before slicing. - AuthenticatorAttestationResponse.Parse: wrap FromCbor (which parses attacker-controlled authenticator data) so any failure surfaces as a Fido2VerificationException. - AuthenticatorAssertionResponse.Parse: null-guard the response and wrap AuthenticatorData.Parse. - Apple: type-check every x5c element before the (byte[]) cast, not just the first. - X509CertificateHelper.CreateFromRawData: convert a malformed-certificate CryptographicException into a Fido2VerificationException. Adds MalformedInputHardeningTests covering each path.
- Registration (AuthenticatorAttestationResponse): enforce BackedUpCredentialPolicy, mirroring the assertion path. A credential can already be backed up (BS set) at creation, e.g. a synced passkey, so the policy must apply at registration and not only at assertion. - Assertion (AuthenticatorAssertionResponse): the signature-counter clone check used `authData.SignCount > 0 && authData.SignCount <= stored`, which silently skipped the case where a previously-counting credential (stored counter > 0) presents a zero counter -- exactly the cloned-authenticator signal from WebAuthn L3 7.2 step 21. Use the spec condition: (new != 0 || stored != 0) && new <= stored. Adds registration tests for the required/disallowed backup-state policies and an assertion test for the zero-count-with-nonzero-stored-counter case.
WebAuthn 8.4 step 5 requires the attested key's origin to equal KM_ORIGIN_GENERATED and its purpose to contain KM_PURPOSE_SIGN. The verifier defaulted both to the accepted value and only overrode them when the tag was present, so an attestation that simply omitted origin/purpose passed, and only the first element of the purpose SET was inspected. - IsOriginGenerated: require the origin (702) tag to be present in an authorization list and equal KM_ORIGIN_GENERATED; a missing tag fails. - IsPurposeSign: require the purpose (1) tag to be present and inspect the whole SET for KM_PURPOSE_SIGN rather than just its first element. - Wrap the step-5 checks so a malformed authorization list fails as a Fido2VerificationException rather than a raw ASN.1/index exception. Updates the test fixtures (which relied on the fail-open default and carried empty authorization lists) to include a spec-compliant origin and purpose, and adds tests asserting a missing origin or purpose is rejected.
M1 - Pin to the FIDO Alliance root on every path. X509Chain's default System trust mode makes Build() succeed for a chain to ANY publicly-trusted CA, and the pinned-root thumbprint was only checked in the failure branch. So on a host that trusts the bundled GlobalSign root (the common case) a BLOB signed under an unrelated public CA was accepted. The terminal certificate is now required to be the pinned root whether or not the host trust store accepts the chain. M2 - Verify the pinned root before consulting revocation, and restrict the CRL distribution point to http(s). The CDP is taken from a certificate in the (previously unvalidated) chain, so fetching it was an SSRF vector; the chain is now confirmed to pin to the FIDO root before any CDP is contacted, and non-http(s) CDPs are skipped. M3 - Document that DistributedCacheMetadataService stores the BLOB without its signature and trusts it unre-validated on read, so the distributed cache is part of the trust boundary and must be access-controlled. DeserializeAndValidateBlobAsync takes the trust root as a parameter (internal) so the pinning can be tested with a self-built chain; production still passes the bundled root. Adds tests that a chain pinning to the provided root validates and one that does not is refused before any fetch.
The apple anonymous attestation verifier checked the credCert nonce and that its public key matched the credential, but never verified that the credCert chained to Apple's WebAuthn root. Apple platform authenticators are not in the FIDO Metadata Service, so metadata-based trust-anchor validation never runs for them, and no root was bundled -- a self-signed credCert carrying a matching nonce and public key was accepted as an "apple" (AttestationType.Basic) attestation. - Bundle Apple's published WebAuthn Root CA and validate the credCert chain against it (CustomRootTrust), after the cheaper nonce/public-key checks so existing negative tests are unaffected. - Make the root injectable: Fido2Configuration.AppleWebAuthnRootCertificate overrides it (defaulting to the bundled root), plumbed through AttestationVerifier.Create. - AppleAppAttest reuses the apple verifier for the nonce/public-key checks only; it validates its own chain against the App Attest root (with expired-development-leaf handling), so it now passes verifyChain: false. Tests: a positive apple attestation with an injected test root, a chain-not-anchored-to-the-configured-root rejection, a rewritten public-key-mismatch test, and a guard that the bundled root is the genuine issuer of the real "Apple WebAuthn CA 1" intermediate (catches a transcription error in the embedded certificate).
The android-safetynet verifier checked the JWS signature against the certs in its own header and that the leaf was issued to attest.android.com, but never verified that the chain terminated at a Google root. Google's SafetyNet root is not in the FIDO Metadata Service, so metadata-based trust-anchor validation never runs for it -- a self-signed certificate issued to attest.android.com, signing its own response JWT, was accepted. - Bundle Google Trust Services root R1 and validate the certificate chain against it (CustomRootTrust), after the existing checks so negative tests are unaffected. - Make the root injectable: Fido2Configuration.AndroidSafetyNetRootCertificate overrides it (defaulting to the bundled root), plumbed through AttestationVerifier.Create. Tests inject their self-signed root for the positive cases, plus a guard on the bundled root's SHA-256 thumbprint (catches a transcription error in the embedded certificate). Note: android-safetynet is deprecated (WebAuthn L3 8.5) and Google has wound down the SafetyNet Attestation API. The bundled GTS root R1 is authenticated here as a genuine current Google root (Google's live TLS endpoints chain to it), but this repo has no captured real SafetyNet response to confirm the format still chains specifically to GTS R1; if a deployment sees a different root, override it via configuration.
WebAuthn 8.3 step 2 requires the key described by pubArea to be identical
to the credential public key in authenticatorData. The comparison only ran
for RSA (kty 3) and EC2 (kty 2); any other key type fell through and the
statement went on to pass every remaining check. That matters because
certInfo only certifies pubArea: TPM2_Certify takes qualifyingData from the
caller, so anyone with a certified AIK can have the TPM certify one of its
own keys over SHA-256(authData || clientDataHash) for an authData whose
credential public key is a software Ed25519 key. No TPM 2.0 holds an OKP
key, so such a credential cannot be the one pubArea describes, and the
verifier now rejects it outright rather than skipping the comparison.
- Reject any credential public key that is not RSA or EC2, and require
pubArea's TPMI_ALG_PUBLIC to match the kind of key being compared, so an
RSA key against an ECC pubArea (or vice versa) fails as a mismatch rather
than as a raw ArgumentOutOfRangeException from a null CurveID.
- Read pubArea's RSA exponent big-endian. TPM canonical form is big-endian
throughout, but PubArea used BitConverter.ToUInt32 (host order), and the
COSE exponent was decoded as e[0] + (e[1] << 8) + (e[2] << 16). Both are
little-endian readings that only agree with each other because 65537 is
the palindrome 01 00 01 and Windows writes the exponent as 0 ("default");
an authenticator marshalling 65537 explicitly as 00 01 00 01 was rejected
with an exponent mismatch, and a COSE exponent of any length other than
three octets (e.g. 3) threw IndexOutOfRangeException. The COSE value is
now decoded as the unsigned big-endian integer RFC 8230 defines, of any
length, and must fit the 32-bit field.
- Refresh the TPM manufacturer allowlist from TCG TPM Vendor ID Registry
1.02 (2020) to 1.06 (2024): adds Ant Group, HPI, Huawei (HISI), NSING and
SecEdge, whose TPMs were being refused as "Invalid TPM manufacturer".
- Parse pubArea and certInfo, and the manufacturer id, defensively: a
truncated structure, a curve with no TPM equivalent, or a non-hex
manufacturer string now surface as Fido2VerificationException instead
of ArgumentOutOfRangeException, KeyNotFoundException or FormatException.
The test fixtures wrote the exponent little-endian too (PubAreaHelper and
one hand-assembled pubArea), which is why the suite never noticed; they
now marshal it as a TPM does. Adds tests for the OKP rejection, explicit
and default (zero) big-endian exponents, a little-endian exponent being
refused, COSE exponents of one, four and five octets, RSA/ECC pubArea type
mismatches, an unsupported curve, a v1.05+ manufacturer, a non-hex
manufacturer, and truncated pubArea/certInfo.
…nput A second sweep for attacker-controlled input that escaped verification as a raw runtime exception, past the parsing paths the earlier hardening covered. Each of these is reachable from a registration or assertion request by an unauthenticated caller, so a relying party catching only Fido2VerificationException surfaced a 500 instead of a refusal: - clientDataJSON with no "origin" (or null/empty) threw ArgumentNullException from the URI parser, and one that isn't a URI threw UriFormatException. BaseVerify now refuses both, with the new MissingAuthenticatorResponseOrigin / InvalidAuthenticatorResponseOrigin codes. - fido-u2f with an RSA attestation certificate dereferenced the null from GetECDsaPublicKey() (NullReferenceException). It is now the "not an EC public key over P-256" failure step 2b calls for. The verifier also checks that the credential public key is EC2 before reading its "-2"/"-3" members, and that each coordinate is 32 bytes as steps 4a/4b require, rather than assembling the U2F public key from whatever was there. - android-key with an RSA attestation certificate: the same null dereference; now a Fido2VerificationException. - packed with an attestation certificate whose subject is empty threw ArgumentOutOfRangeException from IsValidPackedAttnCertSubject (an empty subject decodes to one line with no '='). It now fails the C/O/OU/CN requirement like any other bad subject. - An id-fido-gen-ce-aaguid extension that is not an OCTET STRING, or not 16 bytes long, threw from the ASN.1 decoder or the Guid constructor (packed and tpm). Both are refused explicitly. - An attestation statement whose alg does not fit the certificate's key (RS256 with an EC certificate, ES256 with an RSA one, an alg/curve pairing that isn't ECDSA's) threw InvalidOperationException out of CredentialPublicKey. Those, the unknown-kty cases, and an unknown public key algorithm OID in a certificate (which threw a bare Exception from COSE.GetKeyTypeFromOid) are now Fido2VerificationException with the new InvalidCredentialPublicKey code. The one existing test that pinned InvalidOperationException is updated. - android-safetynet with a JWT header that base64url-decodes but isn't JSON threw JsonException; it is now the malformed-JWT failure. Adds tests for each: missing/null/empty/unparseable origins, U2F with an RSA certificate, a non-EC2 credential key and 33-byte coordinates, an RSA android-key certificate, an empty packed subject, a 3-byte and a UTF8String AAGUID extension, alg/certificate mismatches both ways, the CredentialPublicKey kty/alg/curve/OID cases, and a non-JSON SafetyNet header.
…tches GetLatestStatusReport() returned the last element of statusReports, but the Metadata Service defines no order for that array, and the published BLOB mostly lists reports newest first: in BLOB passwordless-lib#280, 42 entries with more than one effective date are newest first and 3 are oldest first. For those 42 the library was reading the oldest report as the current one. One of them is TruU Windows Authenticator (ba86dc56-635f-4141-aef6-00227b1b9af6), whose reports are [REVOKED 2023-12-20, NOT_FIDO_CERTIFIED 2023-11-07]; the REVOKED status was never seen, so the UndesiredAuthenticatorMetadataStatuses check let registrations through. The latest report is now the one with the most recent effectiveDate. Reports with equal or missing dates keep the old last-wins order (the spec's own examples are oldest first, and FileSystemMetadataRepository produces undated reports), and an entry without reports returns null instead of throwing. DistributedCacheMetadataService cached a fetch that produced nothing. Both GetOrCreateAsync factories returned null without setting an expiry, and the memory cache stores null values, so one transient MDS failure on the first lookup stood in for the BLOB until the process restarted, with every registration after it verified against no metadata at all. A failed fetch is no longer cached, so the next lookup retries. An AAGUID that a BLOB was actually searched for and does not contain is still cached as a miss, but only until the earliest searched BLOB expires, so a model added to MDS later is found once the BLOB rolls over. FileSystemMetadataRepository.GetBLOBAsync added into a dictionary it never cleared, so the second call (a cache rollover; the BLOB it returns has no nextUpdate, so DistributedCacheMetadataService refetches on every memory-cache expiry) threw "An item with the same key has already been added", which the service logged as an error every hour. The directory is now read into a fresh dictionary on each call, and two statements sharing an AAGUID are reported as a Fido2MetadataException naming the file rather than an ArgumentException. Adds tests for newest-first, oldest-first, date-time, tied, undated and empty status reports; a registration refused for the TruU-shaped entry; a failed fetch being retried on the next lookup; an unknown AAGUID cached only after a BLOB was searched and only until it expires; and the filesystem repository being fetched twice and refusing duplicates.
id:FFFFF1D0 is the tcg-at-tpmManufacturer the FIDO conformance tools' simulated TPM puts in its AIK certificates. It is not a TCG-registered vendor, but it sat in the static allowlist next to the real ones, so a production registration whose AIK certificate named it was accepted, and since Tpm is internal a Relying Party could not remove it. The ceremony already knows when it is being driven by the tools: it derives FidoValidationMode.FidoConformance2024 from IMetadataService.ConformanceTesting(), which is true only when the ConformanceMetadataRepository is registered, and trust anchor validation already relaxes online revocation on that signal. That mode now travels on VerifyAttestationRequest (with a matching AttestationVerifier.VerifyAsync overload), the ceremony computes it once for both uses, and the TPM verifier accepts id:FFFFF1D0 only under it. The allowlist itself is now exactly the registry's product implementations. The unit-test fixtures named id:FFFFF1D0 only as "some valid manufacturer"; they now name Microsoft's. The two captured TPM responses (attestationTPMSHA1/SHA256Response.json) are conformance-tool captures, and are now verified with a metadata service that says so rather than against the live MDS, which had no entry for them and was only being called to return nothing. Adds tests that the tools' manufacturer is refused by default, both through the ceremony and the verifier, and accepted under conformance validation, again both ways.
…produce WebAuthn 7.1 step 25 has the Relying Party assess the attestation: verify that None attestation is acceptable under its policy, that Self attestation is acceptable under its policy, or that the trust path chains to an acceptable root. The library made every attestation type succeed and told the Relying Party nothing about which it had verified, so a policy like "no self attestation" or "only hardware-attested keys" had no input to act on. Nor could the AAGUID be trusted: self attestation is signed with the credential key itself, so a client can claim any AAGUID with it, and with the model's metadata saying basic_full TrustAnchor.Verify skipped everything (the trust path is null) and the registration was recorded under that AAGUID. - RegisteredPublicKeyCredential.AttestationType carries the type the verification procedure established: "none", "self", "basic", "attca" or "anonca". - TrustAnchor.Verify gains an overload taking the attestation type, and refuses Self attestation when the model's metadata statement does not declare basic_surrogate: a model that only does basic_full (or anonca) never produces it, so the AAGUID is a false claim. None attestation is left alone, since a client that strips the attestation on the Relying Party's behalf legitimately keeps the AAGUID. - AttestationType.AnonCA is added (6.5.3), and the apple verifier returns it as 8.8 step 7 says, instead of Basic. apple-appattest, which reuses that verifier's nonce and key checks, keeps reporting Basic: App Attest is not a WebAuthn attestation and Apple assigns it no WebAuthn type. - Registration checks that credential.rawId equals the credentialId in the attested credential data. The two come from different parts of the response and only the attested one is signed; nothing compared them, and the test harness had been sending rawId f1d0 against a random attested id since 2019 without anything noticing. The harness and the hand-built responses in AuthenticatorResponseTests now send the attested id, as a real client does. Adds tests for self attestation against basic_full-only, basic_surrogate-only and both-declared metadata, the attestation type reported by every format's positive test, apple reporting anonca, and a rawId that is missing or differs from the attested credential id.
# Conflicts: # Src/Fido2/AttestationFormat/AndroidSafetyNet.cs # Src/Fido2/AttestationFormat/Apple.cs # Src/Fido2/AttestationFormat/AppleAppAttest.cs # Tests/Fido2.Tests/Attestation/Apple.cs # Tests/Fido2.Tests/Fido2Tests.cs
…26-09 # Conflicts: # Src/Fido2/Metadata/Fido2MetadataServiceRepository.cs
…estation-checks merge
- AttestationVerifier.Create("tpm") lost its default config argument when
merged; two TPM tests still called it without one.
- Fido2MetadataServiceRepositoryTests' success-path chain test used a leaf
cert with no CRL distribution point, which the newly-merged fail-closed
CRL lookup (from fix/trust-anchor-cross-platform) now rejects instead of
silently skipping. Gave the leaf a real CDP and served an empty CRL for it.
…Not32Bytes The test built its malformed (33-byte-coordinate) public key by round-tripping bad ECParameters through ECDsa.Create, which OpenSSL (Linux) tolerates but Windows' CNG backend rejects with a raw PlatformNotSupportedException / CryptographicException before the test ever reaches the code under test. Corrupt the coordinates on an already-successfully-constructed CredentialPublicKey instead, via a new CborMap.Set(KeyTypeParameter, byte[]) mutator, so no invalid EC point is ever run through ECDsa.Create.
CredentialPublicKey.CreateECDsa handed X/Y straight to ECDsa.Create without checking they matched the curve's coordinate size. Both are attacker-reachable (a credential public key in authenticator data, or an attestation statement's alg paired with its certificate's key), and a malformed length surfaced as a raw, platform-dependent exception: OpenSSL tolerates some malformed lengths CNG (Windows) rejects, so a 33-byte P-256 x-coordinate produced a Fido2VerificationException(MalformedAttestationObject) on Windows -- from the generic attestation-object parse catch, before the format verifier's own length check ever ran -- but sailed through parsing on Linux, where the format-specific check (e.g. fido-u2f's WebAuthn 8.6 step 4a/4b) caught it instead with a different code and message. Length is now checked against the curve up front, so a malformed coordinate is always a Fido2VerificationException(InvalidCredentialPublicKey) with a consistent message, regardless of host OS. Updates the two tests whose expected exception depended on the old, platform-dependent path.
Cherry-picked from conformance-rejection-reasons (1e75bb0), off upstream, not
part of the security fixes below -- needed to run the conformance tool
against this branch's library changes without every correctly-rejected
registration/assertion crashing the request pipeline instead of reporting
{status: failed, errorMessage} the way the tool expects.
…ead of crashing Cherry-picked from conformance-rejection-reasons (d5eb2bd), adapted to this branch: the original commit's context included WebAuthn L3 crossOrigin/ topOrigin support and a NameHashAlgorithm helper that only exist on that branch's separate lineage, not here, so those parts are dropped along with the tests that depended on them. What actually carries over: - android-key: an undecodable attestation public key surfaced as a raw OpenSslCryptographicException instead of a Fido2VerificationException -- this was the one real crash in the last conformance run (traffic.jsonl sequence 339, "digital envelope routines::decode error"). - A missing, empty or unparseable clientDataJSON.origin escaped as ArgumentNullException or UriFormatException from the URI parser. - Every remaining string-only Fido2VerificationException in the attestation formats, TrustAnchor, TokenBindingDto and the response parsers now carries a Fido2ErrorCode (InvalidCertificateChain and MissingAttestationObject are new), so a rejection with Code == Unknown no longer happens on any path the conformance suite exercises in the last run's traffic log. One test message updated to match the shared error message its case now produces (TestAndroidKeyX5cCertNotEc).
Cherry-picked from conformance-rejection-reasons (a850def). The subclass bypassed the coded constructor, so an authenticator refused for its metadata status was the last rejection the conformance suite reported with Code == Unknown (traffic.jsonl sequence 351/353 across the last two runs).
Cherry-picked from conformance-rejection-reasons (56de33b). The conformance
controller was ignoring conformance:metadata entirely and always reading
{MDSCacheDirPath}/Conformance as a directory of already-unpacked JSON files
-- which was empty, so every attestation test failed with AaGuidNotFound
(explains the run reporting ~all P- test failures: the metadata zip being
pointed at via conformance:metadata was never actually being read).
conformance:metadata now accepts either the zip the tool exports directly or
a directory it was unpacked into, and the demo logs at startup how many
statements it loaded (or that it found none, with the same explanation).
FileSystemMetadataRepository also now reads *.json recursively, since the
tool's zip unpacks to metadataStatements/*.json.
… ulong AuthenticatorGetInfo.VendorPrototypeConfigCommands was int[]; the FIDO conformance tool's test metadata sends values up to 2^64 to exercise this field (one of its 25 exported statements has [18446744073709552000, 184467440737095520]), which overflows int, long and even ulong. That one statement's JsonException took down the entire directory load, so every attestation test failed with AaGuidNotFound regardless of which authenticator it used -- not just the one exercising this field. Widened to double[] (subcommand identifiers are otherwise always small, and round-trip through double exactly). Also hardened FileSystemMetadataRepository.GetBLOBAsync so one statement file with an unexpected shape is skipped rather than failing every other statement in the directory, the same way the rest of this branch already treats malformed input from an outside source as a rejection of that input rather than a crash of the whole operation.
Cherry-picked from a much earlier commit (638a6d5), adapted: dropped an AuthenticatorAttachment property this branch doesn't have (unrelated context from that commit's original lineage), and kept both the existing RawId-missing check and the new Id-decodes-to-RawId check together in AuthenticatorAttestationResponse rather than replacing one with the other. Fixes two FIDO2 Server conformance cases (F-3, F-4) that were still failing after everything else in this branch, for both attestation and assertion: - F-4, missing "type": PublicKeyCredentialType.PublicKey is enum member zero, and Type was a non-nullable PublicKeyCredentialType, so a JSON payload that omits "type" entirely left it at its default -- which silently is PublicKey. Making Type nullable makes "absent" distinguishable from "explicitly public-key" again. - F-3, "id" not base64url: credential.id is defined as base64url(rawId), but this library only ever decoded RawId; Id was checked for null/empty and never read again, so a garbled Id never surfaced. Both Verify methods now decode Id and require it to match RawId's bytes exactly. One existing test (TestAuthenticatorAttestationResponseRawIdNotAttestedCredentialId) updated: it relied on Id encoding a different, correct credential ID while RawId was wrong, which now trips the new id-must-match-rawId check before ever reaching the check the test targets.
This was referenced Sep 29, 2026
This was referenced Oct 6, 2026
This was referenced Oct 14, 2026
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
1 participant
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.
Security hardening batch: trust-chain validation, attestation bypasses, malformed-input DoS
Fourteen independent fixes across attestation verification, trust-chain validation, and malformed-input handling, bundled into one PR rather than trickled out separately. Validated end-to-end against the FIDO2 conformance test tool: after these fixes, the only remaining failure is a confirmed bug in the tool itself (Resp-9 P-4, TPM ES256, tracked at fido-alliance/conformance-test-tools-resources#862), not this library.
Fixes #293.
Attestation-bypass / trust fixes
TPM2_Certifytakes caller-chosenqualifyingData, so skipping the check let a bogus attestation through for that key type); the pubArea public exponent was read little-endian instead of big-endian, so most non-default exponents parsed wrong; the TCG vendor list was years out of date. The conformance tools' own TPM manufacturer ID is no longer trusted outside an explicit conformance-testing mode.X509Chainonly honors a non-self-signedCustomTrustStoreentry as a trust anchor on Windows; on Linux/macOS the same metadata-declared intermediate-CA anchor was rejected, and revocation checking against it was silently skipped everywhere except Windows. Chain building and CRL checking are now done consistently on every platform, including a from-scratch CRL decoder since OpenSSL never consults a CRL for a partial chain (confirmed: zero fetches against a local CRL server).rawIdis checked against the attested credential ID; Apple's attestation is now reported asAnonCArather thanBasic, matching what it actually is.Malformed-input hardening (unauthenticated DoS)
ArgumentOutOfRangeException,KeyNotFoundException,NullReferenceException, ...) on malformed input instead of aFido2VerificationException, so a caller catching only the library's own exception type would not handle a crafted registration/assertion cleanly. All now surface asFido2VerificationException(or a more specific coded exception).Spec-conformance fix
authData.signCount is nonzero || credentialRecord.signCount is nonzero(WebAuthn L3 §7.2 step 22) was implemented as&&, so a previously-counting credential that suddenly presentedsignCount = 0bypassed clone-authenticator detection entirely. Also enforcesBackedUpCredentialPolicyat registration, not just assertion.Found via conformance testing, fixed the same way
Running the FIDO2 conformance tool against this branch surfaced a few more gaps, some in the library and some in the
Demoproject's conformance-test harness:Demoonly): the conformance endpoints had no exception handling, so a correctly-rejected registration/assertion crashed the request pipeline instead of returning the JSON response the tool expects. Added a middleware that reports rejections cleanly and logs the exception type/code/message per request, which is what made classifying every rejection in this list possible in the first place. This resolves one of the most frustrating aspects of conformance, ensuring that the fail tests are failing for the right reason and not just failing.UndesiredMetadataStatusFido2VerificationException: bypassed the coded exception constructor, so an authenticator rejected for its metadata status was the one rejection the library couldn't report a specific error code for.Demoonly):conformance:metadatacan now point directly at the zip the tool exports, in addition to a directory it was unpacked into (which already worked and still does).int[]field they were parsed into and crashed the entire metadata load (not just that one statement). Widened the field and made metadata-directory loading resilient to one malformed statement file.type/ forgedidin ceremony responses:Typewas a non-nullable enum wherePublicKeyCredentialType.PublicKeyis member zero, so a payload omittingtypeentirely silently defaulted to valid;idis defined asbase64url(rawId)but was never actually checked against it. Both now produce a proper rejection instead of silently passing.Testing
Tests/Fido2.Tests).Fido2.AspNet.Tests(10),Fido2.Ctap2.Tests(102),Fido2.Models.Tests(17) all green.