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

redesign the Retry Token API #3494

Closed
marten-seemann opened this issue Aug 3, 2022 · 2 comments · Fixed by #3501
Closed

redesign the Retry Token API #3494

marten-seemann opened this issue Aug 3, 2022 · 2 comments · Fixed by #3501

Comments

@marten-seemann
Copy link
Member

marten-seemann commented Aug 3, 2022

Our current API (the AcceptToken config option) is confusing and error-prone. In particular:

  1. It requires users to understand (in detail!) how QUIC's address validation works.
    • This includes the distinction between tokens sent in Retry packets and tokens sent in NEW_TOKEN frames.
    • This callback is called twice, when receiving the first packet on a new connection, and to validate the token.
  2. It is possible to send more than a single Retry (which is forbidden by the RFC) by returning false multiple times for a connection attempt.
  3. It requires users to reimplement the defaultAcceptToken logic, if all they want to do is require address validation when their application is under load.
  4. It makes it impossible to know if the callback actually validated the address (in which case we can remove the 3x amplification limit), or just rubber-stamped the connection attempt (in which case we still need to apply the amplification limit). See Disable anti-amplification limit by address validation token #3326 (review) for details.

I suggest replacing the AcceptToken callback with 3 config options:

  1. RequireAddressValidation(net.Addr) bool: if set and returns true, the server will perform a Retry. It is called:
    • for new connection attempts (Initial packet with empty token)
    • for Initial packets where the token cannot be decoded. This can happen if the server lost state (reboot) and can't decrypt the token any more.
    • for Initial packets with a token issued on a previous connection, if validation of that token fails
  2. MaxTokenAge time.Duration (default: 24h): allows controlling the maximum age of a token issued on a previous connection. Most ISPs assign new IP addresses once a day, so 24h seems to be a reasonable default.
  3. MaxRetryTokenAge time.Duration (default: 5s): allows controlling the maximum age of a token issued in a Retry packet. Such a token has to be used on the same connection attempt, so 5s seems to be a reasonable default.

In case the token is younger than the maximum age for the respective token type, the server would perform the following validation:

  • if the address is a net.UDPAddr (this is the common case): validate that the IP (but not necessarily the port) from the token matches the current remote address
  • if the address is not a net.UDPAddr (can happen if running on top of a custom net.PacketConn): validate that addr.String() from the token matches the current address

Open question: Is there any use case where a perfect match between the IP address in the token and the current IP address is not required? Are there cases where one would accept an entire address range as valid? What about IPv6?

@marten-seemann
Copy link
Member Author

Looks like RFC 9000 Section 8.1.4 answers the open question:

Tokens sent in NEW_TOKEN frames MUST include information that allows the server to verify that the client IP address has not changed from when the token was issued.

If we're that strict about address changes that happen between two subsequent connections, there's no reason to be less strict for the address validation that happens during the handshake. In fact, we should also validate that the port didn't change, when using a Retry token.

@mholt
Copy link
Contributor

mholt commented Aug 3, 2022

I feel like problems 1 and 3 are most relevant to me/Caddy.

  1. It requires users to reimplement the defaultAcceptToken logic, if all they want to do is require address validation when their application is under load.

Yeah, so I think this is what I'm interested in.

Is there a reason that defaultAcceptToken would ever NOT be used? I'm wondering if you could just always run that logic as a baseline, and then if it passes, then proceed to run the user's function after that.

I think I like your suggested solution though. It will still let us mitigate amplification attacks when under load, yeah?

Am excited to get this wrapped up for Caddy 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants