Repository navigation
Conversation
A keyed algorithm declares a list of key types (KeyedAlgorithm.KeyTypes of AlgorithmKeyType), so one algorithm name serves several key types, as RFC 8037 section 3.2 needs for ECDH-ES over EC and OKP keys. The ECDH-ES algorithms take their key types from registered IEcdhKeyAgreement strategies; the core registers the EC agreement over P-256, P-384 and P-521. The epk is written and read through the key type's ISecretKeyTypeDescriptor and JSON Web Key rules: only the type's required members are written, the kty must be the recipient key's, and a received epk holding any private member is refused. The JWE decryption key check, ValidateKeyAlgorithm, the client authentication and decryption policies, tenant key provisioning (with an optional SecretType) and the management capabilities document (KeyTypes per secret type) read the key types. ECDH-ES+A*KW now wraps a random content encryption key, and AEAD algorithms are no longer keyed algorithms. ADR-1389 records the decision. Closes #327
NCode.Identity.Jose.EdDsa implements the X25519 and X448 functions of RFC 7748 clean-room, as constant-time Montgomery ladders over its existing field arithmetic, and AddEdDsa() registers them as the OKP agreement of ECDH-ES, ECDH-ES+A128KW, ECDH-ES+A192KW and ECDH-ES+A256KW (RFC 8037 section 3.2). The OKP key type generates, imports, exports and stores crv X25519 and X448 keys as it does the signing keys. The curve catalog records each curve's use, so an X25519 or X448 key never signs and an Ed25519 or Ed448 key never agrees, although X25519 and Ed25519 keys have the same size. An all-zero shared secret, from an epk or a recipient key of small order, is refused on both sides. Tests cover the RFC 7748 section 5.2 and section 6 vectors (iterated to 1,000), the RFC 8037 Appendix A.6 example, the RFC 8410 private key example, a differential oracle, and JWE round trips over each curve and key wrap. ADR-1390 records the decision. Closes #336
NCode.Identity.Jose.Secp256k1 adds ES256K (RFC 8812): AddSecp256k1() registers the secp256k1 curve (crv "secp256k1") in the elliptic-curve catalog and an ES256K ECDSA algorithm bound to it, so a P-256 key never signs as ES256K and a secp256k1 key never signs as ES256. Registration probes the platform first and registers nothing where its cryptography lacks the curve, as on macOS. An ECDSA algorithm is data: an EccSignatureAlgorithmSpecification service names its code, curve and hash, and the JOSE library serves one algorithm per specification beside ES256, ES384 and ES512, failing at startup on an empty or duplicate code. AlgorithmKeyType.Curve names the curve an algorithm is bound to, so tenant key provisioning generates ES256K, Ed25519 and Ed448 keys on their curves, and a stored key satisfies an algorithm only when the algorithm accepts its public half. Recorded in ADR-1391, which amends ADR-1169, ADR-1272 and ADR-1390. Closes #337
- Check a key's curve when it is declared for an algorithm: AlgorithmKeyType lists its curves as AlgorithmCurve (crv and key size), KeyAlgorithmDeclaration carries the curve and, where the caller holds it, the key, and the client key importer and the management update pass the key, so an Ed25519 key for ECDH-ES or a P-256 key for ES256K is refused at registration. - Report a JWE decryption key on a curve the algorithm refuses as the caller's configuration error. - Choose the ECDH-ES agreement per key, merge agreements that serve one key type, and order key types independently of registration order. - Provision tenant keys and management keys created without a curve on the first of the algorithm's curves with the key's size, so OKP keys for ECDH-ES and EdDSA can be provisioned; list each key type's curves in the algorithms capability and offer only those in the console. - Test hygiene: Linux-only fact, region names, one behaviour per test, an honest label for the X25519 PKCS#8 input, and doc comments on the X25519 and X448 constants.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #532 +/- ##
==========================================
+ Coverage 92.09% 92.28% +0.18%
==========================================
Files 1495 1518 +23
Lines 62761 63825 +1064
Branches 306 305 -1
==========================================
+ Hits 57798 58899 +1101
+ Misses 4963 4926 -37
Flags with carried forward coverage won't be shown. Click here to find out more. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
![ncode-review[bot]](https://avatars.githubusercontent.com/u/12114478?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 NCode Review at 0af7f02: 31 new findings on the changed lines.
Not on a changed line:
- 🟡 F28 Nit: Summary line exceeds the wrap width
src/NCode.Identity.OpenId.Management/Endpoints/Capabilities/AlgorithmCapabilitiesProvider.cs:30. The reworded 'with its implied use and each compatible secret type...' line runs past the width the rest of the comment wraps at. Fix: Rewrap the summary at the same column as the other lines.
|
Superseded by the review at df1e680. |
polewskm
changed the title
An asymmetric key names its curve (AsymmetricSecretKey.CurveName, from the elliptic-curve catalog for ecc keys), and AlgorithmKeyType.IsKeyCompatible is the one rule algorithms and ECDH agreements check a key's type, size and curve by. AlgorithmKeyType.OnCurves takes its sizes from its curves. A KeyAlgorithmDeclaration carries the key itself, and a key to generate is resolved onto a curve by TryResolveKeyGeneration, which tenant provisioning and management creation share; ValidateCreate returns the generation parameters, and a tenant key may name its Curve. A stored tenant key without a readable public half does not satisfy a specification. ECDH agreements keep registration order, the wrapping algorithm agrees its KEK through a named helper, ES256/ES384/ES512 are registered as data and every ECDSA specification's curve must be in the catalog, AddSecretsLibrary registers logging, and the OKP curves are objects of their kind. The console offers discovered secret types and every curve an algorithm lists.
…follow-up) A tenant key specification's curve is part of the key it names: a stored key satisfies it only when its public half is on that curve, so a configured X448 or Brainpool curve is provisioned beside an X25519 or P-256 key. TryResolveKeyGeneration refuses a curve declared for a key type the algorithm names no curve for, and ResolvedKeyGeneration.Curve is null for such a type. A held key of a type without curves whose material is not at hand, such as a stored symmetric key, is checked by ValidateKeySize over a KeySizeDeclaration, which refuses a curve-bound key type. GenerateSecretRequest carries the validated SecretKeyGenerationParameters whole as GenerationParameters, so no handler copies the size and curve field by field; a shared secret refuses a curve. The ECDH-ES algorithms refuse agreements of one key type that mix curves with sizes only, so the merged key type always lists its curves. The console offers only the discovered secret types, and a secret type with curves takes a curve, the type's first being chosen with it. Docs: OpenIdTenantKeyOptions.KeySizeBits names the algorithm's first curve, ISecretValidator.ValidateUpdate describes the public-half check, the ECDSA specification checks run when the algorithm collection is built, ADR-1390 and ADR-1392 state their context without narrating it, and the index lists what ADR-1389 amends.
…es256k # Conflicts: # docs/decisions/README.md
Contributor
Author
|
Autopilot: The NCode Review finding that isn't on a changed line:
Generated by Claude Code |
Contributor
Author
|
Autopilot: Two checks are red at df1e680, and both are this PR's to fix.
Generated by Claude Code |
Contributor
Author
|
/review |
🤖 Review completeVerdict: ✅ Approve — Deep review (sensitive paths): Re-review of what changed since the last review at 0af7f02: 511 hunks in 137 files and changes taken back in 83 files, by correctness and security. No further findings above nits. 2 reviewers run — 0 findings (run details)
Trial: findings are posted as review threads and in the check run. $3.33 on Amazon Bedrock, 31s, at df1e680. (how to interact) |
…curve selection The secret-type select held the seeded `symmetric` while the discovered type list was still empty, which MUI reports as an out-of-range value. The form now selects only a type it offers: the seeded type when offered, otherwise the first offered type, and none (with submit blocked) while the types load. A selected curve now fixes the key size by derivation rather than by copying it into state. SecretsManager forwarded the curve only for `ecc`, so an `okp` secret lost the curve its key type requires; it now forwards the form's curve for every secret type with curves. The container tests that create a secret now discover a secret type and wait for the form to offer it.
…es256k # Conflicts: # CHANGELOG.md # src/NCode.Identity.Jose/Algorithms/KeyManagement/Pbes2KeyManagementAlgorithm.cs
Contributor
Author
|
Autopilot: Both fixes are pushed. CI was green on d703fc1, and it reruns now on 56e7835.
Generated by Claude Code |
Cover the refusals of the EC and OKP agreements called directly (a shared-secret buffer of the wrong size, a recipient on a curve or of a size the agreement does not serve, a public-only recipient, an ephemeral key of another type), the ECDH-ES algorithm's refusals of a key type without a JSON Web Key form, of an ephemeral key its key type cannot read, of a contributed key type whose required members name a private member or are missing from its export, and of a malformed apu or apv, the merge of size-only agreements, and the matching gaps in AlgorithmCollectionExtensions, DefaultTenantKeyPolicy, the OKP curves and keys, and the X25519 and X448 guards. Remove two dead branches: DecodePartyInfo decodes through Base64Url.Decode, since TryDecode into a buffer of the decoded size either throws or fills it, and TryResolveKeyGeneration reports the declared size alone, since a curve's own size is read only when it was not found.
Contributor
Author
|
Autopilot: bbf9977 covers the refusal paths Codecov reported as untested:
Every flagged file except Generated by Claude Code |
This branch was successfully 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.
The JOSE algorithm model lets one algorithm name serve every key type it declares. On that model, ECDH-ES works over any registered key type, X25519 and X448 are available, and ES256K is an opt-in package.
KeyedAlgorithm.KeyTypeslistsAlgorithmKeyTypeentries in order of preference. Each entry gives a secret type, its sizes and its curves; a key type bound to curves takes its sizes from them (AlgorithmKeyType.OnCurves).IEcdhKeyAgreements. The core serves P-256, P-384 and P-521.epkis written and read through the key type's own descriptors: only its required public members, never a private one.ECDH-ES+A*KWgenerates a random content encryption key and wraps it under the key agreed with theepk(RFC 7518 §4.6).algorithmscapability lists each compatible secret type with its sizes and curves, and the console offers exactly those.AddEdDsa()registers clean-room RFC 7748 agreements: a constant-time Montgomery ladder that refuses an all-zero shared secret, tested against the RFC 7748 and RFC 8037 vectors.OKPkeys apart from the Ed25519/Ed448 signing keys.NCode.Identity.Jose.Secp256k1providesAddSecp256k1()onIServiceBuilder<JoseLibrary>.EccSignatureAlgorithmSpecificationas data, and registers nothing where the platform lacks the curve.IAlgorithmCollection.TryResolveKeyGenerationplaces every generated key on one of the algorithm's curves, and is shared by tenant provisioning and management create.ISecretValidator.ValidateCreatereturns the resolvedSecretKeyGenerationParameters.SecretTypeandCurve.Closes #327
Closes #336
Closes #337