diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index f5b62fecf..e12e01efc 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.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._ diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 7bc9b0ea9..40231a284 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.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: diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index 85331beb8..ea9b7b8b4 100644 --- a/.github/workflows/regression.yml +++ b/.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: diff --git a/coraza.conf-recommended b/coraza.conf-recommended index f2960df8a..4fd338f56 100644 --- a/coraza.conf-recommended +++ b/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 ----------------------------------------------------------- diff --git a/loggers/logger.go b/loggers/logger.go index 765119c15..35621c7e1 100644 --- a/loggers/logger.go +++ b/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 diff --git a/loggers/serial_writer.go b/loggers/serial_writer.go index 0fd8db082..68f1dcbce 100644 --- a/loggers/serial_writer.go +++ b/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 } diff --git a/seclang/directives.go b/seclang/directives.go index 3a13a5f10..075ee11af 100644 --- a/seclang/directives.go +++ b/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, diff --git a/seclang/directives_test.go b/seclang/directives_test.go index 8ccf2133b..4a7c388ea 100644 --- a/seclang/directives_test.go +++ b/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) + } + +} diff --git a/transaction.go b/transaction.go index 46d3864de..74f91ac4e 100644 --- a/transaction.go +++ b/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()) + } } } diff --git a/waf.go b/waf.go index 1a9f002d7..197929684 100644 --- a/waf.go +++ b/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 diff --git a/waf_test.go b/waf_test.go index fa0f0e61b..6f05bae13 100644 --- a/waf_test.go +++ b/waf_test.go @@ -27,3 +27,13 @@ func TestWAFInitialize(t *testing.T) { func TestNewTransaction(t *testing.T) { } + +func TestAuditLoggerVars(t *testing.T) { + waf := NewWaf() + waf.AuditLog = "/tmp/test.log" + waf.AuditLogFormat = "native" + waf.AuditLogType = "serial" + if err := waf.UpdateAuditLogger(); err != nil { + t.Error(err) + } +}