feat(crypto): support Ed25519/EdDSA in COSE parser for WebAuthn#10080
Closed
sea-snake wants to merge 4 commits intodfinity:masterfrom
Closed
feat(crypto): support Ed25519/EdDSA in COSE parser for WebAuthn#10080sea-snake wants to merge 4 commits intodfinity:masterfrom
sea-snake wants to merge 4 commits intodfinity:masterfrom
Conversation
The COSE parser only accepted ECDSA-P256 (kty=EC2, alg=ES256) and RSA-PKCS1-SHA256 (kty=RSA, alg=RS256). WebAuthn authenticators that produce kty=OKP, alg=EdDSA, crv=Ed25519 keys (e.g. NitroKey 3A) were rejected with the misleading error "Algorithm Unspecified not supported" - the wrapper hardcodes AlgorithmId::Unspecified for any unsupported algorithm, so the actual Ed25519 alg never surfaces. Add an Ed25519/EdDSA branch to CosePublicKey::from_cbor and a parse_eddsa_ed25519 helper that produces an RFC 8410 SPKI DER. Wire Ed25519PublicKeyDerWrappedCose through the standalone sig verifier and the ingress validator's WebAuthn signature path. Per WebAuthn §6.5.6, EdDSA WebAuthn signatures are 64 raw bytes (no DER wrapping), which basic_sig_from_webauthn_sig now passes through unchanged. Tests cover: parsing the canonical RFC 8032 TEST 1 keypair as a COSE key, end-to-end signature verification through the parsed DER, parsing a captured NitroKey 3A WebAuthn registration, and rejection of Ed448 (crv=7) and short-x malformed keys. The validator's webauthn module gets matching Ed25519 fixtures. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the IC WebAuthn/COSE public key handling to accept Ed25519 (COSE kty=OKP, alg=EdDSA, crv=Ed25519) keys, enabling WebAuthn authenticators that emit EdDSA keys to be validated end-to-end through the existing signature verification pipeline.
Changes:
- Add Ed25519 support to the COSE parser, emitting RFC 8410 SPKI DER for Ed25519 public keys.
- Propagate the new COSE-wrapped Ed25519 key content type through the standalone sig verifier and ingress validation WebAuthn path.
- Add/extend tests for COSE Ed25519 parsing and WebAuthn Ed25519 signature verification.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rs/validator/src/webauthn.rs | Accept raw Ed25519 WebAuthn signatures and add Ed25519 validation tests. |
| rs/validator/src/ingress_validation.rs | Treat COSE-wrapped Ed25519 keys as WebAuthn keys for request/delegation validation. |
| rs/crypto/standalone-sig-verifier/src/sign_utils.rs | Introduce Ed25519PublicKeyDerWrappedCose and map AlgorithmId::Ed25519 accordingly. |
| rs/crypto/internal/crypto_lib/basic_sig/cose/tests/tests.rs | Add COSE Ed25519 parsing/verifying tests and negative cases (unsupported curve / short x). |
| rs/crypto/internal/crypto_lib/basic_sig/cose/src/lib.rs | Implement COSE OKP/EdDSA/Ed25519 parsing that returns RFC 8410 DER. |
| rs/crypto/internal/crypto_lib/basic_sig/cose/Cargo.toml | Add ic-ed25519 dependency to the COSE crate. |
| rs/crypto/internal/crypto_lib/basic_sig/cose/BUILD.bazel | Add Bazel dependency on //packages/ic-ed25519. |
| Cargo.lock | Record the added ic-ed25519 dependency in the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+165
to
+168
| /// Parse a COSE EdDSA / Ed25519 key | ||
| fn parse_eddsa_ed25519(fields: &CborMap) -> Result<Self, CosePublicKeyParseError> { | ||
| Self::verify_key_ops(fields)?; | ||
|
|
Comment on lines
+363
to
+364
| Blob(sig.authenticator_data().0), | ||
| Blob(sig.client_data_json().0), |
Author
|
Superseded by #10081 (same changes, branch on dfinity/ic instead of fork). |
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.
Problem
The IC's COSE parser at
rs/crypto/internal/crypto_lib/basic_sig/cose/src/lib.rsonly accepts ECDSA-P256 (kty=EC2,alg=ES256) and RSA-PKCS1-SHA256 (kty=RSA,alg=RS256) keys. WebAuthn authenticators that producekty=OKP,alg=EdDSA,crv=Ed25519keys (e.g. NitroKey 3A) are rejected with the misleading error:The "Unspecified" comes from the wrapper at
parse_cose_public_key, which hardcodesAlgorithmId::Unspecifiedfor anyAlgorithmNotSupportedoutcome — so the actual EdDSAalgvalue never surfaces.Why Ed25519 recovery phrases work but Ed25519 WebAuthn keys don't
The IC already has full Ed25519 support for non-WebAuthn keys. Recovery phrases in Internet Identity generate raw Ed25519 keys that are DER-encoded with the standard Ed25519 algorithm identifier (RFC 8410, OID
1.3.101.112). These take theed25519_algorithm_identifier()path inuser_public_key_from_bytesand are verified viaverify_basic_sig_by_public_key(AlgorithmId::Ed25519, ...)with a plain signature — no WebAuthn envelope, no COSE parsing.WebAuthn authenticators like the NitroKey 3A take a different code path: the public key is COSE-encoded and DER-wrapped with the IC's COSE OID (
1.3.6.1.4.1.56387.1.1). This hits thecose_algorithm_identifier()branch, which callsparse_cose_public_key— and that parser only knew ECDSA-P256 and RSA-SHA256. The Ed25519 cryptographic primitives were always there; the COSE parser just never learned to unwrap Ed25519/EdDSA keys from the COSE map format that WebAuthn produces.This was reported via dfinity/internet-identity#3835 with a captured COSE key from a NitroKey 3A. The captured key has only the standard 4 COSE map entries (no
key_opsor other extension fields), confirming the root cause is the parser's algorithm allowlist, not extra entries.Changes
cosecrateCosePublicKey::Ed25519(Vec<u8>)variant.parse_eddsa_ed25519helper that decodeskty=OKP / alg=EdDSA / crv=Ed25519 / x=<32 bytes>and returns the RFC 8410 SPKI DER viaic_ed25519::PublicKey::deserialize_raw().serialize_rfc8410_der().CosePublicKey::from_cbor.ic-ed25519to the crate'sCargo.tomlandBUILD.bazel.ic-crypto-standalone-sig-verifierKeyBytesContentType::Ed25519PublicKeyDerWrappedCose.AlgorithmId::Ed25519to it incose_key_bytes_content_type.ic-validatoringress_validation.rs: includeEd25519PublicKeyDerWrappedCosein the WebAuthn signature path (both invalidate_signed_requestandvalidate_signed_delegation).webauthn.rs: add anAlgorithmId::Ed25519arm tobasic_sig_from_webauthn_sig. Per WebAuthn §6.5.6, EdDSA WebAuthn signatures are 64 raw bytes (no DER wrapping) — they pass through unchanged.Tests
cose/tests/tests.rs: parsing of the RFC 8032 TEST 1 keypair as a COSE key, end-to-end signature verification through the parsed DER, parsing of the captured NitroKey 3A WebAuthn key, rejection ofcrv=Ed448and short-x malformed keys.validator/src/webauthn.rs: a newed25519test module mirroring the existingecdsaandrsamodules, with a valid-signature, wrong-message, and malformed-signature test. Updates the existingshould_return_error_if_algorithm_id_is_not_supportedtest (which previously asserted Ed25519 was unsupported) to useAlgorithmId::EcdsaSecp256k1instead.Verification
All clean locally. Bazel build/test was not run in the development environment — please verify in CI.
Related