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

Change connection ID with Transport Parameters #1041

Closed
wants to merge 3 commits into from

Conversation

ianswett
Copy link
Contributor

@ianswett ianswett commented Jan 9, 2018

Change the connection ID by specifying a new server chosen connection ID in transport parameters. Allows a new connection ID to be supplied in Retry packets(Fixing issue #713) and allows multiple handshakes to occur on the same 4-tuple, fixing #714.

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

I have a few reservations about using transport parameters for this. I know we had some concerns about using NEW_CONNECTION_ID for this, but it might be a better approach. I can already think of at least one gotcha with that, but it's potentially a cleaner model.

handshake, it chooses a new value for the connection ID and sends that in a
Handshake packet ({{packet-handshake}}). The server MAY choose to use the value
that the client initially selects.
handshake, it may chooses a new value for the connection ID and sends that in
Copy link
Member

Choose a reason for hiding this comment

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

MAY choose I think

that the client initially selects.
handshake, it may chooses a new value for the connection ID and sends that in
the server_connection_id transport parameter ({{transport-parameter-definitions}})
of the Handshake packet ({{packet-handshake}}).
Copy link
Member

Choose a reason for hiding this comment

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

A transport parameter can't be "of the Handshake packet". You can drop from "of" onward.


Once the client receives the connection ID that the server has chosen, it MUST
If the client receives a new connection ID that the server has chosen, it MUST
Copy link
Member

Choose a reason for hiding this comment

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

Transport parameters are only really available once the handshake completes. For one, they are in TLS EncryptedExtensions, which isn't necessarily right up front. More importantly, though we could release them before the handshake completes, it makes it harder to reason about their authenticity. Given that transport parameters are used for client address validation, that's problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aren't they available at the server side when a CHLO is received? And shouldn't they be available in the stateless retry?

use it for all subsequent Handshake ({{packet-handshake}}) and 1-RTT
({{packet-protected}}) packets but not for 0-RTT packets ({{packet-protected}}).

The server MUST NOT switch to the new connection ID until has received a
packet containing the connection ID from the client. In particular, the
Copy link
Member

Choose a reason for hiding this comment

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

I think that it is easier to make the switch with the change to 1-RTT protected packets. Creating another mini-protocol here complicates things considerably. Outside of cases with packet loss (where you can't rely on the client being able to recover the transport parameters), the only real difference is that you are changing the connection ID sent with the client's second flight (it's set of Handshake packets).

Now, I agree that there is potentially some value in having the right markings on the client Handshake packets, but maybe the right answer here is to use NEW_CONNECTION_ID.

Copy link
Contributor Author

@ianswett ianswett Jan 9, 2018

Choose a reason for hiding this comment

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

One of the goals of this change is to allow connection ID changes to occur before switching to 1RTT, so only allowing the switch with 1RTT packets defeats half the point.

NEW_CONNECTION_ID wouldn't change the encryption level issues. And we'd have to allow it during the handshake, which we don't today. It seems more complex than this approach, but it's probably doable.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @ianswett, that the client should be the one that starts using the new connection ID first. Otherwise, you are back to one of the original problems of multiple clients on the same 4-tuple and it is impossible to determine which of them to deliver the newly received packet to.

@ianswett
Copy link
Contributor Author

ianswett commented Jan 9, 2018

Possibly I'm confused on the use of transport parameters. It needs to be possible to negotiate in transport params, so they would have to be available to QUIC prior to the TLS handshake completing.

@ianswett
Copy link
Contributor Author

ianswett commented Jan 9, 2018

One other concern about NEW_CONNECTION_ID is that the stateless retry is part of the TLS transcript, and so if the server chosen connection ID is in transport params, the handshake will fail at the end if the connection ID is changed. I'm not sure why an attacker would want to insert a NEW_CONNECTION_ID frame in this context, but I think it would be impossible for the client to detect that it had occurred?

But if that's not an issue or we don't care, then NEW_CONNECTION_ID is workable.

My core goals are to solve the specified issues. I do think it's better to have the client be the first to use the new connection ID. The approach of the server suddenly changing the connection ID is very odd and as discussed, causes problems with multiple simultaneous handshakes.

@MikeBishop
Copy link
Contributor

MikeBishop commented Jan 9, 2018

"Negotiate" is perhaps overstating things in the transport parameters.

During connection establishment, both endpoints make authenticated declarations of their transport parameters. These declarations are made unilaterally by each endpoint. Endpoints are required to comply with the restrictions implied by these parameters; the description of each parameter includes rules for its handling.

In what scenario do you think the transport parameters have to be available before the handshake completes?

(And yes, I believe you're correct that an attacker could transparently insert/modify a NEW_CONNECTION_ID frame in a Retry packet if we allowed them to be present. Of course, can't an on-path attacker also modify a Retry arbitrarily?)

@ianswett
Copy link
Contributor Author

ianswett commented Jan 9, 2018

The main one I had in mind was feature negotiation. A client wants to negotiate non-standard feature A and the server replies that A is ok. I guess a server could enumerate all non-standard features it supported and both could agree after the handshake?

There are probably other use cases I'm not thinking of, but maybe all of them can be worked around.

@martinthomson
Copy link
Member

It's possible to use the declarations in transport parameters to effect negotiation, sure. If I declare feature A and you do too, then it's safe to assume that using feature A won't cause problems. I'd say that we probably need that for - for example - changing the ACK frame format to support new information. And calling that negotiation isn't a stretch. Right now, we don't have anything that negotiates and the text reflects that.

As for NEW_CONNECTION_ID, sure, an attacker can insert a connection ID. The likely effect is to cause packets to be routed incorrectly, likely disrupting the handshake. We've agreed that if an attacker is able to disrupt the handshake to this degree, then we're not going to try to stop them. An attacker with that capability could also choose to block the handshake or tamper with it in other ways that would cause failure. If the concern is misrouting, they could easily rewrite the connection ID.

If the server is able to (maybe because it didn't use the connection ID) and decides to accept the incorrectly-marked packets, then it just became complicit and I'm not sure that we need to worry about that.

One thing that I was wrong about here was that transport parameters are available to a client because the TLS handshake is complete. There's a nasty race condition there though. The handshake is really only complete once the client has sent its last flight, and that means using handshake packet protection keys, but using transport parameters from the completed handshake. It's rather awkward.

We would also need to be clear about what connection ID is used for Handshake packets that the client sends when there is packet loss and it has to acknowledge server Handshake packets.

@ianswett
Copy link
Contributor Author

Thanks Martin, did you want me to re-write this using the NEW_CONNECTION_ID frame or do you think the transport parameters approach is also workable?

@martinthomson
Copy link
Member

I don't know. Maybe we can talk about this in Melbourne.

martinthomson added a commit that referenced this pull request Jan 24, 2018
Part of why we were echoing the connection ID in Retry is now no longer
relevant. We now have encryption providing the primary proof of receipt
for the Initial packet.

Allowing the server to pick a new connection ID and include that in the
Retry packet header allows servers to steer clients.

Closes #713, #1041.
@ianswett ianswett closed this Feb 13, 2018
@martinthomson martinthomson deleted the ianswett-conn-id branch May 29, 2018 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants