gr1ffon/quic-go
1
0
Fork 0
forked from quic-go/quic-go
Code Pull Requests Activity

make sure that the SHLO was sent before returning Listener.Accept

Browse Source 
This fixes a race condition: In the server crypto setup, we would send
protocol.EncryptionForwardSecure on the aeadChan as soon as the SHLO was
composed, but before it was written to the crypto stream. This lead to
Listener.Accept returning the session already. If the server was ready
to write a lot of data then, this data could be sent before the crypto
setup would write on the crypto stream, therefore sending a lot of
undecryptable packets to the client, which would eventually lead to a
connection error (too many undecryptable packets).
This commit is contained in:
Marten Seemann
2017-08-27 18:41:09 +07:00
parent a6719bf417
commit bb670be93a
2 changed files with 10 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
handshake/crypto_setup_server.go
View File

@@ -38,8 +38,8 @@ type cryptoSetupServer struct {
secureAEAD crypto.AEAD
forwardSecureAEAD crypto.AEAD
receivedForwardSecurePacket bool
sentSHLO bool
receivedSecurePacket bool
sentSHLO chan struct{} // this channel is closed as soon as the SHLO has been written
aeadChanged chan<- protocol.EncryptionLevel
keyDerivation KeyDerivationFunction
@@ -93,6 +93,7 @@ func NewCryptoSetup(
cryptoStream: cryptoStream,
connectionParameters: connectionParametersManager,
acceptSTKCallback: acceptSTK,
sentSHLO: make(chan struct{}),
aeadChanged: aeadChanged,
}, nil
}
@@ -167,10 +168,11 @@ func (h *cryptoSetupServer) handleMessage(chloData []byte, cryptoData map[Tag][]
if err != nil {
return false, err
}
_, err = h.cryptoStream.Write(reply)
if err != nil {
if _, err := h.cryptoStream.Write(reply); err != nil {
return false, err
}
h.aeadChanged <- protocol.EncryptionForwardSecure
close(h.sentSHLO)
return true, nil
}
@@ -193,6 +195,8 @@ func (h *cryptoSetupServer) Open(dst, src []byte, packetNumber protocol.PacketNu
if err == nil {
if !h.receivedForwardSecurePacket { // this is the first forward secure packet we receive from the client
h.receivedForwardSecurePacket = true
// wait until protocol.EncryptionForwardSecure was sent on the aeadChan
<-h.sentSHLO
close(h.aeadChanged)
}
return res, protocol.EncryptionForwardSecure, nil
@@ -451,9 +455,6 @@ func (h *cryptoSetupServer) handleCHLO(sni string, data []byte, cryptoData map[T
var reply bytes.Buffer
message.Write(&reply)
utils.Debugf("Sending %s", message)
h.aeadChanged <- protocol.EncryptionForwardSecure
return reply.Bytes(), nil
}

6
handshake/crypto_setup_server_test.go
View File

@@ -350,7 +350,6 @@ var _ = Describe("Server Crypto Setup", func() {
Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure)))
Expect(stream.dataWritten.Bytes()).To(ContainSubstring("SHLO"))
Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionForwardSecure)))
Expect(aeadChanged).ToNot(Receive())
Expect(aeadChanged).ToNot(BeClosed())
})
@@ -384,6 +383,7 @@ var _ = Describe("Server Crypto Setup", func() {
Expect(stream.dataWritten.Bytes()).ToNot(ContainSubstring("REJ"))
Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure)))
Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionForwardSecure)))
Expect(aeadChanged).ToNot(BeClosed())
})
It("recognizes inchoate CHLOs missing SCID", func() {
@@ -554,6 +554,8 @@ var _ = Describe("Server Crypto Setup", func() {
TagKEXS: kexs,
})
Expect(err).ToNot(HaveOccurred())
Expect(aeadChanged).To(Receive(Equal(protocol.EncryptionSecure)))
close(cs.sentSHLO)
}
Context("null encryption", func() {
@@ -654,8 +656,6 @@ var _ = Describe("Server Crypto Setup", func() {
doCHLO()
_, _, err := cs.Open(nil, []byte("forward secure encrypted"), 0, []byte{})
Expect(err).ToNot(HaveOccurred())
Expect(aeadChanged).To(Receive()) // consume the protocol.EncryptionSecure
Expect(aeadChanged).To(Receive()) // consume the protocol.EncryptionForwardSecure
Expect(aeadChanged).To(BeClosed())
})
})