Skip to content

secretsharing: check that share ID is not zero. #644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bwesterb merged 1 commit into from
Jul 15, 2026
+37 −1
Merged

secretsharing: check that share ID is not zero. #644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
secretsharing: check that share ID is not zero. 
Recover() didn't check whether the share ID is zero, which allows one
manipulated share to determine the final recovered secret. The
applicability is rather narrow: the Verify() function that checks secret
commitments already did check whether the share ID is zero. Without
calling Verify(), an attacker can already manipulate the shared secret already
to a certain degree. Arguably, we wouldn't need to check it here, but
it's cheap to do so.
  • Loading branch information
@bwesterb
bwesterb committed Jul 15, 2026
commit 786e7431b8e85bbc879ac9c27a7eb39070a15dc6
12 changes: 11 additions & 1 deletion secretsharing/ss.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
package secretsharing

import (
"errors"
"fmt"
"io"

"github.com/cloudflare/circl/group"
"github.com/cloudflare/circl/math/polynomial"
)

var errZeroID = errors.New("secretsharing: share ID cannot be zero")

// Share represents a share of a secret.
type Share struct {
// ID uniquely identifies a share in a secret sharing instance. ID is never zero.
Expand Down Expand Up @@ -120,13 +123,20 @@ func Verify(t uint, s Share, c SecretCommitment) bool {
}

// Recover returns a secret provided more than t different shares are given.
// Returns an error if the number of shares is not above the threshold t.
// Returns an error if the number of shares is not above the threshold t, or if
// any share has a zero ID.
Comment thread
bwesterb marked this conversation as resolved.
// Panics if some shares are duplicated, i.e., shares must have different IDs.
func Recover(t uint, shares []Share) (secret group.Scalar, err error) {
if l := len(shares); l <= int(t) {
return nil, errThreshold(t, uint(l))
}

for i := range shares {
if shares[i].ID.IsZero() {
return nil, errZeroID
}
}

x := make([]group.Scalar, t+1)
px := make([]group.Scalar, t+1)
for i := range shares[:t+1] {
Expand Down
26 changes: 26 additions & 0 deletions secretsharing/ss_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,32 @@ func TestShareWithID(tt *testing.T) {
})
}

func TestRecoverZeroID(tt *testing.T) {
g := group.P256
t := uint(2)
n := uint(5)

secret := g.RandomScalar(rand.Reader)
ss := secretsharing.New(rand.Reader, t, secret)
shares := ss.Share(n)

// A single Share{ID:0, Value:V} would otherwise force Recover to return
// the attacker-chosen V, independent of the honest shares.
attackerValue := g.RandomScalar(rand.Reader)
tampered := make([]secretsharing.Share, 0, t+1)
tampered = append(tampered, secretsharing.Share{
ID: g.NewScalar(),

@lukevalenta lukevalenta Jul 15, 2026

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.

Optional, to make it a little more clear that this is the zero ID.

Suggested change
ID: g.NewScalar(),
ID: g.NewScalar(), // a zero-valued scalar, the tampered share

Value: attackerValue,
})
for i := uint(0); i < t; i++ {
tampered = append(tampered, shares[i])
}

got, err := secretsharing.Recover(t, tampered)
test.CheckIsErr(tt, err, "must fail to recover with a zero share ID")
test.CheckOk(got == nil, "must not recover a secret with a zero share ID", tt)
}

func BenchmarkSecretSharing(b *testing.B) {
g := group.P256
t := uint(3)
Expand Down
Loading