Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should connections be disconnected on malformed Initial packets? #4350

Closed
huitema opened this issue Nov 13, 2020 · 6 comments · Fixed by #4359
Closed

Should connections be disconnected on malformed Initial packets? #4350

huitema opened this issue Nov 13, 2020 · 6 comments · Fixed by #4359
Labels
-transport ietf-lc An issue that was raised during IETF Last Call.

Comments

@huitema
Copy link
Contributor

huitema commented Nov 13, 2020

This issue derives from the efforts of @kazu-yamamoto to write an HTTP3 test suite, and outline potential implementation issues when sticking to the spec. Section 17.2 of the transport spec describes the format of long header packets, and says:

Reserved Bits:
Two bits (those with a mask of 0x0c) of byte 0 are reserved across multiple 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.5 of [QUIC-TLS].

I can see doing that for Handshake or 0-RTT packets, but for Initial packets that feels risky. Any man-on-the-side can send forged Initial packets, including forged Initial packets with reserved bits set. Do we really intend to say that such packets cause the connection to break? What if the handshake has already progressed?

My gut feeling is that it is simpler and safer to ignore malformed Initial packets and discard them silently.

@kazuho
Copy link
Member

kazuho commented Nov 13, 2020

My gut feeling is that it is simpler and safer to ignore malformed Initial packets and discard them silently.

IIUC, the last paragraph of section 21.2 allows endpoints to implement such strategy, quote: Endpoints are permitted to use other methods to detect and attempt to recover from interference with the handshake. Invalid packets may be identified and discarded using other methods, but no specific method is mandated in this document.

@martinthomson
Copy link
Member

@kazuho has correctly identified the escape hatch we left. Initial packets are the only place where this really applies, but it goes much further than just those two bits.

@larseggert larseggert added -transport ietf-lc An issue that was raised during IETF Last Call. labels Nov 13, 2020
@larseggert larseggert added this to Triage in Late Stage Processing via automation Nov 13, 2020
@ptrd
Copy link

ptrd commented Nov 13, 2020

Are you saying the MUST is not a real MUST because in the "Security Considerations" there is a suggestion that implementations could deviate??? IMHO that would be weird, given emphasis on the meaning of MUST.

martinthomson added a commit that referenced this issue Nov 16, 2020
But we managed to hide that in the security considerations.

Closes #4350.
@kazu-yamamoto
Copy link
Contributor

My opinion is:

  • An endpoint MUST send PROTOCOL_VIOLATION for 1-RTT.
  • An endpoint MAY send PROTOCOL_VIOLATION or MAY discard the packet for other types.

@martinthomson
Copy link
Member

@kazu-yamamoto, the proposal here is to allow discarding only for Initial. Other types are effectively authenticated*.

* Handshake might not be confirmed in the sense that you have key confirmation and peer authentication, but I can't imagine anyone attempting to do anything other than abort if an attacker gets that far).

@kazu-yamamoto
Copy link
Contributor

I'm OK with discarding only Initial.

Late Stage Processing automation moved this from Triage to Issue Handled Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport ietf-lc An issue that was raised during IETF Last Call.
Projects
Late Stage Processing
  
Issue Handled
Development

Successfully merging a pull request may close this issue.

6 participants