Add secp384r1 (NIST P-384) curve support by sridhar-panigrahi · Pull Request #59 · LFDT-Lockness/generic-ec

sridhar-panigrahi · 2026-03-12T18:34:32Z

Summary

Hey! Following up on the discussion in #lockness-contribute and the plan laid out in #58 — this PR adds secp384r1 (NIST P-384) as the first curve with a larger scalar size (48 bytes / 384 bits) to the library.

The goal here was to actually plug in a bigger curve, run the full test suite against it, and see what breaks. The good news is: nothing broke. The existing scalar pipeline and the Straus NAF fix from #57 handle the 48-byte scalars cleanly.

What's in here

p384 crate wired in — new secp384r1 feature flag through both generic-ec-curves and generic-ec, following the same RustCrypto pattern as the existing curves
Scalar traits implemented — CurveName, FromUniformBytes (L=64 bytes per RFC 9380), BytesModOrder, Reduce<48> via U384, and NoInvalidPoints
New reduction utilities — scalar_from_{be,le}_bytes_mod_order_reducing_48 for Horner-method mod-order reduction using 48-byte chunks (same pattern as the _reducing_32 variants)
Every test instantiated for secp384r1 — scalar ops, point ops, serde round-trips, Straus multiscalar multiplication, affine coordinates, dlog_eq ZK proofs, Reduce<48>, and from_bytes_mod_order with various input sizes
Benchmarks added for the new curve

Test results

All 180 tests pass, including all the new secp384r1 ones. Clippy is clean too. No hardcoded scalar-size assumptions surfaced beyond the one already fixed in #57.

What's next (per the plan in #58)

Look at adding more curves with different scalar sizes (curve448, P-521) to stress-test further
Update cggmp24 and givre to use these curves and make sure their tests pass
Do a careful review for any remaining scalar-size assumptions that the test suite might not cover

Happy to adjust anything based on your feedback!

Relates to #58
Depends on #57

Test plan

cargo test --all-features — 180/180 tests pass
cargo clippy --all-features — no warnings
Scalar byte round-trips (BE/LE) work for 48-byte scalars
from_bytes_mod_order works with various input lengths (8, 16, 32, 48, 64, 80, 96, 128, 256, 512 bytes and random lengths)
Straus multiscalar multiplication works with 384-bit NAFs
Serde serialization/deserialization round-trips
Affine coordinate access (x, y, parity) works
dlog_eq zero-knowledge proofs pass (both interactive and non-interactive)

socket-security · 2026-03-12T18:35:30Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	p384@0.13.1

View full report

sridhar-panigrahi · 2026-03-12T18:37:37Z

@survived , let me know your thoughts on this !

survived

Hi @sridhar-panigrahi, the PR looks good on my end. There's one unnecessary change I found, but rather than that it's good!

Could you do GPG-signing on your commits? We can't merge unsigned commits unfortunately (it's mentioned in CONTRIBUTING.md).

In meantime, I'll update cggmp to use this curve and see if all works and it's able to generate valid P-384 ECDSA signatures.

generic-ec-curves/src/rust_crypto/mod.rs

generic-ec-curves/src/utils.rs

The `add_scalar` method in `NafMatrix` hardcodes `x_u64[0..4]` when reading scalar bytes, which only works for 32-byte scalars. This panics for curves with different scalar sizes like P-384 or P-521. Use the actual scalar byte length instead. Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

The `doc_auto_cfg` feature was merged into `doc_cfg` in Rust 1.92.0 (see rust-lang/rust#138907), causing nightly rustdoc builds to fail with E0557. Remove it from the feature list. Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

This is the first curve with a larger scalar size (48 bytes instead of 32) to be added to the library. It exercises the full scalar pipeline with a non-256-bit field and validates that no hardcoded scalar-size assumptions remain. What's included: - p384 crate dependency and `secp384r1` feature flag wired through generic-ec-curves and generic-ec - CurveName, FromUniformBytes (L=64 per RFC 9380), BytesModOrder, Reduce<48>, and NoInvalidPoints implementations - New scalar_from_{be,le}_bytes_mod_order_reducing_48 utility functions for Horner-method reduction using 48-byte chunks - All existing tests instantiated for Secp384r1: scalar ops, point ops, serde, multiscalar (Straus), coordinates, dlog_eq ZKPs, Reduce<48> - Benchmarks updated for the new curve All 180 tests pass. Zero breakage — the dynamic scalar-size handling (including the Straus NAF fix from LFDT-Lockness#57) generalizes cleanly to 384-bit scalars. Relates to LFDT-Lockness#58 Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi · 2026-03-16T18:59:12Z

Hi @survived and @maurges, thanks for the feedback! I've gone ahead and addressed both of the review comments:

Removed secp384r1 from the Sha256 cfg import guard since P-384 uses sha2::Sha384 instead (thanks for catching that @survived!)
Unified the duplicated _32 and _48 helpers into a single generic scalar_from_{le,be}_bytes_mod_order_reducing<S, const N: usize> to avoid the code duplication (great suggestion @maurges!)

All commits are GPG-signed as well. Let me know if there's anything else you'd like me to change!

maurges · 2026-03-17T13:01:18Z

Thanks for the change. It seems you haven't signed off on the latest commit, that's why the CI is failing now

- Remove `secp384r1` from the `Sha256` cfg import guard; P-384 uses `sha2::Sha384`, not `Sha256` (reported by survived) - Replace the duplicated `scalar_from_{le,be}_bytes_mod_order_reducing_32` and `_48` helpers with a single pair generic over `const N: usize`, reducing code duplication (suggested by maurges) Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi · 2026-03-17T19:04:56Z

@maurges Added the DCO sign-off to the latest commit. CI checks are passing now.

sridhar-panigrahi · 2026-03-20T00:07:51Z

@survived @maurges , please do let me know if you think there could be any changes
.

maurges · 2026-03-20T09:33:52Z

The code looks good, I resolved the remaining comments. The CI is still failing - it seems you updated the docs in lib.rs without updating the readme accordingly. Currently they need to manually be kept in sync, and you can test that on your machine by running cd generic-ec; cargo rdme -r ../README.md --check (the same command as in CI)

survived · 2026-03-20T10:55:29Z

by running cd generic-ec; cargo rdme -r ../README.md --check (the same command as in CI)

We have make readme for that!

Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi · 2026-03-20T11:06:04Z

@maurges @survived Thanks for the heads up! I've synced the README with
cargo rdme and pushed the fix. Good to know about make readme — I'll use
that going forward. CI is green now.

survived

All looks good! I tested that cggmp can successfully yield a valid signature on secp384r1 curve in LFDT-Lockness/cggmp21#173

sridhar-panigrahi · 2026-03-20T11:24:45Z

@survived Awesome, thanks for testing and approving! Great to hear the
P-384 ECDSA signatures work end-to-end with cggmp.

sridhar-panigrahi mentioned this pull request Mar 12, 2026

Add support of curves with larger scalar sizes #58

Open

survived reviewed Mar 16, 2026

View reviewed changes

generic-ec-curves/src/rust_crypto/mod.rs Show resolved Hide resolved

maurges reviewed Mar 16, 2026

View reviewed changes

generic-ec-curves/src/utils.rs Outdated Show resolved Hide resolved

sridhar-panigrahi force-pushed the feat/add-secp384r1-support branch from d257a78 to bcc44ee Compare March 16, 2026 18:51

sridhar-panigrahi added 3 commits March 17, 2026 00:23

Remove deprecated doc_auto_cfg feature

40356da

The `doc_auto_cfg` feature was merged into `doc_cfg` in Rust 1.92.0 (see rust-lang/rust#138907), causing nightly rustdoc builds to fail with E0557. Remove it from the feature list. Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

sridhar-panigrahi force-pushed the feat/add-secp384r1-support branch from bcc44ee to c0011e2 Compare March 16, 2026 18:53

sridhar-panigrahi force-pushed the feat/add-secp384r1-support branch from c0011e2 to 28fa09a Compare March 17, 2026 19:03

Sync README.md with lib.rs docs via cargo rdme

5f32742

Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>

survived mentioned this pull request Mar 20, 2026

Add support of secp384r1 curve LFDT-Lockness/cggmp21#173

Merged

survived approved these changes Mar 20, 2026

View reviewed changes

maurges approved these changes Mar 20, 2026

View reviewed changes

maurges merged commit b51cb69 into LFDT-Lockness:m Mar 20, 2026
26 checks passed

survived mentioned this pull request Mar 20, 2026

Fix hardcoded scalar size in Straus NAF computation #57

Closed

Conversation

sridhar-panigrahi commented Mar 12, 2026

Summary

What's in here

Test results

What's next (per the plan in #58)

Test plan

Uh oh!

socket-security bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sridhar-panigrahi commented Mar 12, 2026

Uh oh!

survived left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sridhar-panigrahi commented Mar 16, 2026

Uh oh!

maurges commented Mar 17, 2026

Uh oh!

sridhar-panigrahi commented Mar 17, 2026

Uh oh!

sridhar-panigrahi commented Mar 20, 2026

Uh oh!

maurges commented Mar 20, 2026

Uh oh!

survived commented Mar 20, 2026

Uh oh!

sridhar-panigrahi commented Mar 20, 2026

Uh oh!

survived left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sridhar-panigrahi commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

socket-security bot commented Mar 12, 2026 •

edited

Loading

survived left a comment •

edited

Loading