use cryptographic random for determining skipped packet numbers

Author: marten-seemann

internal/ackhandler/packet_number_generator.go

@@ -3,7 +3,6 @@ package ackhandler
 import (
 	"crypto/rand"
 	"encoding/binary"
-	mrand "math/rand"
 
 	"github.com/lucas-clemente/quic-go/internal/protocol"
 	"github.com/lucas-clemente/quic-go/internal/utils"
@@ -34,25 +33,45 @@ func (p *sequentialPacketNumberGenerator) Pop() protocol.PacketNumber {
 	return next
 }
 
+type rng struct {
+	buf [4]byte
+}
+
+func (r *rng) Int31() int32 {
+	rand.Read(r.buf[:])
+	return int32(binary.BigEndian.Uint32(r.buf[:]) & ^uint32(1<<31))
+}
+
+// copied from the standard library math/rand implementation of Int63n
+func (r *rng) Int31n(n int32) int32 {
+	if n&(n-1) == 0 { // n is power of two, can mask
+		return r.Int31() & (n - 1)
+	}
+	max := int32((1 << 31) - 1 - (1<<31)%uint32(n))
+	v := r.Int31()
+	for v > max {
+		v = r.Int31()
+	}
+	return v % n
+}
+
 // The skippingPacketNumberGenerator generates the packet number for the next packet
 // it randomly skips a packet number every averagePeriod packets (on average).
 // It is guaranteed to never skip two consecutive packet numbers.
 type skippingPacketNumberGenerator struct {
-	rand      *mrand.Rand
 	period    protocol.PacketNumber
 	maxPeriod protocol.PacketNumber
 
 	next       protocol.PacketNumber
 	nextToSkip protocol.PacketNumber
+
+	rng rng
 }
 
 var _ packetNumberGenerator = &skippingPacketNumberGenerator{}
 
 func newSkippingPacketNumberGenerator(initial, initialPeriod, maxPeriod protocol.PacketNumber) packetNumberGenerator {
-	b := make([]byte, 8)
-	rand.Read(b) // it's not the end of the world if we don't get perfect random here
 	g := &skippingPacketNumberGenerator{
-		rand:      mrand.New(mrand.NewSource(int64(binary.LittleEndian.Uint64(b)))),
 		next:      initial,
 		period:    initialPeriod,
 		maxPeriod: maxPeriod,
@@ -77,6 +96,6 @@ func (p *skippingPacketNumberGenerator) Pop() protocol.PacketNumber {
 
 func (p *skippingPacketNumberGenerator) generateNewSkip() {
 	// make sure that there are never two consecutive packet numbers that are skipped
-	p.nextToSkip = p.next + 2 + protocol.PacketNumber(p.rand.Int63n(int64(2*p.period)))
+	p.nextToSkip = p.next + 2 + protocol.PacketNumber(p.rng.Int31n(int32(2*p.period)))
 	p.period = utils.MinPacketNumber(2*p.period, p.maxPeriod)
 }

internal/ackhandler/packet_number_generator_test.go

@@ -2,6 +2,7 @@ package ackhandler
 
 import (
 	"fmt"
+	"math"
 
 	"github.com/lucas-clemente/quic-go/internal/protocol"
 
@@ -27,6 +28,10 @@ var _ = Describe("Skipping Packet Number Generator", func() {
 	const initialPeriod protocol.PacketNumber = 25
 	const maxPeriod protocol.PacketNumber = 300
 
+	It("uses a maximum period that is sufficiently small such that using a 32-bit random number is ok", func() {
+		Expect(2 * protocol.SkipPacketMaxPeriod).To(BeNumerically("<", math.MaxInt32))
+	})
+
 	It("can be initialized to return any first packet number", func() {
 		png := newSkippingPacketNumberGenerator(12345, initialPeriod, maxPeriod)
 		Expect(png.Pop()).To(Equal(protocol.PacketNumber(12345)))