diff --git a/session.go b/session.go index 52defc88e..a9252c72a 100644 --- a/session.go +++ b/session.go @@ -174,7 +174,8 @@ type session struct { handshakeCtx context.Context handshakeCtxCancel context.CancelFunc - undecryptablePackets []*receivedPacket + undecryptablePackets []*receivedPacket // undecryptable packets, waiting for a change in encryption level + undecryptablePacketsToProcess []*receivedPacket clientHelloWritten <-chan *wire.TransportParameters earlySessionReadyChan chan struct{} @@ -555,22 +556,14 @@ runLoop: s.maybeResetTimer() - select { - case closeErr = <-s.closeChan: - break runLoop - case <-s.timer.Chan(): - s.timer.SetRead() - // We do all the interesting stuff after the switch statement, so - // nothing to see here. - case <-s.sendingScheduled: - // We do all the interesting stuff after the switch statement, so - // nothing to see here. - case p := <-s.receivedPackets: - // 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 + var processedUndecryptablePacket bool + if len(s.undecryptablePacketsToProcess) > 0 { + queue := s.undecryptablePacketsToProcess + s.undecryptablePacketsToProcess = nil + for _, p := range queue { + if processed := s.handlePacketImpl(p); processed { + processedUndecryptablePacket = true + } } // Don't set timers and send packets if the packet made us close the session. select { @@ -578,8 +571,34 @@ runLoop: break runLoop default: } - case <-s.handshakeCompleteChan: - s.handleHandshakeComplete() + } else if !processedUndecryptablePacket { + select { + case closeErr = <-s.closeChan: + break runLoop + case <-s.timer.Chan(): + s.timer.SetRead() + // We do all the interesting stuff after the switch statement, so + // nothing to see here. + case <-s.sendingScheduled: + // We do all the interesting stuff after the switch statement, so + // nothing to see here. + case p := <-s.receivedPackets: + s.sentPacketHandler.ReceivedBytes(p.Size()) + // 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 + } + // Don't set timers and send packets if the packet made us close the session. + select { + case closeErr = <-s.closeChan: + break runLoop + default: + } + case <-s.handshakeCompleteChan: + s.handleHandshakeComplete() + } } now := time.Now() @@ -737,7 +756,6 @@ func (s *session) handlePacketImpl(rp *receivedPacket) bool { var processed bool data := rp.data p := rp - s.sentPacketHandler.ReceivedBytes(protocol.ByteCount(len(data))) for len(data) > 0 { if counter > 0 { p = p.Clone() @@ -1158,7 +1176,9 @@ func (s *session) handleCryptoFrame(frame *wire.CryptoFrame, encLevel protocol.E return err } if encLevelChanged { - s.tryDecryptingQueuedPackets() + // Queue all packets for decryption that have been undecryptable so far. + s.undecryptablePacketsToProcess = s.undecryptablePackets + s.undecryptablePackets = nil } return nil } @@ -1719,13 +1739,6 @@ func (s *session) tryQueueingUndecryptablePacket(p *receivedPacket, hdr *wire.He s.undecryptablePackets = append(s.undecryptablePackets, p) } -func (s *session) tryDecryptingQueuedPackets() { - for _, p := range s.undecryptablePackets { - s.handlePacket(p) - } - s.undecryptablePackets = s.undecryptablePackets[:0] -} - func (s *session) queueControlFrame(f wire.Frame) { s.framer.QueueControlFrame(f) s.scheduleSending() diff --git a/session_test.go b/session_test.go index c61e4d505..df4ecfa85 100644 --- a/session_test.go +++ b/session_test.go @@ -2434,7 +2434,6 @@ var _ = Describe("Client Session", func() { It("handles Retry packets", func() { sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) sess.sentPacketHandler = sph - sph.EXPECT().ReceivedBytes(gomock.Any()) sph.EXPECT().ResetForRetry() cryptoSetup.EXPECT().ChangeConnectionID(protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}) packer.EXPECT().SetToken([]byte("foobar")) @@ -2721,7 +2720,6 @@ var _ = Describe("Client Session", func() { It("ignores Initial packets which use original source id, after accepting a Retry", func() { sph := mockackhandler.NewMockSentPacketHandler(mockCtrl) sess.sentPacketHandler = sph - sph.EXPECT().ReceivedBytes(gomock.Any()).Times(2) sph.EXPECT().ResetForRetry() newSrcConnID := protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef} cryptoSetup.EXPECT().ChangeConnectionID(newSrcConnID)