fix: allow increment of 1 in Monotonic entropy for io.Reader sources
MonotonicEntropy.random rejected a validly drawn value of 0 in its rejection-sampling loop, making an increment of exactly 1 unreachable for entropy sources without a fast Int63n path (e.g. crypto/rand.Reader). For inc == 2 this made the increment fully deterministic (always 2), contradicting the documented "random number between 1 and inc inclusive" contract. Restructure the loop to match the crypto/rand.Int pattern it is adapted from, so 0 is a valid draw and the result is correctly uniform over [1, inc].
This commit is contained in:
1 parent
8b0a7fd89a
commit
4c737a9be4
2 files changed
+53
-4
No files matched your search
@@ -641,7 +641,7 @@ func (m *MonotonicEntropy) increment() error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// random returns a uniform random value in [1, m.inc), reading entropy
|
// random returns a uniform random value in [1, m.inc], reading entropy
|
||||||
// from m.Reader. When m.inc == 0 || m.inc == 1, it returns 1.
|
// from m.Reader. When m.inc == 0 || m.inc == 1, it returns 1.
|
||||||
// Adapted from: https://golang.org/pkg/crypto/rand/#Int
|
// Adapted from: https://golang.org/pkg/crypto/rand/#Int
|
||||||
func (m *MonotonicEntropy) random() (inc uint64, err error) {
|
func (m *MonotonicEntropy) random() (inc uint64, err error) {
|
||||||
@@ -651,7 +651,7 @@ func (m *MonotonicEntropy) random() (inc uint64, err error) {
|
|||||||
|
|
||||||
// Fast path for using a underlying rand.Rand directly.
|
// Fast path for using a underlying rand.Rand directly.
|
||||||
if m.rng != nil {
|
if m.rng != nil {
|
||||||
// Range: [1, m.inc)
|
// Range: [1, m.inc]
|
||||||
return 1 + uint64(m.rng.Int63n(int64(m.inc))), nil
|
return 1 + uint64(m.rng.Int63n(int64(m.inc))), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -667,7 +667,7 @@ func (m *MonotonicEntropy) random() (inc uint64, err error) {
|
|||||||
msbitLen = 8
|
msbitLen = 8
|
||||||
}
|
}
|
||||||
|
|
||||||
for inc == 0 || inc >= m.inc {
|
for {
|
||||||
if _, err = io.ReadFull(m.Reader, m.rand[:byteLen]); err != nil {
|
if _, err = io.ReadFull(m.Reader, m.rand[:byteLen]); err != nil {
|
||||||
return 0, err
|
return 0, err
|
||||||
}
|
}
|
||||||
@@ -688,9 +688,14 @@ func (m *MonotonicEntropy) random() (inc uint64, err error) {
|
|||||||
case 5, 6, 7, 8:
|
case 5, 6, 7, 8:
|
||||||
inc = uint64(binary.LittleEndian.Uint64(m.rand[:8]))
|
inc = uint64(binary.LittleEndian.Uint64(m.rand[:8]))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// inc may exceed m.inc; redraw rather than bias the low end of the range.
|
||||||
|
if inc < m.inc {
|
||||||
|
break
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Range: [1, m.inc)
|
// Range: [1, m.inc]
|
||||||
return 1 + inc, nil
|
return 1 + inc, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"math"
|
"math"
|
||||||
|
"math/big"
|
||||||
"math/rand"
|
"math/rand"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -629,6 +630,49 @@ func TestMonotonic(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Regression test: random() rejected a masked draw of 0, making an
|
||||||
|
// increment of 1 unreachable and inc == 2 fully deterministic. The reader
|
||||||
|
// yields an exact byte sequence so the result does not depend on chance.
|
||||||
|
func TestMonotonicIncrementRange(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// The first 10 bytes seed the first ULID; the rest are consumed by random().
|
||||||
|
seed := append([]byte{0x01}, bytes.Repeat([]byte{0x00}, 9)...)
|
||||||
|
draws := []byte{0x00, 0x01, 0x02, 0x03, 0x00}
|
||||||
|
reader := bytes.NewReader(append(seed, draws...))
|
||||||
|
|
||||||
|
entropy := ulid.Monotonic(reader, 2)
|
||||||
|
|
||||||
|
prev, err := ulid.New(123, entropy)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// masked 0 and 1 are accepted as increments 1 and 2; 2 and 3 are redrawn
|
||||||
|
wantDeltas := []uint64{1, 2, 1}
|
||||||
|
|
||||||
|
for i, want := range wantDeltas {
|
||||||
|
next, err := ulid.New(123, entropy)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("draw %d: %v", i, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if prev.Compare(next) >= 0 {
|
||||||
|
t.Fatalf("draw %d: monotonicity violated: prev=%v next=%v", i, prev, next)
|
||||||
|
}
|
||||||
|
|
||||||
|
delta := new(big.Int).Sub(
|
||||||
|
new(big.Int).SetBytes(next.Entropy()),
|
||||||
|
new(big.Int).SetBytes(prev.Entropy()),
|
||||||
|
).Uint64()
|
||||||
|
if delta != want {
|
||||||
|
t.Fatalf("draw %d: delta = %d, want %d", i, delta, want)
|
||||||
|
}
|
||||||
|
|
||||||
|
prev = next
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestMonotonicOverflow(t *testing.T) {
|
func TestMonotonicOverflow(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user