diff --git a/internal/api/outbound_url.go b/internal/api/outbound_url.go new file mode 100644 index 000000000..819019c72 --- /dev/null +++ b/internal/api/outbound_url.go @@ -0,0 +1,170 @@ +package api + +import ( + "context" + "crypto/tls" + "fmt" + "net" + "net/http" + "net/url" + "strings" + "time" +) + +var resolveOutboundFetchIPs = net.DefaultResolver.LookupIPAddr +var allowLoopbackOutboundFetch bool + +type outboundURLOptions struct { + allowPrivateIPs bool +} + +func validateOutboundFetchURL(ctx context.Context, rawURL string, allowedSchemes []string, opts outboundURLOptions) (*url.URL, error) { + if strings.TrimSpace(rawURL) == "" || len(rawURL) > maxURLLength { + return nil, fmt.Errorf("invalid URL length") + } + + parsed, err := url.ParseRequestURI(rawURL) + if err != nil { + return nil, fmt.Errorf("invalid URL format") + } + if parsed.User != nil { + return nil, fmt.Errorf("embedded credentials are not allowed") + } + if parsed.Fragment != "" { + return nil, fmt.Errorf("URL fragments are not allowed") + } + if parsed.Hostname() == "" { + return nil, fmt.Errorf("URL missing hostname") + } + + allowed := false + for _, scheme := range allowedSchemes { + if strings.EqualFold(parsed.Scheme, scheme) { + allowed = true + break + } + } + if !allowed { + return nil, fmt.Errorf("URL scheme must be one of: %s", strings.Join(allowedSchemes, ", ")) + } + + if err := validateOutboundFetchHost(ctx, parsed.Hostname(), opts); err != nil { + return nil, err + } + return parsed, nil +} + +func validateOutboundFetchHost(ctx context.Context, host string, opts outboundURLOptions) error { + host = strings.TrimSpace(host) + if host == "" { + return fmt.Errorf("URL missing hostname") + } + + switch strings.ToLower(host) { + case "metadata.google.internal", "metadata.goog": + return fmt.Errorf("metadata service host is not allowed") + } + + if ip := net.ParseIP(host); ip != nil { + return validateOutboundFetchIP(ip, opts) + } + + resolveCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + addrs, err := resolveOutboundFetchIPs(resolveCtx, host) + if err != nil { + return fmt.Errorf("failed to resolve hostname %s: %w", host, err) + } + if len(addrs) == 0 { + return fmt.Errorf("hostname %s did not resolve", host) + } + + for _, addr := range addrs { + if err := validateOutboundFetchIP(addr.IP, opts); err != nil { + return err + } + } + return nil +} + +func validateOutboundFetchIP(ip net.IP, opts outboundURLOptions) error { + if ip == nil { + return fmt.Errorf("invalid IP address") + } + if ip.IsLoopback() { + if allowLoopbackOutboundFetch { + return nil + } + return fmt.Errorf("loopback addresses are not allowed") + } + if ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + return fmt.Errorf("link-local addresses are not allowed") + } + if ip.IsMulticast() { + return fmt.Errorf("multicast addresses are not allowed") + } + if ip.IsUnspecified() { + return fmt.Errorf("unspecified addresses are not allowed") + } + if ip.Equal(net.ParseIP("169.254.169.254")) { + return fmt.Errorf("metadata service address is not allowed") + } + if !opts.allowPrivateIPs && isPrivateOutboundIP(ip) { + return fmt.Errorf("private addresses are not allowed") + } + return nil +} + +func isPrivateOutboundIP(ip net.IP) bool { + privateRanges := []string{ + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "127.0.0.0/8", + "169.254.0.0/16", + "::1/128", + "fe80::/10", + "fc00::/7", + } + + for _, cidr := range privateRanges { + _, ipnet, err := net.ParseCIDR(cidr) + if err != nil { + continue + } + if ipnet.Contains(ip) { + return true + } + } + return false +} + +func sameOriginRedirectPolicy(allowedSchemes []string, opts outboundURLOptions) func(req *http.Request, via []*http.Request) error { + return func(req *http.Request, via []*http.Request) error { + if len(via) == 0 { + return nil + } + validated, err := validateOutboundFetchURL(req.Context(), req.URL.String(), allowedSchemes, opts) + if err != nil { + return err + } + origin := via[0].URL + if !strings.EqualFold(validated.Scheme, origin.Scheme) || !strings.EqualFold(validated.Host, origin.Host) { + return fmt.Errorf("redirects must stay on the same origin") + } + return nil + } +} + +func newRestrictedOutboundHTTPClient(timeout time.Duration, opts outboundURLOptions) *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{ + MinVersion: tls.VersionTLS12, + } + return &http.Client{ + Transport: transport, + Timeout: timeout, + CheckRedirect: sameOriginRedirectPolicy([]string{"https", "http"}, opts), + } +} diff --git a/internal/api/outbound_url_test.go b/internal/api/outbound_url_test.go new file mode 100644 index 000000000..d0478caa9 --- /dev/null +++ b/internal/api/outbound_url_test.go @@ -0,0 +1,37 @@ +package api + +import ( + "context" + "net/http" + "testing" +) + +func TestValidateSSOFetchURLRejectsEmbeddedCredentials(t *testing.T) { + _, err := validateSSOFetchURL(context.Background(), "https://user:pass@example.com/.well-known/openid-configuration") + if err == nil { + t.Fatal("expected embedded credentials to be rejected") + } +} + +func TestValidateSSOFetchURLRejectsLoopbackByDefault(t *testing.T) { + _, err := validateSSOFetchURL(context.Background(), "http://127.0.0.1:8080/metadata") + if err == nil { + t.Fatal("expected loopback URL to be rejected") + } +} + +func TestSameOriginRedirectPolicyRejectsCrossOrigin(t *testing.T) { + redirectedReq, err := http.NewRequest(http.MethodGet, "https://idp-two.example.com/metadata", nil) + if err != nil { + t.Fatalf("new redirected request: %v", err) + } + originReq, err := http.NewRequest(http.MethodGet, "https://idp-one.example.com/metadata", nil) + if err != nil { + t.Fatalf("new origin request: %v", err) + } + + err = sameOriginRedirectPolicy([]string{"https", "http"}, outboundURLOptions{allowPrivateIPs: true})(redirectedReq, []*http.Request{originReq}) + if err == nil { + t.Fatal("expected cross-origin redirect to be rejected") + } +} diff --git a/internal/api/saml_service.go b/internal/api/saml_service.go index 62ed1421f..f7303b41c 100644 --- a/internal/api/saml_service.go +++ b/internal/api/saml_service.go @@ -3,7 +3,6 @@ package api import ( "context" "crypto/rsa" - "crypto/tls" "crypto/x509" "encoding/pem" "encoding/xml" @@ -13,6 +12,7 @@ import ( "net/http" "net/url" "os" + "path/filepath" "strings" "sync" "time" @@ -73,14 +73,9 @@ func NewSAMLService(ctx context.Context, providerID string, cfg *config.SAMLProv } func newSAMLHTTPClient() *http.Client { - transport := http.DefaultTransport.(*http.Transport).Clone() - transport.TLSClientConfig = &tls.Config{ - MinVersion: tls.VersionTLS12, - } - return &http.Client{ - Transport: transport, - Timeout: 30 * time.Second, - } + return newRestrictedOutboundHTTPClient(30*time.Second, outboundURLOptions{ + allowPrivateIPs: true, + }) } // loadIDPMetadata loads Identity Provider metadata from URL or XML @@ -121,7 +116,12 @@ func (s *SAMLService) loadIDPMetadata(ctx context.Context) error { } func (s *SAMLService) fetchIDPMetadataFromURL(ctx context.Context, metadataURL string) (*saml.EntityDescriptor, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL, nil) + validatedURL, err := validateSSOFetchURL(ctx, metadataURL) + if err != nil { + return nil, err + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedURL.String(), nil) if err != nil { return nil, err } @@ -227,7 +227,7 @@ func (s *SAMLService) addIDPCertificate(metadata *saml.EntityDescriptor) error { var err error if s.config.IDPCertFile != "" { - certData, err = os.ReadFile(s.config.IDPCertFile) + certData, err = readSAMLCredentialFile(s.config.IDPCertFile) if err != nil { return fmt.Errorf("failed to read idp certificate file: %w", err) } @@ -341,7 +341,7 @@ func (s *SAMLService) loadSPCredentials() (*x509.Certificate, *rsa.PrivateKey, e // Load certificate if s.config.SPCertFile != "" { - certData, err = os.ReadFile(s.config.SPCertFile) + certData, err = readSAMLCredentialFile(s.config.SPCertFile) if err != nil { return nil, nil, fmt.Errorf("failed to read sp certificate file: %w", err) } @@ -353,7 +353,7 @@ func (s *SAMLService) loadSPCredentials() (*x509.Certificate, *rsa.PrivateKey, e // Load private key if s.config.SPKeyFile != "" { - keyData, err = os.ReadFile(s.config.SPKeyFile) + keyData, err = readSAMLCredentialFile(s.config.SPKeyFile) if err != nil { return nil, nil, fmt.Errorf("failed to read sp private key file: %w", err) } @@ -404,6 +404,26 @@ func (s *SAMLService) loadSPCredentials() (*x509.Certificate, *rsa.PrivateKey, e return cert, key, nil } +func readSAMLCredentialFile(rawPath string) ([]byte, error) { + cleaned := filepath.Clean(strings.TrimSpace(rawPath)) + if cleaned == "" { + return nil, errors.New("credential file path is empty") + } + + absolute, err := filepath.Abs(cleaned) + if err != nil { + return nil, fmt.Errorf("resolve credential file path: %w", err) + } + info, err := os.Stat(absolute) + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("credential file must be a regular file") + } + return os.ReadFile(absolute) +} + // MakeAuthRequest creates a SAML AuthnRequest and returns the redirect URL func (s *SAMLService) MakeAuthRequest(relayState string) (string, error) { s.mu.RLock() diff --git a/internal/api/saml_service_additional_test.go b/internal/api/saml_service_additional_test.go index 9c85987d8..c3cdd26a7 100644 --- a/internal/api/saml_service_additional_test.go +++ b/internal/api/saml_service_additional_test.go @@ -85,6 +85,7 @@ func TestSAMLServiceIdentifiers(t *testing.T) { } func TestRefreshMetadata_Success(t *testing.T) { + allowSSOLoopbackFetchForTest(t) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) w.Write([]byte(` @@ -111,6 +112,7 @@ func TestRefreshMetadata_Success(t *testing.T) { } func TestFetchIDPMetadataFromURL_NonOK(t *testing.T) { + allowSSOLoopbackFetchForTest(t) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) })) diff --git a/internal/api/saml_service_test.go b/internal/api/saml_service_test.go index 2b554a5f3..50aecd9e0 100644 --- a/internal/api/saml_service_test.go +++ b/internal/api/saml_service_test.go @@ -194,6 +194,7 @@ func TestSAMLServiceBasicFlows(t *testing.T) { } func TestFetchMetadataFromURL(t *testing.T) { + allowSSOLoopbackFetchForTest(t) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) w.Write([]byte(` diff --git a/internal/api/sso_handlers.go b/internal/api/sso_handlers.go index 9c5d9f24b..c78b196ef 100644 --- a/internal/api/sso_handlers.go +++ b/internal/api/sso_handlers.go @@ -2,7 +2,6 @@ package api import ( "context" - "crypto/tls" "crypto/x509" "encoding/base64" "encoding/json" @@ -74,6 +73,12 @@ func validateURL(urlStr string, allowedSchemes []string) bool { return false } +func validateSSOFetchURL(ctx context.Context, rawURL string) (*url.URL, error) { + return validateOutboundFetchURL(ctx, rawURL, []string{"https", "http"}, outboundURLOptions{ + allowPrivateIPs: true, + }) +} + // SSOProviderResponse represents an SSO provider for API responses type SSOProviderResponse struct { ID string `json:"id"` @@ -794,8 +799,17 @@ func (r *Router) testOIDCConnection(ctx context.Context, cfg *OIDCTestConfig) SS // Fetch OIDC discovery document discoveryURL := strings.TrimRight(cfg.IssuerURL, "/") + "/.well-known/openid-configuration" + validatedDiscoveryURL, err := validateSSOFetchURL(ctx, discoveryURL) + if err != nil { + return SSOTestResponse{ + Success: false, + Message: "Discovery URL failed security validation", + Error: err.Error(), + } + } + httpClient := newTestHTTPClient() - req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedDiscoveryURL.String(), nil) if err != nil { return SSOTestResponse{ Success: false, @@ -1018,18 +1032,18 @@ func (r *Router) handleMetadataPreview(w http.ResponseWriter, req *http.Request) // ============================================================================ func newTestHTTPClient() *http.Client { - transport := http.DefaultTransport.(*http.Transport).Clone() - transport.TLSClientConfig = &tls.Config{ - MinVersion: tls.VersionTLS12, - } - return &http.Client{ - Transport: transport, - Timeout: testConnectionTimeout, - } + return newRestrictedOutboundHTTPClient(testConnectionTimeout, outboundURLOptions{ + allowPrivateIPs: true, + }) } func fetchSAMLMetadataFromURL(ctx context.Context, client *http.Client, metadataURL string) ([]byte, *saml.EntityDescriptor, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, metadataURL, nil) + validatedURL, err := validateSSOFetchURL(ctx, metadataURL) + if err != nil { + return nil, nil, err + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, validatedURL.String(), nil) if err != nil { return nil, nil, err } diff --git a/internal/api/sso_handlers_test.go b/internal/api/sso_handlers_test.go index d28841360..397e89a08 100644 --- a/internal/api/sso_handlers_test.go +++ b/internal/api/sso_handlers_test.go @@ -25,6 +25,15 @@ func setTestIP(req *http.Request) { req.RemoteAddr = ip + ":12345" } +func allowSSOLoopbackFetchForTest(t *testing.T) { + t.Helper() + prev := allowLoopbackOutboundFetch + allowLoopbackOutboundFetch = true + t.Cleanup(func() { + allowLoopbackOutboundFetch = prev + }) +} + // Sample SAML metadata for testing const testSAMLMetadata = ` @@ -53,6 +62,7 @@ const testOIDCDiscovery = `{ }` func TestHandleTestSSOProvider_SAMLSuccess(t *testing.T) { + allowSSOLoopbackFetchForTest(t) // Create mock SAML metadata server metadataServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/xml") @@ -138,6 +148,7 @@ func TestHandleTestSSOProvider_SAMLMetadataXML(t *testing.T) { } func TestHandleTestSSOProvider_SAMLFetchError(t *testing.T) { + allowSSOLoopbackFetchForTest(t) // Server that returns 500 errorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) @@ -204,6 +215,7 @@ func TestHandleTestSSOProvider_SAMLInvalidXML(t *testing.T) { } func TestHandleTestSSOProvider_OIDCSuccess(t *testing.T) { + allowSSOLoopbackFetchForTest(t) // Create mock OIDC discovery server discoveryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/.well-known/openid-configuration" { @@ -254,6 +266,7 @@ func TestHandleTestSSOProvider_OIDCSuccess(t *testing.T) { } func TestHandleTestSSOProvider_OIDCFetchError(t *testing.T) { + allowSSOLoopbackFetchForTest(t) reqBody := SSOTestRequest{ Type: "oidc", OIDC: &OIDCTestConfig{ @@ -360,6 +373,7 @@ func TestHandleTestSSOProvider_MissingConfig(t *testing.T) { } func TestHandleMetadataPreview_Success(t *testing.T) { + allowSSOLoopbackFetchForTest(t) // Create mock metadata server metadataServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/xml") @@ -457,6 +471,7 @@ func TestHandleMetadataPreview_InvalidURL(t *testing.T) { } func TestHandleMetadataPreview_FetchError(t *testing.T) { + allowSSOLoopbackFetchForTest(t) reqBody := MetadataPreviewRequest{ Type: "saml", MetadataURL: "http://localhost:99999/metadata", diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index 09120d483..bcd92f8ff 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -2963,6 +2963,18 @@ func (n *NotificationManager) ValidateWebhookURL(webhookURL string) error { return nil } +func (n *NotificationManager) validatedWebhookRequestURL(webhookURL string) (*url.URL, error) { + if err := n.ValidateWebhookURL(webhookURL); err != nil { + return nil, err + } + parsed, err := url.Parse(webhookURL) + if err != nil { + return nil, fmt.Errorf("invalid URL format: %w", err) + } + parsed.Fragment = "" + return parsed, nil +} + // isPrivateIP checks if an IP address is in a private range func isPrivateIP(ip net.IP) bool { // Private IPv4 ranges diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index e7c4b0469..107b7082c 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -407,7 +407,12 @@ func (n *NotificationManager) sendWebhookOnceWithResponse(webhook EnhancedWebhoo method = "POST" } - req, err := http.NewRequest(method, webhook.URL, bytes.NewBuffer(payload)) + validatedURL, err := n.validatedWebhookRequestURL(webhook.URL) + if err != nil { + return nil, fmt.Errorf("webhook URL validation failed: %w", err) + } + + req, err := http.NewRequest(method, validatedURL.String(), bytes.NewBuffer(payload)) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } @@ -562,7 +567,12 @@ func (n *NotificationManager) TestEnhancedWebhook(webhook EnhancedWebhookConfig) } } - req, err := http.NewRequest(method, webhookURL, bytes.NewBuffer(payload)) + validatedURL, err := n.validatedWebhookRequestURL(webhookURL) + if err != nil { + return 0, "", fmt.Errorf("webhook URL validation failed: %w", err) + } + + req, err := http.NewRequest(method, validatedURL.String(), bytes.NewBuffer(payload)) if err != nil { return 0, "", fmt.Errorf("failed to create request: %w", err) }