Document + pin ExtractPublicKey on-curve guarantee (closes #10) - #13
Conversation
JwkConverter.ExtractPublicKey already validates EC (x, y) against the stated curve via EcPointValidator.EnsureOnCurve before returning, but the public contract never promised it. A downstream caller importing an attacker-supplied JWK (e.g. a JWE epk) and feeding it to DeriveSharedSecret could not rely on the invalid-curve defense (RFC 7518 §6.2.2) being applied. - State the on-curve guarantee explicitly in the ExtractPublicKey XML doc, incl. the CryptographicException contract for off-curve / out-of-range / identity points. - Add a regression test using a *fabricated, self-consistent* JWK (valid coordinate lengths, (x,y)=(2,3), not derived from any real point) — the genuine invalid-curve case, distinct from bit-flipping a legitimate point — across P-256/384/521/secp256k1. - Record the guarantee in the FR-8 acceptance criteria (PRD). - Start the 1.1.0 CHANGELOG section; bump the dev-default version to 1.1.0 (release CI still derives the published version from the git tag). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
moisesja
left a comment
There was a problem hiding this comment.
Looks good to me — clean, well-scoped, and low-risk. ✅ (Posting as a comment since GitHub won't let me formally approve this account's own PR.)
Verified the central claim. The PR asserts "no behavior change — the validation was already in place," and the source confirms it: JwkConverter.ExtractPublicKey already calls EcPointValidator.EnsureOnCurve(keyType, x, y) (JwkConverter.cs:110) on the decoded coordinates before reconstructing the SEC1 point and returning, for every EC key type. The new XML doc, PRD FR-8 criterion, and changelog entry all describe this existing behavior accurately — they make an undocumented guarantee contractual rather than changing it.
The regression test is the right test. ExtractPublicKey_ArbitraryForgedJwk_Throws uses correctly-sized but fabricated (x, y) = (2, 3) coordinates that decode cleanly (so they reach EnsureOnCurve) yet don't satisfy the curve equation — the genuine "attacker mints an epk from scratch" case, and meaningfully distinct from the existing bit-flip test. Covering all four EC curves (P-256/384/521/secp256k1) matches the documented scope.
Minor, non-blocking observations (no change required):
- The changelog's
[Unreleased]heading references #10/#11/#12 as the 1.1.0 target, but this PR only delivers #10. Fine for an Unreleased rollup; just flagging so the section doesn't read as if all three landed here. - The dev-default version bump to 1.1.0 is appropriate given the comment notes release CI derives the published version from the git tag.
CI: macOS and the no-native (BBS-absent) job are green; Windows/Ubuntu were still in progress at review time (the only reason the PR shows unstable). Worth a glance before merge, but nothing in this diff suggests a platform-specific risk.
Generated by Claude Code
Summary
Closes #10.
JwkConverter.ExtractPublicKeyalready enforces the invalid-curve defense (it callsEcPointValidator.EnsureOnCurveon every EC point before returning), but the public contract never promised it. A consumer importing an attacker-supplied JWK — e.g. a JWEepk— and feeding the result intoICryptoProvider.DeriveSharedSecrettherefore could not rely on the protection, and an undocumented behavior could silently regress.This PR makes the existing guarantee contractual and pins it with a regression test. No behavior change — the validation was already in place.
Changes
JwkConverter.ExtractPublicKeyXML doc now states the on-curve guarantee explicitly: every EC key type (P-256/P-384/P-521/secp256k1) is validated against the stated curve viaEcPointValidator.EnsureOnCurvebefore returning, throwingCryptographicExceptionfor an off-curve / out-of-range / identity point (RFC 7518 §6.2.2; Antipa et al. 2003; Jager–Schwenk–Somorovsky 2015). OKP curves are a no-op (on-curve by construction). TheCryptographicException/ArgumentExceptionsplit is documented too.ExtractPublicKey_ArbitraryForgedJwk_Throws(P-256/384/521/secp256k1): a fabricated, self-consistent JWK — valid coordinate lengths,(x, y) = (2, 3), not derived from any real point — is rejected at the import boundary. This is the genuine invalid-curve case the issue asks for, distinct from the existing bit-flip test (ExtractPublicKey_OffCurveJwk_Throws).[Unreleased]/ 1.1.0 section; dev-default version bumped to 1.1.0 (release CI still derives the published version from the git tag).Acceptance criteria (issue #10)
ExtractPublicKeyrejects an off-curve EC JWK withCryptographicException— already enforced, now contractual.Verification
dotnet build -warnaserrorclean.🤖 Generated with Claude Code