From 4226540cb935df4c65787e48b325e140e44ad710 Mon Sep 17 00:00:00 2001 From: martinduke Date: Sun, 30 Dec 2018 22:06:23 -0800 Subject: [PATCH 1/8] Get rid of DoS vulnerability in Reserved Bits As currently written, any garbage packet, after removing header protection, will have nonzero reserved bits and bring the connection down. Not good! Similarly, it is perverse to force me to actually decrypt the full packet to see if it turns out to be a misbehaving end host and comply with the other possible reading of this requirement. Let the receiver just drop the bad packet after header decryption and carry on. --- draft-ietf-quic-transport.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index 46445d6d24..a83907b787 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3402,9 +3402,9 @@ Reserved Bits (R): : The next two bits (those with a mask of 0x0c) of byte 0 are reserved. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). - The value included prior to protection MUST be set to 0. An endpoint MUST - treat receipt of a packet that has a non-zero value for these bits after - removing protection as a connection error of type PROTOCOL_VIOLATION. + The value included prior to protection MUST be set to 0. An endpoint MUST + silently drop any packet that has a non-zero value for these bits after + removing protection. Packet Number Length (P): @@ -3528,9 +3528,8 @@ Reserved Bits (R): : The next two bits (those with a mask of 0x18) of byte 0 are reserved. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An - endpoint MUST treat receipt of a packet that has a non-zero value for these - bits after removing protection as a connection error of type - PROTOCOL_VIOLATION. + endpoint MUST silently drop any packet that has a non-zero value for these + bits after removing protection. Key Phase (K): From 4ae95269177232b01918af4ccc7b006b63cdb3d5 Mon Sep 17 00:00:00 2001 From: martinduke Date: Sun, 30 Dec 2018 22:08:01 -0800 Subject: [PATCH 2/8] whitespace --- draft-ietf-quic-transport.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index a83907b787..d0c0b52813 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3402,7 +3402,7 @@ Reserved Bits (R): : The next two bits (those with a mask of 0x0c) of byte 0 are reserved. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). - The value included prior to protection MUST be set to 0. An endpoint MUST + The value included prior to protection MUST be set to 0. An endpoint MUST silently drop any packet that has a non-zero value for these bits after removing protection. From f668691287e3eccf8e330761fdb79f44e4d4edb8 Mon Sep 17 00:00:00 2001 From: martinduke Date: Sun, 30 Dec 2018 22:54:24 -0800 Subject: [PATCH 3/8] Addressed MT's issue This is a clarification now, rather than a change. Perhaps we should add more about why, but at least this makes it clear. --- draft-ietf-quic-transport.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index d0c0b52813..88a5c41221 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3402,9 +3402,10 @@ Reserved Bits (R): : The next two bits (those with a mask of 0x0c) of byte 0 are reserved. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). - The value included prior to protection MUST be set to 0. An endpoint MUST - silently drop any packet that has a non-zero value for these bits after - removing protection. + The value included prior to protection MUST be set to 0. An endpoint MUST treat + receipt of a packet that has a non-zero value for these bits after removing + packet (not just header) protection as a connection error of type + PROTOCOL_VIOLATION. Packet Number Length (P): @@ -3528,8 +3529,9 @@ Reserved Bits (R): : The next two bits (those with a mask of 0x18) of byte 0 are reserved. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An - endpoint MUST silently drop any packet that has a non-zero value for these - bits after removing protection. + endpoint MUST treat receipt of a packet that has a non-zero value for these + bits after removing packet (not just header) protection as a connection error + of type PROTOCOL_VIOLATION. Key Phase (K): From f780ef402954ac5a9efabb23a556769f121dbc1d Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Wed, 9 Jan 2019 08:44:08 -0800 Subject: [PATCH 4/8] Update draft-ietf-quic-transport.md Co-Authored-By: martinduke --- draft-ietf-quic-transport.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index 88a5c41221..6a330c9849 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3404,7 +3404,7 @@ Reserved Bits (R): bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these bits after removing - packet (not just header) protection as a connection error of type + packet and header protection as a connection error of type PROTOCOL_VIOLATION. Packet Number Length (P): From 74da0422dd2f96a1c77357d12916a62a1e7dc326 Mon Sep 17 00:00:00 2001 From: martinduke Date: Tue, 5 Feb 2019 15:29:18 -0800 Subject: [PATCH 5/8] Cleaned up after discussion in Tokyo --- draft-ietf-quic-transport.md | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index 0ebbe1e47b..200f3a0e51 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3420,19 +3420,7 @@ Long Packet Type (T): Type-Specific Bits (X): -: The next two bits (those with a mask of 0x0c) of byte 0 are reserved. These - bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). - The value included prior to protection MUST be set to 0. An endpoint MUST treat - receipt of a packet that has a non-zero value for these bits after removing - packet and header protection as a connection error of type PROTOCOL_VIOLATION. - -Packet Number Length (P): - -: The least significant two bits (those with a mask of 0x03) of byte 0 contain - the length of the packet number, encoded as an unsigned, two-bit integer that - is one less than the length of the packet number field in bytes. That is, the - length of the packet number field is the value of this field, plus one. These - bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). +: The lower four bits (those with a mask of 0x0f) of byte 0 are type-specific. Version: @@ -3944,7 +3932,7 @@ Reserved Bits (R): bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these - bits after removing protection as a connection error of type + bits after removing packet and header protection as a connection error of type PROTOCOL_VIOLATION. Key Phase (K): From 7d3489be3b673898d35c3422b29d9cea43c875fa Mon Sep 17 00:00:00 2001 From: martinduke Date: Tue, 5 Feb 2019 15:35:59 -0800 Subject: [PATCH 6/8] Added more explanatory text --- draft-ietf-quic-transport.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index 200f3a0e51..f69d54a1a8 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3481,8 +3481,10 @@ Reserved Bits (R): packet types. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these - bits after removing packet and header protection as a connection error of type - PROTOCOL_VIOLATION. + bits after removing both packet and header protection as a connection error of + type PROTOCOL_VIOLATION. Discarding such a packet after only removing header + protection can expose the endpoint to attacks (see Section 9.3 of + {{QUIC-TLS}}). Packet Number Length (P): @@ -3932,8 +3934,10 @@ Reserved Bits (R): bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these - bits after removing packet and header protection as a connection error of type - PROTOCOL_VIOLATION. + bits after removing both packet and header protection as a connection error + of type PROTOCOL_VIOLATION. Discarding such a packet after only removing + header protection can expose the endpoint to attacks (see Section 9.3 of + {{QUIC-TLS}}). Key Phase (K): From d291c753fcd49194a1c6761f82bbe57e2c188535 Mon Sep 17 00:00:00 2001 From: martinduke Date: Tue, 5 Feb 2019 15:37:00 -0800 Subject: [PATCH 7/8] typos --- draft-ietf-quic-transport.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index f69d54a1a8..81472c0aa3 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3481,7 +3481,7 @@ Reserved Bits (R): packet types. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these - bits after removing both packet and header protection as a connection error of + bits, after removing both packet and header protection, as a connection error of type PROTOCOL_VIOLATION. Discarding such a packet after only removing header protection can expose the endpoint to attacks (see Section 9.3 of {{QUIC-TLS}}). @@ -3934,7 +3934,7 @@ Reserved Bits (R): bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these - bits after removing both packet and header protection as a connection error + bits, after removing both packet and header protection, as a connection error of type PROTOCOL_VIOLATION. Discarding such a packet after only removing header protection can expose the endpoint to attacks (see Section 9.3 of {{QUIC-TLS}}). From a581cbaf6700a7c27ea54c5ff61374e1328106d8 Mon Sep 17 00:00:00 2001 From: martinduke Date: Tue, 5 Feb 2019 15:41:40 -0800 Subject: [PATCH 8/8] Fix failed check --- draft-ietf-quic-transport.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/draft-ietf-quic-transport.md b/draft-ietf-quic-transport.md index 81472c0aa3..0d55ef83a9 100644 --- a/draft-ietf-quic-transport.md +++ b/draft-ietf-quic-transport.md @@ -3481,9 +3481,9 @@ Reserved Bits (R): packet types. These bits are protected using header protection (see Section 5.4 of {{QUIC-TLS}}). The value included prior to protection MUST be set to 0. An endpoint MUST treat receipt of a packet that has a non-zero value for these - bits, after removing both packet and header protection, as a connection error of - type PROTOCOL_VIOLATION. Discarding such a packet after only removing header - protection can expose the endpoint to attacks (see Section 9.3 of + bits, after removing both packet and header protection, as a connection error + of type PROTOCOL_VIOLATION. Discarding such a packet after only removing + header protection can expose the endpoint to attacks (see Section 9.3 of {{QUIC-TLS}}). Packet Number Length (P):