Harden SAML URL validation paths

This commit is contained in:
rcourtman
2026-04-01 12:00:31 +01:00
parent c5f5af7abf
commit 5a0f5aa68b
6 changed files with 137 additions and 19 deletions
+31 -2
View File
@@ -68,12 +68,26 @@ func validateURL(urlStr string, allowedSchemes []string) bool {
if err != nil {
return false
}
schemeAllowed := false
for _, scheme := range allowedSchemes {
if strings.EqualFold(parsed.Scheme, scheme) {
return true
schemeAllowed = true
break
}
}
return false
if !schemeAllowed {
return false
}
switch strings.ToLower(parsed.Scheme) {
case "http", "https":
_, err := securityutil.NormalizeAbsoluteHTTPURL(urlStr)
return err == nil
case "data":
return parsed.Opaque != ""
default:
return false
}
}
// SSOProviderResponse represents an SSO provider for API responses
@@ -313,6 +327,10 @@ func (r *Router) handleCreateSSOProvider(w http.ResponseWriter, req *http.Reques
writeErrorResponse(w, http.StatusBadRequest, "validation_error", "Invalid SAML SSO URL", nil)
return
}
if provider.SAML.IDPSLOURL != "" && !validateURL(provider.SAML.IDPSLOURL, []string{"https", "http"}) {
writeErrorResponse(w, http.StatusBadRequest, "validation_error", "Invalid SAML SLO URL", nil)
return
}
}
// Security: Validate icon URL if provided
@@ -431,6 +449,10 @@ func (r *Router) handleUpdateSSOProvider(w http.ResponseWriter, req *http.Reques
writeErrorResponse(w, http.StatusBadRequest, "validation_error", "Invalid SAML SSO URL", nil)
return
}
if updated.SAML.IDPSLOURL != "" && !validateURL(updated.SAML.IDPSLOURL, []string{"https", "http"}) {
writeErrorResponse(w, http.StatusBadRequest, "validation_error", "Invalid SAML SLO URL", nil)
return
}
}
// Security: Validate icon URL if provided
@@ -836,6 +858,13 @@ func (r *Router) testSAMLConnection(ctx context.Context, cfg *SAMLTestConfig) SS
Error: "invalid_url",
}
}
if cfg.IDPSLOURL != "" && !validateURL(cfg.IDPSLOURL, []string{"https", "http"}) {
return SSOTestResponse{
Success: false,
Message: "Invalid SLO URL format",
Error: "invalid_url",
}
}
return SSOTestResponse{
Success: true,
Message: "SSO URL is valid (manual configuration)",
+40 -13
View File
@@ -11,7 +11,6 @@ import (
"fmt"
"io"
"net/http"
"net/url"
"os"
"strings"
"sync"
@@ -171,7 +170,7 @@ func (s *SAMLService) buildManualMetadata() (*saml.EntityDescriptor, error) {
return nil, errors.New("idp sso url is required for manual configuration")
}
ssoURL, err := url.Parse(s.config.IDPSSOURL)
ssoURL, err := securityutil.NormalizeAbsoluteHTTPURL(s.config.IDPSSOURL)
if err != nil {
return nil, fmt.Errorf("invalid idp sso url: %w", err)
}
@@ -181,7 +180,7 @@ func (s *SAMLService) buildManualMetadata() (*saml.EntityDescriptor, error) {
entityID = s.config.IDPIssuer
}
if entityID == "" {
entityID = s.config.IDPSSOURL
entityID = ssoURL.String()
}
metadata := &saml.EntityDescriptor{
@@ -209,14 +208,15 @@ func (s *SAMLService) buildManualMetadata() (*saml.EntityDescriptor, error) {
// Add SLO endpoint if configured
if s.config.IDPSLOURL != "" {
sloURL, err := url.Parse(s.config.IDPSLOURL)
if err == nil {
metadata.IDPSSODescriptors[0].SingleLogoutServices = []saml.Endpoint{
{
Binding: saml.HTTPRedirectBinding,
Location: sloURL.String(),
},
}
sloURL, err := securityutil.NormalizeAbsoluteHTTPURL(s.config.IDPSLOURL)
if err != nil {
return nil, fmt.Errorf("invalid idp slo url: %w", err)
}
metadata.IDPSSODescriptors[0].SingleLogoutServices = []saml.Endpoint{
{
Binding: saml.HTTPRedirectBinding,
Location: sloURL.String(),
},
}
}
@@ -422,6 +422,10 @@ func (s *SAMLService) MakeAuthRequest(relayState string) (string, error) {
if relayState == "" {
relayState = "/"
}
if len(s.idpMetadata.IDPSSODescriptors) == 0 ||
len(s.idpMetadata.IDPSSODescriptors[0].SingleSignOnServices) == 0 {
return "", errors.New("idp does not support single sign-on")
}
// Use the simple redirect method
redirectURL, err := s.sp.MakeRedirectAuthenticationRequest(relayState)
@@ -434,7 +438,11 @@ func (s *SAMLService) MakeAuthRequest(relayState string) (string, error) {
Str("redirect_url", redirectURL.String()).
Msg("Created SAML AuthnRequest")
return redirectURL.String(), nil
validatedURL, err := validateSAMLRedirectTarget(redirectURL.String(), s.idpMetadata.IDPSSODescriptors[0].SingleSignOnServices)
if err != nil {
return "", fmt.Errorf("failed to validate auth redirect: %w", err)
}
return validatedURL, nil
}
// ProcessResponse processes a SAML response and extracts user information
@@ -570,7 +578,26 @@ func (s *SAMLService) MakeLogoutRequest(nameID, sessionIdx string) (string, erro
// Build redirect URL
redirectURL := req.Redirect("")
return redirectURL.String(), nil
return validateSAMLRedirectTarget(redirectURL.String(), s.idpMetadata.IDPSSODescriptors[0].SingleLogoutServices)
}
func validateSAMLRedirectTarget(rawURL string, allowedEndpoints []saml.Endpoint) (string, error) {
validatedURL, err := securityutil.NormalizeAbsoluteHTTPURL(rawURL)
if err != nil {
return "", err
}
for _, endpoint := range allowedEndpoints {
endpointURL, err := securityutil.NormalizeAbsoluteHTTPURL(endpoint.Location)
if err != nil {
continue
}
if strings.EqualFold(validatedURL.Scheme, endpointURL.Scheme) &&
strings.EqualFold(validatedURL.Host, endpointURL.Host) &&
validatedURL.Path == endpointURL.Path {
return validatedURL.String(), nil
}
}
return "", fmt.Errorf("redirect target does not match configured SAML endpoint")
}
// RefreshMetadata reloads IdP metadata (useful for key rotation)
+30
View File
@@ -14,6 +14,7 @@ import (
"testing"
"time"
"github.com/crewjam/saml"
"github.com/rcourtman/pulse-go-rewrite/internal/config"
)
@@ -101,6 +102,17 @@ func TestBuildManualMetadataAndCertificate(t *testing.T) {
if len(metadata.IDPSSODescriptors[0].KeyDescriptors) == 0 {
t.Fatal("expected key descriptor with certificate")
}
cfg.IDPSSOURL = "https://user:pass@idp.example.com/sso"
if _, err := service.buildManualMetadata(); err == nil {
t.Fatal("expected error for idp sso URL with embedded credentials")
}
cfg.IDPSSOURL = "https://idp.example.com/sso"
cfg.IDPSLOURL = "https://user:pass@idp.example.com/slo"
if _, err := service.buildManualMetadata(); err == nil {
t.Fatal("expected error for idp slo URL with embedded credentials")
}
}
func TestLoadSPCredentials(t *testing.T) {
@@ -192,6 +204,24 @@ func TestSAMLServiceBasicFlows(t *testing.T) {
}
}
func TestValidateSAMLRedirectTarget(t *testing.T) {
allowed := []saml.Endpoint{
{Location: "https://idp.example.com/sso"},
}
got, err := validateSAMLRedirectTarget("https://idp.example.com/sso?SAMLRequest=test", allowed)
if err != nil {
t.Fatalf("validate redirect: %v", err)
}
if !strings.Contains(got, "SAMLRequest=test") {
t.Fatalf("unexpected validated redirect: %s", got)
}
if _, err := validateSAMLRedirectTarget("https://evil.example.com/sso?SAMLRequest=test", allowed); err == nil {
t.Fatal("expected error for unexpected redirect target")
}
}
func TestFetchMetadataFromURL(t *testing.T) {
server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
+2
View File
@@ -41,7 +41,9 @@ func TestValidateURL(t *testing.T) {
}{
{"https://example.com", []string{"https"}, true},
{"http://example.com", []string{"https"}, false},
{"https://user:pass@example.com", []string{"https"}, false},
{"ftp://example.com", []string{"http", "https"}, false},
{"data:image/png;base64,abc", []string{"data"}, true},
{"not-a-url", []string{"https"}, false},
{"", []string{"https"}, false},
}
+19 -4
View File
@@ -4,6 +4,8 @@ import (
"fmt"
"net/url"
"strings"
"github.com/rcourtman/pulse-go-rewrite/internal/securityutil"
)
// SSOProviderType defines the type of SSO provider
@@ -298,14 +300,20 @@ func validateSAMLProvider(cfg *SAMLProviderConfig) error {
}
if cfg.IDPMetadataURL != "" {
if _, err := url.ParseRequestURI(cfg.IDPMetadataURL); err != nil {
return fmt.Errorf("invalid idp metadata url: %w", err)
if err := validateAbsoluteHTTPURL("idp metadata url", cfg.IDPMetadataURL); err != nil {
return err
}
}
if cfg.IDPSSOURL != "" {
if _, err := url.ParseRequestURI(cfg.IDPSSOURL); err != nil {
return fmt.Errorf("invalid idp sso url: %w", err)
if err := validateAbsoluteHTTPURL("idp sso url", cfg.IDPSSOURL); err != nil {
return err
}
}
if cfg.IDPSLOURL != "" {
if err := validateAbsoluteHTTPURL("idp slo url", cfg.IDPSLOURL); err != nil {
return err
}
}
@@ -321,6 +329,13 @@ func validateSAMLProvider(cfg *SAMLProviderConfig) error {
return nil
}
func validateAbsoluteHTTPURL(fieldName, raw string) error {
if _, err := securityutil.NormalizeAbsoluteHTTPURL(raw); err != nil {
return fmt.Errorf("invalid %s: %w", fieldName, err)
}
return nil
}
// Clone creates a deep copy of the SSO configuration
func (c *SSOConfig) Clone() *SSOConfig {
if c == nil {
+15
View File
@@ -710,6 +710,21 @@ func TestValidateSAMLProvider(t *testing.T) {
},
wantErr: true,
},
{
name: "rejects SSO URL with embedded credentials",
cfg: &SAMLProviderConfig{
IDPSSOURL: "https://user:pass@idp.example.com/sso",
},
wantErr: true,
},
{
name: "rejects invalid SLO URL",
cfg: &SAMLProviderConfig{
IDPSSOURL: "https://idp.example.com/sso",
IDPSLOURL: "https://user:pass@idp.example.com/slo",
},
wantErr: true,
},
{
name: "signing enabled without cert",
cfg: &SAMLProviderConfig{