Merge pull request #473 from lucas-clemente/fix-471

only send Public Reset after a timeout
This commit is contained in:
Marten Seemann
2017-03-08 18:49:09 +07:00
committed by GitHub
3 changed files with 76 additions and 39 deletions

View File

@@ -23,6 +23,10 @@ const InitialCongestionWindow = 32
// session queues for later until it sends a public reset.
const MaxUndecryptablePackets = 10
// PublicResetTimeout is the time to wait before sending a Public Reset when receiving too many undecryptable packets during the handshake
// This timeout allows the Go scheduler to switch to the Go rountine that reads the crypto stream and to escalate the crypto
const PublicResetTimeout = 500 * time.Millisecond
// AckSendDelay is the maximum delay that can be applied to an ACK for a retransmittable packet
// This is the value Chromium is using
const AckSendDelay = 25 * time.Millisecond

View File

@@ -76,8 +76,12 @@ type session struct {
runClosed chan struct{}
closed uint32 // atomic bool
undecryptablePackets []*receivedPacket
aeadChanged chan protocol.EncryptionLevel
// when we receive too many undecryptable packets during the handshake, we send a Public reset
// but only after a time of protocol.PublicResetTimeout has passed
undecryptablePackets []*receivedPacket
receivedTooManyUndecrytablePacketsTime time.Time
aeadChanged chan protocol.EncryptionLevel
nextAckScheduledTime time.Time
@@ -250,10 +254,14 @@ runLoop:
if err := s.sendPacket(); err != nil {
s.close(err)
}
if time.Now().Sub(s.lastNetworkActivityTime) >= s.idleTimeout() {
now := time.Now()
if !s.receivedTooManyUndecrytablePacketsTime.IsZero() && s.receivedTooManyUndecrytablePacketsTime.Add(protocol.PublicResetTimeout).Before(now) {
s.close(qerr.Error(qerr.DecryptionFailure, "too many undecryptable packets received"))
}
if now.Sub(s.lastNetworkActivityTime) >= s.idleTimeout() {
s.close(qerr.Error(qerr.NetworkIdleTimeout, "No recent network activity."))
}
if !s.cryptoSetup.HandshakeComplete() && time.Now().Sub(s.sessionCreationTime) >= protocol.MaxTimeForCryptoHandshake {
if !s.cryptoSetup.HandshakeComplete() && now.Sub(s.sessionCreationTime) >= protocol.MaxTimeForCryptoHandshake {
s.close(qerr.Error(qerr.NetworkIdleTimeout, "Crypto handshake did not complete in time."))
}
s.garbageCollectStreams()
@@ -276,6 +284,9 @@ func (s *session) maybeResetTimer() {
handshakeDeadline := s.sessionCreationTime.Add(protocol.MaxTimeForCryptoHandshake)
nextDeadline = utils.MinTime(nextDeadline, handshakeDeadline)
}
if !s.receivedTooManyUndecrytablePacketsTime.IsZero() {
nextDeadline = utils.MinTime(nextDeadline, s.receivedTooManyUndecrytablePacketsTime.Add(protocol.PublicResetTimeout))
}
if nextDeadline.Equal(s.currentDeadline) {
// No need to reset the timer
@@ -774,8 +785,10 @@ func (s *session) tryQueueingUndecryptablePacket(p *receivedPacket) {
return
}
utils.Infof("Queueing packet 0x%x for later decryption", p.publicHeader.PacketNumber)
if len(s.undecryptablePackets)+1 >= protocol.MaxUndecryptablePackets {
s.close(qerr.Error(qerr.DecryptionFailure, "too many undecryptable packets received"))
if len(s.undecryptablePackets)+1 >= protocol.MaxUndecryptablePackets && s.receivedTooManyUndecrytablePacketsTime.IsZero() {
s.receivedTooManyUndecrytablePacketsTime = time.Now()
s.maybeResetTimer()
return
}
s.undecryptablePackets = append(s.undecryptablePackets, p)
}

View File

@@ -1174,43 +1174,63 @@ var _ = Describe("Session", func() {
Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.InvalidCryptoMessageType))
})
It("sends public reset after too many undecryptable packets", func() {
// Write protocol.MaxUndecryptablePackets and expect a public reset to happen
for i := 0; i < protocol.MaxUndecryptablePackets; i++ {
hdr := &PublicHeader{
PacketNumber: protocol.PacketNumber(i + 1),
Context("sending a Public Reset when receiving undecryptable packets during the handshake", func() {
It("doesn't immediately send a Public Reset after receiving too many undecryptable packets", func() {
go sess.run()
for i := 0; i < protocol.MaxUndecryptablePackets; i++ {
hdr := &PublicHeader{
PacketNumber: protocol.PacketNumber(i + 1),
}
sess.handlePacket(&receivedPacket{publicHeader: hdr, data: []byte("foobar")})
}
sess.handlePacket(&receivedPacket{publicHeader: hdr, data: []byte("foobar")})
}
sess.run()
sess.scheduleSending()
Consistently(func() [][]byte { return mconn.written }).Should(HaveLen(0))
})
Expect(mconn.written).To(HaveLen(1))
Expect(mconn.written[0]).To(ContainSubstring(string([]byte("PRST"))))
Expect(sess.runClosed).To(Receive())
})
It("ignores undecryptable packets after the handshake is complete", func() {
*(*bool)(unsafe.Pointer(reflect.ValueOf(sess.cryptoSetup).Elem().FieldByName("receivedForwardSecurePacket").UnsafeAddr())) = true
for i := 0; i < protocol.MaxUndecryptablePackets; i++ {
hdr := &PublicHeader{
PacketNumber: protocol.PacketNumber(i + 1),
It("sets a deadline to send a Public Reset after receiving too many undecryptable packets", func() {
go sess.run()
for i := 0; i < protocol.MaxUndecryptablePackets; i++ {
hdr := &PublicHeader{
PacketNumber: protocol.PacketNumber(i + 1),
}
sess.handlePacket(&receivedPacket{publicHeader: hdr, data: []byte("foobar")})
}
sess.handlePacket(&receivedPacket{publicHeader: hdr, data: []byte("foobar")})
}
go sess.run()
Consistently(sess.undecryptablePackets).Should(HaveLen(0))
sess.closeImpl(nil, true)
Eventually(sess.runClosed).Should(Receive())
})
Eventually(func() time.Time { return sess.receivedTooManyUndecrytablePacketsTime }).Should(BeTemporally("~", time.Now(), 10*time.Millisecond))
})
It("unqueues undecryptable packets for later decryption", func() {
sess.undecryptablePackets = []*receivedPacket{{
publicHeader: &PublicHeader{PacketNumber: protocol.PacketNumber(42)},
}}
Expect(sess.receivedPackets).NotTo(Receive())
sess.tryDecryptingQueuedPackets()
Expect(sess.undecryptablePackets).To(BeEmpty())
Expect(sess.receivedPackets).To(Receive())
It("sends a Public Reset after a timeout", func() {
sess.receivedTooManyUndecrytablePacketsTime = time.Now().Add(-protocol.PublicResetTimeout)
go sess.run()
time.Sleep(10 * time.Millisecond) // wait for the run loop to spin up
sess.scheduleSending() // wake up the run loop
Eventually(func() [][]byte { return mconn.written }).Should(HaveLen(1))
Expect(mconn.written[0]).To(ContainSubstring(string([]byte("PRST"))))
Expect(sess.runClosed).To(Receive())
})
It("ignores undecryptable packets after the handshake is complete", func() {
*(*bool)(unsafe.Pointer(reflect.ValueOf(sess.cryptoSetup).Elem().FieldByName("receivedForwardSecurePacket").UnsafeAddr())) = true
for i := 0; i < protocol.MaxUndecryptablePackets; i++ {
hdr := &PublicHeader{
PacketNumber: protocol.PacketNumber(i + 1),
}
sess.handlePacket(&receivedPacket{publicHeader: hdr, data: []byte("foobar")})
}
go sess.run()
Consistently(sess.undecryptablePackets).Should(HaveLen(0))
sess.closeImpl(nil, true)
Eventually(sess.runClosed).Should(Receive())
})
It("unqueues undecryptable packets for later decryption", func() {
sess.undecryptablePackets = []*receivedPacket{{
publicHeader: &PublicHeader{PacketNumber: protocol.PacketNumber(42)},
}}
Expect(sess.receivedPackets).NotTo(Receive())
sess.tryDecryptingQueuedPackets()
Expect(sess.undecryptablePackets).To(BeEmpty())
Expect(sess.receivedPackets).To(Receive())
})
})
It("calls the cryptoChangeCallback when the AEAD changes", func(done Done) {