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 -->
302 lines
9.3 KiB
Go
302 lines
9.3 KiB
Go
package fleethttp
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// noopResolver returns a known public IP so that CheckURLForSSRF reaches the
|
|
// IP-range check without triggering real DNS lookups.
|
|
func noopResolver(_ context.Context, _ string) ([]string, error) {
|
|
return []string{"93.184.216.34"}, nil // example.com
|
|
}
|
|
|
|
func TestCheckURLForSSRFBlockedLiteralIPs(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
blocked := []string{
|
|
"http://127.0.0.1/mscep/mscep.dll",
|
|
"http://127.255.255.255/path",
|
|
"http://10.0.0.1/admin",
|
|
"http://10.255.255.255/admin",
|
|
"http://172.16.0.1/admin",
|
|
"http://172.31.255.255/admin",
|
|
"http://192.168.0.1/admin",
|
|
"http://192.168.255.255/admin",
|
|
"http://169.254.169.254/latest/meta-data/",
|
|
"http://169.254.0.1/whatever",
|
|
"http://100.64.0.1/admin",
|
|
"http://100.127.255.255/admin",
|
|
"http://0.0.0.0/path",
|
|
"http://[::1]/path",
|
|
"http://[fe80::1]/path",
|
|
"http://[fc00::1]/path",
|
|
"http://[fdff::1]/path",
|
|
}
|
|
|
|
for _, u := range blocked {
|
|
t.Run(u, func(t *testing.T) {
|
|
t.Parallel()
|
|
err := CheckURLForSSRF(context.Background(), u, noopResolver)
|
|
require.Error(t, err, "expected SSRF block for %s", u)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr), "expected SSRFError for %s, got %T: %v", u, err, err)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestCheckURLForSSRFAllowedPublicIPs(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
allowed := []string{
|
|
"https://ndes.corp.example.com/mscep/mscep.dll",
|
|
"https://93.184.216.34/path", // example.com
|
|
"http://8.8.8.8/path", // Google DNS
|
|
"https://1.1.1.1/path", // Cloudflare DNS
|
|
}
|
|
|
|
for _, u := range allowed {
|
|
t.Run(u, func(t *testing.T) {
|
|
t.Parallel()
|
|
err := CheckURLForSSRF(context.Background(), u, noopResolver)
|
|
assert.NoError(t, err, "expected no SSRF block for %s", u)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestCheckURLForSSRFDNSResolutionBlocked(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Simulate a hostname that resolves to a private IP
|
|
privateResolver := func(_ context.Context, _ string) ([]string, error) {
|
|
return []string{"192.168.1.100"}, nil
|
|
}
|
|
|
|
err := CheckURLForSSRF(context.Background(), "https://attacker-controlled.example.com/admin", privateResolver)
|
|
require.Error(t, err)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr))
|
|
assert.Equal(t, net.ParseIP("192.168.1.100").String(), ssrfErr.IP.String())
|
|
}
|
|
|
|
func TestCheckURLForSSRFMetadataEndpoints(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
metadataURLs := []string{
|
|
"http://169.254.169.254/latest/meta-data/iam/security-credentials/",
|
|
"http://169.254.169.254/metadata/instance?api-version=2021-02-01",
|
|
}
|
|
|
|
for _, u := range metadataURLs {
|
|
t.Run(u, func(t *testing.T) {
|
|
t.Parallel()
|
|
err := CheckURLForSSRF(context.Background(), u, noopResolver)
|
|
require.Error(t, err)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr))
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestCheckURLForSSRFBadScheme(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
err := CheckURLForSSRF(context.Background(), "file:///etc/passwd", noopResolver)
|
|
require.Error(t, err)
|
|
var ssrfErr *SSRFError
|
|
assert.False(t, errors.As(err, &ssrfErr), "bad-scheme error should not be an SSRFError")
|
|
}
|
|
|
|
func TestCheckURLForSSRFResolverError(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
failResolver := func(_ context.Context, _ string) ([]string, error) {
|
|
return nil, errors.New("simulated DNS failure")
|
|
}
|
|
err := CheckURLForSSRF(context.Background(), "https://cant-resolve.example.com/admin", failResolver)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "resolving host")
|
|
}
|
|
|
|
func TestCheckURLForSSRF_UnparseableAddressFailsClosed(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// A custom resolver returning a non-IP string will be blocked
|
|
badResolver := func(_ context.Context, _ string) ([]string, error) {
|
|
return []string{"not-an-ip"}, nil
|
|
}
|
|
err := CheckURLForSSRF(context.Background(), "https://example.com/admin", badResolver)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "not a valid IP")
|
|
}
|
|
|
|
func TestCheckURLForSSRFMultipleResolutions(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
mixedResolver := func(_ context.Context, _ string) ([]string, error) {
|
|
return []string{"93.184.216.34", "10.0.0.1"}, nil
|
|
}
|
|
err := CheckURLForSSRF(context.Background(), "https://mixed.example.com/admin", mixedResolver)
|
|
require.Error(t, err)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr))
|
|
assert.Equal(t, net.ParseIP("10.0.0.1").String(), ssrfErr.IP.String())
|
|
}
|
|
|
|
func TestCheckURLForSSRFIPv4MappedBypass(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// An attacker could supply an IPv4-mapped IPv6 address like ::ffff:192.168.1.1
|
|
// to reach a private IPv4 host while bypassing the IPv4 blocklist check.
|
|
blocked := []string{
|
|
"http://[::ffff:192.168.1.1]/admin", // RFC 1918 private
|
|
"http://[::ffff:127.0.0.1]/admin", // Loopback
|
|
"http://[::ffff:169.254.169.254]/admin", // Link-local metadata
|
|
"http://[::ffff:10.0.0.1]/admin", // RFC 1918 private
|
|
}
|
|
for _, u := range blocked {
|
|
t.Run(u, func(t *testing.T) {
|
|
t.Parallel()
|
|
err := CheckURLForSSRF(context.Background(), u, noopResolver)
|
|
require.Error(t, err, "expected SSRF block for IPv4-mapped %s", u)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr), "expected SSRFError for %s, got %T: %v", u, err, err)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestCheckURLForSSRFSSRFErrorMessage(t *testing.T) {
|
|
err := CheckURLForSSRF(context.Background(), "http://127.0.0.1/admin", noopResolver)
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "blocked address")
|
|
assert.Contains(t, err.Error(), "127.0.0.1")
|
|
}
|
|
|
|
// noopDial is used as the dial parameter so tests never open real sockets.
|
|
func noopDial(_ context.Context, _, _ string) (net.Conn, error) {
|
|
return nil, errors.New("no-op dial: connection not attempted in tests")
|
|
}
|
|
|
|
// captureDial records the addr passed to dial without opening a real socket.
|
|
func captureDial(got *string) func(ctx context.Context, network, addr string) (net.Conn, error) {
|
|
return func(_ context.Context, _, addr string) (net.Conn, error) {
|
|
*got = addr
|
|
return nil, errors.New("no-op dial: connection not attempted in tests")
|
|
}
|
|
}
|
|
|
|
// staticResolver returns a fixed list of IPs for any host.
|
|
func staticResolver(ips ...string) func(ctx context.Context, host string) ([]string, error) {
|
|
return func(_ context.Context, _ string) ([]string, error) {
|
|
return ips, nil
|
|
}
|
|
}
|
|
|
|
func TestSSRFDialContextBlocksPrivateIPs(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
blocked := []struct {
|
|
addr string
|
|
ip string
|
|
}{
|
|
{"127.0.0.1:80", "127.0.0.1"},
|
|
{"10.0.0.1:443", "10.0.0.1"},
|
|
{"172.16.0.1:8080", "172.16.0.1"},
|
|
{"192.168.1.1:443", "192.168.1.1"},
|
|
{"169.254.169.254:80", "169.254.169.254"},
|
|
}
|
|
|
|
for _, tc := range blocked {
|
|
t.Run(tc.addr, func(t *testing.T) {
|
|
t.Parallel()
|
|
dial := SSRFDialContext(nil, staticResolver(tc.ip), noopDial)
|
|
conn, err := dial(context.Background(), "tcp", tc.addr)
|
|
require.Error(t, err, "expected dial to be blocked for %s", tc.addr)
|
|
assert.Nil(t, conn)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr), "expected SSRFError for %s, got %T: %v", tc.addr, err, err)
|
|
assert.Equal(t, net.ParseIP(tc.ip).String(), ssrfErr.IP.String())
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestSSRFDialContextAllowsPublicIPs(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
publicIPs := []string{
|
|
"93.184.216.34",
|
|
"8.8.8.8",
|
|
"1.1.1.1",
|
|
}
|
|
|
|
for _, publicIP := range publicIPs {
|
|
t.Run(publicIP, func(t *testing.T) {
|
|
t.Parallel()
|
|
dial := SSRFDialContext(nil, staticResolver(publicIP), noopDial)
|
|
_, err := dial(context.Background(), "tcp", publicIP+":80")
|
|
var ssrfErr *SSRFError
|
|
assert.False(t, errors.As(err, &ssrfErr), "public IP %s should not be SSRF-blocked", publicIP)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestSSRFDialContextBlocksMixedResolution(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Simulates DNS rebinding: resolver returns one public and one private IP.
|
|
dial := SSRFDialContext(nil, staticResolver("93.184.216.34", "192.168.1.100"), noopDial)
|
|
|
|
_, err := dial(context.Background(), "tcp", "attacker.example.com:443")
|
|
require.Error(t, err)
|
|
var ssrfErr *SSRFError
|
|
assert.True(t, errors.As(err, &ssrfErr))
|
|
assert.Equal(t, net.ParseIP("192.168.1.100").String(), ssrfErr.IP.String())
|
|
}
|
|
|
|
func TestSSRFDialContextResolverError(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
failResolver := func(_ context.Context, _ string) ([]string, error) {
|
|
return nil, errors.New("simulated DNS failure")
|
|
}
|
|
dial := SSRFDialContext(nil, failResolver, noopDial)
|
|
_, err := dial(context.Background(), "tcp", "cant-resolve.example.com:443")
|
|
require.Error(t, err)
|
|
assert.Contains(t, err.Error(), "resolving")
|
|
}
|
|
|
|
func TestSSRFDialContextDialsResolvedIP(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
var gotAddr string
|
|
dialFn := SSRFDialContext(nil, staticResolver("93.184.216.34"), captureDial(&gotAddr))
|
|
_, _ = dialFn(context.Background(), "tcp", "example.com:443")
|
|
|
|
// The dialer must receive the resolved IP, not "example.com".
|
|
host, port, err := net.SplitHostPort(gotAddr)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "443", port)
|
|
assert.NotEmpty(t, host)
|
|
assert.NotEqual(t, "example.com", host, "dialer must receive resolved IP, not the hostname")
|
|
assert.Equal(t, net.ParseIP("93.184.216.34").String(), net.ParseIP(host).String())
|
|
}
|
|
|
|
func TestSSRFDialContextNilsUseDefaults(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
dial := SSRFDialContext(nil, nil, nil)
|
|
require.NotNil(t, dial)
|
|
}
|
|
|
|
func TestNewSSRFProtectedTransportHasDialContext(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tr := NewSSRFProtectedTransport()
|
|
require.NotNil(t, tr.DialContext, "NewSSRFProtectedTransport() must set DialContext for SSRF protection")
|
|
}
|