Consistently label cofactor-absent ECDH test cases

[sgmenda]

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) or ModifiedCofactor (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 = -1 and cofactor = 2 test 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:

• Adds a NoCofactor note with bugType: "MISSING_PARAMETER" and the RFC reference.
• Changes the flag from NegativeCofactor/ModifiedCofactor to NoCofactor.
• Updates comments from "cofactor = 0" / "cofactor = None" to "no cofactor".

Testing:

• go run ./tools/vectorlint passes with 0 invalid files.
• tools/reformat_json.py produces no additional changes (formatting is canonical).
• ASN.1 field counts verified for all 40 non-PEM test cases (20 absent + 20 present) across 10 curves using 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.

[dkostic]

lgtm

[cpu]

Thank you for the detailed PR description/methodology 🙇

This looks correct to me. Would you mind addressing the conflicts?

[cpu]

@sgmenda Gentle bump. No immediate rush, but I think this could be merged if you were able to rebase away the conflicts.

[sgmenda]

@cpu oops, lost track, sorry about that. :/ will resolve later today.

[cpu]

Hmm, something's gone wonky and I can't seem to merge:

Merging is blocked due to failing merge requirements

and

This branch cannot be rebased due to conflicts

Could you try again, or open a second PR if that's easier?

[sgmenda]

@cpu I pushed an empty commit to refresh CI, could you try again? if that doesn't work, I'll open a new PR.

[cpu]

cpu I pushed an empty commit to refresh CI, could you try again?

Confusingly, CI is happy now, and it was before as well. It's specifically the GitHub UI for merging that's ghosted out and unavailable due to "failing merge requirements". The only other hint in the UI is the continued presence of "This branch cannot be rebased due to conflicts":

I'm not sure why this is the case, since typically it would list which files are conflicted and that information is absent. 😕

[botovq]

I don't think any merges to main had merge commits. Is "Allow merge commits" disabled in the repo settings?

[cpu]

Is "Allow merge commits" disabled in the repo settings?

@FiloSottile Are you able to check this? My permissions level on this repo doesn't allow viewing the branch protection rules or other related settings.

[sgmenda]

odd. on my end, it shows no conflicts.

[cpu]

Since I think Filippo is travelling this week for RWC I figured we should try to find a solution that doesn't require changing the repo settings.

I took the one commit with the changes from this branch and rebased it onto main instead of having commits merging main in here. That seems to have done the trick 👍

[sgmenda]

@cpu tyty <3