-
Notifications
You must be signed in to change notification settings - Fork 21
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
switch to using 3xmaxPTO between key updates #257
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please specify the "PTO" a bit more precisely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works, but it’s additional complexity compared to RFC 9001. #215 proposes a different solution that doesn’t require us to do anything differently.
@marten-seemann I do not think changing 3PTO to 3maxPTO is a great deal. For existing QUIC stacks using three keys and PN in conjunction, switching to two keys would be a simplification. Compared to that, it is my opinion that using different keys on each path and running key updates separately as proposed in #215 is an unnecessary complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take it or leave it, but we might want to clarify that this value is different from the recommendation that we have in QUIC version 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the edited version, and I agree with the suggested change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's simpler than the current text, therefore I think we should merge it for now (and potentially keep discussing further changes later).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's very clear. Looks good to me.
Closes #209.