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

Prevent linkability from responding to migration #2969

Merged
merged 7 commits into from
Sep 13, 2019

Conversation

martinthomson
Copy link
Member

This closes the rather serious hole we left when we attempted to limit
the effect of perpetual changing of connection IDs. This uses a
narrower formulation, that I believe will avoid perpetual changes. But
it does require reciprocal connection ID changes where endpoints
genuinely do migrate.

It's a design change unfortunately, but I hope non-controversial.

Closes #2778.

This closes the rather serious hole we left when we attempted to limit
the effect of perpetual changing of connection IDs.  This uses a
narrower formulation, that I believe will avoid perpetual changes.  But
it does require reciprocal connection ID changes where endpoints
genuinely do migrate.

It's a design change unfortunately, but I hope non-controversial.

Closes #2778.
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Aug 15, 2019
Rather than focus on migration, use address change, which is far more
direct and easier to test.

Also, rather than require that the connection ID be previously unused in
a global sense, scope it to those that are active.
Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

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

Thank you for the changes. I think it's much better, the discussion of "active" connection ID looks good.

ensures that packet numbers cannot be used to correlate activity. This does not
prevent other properties of packets, such as timing and size, from being used to
An endpoint MUST use a new connection ID if it initiates connection migration as
described in {{initiating-migration}}. An endpoint MUST use a new 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.

Maybe add something like: "or when probing a new path ({{#probing}})"?

described in {{initiating-migration}}. An endpoint MUST use a new connection ID
in response to a change in the address of a peer if the packet with the new peer
address uses an active connection ID that has not been previously used by the
peer.
Copy link
Member

Choose a reason for hiding this comment

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

What should the responding endpoint do, when it does not have any unused Connection IDs?

I think we have at least four options, but neither of them seem to be very good: a) respond using a Connection ID already in use, b) error-close the connection (on the active path), c) respond with a stateless reset, d) drop the packet.

Option a has the privacy concern. I do not think that option b or c is viable, as it creates an attack vector (a middlebox might race the packet that initiates the migration from a different source address. If the spoofed packet reaches the server before the legitimate one, the connection might get dropped as the server might have used all the spare connection IDs for responding to the spoofed packets). Option d seems a bit odd.

Relatedly, Section 9.1 (Probing a New Path) states quote: An endpoint that uses a new local address needs to ensure that at least one new connection ID is available at the peer. That can be achieved by including a NEW_CONNECTION_ID frame in the probe. I think that this is a good advice and that we might want to promote the sentences to here, so that it would cover both path probing and immediate connection migration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, option d is the only viable option I can see. I will add text to that effect.

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 added a recommendation to send NEW_CONNECTION_ID when migrating. It's a little janky, because you have to include the frame in every packet to be effective. Unless you like the wonderful cascading loss that results from your peer being forced to drop all your packets because the golden one didn't get through. Of course, then you need to worry about how to manage that retransmission logic...

Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the changes.

Copy link
Contributor

@erickinnear erickinnear left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up! Latest content looks great, just two minor wording questions and this looks ready to go.

that migration is possible and packets sent on different paths cannot be
correlated, endpoints SHOULD provide new connection IDs before peers migrate.
An endpoint that exhausts available connection IDs cannot migrate. Similarly,
an endpoint is unable to respond to probes or an attempt by its peer to migrate
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 parses oddly for me, I assume it's in the context of the previous sentence, but maybe something like

An endpoint that exhausts available connection IDs cannot probe new paths or initiate 
migration, nor can it respond to probes or attempts by its peer to migrate.

and replace both sentences.
(Not providing that as a suggested change since it'll mess up the wrapping, sorry!)

an endpoint is unable to respond to probes or an attempt by its peer to migrate
or probe. To ensure that migration is possible and packets sent on different
paths cannot be correlated, endpoints SHOULD provide new connection IDs before
peers migrate. If a peer might have exhausted available connection IDs, a
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to reference the other section that talks about SHOULD provide a new CID for every one that gets retired, etc.? I think that's {{issue-cid}}.

Specifically the below paragraph:

An endpoint SHOULD supply a new connection ID when it receives a packet with a
previously unused connection ID or when the peer retires one, unless providing
the new connection ID would exceed the peer's limit.  An endpoint MAY limit the
frequency or the total number of connection IDs issued for each connection to
avoid the risk of running out of connection IDs; see {{reset-token}}.
Suggested change
peers migrate. If a peer might have exhausted available connection IDs, a
peers migrate ({{issue-cid}}). If a peer might have exhausted available connection IDs, a

If that doesn't mess up the spacing.

@erickinnear
Copy link
Contributor

Latest changes look great! 👍

@martinthomson martinthomson merged commit 0ac214e into master Sep 13, 2019
@martinthomson martinthomson deleted the respond-migration-cid branch September 13, 2019 04:22
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.

CID change still required in response to migration?
6 participants