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

Remember Fewer Transport Parameters for 0-RTT #2464

Closed
martinduke opened this issue Feb 14, 2019 · 9 comments · Fixed by #2467
Closed

Remember Fewer Transport Parameters for 0-RTT #2464

martinduke opened this issue Feb 14, 2019 · 9 comments · Fixed by #2467
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@martinduke
Copy link
Contributor

Sec 7.3.1 of quic-transport says:
A client that attempts to send 0-RTT data MUST remember the transport parameters used by the server. The transport parameters that the server advertises during connection establishment apply to all connections that are resumed using the keying material established during that handshake. Remembered transport parameters apply to the new connection until the handshake completes and new transport parameters from the server can be provided.

Later on, the section makes it clear that this is really about the flow control parameters, which makes a lot of sense, but here I am storing all this data to be compliant.

I propose that we change the MUST above to refer to only the 6 FC parameters. This reduces the stuff that clients have to store.

going through each:
original_connection_id: clearly useless
idle_timeout: useless in handshake
stateless_reset_token: clearly useless
max_packet_size: could be useful, but meh
ack_delay_exponent: not useful before we get the server Handshake
max_ack_delay: same
disable_migration: same
preferred_address: same

@MikeBishop
Copy link
Contributor

It's not just the flow control parameters -- it's also support for extensions in TPs yet-to-be-defined. So while I would be amenable to reorienting this issue to create an exception for the ones you've marked as useless, I think this is the correct default rule.

@martinduke
Copy link
Contributor Author

good point. I'm happy to write a PR as you describe if others agree.

@ianswett
Copy link
Contributor

I agree that a good number of those are useless.

Possibly we should explicitly state which params need to be stored and extensions should be required to state whether they need to be stored when they're defined?

@martinthomson
Copy link
Member

How is anyone going to know if you don't remember these values? Just remember the ones you know you need to remember.

@MikeBishop
Copy link
Contributor

A conformance test that deliberately lowers them after accepting 0-RTT and checks whether the client chokes.

@martinthomson
Copy link
Member

You can't lower preferred addresses, disable migration, etc.. Basically, we should allow all those that @martinduke points out make no sense to remember to change. That's the real fix here.

@mnot mnot added the design An issue that affects the design of the protocol; resolution requires consensus. label Feb 19, 2019
@janaiyengar
Copy link
Contributor

This makes sense, and the list is good.

@mnot mnot added the proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. label Mar 27, 2019
@mnot mnot added has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. and removed proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. labels Apr 23, 2019
@mnot
Copy link
Member

mnot commented Apr 23, 2019

Editors: please don't close the issue before consensus is declared.

@MikeBishop
Copy link
Contributor

For context, the PR was marked editorial but the issue it auto-closed was marked design. Comment from the PR discussion:

Thanks for doing this Martin. I will raise this on the list so that people are clear about this. Technically, there isn't a change here, but it certainly looks like it if you don't understand what is going on.

The PR changes the text from "MUST remember everything" to "MUST NOT remember the things that don't make sense to remember and you weren't actually remembering anyway, but MUST remember everything else". That is, there's no functional change to real implementations, but the normative requirements in the text didn't match what was actually possible to implement. Martin's right -- this is really an editorial change, despite touching normative text.

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. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants