diff --git a/packet_packer.go b/packet_packer.go index 5d96737b5..430c943b0 100644 --- a/packet_packer.go +++ b/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 - entropyBit bool - raw []byte - frames []frames.Frame + number protocol.PacketNumber + raw []byte + 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 - if p.version <= protocol.Version33 { - currentPacketNumber = p.lastPacketNumber + 1 - } else { - currentPacketNumber = p.packetNumberGenerator.Peek() - } + 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 { - p.lastPacketNumber++ - } else { - num := p.packetNumberGenerator.Pop() - if num != currentPacketNumber { - 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, - entropyBit: entropyBit, - raw: raw, - frames: payloadFrames, + number: currentPacketNumber, + raw: raw, + frames: payloadFrames, }, nil } diff --git a/packet_packer_test.go b/packet_packer_test.go index 8675ae737..a31081bca 100644 --- a/packet_packer_test.go +++ b/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, 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() { + It("only increases the packet number when there is an actual packet to send", 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, diff --git a/utils/utils.go b/utils/utils.go index 197d59237..321b80850 100644 --- a/utils/utils.go +++ b/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 diff --git a/utils/utils_test.go b/utils/utils_test.go index 753188a20..cb4b8270a 100644 --- a/utils/utils_test.go +++ b/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{