Skip to content

General bugfixes - #661

Merged
aseigler merged 5 commits into
passwordless-lib:mainfrom
aseigler:general-bugfixes
Sep 11, 2026
Merged

aseigler merged 5 commits into
passwordless-lib:mainfrom
aseigler:general-bugfixes

Conversation

@aseigler

Copy link
Copy Markdown
Collaborator
  • Adds a couple MDS summaries that are missing and fixes a minor spelling error
  • Implements check for user.Id length (must be 1 - 64 bytes)
  • Report Ed448 as unimplemented rather than unknown
  • Skip the hash-algorithm detail TPM ECC/RSA schemes carry (fixes new conformance test fail)

…liance

- Change distributed cache interval from 8 days to 25 hours
- Update GetDistributedCacheAbsoluteExpiryTime to use buffer period semantics
- This allows cross-server caching efficiency within MDS v3.1.1 rate limit window
- Memory cache still expires at nextUpdateTime for rate limiting enforcement
WebAuthn requires a user handle of at least 1 and at most 64 bytes. The upper
bound was already declared on PublicKeyCredentialUserEntity; the lower bound was
not, and neither was checked when building creation options, so a zero-length
handle reached the authenticator and failed there instead.
An Ed448 credential public key fell into the "Missing or unknown crv" branch and
surfaced as an InvalidOperationException. The curve is well known, the gap is
that NSec.Cryptography has no Ed448 support, so say that instead. WebAuthn L3
ships a test vector for this key type (§16.12).
Conformance test P-4 (a TPM attestation for ES256) failed the whole ceremony
with a NullReferenceException in PubArea's constructor, reading a null
ECPoint.X/Y.

TPMU_ASYM_SCHEME (reached via TPMT_ECC_SCHEME/TPMT_RSA_SCHEME) is a union:
its 2-byte algorithm selector is the entire field only when that selector is
TPM_ALG_NULL. For every other scheme this library can see here -- ECDSA,
RSASSA, RSAPSS, OAEP, ECDH, ECSCHNORR, SM2 -- the selector is followed by a
TPMI_ALG_HASH naming the signature hash algorithm (ECDAA adds a further
count). PubArea read the selector as if it were the whole field and moved on,
so a pubArea naming an explicit hash algorithm shifted every field read after
it -- CurveID, KDF, and the EC point -- two bytes out of place. fido2-net-lib's
own TPM test fixtures never caught this because they all hardcode
TPM_ALG_NULL for Scheme, the one selector value with no trailing detail; the
FIDO conformance tool's TPM simulator sends TPM_ALG_ECDSA with an explicit
TPM_ALG_SHA256 for ES256.

PubArea now consumes and discards that detail once it reads the selector, so
offsets stay aligned regardless of which scheme is named. A new test pins
this down directly: constructing a pubArea with an ECDSA/SHA256 scheme
reproduces the reported crash against the pre-fix code and passes against
the fix.
@codecov

codecov Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.12%. Comparing base (b141b59) to head (d644a84).

Files with missing lines Patch % Lines
Src/Fido2/AttestationFormat/Tpm.cs 60.00% 1 Missing and 1 partial ⚠️
...rc/Fido2.AspNet/DistributedCacheMetadataService.cs 66.66% 0 Missing and 1 partial ⚠️
Src/Fido2/Objects/CredentialPublicKey.cs 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #661      +/-   ##
==========================================
+ Coverage   78.64%   80.12%   +1.47%     
==========================================
  Files          98       98              
  Lines        2548     2556       +8     
  Branches      425      428       +3     
==========================================
+ Hits         2004     2048      +44     
+ Misses        434      398      -36     
  Partials      110      110              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Neither had any test exercising them.

No test anywhere in the suite calls Fido2.RequestNewCredential() --
every other registration test builds CredentialCreateOptions directly
via object initializer, bypassing the options-generation path entirely.
That left the 1-64 byte user handle bound this branch adds (and the
whole CredentialCreateOptions.Create factory around it) completely
unverified. RequestNewCredentialTests covers the empty, null, oversized,
and both boundary (1 and 64 byte) cases, plus a normal case asserting
the returned options actually carry the challenge/user/rp.

CredentialPublicKeyTests gains a case constructing an OKP/EdDSA/Ed448
key and asserting it's refused with Fido2VerificationException +
UnimplementedAlgorithm rather than the InvalidOperationException it
used to crash with.
@aseigler
aseigler merged commit 5cea375 into passwordless-lib:main Sep 11, 2026
8 of 9 checks passed
aseigler added a commit to aseigler/fido2-net-lib that referenced this pull request Sep 12, 2026
Resolves conflicts arising from Fido2.Ctap2 and MDS work that had been
developed in parallel on this branch and squash-merged separately
upstream. Kept this branch's versions throughout, since they are a
strict superset (BOM/CRLF-only diffs for shared CTAP2 files; MDS
anchor fixes verified against the spec HTML; the metadata repository
and CredentialPublicKey rewrites carry everything upstream had plus
x5u support, rollback protection, and the CommandIdentifierArray
converter). Updated Ed448IsRefusedAsInvalidRatherThanCrashing (renamed
from ...Unimplemented) to match this branch's stricter, spec-correct
handling of alg=EdDSA with crv=Ed448.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant