Consistently label cofactor-absent ECDH test cases#218
Open
Consistently label cofactor-absent ECDH test cases#218
Conversation
cpu
approved these changes
Feb 20, 2026
Member
cpu
left a comment
There was a problem hiding this comment.
Thank you for the detailed PR description/methodology 🙇
This looks correct to me. Would you mind addressing the conflicts?
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
3 participants
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 cofactor of an elliptic curve is the ratio of the total number of points on the curve to the order of the base point. For the NIST P curves (P-224, P-256, P-384, P-521), the cofactor is 1 — every point on the curve is generated by the base point. For Curve25519, the cofactor is 8, meaning only 1/8 of the points on the curve are generated by the base point. The cofactor-8 structure of Curve25519 has been a recurring source of bugs, like signature malleability in Ed25519 (see ristretto.group). In practice, a missing cofactor field is unlikely to cause issues for the P curves since implementations can safely hardcode cofactor = 1, but for curves with larger cofactors the value must be parsed correctly from the parameters.
When elliptic curve parameters are encoded explicitly in ASN.1 (the ECParameters SEQUENCE in X9.62 / SEC 1), the cofactor is an optional INTEGER field at the end of the structure. However, RFC 3279 §2.3.5 says it MUST be present in ECDH public keys.
28 ECDH test cases across 14 files (10 non-PEM + 4 PEM) covering 10 cofactor-1 curves (secp224r1, secp256r1, secp256k1, secp384r1, secp521r1, brainpoolP224r1, brainpoolP256r1, brainpoolP320r1, brainpoolP384r1, brainpoolP512r1) have the cofactor INTEGER entirely absent from the ECParameters, but were tagged with
NegativeCofactor(the"cofactor = 0"cases) orModifiedCofactor(the"cofactor = None"cases).ASN.1 parsing confirms these keys have 5 top-level fields in the ECParameters SEQUENCE (cofactor missing), while the adjacent
cofactor = -1andcofactor = 2test cases have 6 fields (cofactor present). See this gist for a verification script.These test cases are correctly marked
result: "invalid", but the flag should reflect the actual reason. This PR:NoCofactornote withbugType: "MISSING_PARAMETER"and the RFC reference.NegativeCofactor/ModifiedCofactortoNoCofactor."cofactor = 0"/"cofactor = None"to"no cofactor".Testing:
go run ./tools/vectorlintpasses with 0 invalid files.tools/reformat_json.pyproduces no additional changes (formatting is canonical).openssl asn1parse -strparse.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.