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

feat(s2n-quic-dc): Periodically re-handshake existing path secrets #2276

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

Mark-Simulacrum
Copy link
Collaborator

Description of changes:

  • Remove unused ongoing_handshakes storage, since we no longer expect to use it. This is just drive-by cleanup.
    • See commit message for details on justification.
  • Introduce new mechanism to enqueue a secret as stale based on time since it was added. Currently we keep requesting new handshakes every minute (at a random time), which is probably too often, but should be fine for current usage. A follow-up PR will add a builder for configuring the Map, which will ease providing knobs for customer-dependent usage.

Testing:

Unfortunately, essentially none yet. I suspect that we'll want to have a long-lived canary of some kind that observes handshakes rates, etc. in practice as our highest fidelity test. Periodic, randomized functionality is otherwise pretty annoying to test.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

The correctness property referenced in the removed comment is/will be
moved into s2n-quic itself via deduplicate flag on the connection
attempts. Our goal is to ensure that the path secret in `peers` is the
most recent path secret from the perspective of both the initiator
(client) and acceptor (server) side. If handshakes to the same peer
could overlap, then the server may see secrets from the same peer appear
in a different order than the client. That situation would eventually
lead to us being more likely to remove the wrong secret, since we
incorrectly think it's been replaced.
This avoids evicting state, but peers should be periodically
re-handshaked to rotate the symmetric keys. We will request a handshakes
starting at a random time and until there is a newer secret inserted, we
will keep requesting a handshake roughly every minute (how often our
background cleaning thread runs).
Copy link
Contributor

@camshaft camshaft left a comment

Choose a reason for hiding this comment

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

Looks great! Much simpler than I was thinking initially.

@camshaft camshaft enabled auto-merge (squash) July 18, 2024 21:14
@camshaft camshaft merged commit 2c95dd9 into aws:main Jul 18, 2024
118 of 119 checks passed
@Mark-Simulacrum Mark-Simulacrum deleted the refactor-path-secret branch July 19, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants