quicwg / base-drafts

Internet-Drafts that make up the base QUIC specification
https://quicwg.org
1.63k stars 204 forks source link

Clients cannot abandon Initial packets while server can still send initial close #2541

Closed mjoras closed 5 years ago

mjoras commented 5 years ago

17.2.2.1 of the spec currently reads: "A client stops both sending and processing Initial packets when it sends its first Handshake packet. Though packets might still be in flight or awaiting acknowledgment, no further Initial packets need to be exchanged beyond this point. Initial packet protection keys are discarded (see Section 4.10 of [QUIC-TLS]) along with any loss recovery and congestion control state (see Sections 5.3.1.2 and 6.9 of [QUIC-RECOVERY])."

I believe this is inconsistent with section 10.3, regarding sending an initial connection close: "If the connection has been successfully established, endpoints MUST send any CONNECTION_CLOSE frames in a 1-RTT packet. Prior to connection establishment a peer might not have 1-RTT keys, so endpoints SHOULD send CONNECTION_CLOSE frames in a Handshake packet. If the endpoint does not have Handshake keys, or it is not certain that the peer has Handshake keys, it MAY send CONNECTION_CLOSE frames in an Initial packet. If multiple packets are sent, they can be coalesced (see Section 12.2) to facilitate retransmission."

The client may have started sending handshake packets at the point which the server decides to send a close. Since the server does not know if the client has the handshake keys it MAY send the close in an initial packet. However since the client stops (though it is missing a MUST or MAY qualifier) processing initial packets it will never process the close from the server.

siyengar commented 5 years ago

Ya we encountered this issue while interoping with @larseggert. mvfst sends the close in initial keys until we're sure that the client has the onertt keys. I think this should be allowed.

nibanks commented 5 years ago

We really want to throw away initial keys ASAP. What about sending connection close in ALL encryption levels the peer might accept? Send a coalesced Initial/Handshake packet, each containing connection close.

DavidSchinazi commented 5 years ago

I agree with @nibanks. Keeping the initial keys around for longer opens up a DoS attack vector. Instead having the server send CONNECTION_CLOSE with all the keys it supports sounds safer, and using a coalesced packet can make that efficient.

kazuho commented 5 years ago

I agree with the principle of throwing Initial keys ASAP. Another strategy that can be employed by the server to avoid the issue is send CONNECTION_CLOSE in Initial packets, but continue waiting for Handshake packets until it discards the connection. When it receives a Handshake packet (that successfully authenticates), it sends a CONNECTION_CLOSE frame in Handshake packet. The approach works because a client is required to send an empty Handshake packet even if there's no data to be sent.

siyengar commented 5 years ago

that's an interesting idea, but kind of annoying. It makes coalesced packets a requirement for correct behavior, that we don't have right now. I'm not very happy making sending coalesced packets a requirement. Maybe in different packets might work.

nibanks commented 5 years ago

I don't think coalesced packets would be a requirement. You could still send separate UDP datagrams.

ianswett commented 5 years ago

My understanding is that the KEYS_READY/RETIRE_KEYS/etc(#2214) change is going to change the key discarding dynamics enough that this issue may not longer be a problem?

In particular, I'd expect the client to send RETIRE_KEYS when it first has Handshake keys and the server to send it when it receives a Handshake packet from the client. That would delay dropping the Initial keys just enough that I believe there should no longer be a confusion about what the highest encryption level the peer will process?

larseggert commented 5 years ago

It looks like this will become resolved once #2214 lands.

huitema commented 5 years ago

Note the trade-off there: once the handshake is established, the peers have a secure channel modulo possible MITM. Critical messages like connection close really ought to be sent on the secure channel. The client that accepts connection close on the Initial channel opens itself to the equivalent of a spoofed TCP RST. Secure implementations must be allowed to ignore all Initial packets once handshake is established.

ekr commented 5 years ago

See also #2741

mnot commented 5 years ago

Hi @DavidSchinazi any progress on a proposal?

DavidSchinazi commented 5 years ago

@mnot We're discussing discarding initial keys in the design team. No significant progress yet.

DavidSchinazi commented 5 years ago

This issue should now be resolved by #2688. @mjoras can you please confirm if you think the issue is now resolved, and close it if it is?

mnot commented 5 years ago

Please don't close design issues; we need to follow the process to assure that they reflect consensus.

DavidSchinazi commented 5 years ago

@mnot Apologies for asking @mjoras to close the issue. Sounds like I misunderstood the process here. Is the process documented somewhere or is this just common sense that I might be missing?

erickinnear commented 5 years ago

(https://github.com/quicwg/base-drafts/blob/master/CONTRIBUTING.md#late-stage-process) Might need some thought around issues that don't end up with a PR, though.

MikeBishop commented 5 years ago

See https://github.com/quicwg/base-drafts/blob/master/CONTRIBUTING.md#late-stage-process. Since this one was resolved by a PR that was already consensus-called and merged, it basically just needs the confirmation that everyone agrees this is addressed.