fix timing side channel when reading headers with invalid reserved bits

This commit is contained in:
Marten Seemann 2019-01-28 11:10:50 +09:00
parent 4fe0f6752c
commit 7b9b711a77
4 changed files with 105 additions and 26 deletions

View file

@ -10,6 +10,12 @@ import (
"github.com/lucas-clemente/quic-go/internal/utils"
)
// ErrInvalidReservedBits is returned when the reserved bits are incorrect.
// When this error is returned, parsing continues, and an ExtendedHeader is returned.
// This is necessary because we need to decrypt the packet in that case,
// in order to avoid a timing side-channel.
var ErrInvalidReservedBits = errors.New("invalid reserved bits")
// ExtendedHeader is the header of a QUIC packet.
type ExtendedHeader struct {
Header
@ -39,20 +45,17 @@ func (h *ExtendedHeader) parse(b *bytes.Reader, v protocol.VersionNumber) (*Exte
}
func (h *ExtendedHeader) parseLongHeader(b *bytes.Reader, v protocol.VersionNumber) (*ExtendedHeader, error) {
if h.typeByte&0xc != 0 {
return nil, errors.New("5th and 6th bit must be 0")
}
if err := h.readPacketNumber(b); err != nil {
return nil, err
}
return h, nil
var err error
if h.typeByte&0xc != 0 {
err = ErrInvalidReservedBits
}
return h, err
}
func (h *ExtendedHeader) parseShortHeader(b *bytes.Reader, v protocol.VersionNumber) (*ExtendedHeader, error) {
if h.typeByte&0x18 != 0 {
return nil, errors.New("4th and 5th bit must be 0")
}
h.KeyPhase = protocol.KeyPhaseZero
if h.typeByte&0x4 > 0 {
h.KeyPhase = protocol.KeyPhaseOne
@ -61,7 +64,11 @@ func (h *ExtendedHeader) parseShortHeader(b *bytes.Reader, v protocol.VersionNum
if err := h.readPacketNumber(b); err != nil {
return nil, err
}
return h, nil
var err error
if h.typeByte&0x18 != 0 {
err = ErrInvalidReservedBits
}
return h, err
}
func (h *ExtendedHeader) readPacketNumber(b *bytes.Reader) error {

View file

@ -291,15 +291,18 @@ var _ = Describe("Header Parsing", func() {
})
It("errors if the 5th or 6th bit are set", func() {
data := []byte{0xc0 | 0x2<<4 /* set the 5th bit */ | 0x8}
data := []byte{0xc0 | 0x2<<4 | 0x8 /* set the 5th bit */ | 0x1 /* 2 byte packet number */}
data = appendVersion(data, versionIETFFrames)
data = append(data, 0x0) // connection ID lengths
data = append(data, encodeVarInt(0)...) // length
data = append(data, 0x0) // connection ID lengths
data = append(data, encodeVarInt(2)...) // length
data = append(data, []byte{0x12, 0x34}...) // packet number
hdr, _, _, err := ParsePacket(data, 0)
Expect(err).ToNot(HaveOccurred())
Expect(hdr.Type).To(Equal(protocol.PacketTypeHandshake))
_, err = hdr.ParseExtended(bytes.NewReader(data), versionIETFFrames)
Expect(err).To(MatchError("5th and 6th bit must be 0"))
extHdr, err := hdr.ParseExtended(bytes.NewReader(data), versionIETFFrames)
Expect(err).To(MatchError(ErrInvalidReservedBits))
Expect(extHdr).ToNot(BeNil())
Expect(extHdr.PacketNumber).To(Equal(protocol.PacketNumber(0x1234)))
})
It("errors on EOF, when parsing the header", func() {
@ -443,11 +446,14 @@ var _ = Describe("Header Parsing", func() {
It("errors if the 4th or 5th bit are set", func() {
connID := protocol.ConnectionID{1, 2, 3, 4, 5}
data := append([]byte{0x40 | 0x10 /* set the 4th bit */}, connID...)
data = append(data, 0x42) // packet number
hdr, _, _, err := ParsePacket(data, 5)
Expect(err).ToNot(HaveOccurred())
Expect(hdr.IsLongHeader).To(BeFalse())
_, err = hdr.ParseExtended(bytes.NewReader(data), versionIETFFrames)
Expect(err).To(MatchError("4th and 5th bit must be 0"))
extHdr, err := hdr.ParseExtended(bytes.NewReader(data), versionIETFFrames)
Expect(err).To(MatchError(ErrInvalidReservedBits))
Expect(extHdr).ToNot(BeNil())
Expect(extHdr.PacketNumber).To(Equal(protocol.PacketNumber(0x42)))
})
It("reads a Short Header with a 5 byte connection ID", func() {

View file

@ -91,28 +91,40 @@ func (u *packetUnpacker) Unpack(hdr *wire.Header, data []byte) (*unpackedPacket,
}
func (u *packetUnpacker) unpackLongHeaderPacket(opener handshake.LongHeaderOpener, hdr *wire.Header, data []byte) (*wire.ExtendedHeader, []byte, error) {
extHdr, err := u.unpack(opener, hdr, data)
if err != nil {
return nil, nil, err
extHdr, parseErr := u.unpack(opener, hdr, data)
// If the reserved bits are set incorrectly, we still need to continue unpacking.
// This avoids a timing side-channel, which otherwise might allow an attacker
// to gain information about the header encryption.
if parseErr != nil && parseErr != wire.ErrInvalidReservedBits {
return nil, nil, fmt.Errorf("error parsing extended header: %s", parseErr)
}
extHdrLen := extHdr.GetLength(u.version)
decrypted, err := opener.Open(data[extHdrLen:extHdrLen], data[extHdrLen:], extHdr.PacketNumber, data[:extHdrLen])
if err != nil {
return nil, nil, err
}
if parseErr != nil {
return nil, nil, parseErr
}
return extHdr, decrypted, nil
}
func (u *packetUnpacker) unpackShortHeaderPacket(opener handshake.ShortHeaderOpener, hdr *wire.Header, data []byte) (*wire.ExtendedHeader, []byte, error) {
extHdr, err := u.unpack(opener, hdr, data)
if err != nil {
return nil, nil, err
extHdr, parseErr := u.unpack(opener, hdr, data)
// If the reserved bits are set incorrectly, we still need to continue unpacking.
// This avoids a timing side-channel, which otherwise might allow an attacker
// to gain information about the header encryption.
if parseErr != nil && parseErr != wire.ErrInvalidReservedBits {
return nil, nil, parseErr
}
extHdrLen := extHdr.GetLength(u.version)
decrypted, err := opener.Open(data[extHdrLen:extHdrLen], data[extHdrLen:], extHdr.PacketNumber, extHdr.KeyPhase, data[:extHdrLen])
if err != nil {
return nil, nil, err
}
if parseErr != nil {
return nil, nil, parseErr
}
return extHdr, decrypted, nil
}
@ -134,9 +146,9 @@ func (u *packetUnpacker) unpack(hd headerDecryptor, hdr *wire.Header, data []byt
data[hdrLen:hdrLen+4],
)
// 3. parse the header (and learn the actual length of the packet number)
extHdr, err := hdr.ParseExtended(r, u.version)
if err != nil {
return nil, fmt.Errorf("error parsing extended header: %s", err)
extHdr, parseErr := hdr.ParseExtended(r, u.version)
if parseErr != nil && parseErr != wire.ErrInvalidReservedBits {
return nil, parseErr
}
// 4. if the packet number is shorter than 4 bytes, replace the remaining bytes with the copy we saved earlier
if extHdr.PacketNumberLen != protocol.PacketNumberLen4 {
@ -148,5 +160,5 @@ func (u *packetUnpacker) unpack(hd headerDecryptor, hdr *wire.Header, data []byt
u.largestRcvdPacketNumber,
extHdr.PacketNumber,
)
return extHdr, nil
return extHdr, parseErr
}

View file

@ -110,6 +110,60 @@ var _ = Describe("Packet Unpacker", func() {
Expect(err).To(MatchError("test err"))
})
It("defends against the timing side-channel when the reserved bits are wrong, for long header packets", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{
IsLongHeader: true,
Type: protocol.PacketTypeHandshake,
DestConnectionID: connID,
Version: version,
},
PacketNumber: 0x1337,
PacketNumberLen: 2,
}
hdr, hdrRaw := getHeader(extHdr)
hdrRaw[0] |= 0xc
opener := mocks.NewMockLongHeaderOpener(mockCtrl)
opener.EXPECT().DecryptHeader(gomock.Any(), gomock.Any(), gomock.Any())
cs.EXPECT().GetHandshakeOpener().Return(opener, nil)
opener.EXPECT().Open(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte("payload"), nil)
_, err := unpacker.Unpack(hdr, append(hdrRaw, payload...))
Expect(err).To(MatchError(wire.ErrInvalidReservedBits))
})
It("defends against the timing side-channel when the reserved bits are wrong, for short header packets", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: connID},
PacketNumber: 0x1337,
PacketNumberLen: 2,
}
hdr, hdrRaw := getHeader(extHdr)
hdrRaw[0] |= 0x18
opener := mocks.NewMockShortHeaderOpener(mockCtrl)
opener.EXPECT().DecryptHeader(gomock.Any(), gomock.Any(), gomock.Any())
cs.EXPECT().Get1RTTOpener().Return(opener, nil)
opener.EXPECT().Open(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte("payload"), nil)
_, err := unpacker.Unpack(hdr, append(hdrRaw, payload...))
Expect(err).To(MatchError(wire.ErrInvalidReservedBits))
})
It("returns the decryption error, when unpacking a packet with wrong reserved bits fails", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: connID},
PacketNumber: 0x1337,
PacketNumberLen: 2,
}
hdr, hdrRaw := getHeader(extHdr)
hdrRaw[0] |= 0x18
opener := mocks.NewMockShortHeaderOpener(mockCtrl)
opener.EXPECT().DecryptHeader(gomock.Any(), gomock.Any(), gomock.Any())
cs.EXPECT().Get1RTTOpener().Return(opener, nil)
testErr := errors.New("decryption error")
opener.EXPECT().Open(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, testErr)
_, err := unpacker.Unpack(hdr, append(hdrRaw, payload...))
Expect(err).To(MatchError(testErr))
})
It("decrypts the header", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{