Skip to content

oprf: reject invalid deterministic blinds - #688

Merged
bwesterb merged 1 commit into
mainfrom
bas/zk30
Aug 5, 2026
Merged

oprf: reject invalid deterministic blinds#688
bwesterb merged 1 commit into
mainfrom
bas/zk30

Conversation

@bwesterb

@bwesterb bwesterb commented Jul 31, 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 found 2 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread oprf/client.go
Comment on lines +54 to +57
blind := blinds[i]
if blind == nil || *blind.Group().Params() != *c.params.group.Params() || blind.IsZero() {
return nil, nil, ErrInvalidInput
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 New validation rules land without any reference to the standard that requires them

The new rejection of empty and zero blinding values is added (blind.IsZero() at oprf/client.go:55) without any comment or commit-message reference to the specification that mandates it, so reviewers cannot check the rule against the standard.
Impact: Reviewers and future maintainers have no traceable justification for the new rejection rules, which the project checklist requires for every implementation change.

Repository rule requiring a spec citation

REVIEW.md (Correctness section) states: "Implementation cites its spec — RFC, FIPS, IETF draft, or paper (ia.cr/...) — in code comments or commit message." The new validation block at oprf/client.go:54-57 and the identity check at oprf/client.go:64-66 add spec-driven behavior (RFC 9497 requires blinds to be non-zero elements of the suite's scalar field), yet neither the code nor the commit message (oprf: reject invalid deterministic blinds, no body) mentions RFC 9497 or the relevant section.

Suggested change
blind := blinds[i]
if blind == nil || *blind.Group().Params() != *c.params.group.Params() || blind.IsZero() {
return nil, nil, ErrInvalidInput
}
// RFC 9497, Section 3.3.1 (Blind): the blind must be a non-zero
// scalar of the suite's group, otherwise it cannot be inverted
// during Finalize.
blind := blinds[i]
if blind == nil || *blind.Group().Params() != *c.params.group.Params() || blind.IsZero() {
return nil, nil, ErrInvalidInput
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread oprf/client.go
Comment on lines +54 to +57
blind := blinds[i]
if blind == nil || *blind.Group().Params() != *c.params.group.Params() || blind.IsZero() {
return nil, nil, ErrInvalidInput
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟨 Blind group membership is checked only by comparing element/scalar byte lengths

The new validation accepts a caller-supplied blinding scalar whenever its group's byte-length parameters match the suite's (*blind.Group().Params() != *c.params.group.Params() at oprf/client.go:55), rather than verifying the scalar actually belongs to the suite's group. Any group whose group.Params (element, compressed element, and scalar lengths) coincide with the suite's would pass this check and then be passed into Mul at oprf/client.go:63, where implementations perform a concrete-type assertion and would panic on attacker/caller-supplied data. Today none of the four supported groups share identical parameters, so the check happens to be sufficient, but it is a fragile invariant for a validation boundary.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@bwesterb
bwesterb merged commit ddeb7fc into main Aug 5, 2026
14 checks passed
@bwesterb
bwesterb deleted the bas/zk30 branch August 5, 2026 16:26
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