diff --git a/.github/workflows/waf-integration.yml b/.github/workflows/waf-integration.yml index b5cd3ae3..ac325622 100644 --- a/.github/workflows/waf-integration.yml +++ b/.github/workflows/waf-integration.yml @@ -45,6 +45,35 @@ jobs: scripts/coraza_integration.sh 2>&1 | tee waf-test-output.txt exit ${PIPESTATUS[0]} + - name: Dump Debug Info on Failure + if: failure() + run: | + echo "## 🔍 Debug Information" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + echo "### Container Status" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + docker ps -a --filter "name=charon" --filter "name=coraza" >> $GITHUB_STEP_SUMMARY 2>&1 || true + echo '```' >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + echo "### Caddy Admin Config" >> $GITHUB_STEP_SUMMARY + echo '```json' >> $GITHUB_STEP_SUMMARY + curl -s http://localhost:2019/config 2>/dev/null | head -200 >> $GITHUB_STEP_SUMMARY || echo "Could not retrieve Caddy config" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + echo "### Charon Container Logs (last 100 lines)" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + docker logs charon-debug 2>&1 | tail -100 >> $GITHUB_STEP_SUMMARY || echo "No container logs available" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + echo "### WAF Ruleset Files" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + docker exec charon-debug sh -c 'ls -la /app/data/caddy/coraza/rulesets/ 2>/dev/null && echo "---" && cat /app/data/caddy/coraza/rulesets/*.conf 2>/dev/null' >> $GITHUB_STEP_SUMMARY || echo "No ruleset files found" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + - name: WAF Integration Summary if: always() run: | diff --git a/backend/internal/caddy/config.go b/backend/internal/caddy/config.go index 3ebb0f07..6cdb1775 100644 --- a/backend/internal/caddy/config.go +++ b/backend/internal/caddy/config.go @@ -721,10 +721,19 @@ func buildCrowdSecHandler(host *models.ProxyHost, secCfg *models.SecurityConfig, return h, nil } -// buildWAFHandler returns a placeholder WAF handler (Coraza) configuration. -// This is a stub; integration with a Coraza caddy plugin would be required -// for real runtime enforcement. +// buildWAFHandler returns a WAF handler (Coraza) configuration. +// The coraza-caddy plugin registers as http.handlers.waf and expects: +// - handler: "waf" +// - directives: ModSecurity directive string including Include statements func buildWAFHandler(host *models.ProxyHost, rulesets []models.SecurityRuleSet, rulesetPaths map[string]string, secCfg *models.SecurityConfig, wafEnabled bool) (Handler, error) { + // Early exit if WAF is disabled + if !wafEnabled { + return nil, nil + } + if secCfg != nil && secCfg.WAFMode == "disabled" { + return nil, nil + } + // If the host provided an advanced_config containing a 'ruleset_name', prefer that value var hostRulesetName string if host != nil && host.AdvancedConfig != "" { @@ -738,23 +747,56 @@ func buildWAFHandler(host *models.ProxyHost, rulesets []models.SecurityRuleSet, } } - // Find a ruleset to associate with WAF; prefer name match by host.Application, host.AdvancedConfig ruleset_name or default 'owasp-crs' + // Find a ruleset to associate with WAF + // Priority order: + // 1. Exact match to secCfg.WAFRulesSource (user's global choice) + // 2. Exact match to hostRulesetName (per-host advanced_config) + // 3. Match to host.Application (app-specific defaults) + // 4. Fallback to owasp-crs var selected *models.SecurityRuleSet + var hostRulesetMatch, appMatch, owaspFallback *models.SecurityRuleSet + + // First pass: find all potential matches for i, r := range rulesets { - if r.Name == "owasp-crs" || (host != nil && r.Name == host.Application) || (hostRulesetName != "" && r.Name == hostRulesetName) || (secCfg != nil && r.Name == secCfg.WAFRulesSource) { + // Priority 1: Global WAF rules source - highest priority, select immediately + if secCfg != nil && secCfg.WAFRulesSource != "" && r.Name == secCfg.WAFRulesSource { selected = &rulesets[i] break } + // Priority 2: Per-host ruleset name from advanced_config + if hostRulesetName != "" && r.Name == hostRulesetName && hostRulesetMatch == nil { + hostRulesetMatch = &rulesets[i] + } + // Priority 3: Match by host application + if host != nil && r.Name == host.Application && appMatch == nil { + appMatch = &rulesets[i] + } + // Priority 4: Track owasp-crs as fallback + if r.Name == "owasp-crs" && owaspFallback == nil { + owaspFallback = &rulesets[i] + } } - if !wafEnabled { - return nil, nil + // Second pass: select by priority if not already selected + if selected == nil { + if hostRulesetMatch != nil { + selected = hostRulesetMatch + } else if appMatch != nil { + selected = appMatch + } else if owaspFallback != nil { + selected = owaspFallback + } } + + // Build the handler with directives h := Handler{"handler": "waf"} + directivesSet := false + if selected != nil { if rulesetPaths != nil { if p, ok := rulesetPaths[selected.Name]; ok && p != "" { h["directives"] = fmt.Sprintf("Include %s", p) + directivesSet = true } } } else if secCfg != nil && secCfg.WAFRulesSource != "" { @@ -762,14 +804,16 @@ func buildWAFHandler(host *models.ProxyHost, rulesets []models.SecurityRuleSet, if rulesetPaths != nil { if p, ok := rulesetPaths[secCfg.WAFRulesSource]; ok && p != "" { h["directives"] = fmt.Sprintf("Include %s", p) + directivesSet = true } } } - // WAF enablement is handled by the caller. Don't add a 'mode' field - // here because the module expects a specific configuration schema. - if secCfg != nil && secCfg.WAFMode == "disabled" { + + // Bug fix: Don't return a WAF handler without directives - it creates a no-op WAF + if !directivesSet { return nil, nil } + return h, nil } diff --git a/backend/internal/caddy/config_extra_test.go b/backend/internal/caddy/config_extra_test.go index e30b1412..62f6ef70 100644 --- a/backend/internal/caddy/config_extra_test.go +++ b/backend/internal/caddy/config_extra_test.go @@ -222,8 +222,11 @@ func TestGenerateConfig_SecurityPipeline_Order(t *testing.T) { acl := models.AccessList{ID: 200, Name: "WL", Enabled: true, Type: "whitelist", IPRules: ipRules} host := models.ProxyHost{UUID: "pipeline1", DomainNames: "pipe.example.com", Enabled: true, ForwardHost: "app", ForwardPort: 8080, AccessListID: &acl.ID, AccessList: &acl, HSTSEnabled: true, BlockExploits: true} + // Provide rulesets and paths so WAF handler is created with directives + rulesets := []models.SecurityRuleSet{{Name: "owasp-crs"}} + rulesetPaths := map[string]string{"owasp-crs": "/tmp/owasp.conf"} secCfg := &models.SecurityConfig{CrowdSecMode: "local"} - cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, true, true, true, true, "", nil, nil, nil, secCfg) + cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, true, true, true, true, "", rulesets, rulesetPaths, nil, secCfg) require.NoError(t, err) route := cfg.Apps.HTTP.Servers["charon_server"].Routes[0] diff --git a/backend/internal/caddy/config_generate_additional_test.go b/backend/internal/caddy/config_generate_additional_test.go index b78023c2..675070af 100644 --- a/backend/internal/caddy/config_generate_additional_test.go +++ b/backend/internal/caddy/config_generate_additional_test.go @@ -50,8 +50,11 @@ func TestGenerateConfig_SecurityPipeline_Order_Locations(t *testing.T) { acl := models.AccessList{ID: 201, Name: "WL2", Enabled: true, Type: "whitelist", IPRules: ipRules} host := models.ProxyHost{UUID: "pipeline2", DomainNames: "pipe-loc.example.com", Enabled: true, ForwardHost: "app", ForwardPort: 8080, AccessListID: &acl.ID, AccessList: &acl, HSTSEnabled: true, BlockExploits: true, Locations: []models.Location{{Path: "/loc", ForwardHost: "app", ForwardPort: 9000}}} + // Provide rulesets and paths so WAF handler is created with directives + rulesets := []models.SecurityRuleSet{{Name: "owasp-crs"}} + rulesetPaths := map[string]string{"owasp-crs": "/tmp/owasp.conf"} sec := &models.SecurityConfig{CrowdSecMode: "local"} - cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, true, true, true, true, "", nil, nil, nil, sec) + cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, true, true, true, true, "", rulesets, rulesetPaths, nil, sec) require.NoError(t, err) server := cfg.Apps.HTTP.Servers["charon_server"] @@ -168,21 +171,20 @@ func TestGenerateConfig_WAFModeAndRulesetReference(t *testing.T) { sec := &models.SecurityConfig{WAFMode: "block", WAFRulesSource: "nonexistent-rs"} cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", nil, nil, nil, sec) require.NoError(t, err) - // Since a ruleset name was requested but none exists, waf handler should include a reference but no directives + // Since a ruleset name was requested but none exists, NO waf handler should be created + // (Bug fix: don't create a no-op WAF handler without directives) route := cfg.Apps.HTTP.Servers["charon_server"].Routes[0] - found := false for _, h := range route.Handle { if hn, ok := h["handler"].(string); ok && hn == "waf" { - if _, ok := h["directives"]; !ok { - found = true - } + t.Fatalf("expected NO waf handler when referenced ruleset does not exist, but found: %v", h) } } - require.True(t, found, "expected waf handler without directives when referenced ruleset does not exist") - // Now test learning/monitor mode mapping + // Now test with valid ruleset - WAF handler should be created + rulesets := []models.SecurityRuleSet{{Name: "owasp-crs"}} + rulesetPaths := map[string]string{"owasp-crs": "/tmp/owasp.conf"} sec2 := &models.SecurityConfig{WAFMode: "block", WAFLearning: true} - cfg2, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", nil, nil, nil, sec2) + cfg2, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", rulesets, rulesetPaths, nil, sec2) require.NoError(t, err) route2 := cfg2.Apps.HTTP.Servers["charon_server"].Routes[0] monitorFound := false @@ -191,7 +193,7 @@ func TestGenerateConfig_WAFModeAndRulesetReference(t *testing.T) { monitorFound = true } } - require.True(t, monitorFound, "expected waf handler when WAFLearning is true") + require.True(t, monitorFound, "expected waf handler when WAFLearning is true and ruleset exists") } func TestGenerateConfig_WAFModeDisabledSkipsHandler(t *testing.T) { diff --git a/backend/internal/caddy/config_waf_security_test.go b/backend/internal/caddy/config_waf_security_test.go new file mode 100644 index 00000000..842f7a95 --- /dev/null +++ b/backend/internal/caddy/config_waf_security_test.go @@ -0,0 +1,283 @@ +package caddy + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/Wikid82/charon/backend/internal/models" +) + +// TestBuildWAFHandler_PathTraversalAttack tests path traversal attempts in ruleset names +func TestBuildWAFHandler_PathTraversalAttack(t *testing.T) { + tests := []struct { + name string + rulesetName string + shouldMatch bool // Whether the ruleset should be found + description string + }{ + { + name: "Path traversal in ruleset name", + rulesetName: "../../../etc/passwd", + shouldMatch: false, + description: "Ruleset with path traversal should not match any legitimate path", + }, + { + name: "Null byte injection", + rulesetName: "rules\x00.conf", + shouldMatch: false, + description: "Ruleset with null bytes should not match", + }, + { + name: "URL encoded traversal", + rulesetName: "..%2F..%2Fetc%2Fpasswd", + shouldMatch: false, + description: "URL encoded path traversal should not match", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + host := &models.ProxyHost{UUID: "test-host"} + rulesets := []models.SecurityRuleSet{{Name: tc.rulesetName}} + // Only provide paths for legitimate rulesets + rulesetPaths := map[string]string{ + "owasp-crs": "/app/data/caddy/coraza/rulesets/owasp-crs.conf", + } + secCfg := &models.SecurityConfig{WAFMode: "block", WAFRulesSource: tc.rulesetName} + + handler, err := buildWAFHandler(host, rulesets, rulesetPaths, secCfg, true) + require.NoError(t, err) + + if tc.shouldMatch { + require.NotNil(t, handler) + } else { + // Handler should be nil since no matching path exists + require.Nil(t, handler, tc.description) + } + }) + } +} + +// TestBuildWAFHandler_SQLInjectionInRulesetName tests SQL injection patterns in ruleset names +func TestBuildWAFHandler_SQLInjectionInRulesetName(t *testing.T) { + sqlInjectionPatterns := []string{ + "'; DROP TABLE rulesets; --", + "1' OR '1'='1", + "UNION SELECT * FROM users--", + "admin'/*", + } + + for _, pattern := range sqlInjectionPatterns { + t.Run(pattern, func(t *testing.T) { + host := &models.ProxyHost{UUID: "test-host"} + // Create ruleset with malicious name but only provide path for safe ruleset + rulesets := []models.SecurityRuleSet{{Name: pattern}, {Name: "owasp-crs"}} + rulesetPaths := map[string]string{ + "owasp-crs": "/app/data/caddy/coraza/rulesets/owasp-crs.conf", + } + secCfg := &models.SecurityConfig{WAFMode: "block", WAFRulesSource: pattern} + + handler, err := buildWAFHandler(host, rulesets, rulesetPaths, secCfg, true) + require.NoError(t, err) + // Should return nil since the malicious name has no corresponding path + require.Nil(t, handler, "SQL injection pattern should not produce valid handler") + }) + } +} + +// TestBuildWAFHandler_XSSInAdvancedConfig tests XSS patterns in advanced_config JSON +func TestBuildWAFHandler_XSSInAdvancedConfig(t *testing.T) { + xssPatterns := []string{ + `{"ruleset_name":""}`, + `{"ruleset_name":""}`, + `{"ruleset_name":"javascript:alert(1)"}`, + `{"ruleset_name":""}`, + } + + for _, pattern := range xssPatterns { + t.Run(pattern, func(t *testing.T) { + host := &models.ProxyHost{ + UUID: "test-host", + AdvancedConfig: pattern, + } + rulesets := []models.SecurityRuleSet{{Name: "owasp-crs"}} + rulesetPaths := map[string]string{ + "owasp-crs": "/app/data/caddy/coraza/rulesets/owasp-crs.conf", + } + secCfg := &models.SecurityConfig{WAFMode: "block"} + + handler, err := buildWAFHandler(host, rulesets, rulesetPaths, secCfg, true) + require.NoError(t, err) + // Should fall back to owasp-crs since XSS pattern won't match any ruleset + require.NotNil(t, handler) + directives := handler["directives"].(string) + require.Contains(t, directives, "owasp-crs") + // Ensure XSS content is NOT in the output + require.NotContains(t, directives, "