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

fix(auth): Include state in authentication requests #743

Merged
merged 6 commits into from Sep 1, 2020
Merged

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Sep 1, 2020

Changes the authentication payloads so that Sentry can authenticate without keeping state between the request and response API calls. This fixes a range of issues with the former authentication system.

Issues

  • Only one Relay could use the same identifier. While this was intended, it introduced a race condition and DoS vector to deny authentication if multiple Relays authenticate concurrently with the same identifier. In the worst case, an attacker could prevent authentication using replay attacks.
  • The upstream was required to keep state between the register and response calls. Spamming the upstream with register requests would result in increased resource usage over time.
  • The random token created by the upstream was enforced at no point. This means that the downstream Relay could have swapped the token and still succeed with registration.

Changes

This change introduces RegisterState, a data container that is signed with a secret by the upstream. As it contains the public key and a timestamp, it ensures that the downstream cannot tamper with the state or use another Relay's token. Since the public key of the state is also used to verify the Relay request signature, the upstream can rely on the state without keeping caches. By encoding the state into the token, the HTTP API remains backwards compatible.

In addition to this, the default challenge TTL has been reduced from 15 minutes to 60 seconds.

Release

This change contains breaking changes to the Python API and requires a minor bump of the sentry_relay library. It is backwards compatible on the HTTP API. After updating in Sentry, old Relays will also use the new authentication system.

Deployment

This change can be deployed independently to Sentry and Relay in no particular order. Updating the library in Sentry will require code changes.

There is a minimal potential for breakage if Relays are authenticating while Sentry is being deployed. However, Relays will retry on authentication failure and as such, the condition resolves automatically as soon as the deployment finishes.

@jan-auer jan-auer requested review from mitsuhiko and a team September 1, 2020 14:09
@jan-auer jan-auer self-assigned this Sep 1, 2020
@jan-auer jan-auer requested a review from tonyo September 1, 2020 14:19
relay-auth/src/lib.rs Outdated Show resolved Hide resolved
@jan-auer jan-auer merged commit 1897b78 into master Sep 1, 2020
@jan-auer jan-auer deleted the fix/relay-auth branch September 1, 2020 15:35
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.

None yet

3 participants