remove entropy from packet packer

packet_packer.go

@@ -8,14 +8,12 @@ import (
 	"github.com/lucas-clemente/quic-go/frames"
 	"github.com/lucas-clemente/quic-go/handshake"
 	"github.com/lucas-clemente/quic-go/protocol"
-	"github.com/lucas-clemente/quic-go/utils"
 )
 
 type packedPacket struct {
-	number     protocol.PacketNumber
+	number protocol.PacketNumber
-	entropyBit bool
+	raw    []byte
-	raw        []byte
+	frames []frames.Frame
-	frames     []frames.Frame
 }
 
 type packetPacker struct {
@@ -23,7 +21,6 @@ type packetPacker struct {
 	version      protocol.VersionNumber
 	cryptoSetup  *handshake.CryptoSetup
 
-	lastPacketNumber      protocol.PacketNumber // TODO: remove when dropping support for QUIC 33
 	packetNumberGenerator *packetNumberGenerator
 
 	connectionParametersManager *handshake.ConnectionParametersManager
@@ -61,12 +58,7 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, con
 		p.controlFrames = append(p.controlFrames, controlFrames...)
 	}
 
-	var currentPacketNumber protocol.PacketNumber
+	currentPacketNumber := p.packetNumberGenerator.Peek()
-	if p.version <= protocol.Version33 {
-		currentPacketNumber = p.lastPacketNumber + 1
-	} else {
-		currentPacketNumber = p.packetNumberGenerator.Peek()
-	}
 
 	// cryptoSetup needs to be locked here, so that the AEADs are not changed between
 	// calling DiversificationNonce() and Seal().
@@ -132,20 +124,6 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, con
 
 	payloadStartIndex := buffer.Len()
 
-	// set entropy bit in Private Header, for QUIC version < 34
-	var entropyBit bool
-	if p.version < protocol.Version34 {
-		entropyBit, err = utils.RandomBit()
-		if err != nil {
-			return nil, err
-		}
-		if entropyBit {
-			buffer.WriteByte(1)
-		} else {
-			buffer.WriteByte(0)
-		}
-	}
-
 	for _, frame := range payloadFrames {
 		err := frame.Write(buffer, p.version)
 		if err != nil {
@@ -161,20 +139,15 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, con
 	p.cryptoSetup.Seal(raw[payloadStartIndex:payloadStartIndex], raw[payloadStartIndex:], currentPacketNumber, raw[:payloadStartIndex])
 	raw = raw[0 : buffer.Len()+12]
 
-	if p.version <= protocol.Version33 {
+	num := p.packetNumberGenerator.Pop()
-		p.lastPacketNumber++
+	if num != currentPacketNumber {
-	} else {
+		return nil, errors.New("PacketPacker BUG: Peeked and Popped packet numbers do not match.")
-		num := p.packetNumberGenerator.Pop()
-		if num != currentPacketNumber {
-			return nil, errors.New("PacketPacker BUG: Peeked and Popped packet numbers do not match.")
-		}
 	}
 
 	return &packedPacket{
-		number:     currentPacketNumber,
+		number: currentPacketNumber,
-		entropyBit: entropyBit,
+		raw:    raw,
-		raw:        raw,
+		frames: payloadFrames,
-		frames:     payloadFrames,
 	}, nil
 }
 

packet_packer_test.go

@@ -35,39 +35,12 @@ var _ = Describe("Packet packer", func() {
 		packer.version = protocol.Version34
 	})
 
-	AfterEach(func() {
-		packer.lastPacketNumber = 0
-	})
-
 	It("returns nil when no packet is queued", func() {
 		p, err := packer.PackPacket(nil, []frames.Frame{}, 0, true)
 		Expect(p).To(BeNil())
 		Expect(err).ToNot(HaveOccurred())
 	})
 
-	It("doesn't set a private header for QUIC version >= 34", func() {
-		// This is not trivial to test, since PackPacket() already encrypts the packet
-		// So pack the packet for QUIC 33, then for QUIC 34. The packet for QUIC 33 should be 1 byte longer, since it contains the Private Header
-		f := &frames.StreamFrame{
-			StreamID: 5,
-			Data:     []byte("foobar"),
-		}
-		// pack the packet for QUIC version 33
-		packer.version = protocol.Version33
-		streamFramer.AddFrameForRetransmission(f)
-		p33, err := packer.PackPacket(nil, []frames.Frame{}, 0, true)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(p33).ToNot(BeNil())
-		// pack the packet for QUIC version 34
-		packer.version = protocol.Version34
-		streamFramer.AddFrameForRetransmission(f)
-		p34, err := packer.PackPacket(nil, []frames.Frame{}, 0, true)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(p34).ToNot(BeNil())
-		Expect(p34.entropyBit).To(BeFalse())
-		Expect(p34.raw).To(HaveLen(len(p33.raw) - 1))
-	})
-
 	It("packs single packets", func() {
 		f := &frames.StreamFrame{
 			StreamID: 5,
@@ -134,9 +107,8 @@ var _ = Describe("Packet packer", func() {
 	})
 
 	It("sets the LeastUnackedDelta length of a StopWaitingFrame", func() {
-		packer.version = protocol.Version33             // TODO: find a different way to test this when dropping support for QUIC 33
 		packetNumber := protocol.PacketNumber(0xDECAFB) // will result in a 4 byte packet number
-		packer.lastPacketNumber = packetNumber - 1
+		packer.packetNumberGenerator.next = packetNumber
 		swf := &frames.StopWaitingFrame{LeastUnacked: packetNumber - 0x100}
 		p, err := packer.PackPacket(swf, []frames.Frame{&frames.ConnectionCloseFrame{}}, 0, true)
 		Expect(err).ToNot(HaveOccurred())
@@ -194,30 +166,7 @@ var _ = Describe("Packet packer", func() {
 		Expect(payloadFrames).To(HaveLen(10))
 	})
 
-	// TODO: remove once we drop support for QUIC 33
+	It("only increases the packet number when there is an actual packet to send", func() {
-	It("only increases the packet number when there is an actual packet to send, in QUIC 33", func() {
-		packer.version = protocol.Version33
-		f := &frames.StreamFrame{
-			StreamID: 5,
-			Data:     []byte{0xDE, 0xCA, 0xFB, 0xAD},
-		}
-		streamFramer.AddFrameForRetransmission(f)
-		p, err := packer.PackPacket(nil, []frames.Frame{}, 0, true)
-		Expect(p).ToNot(BeNil())
-		Expect(err).ToNot(HaveOccurred())
-		Expect(packer.lastPacketNumber).To(Equal(protocol.PacketNumber(1)))
-		p, err = packer.PackPacket(nil, []frames.Frame{}, 0, true)
-		Expect(p).To(BeNil())
-		Expect(err).ToNot(HaveOccurred())
-		Expect(packer.lastPacketNumber).To(Equal(protocol.PacketNumber(1)))
-		streamFramer.AddFrameForRetransmission(f)
-		p, err = packer.PackPacket(nil, []frames.Frame{}, 0, true)
-		Expect(p).ToNot(BeNil())
-		Expect(err).ToNot(HaveOccurred())
-		Expect(packer.lastPacketNumber).To(Equal(protocol.PacketNumber(2)))
-	})
-
-	It("only increases the packet number when there is an actual packet to send, in QUIC 34", func() {
 		packer.packetNumberGenerator.nextToSkip = 1000
 		p, err := packer.PackPacket(nil, []frames.Frame{}, 0, true)
 		Expect(p).To(BeNil())
@@ -312,19 +261,6 @@ var _ = Describe("Packet packer", func() {
 			Expect(p.raw).To(ContainSubstring(string(f3.Data)))
 		})
 
-		It("packs a packet with a stream frame larger than maximum size, in QUIC < 34", func() {
-			packer.version = protocol.Version33
-			f := &frames.StreamFrame{
-				StreamID: 5,
-				Offset:   1,
-				Data:     bytes.Repeat([]byte{'f'}, int(protocol.MaxPacketSize)+100),
-			}
-			streamFramer.AddFrameForRetransmission(f)
-			p, err := packer.PackPacket(nil, []frames.Frame{}, 0, true)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(p.raw).To(HaveLen(int(protocol.MaxPacketSize)))
-		})
-
 		It("splits one stream frame larger than maximum size", func() {
 			f := &frames.StreamFrame{
 				StreamID: 7,

utils/utils.go

@@ -2,7 +2,6 @@ package utils
 
 import (
 	"bytes"
-	"crypto/rand"
 	"io"
 
 	"github.com/lucas-clemente/quic-go/protocol"
@@ -163,13 +162,6 @@ func WriteUint16(b *bytes.Buffer, i uint16) {
 	b.WriteByte(uint8(i >> 8))
 }
 
-// RandomBit returns a cryptographically secure random bit (encoded as true / false)
-func RandomBit() (bool, error) {
-	b := make([]byte, 1)
-	_, err := rand.Read(b)
-	return b[0]%2 == 0, err
-}
-
 // Uint32Slice attaches the methods of sort.Interface to []uint32, sorting in increasing order.
 type Uint32Slice []uint32
 

utils/utils_test.go

@@ -174,24 +174,6 @@ var _ = Describe("Utils", func() {
 		})
 	})
 
-	Context("Rand", func() {
-		It("returns either true or false", func() {
-			countTrue := 0
-			countFalse := 0
-			for i := 0; i < 100; i++ {
-				val, err := RandomBit()
-				Expect(err).NotTo(HaveOccurred())
-				if val {
-					countTrue++
-				} else {
-					countFalse++
-				}
-			}
-			Expect(countTrue).ToNot(BeZero())
-			Expect(countFalse).ToNot(BeZero())
-		})
-	})
-
 	Context("ReadUintN", func() {
 		It("reads n bytes", func() {
 			m := map[uint8]uint64{