Skip to content

Refactor EC key padding length lookup into shared cryptoECC helper; add BN-254 support#26

Merged
ljoy913 merged 3 commits into
ljoy/update-build-dependenciesfrom
copilot/fix-bn-254-padding-issue
Jun 28, 2026
Merged

Refactor EC key padding length lookup into shared cryptoECC helper; add BN-254 support#26
ljoy913 merged 3 commits into
ljoy/update-build-dependenciesfrom
copilot/fix-bn-254-padding-issue

Conversation

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

PR #25’s leading-zero padding logic introduced four duplicated curve-length tables in ECDH/ECDSA, and all four omitted BN-254. This change consolidates curve element-length resolution into a single shared source and makes lookups case-insensitive to avoid drift and mismatches.

  • Shared curve element-length source in cryptoECC

    • Added curveElementLengths in src/cryptoECC.js with the full supported set, including "BN-254": 32.
    • Added/exported cryptoECC.curveElementLength(curveName) to normalize via toUpperCase() and return the fixed element byte length.
  • Replaced duplicated partLen tables (4 call sites)

    • src/ecdh.js
      • generateKey
      • importKey (p.format === "jwk")
    • src/ecdsa.js
      • generateKey
      • importKey (p.format === "jwk")
    • Each now reads from cryptoECC.curveElementLength(...) instead of inline object literals.
  • Targeted BN-254 coverage updates in existing ECC tests

    • Added BN-254 key-generation test cases in test/Test.Ecdh.js and test/Test.Ecdsa.js.
    • Updated test key-length checks to use cryptoECC.curveElementLength(...) directly instead of duplicated ecdhKeyLengths / ecdsaKeyLengths tables.

Example of the lookup change:

// before
var partLen = {
    "P-256": 32, "P-384": 48, "P-521": 66,
    "NUMSP256D1": 32, "NUMSP256T1": 32,
    "NUMSP384D1": 48, "NUMSP384T1": 48,
    "NUMSP512D1": 64, "NUMSP512T1": 64
}[p.algorithm.namedCurve];

// after
var partLen = cryptoECC.curveElementLength(p.algorithm.namedCurve);

Copilot AI changed the title [WIP] Fix padding logic for BN-254 keys and remove duplication Refactor EC key padding length lookup into shared cryptoECC helper; add BN-254 support Jun 27, 2026
Copilot AI requested a review from zaverucha June 27, 2026 18:08

@zaverucha zaverucha left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test code in Test.Ecdsa.js and Test.Ecdh.js duplicates the tables (ecdhKeyLengths and ecdsaKeyLengths) rather than using cryptoECC.curveElementLength.

Copilot AI requested a review from zaverucha June 27, 2026 18:26
@zaverucha zaverucha marked this pull request as ready for review June 27, 2026 20:02
@zaverucha zaverucha requested a review from ljoy913 June 27, 2026 20:02
@ljoy913 ljoy913 merged commit 1b0cca2 into ljoy/update-build-dependencies Jun 28, 2026
1 check passed
ljoy913 added a commit that referenced this pull request Jun 28, 2026
ljoy913 added a commit that referenced this pull request Jun 28, 2026
The #26 change replaced the tests' own curve-length tables with calls to
cryptoECC.curveElementLength(). cryptoECC is an internal module inside the
UMD bundle and is not exposed as a global, so SubtleTests.html threw
ReferenceError: cryptoECC is not defined. Restore independent test-local
tables (keeping the new BN-254 entry); the production de-duplication in
ecdh.js/ecdsa.js/cryptoECC.js is unaffected.
@ljoy913 ljoy913 deleted the copilot/fix-bn-254-padding-issue branch June 29, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants