Refactor WAF handler configuration to use 'include' array instead of 'rules_file'

- Updated the GenerateConfig function to replace 'rules_file' with 'include' for WAF handlers, aligning with the coraza-caddy plugin requirements.
- Modified related tests to check for the presence of 'include' instead of 'rules_file'.
- Enhanced the ApplyConfig method to prepend necessary Coraza directives to ruleset files if not already present.
- Added tests to verify that the SecRuleEngine directives are correctly prepended and that existing directives are not duplicated.
- Implemented debug logging for generated config size and content.
This commit is contained in:
GitHub Actions
2025-12-02 01:32:47 +00:00
parent 202e457d2c
commit a08edf1895
5 changed files with 902 additions and 262 deletions

File diff suppressed because it is too large Load Diff

View File

@@ -328,13 +328,13 @@ func GenerateConfig(hosts []models.ProxyHost, storageDir string, acmeEmail strin
// Ensure it has a "handler" key
if _, ok := v["handler"]; ok {
// Capture ruleset_name if present, remove it from advanced_config,
// and convert it to rules_files if this is a waf handler.
// and set up 'include' array for coraza-caddy plugin.
if rn, has := v["ruleset_name"]; has {
if rnStr, ok := rn.(string); ok && rnStr != "" {
// Only add rules_files if we map the name to a path
// Set 'include' array with the ruleset file path for coraza-caddy
if rulesetPaths != nil {
if p, ok := rulesetPaths[rnStr]; ok && p != "" {
v["rules_file"] = p
v["include"] = []string{p}
}
}
}
@@ -352,7 +352,7 @@ func GenerateConfig(hosts []models.ProxyHost, storageDir string, acmeEmail strin
if rnStr, ok := rn.(string); ok && rnStr != "" {
if rulesetPaths != nil {
if p, ok := rulesetPaths[rnStr]; ok && p != "" {
m["rules_file"] = p
m["include"] = []string{p}
}
}
}
@@ -753,15 +753,15 @@ func buildWAFHandler(host *models.ProxyHost, rulesets []models.SecurityRuleSet,
h := Handler{"handler": "waf"}
if selected != nil {
if rulesetPaths != nil {
if p, ok := rulesetPaths[selected.Name]; ok && p != "" {
h["rules_file"] = p
if p, ok := rulesetPaths[selected.Name]; ok && p != "" {
h["include"] = []string{p}
}
}
} else if secCfg != nil && secCfg.WAFRulesSource != "" {
// If there was a requested ruleset name but nothing matched, include a rules_files entry if path known
// If there was a requested ruleset name but nothing matched, include path if known
if rulesetPaths != nil {
if p, ok := rulesetPaths[secCfg.WAFRulesSource]; ok && p != "" {
h["rules_file"] = p
if p, ok := rulesetPaths[secCfg.WAFRulesSource]; ok && p != "" {
h["include"] = []string{p}
}
}
}

View File

@@ -168,17 +168,17 @@ 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 rules_files
// Since a ruleset name was requested but none exists, waf handler should include a reference but no include array
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["rules_file"]; !ok {
if _, ok := h["include"]; !ok {
found = true
}
}
}
require.True(t, found, "expected waf handler without rules_files when referenced ruleset does not exist")
require.True(t, found, "expected waf handler without include array when referenced ruleset does not exist")
// Now test learning/monitor mode mapping
sec2 := &models.SecurityConfig{WAFMode: "block", WAFLearning: true}
@@ -218,13 +218,13 @@ func TestGenerateConfig_WAFSelectedSetsContentAndMode(t *testing.T) {
found := false
for _, h := range route.Handle {
if hn, ok := h["handler"].(string); ok && hn == "waf" {
if rf, ok := h["rules_file"].(string); ok && rf != "" {
if incl, ok := h["include"].([]string); ok && len(incl) > 0 {
found = true
break
}
}
}
require.True(t, found, "expected waf handler with rules_files to be present")
require.True(t, found, "expected waf handler with include array to be present")
}
func TestGenerateConfig_DecisionAdminPartsEmpty(t *testing.T) {
@@ -271,11 +271,11 @@ func TestGenerateConfig_WAFUsesRuleSet(t *testing.T) {
cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", []models.SecurityRuleSet{rs}, rulesetPaths, nil, nil)
require.NoError(t, err)
route := cfg.Apps.HTTP.Servers["charon_server"].Routes[0]
// check waf handler present with rules_files
// check waf handler present with include array
found := false
for _, h := range route.Handle {
if hn, ok := h["handler"].(string); ok && hn == "waf" {
if rf, ok := h["rules_file"].(string); ok && rf != "" {
if incl, ok := h["include"].([]string); ok && len(incl) > 0 {
found = true
break
}
@@ -283,7 +283,7 @@ func TestGenerateConfig_WAFUsesRuleSet(t *testing.T) {
}
if !found {
b2, _ := json.MarshalIndent(route.Handle, "", " ")
t.Fatalf("waf handler with rules_file should be present; handlers: %s", string(b2))
t.Fatalf("waf handler with include array should be present; handlers: %s", string(b2))
}
}
@@ -295,17 +295,17 @@ func TestGenerateConfig_WAFUsesRuleSetFromAdvancedConfig(t *testing.T) {
cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", []models.SecurityRuleSet{rs}, rulesetPaths, nil, nil)
require.NoError(t, err)
route := cfg.Apps.HTTP.Servers["charon_server"].Routes[0]
// check waf handler present with rules_files coming from host AdvancedConfig
// check waf handler present with include array coming from host AdvancedConfig
found := false
for _, h := range route.Handle {
if hn, ok := h["handler"].(string); ok && hn == "waf" {
if rf, ok := h["rules_file"].(string); ok && rf == "/tmp/host-rs.conf" {
if incl, ok := h["include"].([]string); ok && len(incl) > 0 && incl[0] == "/tmp/host-rs.conf" {
found = true
break
}
}
}
require.True(t, found, "waf handler with rules_files should include host advanced_config ruleset path")
require.True(t, found, "waf handler with include array should include host advanced_config ruleset path")
}
func TestGenerateConfig_WAFUsesRuleSetFromAdvancedConfig_Array(t *testing.T) {
@@ -316,17 +316,17 @@ func TestGenerateConfig_WAFUsesRuleSetFromAdvancedConfig_Array(t *testing.T) {
cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", []models.SecurityRuleSet{rs}, rulesetPaths, nil, nil)
require.NoError(t, err)
route := cfg.Apps.HTTP.Servers["charon_server"].Routes[0]
// check waf handler present with rules_file coming from host AdvancedConfig array
// check waf handler present with include array coming from host AdvancedConfig array
found := false
for _, h := range route.Handle {
if hn, ok := h["handler"].(string); ok && hn == "waf" {
if rf, ok := h["rules_file"].(string); ok && rf == "/tmp/host-rs-array.conf" {
if incl, ok := h["include"].([]string); ok && len(incl) > 0 && incl[0] == "/tmp/host-rs-array.conf" {
found = true
break
}
}
}
require.True(t, found, "waf handler with rules_file should include host advanced_config array ruleset path")
require.True(t, found, "waf handler with include array should include host advanced_config array ruleset path")
}
func TestGenerateConfig_WAFUsesRulesetFromSecCfgFallback(t *testing.T) {
@@ -336,18 +336,18 @@ func TestGenerateConfig_WAFUsesRulesetFromSecCfgFallback(t *testing.T) {
rulesetPaths := map[string]string{"owasp-crs": "/tmp/owasp-fallback.conf"}
cfg, err := GenerateConfig([]models.ProxyHost{host}, "/tmp/caddy-data", "", "", "", false, false, true, false, false, "", nil, rulesetPaths, nil, sec)
require.NoError(t, err)
// since secCfg requested owasp-crs and we have a path, the wf handler should include rules_file
// since secCfg requested owasp-crs and we have a path, the waf handler should include the path in include array
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 rf, ok := h["rules_file"].(string); ok && rf == "/tmp/owasp-fallback.conf" {
if incl, ok := h["include"].([]string); ok && len(incl) > 0 && incl[0] == "/tmp/owasp-fallback.conf" {
found = true
break
}
}
}
require.True(t, found, "waf handler with rules_file should include fallback secCfg ruleset path")
require.True(t, found, "waf handler with include array should include fallback secCfg ruleset path")
}
func TestGenerateConfig_RateLimitFromSecCfg(t *testing.T) {

View File

@@ -20,12 +20,13 @@ import (
// Test hooks to allow overriding OS and JSON functions
var (
writeFileFunc = os.WriteFile
readFileFunc = os.ReadFile
removeFileFunc = os.Remove
readDirFunc = os.ReadDir
statFunc = os.Stat
jsonMarshalFunc = json.MarshalIndent
writeFileFunc = os.WriteFile
readFileFunc = os.ReadFile
removeFileFunc = os.Remove
readDirFunc = os.ReadDir
statFunc = os.Stat
jsonMarshalFunc = json.MarshalIndent
jsonMarshalDebugFunc = json.Marshal // For debug logging, separate hook for testing
// Test hooks for bandaging validation/generation flows
generateConfigFunc = GenerateConfig
validateConfigFunc = Validate
@@ -118,10 +119,22 @@ func (m *Manager) ApplyConfig(ctx context.Context) error {
safeName := strings.ReplaceAll(strings.ToLower(rs.Name), " ", "-")
safeName = strings.ReplaceAll(safeName, "/", "-")
filePath := filepath.Join(corazaDir, safeName+".conf")
if err := writeFileFunc(filePath, []byte(rs.Content), 0600); err != nil {
// Prepend required Coraza directives if not already present.
// These are essential for the WAF to actually enforce rules:
// - SecRuleEngine On: enables blocking mode (default is DetectionOnly)
// - SecRequestBodyAccess On: allows inspecting POST body content
content := rs.Content
if !strings.Contains(strings.ToLower(content), "secruleengine") {
content = "SecRuleEngine On\nSecRequestBodyAccess On\n\n" + content
}
// Write ruleset file with world-readable permissions so the Caddy
// process (which may run as an unprivileged user) can read it.
if err := writeFileFunc(filePath, []byte(content), 0644); err != nil {
logger.Log().WithError(err).WithField("ruleset", rs.Name).Warn("failed to write coraza ruleset file")
} else {
// Log a short fingerprint for debugging and confirm path
rulesetPaths[rs.Name] = filePath
logger.Log().WithField("ruleset", rs.Name).WithField("path", filePath).Info("wrote coraza ruleset file")
}
}
}
@@ -131,6 +144,13 @@ func (m *Manager) ApplyConfig(ctx context.Context) error {
return fmt.Errorf("generate config: %w", err)
}
// Log generated config size and a compact JSON snippet for debugging when in debug mode
if cfgJSON, jerr := jsonMarshalDebugFunc(config); jerr == nil {
logger.Log().WithField("config_json_len", len(cfgJSON)).Debug("generated Caddy config JSON")
} else {
logger.Log().WithError(jerr).Warn("failed to marshal generated config for debug logging")
}
// Validate before applying
if err := validateConfigFunc(config); err != nil {
return fmt.Errorf("validation failed: %w", err)

View File

@@ -718,12 +718,15 @@ func TestManager_ApplyConfig_IncludesWAFHandlerWithRuleset(t *testing.T) {
if h == "ruleset.example.com" {
for _, handle := range r.Handle {
if handlerName, ok := handle["handler"].(string); ok && handlerName == "waf" {
// Validate rules_file or inline ruleset_content presence
if rf, ok := handle["rules_file"].(string); ok && rf != "" {
// Ensure file exists and contains our content
b, err := os.ReadFile(rf)
if err == nil && string(b) == "test-rule-content" {
found = true
// Validate include array (coraza-caddy schema) or inline ruleset_content presence
if incl, ok := handle["include"].([]interface{}); ok && len(incl) > 0 {
if rf, ok := incl[0].(string); ok && rf != "" {
// Ensure file exists and contains our content
// Note: manager prepends SecRuleEngine On directives, so we check Contains
b, err := os.ReadFile(rf)
if err == nil && strings.Contains(string(b), "test-rule-content") {
found = true
}
}
}
// Inline content may also exist as a fallback
@@ -990,3 +993,158 @@ func TestManager_ApplyConfig_ReappliesOnFlagChange(t *testing.T) {
}
assert.True(t, hasCrowdsec, "CrowdSec handler should be present when enabled via DB")
}
func TestManager_ApplyConfig_PrependsSecRuleEngineDirectives(t *testing.T) {
tmp := t.TempDir()
dsn := fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name()+"secruleengine")
db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{})
assert.NoError(t, err)
assert.NoError(t, db.AutoMigrate(&models.ProxyHost{}, &models.Location{}, &models.Setting{}, &models.CaddyConfig{}, &models.SSLCertificate{}, &models.SecurityConfig{}, &models.SecurityRuleSet{}))
// Create host and ruleset without SecRuleEngine directive
h := models.ProxyHost{DomainNames: "prepend.example.com", ForwardHost: "127.0.0.1", ForwardPort: 8080, Enabled: true}
db.Create(&h)
// Ruleset content without SecRuleEngine - should be prepended
ruleContent := `SecRule REQUEST_BODY "<script>" "id:12345,phase:2,deny,status:403,msg:'XSS blocked'"`
rs := models.SecurityRuleSet{Name: "test-xss", Content: ruleContent}
assert.NoError(t, db.Create(&rs).Error)
sec := models.SecurityConfig{Name: "default", Enabled: true, AdminWhitelist: "10.0.0.1/32", WAFMode: "block", WAFRulesSource: "test-xss"}
assert.NoError(t, db.Create(&sec).Error)
caddyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/load" && r.Method == http.MethodPost {
w.WriteHeader(http.StatusOK)
return
}
if r.URL.Path == "/config/" && r.Method == http.MethodGet {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"apps":{"http":{}}}`))
return
}
w.WriteHeader(http.StatusNotFound)
}))
defer caddyServer.Close()
client := NewClient(caddyServer.URL)
// Capture written file content
var writtenContent []byte
origWrite := writeFileFunc
writeFileFunc = func(path string, b []byte, perm os.FileMode) error {
if strings.Contains(path, "test-xss") {
writtenContent = b
}
return origWrite(path, b, perm)
}
defer func() { writeFileFunc = origWrite }()
manager := NewManager(client, db, tmp, "", false, config.SecurityConfig{CerberusEnabled: true, WAFMode: "block"})
assert.NoError(t, manager.ApplyConfig(context.Background()))
// Verify SecRuleEngine On was prepended
content := string(writtenContent)
assert.True(t, strings.Contains(content, "SecRuleEngine On"), "SecRuleEngine On should be prepended")
assert.True(t, strings.Contains(content, "SecRequestBodyAccess On"), "SecRequestBodyAccess On should be prepended")
// Verify original content is still present
assert.True(t, strings.Contains(content, ruleContent), "Original rule content should be preserved")
// Verify order: directives should come before the original rule
assert.True(t, strings.Index(content, "SecRuleEngine On") < strings.Index(content, "SecRule REQUEST_BODY"), "SecRuleEngine should appear before SecRule")
}
func TestManager_ApplyConfig_DoesNotPrependIfSecRuleEngineExists(t *testing.T) {
tmp := t.TempDir()
dsn := fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name()+"secruleengine-exists")
db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{})
assert.NoError(t, err)
assert.NoError(t, db.AutoMigrate(&models.ProxyHost{}, &models.Location{}, &models.Setting{}, &models.CaddyConfig{}, &models.SSLCertificate{}, &models.SecurityConfig{}, &models.SecurityRuleSet{}))
// Create host and ruleset that already has SecRuleEngine
h := models.ProxyHost{DomainNames: "noprepend.example.com", ForwardHost: "127.0.0.1", ForwardPort: 8080, Enabled: true}
db.Create(&h)
// Ruleset content already contains SecRuleEngine
ruleContent := `SecRuleEngine DetectionOnly
SecRequestBodyAccess On
SecRule REQUEST_BODY "<script>" "id:12345,phase:2,deny,status:403,msg:'XSS blocked'"`
rs := models.SecurityRuleSet{Name: "test-existing", Content: ruleContent}
assert.NoError(t, db.Create(&rs).Error)
sec := models.SecurityConfig{Name: "default", Enabled: true, AdminWhitelist: "10.0.0.1/32", WAFMode: "block", WAFRulesSource: "test-existing"}
assert.NoError(t, db.Create(&sec).Error)
caddyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/load" && r.Method == http.MethodPost {
w.WriteHeader(http.StatusOK)
return
}
if r.URL.Path == "/config/" && r.Method == http.MethodGet {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"apps":{"http":{}}}`))
return
}
w.WriteHeader(http.StatusNotFound)
}))
defer caddyServer.Close()
client := NewClient(caddyServer.URL)
// Capture written file content
var writtenContent []byte
origWrite := writeFileFunc
writeFileFunc = func(path string, b []byte, perm os.FileMode) error {
if strings.Contains(path, "test-existing") {
writtenContent = b
}
return origWrite(path, b, perm)
}
defer func() { writeFileFunc = origWrite }()
manager := NewManager(client, db, tmp, "", false, config.SecurityConfig{CerberusEnabled: true, WAFMode: "block"})
assert.NoError(t, manager.ApplyConfig(context.Background()))
// Verify content is written exactly as provided (no extra prepending)
content := string(writtenContent)
// Count occurrences of SecRuleEngine - should be exactly 1
assert.Equal(t, 1, strings.Count(strings.ToLower(content), "secruleengine"), "SecRuleEngine should appear exactly once (not prepended)")
// Verify the original content is preserved exactly
assert.Equal(t, ruleContent, content, "Content should be exactly as provided when SecRuleEngine exists")
}
func TestManager_ApplyConfig_DebugMarshalFailure(t *testing.T) {
tmp := t.TempDir()
dsn := fmt.Sprintf("file:%s?mode=memory&cache=shared", t.Name()+"debugmarshal")
db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{})
assert.NoError(t, err)
assert.NoError(t, db.AutoMigrate(&models.ProxyHost{}, &models.Location{}, &models.Setting{}, &models.CaddyConfig{}, &models.SSLCertificate{}, &models.SecurityConfig{}, &models.SecurityRuleSet{}))
// Create a simple host
h := models.ProxyHost{DomainNames: "marshal.example.com", ForwardHost: "127.0.0.1", ForwardPort: 8080, Enabled: true}
db.Create(&h)
sec := models.SecurityConfig{Name: "default", Enabled: true, AdminWhitelist: "10.0.0.1/32"}
db.Create(&sec)
caddyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/load" && r.Method == http.MethodPost {
w.WriteHeader(http.StatusOK)
return
}
if r.URL.Path == "/config/" && r.Method == http.MethodGet {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"apps":{"http":{}}}`))
return
}
w.WriteHeader(http.StatusNotFound)
}))
defer caddyServer.Close()
client := NewClient(caddyServer.URL)
// Stub jsonMarshalDebugFunc to return an error (exercises the else branch in debug logging)
origMarshalDebug := jsonMarshalDebugFunc
jsonMarshalDebugFunc = func(v interface{}) ([]byte, error) {
return nil, fmt.Errorf("simulated marshal error")
}
defer func() { jsonMarshalDebugFunc = origMarshalDebug }()
manager := NewManager(client, db, tmp, "", false, config.SecurityConfig{CerberusEnabled: true})
// ApplyConfig should still succeed even if debug logging fails
assert.NoError(t, manager.ApplyConfig(context.Background()))
}