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

Stateless Reset during Path Migration #1259

Closed
wants to merge 5 commits into from
Closed

Conversation

MikeBishop
Copy link
Contributor

Extracted from #1251 at @martinthomson's request. Fixes #1258.

@MikeBishop MikeBishop added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Apr 2, 2018
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.

Please add a note to the effect that a Stateless Reset causes the connection ID to be unusable thereafter.

consider path validation to have failed.
consider path validation to have failed. After receiving a Stateless Reset, the
client MUST NOT send additional packets with the Connection ID used on the
probing packet.

Copy link
Contributor

Choose a reason for hiding this comment

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

with high probability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With high probability what? This isn't a statement that you probably reached the wrong server, it's a normative requirement that the client abandon the path if a certain response occurs.

Copy link
Contributor

Choose a reason for hiding this comment

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

When you create new connections, you use a random connection ID. If "high probability" is not there, it means that you have to log all previous connections and test against those, otherwise you can just assume the random ID is unlikely to collide, but if it does, the world will continue to exist as best as it can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not following. This text isn't about new connections, and the CID isn't random -- it's supplied by 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.

If you create a new Initial packet for a new connection, you use a random CID before using the server provided CID. If you consider this a separate namespace, then your are right. And if you are only referring to the current connection you are also right. But otherwise you risk accidentially creating an already used ID in a new connection, and with, say, 32 bit CID's it is non-neglectible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that this text is about path validation using probing packets, yes, I'm assuming we're only talking about the current connection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I think I see what you're saying now. If the prohibition is read as "you must never use this CID again for anything ever," then the client would be obligated to keep such CIDs persisted and make sure that it never has a randomly-chosen initial CID on a future connection which happens to collide with a CID that got a stateless reset at some point on a past connection. Yeah, this prohibition is connection-scoped, because now the Stateless Reset token has been leaked and it's vulnerable to packet injection attacks.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks

@@ -1490,8 +1490,8 @@ and connection ID to reach a different server instance which does not posses the
necessary connection state. Receiving a Stateless Reset in response to a probing
packet SHOULD NOT terminate the connection, but MUST cause the endpoint to
consider path validation to have failed. After receiving a Stateless Reset, the
client MUST NOT send additional packets with the Connection ID used on the
probing packet.
client MUST NOT send additional packets on the same connection the Connection ID
Copy link
Member

Choose a reason for hiding this comment

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

missing a word here somewhere

@@ -1485,6 +1485,13 @@ path validation with other frames. For instance, an endpoint may pad a packet
carrying a PATH_CHALLENGE for PMTU discovery, or an endpoint may bundle a
PATH_RESPONSE with its own PATH_CHALLENGE.

Differences in routing on the Internet might cause the same destination address
and connection ID to reach a different server instance which does not possess
the necessary connection state. Receiving a Stateless Reset in response to a
Copy link
Member

Choose a reason for hiding this comment

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

May I question if it is a good idea to let a different server (that does not know the connection state) send a Stateless Reset?

My understanding is that the static key that is used to generate stateless reset tokens cannot be shared between endpoints that do not share the connection states. The attack (credit goes to @siyengar) is as follows.

Consider the case where we have two different servers A and B that do not share the connection state but shares the static key that is being used to generate the stateless reset tokens.

An on-path attacker wants to kill a connection that goes to A. The attacker creates a packet that carries the same CID as the connection he/she wants to kill, and send it to server B. Server B returns a stateless reset. The attacker then forwards the stateless reset to the client and the connection gets reset.

To summarize, I believe that you can send a stateless reset only from a server that knows (or should have known) the connection state.

Copy link
Contributor Author

@MikeBishop MikeBishop Apr 9, 2018

Choose a reason for hiding this comment

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

Yes, that's the attack in #1230, and we don't currently have good guidance around that problem. I think limiting the scope of the key is probably the right choice, but unfortunately that's not something the protocol (or even the implementations) control -- it's a deployment decision.

Probably the right choice is some text in Security Considerations.

Copy link
Member

Choose a reason for hiding this comment

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

@MikeBishop Thank you for the answer and pointing to your comment on #1230, I missed that. I will make my future comments there.

@martinthomson
Copy link
Member

Based on the discussion thus far, I don't think that we should take this. Instead, we should make it clear that whatever instances share a key for stateless reset MUST use the same connection ID space. Otherwise we'll end up deep in all sorts of nasty DoS problems.

@MikeBishop
Copy link
Contributor Author

I'm amenable to that. It has the interesting result that, during path validation, we'll be provoking Stateless Resets that parse as invalid. Some implementations might still want to use that as a hint that the path isn't likely to work, but we don't need that to be normative.

martinthomson added a commit that referenced this pull request May 18, 2018
@MikeBishop MikeBishop closed this May 24, 2018
@MikeBishop MikeBishop deleted the sr-during-pv branch June 14, 2018 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants