Skip to content

Added deny list for checking external user submitted urls #39947

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

Merged
ksykulev merged 8 commits into from
Feb 17, 2026
Merged

Added deny list for checking external user submitted urls #39947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0ec6642
Added deny list for checking external user submitted urls
ksykulev Feb 16, 2026
6c19668
ai code review comments
ksykulev Feb 16, 2026
cd4bfa5
Added explicit opt-in ssrf protected transport
ksykulev Feb 16, 2026
b46476e
prevent one last dns rebinding vector
ksykulev Feb 16, 2026
8a2e1fe
Setting dev mode explicitly for ca integration tests
ksykulev Feb 16, 2026
af766b5
adding more --dev flag to tests
ksykulev Feb 16, 2026
9fe8ca5
fixing tests. try multiple resolved ip addresses
ksykulev Feb 17, 2026
81836ec
more test failures
ksykulev Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/14284-external-deny-list
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Added deny list for checking external urls the fleet server will attempt to contact that are user submitted. Refer to pkg/fleethttp/ssrf.go for full list. In development, the --dev flag skips this check so that testing locally is not impacted. Certificate authorities is the first place this is implemented.
5 changes: 4 additions & 1 deletion cmd/fleetctl/integrationtest/gitops/gitops_enterprise_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,10 @@ team_settings:
// At the same time, GitOps uploads Apple profiles that use the newly configured CAs.
func (s *enterpriseIntegrationGitopsTestSuite) TestCAIntegrations() {
t := s.T()

dev_mode.IsEnabled = true
t.Cleanup(func() { dev_mode.IsEnabled = false })

user := s.createGitOpsUser(t)
fleetctlConfig := s.createFleetctlConfig(t, user)

Expand Down Expand Up @@ -3477,7 +3481,6 @@ team_settings:
} else {
require.NoError(t, err)
}

})
}
}
72 changes: 33 additions & 39 deletions ee/server/service/certificate_authorities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"context"
"errors"
"fmt"
"net/url"
"regexp"
"strings"

"github.com/fleetdm/fleet/v4/pkg/fleethttp"
"github.com/fleetdm/fleet/v4/server/authz"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
"github.com/fleetdm/fleet/v4/server/fleet"
Expand Down Expand Up @@ -208,8 +208,8 @@ func (svc *Service) validatePayload(p *fleet.CertificateAuthorityPayload, errPre
}

func (svc *Service) validateDigicert(ctx context.Context, digicertCA *fleet.DigiCertCA, errPrefix string) error {
if err := validateURL(digicertCA.URL, "DigiCert", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, digicertCA.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sDigiCert URL is invalid: %v", errPrefix, err))
Copy link
Copy Markdown
Member

@getvictor getvictor Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, are we getting a clear message in the frontend for this?

Copy link
Copy Markdown
Contributor Author

@ksykulev ksykulev Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example:
Image

}
if digicertCA.APIToken == "" || digicertCA.APIToken == fleet.MaskedPassword {
return fleet.NewInvalidArgumentError("api_token", fmt.Sprintf("%sInvalid API token. Please correct and try again.", errPrefix))
Expand Down Expand Up @@ -321,8 +321,8 @@ func (svc *Service) validateHydrant(ctx context.Context, hydrantCA *fleet.Hydran
if err := validateCAName(hydrantCA.Name, errPrefix); err != nil {
return err
}
if err := validateURL(hydrantCA.URL, "Hydrant", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, hydrantCA.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sHydrant URL is invalid: %v", errPrefix, err))
}
if hydrantCA.ClientID == "" {
return fleet.NewInvalidArgumentError("client_id", fmt.Sprintf("%sInvalid Hydrant Client ID. Please correct and try again.", errPrefix))
Expand All @@ -346,8 +346,8 @@ func (svc *Service) validateEST(ctx context.Context, estProxyCA *fleet.ESTProxyC
if err := validateCAName(estProxyCA.Name, errPrefix); err != nil {
return err
}
if err := validateURL(estProxyCA.URL, "EST", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, estProxyCA.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sEST URL is invalid: %v", errPrefix, err))
}
if estProxyCA.Username == "" {
return fleet.NewInvalidArgumentError("username", fmt.Sprintf("%sInvalid EST Username. Please correct and try again.", errPrefix))
Expand All @@ -361,18 +361,12 @@ func (svc *Service) validateEST(ctx context.Context, estProxyCA *fleet.ESTProxyC
return nil
}

func validateURL(caURL, displayType, errPrefix string) error {
if u, err := url.ParseRequestURI(caURL); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sInvalid %s URL. Please correct and try again.", errPrefix, displayType))
} else if u.Scheme != "https" && u.Scheme != "http" {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%s%s URL scheme must be https or http", errPrefix, displayType))
}
return nil
}

func (svc *Service) validateNDESSCEPProxy(ctx context.Context, ndesSCEP *fleet.NDESSCEPProxyCA, errPrefix string) error {
if err := validateURL(ndesSCEP.URL, "NDES SCEP", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, ndesSCEP.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sNDES SCEP URL is invalid: %v", errPrefix, err))
}
if err := fleethttp.CheckURLForSSRF(ctx, ndesSCEP.AdminURL, nil); err != nil {
return fleet.NewInvalidArgumentError("admin_url", fmt.Sprintf("%sNDES SCEP admin URL is invalid: %v", errPrefix, err))
}
if err := svc.scepConfigService.ValidateSCEPURL(ctx, ndesSCEP.URL); err != nil {
level.Error(svc.logger).Log("msg", "Failed to validate NDES SCEP URL", "err", err)
Expand All @@ -396,8 +390,8 @@ func (svc *Service) validateCustomSCEPProxy(ctx context.Context, customSCEP *fle
if err := validateCAName(customSCEP.Name, errPrefix); err != nil {
return err
}
if err := validateURL(customSCEP.URL, "SCEP", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, customSCEP.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sCustom SCEP Proxy URL is invalid: %v", errPrefix, err))
}
if customSCEP.Challenge == "" || customSCEP.Challenge == fleet.MaskedPassword {
return fleet.NewInvalidArgumentError("challenge", fmt.Sprintf("%sCustom SCEP Proxy challenge cannot be empty", errPrefix))
Expand All @@ -413,8 +407,8 @@ func (svc *Service) validateSmallstepSCEPProxy(ctx context.Context, smallstepSCE
if err := validateCAName(smallstepSCEP.Name, errPrefix); err != nil {
return err
}
if err := validateURL(smallstepSCEP.URL, "Smallstep SCEP", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, smallstepSCEP.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sSmallstep SCEP URL is invalid: %v", errPrefix, err))
}
if smallstepSCEP.Username == "" {
return fleet.NewInvalidArgumentError("username", fmt.Sprintf("%sSmallstep username cannot be empty", errPrefix))
Expand Down Expand Up @@ -1257,10 +1251,9 @@ func (svc *Service) validateDigicertUpdate(ctx context.Context, digicert *fleet.
}
}
if digicert.URL != nil {
if err := validateURL(*digicert.URL, "DigiCert", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *digicert.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sDigiCert URL is invalid: %v", errPrefix, err))
}

// We want to generate a DigiCertCA struct with all required fields to verify the new URL.
// If URL or APIToken are not being updated we use the existing values from oldCA
digicertCA := fleet.DigiCertCA{
Expand Down Expand Up @@ -1338,10 +1331,9 @@ func (svc *Service) validateHydrantUpdate(ctx context.Context, hydrant *fleet.Hy
}
}
if hydrant.URL != nil {
if err := validateURL(*hydrant.URL, "Hydrant", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *hydrant.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sHydrant URL is invalid: %v", errPrefix, err))
}

hydrantCAToVerify := fleet.ESTProxyCA{ // The hydrant service for verification only requires the URL.
URL: *hydrant.URL,
}
Expand Down Expand Up @@ -1370,10 +1362,9 @@ func (svc *Service) validateCustomESTUpdate(ctx context.Context, estUpdate *flee
}
}
if estUpdate.URL != nil {
if err := validateURL(*estUpdate.URL, "EST", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *estUpdate.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sEST URL is invalid: %v", errPrefix, err))
}

hydrantCAToVerify := fleet.ESTProxyCA{ // The EST service for verification only requires the URL.
URL: *estUpdate.URL,
}
Expand All @@ -1399,8 +1390,8 @@ func (svc *Service) validateNDESSCEPProxyUpdate(ctx context.Context, ndesSCEP *f
// some methods in this fuction require the NDESSCEPProxyCA type so we convert the ndes update payload here

if ndesSCEP.URL != nil {
if err := validateURL(*ndesSCEP.URL, "NDES SCEP", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *ndesSCEP.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sNDES SCEP URL is invalid: %v", errPrefix, err))
}
if err := svc.scepConfigService.ValidateSCEPURL(ctx, *ndesSCEP.URL); err != nil {
level.Error(svc.logger).Log("msg", "Failed to validate NDES SCEP URL", "err", err)
Expand All @@ -1414,6 +1405,9 @@ func (svc *Service) validateNDESSCEPProxyUpdate(ctx context.Context, ndesSCEP *f
}
}

if err := fleethttp.CheckURLForSSRF(ctx, *ndesSCEP.AdminURL, nil); err != nil {
return fleet.NewInvalidArgumentError("admin_url", fmt.Sprintf("%sNDES SCEP admin URL is invalid: %v", errPrefix, err))
}
// We want to generate a NDESSCEPProxyCA struct with all required fields to verify the admin URL.
// If URL, Username or Password are not being updated we use the existing values from oldCA
NDESProxy := fleet.NDESSCEPProxyCA{
Expand Down Expand Up @@ -1457,8 +1451,8 @@ func (svc *Service) validateCustomSCEPProxyUpdate(ctx context.Context, customSCE
}
}
if customSCEP.URL != nil {
if err := validateURL(*customSCEP.URL, "SCEP", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *customSCEP.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sCustom SCEP Proxy URL is invalid: %v", errPrefix, err))
}
if err := svc.scepConfigService.ValidateSCEPURL(ctx, *customSCEP.URL); err != nil {
level.Error(svc.logger).Log("msg", "Failed to validate custom SCEP URL", "err", err)
Expand All @@ -1481,8 +1475,8 @@ func (svc *Service) validateSmallstepSCEPProxyUpdate(ctx context.Context, smalls
}
}
if smallstep.URL != nil {
if err := validateURL(*smallstep.URL, "SCEP", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *smallstep.URL, nil); err != nil {
return fleet.NewInvalidArgumentError("url", fmt.Sprintf("%sSmallstep SCEP URL is invalid: %v", errPrefix, err))
}
if err := svc.scepConfigService.ValidateSCEPURL(ctx, *smallstep.URL); err != nil {
level.Error(svc.logger).Log("msg", "Failed to validate Smallstep SCEP URL", "err", err)
Expand All @@ -1506,8 +1500,8 @@ func (svc *Service) validateSmallstepSCEPProxyUpdate(ctx context.Context, smalls

// Additional validation if url was updated
if smallstep.ChallengeURL != nil {
if err := validateURL(*smallstep.ChallengeURL, "Challenge", errPrefix); err != nil {
return err
if err := fleethttp.CheckURLForSSRF(ctx, *smallstep.ChallengeURL, nil); err != nil {
return fleet.NewInvalidArgumentError("challenge_url", fmt.Sprintf("%sChallenge URL is invalid: %v", errPrefix, err))
}
smallstepSCEPProxy.ChallengeURL = *smallstep.ChallengeURL
}
Expand Down
31 changes: 19 additions & 12 deletions ee/server/service/certificate_authorities_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/fleetdm/fleet/v4/ee/server/service/est"
"github.com/fleetdm/fleet/v4/server/authz"
"github.com/fleetdm/fleet/v4/server/contexts/viewer"
"github.com/fleetdm/fleet/v4/server/dev_mode"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/mock"
scep_mock "github.com/fleetdm/fleet/v4/server/mock/scep"
Expand Down Expand Up @@ -144,6 +145,10 @@ func setupMockCAServers(t *testing.T) (digicertServer, hydrantServer *httptest.S
}

func TestCreatingCertificateAuthorities(t *testing.T) {
// Enable dev mode so CheckURLForSSRF skips the private-IP blocklist for the duration of this test.
dev_mode.IsEnabled = true
t.Cleanup(func() { dev_mode.IsEnabled = false })

digicertServer, hydrantServer := setupMockCAServers(t)
digicertURL := digicertServer.URL
hydrantURL := hydrantServer.URL
Expand Down Expand Up @@ -534,7 +539,7 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
}

createdCA, err := svc.NewCertificateAuthority(ctx, createDigicertRequest)
require.ErrorContains(t, err, "Invalid DigiCert URL")
require.ErrorContains(t, err, "DigiCert URL is invalid")
require.Len(t, createdCAs, 0)
require.Nil(t, createdCA)
})
Expand Down Expand Up @@ -675,7 +680,7 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
}

createdCA, err := svc.NewCertificateAuthority(ctx, createHydrantRequest)
require.ErrorContains(t, err, "Invalid Hydrant URL.")
require.ErrorContains(t, err, "Hydrant URL is invalid")
require.Len(t, createdCAs, 0)
require.Nil(t, createdCA)
})
Expand Down Expand Up @@ -763,7 +768,7 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
}

createdCA, err := svc.NewCertificateAuthority(ctx, createCustomSCEPRequest)
require.ErrorContains(t, err, "Invalid SCEP URL.")
require.ErrorContains(t, err, "Custom SCEP Proxy URL is invalid")
require.Len(t, createdCAs, 0)
require.Nil(t, createdCA)
})
Expand Down Expand Up @@ -819,7 +824,7 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
}

createdCA, err := svc.NewCertificateAuthority(ctx, createNDESSCEPRequest)
require.ErrorContains(t, err, "Invalid NDES SCEP URL.")
require.ErrorContains(t, err, "NDES SCEP URL is invalid")
require.Len(t, createdCAs, 0)
require.Nil(t, createdCA)
})
Expand Down Expand Up @@ -979,7 +984,7 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
}

createdCA, err := svc.NewCertificateAuthority(ctx, createSmallstepRequest)
require.ErrorContains(t, err, "Invalid Smallstep SCEP URL.")
require.ErrorContains(t, err, "Smallstep SCEP URL is invalid")
require.Len(t, createdCAs, 0)
require.Nil(t, createdCA)
})
Expand Down Expand Up @@ -1092,7 +1097,9 @@ func TestCreatingCertificateAuthorities(t *testing.T) {
}

func TestUpdatingCertificateAuthorities(t *testing.T) {
t.Parallel()
// Enable dev mode so CheckURLForSSRF skips the private-IP blocklist for the duration of this test.
dev_mode.IsEnabled = true
t.Cleanup(func() { dev_mode.IsEnabled = false })

digicertServer, hydrantServer := setupMockCAServers(t)
digicertURL := digicertServer.URL
Expand Down Expand Up @@ -1360,7 +1367,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}

err := svc.UpdateCertificateAuthority(ctx, digicertID, payload)
require.EqualError(t, err, "validation failed: url Couldn't edit certificate authority. Invalid DigiCert URL. Please correct and try again.")
require.ErrorContains(t, err, "DigiCert URL is invalid")
})

t.Run("Bad URL Path", func(t *testing.T) {
Expand Down Expand Up @@ -1502,7 +1509,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}

err := svc.UpdateCertificateAuthority(ctx, hydrantID, payload)
require.EqualError(t, err, "validation failed: url Couldn't edit certificate authority. Invalid Hydrant URL. Please correct and try again.")
require.ErrorContains(t, err, "Hydrant URL is invalid")
})

t.Run("Bad URL", func(t *testing.T) {
Expand Down Expand Up @@ -1588,7 +1595,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}

err := svc.UpdateCertificateAuthority(ctx, scepID, payload)
require.EqualError(t, err, "validation failed: url Couldn't edit certificate authority. Invalid SCEP URL. Please correct and try again.")
require.ErrorContains(t, err, "Custom SCEP Proxy URL is invalid")
})

t.Run("Requires challenge when updating URL", func(t *testing.T) {
Expand Down Expand Up @@ -1651,7 +1658,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}

err := svc.UpdateCertificateAuthority(ctx, ndesID, payload)
require.EqualError(t, err, "validation failed: url Couldn't edit certificate authority. Invalid NDES SCEP URL. Please correct and try again.")
require.ErrorContains(t, err, "NDES SCEP URL is invalid")
})

t.Run("Bad SCEP URL", func(t *testing.T) {
Expand Down Expand Up @@ -1810,7 +1817,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}

err := svc.UpdateCertificateAuthority(ctx, smallstepID, payload)
require.EqualError(t, err, "validation failed: url Couldn't edit certificate authority. Invalid SCEP URL. Please correct and try again.")
require.ErrorContains(t, err, "Smallstep SCEP URL is invalid")
})

t.Run("Invalid Challenge URL format", func(t *testing.T) {
Expand All @@ -1826,7 +1833,7 @@ func TestUpdatingCertificateAuthorities(t *testing.T) {
}

err := svc.UpdateCertificateAuthority(ctx, smallstepID, payload)
require.EqualError(t, err, "validation failed: url Couldn't edit certificate authority. Invalid Challenge URL. Please correct and try again.")
require.ErrorContains(t, err, "Challenge URL is invalid")
})

t.Run("Bad Smallstep SCEP URL", func(t *testing.T) {
Expand Down
8 changes: 5 additions & 3 deletions ee/server/service/scep_proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ func (s *SCEPConfigService) GetNDESSCEPChallenge(ctx context.Context, proxy flee
// Get the challenge from NDES
client := fleethttp.NewClient(fleethttp.WithTimeout(*s.Timeout))
client.Transport = ntlmssp.Negotiator{
RoundTripper: fleethttp.NewTransport(),
RoundTripper: fleethttp.NewSSRFProtectedTransport(),
}
req, err := http.NewRequest(http.MethodGet, adminURL, http.NoBody)
if err != nil {
Expand Down Expand Up @@ -586,8 +586,10 @@ func (s *SCEPConfigService) ValidateSmallstepChallengeURL(ctx context.Context, c
}

func (s *SCEPConfigService) GetSmallstepSCEPChallenge(ctx context.Context, ca fleet.SmallstepSCEPProxyCA) (string, error) {
// Get the challenge from Smallstep
client := fleethttp.NewClient(fleethttp.WithTimeout(30 * time.Second))
client := fleethttp.NewClient(
fleethttp.WithTimeout(30*time.Second),
fleethttp.WithTransport(fleethttp.NewSSRFProtectedTransport()),
)
Copy link
Copy Markdown
Contributor Author

@ksykulev ksykulev Feb 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted not to change NewClient internals to always use NewSSRFProtectedTransport(). Doing this would likely break all integration and unit tests that dial localhost, about 100+ places. Since Localhost (127.0.0.0/8) is in the blocklist.

Copy link
Copy Markdown
Contributor Author

@ksykulev ksykulev Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/fleetdm/fleet/actions/runs/22078563488?pr=39947

var reqBody bytes.Buffer
if err := json.NewEncoder(&reqBody).Encode(fleet.SmallstepChallengeRequestBody{
Webhook: fleet.SmallstepChallengeWebhook{
Expand Down
Loading
Loading