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.

Sign up for GitHub

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

Jump to bottom

Implement more comprehensive SSRF mitigation #6362

Merged
merged 2 commits into from
Jun 29, 2023
Merged

Implement more comprehensive SSRF mitigation #6362

merged 2 commits into from
Jun 29, 2023

Commits on Jun 25, 2023

  1. Implement more comprehensive SSRF mitigation 

    The current mitigation approach (resolving the IP address and checking if it's
in the private range) is insufficient for a few reasons:

* It is susceptible to DNS rebinding (an attacker-controlled DNS name resolving
  to a public IP address when queried during the check, and to a private IP
  address afterwards).

* It is susceptible to redirect-based attacks (a server with a public address
  redirecting to a server with a private address).

* It is only applied when downloading remote files of tasks (and is not easily
  reusable).

Replace it with an approach based on smokescreen, a proxy that blocks connections
to private IP addresses. In addition, use this proxy for webhooks, since they also
make requests to untrusted URLs.

The benefits of smokescreen are as follows:

* It's not susceptible to the problems listed above.

* It's configurable, so system administrators can allow certain private IP ranges
  if necessary. This configurability is exposed via the `SMOKESCREEN_OPTS`
  environment variable.

* It doesn't require much code to use.

The drawbacks of smokescreen are:

* It's not as clear when the request fails due to smokescreen (compared to
  manual IP validation). To compensate, make the error message in
  `_download_data` more verbose.

* The smokescreen project seems to be in early development (judging by the
  0.0.x version numbers). Still, Stripe itself uses it, so it should be good
  enough. It's also not very convenient to set up (on account of not providing
  binaries), so disable it in development environments.

Keep the scheme check from `_validate_url`. I don't think this check
prevents any attacks (as requests only supports http/https to begin with),
but it provides a friendly error message in case the user tries to use an
unsupported scheme.
    @SpecLad
    SpecLad committed Jun 25, 2023
    Configuration menu
    Copy the full SHA
    68fdf4d View commit details
    Browse the repository at this point in the history

  2. Adapt the Helm chart to the usage of smokescreen

    @SpecLad
    SpecLad committed Jun 25, 2023
    Configuration menu
    Copy the full SHA
    8725383 View commit details
    Browse the repository at this point in the history