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

Jump to bottom

Authenticate connection IDs #3499

Merged
merged 19 commits into from May 18, 2020
Merged

Authenticate connection IDs #3499

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2fc5157
Authenticate connection IDs
martinthomson Mar 4, 2020
0560290
Review feedback from David and Martin
martinthomson Mar 11, 2020
beba71f
Less text, more cross-reference
martinthomson Mar 11, 2020
efb4c78
singular
martinthomson Mar 22, 2020
41928cd
complete sentences.
martinthomson Mar 24, 2020
f44f24e
Merge branch 'master' into authenticate-hs-cid
martinthomson Mar 24, 2020
c7a2360
Jana's suggestion from review
martinthomson Apr 9, 2020
6f5e547
Reformat
martinthomson Apr 9, 2020
8cec1c2
Rename to initial_connection_id
martinthomson Apr 9, 2020
b0ef978
Merge branch 'master' into authenticate-hs-cid
martinthomson Apr 9, 2020
e2f2b33
Restore active_connection_id_limit
martinthomson Apr 13, 2020
e30cf5f
Editorial comments thanks to @DavidSchinazi
martinthomson May 11, 2020
81bcdb6
Merge branch 'master' into authenticate-hs-cid
martinthomson May 11, 2020
740cd92
Add a picture
martinthomson May 12, 2020
6c04620
Merge branch 'master' into authenticate-hs-cid
martinthomson May 12, 2020
6ae7f18
Use a more generic reference
martinthomson May 12, 2020
fee4020
Correct a few more errors
martinthomson May 12, 2020
27dfb69
Fewer words
martinthomson May 12, 2020
62103ce
Only valid packets change this state
martinthomson May 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 30 additions & 27 deletions draft-ietf-quic-transport.md
View file
Open in desktop
Expand Up @@ -1479,25 +1479,27 @@ lifetime of a connection, especially in response to connection migration

The choice each endpoint makes about connection IDs during the handshake is
authenticated by including all values in transport parameters; see
{{transport-parameters}}. This ensures that all connection IDs are authenticated
by the cryptographic handshake.
{{transport-parameters}}. This ensures that all connection IDs used for the
handshake are also authenticated by the cryptographic handshake.

Each endpoint includes the value of the Source Connection ID field from Initial
packets it sends in the handshake_connection_id transport parameter; see
Each endpoint includes the value of the Source Connection ID field from the
most recent Initial packet it sent in the handshake_connection_id transport
parameter; see
{{transport-parameter-definitions}}. A server includes the Destination
Connection ID field it receives in the original Initial packet from the client
Connection ID field it receives in original Initial packets from the client
- Initial packets received by the server prior to sending a Retry packet -
in the original_connection_id transport parameter. After sending a Retry
packet, a server also includes the Source Connection ID field from the Retry
packet in the retry_connection_id transport parameter.

Each endpoint ensures that the values of the peer's transport parameters
match the values the endpoint sent in its packet headers. The values
The values
provided by a peer for these transport parameters MUST match the
values that an endpoint used in the Destination Connection ID field of an
Initial packet. Including connection ID values in transport parameters and
verifying them ensures that the choice of connection ID cannot be influenced by
an attacker. An endpoint MUST treat any of the following as a connection error
of type PROTOCOL_VIOLATION:
values that an endpoint used in the Destination Connection ID field of Initial
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence appears to apply to all 3 transport parameters, but it mentions Destination CID. I would s/Destination/Source and Destination/

packets that it sent. Including connection ID values in transport parameters and
verifying them ensures that that an attacker cannot influence the choice of
connection ID for a successful connection by injecting packets carrying
attacker-chosen connection IDs during the handshake. An endpoint MUST
treat any of the following as a connection error of type PROTOCOL_VIOLATION:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The absence / presence checks should result in TRANSPORT_PARAMETER_ERROR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I agree. TRANSPORT_PARAMETER_ERROR indicates a parse issue, whereas this validation is performed somewhere else in our implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our rule for FRAME_ENCODING_ERROR is that we use that error code when the error is detectable while parsing the frame, without accessing connection state. I'd argue that we should apply a similar logic here. The absence of initial_source_connection_id and original_destination_connection_id definitely falls in that category, and I'd put the absence / presence of retry_source_connection_id in the same category, although I could see why someone could make an argument against this.

The mismatch seems to be a clear case for PROTOCOL_VIOLATION.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that rationale. I could also see my way to choose TRANSPORT_PARAMETER_ERROR. We use that for transport parameters that are present when they are disallowed (like preferred_address from a client) in addition to obvious encoding problems, which suggests that you might implement this as if transport_parameters.bad() then TRANSPORT_PARAMETER_ERROR, but this validation does involve accessing external state, as you say, so a different code is fully justified.


* absence of the handshake_connection_id transport parameter from either
endpoint,
Expand Down Expand Up @@ -4439,15 +4441,17 @@ processing a Retry packet; {{packet-0rtt}} contains more information on this.

A server acknowledges the use of a Retry packet for a connection using the
retry_connection_id transport parameter; see
{{transport-parameter-definitions}}. If the server sends a Retry packet, the
server adds a retry_connection_id transport parameter that contains the value of
the Source Connection ID field from the Retry packet.

A server always includes an original_connection_id transport parameter. If the
server sends a Retry packet, it MUST include the Destination Connection ID field
from the client's first Initial packet in the original_connection_id transport
parameter; otherwise the original_connection_id transport parameter MUST be
copied from the client Initial.
{{transport-parameter-definitions}}. If it sends a Retry packet, the server
also subsequently includes the value of the Source Connection ID field from
the Retry packet in its retry_connection_id transport parameter.

A server always includes an original_connection_id transport parameter. If it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this entire paragraph as it appears to be redundant with other parts of this PR, and it seems to have discrepancies.

sends a Retry packet, the server MUST subsequently include the Destination
Connection ID field from the client's original Initial packets - packets received
by the server prior to sending the Retry packet - in the original_connection_id
transport parameter. If the server did not send a Retry packet, the value of the
original_connection_id transport parameter MUST be copied from the
Destination Connection ID field of the most recent client Initial packets.

If the client received and processed a Retry packet, it MUST validate that the
retry_connection_id transport parameter is present and correct; otherwise, it
Expand Down Expand Up @@ -4659,9 +4663,8 @@ original_connection_id (0x00):

: The value of the Destination Connection ID field from the first Initial packet
sent by the client; see {{cid-auth}}. This transport parameter is only sent
by a server. This is the same value sent in the Original Destination
Connection ID field that is used to construct a Retry packet (see
{{packet-retry}}).
by a server. The value of this parameter depends on whether the server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the sentence "The value of this parameter depends on whether the server sent a Retry packet". From reading the rest of this PR it sounds like this TP has the same value no matter what happened with RETRY. Am I missing something?

sent a Retry packet; see {{packet-retry}}.

max_idle_timeout (0x01):

Expand Down Expand Up @@ -4853,9 +4856,9 @@ initial_max_stream_data_uni) are equivalent to sending a MAX_STREAM_DATA frame
immediately after opening. If the transport parameter is absent, streams of
that type start with a flow control limit of 0.

A client MUST NOT include server-only transport parameters
(original_connection_id, preferred_address, retry_connection_id, and
stateless_reset_token). A server MUST treat receipt of any of these transport
A client MUST NOT include any server-only transport parameter:
original_connection_id, preferred_address, retry_connection_id, or
stateless_reset_token. A server MUST treat receipt of any of these transport
parameters as a connection error of type TRANSPORT_PARAMETER_ERROR.


Expand Down