Define stateless CONNECTION_CLOSE #3292

martinthomson · 2019-12-09T04:08:24Z

This is trickier than I had imagined. Sending CONNECTION_CLOSE is
probably fine, but it's harder to do this correctly now. You can't just
send an unauthenticated CONNECTION_CLOSE because that might disrupt a
real connection. So there are two goals in tension:

Don't kill an active connection (attempt) unnecessarily.
Provide feedback about errors.

The observation is that an attacker can disrupt connections by eliciting
a CONNECTION_CLOSE, so feedback naturally leads to an exposure to a DoS
attack. That's unfortunate, but we have established that we don't care
about DoS by an on-path attacker prior to handshake completion.
Anything we do here has got to be best effort.

DoS prevention would say that you just discard junk, and that is
probably the right answer. But we have a number of cases where the
robustness of the system depends on getting feedback.

Either way, we agreed to allow CONNECTION_CLOSE in Initial, so the
exposure exists anyway. So this contains advice. Maybe too much
advice, but I thought that I'd see what people thought.

Closes #3269.
Closes #3297.

This is trickier than I had imagined. Sending CONNECTION_CLOSE is probably fine, but it's harder to do this correctly now. You can't just send an unauthenticated CONNECTION_CLOSE because that might disrupt a real connection. So there are two goals in tension: 1. Don't kill an active connection (attempt) unnecessarily. 2. Provide feedback about errors. The observation is that an attacker can disrupt connections by eliciting a CONNECTION_CLOSE, so feedback naturally leads to an exposure to a DoS attack. That's unfortunate, but we have established that we don't care about DoS by an on-path attacker prior to handshake completion. Anything we do here has got to be best effort. DoS prevention would say that you just discard junk, and that is probably the right answer. But we have a number of cases where the robustness of the system depends on getting feedback. Either way, we agreed to allow CONNECTION_CLOSE in Initial, so the exposure exists anyway. So this contains advice. Maybe too much advice, but I thought that I'd see what people thought. Closes #3269.

kazuho · 2019-12-09T15:41:02Z

draft-ietf-quic-transport.md

+packets in the datagram SHOULD also be discarded.  A server MAY send a
+CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION in addition to
+discarding a packet if that does not affect a connection for which the server
+has established state; see {{immediate-close}}.


I am confused by the last sentence. Is it saying that when a server receives a small Initial packet for a connection for which it already has state, the server MAY send a PROTOCOL_VIOLATION, but it must retain the state for that connection? Assuming that is the case, I wonder if there is a value in having such a requirement. I am not sure if there is a case such a behavior helps us mitigate attacks.

My understanding was essentially that it's OK for the server to send PROTOCOL_VIOLATION in response to an Initial for which there isn't connection state already (i.e. the first Initial packet for a connection); because that means it couldn't affect a pre-existing connection.

Yeah if we interpret the last sentence that way, I think it would contradict with the "MUST discard" in the first sentence?

I'm confused here too. What does it mean to not affect the connection if the server just sent out a CONNECTION_CLOSE for it?

I am in agreement with @ianswett : perhaps it's best to recommend dropping the connection and not send anything in response. This is a simple enough error for a client to avoid.

@kazuho, I agree with you about the first two. The rule I'm proposing is MUST discard, but MAY immediate-close in addition IF that won't affect state the server holds. Yes, that might affect state the client holds (but the server does not), but that's a risk the server takes. It's a trade-off about the level of feedback the server wants to provide vs. the risk of DoS that the server has to make.

Are we agreeing violently, but talking about how to phrase the recommendation to not immediate-close?

@martinthomson

The rule I'm proposing is MUST discard, but MAY immediate-close in addition IF that won't affect state the server holds.

I think we might have confusion in what "immediate-close" means? My understanding is that "immediate-close" is an action that affects state. It is defined as entering the closing state, then draining state. If the intention is to suggest that a server might send PROTOCOL_VIOLATION without affecting the connection state, then it is confusing to use the term "immediate-close."

That said, what I stated in my comment right above is that I think a state-ful "immediate-close" is fine, but sending PROTOCOL_VIOLATION without affecting connection state does not make sense.

We've now unfortunately lost this comment in the diffs :-/ But that was my confusion too -- sending CONNECTION_CLOSE changes state. I don't understand what it means to send PROTOCOL_VIOLATION without affecting state.

Let's see if I can keep this thread alive. This is sending a packet with a CONNECTION_CLOSE, not an immediate close (I was under the impression that the shorthand was OK here, but it was clearly a mistake). The goal is to use CONNECTION_CLOSE as a signal only, with the intent of triggering the corresponding logic at the client. But the server is discarding this packet and so it won't have any connection state. This won't result in creation or destruction of server state.

The goal is to use CONNECTION_CLOSE as a signal only, with the intent of triggering the corresponding logic at the client.

I think it is not a bad idea to have a MAY for that.

But the server is discarding this packet and so it won't have any connection state. This won't result in creation or destruction of server state.

I do not think that the sentences reflect what you think. Quoted below are the last two sentences of this paragraph. It is my understanding that the intention of the last sentence is to clarify the rationale behind the previous sentence, but as you can see from the emphasized text, they are discussing about different conditions.

A server MAY send a CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION in addition to discarding a packet if that does not affect a connection for which the server has previously established state. Sending CONNECTION_CLOSE will not affect server state in the same way as an immediate close ({{immediate-close}}) as the server has no state, but it will cause any client to terminate a connection attempt.

I think what we actually want to state is something like: A server that has no existing state for a connection MUST discard an Initial packet that is carried in a UDP datagram that is smaller than 1200 bytes. Other packets in that datagram SHOULD also be discarded. In addition to discarding the packet, a server MAY respond with a CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION, without creating state. When a server receives such an Initial packet for which it already has connection state, it MUST either drop that packet or close the connection with error code PROTOCOL_VIOLATION ({{immediate-close}}).

ianswett

LG, some comments

draft-ietf-quic-transport.md

ianswett · 2019-12-09T16:55:48Z

draft-ietf-quic-transport.md

+A server that has no existing state for a connection MUST discard an Initial
+packet that is carried in a UDP datagram that is smaller than 1200 bytes.  Other
+packets in the datagram SHOULD also be discarded.  A server MAY send a
+CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION in addition to


I didn't realize we had text in here that allowed this. I think we're better off with always dropping <1200 byte packets, personally, and keeping the MUST you added above.

janaiyengar · 2019-12-09T21:51:37Z

draft-ietf-quic-transport.md

+
+A CONNECTION_CLOSE frame that is sent in an Initial packet in response to
+unauthenticated information - the content of Initial or Handshake packets
+primarily - might result in denial of service for a legitimate connection.  QUIC


Editorial comment: break this sentence. Suggestion: "A CONNECTION_CLOSE frame might be sent in an Initial packet or in response to unauthenticated information received in Initial or Handshake packets. Such a response might result in a denial of service for a legitimate connection."

janaiyengar · 2019-12-09T22:15:54Z

draft-ietf-quic-transport.md

+packets in the datagram SHOULD also be discarded.  A server MAY send a
+CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION in addition to
+discarding a packet if that does not affect a connection for which the server
+has established state; see {{immediate-close}}.


I'm confused here too. What does it mean to not affect the connection if the server just sent out a CONNECTION_CLOSE for it?

janaiyengar · 2019-12-09T22:16:42Z

draft-ietf-quic-transport.md

+packets in the datagram SHOULD also be discarded.  A server MAY send a
+CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION in addition to
+discarding a packet if that does not affect a connection for which the server
+has established state; see {{immediate-close}}.


I am in agreement with @ianswett : perhaps it's best to recommend dropping the connection and not send anything in response. This is a simple enough error for a client to avoid.

This says that immediate close when you don't have state doesn't have to establish state. Thus, if an endpoint has to immediately close, which can do so without entering the closing period, which would establish state. It makes a general statement, which is that during the handshake - that is, for unauthenticated inputs - packets can be discarded rather than causing an immediate close. This greatly simplifies the text for a small initial. Now it only needs to say "MUST discard, MAY immediate close as well".

janaiyengar

A few editorial suggestions, but this looks good to me now.

draft-ietf-quic-transport.md

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

kazuho

LGTM modulo the editorial concerns below.

kazuho · 2019-12-11T03:51:38Z

draft-ietf-quic-transport.md

+The source and destination connection IDs are the primary means of protection
+against off-path attack during the handshake.  These are required to match those
+set by a peer.  Except for an Initial and stateless reset packets, an endpoint
+only accepts packets that include a destination connection that matches a


s/a destination connection/a Destination Connection ID/

kazuho · 2019-12-11T03:52:12Z

draft-ietf-quic-transport.md

+Addresses cannot change during the handshake, so endpoints can discard packets
+that are received on a different network path.
+
+The source and destination connection IDs are the primary means of protection


s/source and destination connection IDs/Source and Destination Connection IDs/

It is my understanding is that we use CamelCase for these terms.

We might have gotten sloppy on the distinction, but I believe in most places we capitalize the field names, but lower-case the concepts. That is, "The Destination Connection ID field contains the destination connection ID."

Yup. I don't think that we have the concept of a "destination connection ID" separate from the field though, so I am now referring to the fields directly.

martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Dec 9, 2019

nibanks approved these changes Dec 9, 2019

View reviewed changes

kazuho reviewed Dec 9, 2019

View reviewed changes

ianswett reviewed Dec 9, 2019

View reviewed changes

janaiyengar reviewed Dec 9, 2019

View reviewed changes

martinthomson mentioned this pull request Dec 9, 2019

CONNECTION_CLOSE in Handshake too #3293

Merged

martinthomson added 5 commits December 10, 2019 10:50

Add point about off-path packets to handshake DoS section

ffa7f98

Reword

173d006

Does this help?

a2f0672

Merge branch 'master' into close-small-initial

e99cd67

janaiyengar approved these changes Dec 11, 2019

View reviewed changes

martinthomson and others added 2 commits December 11, 2019 11:41

Rewording suggestions

7864bda

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

Rewrap

4722898

kazuho approved these changes Dec 11, 2019

View reviewed changes

Refer to fields properly

6971443

martinthomson changed the title ~~Curtail CONNECTION_CLOSE for small Initial~~ Define stateless CONNECTION_CLOSE Dec 11, 2019

martinthomson mentioned this pull request Dec 11, 2019

Immediate close without state #3297

Closed

ianswett approved these changes Dec 12, 2019

View reviewed changes

marten-seemann approved these changes Jan 24, 2020

View reviewed changes

Merge branch 'master' into close-small-initial

5020e34

martinthomson merged commit f45e58d into master Feb 12, 2020

martinthomson deleted the close-small-initial branch February 12, 2020 00:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define stateless CONNECTION_CLOSE #3292

Define stateless CONNECTION_CLOSE #3292

martinthomson commented Dec 9, 2019 •

edited

Loading

kazuho Dec 9, 2019

nibanks Dec 9, 2019

kazuho Dec 9, 2019

janaiyengar Dec 9, 2019

janaiyengar Dec 9, 2019

martinthomson Dec 10, 2019

kazuho Dec 10, 2019

janaiyengar Dec 10, 2019

martinthomson Dec 10, 2019

kazuho Dec 10, 2019

ianswett left a comment

ianswett Dec 9, 2019

janaiyengar Dec 9, 2019

janaiyengar Dec 9, 2019

janaiyengar Dec 9, 2019

janaiyengar left a comment

kazuho left a comment

kazuho Dec 11, 2019

kazuho Dec 11, 2019

MikeBishop Dec 11, 2019 •

edited

Loading

martinthomson Dec 11, 2019

Define stateless CONNECTION_CLOSE #3292

Define stateless CONNECTION_CLOSE #3292

Conversation

martinthomson commented Dec 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janaiyengar left a comment

Choose a reason for hiding this comment

kazuho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeBishop Dec 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson commented Dec 9, 2019 •

edited

Loading

MikeBishop Dec 11, 2019 •

edited

Loading