fix: audit logger, added format and type

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,4 +1,5 @@
-> Please check if what you want to add to `coraza-waf` list meets [quality standards](https://github.com/jptosso/coraza-waf/blob/master/CONTRIBUTING.md#quality-standards) before sending pull request. Thanks!
+> Thank you for contributing to Coraza WAF, your effort is greatly appreciated
+> Before submitting check if what you want to add to `coraza-waf` list meets [quality standards](https://github.com/jptosso/coraza-waf/blob/master/CONTRIBUTING.md#quality-standards) before sending pull request. Thanks!
 
 **Note**: _that go.mod and go.sum can only be modified for tested dependency updates or justified new features._
 

.github/workflows/codeql-analysis.yml

@@ -1,9 +1,10 @@
 name: "CodeQL"
-
 on:
   push:
     branches: 
       - '*'
+      - 'v1/*'
+      - 'v2/*'
   pull_request:
     # The branches below must be a subset of the branches above
     branches: [master]
@@ -13,7 +14,6 @@ jobs:
   analyze:
     name: Analyze
     runs-on: ubuntu-latest
-
     strategy:
       fail-fast: false
       matrix:

.github/workflows/regression.yml

@@ -29,7 +29,6 @@ jobs:
       run: go mod vendor
     - name: Tests and coverage
       run: go test -v -coverpkg=./... -coverprofile=coverage-waf.out ./...
-    # now we also test CGO disabled    
     - name: SonarCloud Scan
       uses: sonarsource/sonarcloud-github-action@master
       env:

coraza.conf-recommended

@@ -232,7 +232,7 @@ SecAuditLogParts ABIJDEFHZ
 # Use a single file for logging. This is much easier to look at, but
 # assumes that you will use the audit log only ocassionally.
 #
-SecAuditLog Serial
+SecAuditLogType Serial
 
 
 # -- Miscellaneous -----------------------------------------------------------

loggers/logger.go

@@ -17,8 +17,7 @@ package loggers
 import (
 	"fmt"
 	"io/fs"
-	"os"
-	"path"
+	"path/filepath"
 )
 
 // Logger is a wrapper to hold configurations, a writer and a formatter
@@ -78,6 +77,36 @@ func (l *Logger) SetWriter(name string) error {
 	return l.writer.Init(l)
 }
 
+// SetFile sets the file for the logger
+// The file path must exist and must be absolute
+func (l *Logger) SetFile(file string) error {
+	if !filepath.IsAbs(file) {
+		return fmt.Errorf("file path must be absolute")
+	}
+	l.file = file
+	return nil
+}
+
+// SetDir sets the directory for the concurrent logger
+// The directory must exist and must be absolute
+func (l *Logger) SetDir(dir string) error {
+	if !filepath.IsAbs(dir) {
+		return fmt.Errorf("directory path must be absolute")
+	}
+	l.directory = dir
+	return nil
+}
+
+// SetFileMode sets the file mode for the logger
+func (l *Logger) SetFileMode(mode fs.FileMode) {
+	l.fileMode = mode
+}
+
+// SetDirMode sets the directory mode for the logger
+func (l *Logger) SetDirMode(mode fs.FileMode) {
+	l.dirMode = mode
+}
+
 // LogFormatter is the interface for all log formatters
 // A LogFormatter receives an auditlog and generates "readable" audit log
 type LogFormatter = func(al AuditLog) ([]byte, error)
@@ -137,23 +166,12 @@ func getLogFormatter(name string) (LogFormatter, error) {
 // Filemode: 0644
 // Formatter: native
 // Writer: serial
-// Path: /tmp/coraza-audit.log
 func NewAuditLogger() (*Logger, error) {
-	/*
-		if file == "" {
-			return nil, fmt.Errorf("invalid file")
-		}
-		if directory == "" {
-			return nil, fmt.Errorf("invalid directory")
-		}*/
 	dirMode := fs.FileMode(0755)
 	fileMode := fs.FileMode(0644)
-	f := path.Join(os.TempDir(), "coraza-audit.log")
 	l := &Logger{
-		file:      f,
-		directory: "/opt/coraza/var/log/audit/",
-		dirMode:   dirMode,
-		fileMode:  fileMode,
+		dirMode:  dirMode,
+		fileMode: fileMode,
 	}
 	if err := l.SetFormatter("native"); err != nil {
 		return nil, err

loggers/serial_writer.go

@@ -29,7 +29,7 @@ type serialWriter struct {
 func (sl *serialWriter) Init(l *Logger) error {
 	sl.l = l
 	var err error
-	sl.file, err = os.OpenFile(l.file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	sl.file, err = os.OpenFile(l.file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, l.fileMode)
 	if err != nil {
 		return err
 	}

seclang/directives.go

@@ -263,16 +263,46 @@ func directiveSecCollectionTimeout(p *Parser, opts string) error {
 
 func directiveSecAuditLog(p *Parser, opts string) error {
 	if len(opts) == 0 {
-		return errors.New("syntax error: SecAuditLog [concurrent/https/serial/...]")
+		return errors.New("syntax error: SecAuditLog /some/absolute/path.log")
 	}
-	return p.Waf.SetAuditLogger(strings.ToLower(opts))
+	p.Waf.AuditLog = opts
+	if err := p.Waf.UpdateAuditLogger(); err != nil {
+		return err
+	}
+	return nil
+}
+
+func directiveSecAuditLogType(p *Parser, opts string) error {
+	if len(opts) == 0 {
+		return errors.New("syntax error: SecAuditLogType [concurrent/https/serial/...]")
+	}
+	p.Waf.AuditLogType = strings.ToLower(opts)
+	if err := p.Waf.UpdateAuditLogger(); err != nil {
+		return err
+	}
+	return nil
 }
 
 func directiveSecAuditLogFormat(p *Parser, opts string) error {
 	if len(opts) == 0 {
-		return errors.New("syntax error: SecAuditLogFormat [json/json2/native/...]")
+		return errors.New("syntax error: SecAuditLogFormat [json/jsonlegacy/native/...]")
 	}
-	return p.Waf.SetAuditLoggerFormat(strings.ToLower(opts))
+	p.Waf.AuditLogFormat = strings.ToLower(opts)
+	if err := p.Waf.UpdateAuditLogger(); err != nil {
+		return err
+	}
+	return nil
+}
+
+func directiveSecAuditLogDir(p *Parser, opts string) error {
+	if len(opts) == 0 {
+		return errors.New("syntax error: SecAuditLogDir /some/absolute/path")
+	}
+	if err := p.Waf.UpdateAuditLogger(); err != nil {
+		return err
+	}
+	p.Waf.AuditLogDir = opts
+	return nil
 }
 
 func directiveSecAuditLogRelevantStatus(p *Parser, opts string) error {
@@ -348,6 +378,7 @@ var (
 	_ directive = directiveSecAction
 	_ directive = directiveSecAuditEngine
 	_ directive = directiveSecAuditLog
+	_ directive = directiveSecAuditLogType
 	_ directive = directiveSecAuditLogFormat
 	_ directive = directiveSecAuditLogParts
 	_ directive = directiveSecAuditLogRelevantStatus
@@ -413,12 +444,14 @@ var directivesMap = map[string]directive{
 	"seccollectiontimeout":          directiveSecCollectionTimeout,
 	"secauditlogrelevantstatus":     directiveSecAuditLogRelevantStatus,
 	"secauditlogparts":              directiveSecAuditLogParts,
+	"secauditlogdir":                directiveSecAuditLogDir,
 	"secauditlog":                   directiveSecAuditLog,
 	"secauditengine":                directiveSecAuditEngine,
 	"secaction":                     directiveSecAction,
 	"secdebuglog":                   directiveSecDebugLog,
 	"secdebugloglevel":              directiveSecDebugLogLevel,
 	"secauditlogformat":             directiveSecAuditLogFormat,
+	"secauditlogtype":               directiveSecAuditLogType,
 
 	// Unsupported Directives
 	"secargumentseparator":     directiveUnsupported,

seclang/directives_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	engine "github.com/jptosso/coraza-waf/v2"
+	"github.com/jptosso/coraza-waf/v2/loggers"
 	"github.com/jptosso/coraza-waf/v2/types"
 )
 
@@ -131,3 +132,68 @@ func TestDebugDirectives(t *testing.T) {
 		t.Error("failed to write info log")
 	}
 }
+
+func TestSecAuditLogDirectivesDefaults(t *testing.T) {
+	waf := engine.NewWaf()
+	tmpf, err := ioutil.TempFile("/tmp", "*.log")
+	if err != nil {
+		t.Error(err)
+	}
+	parser, _ := NewParser(waf)
+	if err := directiveSecAuditLog(parser, tmpf.Name()); err != nil {
+		t.Error(err)
+	}
+	if err := directiveSecAuditLogDir(parser, "/tmp"); err != nil {
+		t.Error(err)
+	}
+	if waf.AuditLogger() == nil {
+		t.Error("Invalid audit logger (nil)")
+		return
+	}
+	if err := waf.AuditLogger().Write(loggers.AuditLog{
+		Parts: types.AuditLogParts("ABCDEFGHIJ"),
+		Transaction: loggers.AuditTransaction{
+			ID: "test-12345",
+		},
+	}); err != nil {
+		t.Error(err)
+	}
+	data, err := ioutil.ReadFile(tmpf.Name())
+	if err != nil {
+		t.Error(err)
+	}
+	if !strings.Contains(string(data), "test-12345") {
+		t.Error("failed to write audit log")
+	}
+}
+
+func TestSecAuditLogDirectivesConcurrent(t *testing.T) {
+	waf := engine.NewWaf()
+	tmpf, err := ioutil.TempFile("/tmp", "*.log")
+	if err != nil {
+		t.Error(err)
+	}
+	auditpath := "/tmp/audit/"
+	parser, _ := NewParser(waf)
+	if err := directiveSecAuditLog(parser, tmpf.Name()); err != nil {
+		t.Error(err)
+	}
+	if err := directiveSecAuditLogFormat(parser, "json"); err != nil {
+		t.Error(err)
+	}
+	if err := directiveSecAuditLogDir(parser, auditpath); err != nil {
+		t.Error(err)
+	}
+	if err := directiveSecAuditLogType(parser, "concurrent"); err != nil {
+		t.Error(err)
+	}
+	if err := waf.AuditLogger().Write(loggers.AuditLog{
+		Parts: types.AuditLogParts("ABCDEFGHIJKZ"),
+		Transaction: loggers.AuditTransaction{
+			ID: "test-12345",
+		},
+	}); err != nil {
+		t.Error(err)
+	}
+
+}

transaction.go

@@ -870,8 +870,11 @@ func (tx *Transaction) ProcessLogging() {
 	tx.Waf.Logger.Debug("Transaction marked for audit logging",
 		zap.String("tx", tx.ID),
 	)
-	if err := tx.Waf.AuditLogger().Write(tx.AuditLog()); err != nil {
-		tx.Waf.Logger.Error(err.Error())
+	if tx.Waf.AuditLogger() != nil {
+		// we don't log if there is an empty auditlogger
+		if err := tx.Waf.AuditLogger().Write(tx.AuditLog()); err != nil {
+			tx.Waf.Logger.Error(err.Error())
+		}
 	}
 }
 

waf.go

@@ -114,6 +114,17 @@ type Waf struct {
 	// Path to store data files (ex. cache)
 	DataDir string
 
+	// AUDIT LOG VARIABLES
+
+	// AuditLog contains the log file absolute path
+	AuditLog string
+	// AuditLogDir contains the concurrent logging directory
+	AuditLogDir string
+	// AuditLogFormat is the audit log format
+	AuditLogFormat string
+	// AuditLogType is the audit log type
+	AuditLogType string
+
 	UploadKeepFiles         bool
 	UploadFileMode          fs.FileMode
 	UploadFileLimit         int
@@ -228,16 +239,57 @@ func (w *Waf) NewTransaction() *Transaction {
 	return tx
 }
 
-// SetAuditLogger creates a new logger for the current WAF instance
-// You may add as many loggers as you want
-// Keep in mind loggers may lock go routines
-func (w *Waf) SetAuditLogger(engine string) error {
-	return w.auditLogger.SetWriter(engine)
-}
+// UpdateAuditLogger compiles every SecAuditLog directive
+// into a single *loggers.Logger
+// This is required after updating w.AuditLog* variables
+// It doesn't look to effective but the reason for this is
+// that we have to reinitialize the logger after updating
+// This is not concurrency safe, it should never be called
+// after the rules are being used
+func (w *Waf) UpdateAuditLogger() error {
+	al, err := loggers.NewAuditLogger()
+	if err != nil {
+		return err
+	}
+	if w.AuditLog == "" {
+		// when there is no path we won't log
+		return nil
+	}
+	if err := al.SetFile(w.AuditLog); err != nil {
+		return err
+	}
+
+	// SecAuditLogFormat provides a log format, default is native
+	if w.AuditLogFormat != "" {
+		if err := al.SetFormatter(w.AuditLogFormat); err != nil {
+			return err
+		}
+	} else {
+		if err := al.SetFormatter("native"); err != nil {
+			return err
+		}
+	}
+
+	// SecAuditLogDir provides the log directory,
+	// there is no default value
+	if w.AuditLogDir != "" {
+		if err := al.SetDir(w.AuditLogDir); err != nil {
+			return err
+		}
+	}
 
-// SetAuditLoggerFormat sets the format for the audit logger
-func (w *Waf) SetAuditLoggerFormat(format string) error {
-	return w.auditLogger.SetFormatter(format)
+	// SecAuditLog provides the log type, default is serial
+	if w.AuditLogType != "" {
+		if err := al.SetWriter(w.AuditLogType); err != nil {
+			return err
+		}
+	} else {
+		if err := al.SetWriter("serial"); err != nil {
+			return err
+		}
+	}
+	w.auditLogger = al
+	return nil
 }
 
 // SetDebugLogPath sets the path for the debug log
@@ -272,12 +324,10 @@ func (w *Waf) AuditLogger() *loggers.Logger {
 func NewWaf() *Waf {
 	atom := zap.NewAtomicLevel()
 	atom.SetLevel(zap.FatalLevel)
-	al, _ := loggers.NewAuditLogger()
 	waf := &Waf{
 		ArgumentSeparator:        "&",
 		AuditEngine:              types.AuditEngineOff,
 		AuditLogParts:            []rune("ABCFHZ"),
-		auditLogger:              al,
 		mux:                      &sync.RWMutex{},
 		RequestBodyInMemoryLimit: 131072,
 		RequestBodyLimit:         10000000, // 10mb