feat: add secp256r1 to the ECDSA curves in the interface spec - #394
Open
MRmarioruci wants to merge 3 commits into
Open
MRmarioruci wants to merge 3 commits into
MRmarioruci wants to merge 3 commits into
Conversation
New variant `secp256r1` of `ecdsa_curve`, selecting threshold ECDSA on the NIST P-256 curve in `ecdsa_public_key` and `sign_with_ecdsa`. Key derivation for the new curve is the same generalization of BIP32 instantiated over P-256, which is SLIP-10. The derivation paragraph is split into one bullet per curve, matching how `schnorr_public_key` already presents its two algorithms. Signatures are encoded as for `secp256k1`: P-256 has a 256-bit group order, so r and s are 32 bytes each. The `ecdsa_curve` argument of `ic0.cost_sign_with_ecdsa` accepts 1. This describes the interface, not availability. As for any curve, whether a given `key_id` exists depends on the implementation, so the pages listing the keys deployed on mainnet are unchanged.
github-actions
Bot
added
the
interface-spec
|
🤖 Here's your preview: https://kwnd6-zaaaa-aaaam-ai7va-cai.icp0.io |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review comments were identified, and the interface and documentation changes are consistent.
Pull request overview
Adds secp256r1 support to the IC interface specification for threshold ECDSA.
Changes:
- Adds the P-256 Candid curve variant.
- Documents SLIP-10 derivation and 64-byte signatures.
- Maps System API curve value
1to P-256 and updates the changelog.
File summaries
| File | Description |
|---|---|
public/references/ic.did |
Adds the secp256r1 variant. |
docs/references/ic-interface-spec/management-canister.md |
Documents P-256 derivation and signatures. |
docs/references/ic-interface-spec/changelog.md |
Records the interface addition. |
docs/references/ic-interface-spec/canister-interface.md |
Documents the numeric curve mapping. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
MRmarioruci
marked this pull request as ready for review
September 15, 2026 12:54
Member
Review: secp256r1 in the ECDSA curvesMust fix
Suggestions
|
The entry was written when 0.67.0 was the open section. 0.68.0 has since been cut, so the entry sat under a version released three weeks before the change existed. Git merged it cleanly because the insertion point stayed unambiguous, which is why nothing flagged it.
pull Bot
pushed a commit
to mikeyhodl/ic
that referenced
this pull request
Sep 22, 2026
### Why * The threshold crypto layer already implements ECDSA on NIST P-256. [`EccCurveType`](https://github.com/dfinity/ic/blob/master/rs/crypto/internal/crypto_lib/threshold_sig/canister_threshold_sig/src/utils/group.rs#L25) has a `P256` variant, `AlgorithmId` has `ThresholdEcdsaSecp256r1`, and the IDkg code paths are curve-generic, with no `todo!` or `unimplemented!` left on a P256 arm. * The public [`EcdsaCurve`](https://github.com/dfinity/ic/blob/master/rs/types/management_canister_types/src/lib.rs#L2892) enum has a single variant, so a `secp256r1` key id cannot be named in the management canister API or represented in the registry. The curve is unreachable from outside the crypto layer. * P-256 is the curve the web platform standardised on: WebAuthn, TLS, VAPID web push (ES256) and OAuth DPoP. A canister cannot produce any of those signatures today, so each one needs a key held off-chain. For Internet Identity's web push work that key is the blocker. ### What `EcdsaCurve` gains `Secp256r1` in `rs/types/management_canister_types` and in the published `packages/ic-management-canister-types`, together with the Candid doc comment, the `u32` conversions that decode the `ic0.cost_sign_with_ecdsa` argument, and `FromStr`, which is what NNS proposals parse. [`From<EcdsaCurve> for AlgorithmId`](https://github.com/dfinity/ic/blob/master/rs/types/types/src/crypto.rs#L277) maps the new variant to `AlgorithmId::ThresholdEcdsaSecp256r1`; that is the single conversion handing a key id to the crypto layer. `types.proto` gains `ECDSA_CURVE_SECP256R1 = 2` so an `EcdsaKeyId` round-trips through the registry, plus the regenerated files. Both `tests/ic.did` fixtures gain the variant, which is what the Candid equality tests compare the Rust enum against. Two values need confirming before this merges: `u32` = 1 and protobuf = 2. Each is the next free value in its own scheme, but the `u32` is normative because it is the System API ABI in the interface spec. The matching spec change is dfinity/developer-docs#394. The variant has to land in one commit. Adding it to either the proto or a types crate on its own breaks the other's exhaustive match, so no split compiles. This generates no key and makes no key id available. Enabling a key is a separate registry and governance step. `rs/tests/consensus/tecdsa` is deliberately not extended. `cast_ecdsa_key_id` casts into the published `ic-management-canister-types` through cdk-rs, so r1 system tests need that crate released first, and `verify_signature` needs a P-256 verifier. Happy to follow up with that once the crate is out. ### Tests `cargo check --tests`, the 39 unit tests and both `candid_equality_test`s pass on `ic-management-canister-types`, `ic-management-canister-types-private`, `ic-types` and `ic-protobuf`. `cargo run -p ic-protobuf-generator` reproduces the five generated files exactly. Two gaps CI needs to cover, since neither is reachable on macOS: `rs/orchestrator` does not build there (`libc::gettid`, `SYS_futex`, `libcryptsetup-rs`), so its test arm is unverified, and a match in any other Linux-only crate would not have shown up locally. Unrelated, but worth flagging: running the protobuf generator also rewrites doc-comment escaping in six generated files it does not otherwise touch (`\<=` becomes `<=`, list renumbering). That looks like prost-build 0.14.3 disagreeing with whatever version produced the committed files. Reverted here, but the next regen will hit it.
alin-at-dfinity
pushed a commit
to dfinity/ic
that referenced
this pull request
Sep 22, 2026
### Why * The threshold crypto layer already implements ECDSA on NIST P-256. [`EccCurveType`](https://github.com/dfinity/ic/blob/master/rs/crypto/internal/crypto_lib/threshold_sig/canister_threshold_sig/src/utils/group.rs#L25) has a `P256` variant, `AlgorithmId` has `ThresholdEcdsaSecp256r1`, and the IDkg code paths are curve-generic, with no `todo!` or `unimplemented!` left on a P256 arm. * The public [`EcdsaCurve`](https://github.com/dfinity/ic/blob/master/rs/types/management_canister_types/src/lib.rs#L2892) enum has a single variant, so a `secp256r1` key id cannot be named in the management canister API or represented in the registry. The curve is unreachable from outside the crypto layer. * P-256 is the curve the web platform standardised on: WebAuthn, TLS, VAPID web push (ES256) and OAuth DPoP. A canister cannot produce any of those signatures today, so each one needs a key held off-chain. For Internet Identity's web push work that key is the blocker. ### What `EcdsaCurve` gains `Secp256r1` in `rs/types/management_canister_types` and in the published `packages/ic-management-canister-types`, together with the Candid doc comment, the `u32` conversions that decode the `ic0.cost_sign_with_ecdsa` argument, and `FromStr`, which is what NNS proposals parse. [`From<EcdsaCurve> for AlgorithmId`](https://github.com/dfinity/ic/blob/master/rs/types/types/src/crypto.rs#L277) maps the new variant to `AlgorithmId::ThresholdEcdsaSecp256r1`; that is the single conversion handing a key id to the crypto layer. `types.proto` gains `ECDSA_CURVE_SECP256R1 = 2` so an `EcdsaKeyId` round-trips through the registry, plus the regenerated files. Both `tests/ic.did` fixtures gain the variant, which is what the Candid equality tests compare the Rust enum against. Two values need confirming before this merges: `u32` = 1 and protobuf = 2. Each is the next free value in its own scheme, but the `u32` is normative because it is the System API ABI in the interface spec. The matching spec change is dfinity/developer-docs#394. The variant has to land in one commit. Adding it to either the proto or a types crate on its own breaks the other's exhaustive match, so no split compiles. This generates no key and makes no key id available. Enabling a key is a separate registry and governance step. `rs/tests/consensus/tecdsa` is deliberately not extended. `cast_ecdsa_key_id` casts into the published `ic-management-canister-types` through cdk-rs, so r1 system tests need that crate released first, and `verify_signature` needs a P-256 verifier. Happy to follow up with that once the crate is out. ### Tests `cargo check --tests`, the 39 unit tests and both `candid_equality_test`s pass on `ic-management-canister-types`, `ic-management-canister-types-private`, `ic-types` and `ic-protobuf`. `cargo run -p ic-protobuf-generator` reproduces the five generated files exactly. Two gaps CI needs to cover, since neither is reachable on macOS: `rs/orchestrator` does not build there (`libc::gettid`, `SYS_futex`, `libcryptsetup-rs`), so its test arm is unverified, and a match in any other Linux-only crate would not have shown up locally. Unrelated, but worth flagging: running the protobuf generator also rewrites doc-comment escaping in six generated files it does not otherwise touch (`\<=` becomes `<=`, list renumbering). That looks like prost-build 0.14.3 disagreeing with whatever version produced the committed files. Reverted here, but the next regen will hit it.
This branch has not been deployed
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.
New variant
secp256r1ofecdsa_curve, selecting threshold ECDSA on the NIST P-256 curve inecdsa_public_keyandsign_with_ecdsa. P-256 is the curve the web platform standardised on: WebAuthn, TLS, VAPID web push (ES256) and OAuth DPoP. A canister cannot produce any of those signatures today, so each one needs a key held off-chain.Changes
Key derivation for the new curve is the same generalization of BIP32 instantiated over P-256, which is SLIP-10.
signing/key_derivation.rs:210dispatchesEd25519 => eddsa_ckd, _ => bip32_ckdpub, so K256 and P256 run the same code, andbip32_ckdpubdocuments itself as following SLIP-0010 with BIP32 identical for K256. The derivation paragraph is split into one bullet per curve, matching howschnorr_public_keyalready presents its two algorithms. Signatures are encoded as forsecp256k1, becauseEccCurveType::scalar_bytesis 32 on both curves, so r and s are 32 bytes each. Theecdsa_curveargument ofic0.cost_sign_with_ecdsaaccepts1, and the changelog bullet goes under the current0.67.0heading.Two things deliberately left out. The
u32value1is normative, since it is the System API ABI, so it has to agree withdfinity/ic: please confirm it before this merges. And this describes the interface, not availability. Whether a givenkey_idexists depends on the implementation, so the pages that list the keys deployed on mainnet are unchanged,docs/references/management-canister.mdincluded. Those should change when a key is generated.Paired with dfinity/ic#11573, which adds the variant to the implementation.
Tests
didc check public/references/ic.didpasses, andnode scripts/validate.jspasses on the three changed markdown files.🤖 Generated with Claude Code