Skip to content

ecc/p384: document that package is not fully constant time - #651

Merged
bwesterb merged 2 commits into
mainfrom
bas/gw30
Jul 15, 2026
Merged

ecc/p384: document that package is not fully constant time#651
bwesterb merged 2 commits into
mainfrom
bas/gw30

Conversation

@bwesterb

@bwesterb bwesterb commented Jul 15, 2026

Copy link
Copy Markdown
Member

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@lukevalenta lukevalenta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this worth updating to make this constant-time like crypto/internal/nistec is?

Comment thread ecc/p384/doc.go Outdated
Comment thread ecc/p384/doc.go
// - A new method included for double-point multiplication.
//
// Note: the scalar-multiplication routines in this package are not fully
// constant time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to add a little more detail:

Suggested change
// constant time.
// constant time. Execution time and memory access can depend on the value of
// the secret scalar, so avoid using these routines with secret scalars where
// timing side-channels are a concern.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude suggested this as a description of where the non-constant timedness comes from. I have not carefully checked though, so if it doesn't sound right please ignore:

// Note: the scalar-multiplication routines in this package are not fully
// constant time. The windowed point arithmetic itself is constant time, but
// the scalar reduction and recoding (reduceScalar and toOdd) use variable-time
// math/big operations, so execution time and memory allocations can depend on
// the value of the secret scalar. Avoid using these routines with secret
// scalars where timing side-channels are a concern.

Comment thread ecc/p384/p384opt.go
}

// ScalarMult returns (Qx,Qy)=k*(Px,Py) where k is a number in big-endian form.
// It is not fully constant time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// It is not fully constant time.
// It is not fully constant time; see the package documentation for details.

Comment thread ecc/p384/p384opt.go

// ScalarBaseMult returns k*G, where G is the base point of the group
// and k is an integer in big-endian form.
// and k is an integer in big-endian form. It is not fully constant time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// and k is an integer in big-endian form. It is not fully constant time.
// and k is an integer in big-endian form. It is not fully constant time;
// see the package documentation for details.

Co-authored-by: Luke Valenta <lvalenta@cloudflare.com>
@bwesterb
bwesterb merged commit 62df925 into main Jul 15, 2026
5 of 6 checks passed
@bwesterb
bwesterb deleted the bas/gw30 branch July 15, 2026 14:44
cjpatton pushed a commit that referenced this pull request Jul 22, 2026
Issue #657 asked for an explicit README-level warning that some
experimental CIRCL packages are not constant time. PRs #638 and #651
added per-package constant-time caveats to eight packages, but the
README's Security Disclaimer still only mentioned that the library is
experimental — it never mentioned constant time.

Add a "Constant-Time Caveats" subsection under the existing Security
Disclaimer, pointing readers at the per-package notes in group, oprf,
blindsign/blindrsa/partiallyblindrsa, secretsharing, tss/rsa, zk/dl,
zk/dleq, and ecc/p384.

Documentation-only change: no source files touched, no API change,
no tests added.

Refs: #657
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.

2 participants