Merge pull request #3534 from lucas-clemente/header-parsing

introduce a separate code paths for Short Header packet handling

internal/mocks/logging/connection_tracer.go

@@ -183,16 +183,16 @@ func (mr *MockConnectionTracerMockRecorder) NegotiatedVersion(arg0, arg1, arg2 i
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NegotiatedVersion", reflect.TypeOf((*MockConnectionTracer)(nil).NegotiatedVersion), arg0, arg1, arg2)
 }
 
-// ReceivedPacket mocks base method.
-func (m *MockConnectionTracer) ReceivedPacket(arg0 *wire.ExtendedHeader, arg1 protocol.ByteCount, arg2 []logging.Frame) {
+// ReceivedLongHeaderPacket mocks base method.
+func (m *MockConnectionTracer) ReceivedLongHeaderPacket(arg0 *wire.ExtendedHeader, arg1 protocol.ByteCount, arg2 []logging.Frame) {
 	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "ReceivedPacket", arg0, arg1, arg2)
+	m.ctrl.Call(m, "ReceivedLongHeaderPacket", arg0, arg1, arg2)
 }
 
-// ReceivedPacket indicates an expected call of ReceivedPacket.
-func (mr *MockConnectionTracerMockRecorder) ReceivedPacket(arg0, arg1, arg2 interface{}) *gomock.Call {
+// ReceivedLongHeaderPacket indicates an expected call of ReceivedLongHeaderPacket.
+func (mr *MockConnectionTracerMockRecorder) ReceivedLongHeaderPacket(arg0, arg1, arg2 interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceivedPacket", reflect.TypeOf((*MockConnectionTracer)(nil).ReceivedPacket), arg0, arg1, arg2)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceivedLongHeaderPacket", reflect.TypeOf((*MockConnectionTracer)(nil).ReceivedLongHeaderPacket), arg0, arg1, arg2)
 }
 
 // ReceivedRetry mocks base method.
@@ -207,6 +207,18 @@ func (mr *MockConnectionTracerMockRecorder) ReceivedRetry(arg0 interface{}) *gom
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceivedRetry", reflect.TypeOf((*MockConnectionTracer)(nil).ReceivedRetry), arg0)
 }
 
+// ReceivedShortHeaderPacket mocks base method.
+func (m *MockConnectionTracer) ReceivedShortHeaderPacket(arg0 *wire.ShortHeader, arg1 protocol.ByteCount, arg2 []logging.Frame) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "ReceivedShortHeaderPacket", arg0, arg1, arg2)
+}
+
+// ReceivedShortHeaderPacket indicates an expected call of ReceivedShortHeaderPacket.
+func (mr *MockConnectionTracerMockRecorder) ReceivedShortHeaderPacket(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReceivedShortHeaderPacket", reflect.TypeOf((*MockConnectionTracer)(nil).ReceivedShortHeaderPacket), arg0, arg1, arg2)
+}
+
 // ReceivedTransportParameters mocks base method.
 func (m *MockConnectionTracer) ReceivedTransportParameters(arg0 *wire.TransportParameters) {
 	m.ctrl.T.Helper()

internal/utils/byteoder_big_endian_test.go

@@ -9,6 +9,23 @@ import (
 )
 
 var _ = Describe("Big Endian encoding / decoding", func() {
+	Context("converting", func() {
+		It("Uint16", func() {
+			b := []byte{0x13, 0x37}
+			Expect(BigEndian.Uint16(b)).To(Equal(uint16(0x1337)))
+		})
+
+		It("Uint24", func() {
+			b := []byte{0x13, 0x99, 0x37}
+			Expect(BigEndian.Uint24(b)).To(Equal(uint32(0x139937)))
+		})
+
+		It("Uint32", func() {
+			b := []byte{0xde, 0xad, 0xbe, 0xef}
+			Expect(BigEndian.Uint32(b)).To(Equal(uint32(0xdeadbeef)))
+		})
+	})
+
 	Context("ReadUint16", func() {
 		It("reads a big endian", func() {
 			b := []byte{0x13, 0xEF}

internal/utils/byteorder.go

@@ -7,6 +7,10 @@ import (
 
 // A ByteOrder specifies how to convert byte sequences into 16-, 32-, or 64-bit unsigned integers.
 type ByteOrder interface {
+	Uint32([]byte) uint32
+	Uint24([]byte) uint32
+	Uint16([]byte) uint16
+
 	ReadUint32(io.ByteReader) (uint32, error)
 	ReadUint24(io.ByteReader) (uint32, error)
 	ReadUint16(io.ByteReader) (uint16, error)

internal/utils/byteorder_big_endian.go

@@ -2,6 +2,7 @@ package utils
 
 import (
 	"bytes"
+	"encoding/binary"
 	"io"
 )
 
@@ -73,6 +74,19 @@ func (bigEndian) ReadUint16(b io.ByteReader) (uint16, error) {
 	return uint16(b1) + uint16(b2)<<8, nil
 }
 
+func (bigEndian) Uint32(b []byte) uint32 {
+	return binary.BigEndian.Uint32(b)
+}
+
+func (bigEndian) Uint24(b []byte) uint32 {
+	_ = b[2] // bounds check hint to compiler; see golang.org/issue/14808
+	return uint32(b[2]) | uint32(b[1])<<8 | uint32(b[0])<<16
+}
+
+func (bigEndian) Uint16(b []byte) uint16 {
+	return binary.BigEndian.Uint16(b)
+}
+
 // WriteUint32 writes a uint32
 func (bigEndian) WriteUint32(b *bytes.Buffer, i uint32) {
 	b.Write([]byte{uint8(i >> 24), uint8(i >> 16), uint8(i >> 8), uint8(i)})

internal/wire/short_header.go

@@ -0,0 +1,73 @@
+package wire
+
+import (
+	"errors"
+	"fmt"
+	"io"
+
+	"github.com/lucas-clemente/quic-go/internal/protocol"
+	"github.com/lucas-clemente/quic-go/internal/utils"
+)
+
+type ShortHeader struct {
+	DestConnectionID protocol.ConnectionID
+	PacketNumber     protocol.PacketNumber
+	PacketNumberLen  protocol.PacketNumberLen
+	KeyPhase         protocol.KeyPhaseBit
+}
+
+func ParseShortHeader(data []byte, connIDLen int) (*ShortHeader, error) {
+	if len(data) == 0 {
+		return nil, io.EOF
+	}
+	if data[0]&0x80 > 0 {
+		return nil, errors.New("not a short header packet")
+	}
+	if data[0]&0x40 == 0 {
+		return nil, errors.New("not a QUIC packet")
+	}
+	pnLen := protocol.PacketNumberLen(data[0]&0b11) + 1
+	if len(data) < 1+int(pnLen)+connIDLen {
+		return nil, io.EOF
+	}
+	destConnID := protocol.ParseConnectionID(data[1 : 1+connIDLen])
+
+	pos := 1 + connIDLen
+	var pn protocol.PacketNumber
+	switch pnLen {
+	case protocol.PacketNumberLen1:
+		pn = protocol.PacketNumber(data[pos])
+	case protocol.PacketNumberLen2:
+		pn = protocol.PacketNumber(utils.BigEndian.Uint16(data[pos : pos+2]))
+	case protocol.PacketNumberLen3:
+		pn = protocol.PacketNumber(utils.BigEndian.Uint24(data[pos : pos+3]))
+	case protocol.PacketNumberLen4:
+		pn = protocol.PacketNumber(utils.BigEndian.Uint32(data[pos : pos+4]))
+	default:
+		return nil, fmt.Errorf("invalid packet number length: %d", pnLen)
+	}
+	kp := protocol.KeyPhaseZero
+	if data[0]&0b100 > 0 {
+		kp = protocol.KeyPhaseOne
+	}
+
+	var err error
+	if data[0]&0x18 != 0 {
+		err = ErrInvalidReservedBits
+	}
+	return &ShortHeader{
+		DestConnectionID: destConnID,
+		PacketNumber:     pn,
+		PacketNumberLen:  pnLen,
+		KeyPhase:         kp,
+	}, err
+}
+
+func (h *ShortHeader) Len() protocol.ByteCount {
+	return 1 + protocol.ByteCount(h.DestConnectionID.Len()) + protocol.ByteCount(h.PacketNumberLen)
+}
+
+// Log logs the Header
+func (h *ShortHeader) Log(logger utils.Logger) {
+	logger.Debugf("\tShort Header{DestConnectionID: %s, PacketNumber: %d, PacketNumberLen: %d, KeyPhase: %s}", h.DestConnectionID, h.PacketNumber, h.PacketNumberLen, h.KeyPhase)
+}

internal/wire/short_header_test.go

@@ -0,0 +1,110 @@
+package wire
+
+import (
+	"bytes"
+	"io"
+	"log"
+	"os"
+
+	"github.com/lucas-clemente/quic-go/internal/protocol"
+	"github.com/lucas-clemente/quic-go/internal/utils"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Short Header", func() {
+	Context("Parsing", func() {
+		It("parses", func() {
+			data := []byte{
+				0b01000110,
+				0xde, 0xad, 0xbe, 0xef,
+				0x13, 0x37, 0x99,
+			}
+			hdr, err := ParseShortHeader(data, 4)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(hdr.DestConnectionID).To(Equal(protocol.ParseConnectionID([]byte{0xde, 0xad, 0xbe, 0xef})))
+			Expect(hdr.KeyPhase).To(Equal(protocol.KeyPhaseOne))
+			Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x133799)))
+			Expect(hdr.PacketNumberLen).To(Equal(protocol.PacketNumberLen3))
+		})
+
+		It("errors when the QUIC bit is not set", func() {
+			data := []byte{
+				0b00000101,
+				0xde, 0xad, 0xbe, 0xef,
+				0x13, 0x37,
+			}
+			_, err := ParseShortHeader(data, 4)
+			Expect(err).To(MatchError("not a QUIC packet"))
+		})
+
+		It("errors, but returns the header, when the reserved bits are set", func() {
+			data := []byte{
+				0b01010101,
+				0xde, 0xad, 0xbe, 0xef,
+				0x13, 0x37,
+			}
+			hdr, err := ParseShortHeader(data, 4)
+			Expect(err).To(MatchError(ErrInvalidReservedBits))
+			Expect(hdr).ToNot(BeNil())
+			Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x1337)))
+		})
+
+		It("errors when passed a long header packet", func() {
+			_, err := ParseShortHeader([]byte{0x80}, 4)
+			Expect(err).To(MatchError("not a short header packet"))
+		})
+
+		It("errors on EOF", func() {
+			data := []byte{
+				0b01000110,
+				0xde, 0xad, 0xbe, 0xef,
+				0x13, 0x37, 0x99,
+			}
+			_, err := ParseShortHeader(data, 4)
+			Expect(err).ToNot(HaveOccurred())
+			for i := range data {
+				_, err := ParseShortHeader(data[:i], 4)
+				Expect(err).To(MatchError(io.EOF))
+			}
+		})
+	})
+
+	Context("logging", func() {
+		var (
+			buf    *bytes.Buffer
+			logger utils.Logger
+		)
+
+		BeforeEach(func() {
+			buf = &bytes.Buffer{}
+			logger = utils.DefaultLogger
+			logger.SetLogLevel(utils.LogLevelDebug)
+			log.SetOutput(buf)
+		})
+
+		AfterEach(func() {
+			log.SetOutput(os.Stdout)
+		})
+
+		It("logs Short Headers containing a connection ID", func() {
+			(&ShortHeader{
+				DestConnectionID: protocol.ParseConnectionID([]byte{0xde, 0xad, 0xbe, 0xef, 0xca, 0xfe, 0x13, 0x37}),
+				KeyPhase:         protocol.KeyPhaseOne,
+				PacketNumber:     1337,
+				PacketNumberLen:  4,
+			}).Log(logger)
+			Expect(buf.String()).To(ContainSubstring("Short Header{DestConnectionID: deadbeefcafe1337, PacketNumber: 1337, PacketNumberLen: 4, KeyPhase: 1}"))
+		})
+	})
+
+	It("determines the length", func() {
+		Expect((&ShortHeader{
+			DestConnectionID: protocol.ParseConnectionID([]byte{0xde, 0xaf}),
+			PacketNumber:     0x1337,
+			PacketNumberLen:  protocol.PacketNumberLen3,
+			KeyPhase:         protocol.KeyPhaseOne,
+		}).Len()).To(Equal(protocol.ByteCount(1 + 2 + 3)))
+	})
+})