mirrors/uquic
1
0
Fork 0
mirror of https://github.com/refraction-networking/uquic.git synced 2025-04-03 04:07:35 +03:00
Code Issues Projects Releases Packages Wiki Activity

only queue packets for decryption if the opener is not yet available

Browse source
This commit is contained in:
Marten Seemann 2018-12-20 15:17:18 +06:30
parent 605846cfd8
commit d6c304610d
5 changed files with 59 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

9
internal/handshake/crypto_setup.go
View file

@ -47,6 +47,11 @@ func (m messageType) String() string {
}
}
// ErrOpenerNotYetAvailable is returned when an opener is requested for an encryption level,
// but the corresponding opener has not yet been initialized
// This can happen when packets arrive out of order.
var ErrOpenerNotYetAvailable = errors.New("CryptoSetup: opener at this encryption level not yet available")
type cryptoSetup struct {
tlsConf *qtls.Config
@ -520,12 +525,12 @@ func (h *cryptoSetup) GetOpener(level protocol.EncryptionLevel) (Opener, error)
return h.initialOpener, nil
case protocol.EncryptionHandshake:
if h.handshakeOpener == nil {
return nil, errors.New("CryptoSetup: no opener with encryption level Handshake")
return nil, ErrOpenerNotYetAvailable
}
return h.handshakeOpener, nil
case protocol.Encryption1RTT:
if h.opener == nil {
return nil, errors.New("CryptoSetup: no opener with encryption level 1-RTT")
return nil, ErrOpenerNotYetAvailable
}
return h.opener, nil
default:

2
packet_unpacker.go
View file

@ -61,7 +61,7 @@ func (u *packetUnpacker) Unpack(hdr *wire.Header, data []byte) (*unpackedPacket,
}
opener, err := u.cs.GetOpener(encLevel)
if err != nil {
return nil, qerr.Error(qerr.DecryptionFailure, err.Error())
return nil, err
}
hdrLen := int(hdr.ParsedLen())
// The packet number can be up to 4 bytes long, but we won't know the length until we decrypt it.

5
packet_unpacker_test.go
View file

@ -5,6 +5,7 @@ import (
"errors"
"github.com/golang/mock/gomock"
"github.com/lucas-clemente/quic-go/internal/handshake"
"github.com/lucas-clemente/quic-go/internal/mocks"
"github.com/lucas-clemente/quic-go/internal/protocol"
"github.com/lucas-clemente/quic-go/internal/qerr"
@ -124,9 +125,9 @@ var _ = Describe("Packet Unpacker", func() {
PacketNumberLen: 2,
}
hdr, hdrRaw := getHeader(extHdr)
cs.EXPECT().GetOpener(protocol.Encryption1RTT).Return(nil, errors.New("test err"))
cs.EXPECT().GetOpener(protocol.Encryption1RTT).Return(nil, handshake.ErrOpenerNotYetAvailable)
_, err := unpacker.Unpack(hdr, hdrRaw)
Expect(err).To(MatchError(qerr.Error(qerr.DecryptionFailure, "test err")))
Expect(err).To(MatchError(handshake.ErrOpenerNotYetAvailable))
})
It("returns the error when unpacking fails", func() {

43
session.go
View file

@ -358,13 +358,10 @@ runLoop:
// We do all the interesting stuff after the switch statement, so
// nothing to see here.
case p := <-s.receivedPackets:
err := s.handlePacketImpl(p)
if err != nil {
if qErr, ok := err.(*qerr.QuicError); ok && qErr.ErrorCode == qerr.DecryptionFailure {
s.tryQueueingUndecryptablePacket(p)
continue
}
s.closeLocal(err)
// Only reset the timers if this packet was actually processed.
// This avoids modifying any state when handling undecryptable packets,
// which could be injected by an attacker.
if wasProcessed := s.handlePacketImpl(p); !wasProcessed {
continue
}
// This is a bit unclean, but works properly, since the packet always
@ -473,18 +470,24 @@ func (s *session) handleHandshakeComplete() {
}
}
func (s *session) handlePacketImpl(p *receivedPacket) error {
func (s *session) handlePacketImpl(p *receivedPacket) bool /* was the packet successfully processed */ {
// The server can change the source connection ID with the first Handshake packet.
// After this, all packets with a different source connection have to be ignored.
if s.receivedFirstPacket && p.hdr.IsLongHeader && !p.hdr.SrcConnectionID.Equal(s.destConnID) {
s.logger.Debugf("Dropping packet with unexpected source connection ID: %s (expected %s)", p.hdr.SrcConnectionID, s.destConnID)
return nil
return false
}
packet, err := s.unpacker.Unpack(p.hdr, p.data)
// if the decryption failed, this might be a packet sent by an attacker
if err != nil {
return err
if err == handshake.ErrOpenerNotYetAvailable {
s.tryQueueingUndecryptablePacket(p)
return false
}
// TODO: don't close the connection when receiving 0-RTT packets
s.closeLocal(err)
return false
}
if s.logger.Debug() {
@ -492,15 +495,23 @@ func (s *session) handlePacketImpl(p *receivedPacket) error {
packet.hdr.Log(s.logger)
}
if err := s.handleUnpackedPacket(packet, p.rcvTime); err != nil {
s.closeLocal(err)
return false
}
return true
}
func (s *session) handleUnpackedPacket(packet *unpackedPacket, rcvTime time.Time) error {
// The server can change the source connection ID with the first Handshake packet.
if s.perspective == protocol.PerspectiveClient && !s.receivedFirstPacket && p.hdr.IsLongHeader && !p.hdr.SrcConnectionID.Equal(s.destConnID) {
s.logger.Debugf("Received first packet. Switching destination connection ID to: %s", p.hdr.SrcConnectionID)
s.destConnID = p.hdr.SrcConnectionID
if s.perspective == protocol.PerspectiveClient && !s.receivedFirstPacket && packet.hdr.IsLongHeader && !packet.hdr.SrcConnectionID.Equal(s.destConnID) {
s.logger.Debugf("Received first packet. Switching destination connection ID to: %s", packet.hdr.SrcConnectionID)
s.destConnID = packet.hdr.SrcConnectionID
s.packer.ChangeDestConnectionID(s.destConnID)
}
s.receivedFirstPacket = true
s.lastNetworkActivityTime = p.rcvTime
s.lastNetworkActivityTime = rcvTime
s.keepAlivePingSent = false
// The client completes the handshake first (after sending the CFIN).
@ -514,9 +525,9 @@ func (s *session) handlePacketImpl(p *receivedPacket) error {
// If this is a Retry packet, there's no need to send an ACK.
// The session will be closed and recreated as soon as the crypto setup processed the HRR.
if p.hdr.Type != protocol.PacketTypeRetry {
if packet.hdr.Type != protocol.PacketTypeRetry {
isRetransmittable := ackhandler.HasRetransmittableFrames(packet.frames)
if err := s.receivedPacketHandler.ReceivedPacket(packet.packetNumber, p.rcvTime, isRetransmittable); err != nil {
if err := s.receivedPacketHandler.ReceivedPacket(packet.packetNumber, rcvTime, isRetransmittable); err != nil {
return err
}
}

40
session_test.go
View file

@ -468,7 +468,7 @@ var _ = Describe("Session", func() {
PacketNumberLen: protocol.PacketNumberLen1,
}
rcvTime := time.Now().Add(-10 * time.Second)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{packetNumber: 0x1337}, nil)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{packetNumber: 0x1337, hdr: hdr}, nil)
rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl)
rph.EXPECT().ReceivedPacket(protocol.PacketNumber(0x1337), rcvTime, false)
sess.receivedPacketHandler = rph
@ -476,7 +476,7 @@ var _ = Describe("Session", func() {
rcvTime: rcvTime,
hdr: &hdr.Header,
data: getData(hdr),
})).To(Succeed())
})).To(BeTrue())
})
It("closes when handling a packet fails", func() {
@ -499,28 +499,29 @@ var _ = Describe("Session", func() {
})
It("handles duplicate packets", func() {
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{}, nil).Times(2)
hdr := &wire.ExtendedHeader{
PacketNumber: 5,
PacketNumberLen: protocol.PacketNumberLen1,
}
Expect(sess.handlePacketImpl(&receivedPacket{hdr: &hdr.Header, data: getData(hdr)})).To(Succeed())
Expect(sess.handlePacketImpl(&receivedPacket{hdr: &hdr.Header, data: getData(hdr)})).To(Succeed())
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{hdr: hdr}, nil).Times(2)
Expect(sess.handlePacketImpl(&receivedPacket{hdr: &hdr.Header, data: getData(hdr)})).To(BeTrue())
Expect(sess.handlePacketImpl(&receivedPacket{hdr: &hdr.Header, data: getData(hdr)})).To(BeTrue())
})
It("ignores packets with a different source connection ID", func() {
hdr := &wire.Header{
IsLongHeader: true,
DestConnectionID: sess.destConnID,
SrcConnectionID: sess.srcConnID,
Length: 1,
}
// Send one packet, which might change the connection ID.
// only EXPECT one call to the unpacker
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{}, nil)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{hdr: &wire.ExtendedHeader{Header: *hdr}}, nil)
Expect(sess.handlePacketImpl(&receivedPacket{
hdr: &wire.Header{
IsLongHeader: true,
DestConnectionID: sess.destConnID,
SrcConnectionID: sess.srcConnID,
Length: 1,
},
hdr: hdr,
data: getData(&wire.ExtendedHeader{PacketNumberLen: protocol.PacketNumberLen1}),
})).To(Succeed())
})).To(BeTrue())
// The next packet has to be ignored, since the source connection ID doesn't match.
Expect(sess.handlePacketImpl(&receivedPacket{
hdr: &wire.Header{
@ -530,12 +531,12 @@ var _ = Describe("Session", func() {
Length: 1,
},
data: getData(&wire.ExtendedHeader{PacketNumberLen: protocol.PacketNumberLen1}),
})).To(Succeed())
})).To(BeFalse())
})
Context("updating the remote address", func() {
It("doesn't support connection migration", func() {
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{}, nil)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{hdr: &wire.ExtendedHeader{}}, nil)
origAddr := sess.conn.(*mockConnection).remoteAddr
remoteIP := &net.IPAddr{IP: net.IPv4(192, 168, 0, 100)}
Expect(origAddr).ToNot(Equal(remoteIP))
@ -544,8 +545,7 @@ var _ = Describe("Session", func() {
hdr: &wire.Header{},
data: getData(&wire.ExtendedHeader{PacketNumberLen: protocol.PacketNumberLen1}),
}
err := sess.handlePacketImpl(&p)
Expect(err).ToNot(HaveOccurred())
Expect(sess.handlePacketImpl(&p)).To(BeTrue())
Expect(sess.conn.(*mockConnection).remoteAddr).To(Equal(origAddr))
})
})
@ -1306,7 +1306,9 @@ var _ = Describe("Client Session", func() {
It("changes the connection ID when receiving the first packet from the server", func() {
unpacker := NewMockUnpacker(mockCtrl)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{}, nil)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).DoAndReturn(func(hdr *wire.Header, data []byte) (*unpackedPacket, error) {
return &unpackedPacket{hdr: &wire.ExtendedHeader{Header: *hdr}}, nil
})
sess.unpacker = unpacker
go func() {
defer GinkgoRecover()
@ -1324,7 +1326,7 @@ var _ = Describe("Client Session", func() {
Length: 1,
},
data: []byte{0},
})).To(Succeed())
})).To(BeTrue())
// make sure the go routine returns
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
sessionRunner.EXPECT().retireConnectionID(gomock.Any())