Skip to content

fix: verify certificate chains with the declared signature algorithm (fixes App Attest on Deno) - #64

Open
spendres wants to merge 1 commit into
uebelack:mainfrom
spendres:fix/cross-curve-ecdsa-chain-verify
Open

fix: verify certificate chains with the declared signature algorithm (fixes App Attest on Deno)#64
spendres wants to merge 1 commit into
uebelack:mainfrom
spendres:fix/cross-curve-ecdsa-chain-verify

Conversation

@spendres

Copy link
Copy Markdown

Problem

verifyAttestation rejects every genuine App Attest attestation when run on Deno.

X509Certificate.prototype.verify() is unreliable on non-Node runtimes for cross-curve ECDSA chains. Apple's App Attest chain always has a P-256 leaf signed ecdsa-with-SHA256 by a P-384 sub-CA ("Apple App Attestation CA 1"), and Deno's node:crypto compatibility layer returns false for exactly that link — where Node returns true for the same bytes:

[node v24.9.0]  leaf.verify(subCA.publicKey) : true
[deno 2.9.3]    leaf.verify(subCA.publicKey) : false

The P-384 → P-384 root→sub-CA link passes on both, so only the cross-curve link is affected. It appears the digest is derived from the issuer key's curve (P-384 → SHA-384) instead of read from the certificate's declared signatureAlgorithm (ecdsa-with-SHA256).

Since every App Attest chain contains that link, the failure is total, not intermittent. It surfaces at src/verifyAttestation.js:110 as:

client CA certificate is not signed by Apple App Attestation CA 1

which reads like a forged or malformed client certificate rather than a runtime defect. That made it genuinely hard to diagnose — it disabled device attestation in our production service, and every test stayed green throughout because tests run under Node.

Filed upstream as denoland/deno#36309. Related: #28494 reported the same App Attest use case failing with ERR_NOT_IMPLEMENTED and was closed when verify() was implemented; this is the follow-on, where the method now exists but returns the wrong answer.

Change

Both chain checks go through a new isSignedBy() helper that takes the digest explicitly from the certificate's own signatureAlgorithm OID and verifies with crypto.verify().

This is not a breaking changeverifyAttestation stays synchronous. I specifically avoided the more obvious fix of using a WebCrypto-based chain check (e.g. pkijs's Certificate.verify()), which is also correct on both runtimes but is async and would have forced a major version.

Notes:

  • Uses pkijs, already a dependency. No new dependencies.
  • An unrecognised signature-algorithm OID returns false, so an unverifiable certificate fails closed rather than being accepted.
  • Confirms the diagnosis: passing the digest explicitly makes the identical verification succeed on Deno, so the underlying ECDSA and key handling are fine.

Verification

  • Existing suite passes unchanged: 29 passed (28 existing + 1 new), 100% coverage maintained — the new unsupported-algorithm branch is covered by a test that rewrites the outer signatureAlgorithm OID while leaving the signed bytes intact.
  • prettier --check and eslint clean.
  • A real production App Attest attestation that fails on Deno with the current code verifies successfully with this change on both Node 24.9.0 and Deno 2.9.3.

Happy to adjust the OID table's scope or the naming if you'd prefer something different.

`X509Certificate.prototype.verify()` is unreliable on non-Node runtimes for
cross-curve ECDSA chains. Apple's App Attest chain always has a P-256 leaf
signed ecdsa-with-SHA256 by a P-384 sub-CA, and Deno's node:crypto
compatibility layer returns false for exactly that link, where Node returns
true for the same bytes. It appears to derive the digest from the issuer key's
curve (P-384 -> SHA-384) rather than reading the certificate's declared
signatureAlgorithm (ecdsa-with-SHA256).

Because every App Attest chain contains that link, verifyAttestation rejected
100% of genuine attestations on Deno, failing with "client CA certificate is
not signed by Apple App Attestation CA 1" — which reads like a forged client
certificate rather than a runtime defect. Filed upstream as
denoland/deno#36309

Replace both chain checks with an isSignedBy() helper that takes the digest
explicitly from the certificate's own signatureAlgorithm OID and verifies via
crypto.verify(). This is correct on every runtime and, unlike a WebCrypto-based
chain check, keeps verifyAttestation synchronous — so it is not a breaking
change. It uses pkijs, already a dependency.

An unrecognised signature-algorithm OID returns false, so an unverifiable
certificate fails closed rather than being accepted.

Verified: the existing suite passes unchanged (100% coverage maintained, plus a
new case for the unsupported-algorithm branch), and a real production App
Attest attestation that fails on Deno with the current code verifies
successfully with this change on both Node 24.9.0 and Deno 2.9.3.
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