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 Cleanup #1328

Closed
wants to merge 2 commits into from
Closed

Stateless Reset Cleanup #1328

wants to merge 2 commits into from

Conversation

martinduke
Copy link
Contributor

The Stateless Reset is just a bunch of random bytes except the first one and the token, so I simplified the spec to reflect that.

The Stateless Reset is just a bunch of random bytes except the first one and the token, so I simplified the spec to reflect that.
Improved logical ordering of paragraphs.
@martinduke
Copy link
Contributor Author

The spec is clear that the CID is random, so it's all just random bytes between the short header byte and the token.

However, an alternate way to improve this section is to leave it pretty much the same, except to say that if the stateless reset is in response to a long header, servers should use the SCID instead.

Either is fine with me.

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

Not unreasonable.

length, followed by a Stateless Reset Token.

The server SHOULD send a packet with a short header and a packet number length
of 1 octet. Using the hortest possible packet number encoding minimizes the
Copy link
Contributor

Choose a reason for hiding this comment

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

shortest

packet number length implied by the type field. A packet of this length
simulates a packet with a short header, the maximum-length destination
connection ID, a packet number of the appropriate length and (with the
Stateless Reset Token) an authentication trailer, with no packet payload.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are fully-empty packets permitted? If not, perhaps we should mandate that this be slightly longer, so it appears there's a payload.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's not leak that. The simple fix here is s/18/19/

Stateless Reset Token) an authentication trailer, with no packet payload.

In response to a packet with a short header, the server has no means of
determining the source connection ID and is effectively using a random one
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth being precise about source in which direction. Maybe "The server has no means of determining a short packet's source connection ID, and therefore uses a random destination ID in the Stateless Reset packet."

Copy link
Member

Choose a reason for hiding this comment

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

Reasonable, though make sure to state "destination connection ID"

@MikeBishop MikeBishop added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Apr 26, 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.

This is a fine change, though I have a few comments. (If @martinduke doesn't get around to these, I will do so once I've finished my post-break triage.)

packet number length implied by the type field. A packet of this length
simulates a packet with a short header, the maximum-length destination
connection ID, a packet number of the appropriate length and (with the
Stateless Reset Token) an authentication trailer, with no packet payload.
Copy link
Member

Choose a reason for hiding this comment

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

Yes, let's not leak that. The simple fix here is s/18/19/

Stateless Reset Token) an authentication trailer, with no packet payload.

In response to a packet with a short header, the server has no means of
determining the source connection ID and is effectively using a random one
Copy link
Member

Choose a reason for hiding this comment

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

Reasonable, though make sure to state "destination connection ID"

In response to a packet with a short header, the server has no means of
determining the source connection ID and is effectively using a random one
instead. For a client that depends on the server including a connection ID,
this will mean that this value differs from previous packets. Ths results in
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: s/Ths/This

@martinthomson
Copy link
Member

OBE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants