From f86875f74673b2a6f8fc797a6171897f5787ad11 Mon Sep 17 00:00:00 2001 From: Lucas Clemente Date: Thu, 19 May 2016 16:20:22 +0200 Subject: [PATCH] reject small CHLOs to prevent amplification attacks fixes #1 --- handshake/crypto_setup.go | 4 ++++ handshake/crypto_setup_test.go | 12 ++++++++++-- protocol/protocol.go | 3 +++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/handshake/crypto_setup.go b/handshake/crypto_setup.go index 9a201cc2..88489ab2 100644 --- a/handshake/crypto_setup.go +++ b/handshake/crypto_setup.go @@ -167,6 +167,10 @@ func (h *CryptoSetup) isInchoateCHLO(cryptoData map[Tag][]byte) bool { } func (h *CryptoSetup) handleInchoateCHLO(sni string, data []byte, cryptoData map[Tag][]byte) ([]byte, error) { + if len(data) < protocol.ClientHelloMinimumSize { + return nil, qerr.Error(qerr.CryptoInvalidValueLength, "CHLO too small") + } + var chloOrNil []byte if h.version > protocol.VersionNumber(30) { chloOrNil = data diff --git a/handshake/crypto_setup_test.go b/handshake/crypto_setup_test.go index b8a3ec59..db5bf2ee 100644 --- a/handshake/crypto_setup_test.go +++ b/handshake/crypto_setup_test.go @@ -130,7 +130,7 @@ var _ = Describe("Crypto setup", func() { Context("when responding to client messages", func() { It("generates REJ messages", func() { - response, err := cs.handleInchoateCHLO("", []byte("chlo"), nil) + response, err := cs.handleInchoateCHLO("", bytes.Repeat([]byte{'a'}, protocol.ClientHelloMinimumSize), nil) Expect(err).ToNot(HaveOccurred()) Expect(response).To(HavePrefix("REJ")) Expect(response).To(ContainSubstring("certcompressed")) @@ -163,7 +163,10 @@ var _ = Describe("Crypto setup", func() { }) It("handles long handshake", func() { - WriteHandshakeMessage(&stream.dataToRead, TagCHLO, map[Tag][]byte{TagSNI: []byte("quic.clemente.io")}) + WriteHandshakeMessage(&stream.dataToRead, TagCHLO, map[Tag][]byte{ + TagSNI: []byte("quic.clemente.io"), + TagPAD: bytes.Repeat([]byte{'a'}, protocol.ClientHelloMinimumSize), + }) WriteHandshakeMessage(&stream.dataToRead, TagCHLO, map[Tag][]byte{TagSCID: scfg.ID, TagSNO: cs.nonce, TagSNI: []byte("quic.clemente.io")}) err := cs.HandleCryptoStream() Expect(err).NotTo(HaveOccurred()) @@ -192,6 +195,11 @@ var _ = Describe("Crypto setup", func() { It("recognizes proper CHLOs", func() { Expect(cs.isInchoateCHLO(map[Tag][]byte{TagSCID: scfg.ID, TagSNO: cs.nonce})).To(BeFalse()) }) + + It("errors on too short inchoate CHLOs", func() { + _, err := cs.handleInchoateCHLO("", bytes.Repeat([]byte{'a'}, protocol.ClientHelloMinimumSize-1), nil) + Expect(err).To(MatchError("CryptoInvalidValueLength: CHLO too small")) + }) }) It("errors without SNI", func() { diff --git a/protocol/protocol.go b/protocol/protocol.go index 8f60f280..6d0082d9 100644 --- a/protocol/protocol.go +++ b/protocol/protocol.go @@ -54,3 +54,6 @@ const DefaultRetransmissionTime = 500 * time.Millisecond // MinRetransmissionTime is the minimum RTO time const MinRetransmissionTime = 200 * time.Millisecond + +// ClientHelloMinimumSize is the minimum size the server expectes an inchoate CHLO to have. +const ClientHelloMinimumSize = 1024