Skip to content

sign/bls: rejects aggregated signatures built with duplicated messages.#595

Merged
armfazh merged 1 commit intocloudflare:mainfrom
armfazh:bls_verification
Apr 2, 2026
Merged

sign/bls: rejects aggregated signatures built with duplicated messages.#595
armfazh merged 1 commit intocloudflare:mainfrom
armfazh:bls_verification

Conversation

@armfazh
Copy link
Copy Markdown
Contributor

@armfazh armfazh commented Apr 1, 2026

AggregateVerify must reject attempts to verify aggregated signatures with duplicated messages.

@armfazh armfazh requested review from bwesterb and rozbb April 1, 2026 16:13
@armfazh armfazh added the bug Something isn't working label Apr 1, 2026
// 1. If any two input messages are equal, return INVALID.
set := make(map[string]struct{}, len(msgs))
for _, m := range msgs {
k := string(m)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why cast to string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

a []byte type does not get native comparison (it has to use bytes.Equal), while string does.

Copy link
Copy Markdown

@rozbb rozbb Apr 1, 2026

Choose a reason for hiding this comment

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

Is it guaranteed that bytes.Equal(x, y) if and only if string(x) == string(y) for all bytestrings x, y?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

interesting, the code now encodes the messages into string explictly.
another option is to hash messages, but more costly and collisions might happen.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah I see. My concern now is that this might allocate a huge amount if messages are large.

I think the original string thing might be fine, actually. Go's string comparison is actually a byte comparison. And according to Gemini, Go's compiler avoids allocating for string(x) when it's only used for a map lookup.

The other option is you can do make(map[[32]byte]struct{}, len(msgs)), and use Blake3 or something fast for hashing. Blake3 is cryptographic, so collisions won't be an issue.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am learning a lot about go 😃

@armfazh armfazh force-pushed the bls_verification branch from af92289 to 09bf70f Compare April 1, 2026 21:29
@armfazh armfazh requested a review from rozbb April 1, 2026 21:32
@armfazh armfazh force-pushed the bls_verification branch from 09bf70f to e97b6e7 Compare April 2, 2026 15:12
@armfazh armfazh merged commit 9798df7 into cloudflare:main Apr 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants