Skip to content

ristretto: reject non-canonical scalars#650

Merged
bwesterb merged 1 commit into
mainfrom
bas/gw29
Jul 15, 2026
Merged

ristretto: reject non-canonical scalars#650
bwesterb merged 1 commit into
mainfrom
bas/gw29

Conversation

@bwesterb

@bwesterb bwesterb commented Jul 15, 2026 •
edited by devin-ai-integration Bot
Loading

Copy link
Copy Markdown
Member

Previously we ignored the top 3 bits and reduced otherwise. RFC 9496 allows this, see section 4.4, but advises rejecting all but canonical scalars. Adjust behaviour accordingly.

Open in Devin Review

@bwesterb
ristretto: reject non-canonical scalars
18afce5 
Previously we ignored the top 3 bits and reduced otherwise.
RFC 9496 allows this, see section 4.4, but advises rejecting all but
canonical scalars. Adjust behaviour accordingly.
devin-ai-integration[bot]
devin-ai-integration Bot reviewed Jul 15, 2026
View reviewed changes

@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 potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

lukevalenta
lukevalenta reviewed Jul 15, 2026
View reviewed changes
Comment thread group/ristretto255.go
@bwesterb

bwesterb commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

You still have to press approve, @lukevalenta

@lukevalenta

lukevalenta commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Just flagging: the dependency on https://github.com/bwesterb/go-ristretto seems to go a little against CIRCL's conventions of being self-contained except for golang.org/x/crypto. Perhaps worth folding the implementation in here at some point.

Also, do we need to bump the dep version in https://github.com/cloudflare/circl/blob/main/go.mod#L6 to 1.2.4?

@bwesterb

bwesterb commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Just flagging: the dependency on https://github.com/bwesterb/go-ristretto seems to go a little against CIRCL's conventions of being self-contained except for golang.org/x/crypto.

Yeah, Armando was a bit pragmatic when adding it.

Perhaps worth folding the implementation in here at some point.

Maybe. Not sure it's really worth the investment though.

Also, do we need to bump the dep version in https://github.com/cloudflare/circl/blob/main/go.mod#L6 to 1.2.4?

It's already bumped, no?

Also, you still need to press approve.

@lukevalenta

lukevalenta commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

It's already bumped, no?

D'oh yes 🤦

@bwesterb bwesterb merged commit f19fe76 into main Jul 15, 2026
14 checks passed
@bwesterb bwesterb deleted the bas/gw29 branch July 15, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments
@lukevalenta lukevalenta lukevalenta approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bwesterb @lukevalenta