mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 21:47:20 +00:00
3d4a3e1b87
This PR changes 3 things. 1. Validate `admin_url` + all URLs for HTTPS/non-private 2. Add custom `DialContext` hook in fleethttp.NewClient(), this is needed for DNS-rebinding protection at connection time 3. Validate Smallstep SCEP challenge endpoint # **IMPORTANT** There are two validations occurring. 1. `CheckURLForSSRF` 2. `SSRFDialContext` ## Why? `CheckURLForSSRF` checks the hostname. It resolves DNS, validates the ip, and then returns an error to the user. It protects certificate authority create/update API endpoints. But then `GetSmallstepSCEPChallenge` calls `http.NewRequest(http.MethodPost, ca.ChallengeURL, ...)` with the original hostname This is where `SSRFDialContext` comes into play. It fires when an actual HTTP request is attempted. Meaning Fleet would first build the request, encode the body, set up TLS, etc., before being blocked at the dial. `CheckURLForSSRF` stops the operation before any of that work happens. `SSRFDialContext` protects the actual challenge fetch that happens later at enrollment time. They're not always called together. The dial-time check is the only thing protecting the enrollment request and DNS rebinding. ## Should we remove `CheckURLForSSRF` This is debatable and I don't have a strong opinion. Removing `CheckURLForSSRF` would still provide the same protection. However, it would return a generic connection error from the HTTP client which would make it slightly hard to diagnose why it is broken. ## What's next I implemented this for certificate authorities. I am sure there are other places in the code base that take user submitted urls and could also use this check. That is outside the scope of this particular PR. But worthy to investigate in the near future. If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * Added SSRF protections for validating external URLs and blocking private/IP-metadata ranges; dev mode can bypass checks for local testing * **New Features** * Introduced an SSRF-protected HTTP transport and an option to supply a custom transport per client * **Tests** * Added comprehensive tests covering SSRF validation, dialing behavior, and resolution edge cases <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|---|---|---|
| .. | ||
| fleetctl | ||
| integrationtest | ||
| main.go | ||