From bd5e1852af94a34c22524d0a747e71c89b29d421 Mon Sep 17 00:00:00 2001 From: Juan Pablo Tosso Date: Wed, 17 Nov 2021 00:09:31 -0300 Subject: [PATCH] lint: more lint fixes --- .github/workflows/lint.yml | 17 + .pre-commit-config.yaml | 20 +- .yamllint | 7 + actions/allow.go | 21 +- actions/auditlog.go | 2 +- actions/ctl.go | 20 +- actions/initcol.go | 2 +- actions/setvar.go | 11 +- actions/severity_test.go | 2 +- actions/status.go | 4 +- actions/t.go | 2 +- body_buffer.go | 6 +- body_buffer_test.go | 26 +- bodyprocessors/json.go | 12 +- bodyprocessors/json_test.go | 2 +- bodyprocessors/multipart.go | 16 +- bodyprocessors/urlencoded.go | 18 +- bodyprocessors/xml.go | 2 +- collection.go | 24 +- geo/geo.go | 14 + loggers/auditlog.go | 23 +- loggers/concurrent_writer.go | 11 +- loggers/concurrent_writer_test.go | 4 +- loggers/formats.go | 76 ++-- loggers/formats_test.go | 7 +- loggers/logger.go | 32 +- loggers/serial_writer_test.go | 4 +- operators/inspect_file.go | 8 +- operators/ip_match.go | 8 +- operators/operators.go | 2 +- operators/operators_test.go | 2 +- operators/pm.go | 2 +- operators/pm_from_file.go | 6 +- operators/pm_from_file_test.go | 14 + operators/rbl.go | 23 +- operators/rx.go | 6 +- operators/validate_byte_range.go | 6 +- operators/validate_byte_range_test.go | 8 +- operators/validate_nid.go | 12 +- operators/validate_nid_test.go | 14 + operators/validate_url_encoding.go | 43 ++- operators/verify_cc.go | 30 -- rule.go | 20 +- rule_match.go | 6 +- rulegroup.go | 4 +- seclang/directives.go | 6 +- seclang/directives_test.go | 3 +- seclang/parser.go | 20 +- seclang/rule_parser.go | 61 ++-- seclang/rule_parser_test.go | 6 +- seclang/rules_test.go | 6 +- testdata/engine/rulemetadata.yaml | 32 +- testing/engine.go | 12 +- testing/engine_test.go | 4 +- testing/plugins_test.go | 94 ----- testing/profile.go | 6 +- transaction.go | 84 ++--- transaction_test.go | 30 +- transformations/base64decode.go | 12 +- transformations/cmd_line.go | 9 +- transformations/css_decode.go | 12 +- transformations/escape_seq_decode.go | 14 +- transformations/js_decode.go | 20 +- transformations/normalise_path_win.go | 8 +- transformations/remove_comments.go | 7 +- transformations/remove_comments_char.go | 21 +- transformations/replace_comments.go | 8 +- transformations/transformations_test.go | 4 +- transformations/url_decode.go | 20 +- transformations/url_encode.go | 10 +- types/phase.go | 9 +- types/variables/variables.go | 467 ++++++------------------ types/variables/variables_test.go | 34 ++ utils/strings.go | 2 +- utils/utils.go | 10 - waf.go | 27 +- 76 files changed, 727 insertions(+), 930 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 .yamllint delete mode 100644 operators/verify_cc.go delete mode 100644 testing/plugins_test.go diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 000000000..5abb25010 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,17 @@ +name: lint + +on: + pull_request: + branches: + - '*' + - 'v2/*' + push: + branches: [master, v2/*] + +jobs: + pre-commit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + - uses: pre-commit/action@v2.0.3 \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f08e730e3..029c9f040 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,12 +4,16 @@ repos: hooks: - id: go-fmt - id: go-vet - - id: go-lint +# - id: go-lint - id: go-imports - - id: go-cyclo - args: [-over=15] - - id: no-go-testing - - id: golangci-lint - - id: go-critic - - id: go-unit-tests - - id: go-mod-tidy \ No newline at end of file +# - id: go-cyclo +# args: [-over=37] # we must drop it down to 15 +# - id: golangci-lint +# - id: go-critic +# - id: go-unit-tests + - id: go-mod-tidy +#- repo: https://github.com/adrienverge/yamllint.git +# rev: v1.26.3 +# hooks: +# - id: yamllint +# args: [-c=.yamllint] \ No newline at end of file diff --git a/.yamllint b/.yamllint new file mode 100644 index 000000000..6f7e8d644 --- /dev/null +++ b/.yamllint @@ -0,0 +1,7 @@ +extends: default + +rules: + # 80 chars should be enough, but don't fail if a line is longer + line-length: + max: 80 + level: warning \ No newline at end of file diff --git a/actions/allow.go b/actions/allow.go index 49127523b..40a3273ae 100644 --- a/actions/allow.go +++ b/actions/allow.go @@ -21,26 +21,27 @@ import ( "github.com/jptosso/coraza-waf/v2/types" ) -//0 nothing, 1 phase, 2 request +// 0 nothing, 1 phase, 2 request type allowFn struct { allow int } func (a *allowFn) Init(r *coraza.Rule, b1 string) error { - if b1 == "phase" { - a.allow = 2 //skip current phase - } else if b1 == "request" { - a.allow = 3 //skip phases until RESPONSE_HEADERS - } else if b1 == "" { + switch b1 { + case "phase": + a.allow = 2 // skip current phase + case "request": + a.allow = 3 // skip phases until RESPONSE_HEADERS + case "": a.allow = 1 // skip all phases - } else { - return fmt.Errorf("invalid value for action allow") + default: + return fmt.Errorf("invalid argument %s for allow", b1) } return nil } func (a *allowFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) { - //TODO implement this: + // TODO implement this: /* if a.allow == 1 { tx.RuleEngine = coraza.RULE_ENGINE_OFF @@ -61,6 +62,6 @@ func allow() coraza.RuleAction { } var ( - _ coraza.RuleAction = &allowFn{} + _ coraza.RuleAction = (*allowFn)(nil) _ ruleActionWrapper = allow ) diff --git a/actions/auditlog.go b/actions/auditlog.go index 9bcd19d7c..3516d7d4d 100644 --- a/actions/auditlog.go +++ b/actions/auditlog.go @@ -38,6 +38,6 @@ func auditlog() coraza.RuleAction { } var ( - _ coraza.RuleAction = &auditlogFn{} + _ coraza.RuleAction = (*auditlogFn)(nil) _ ruleActionWrapper = auditlog ) diff --git a/actions/ctl.go b/actions/ctl.go index 86e10aa5a..1650b09ba 100644 --- a/actions/ctl.go +++ b/actions/ctl.go @@ -29,7 +29,7 @@ import ( type ctlFunctionType int const ( - ctlRemoveTargetById ctlFunctionType = 0 + ctlRemoveTargetByID ctlFunctionType = 0 ctlRemoveTargetByTag ctlFunctionType = 1 ctlRemoveTargetByMsg ctlFunctionType = 2 ctlAuditEngine ctlFunctionType = 3 @@ -38,7 +38,7 @@ const ( ctlRequestBodyAccess ctlFunctionType = 6 ctlRequestBodyLimit ctlFunctionType = 7 ctlRuleEngine ctlFunctionType = 8 - ctlRuleRemoveById ctlFunctionType = 9 + ctlRuleRemoveByID ctlFunctionType = 9 ctlRuleRemoveByMsg ctlFunctionType = 10 ctlRuleRemoveByTag ctlFunctionType = 11 ctlHashEngine ctlFunctionType = 12 @@ -64,7 +64,7 @@ func (a *ctlFn) Init(r *coraza.Rule, data string) error { func (a *ctlFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) { switch a.action { - case ctlRemoveTargetById: + case ctlRemoveTargetByID: id, _ := strconv.Atoi(a.value) tx.RemoveRuleTargetById(id, a.collection, a.colKey) case ctlRemoveTargetByTag: @@ -89,7 +89,7 @@ func (a *ctlFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) { } tx.AuditEngine = ae case ctlAuditLogParts: - //TODO lets switch it to a string + // TODO lets switch it to a string tx.AuditLogParts = []rune(a.value) case ctlForceRequestBodyVar: val := strings.ToLower(a.value) @@ -110,7 +110,7 @@ func (a *ctlFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) { tx.Waf.Logger.Error(err.Error()) } tx.RuleEngine = re - case ctlRuleRemoveById: + case ctlRuleRemoveByID: id, _ := strconv.Atoi(a.value) tx.RemoveRuleById(id) case ctlRuleRemoveByMsg: @@ -134,10 +134,10 @@ func (a *ctlFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) { case ctlHashEnforcement: // Not supported yet case ctlDebugLogLevel: - //lvl, _ := strconv.Atoi(a.Value) + // lvl, _ := strconv.Atoi(a.Value) // TODO // We cannot update the log level, it would affect the whole waf instance... - //tx.Waf.SetLogLevel(lvl) + // tx.Waf.SetLogLevel(lvl) } } @@ -149,7 +149,7 @@ func (a *ctlFn) Type() types.RuleActionType { func parseCtl(data string) (ctlFunctionType, string, variables.RuleVariable, string, error) { spl1 := strings.SplitN(data, "=", 2) if len(spl1) != 2 { - return ctlRemoveTargetById, "", 0, "", fmt.Errorf("invalid syntax") + return ctlRemoveTargetByID, "", 0, "", fmt.Errorf("invalid syntax") } spl2 := strings.SplitN(spl1[1], ";", 2) action := spl1[0] @@ -188,13 +188,13 @@ func parseCtl(data string) (ctlFunctionType, string, variables.RuleVariable, str case "ruleEngine": act = ctlRuleEngine case "ruleRemoveById": - act = ctlRuleRemoveById + act = ctlRuleRemoveByID case "ruleRemoveByMsg": act = ctlRuleRemoveByMsg case "ruleRemoveByTag": act = ctlRuleRemoveByTag case "ruleRemoveTargetById": - act = ctlRemoveTargetById + act = ctlRemoveTargetByID case "ruleRemoveTargetByMsg": act = ctlRemoveTargetByMsg case "ruleRemoveTargetByTag": diff --git a/actions/initcol.go b/actions/initcol.go index 6fdca047d..acdc1e4f5 100644 --- a/actions/initcol.go +++ b/actions/initcol.go @@ -37,7 +37,7 @@ func (a *initcolFn) Init(r *coraza.Rule, data string) error { } func (a *initcolFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) { - //tx.Waf.Logger.Error("initcol was used but it's not supported", zap.Int("rule", r.Id)) + // tx.Waf.Logger.Error("initcol was used but it's not supported", zap.Int("rule", r.Id)) /* key := tx.MacroExpansion(a.key) data := tx.Waf.Persistence.Get(a.variable, key) diff --git a/actions/setvar.go b/actions/setvar.go index 01b0110e7..f92224d26 100644 --- a/actions/setvar.go +++ b/actions/setvar.go @@ -71,7 +71,7 @@ func (a *setvarFn) Type() types.RuleActionType { func (a *setvarFn) evaluateTxCollection(r *coraza.Rule, tx *coraza.Transaction, key string, value string) { collection := tx.GetCollection(a.collection) if collection == nil { - //fmt.Println("Invalid Collection " + a.Collection) LOG error? + // fmt.Println("Invalid Collection " + a.Collection) LOG error? return } @@ -84,23 +84,24 @@ func (a *setvarFn) evaluateTxCollection(r *coraza.Rule, tx *coraza.Transaction, collection.Set(tx.MacroExpansion(a.key), []string{"0"}) res = []string{"0"} } - if len(a.value) == 0 { + switch { + case len(a.value) == 0: collection.Set(tx.MacroExpansion(a.key), []string{""}) - } else if a.value[0] == '+' { + case a.value[0] == '+': me, _ := strconv.Atoi(tx.MacroExpansion(a.value[1:])) txv, err := strconv.Atoi(res[0]) if err != nil { return } collection.Set(tx.MacroExpansion(a.key), []string{strconv.Itoa(me + txv)}) - } else if a.value[0] == '-' { + case a.value[0] == '-': me, _ := strconv.Atoi(tx.MacroExpansion(a.value[1:])) txv, err := strconv.Atoi(res[0]) if err != nil { return } collection.Set(tx.MacroExpansion(a.key), []string{strconv.Itoa(txv - me)}) - } else { + default: collection.Set(tx.MacroExpansion(a.key), []string{tx.MacroExpansion(a.value)}) } } diff --git a/actions/severity_test.go b/actions/severity_test.go index 4cdb3c762..944ea8d78 100644 --- a/actions/severity_test.go +++ b/actions/severity_test.go @@ -35,7 +35,7 @@ func TestSeverity(t *testing.T) { {"NOTICE", 5}, {"INFO", 6}, {"DEBUG", 7}, - //numeric input + // numeric input {"0", 0}, {"1", 1}, {"2", 2}, diff --git a/actions/status.go b/actions/status.go index abf801ae8..cdb3ae631 100644 --- a/actions/status.go +++ b/actions/status.go @@ -22,7 +22,7 @@ import ( "github.com/jptosso/coraza-waf/v2/types" ) -var HTTP_STATUSES = []int{100, 101, 102, 103, 200, +var htpStatuses = []int{100, 101, 102, 103, 200, 201, 202, 203, 200, 204, 205, 206, 207, 208, 226, 300, 301, 302, 303, 304, 305, 306, 307, 302, 308, 301, 400, 401, 402, 403, 404, 405, @@ -40,7 +40,7 @@ func (a *statusFn) Init(r *coraza.Rule, b1 string) error { if err != nil { return err } - for _, s := range HTTP_STATUSES { + for _, s := range htpStatuses { if status == s { a.status = status return nil diff --git a/actions/t.go b/actions/t.go index 9824e2688..ceb77405d 100644 --- a/actions/t.go +++ b/actions/t.go @@ -26,7 +26,7 @@ func (a *tFn) Init(r *coraza.Rule, input string) error { // TODO there is a chance that it won't work, it requires tests // none is a special hardcoded transformation, it must remove previous transformations if input == "none" { - //remove elements + // remove elements r.ClearTransformations() return nil } diff --git a/body_buffer.go b/body_buffer.go index 4a24f1273..f2894dd0d 100644 --- a/body_buffer.go +++ b/body_buffer.go @@ -23,7 +23,7 @@ import ( // BodyReader is used to read RequestBody and ResponseBody objects // It will handle memory usage for buffering and processing type bodyBuffer struct { - io.Writer //OK? + io.Writer // OK? tmpDir string buffer *bytes.Buffer writer *os.File @@ -42,7 +42,9 @@ func (br *bodyBuffer) Write(data []byte) (n int, err error) { return 0, err } // we dump the previous buffer - br.writer.Write(br.buffer.Bytes()) + if _, err := br.writer.Write(br.buffer.Bytes()); err != nil { + return 0, err + } defer br.buffer.Reset() } br.length = l diff --git a/body_buffer_test.go b/body_buffer_test.go index a64998877..511defeb1 100644 --- a/body_buffer_test.go +++ b/body_buffer_test.go @@ -23,9 +23,13 @@ import ( func TestBodyReaderMemory(t *testing.T) { br := NewBodyReader("/tmp", 500) - br.Write([]byte("test")) + if _, err := br.Write([]byte("test")); err != nil { + t.Error(err) + } buf := new(strings.Builder) - io.Copy(buf, br.Reader()) + if _, err := io.Copy(buf, br.Reader()); err != nil { + t.Error(err) + } if buf.String() != "test" { t.Error("Failed to get BodyReader from memory") } @@ -35,14 +39,18 @@ func TestBodyReaderMemory(t *testing.T) { func TestBodyReaderFile(t *testing.T) { // body reader memory limit is 1 byte br := NewBodyReader("/tmp", 1) - br.Write([]byte("test")) + if _, err := br.Write([]byte("test")); err != nil { + t.Error(err) + } buf := new(strings.Builder) - io.Copy(buf, br.Reader()) + if _, err := io.Copy(buf, br.Reader()); err != nil { + t.Error(err) + } if buf.String() != "test" { t.Error("Failed to get BodyReader from file") } // Let's check if files are being deleted - f := br.Reader().(*os.File) + f := (br.Reader()).(*os.File) if _, err := os.Stat(f.Name()); os.IsNotExist(err) { t.Error("BodyReader's Tmp file does not exist") } @@ -55,9 +63,13 @@ func TestBodyReaderFile(t *testing.T) { func TestBodyReaderWriteFromReader(t *testing.T) { br := NewBodyReader("/tmp", 5) b := strings.NewReader("test") - io.Copy(br, b) + if _, err := io.Copy(br, b); err != nil { + t.Error(err) + } buf := new(strings.Builder) - io.Copy(buf, br.Reader()) + if _, err := io.Copy(buf, br.Reader()); err != nil { + t.Error(err) + } if buf.String() != "test" { t.Error("Failed to write bodyreader from io.Reader") } diff --git a/bodyprocessors/json.go b/bodyprocessors/json.go index e39098d9c..4a361df5d 100644 --- a/bodyprocessors/json.go +++ b/bodyprocessors/json.go @@ -73,7 +73,7 @@ func (js *jsonBodyProcessor) Find(expr string) (map[string][]string, error) { } func (js *jsonBodyProcessor) VariableHook() variables.RuleVariable { - return variables.Json + return variables.JSON } // Transform JSON to a map[string]string @@ -108,12 +108,12 @@ func interfaceToMap(data map[string]interface{}) (map[string]string, error) { } // we set the parent key to count the number of items result[key] = strconv.Itoa(len(m)) - if m2, err := interfaceToMap(m); err != nil { + m2, err := interfaceToMap(m) + if err != nil { return nil, err - } else { - for key2, value2 := range m2 { - result[key+"."+key2] = value2 - } + } + for key2, value2 := range m2 { + result[key+"."+key2] = value2 } case string: result[key] = value.(string) diff --git a/bodyprocessors/json_test.go b/bodyprocessors/json_test.go index 57f7936c4..04843a912 100644 --- a/bodyprocessors/json_test.go +++ b/bodyprocessors/json_test.go @@ -61,7 +61,7 @@ func TestJSONToMap(t *testing.T) { if err != nil { t.Error(err) } - //fmt.Println(jsonMap) + // fmt.Println(jsonMap) for k, v := range asserts { if jsonMap[k] != v { t.Errorf("Expected %s=%s", k, v) diff --git a/bodyprocessors/multipart.go b/bodyprocessors/multipart.go index 8d8908360..8671e2628 100644 --- a/bodyprocessors/multipart.go +++ b/bodyprocessors/multipart.go @@ -26,7 +26,7 @@ type multipartBodyProcessor struct { collections *collectionsMap } -func (ue *multipartBodyProcessor) Read(reader io.Reader, mime string, storagePath string) error { +func (mbp *multipartBodyProcessor) Read(reader io.Reader, mime string, storagePath string) error { req, _ := http.NewRequest("GET", "/", reader) req.Header.Set("Content-Type", mime) err := req.ParseMultipartForm(1000000000) @@ -62,7 +62,7 @@ func (ue *multipartBodyProcessor) Read(reader io.Reader, mime string, storagePat fcs := map[string][]string{ "": {fmt.Sprintf("%d", totalSize)}, } - ue.collections = &collectionsMap{ + mbp.collections = &collectionsMap{ variables.FilesNames: fn, variables.Files: fl, variables.FilesSizes: fs, @@ -74,18 +74,18 @@ func (ue *multipartBodyProcessor) Read(reader io.Reader, mime string, storagePat return nil } -func (js *multipartBodyProcessor) Collections() collectionsMap { - return *js.collections +func (mbp *multipartBodyProcessor) Collections() collectionsMap { + return *mbp.collections } -func (js *multipartBodyProcessor) Find(expr string) (map[string][]string, error) { +func (mbp *multipartBodyProcessor) Find(expr string) (map[string][]string, error) { return nil, nil } -func (js *multipartBodyProcessor) VariableHook() variables.RuleVariable { - return variables.Json +func (mbp *multipartBodyProcessor) VariableHook() variables.RuleVariable { + return variables.JSON } var ( - _ BodyProcessor = &multipartBodyProcessor{} + _ BodyProcessor = (*multipartBodyProcessor)(nil) ) diff --git a/bodyprocessors/urlencoded.go b/bodyprocessors/urlencoded.go index ac1eb1651..2753b438b 100644 --- a/bodyprocessors/urlencoded.go +++ b/bodyprocessors/urlencoded.go @@ -27,21 +27,21 @@ type urlencodedBodyProcessor struct { collections *collectionsMap } -func (ue *urlencodedBodyProcessor) Read(reader io.Reader, _ string, _ string) error { +func (ubp *urlencodedBodyProcessor) Read(reader io.Reader, _ string, _ string) error { buf := new(strings.Builder) if _, err := io.Copy(buf, reader); err != nil { return err } b := buf.String() - //TODO add url encode validation - //tx.GetCollection(VARIABLE_URLENCODED_ERROR).Set("", []string{err.Error()}) + // TODO add url encode validation + // tx.GetCollection(VARIABLE_URLENCODED_ERROR).Set("", []string{err.Error()}) values := utils.ParseQuery(b, "&") m := map[string][]string{} for k, vs := range values { m[k] = vs } - ue.collections = &collectionsMap{ + ubp.collections = &collectionsMap{ variables.ArgsPost: m, variables.Args: m, variables.RequestBody: map[string][]string{ @@ -54,16 +54,16 @@ func (ue *urlencodedBodyProcessor) Read(reader io.Reader, _ string, _ string) er return nil } -func (js *urlencodedBodyProcessor) Collections() collectionsMap { - return *js.collections +func (ubp *urlencodedBodyProcessor) Collections() collectionsMap { + return *ubp.collections } -func (js *urlencodedBodyProcessor) Find(expr string) (map[string][]string, error) { +func (ubp *urlencodedBodyProcessor) Find(expr string) (map[string][]string, error) { return nil, nil } -func (js *urlencodedBodyProcessor) VariableHook() variables.RuleVariable { - return variables.Json +func (ubp *urlencodedBodyProcessor) VariableHook() variables.RuleVariable { + return variables.JSON } var ( diff --git a/bodyprocessors/xml.go b/bodyprocessors/xml.go index 1aa2896ab..60a07847a 100644 --- a/bodyprocessors/xml.go +++ b/bodyprocessors/xml.go @@ -54,7 +54,7 @@ func (xml *xmlBodyProcessor) Find(expr string) (map[string][]string, error) { } func (xml *xmlBodyProcessor) VariableHook() variables.RuleVariable { - return variables.Xml + return variables.XML } var ( diff --git a/collection.go b/collection.go index 00555dbae..747275738 100644 --- a/collection.go +++ b/collection.go @@ -36,12 +36,12 @@ func (c *Collection) Get(key string) []string { return c.data[key] } -//Find is returns a slice of MatchData for the +// Find is returns a slice of MatchData for the // regex or key, exceptions are used to skip // some keys func (c *Collection) Find(key string, re *regexp.Regexp, exceptions []string) []MatchData { cdata := c.data - //we return every value in case there is no key but there is a collection + // we return every value in case there is no key but there is a collection va, _ := variables.ParseVariable(c.name) if len(key) == 0 { data := []MatchData{} @@ -82,7 +82,7 @@ func (c *Collection) Find(key string, re *regexp.Regexp, exceptions []string) [] return result } else { ret := []MatchData{} - //We pass through every record to apply filters + // We pass through every record to apply filters for k := range cdata { if utils.StringInSlice(k, exceptions) { continue @@ -102,36 +102,32 @@ func (c *Collection) Find(key string, re *regexp.Regexp, exceptions []string) [] } } -// GetFirstString returns the first string ocurrence of a key +// GetFirstString returns the first string occurrence of a key func (c *Collection) GetFirstString(key string) string { - a := c.data[key] - if len(a) > 0 { + if a, ok := c.data[key]; ok && len(a) > 0 { return a[0] - } else { - return "" } + return "" } -// GetFirstInt64 returns the first int64 ocurrence of a key +// GetFirstInt64 returns the first int64 occurrence of a key func (c *Collection) GetFirstInt64(key string) int64 { a := c.data[key] if len(a) > 0 { i, _ := strconv.ParseInt(a[0], 10, 64) return i - } else { - return 0 } + return 0 } -// GetFirstInt returns the first int ocurrence of a key +// GetFirstInt returns the first int occurrence of a key func (c *Collection) GetFirstInt(key string) int { a := c.data[key] if len(a) > 0 { i, _ := strconv.Atoi(a[0]) return i - } else { - return 0 } + return 0 } // Add a value to some key diff --git a/geo/geo.go b/geo/geo.go index 205434b74..e593cb13b 100644 --- a/geo/geo.go +++ b/geo/geo.go @@ -1,3 +1,17 @@ +// Copyright 2021 Juan Pablo Tosso +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package geo import "fmt" diff --git a/loggers/auditlog.go b/loggers/auditlog.go index bd1eb5e18..48b575d38 100644 --- a/loggers/auditlog.go +++ b/loggers/auditlog.go @@ -33,24 +33,25 @@ type AuditTransaction struct { UnixTimestamp int64 `json:"unix_timestamp"` // Unique ID - Id string `json:"id"` + ID string `json:"id"` // Client IP Address string representation - ClientIp string `json:"client_ip"` + ClientIP string `json:"client_ip"` ClientPort int `json:"client_port"` - HostIp string `json:"host_ip"` + HostIP string `json:"host_ip"` HostPort int `json:"host_port"` - ServerId string `json:"server_id"` + ServerID string `json:"server_id"` Request AuditTransactionRequest `json:"request"` Response AuditTransactionResponse `json:"response"` Producer AuditTransactionProducer `json:"producer"` } type AuditTransactionResponse struct { - Status int `json:"status"` - Headers map[string][]string `json:"headers"` - Body string `json:"body"` + Protocol string `json:"protocol"` + Status int `json:"status"` + Headers map[string][]string `json:"headers"` + Body string `json:"body"` } type AuditTransactionProducer struct { @@ -65,8 +66,8 @@ type AuditTransactionProducer struct { type AuditTransactionRequest struct { Method string `json:"method"` Protocol string `json:"protocol"` - Uri string `json:"uri"` - HttpVersion string `json:"http_version"` + URI string `json:"uri"` + HTTPVersion string `json:"http_version"` Headers map[string][]string `json:"headers"` Body string `json:"body"` Files []AuditTransactionRequestFiles `json:"files"` @@ -87,7 +88,7 @@ type AuditMessage struct { type AuditMessageData struct { File string `json:"file"` Line int `json:"line"` - Id int `json:"id"` + ID int `json:"id"` Rev string `json:"rev"` Msg string `json:"msg"` Data string `json:"data"` @@ -121,7 +122,7 @@ type auditLogLegacy struct { type auditLogLegacyTransaction struct { Time string - TransactionId string + TransactionID string RemoteAddress string RemotePort int LocalAddress string diff --git a/loggers/concurrent_writer.go b/loggers/concurrent_writer.go index 4dd7ea997..8f0b342d4 100644 --- a/loggers/concurrent_writer.go +++ b/loggers/concurrent_writer.go @@ -51,17 +51,16 @@ func (cl concurrentWriter) Write(al AuditLog) error { p2 := fmt.Sprintf("/%s/%s/", t.Format("20060102"), t.Format("20060102-1504")) logdir := path.Join(cl.logger.directory, p2) // Append the filename - fname := fmt.Sprintf("/%s-%s", t.Format("20060102-150405"), al.Transaction.Id) + fname := fmt.Sprintf("/%s-%s", t.Format("20060102-150405"), al.Transaction.ID) filepath := path.Join(logdir, fname) str := fmt.Sprintf("%s %s - - [%s] %q %d %d %q %q %s %q %s %d %d", - al.Transaction.ClientIp, al.Transaction.HostIp, al.Transaction.Timestamp, - fmt.Sprintf("%s %s %s", al.Transaction.Request.Method, al.Transaction.Request.Uri, - al.Transaction.Request.HttpVersion), - al.Transaction.Response.Status, 0 /*response length*/, "-", "-", al.Transaction.Id, + al.Transaction.ClientIP, al.Transaction.HostIP, al.Transaction.Timestamp, + fmt.Sprintf("%s %s %s", al.Transaction.Request.Method, al.Transaction.Request.URI, + al.Transaction.Request.HTTPVersion), + al.Transaction.Response.Status, 0 /*response length*/, "-", "-", al.Transaction.ID, "-", filepath, 0, 0 /*request length*/) err := os.MkdirAll(logdir, cl.logger.dirMode) if err != nil { - //logrus.Error("Failed to create concurrent audit path") return err } diff --git a/loggers/concurrent_writer_test.go b/loggers/concurrent_writer_test.go index 7d7c96302..4297c7de5 100644 --- a/loggers/concurrent_writer_test.go +++ b/loggers/concurrent_writer_test.go @@ -46,7 +46,7 @@ func TestCLogFileCreation(t *testing.T) { al := AuditLog{ Transaction: AuditTransaction{ UnixTimestamp: ts, - Id: "123", + ID: "123", Request: AuditTransactionRequest{}, Response: AuditTransactionResponse{}, }, @@ -58,7 +58,7 @@ func TestCLogFileCreation(t *testing.T) { p2 := fmt.Sprintf("/%s/%s/", tt.Format("20060102"), tt.Format("20060102-1504")) logdir := path.Join("/tmp", p2) // Append the filename - fname := fmt.Sprintf("/%s-%s", tt.Format("20060102-150405"), al.Transaction.Id) + fname := fmt.Sprintf("/%s-%s", tt.Format("20060102-150405"), al.Transaction.ID) p := path.Join(logdir, fname) data, err := os.ReadFile(p) diff --git a/loggers/formats.go b/loggers/formats.go index 445ec9a65..ff0db03ba 100644 --- a/loggers/formats.go +++ b/loggers/formats.go @@ -30,9 +30,39 @@ func jsonFormatter(al AuditLog) ([]byte, error) { } // Coraza json format -// TBI func json2Formatter(al AuditLog) ([]byte, error) { - jsdata, err := json.Marshal(al) + reqHeaders := map[string]string{} + for k, v := range al.Transaction.Request.Headers { + reqHeaders[k] = v[0] + } + resHeaders := map[string]string{} + for k, v := range al.Transaction.Response.Headers { + resHeaders[k] = v[0] + } + al2 := auditLogLegacy{ + Transaction: auditLogLegacyTransaction{ + Time: al.Transaction.Timestamp, + TransactionID: al.Transaction.ID, + RemoteAddress: al.Transaction.ClientIP, + RemotePort: al.Transaction.ClientPort, + LocalAddress: al.Transaction.HostIP, + LocalPort: al.Transaction.HostPort, + }, + Request: auditLogLegacyRequest{ + RequestLine: fmt.Sprintf("%s %s %s", al.Transaction.Request.Method, al.Transaction.Request.URI, al.Transaction.Request.HTTPVersion), + Headers: reqHeaders, + }, + Response: auditLogLegacyResponse{ + Status: al.Transaction.Response.Status, + Protocol: al.Transaction.Response.Protocol, + Headers: resHeaders, + }, + AuditData: auditLogLegacyData{ + Stopwatch: auditLogLegacyStopwatch{}, + }, + } + + jsdata, err := json.Marshal(al2) if err != nil { return nil, err } @@ -86,25 +116,25 @@ func nativeFormatter(al AuditLog) ([]byte, error) { boundary := utils.RandomString(10) parts := map[byte]string{} // [27/Jul/2016:05:46:16 +0200] V5guiH8AAQEAADTeJ2wAAAAK 192.168.3.1 50084 192.168.3.111 80 - parts['A'] = fmt.Sprintf("[%s] %s %s %d %s %d", al.Transaction.Timestamp, al.Transaction.Id, - al.Transaction.ClientIp, al.Transaction.ClientPort, al.Transaction.HostIp, al.Transaction.HostPort) - //GET /url HTTP/1.1 - //Host: example.com - //User-Agent: Mozilla/5.0 - //Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 - //Accept-Language: en-US,en;q=0.5 - //Accept-Encoding: gzip, deflate - //Referer: http://example.com/index.html - //Connection: keep-alive - //Content-Type: application/x-www-form-urlencoded - //Content-Length: 6 - parts['B'] = fmt.Sprintf("%s %s %s\n", al.Transaction.Request.Method, al.Transaction.Request.Uri, al.Transaction.Request.Protocol) + parts['A'] = fmt.Sprintf("[%s] %s %s %d %s %d", al.Transaction.Timestamp, al.Transaction.ID, + al.Transaction.ClientIP, al.Transaction.ClientPort, al.Transaction.HostIP, al.Transaction.HostPort) + // GET /url HTTP/1.1 + // Host: example.com + // User-Agent: Mozilla/5.0 + // Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 + // Accept-Language: en-US,en;q=0.5 + // Accept-Encoding: gzip, deflate + // Referer: http://example.com/index.html + // Connection: keep-alive + // Content-Type: application/x-www-form-urlencoded + // Content-Length: 6 + parts['B'] = fmt.Sprintf("%s %s %s\n", al.Transaction.Request.Method, al.Transaction.Request.URI, al.Transaction.Request.Protocol) for k, vv := range al.Transaction.Request.Headers { for _, v := range vv { parts['B'] += fmt.Sprintf("%s: %s\n", k, v) } } - //b=test + // b=test parts['C'] = al.Transaction.Request.Body parts['E'] = al.Transaction.Response.Body parts['F'] = "" @@ -113,13 +143,13 @@ func nativeFormatter(al AuditLog) ([]byte, error) { parts['F'] += fmt.Sprintf("%s: %s\n", k, v) } } - //Stopwatch: 1470025005945403 1715 (- - -) - //Stopwatch2: 1470025005945403 1715; combined=26, p1=0, p2=0, p3=0, p4=0, p5=26, ↩ - //sr=0, sw=0, l=0, gc=0 - //Response-Body-Transformed: Dechunked - //Producer: ModSecurity for Apache/2.9.1 (http://www.modsecurity.org/). - //Server: Apache - //Engine-Mode: "ENABLED" + // Stopwatch: 1470025005945403 1715 (- - -) + // Stopwatch2: 1470025005945403 1715; combined=26, p1=0, p2=0, p3=0, p4=0, p5=26, ↩ + // sr=0, sw=0, l=0, gc=0 + // Response-Body-Transformed: Dechunked + // Producer: ModSecurity for Apache/2.9.1 (http://www.modsecurity.org/). + // Server: Apache + // Engine-Mode: "ENABLED" parts['H'] = fmt.Sprintf("Stopwatch: %s\nResponse-Body-Transformed: %s\nProducer: %s\nServer: %s", "", "", "", "") parts['K'] = "" for _, r := range al.Messages { diff --git a/loggers/formats_test.go b/loggers/formats_test.go index 7a2350fe4..6cb8d2d92 100644 --- a/loggers/formats_test.go +++ b/loggers/formats_test.go @@ -14,10 +14,6 @@ package loggers -import ( - "testing" -) - /* func TestFormatters(t *testing.T) { al := createAuditLog() @@ -45,7 +41,7 @@ func TestFormatters(t *testing.T) { } } } -}*/ +} func TestModsecBoundary(t *testing.T) { // TODO... @@ -85,3 +81,4 @@ func createAuditLog() AuditLog { }, } } +*/ diff --git a/loggers/logger.go b/loggers/logger.go index a16136054..b58e3fc88 100644 --- a/loggers/logger.go +++ b/loggers/logger.go @@ -19,8 +19,6 @@ import ( "io/fs" "os" "path" - - "github.com/jptosso/coraza-waf/v2/utils" ) // Logger is a wrapper to hold configurations, a writer and a formatter @@ -45,10 +43,16 @@ func (l Logger) Write(al AuditLog) error { return l.writer.Write(al) } +// Close is sed to close the write stream +// It must be called when the waf instance won't be used anymore func (l *Logger) Close() error { return l.writer.Close() } +// SetFormatter sets the formatter for the logger +// A valid formatter created using RegisterLogFormatter(...) is required +// Default formatters are: json, json2 and native +// json2 is an "enhanced" version of the original modsecurity json formatter func (l *Logger) SetFormatter(f string) error { formatter, err := getLogFormatter(f) if err != nil { @@ -58,6 +62,9 @@ func (l *Logger) SetFormatter(f string) error { return nil } +// SetWriter sets the writer for the logger +// A valid writer created using RegisterLogWriter(...) is required +// Default writers are: serial and concurrent func (l *Logger) SetWriter(name string) error { writer, err := getLogWriter(name) if err != nil { @@ -67,7 +74,13 @@ func (l *Logger) SetWriter(name string) error { return nil } +// 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) + +// LogWriter is the interface for all log writers +// A LogWriter receives an auditlog and writes it to the output stream +// An output stream may be a file, a socket, an http request, etc type LogWriter interface { // In case the writer requires previous preparations Init(*Logger) error @@ -88,7 +101,7 @@ func RegisterLogWriter(name string, writer func() LogWriter) { writers[name] = writer } -// GetLogger returns a logger by name +// getLogWriter returns a logger by name // It returns an error if it doesn't exist func getLogWriter(name string) (LogWriter, error) { logger := writers[name] @@ -98,7 +111,7 @@ func getLogWriter(name string) (LogWriter, error) { return logger(), nil } -// RegisterLogFormat registers a new logger format +// RegisterLogFormatter registers a new logger format // it can be used for plugins func RegisterLogFormatter(name string, f func(al AuditLog) ([]byte, error)) { formatters[name] = f @@ -114,6 +127,13 @@ func getLogFormatter(name string) (LogFormatter, error) { return formatter, nil } +// NewAuditLogger creates a default logger +// Default settings are: +// Dirmode: 0755 +// Filemode: 0644 +// Formatter: native +// Writer: serial +// Path: /tmp/coraza-audit.log func NewAuditLogger() (*Logger, error) { /* if file == "" { @@ -125,7 +145,7 @@ func NewAuditLogger() (*Logger, error) { dirMode := fs.FileMode(0755) fileMode := fs.FileMode(0644) s := &serialWriter{} - f := path.Join(os.TempDir(), utils.RandomString(10)+"-coraza.log") + f := path.Join(os.TempDir(), "coraza-audit.log") l := &Logger{ file: f, directory: "/opt/coraza/var/log/audit/", @@ -148,5 +168,5 @@ func init() { RegisterLogFormatter("json", jsonFormatter) RegisterLogFormatter("json2", json2Formatter) RegisterLogFormatter("native", nativeFormatter) - //RegisterLogFormatter("cef", cefFormatter) + // RegisterLogFormatter("cef", cefFormatter) } diff --git a/loggers/serial_writer_test.go b/loggers/serial_writer_test.go index 7c248d96d..b9463e24b 100644 --- a/loggers/serial_writer_test.go +++ b/loggers/serial_writer_test.go @@ -39,12 +39,12 @@ func TestSerialLogger_Write(t *testing.T) { } al := AuditLog{ Transaction: AuditTransaction{ - Id: "test123", + ID: "test123", }, Messages: []AuditMessage{ { Data: AuditMessageData{ - Id: 100, + ID: 100, Raw: "SecAction \"id:100\"", }, }, diff --git a/operators/inspect_file.go b/operators/inspect_file.go index 46c91ca50..925a7921f 100644 --- a/operators/inspect_file.go +++ b/operators/inspect_file.go @@ -32,12 +32,12 @@ func (o *inspectFile) Init(data string) error { } func (o *inspectFile) Evaluate(tx *engine.Transaction, value string) bool { - //TODO parametrize timeout - //TODO add relative path capabilities - //TODO add lua special support + // TODO parametrize timeout + // TODO add relative path capabilities + // TODO add lua special support ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - //Add /bin/bash to context? + // Add /bin/bash to context? cmd := exec.CommandContext(ctx, o.path, value) _, err := cmd.CombinedOutput() if ctx.Err() == context.DeadlineExceeded || err != nil { diff --git a/operators/ip_match.go b/operators/ip_match.go index 6d392e8d2..f0e73bcab 100644 --- a/operators/ip_match.go +++ b/operators/ip_match.go @@ -34,11 +34,11 @@ func (o *ipMatch) Init(data string) error { continue } if strings.Contains(sb, ":") && !strings.Contains(sb, "/") { - //ipv6 - sb = sb + "/128" + // ipv6 + sb += "/128" } else if strings.Contains(sb, ".") && !strings.Contains(sb, "/") { - //ipv4 - sb = sb + "/32" + // ipv4 + sb += "/32" } _, subnet, err := net.ParseCIDR(sb) if err != nil { diff --git a/operators/operators.go b/operators/operators.go index 7f526d393..a85bdc2ce 100644 --- a/operators/operators.go +++ b/operators/operators.go @@ -40,7 +40,7 @@ func init() { RegisterOperator("pmFromFile", func() coraza.RuleOperator { return &pmFromFile{} }) RegisterOperator("pm", func() coraza.RuleOperator { return &pm{} }) RegisterOperator("validateByteRange", func() coraza.RuleOperator { return &validateByteRange{} }) - RegisterOperator("validateUrlEncoding", func() coraza.RuleOperator { return &validateUrlEncoding{} }) + RegisterOperator("validateUrlEncoding", func() coraza.RuleOperator { return &validateURLEncoding{} }) RegisterOperator("streq", func() coraza.RuleOperator { return &streq{} }) RegisterOperator("ipMatch", func() coraza.RuleOperator { return &ipMatch{} }) RegisterOperator("ipMatchFromFile", func() coraza.RuleOperator { return &ipMatchFromFile{} }) diff --git a/operators/operators_test.go b/operators/operators_test.go index 4890b2ced..c68bf077f 100644 --- a/operators/operators_test.go +++ b/operators/operators_test.go @@ -61,7 +61,7 @@ func TestTransformations(t *testing.T) { t.Error("Cannot parse test case") } for _, data := range cases { - //UNMARSHALL does not transform \u0000 to binary + // UNMARSHALL does not transform \u0000 to binary data.Input = strings.ReplaceAll(data.Input, `\u0000`, "\u0000") data.Param = strings.ReplaceAll(data.Param, `\u0000`, "\u0000") diff --git a/operators/pm.go b/operators/pm.go index 521ee5d23..9c3ae3937 100644 --- a/operators/pm.go +++ b/operators/pm.go @@ -21,7 +21,7 @@ import ( "github.com/jptosso/coraza-waf/v2" ) -//TODO according to coraza researchs, re2 matching is faster than ahocorasick +// TODO according to coraza researchs, re2 matching is faster than ahocorasick // maybe we should switch in the future // pm is always lowercase type pm struct { diff --git a/operators/pm_from_file.go b/operators/pm_from_file.go index 4fe6d2a70..50732753a 100644 --- a/operators/pm_from_file.go +++ b/operators/pm_from_file.go @@ -18,7 +18,7 @@ import ( "strings" "github.com/cloudflare/ahocorasick" - engine "github.com/jptosso/coraza-waf/v2" + "github.com/jptosso/coraza-waf/v2" ) type pmFromFile struct { @@ -33,7 +33,7 @@ func (o *pmFromFile) Init(data string) error { if len(l) == 0 { continue } - l = strings.ReplaceAll(l, "\r", "") //CLF + l = strings.ReplaceAll(l, "\r", "") // CLF if l[0] != '#' { lines = append(lines, strings.ToLower(l)) } @@ -45,6 +45,6 @@ func (o *pmFromFile) Init(data string) error { return nil } -func (o *pmFromFile) Evaluate(tx *engine.Transaction, value string) bool { +func (o *pmFromFile) Evaluate(tx *coraza.Transaction, value string) bool { return o.pm.Evaluate(tx, value) } diff --git a/operators/pm_from_file_test.go b/operators/pm_from_file_test.go index 9d3ac5a19..27fcdc766 100644 --- a/operators/pm_from_file_test.go +++ b/operators/pm_from_file_test.go @@ -1,3 +1,17 @@ +// Copyright 2021 Juan Pablo Tosso +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package operators import ( diff --git a/operators/rbl.go b/operators/rbl.go index 93d710108..b8cd9915a 100644 --- a/operators/rbl.go +++ b/operators/rbl.go @@ -20,6 +20,7 @@ import ( "time" "github.com/jptosso/coraza-waf/v2" + "github.com/jptosso/coraza-waf/v2/types/variables" ) type rbl struct { @@ -28,16 +29,16 @@ type rbl struct { func (o *rbl) Init(data string) error { o.service = data - //TODO validate hostname + // TODO validate hostname return nil } -//https://github.com/mrichman/godnsbl -//https://github.com/SpiderLabs/ModSecurity/blob/b66224853b4e9d30e0a44d16b29d5ed3842a6b11/src/operators/rbl.cc +// https://github.com/mrichman/godnsbl +// https://github.com/SpiderLabs/ModSecurity/blob/b66224853b4e9d30e0a44d16b29d5ed3842a6b11/src/operators/rbl.cc func (o *rbl) Evaluate(tx *coraza.Transaction, value string) bool { - //TODO validate address + // TODO validate address c1 := make(chan bool) - //captures := []string{} + captures := []string{} addr := fmt.Sprintf("%s.%s", value, o.service) go func() { @@ -45,21 +46,21 @@ func (o *rbl) Evaluate(tx *coraza.Transaction, value string) bool { if err != nil { c1 <- false } - //var status string + // var status string if len(res) > 0 { txt, _ := net.LookupTXT(addr) if len(txt) > 0 { - //status = txt[0] - //captures = append(captures, txt[0]) - //tx.Collections["tx"].Data["httpbl_msg"] = []string{status} + status := txt[0] + captures = append(captures, txt[0]) + tx.GetCollection(variables.TX).Set("httpbl_msg", []string{status}) } } c1 <- true }() select { case res := <-c1: - if tx.Capture && res { - //tx.AddCapture() + if res && len(captures) > 0 { + tx.CaptureField(0, captures[0]) } return res case <-time.After(1): diff --git a/operators/rx.go b/operators/rx.go index 19993cae8..e6a55869e 100644 --- a/operators/rx.go +++ b/operators/rx.go @@ -41,10 +41,10 @@ func (o *rx) Evaluate(tx *coraza.Transaction, value string) bool { if i == 9 { return true } - //I actually think everything should be capturable, there is no need for the capture action... - //if tx.IsCapturable() { + // I actually think everything should be capturable, there is no need for the capture action... + // if tx.IsCapturable() { tx.CaptureField(i+1, m) - //} + // } } return len(match) > 0 } diff --git a/operators/validate_byte_range.go b/operators/validate_byte_range.go index ce846dfaf..42f434c86 100644 --- a/operators/validate_byte_range.go +++ b/operators/validate_byte_range.go @@ -49,14 +49,14 @@ func (o *validateByteRange) Init(data string) error { rega = append(rega, fmt.Sprintf("[\\x%s-\\x%s]", b1h, b2h)) } rege := strings.Join(rega, "|") - //fmt.Println(rege) + // fmt.Println(rege) o.re = regexp.MustCompile(rege) return nil } func (o *validateByteRange) Evaluate(tx *coraza.Transaction, data string) bool { data = o.re.ReplaceAllString(data, "") - //fmt.Println("DEBUG: ", data, len(data)) - //fmt.Printf("%s: %d\n", data, len(data)) + // fmt.Println("DEBUG: ", data, len(data)) + // fmt.Printf("%s: %d\n", data, len(data)) return len(data) > 0 } diff --git a/operators/validate_byte_range_test.go b/operators/validate_byte_range_test.go index d0d5232ec..7badf94c5 100644 --- a/operators/validate_byte_range_test.go +++ b/operators/validate_byte_range_test.go @@ -22,13 +22,13 @@ import ( func TestCRS920272(t *testing.T) { ranges := "32-36,38-126" - good_strings := [][]int{ + goodStrings := [][]int{ {104, 101, 108, 111, 32, 119, 97, 122, 122, 117, 112, 32, 98, 114, 111}, {38, 104, 101, 108, 111, 32, 119, 97, 122, 122, 117, 112, 32, 98, 114, 111, 126}, {32, 104, 101, 108, 111, 32, 119, 97, 122, 122, 117, 112, 32, 98, 114, 111, 125}, } - bad_strings := [][]int{ + badStrings := [][]int{ {35, 38, 104, 101, 108, 111, 32, 119, 97, 122, 122, 117, 112, 32, 98, 114, 127, 128}, {104, 101, 108, 111, 32, 119, 97, 122, 122, 117, 112, 32, 98, 114, 111, -1}, {104, 101, 108, 111, 32, 119, 97, 122, 122, 117, 112, 32, 98, 114, 111, 0}, @@ -40,14 +40,14 @@ func TestCRS920272(t *testing.T) { } tx := getTransaction() - for _, gs := range good_strings { + for _, gs := range goodStrings { str := asciiToString(gs) if op.Evaluate(tx, str) { t.Errorf("Invalid byte between ranges (positive): %s", str) } } - for _, bs := range bad_strings { + for _, bs := range badStrings { str := asciiToString(bs) if !op.Evaluate(tx, str) { t.Errorf("Invalid byte between ranges (negative): %s", str) diff --git a/operators/validate_nid.go b/operators/validate_nid.go index f8b73c27e..982b4d32d 100644 --- a/operators/validate_nid.go +++ b/operators/validate_nid.go @@ -56,7 +56,7 @@ func (o *validateNid) Evaluate(tx *coraza.Transaction, value string) bool { if i >= 10 { break } - //should we capture more than one NID? + // should we capture more than one NID? if o.fn(m[0]) { res = true if tx.Capture { @@ -71,10 +71,7 @@ func nidCl(nid string) bool { if len(nid) < 8 { return false } - re, err := regexp.Compile(`[^\dk]`) - if err != nil { - return false - } + re := regexp.MustCompile(`[^\dk]`) nid = strings.ToLower(nid) nid = re.ReplaceAllString(nid, "") rut, _ := strconv.Atoi(nid[:len(nid)-1]) @@ -105,10 +102,7 @@ func nidCl(nid string) bool { } func nidUs(nid string) bool { - re, err := regexp.Compile(`[^\d]`) - if err != nil { - return false - } + re := regexp.MustCompile(`[^\d]`) nid = re.ReplaceAllString(nid, "") if len(nid) < 9 { return false diff --git a/operators/validate_nid_test.go b/operators/validate_nid_test.go index 864b87801..442961512 100644 --- a/operators/validate_nid_test.go +++ b/operators/validate_nid_test.go @@ -1,3 +1,17 @@ +// Copyright 2021 Juan Pablo Tosso +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package operators import "testing" diff --git a/operators/validate_url_encoding.go b/operators/validate_url_encoding.go index ce5b4000b..df3230870 100644 --- a/operators/validate_url_encoding.go +++ b/operators/validate_url_encoding.go @@ -16,20 +16,20 @@ package operators import "github.com/jptosso/coraza-waf/v2" -type validateUrlEncoding struct { +type validateURLEncoding struct { } -func (o *validateUrlEncoding) Init(data string) error { +func (o *validateURLEncoding) Init(data string) error { // Does not require initialization return nil } -func (o *validateUrlEncoding) Evaluate(tx *coraza.Transaction, value string) bool { +func (o *validateURLEncoding) Evaluate(tx *coraza.Transaction, value string) bool { if len(value) == 0 { return false } - rc := validateUrlEncodingInternal(value, len(value)) + rc := validateURLEncodingInternal(value, len(value)) switch rc { case 1: /* Encoding is valid */ @@ -38,42 +38,41 @@ func (o *validateUrlEncoding) Evaluate(tx *coraza.Transaction, value string) boo // Invalid URL Encoding: Non-hexadecimal return true case -3: - //Invalid URL Encoding: Not enough characters at the end of input + // Invalid URL Encoding: Not enough characters at the end of input return true case -1: default: - //Invalid URL Encoding: Internal error + // Invalid URL Encoding: Internal error return true } return true } -func validateUrlEncodingInternal(input string, input_length int) int { +func validateURLEncodingInternal(input string, inputLen int) int { var i int - if input_length == 0 { + if inputLen == 0 { return -1 } - for i < input_length { + for i < inputLen { if input[i] == '%' { - if i+2 >= input_length { + if i+2 >= inputLen { /* Not enough bytes. */ return -3 - } else { - /* Here we only decode a %xx combination if it is valid, - * leaving it as is otherwise. - */ - c1 := input[i+1] - c2 := input[i+2] + } + /* Here we only decode a %xx combination if it is valid, + * leaving it as is otherwise. + */ + c1 := input[i+1] + c2 := input[i+2] - if (((c1 >= '0') && (c1 <= '9')) || ((c1 >= 'a') && (c1 <= 'f')) || ((c1 >= 'A') && (c1 <= 'F'))) && (((c2 >= '0') && (c2 <= '9')) || ((c2 >= 'a') && (c2 <= 'f')) || ((c2 >= 'A') && (c2 <= 'F'))) { - i += 3 - } else { - /* Non-hexadecimal characters used in encoding. */ - return -2 - } + if (((c1 >= '0') && (c1 <= '9')) || ((c1 >= 'a') && (c1 <= 'f')) || ((c1 >= 'A') && (c1 <= 'F'))) && (((c2 >= '0') && (c2 <= '9')) || ((c2 >= 'a') && (c2 <= 'f')) || ((c2 >= 'A') && (c2 <= 'F'))) { + i += 3 + } else { + /* Non-hexadecimal characters used in encoding. */ + return -2 } } else { i++ diff --git a/operators/verify_cc.go b/operators/verify_cc.go deleted file mode 100644 index 4fd59a5ce..000000000 --- a/operators/verify_cc.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2021 Juan Pablo Tosso -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package operators - -import "github.com/jptosso/coraza-waf/v2" - -type VerifyCC struct { -} - -func (o *VerifyCC) Init(data string) error { - // not implemented - return nil -} - -func (o *VerifyCC) Evaluate(tx *coraza.Transaction, value string) bool { - //not implemented - return false -} diff --git a/rule.go b/rule.go index 7caf64701..c4f4da78e 100644 --- a/rule.go +++ b/rule.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http:// www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -91,7 +91,7 @@ type ruleVariableParams struct { Key string // If not nil, a regex will be used instead of a key - Regex *regexp.Regexp //for performance + Regex *regexp.Regexp // for performance // A slice of key exceptions Exceptions []string @@ -144,7 +144,7 @@ type Rule struct { // Line of the file where this rule was found Line int - //METADATA + // METADATA // Rule unique identifier, can be a an int Id int @@ -243,7 +243,7 @@ func (r *Rule) Evaluate(tx *Transaction) []MatchData { if v.Count { l := 0 if v.Key != "" { - //Get with macro expansion + // Get with macro expansion values = tx.GetField(v, exceptions) l = len(values) } else { @@ -297,7 +297,7 @@ func (r *Rule) Evaluate(tx *Transaction) []MatchData { tx.Waf.Logger.Debug("Evaluate rule operator", zap.String("txid", tx.Id), zap.Int("rule", rid), zap.String("event", "EVALUATE_RULE_OPERATOR"), - zap.String("operator", "nn"), //TODO fix + zap.String("operator", "nn"), // TODO fix zap.String("data", carg), zap.String("variable", arg.Variable.Name()), zap.String("key", arg.Key), @@ -310,7 +310,7 @@ func (r *Rule) Evaluate(tx *Transaction) []MatchData { } if len(matchedValues) == 0 { - //No match for variables + // No match for variables return matchedValues } @@ -318,7 +318,7 @@ func (r *Rule) Evaluate(tx *Transaction) []MatchData { zap.Int("rule", rid), zap.String("event", "EVALUATE_RULE_OPERATOR"), zap.Any("values", matchedValues)) - // we must match the vars before runing the chains + // we must match the vars before running the chains tx.MatchVariable(matchedValues[0]) // We run non disruptive actions even if there is no chain match @@ -334,7 +334,7 @@ func (r *Rule) Evaluate(tx *Transaction) []MatchData { for nr != nil { m := nr.Evaluate(tx) if len(m) == 0 { - //we fail the chain + // we fail the chain return []MatchData{} } @@ -350,14 +350,14 @@ func (r *Rule) Evaluate(tx *Transaction) []MatchData { MatchedData: matchedValues[0], Message: tx.MacroExpansion(r.Msg), Data: tx.MacroExpansion(r.LogData), - Uri: tx.GetCollection(variables.RequestUri).GetFirstString(""), + Uri: tx.GetCollection(variables.RequestURI).GetFirstString(""), Id: tx.Id, Disruptive: r.Disruptive, ServerIpAddress: tx.GetCollection(variables.ServerAddr).GetFirstString(""), ClientIpAddress: tx.GetCollection(variables.RemoteAddr).GetFirstString(""), }) } - //we need to add disruptive actions in the end, otherwise they would be triggered without their chains. + // we need to add disruptive actions in the end, otherwise they would be triggered without their chains. tx.Waf.Logger.Debug("detecting rule disruptive action", zap.String("txid", tx.Id), zap.Int("rule", r.Id)) for _, a := range r.actions { if a.Function.Type() == types.ActionTypeDisruptive || a.Function.Type() == types.ActionTypeFlow { diff --git a/rule_match.go b/rule_match.go index 42daf5713..6f3960867 100644 --- a/rule_match.go +++ b/rule_match.go @@ -99,7 +99,7 @@ func (mr MatchedRule) matchData() string { func (mr MatchedRule) AuditLog(code int) string { log := &strings.Builder{} - log.WriteString(fmt.Sprintf("[client %q]", mr.ClientIpAddress)) + log.WriteString(fmt.Sprintf("[client %q] ", mr.ClientIpAddress)) if mr.Disruptive { log.WriteString(fmt.Sprintf("Coraza: Access denied with code %d (phase %d). ", code, mr.Rule.Phase)) } else { @@ -110,7 +110,7 @@ func (mr MatchedRule) AuditLog(code int) string { return log.String() } -//ErrorLog returns the same as audit log but without matchData +// ErrorLog returns the same as audit log but without matchData func (mr MatchedRule) ErrorLog(code int) string { msg := mr.Message if len(msg) > 200 { @@ -123,7 +123,7 @@ func (mr MatchedRule) ErrorLog(code int) string { } else { log.WriteString("Coraza: Warning. ") } - //log.WriteString(mr.matchData()) + // log.WriteString(mr.matchData()) log.WriteString(msg) log.WriteString(mr.details()) return log.String() diff --git a/rulegroup.go b/rulegroup.go index 21e1907b0..3616e0d77 100644 --- a/rulegroup.go +++ b/rulegroup.go @@ -143,7 +143,7 @@ func (rg *ruleGroup) Eval(phase types.RulePhase, tx *Transaction) bool { continue } - //we always evaluate secmarkers + // we always evaluate secmarkers if tx.SkipAfter != "" { if r.SecMark == tx.SkipAfter { tx.Waf.Logger.Debug("SkipAfter was finished", zap.String("txid", tx.Id), @@ -162,7 +162,7 @@ func (rg *ruleGroup) Eval(phase types.RulePhase, tx *Transaction) bool { } if tx.Skip > 0 { tx.Skip-- - //Skipping rule + // Skipping rule continue } // we reset captures, matched_vars, matched_vars_names, etc diff --git a/seclang/directives.go b/seclang/directives.go index 30c668899..a7ced9da6 100644 --- a/seclang/directives.go +++ b/seclang/directives.go @@ -246,7 +246,7 @@ func directiveSecHashEngine(p *Parser, opts string) error { } func directiveSecDefaultAction(p *Parser, opts string) error { - return p.AddDefaultActions(opts) + return p.addDefaultActions(opts) } func directiveSecContentInjection(p *Parser, opts string) error { @@ -273,7 +273,7 @@ func directiveSecConnEngine(p *Parser, opts string) error { } func directiveSecCollectionTimeout(p *Parser, opts string) error { - //p.waf.CollectionTimeout, _ = strconv.Atoi(opts) + // p.waf.CollectionTimeout, _ = strconv.Atoi(opts) return nil } @@ -302,7 +302,7 @@ func directiveSecAuditEngine(p *Parser, opts string) error { } func directiveSecDataDir(p *Parser, opts string) error { - //TODO validations + // TODO validations p.Waf.DataDir = opts return nil } diff --git a/seclang/directives_test.go b/seclang/directives_test.go index 37045a894..89d62f56e 100644 --- a/seclang/directives_test.go +++ b/seclang/directives_test.go @@ -60,7 +60,6 @@ func Test_directiveSecAuditLog(t *testing.T) { if w.TmpDir != "/tmp" { t.Error("failed to set SecTmpDir") } - //"SecServerSignature": directiveSecServerSignature, if err := p.FromString("SecSensorId test"); err != nil { t.Error("failed to set parser from string") } @@ -139,6 +138,6 @@ func TestDebugDirectives(t *testing.T) { p.Waf.Logger.Debug("efgh123") data, _ = utils.OpenFile(tmp, "") if !strings.Contains(string(data), "efgh123") { - t.Error("debug data wasn't writen") + t.Error("debug data wasn't written") } } diff --git a/seclang/parser.go b/seclang/parser.go index a1c4c7037..c0047505d 100644 --- a/seclang/parser.go +++ b/seclang/parser.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http:// www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -63,7 +63,7 @@ func (p *Parser) FromFile(profilePath string) error { ) return err } - //TODO validar el error de scanner.Err() + // TODO validar el error de scanner.Err() return nil } @@ -78,7 +78,7 @@ func (p *Parser) FromString(data string) error { p.currentLine++ line := scanner.Text() linebuffer += strings.TrimSpace(line) - //Check if line ends with \ + // Check if line ends with \ match := pattern.MatchString(line) if !match { err := p.evaluate(linebuffer) @@ -97,7 +97,7 @@ func (p *Parser) evaluate(data string) error { if data == "" || data[0] == '#' { return nil } - //first we get the directive + // first we get the directive spl := strings.SplitN(data, " ", 2) opts := "" if len(spl) == 2 { @@ -168,7 +168,7 @@ func (p *Parser) evaluate(data string) error { "secdebuglog": directiveSecDebugLog, "secdebugloglevel": directiveSecDebugLogLevel, - //Unsupported Directives + // Unsupported Directives "secargumentseparator": directiveUnsupported, "seccookieformat": directiveUnsupported, "secruleupdatetargetbytag": directiveUnsupported, @@ -203,7 +203,7 @@ func (p *Parser) ParseRule(data string, withOperator bool) (*engine.Rule, error) spl := strings.SplitN(data, " ", 2) vars := spl[0] - //regex: "(?:[^"\\]|\\.)*" + // regex: "(?:[^"\\]|\\.)*" r := regexp.MustCompile(`"(?:[^"\\]|\\.)*"`) matches := r.FindAllString(data, -1) operator := utils.RemoveQuotes(matches[0]) @@ -226,7 +226,7 @@ func (p *Parser) ParseRule(data string, withOperator bool) (*engine.Rule, error) } } } else { - //quoted actions separated by comma (,) + // quoted actions separated by comma (,) actions = utils.RemoveQuotes(data) err = rp.ParseActions(actions) if err != nil { @@ -259,10 +259,10 @@ func (p *Parser) ParseRule(data string, withOperator bool) (*engine.Rule, error) return rule, nil } -// AddDEfaultActions compiles an actions string +// addDefaultActions compiles an actions string // Requires a phase and a disruptive action, example: -// AddDefaultActions("deny,phase:1,log") -func (p *Parser) AddDefaultActions(data string) error { +// addDefaultActions("deny,phase:1,log") +func (p *Parser) addDefaultActions(data string) error { p.defaultActions = append(p.defaultActions, data) return nil } diff --git a/seclang/rule_parser.go b/seclang/rule_parser.go index 49fa5b8a6..0a7174c1b 100644 --- a/seclang/rule_parser.go +++ b/seclang/rule_parser.go @@ -58,9 +58,9 @@ func (p *ruleParser) ParseVariables(vars string) error { for i := 0; i < len(vars); i++ { c := vars[i] if (c == '|' && curr != 2) || i+1 >= len(vars) || (curr == 2 && c == '/' && !isescaped) { - //if next variable or end - //if regex we ignore | - //we wont support pipe for xpath, maybe later + // if next variable or end + // if regex we ignore | + // we wont support pipe for xpath, maybe later if c != '|' { // we don't want to miss the last character if curr == 0 { @@ -74,12 +74,12 @@ func (p *ruleParser) ParseVariables(vars string) error { if err != nil { return err } - //fmt.Printf("(PREVIOUS %s) %s:%s (%t %t)\n", vars, curvar, curkey, iscount, isnegation) + // fmt.Printf("(PREVIOUS %s) %s:%s (%t %t)\n", vars, curvar, curkey, iscount, isnegation) if isquoted { // if it is quoted we remove the last quote if len(vars) <= i+1 || vars[i+1] != '\'' { if vars[i] != '\'' { - //TODO fix here + // TODO fix here return fmt.Errorf("unclosed quote: " + string(curkey)) } } @@ -104,49 +104,50 @@ func (p *ruleParser) ParseVariables(vars string) error { } switch curr { case 0: - if c == '!' { + switch c { + case '!': isnegation = true - } else if c == '&' { + case '&': iscount = true - } else if c == ':' { - // we skip to key context + case ':': curr = 1 - } else { - // we append the current character + default: curvar = append(curvar, c) } case 1: - if len(curkey) == 0 && (string(curvar) == "XML" || string(curvar) == "JSON") { + switch { + case len(curkey) == 0 && (string(curvar) == "XML" || string(curvar) == "JSON"): // We are starting a XPATH curr = 3 curkey = append(curkey, c) - } else if c == '/' { + case c == '/': // We are starting a regex curr = 2 - } else if c == '\'' { + case c == '\'': // we start a quoted regex // we go back to the loop to find / isquoted = true - } else { + default: curkey = append(curkey, c) } case 2: - //REGEX - if c == '/' && !isescaped { + // REGEX + switch { + case c == '/' && !isescaped: // unescaped / will stop the regex curr = 1 - } else if c == '\\' { + case c == '\\': curkey = append(curkey, '\\') if isescaped { isescaped = false } else { isescaped = true } - } else { + default: curkey = append(curkey, c) } case 3: - //XPATH + // XPATH curkey = append(curkey, c) } } @@ -155,7 +156,7 @@ func (p *ruleParser) ParseVariables(vars string) error { func (p *ruleParser) ParseOperator(operator string) error { if len(operator) == 0 || operator[0] != '@' && operator[0] != '!' { - //default operator RX + // default operator RX operator = "@rx " + operator } spl := strings.SplitN(operator, " ", 2) @@ -201,7 +202,7 @@ func (p *ruleParser) ParseOperator(operator string) error { } func (p *ruleParser) ParseDefaultActions(actions string) error { - act, err := ParseActions(actions) + act, err := parseActions(actions) if err != nil { return err } @@ -231,17 +232,17 @@ func (p *ruleParser) ParseDefaultActions(actions string) error { // ParseActions func (p *ruleParser) ParseActions(actions string) error { - act, err := ParseActions(actions) + act, err := parseActions(actions) if err != nil { return err } - //check if forbidden action: + // check if forbidden action: for _, a := range act { if utils.StringInSlice(a.Key, p.parser.DisabledRuleActions) { return fmt.Errorf("%s rule action is disabled", a.Key) } } - //first we execute metadata rules + // first we execute metadata rules for _, a := range act { if a.Atype == types.ActionTypeMetadata { errs := a.F.Init(p.rule, a.Value) @@ -286,10 +287,10 @@ func newRuleParser(p *Parser) *ruleParser { return rp } -// ParseActions will assign the function name, arguments and +// parseActions will assign the function name, arguments and // function (pkg.actions) for each action splitted by comma (,) // Action arguments are allowed to wrap values between collons('') -func ParseActions(actions string) ([]ruleAction, error) { +func parseActions(actions string) ([]ruleAction, error) { iskey := true ckey := "" cval := "" @@ -297,7 +298,7 @@ func ParseActions(actions string) ([]ruleAction, error) { res := []ruleAction{} for i, c := range actions { if iskey && c == ' ' { - //skip whitespaces in key + // skip whitespaces in key continue } else if !quoted && c == ',' { f, err := actionsmod.GetAction(ckey) @@ -324,7 +325,7 @@ func ParseActions(actions string) ([]ruleAction, error) { } } else if !iskey { if c == ' ' && !quoted { - //skip unquoted whitespaces + // skip unquoted whitespaces continue } cval += string(c) @@ -363,7 +364,7 @@ In the future I shall optimize that redundant log and nolog, it won't actually c */ func mergeActions(origin []ruleAction, defaults []ruleAction) []ruleAction { res := []ruleAction{} - var da ruleAction //Disruptive action + var da ruleAction // Disruptive action for _, action := range defaults { if action.Atype == types.ActionTypeDisruptive { da = action diff --git a/seclang/rule_parser_test.go b/seclang/rule_parser_test.go index 537ac0199..c5dae3a8a 100644 --- a/seclang/rule_parser_test.go +++ b/seclang/rule_parser_test.go @@ -24,10 +24,10 @@ import ( func TestDefaultActions(t *testing.T) { waf := coraza.NewWaf() p, _ := NewParser(waf) - if err := p.AddDefaultActions("log, pass, phase: 1"); err != nil { + if err := p.addDefaultActions("log, pass, phase: 1"); err != nil { t.Error("Error parsing default actions", err) } - if err := p.AddDefaultActions("log, drop, phase:2"); err != nil { + if err := p.addDefaultActions("log, drop, phase:2"); err != nil { t.Error("Could not add default actions") } if len(p.GetDefaultActions()) != 2 { @@ -45,7 +45,7 @@ func TestMergeActions(t *testing.T) { func TestVariables(t *testing.T) { waf := coraza.NewWaf() p, _ := NewParser(waf) - //single variable with key + // single variable with key err := p.FromString(`SecRule REQUEST_HEADERS:test "" "id:1"`) if err != nil { t.Error(err) diff --git a/seclang/rules_test.go b/seclang/rules_test.go index 176cfdebd..015ceb27b 100644 --- a/seclang/rules_test.go +++ b/seclang/rules_test.go @@ -129,14 +129,16 @@ func TestSecAuditLogs(t *testing.T) { tx := waf.NewTransaction() tx.ProcessUri("/test.php?id=1", "get", "http/1.1") tx.ProcessRequestHeaders() - tx.ProcessRequestBody() + if _, err := tx.ProcessRequestBody(); err != nil { + t.Error(err) + } tx.ProcessLogging() if len(tx.MatchedRules) == 0 { t.Error("failed to match rules") } - if tx.AuditLog().Messages[0].Data.Id != 4482 { + if tx.AuditLog().Messages[0].Data.ID != 4482 { t.Error("failed to match rule id") } } diff --git a/testdata/engine/rulemetadata.yaml b/testdata/engine/rulemetadata.yaml index ac25914ea..1c856d43f 100644 --- a/testdata/engine/rulemetadata.yaml +++ b/testdata/engine/rulemetadata.yaml @@ -1,19 +1,19 @@ --- - meta: - author: jptosso - description: Test if the rule metadata - enabled: true - name: rulemetadata.yaml - tests: +meta: + author: jptosso + description: Test if the rule metadata + enabled: true + name: rulemetadata.yaml +tests: +- + test_title: rulemetadata + stages: - - test_title: rulemetadata - stages: - - - stage: - output: - triggered_rules: [1, 2] - non_triggered_rules: - rules: | - SecAction "id:1, log, severity:5" - SecRule HIGHEST_SEVERITY "@eq 5" "id:2, log" + stage: + output: + triggered_rules: [1, 2] + non_triggered_rules: +rules: | + SecAction "id:1, log, severity:5" + SecRule HIGHEST_SEVERITY "@eq 5" "id:2, log" diff --git a/testing/engine.go b/testing/engine.go index 577d135f3..3435b9393 100644 --- a/testing/engine.go +++ b/testing/engine.go @@ -41,7 +41,7 @@ func (stage *ProfileTestStage) Start(waf *engine.Waf) error { return errors.New("failed to parse Raw Request") } } - //Apply tx data + // Apply tx data for k, v := range stage.Stage.Input.Headers { tx.AddRequestHeader(k, v) } @@ -50,14 +50,14 @@ func (stage *ProfileTestStage) Start(waf *engine.Waf) error { method = stage.Stage.Input.Method } - //Request Line + // Request Line httpv := "HTTP/1.1" if stage.Stage.Input.Version != "" { httpv = stage.Stage.Input.Version } - if stage.Stage.Input.Uri != "" { - tx.ProcessUri(stage.Stage.Input.Uri, method, httpv) + if stage.Stage.Input.URI != "" { + tx.ProcessUri(stage.Stage.Input.URI, method, httpv) } // POST DATA @@ -69,7 +69,7 @@ func (stage *ProfileTestStage) Start(waf *engine.Waf) error { _, _ = tx.RequestBodyBuffer.Write([]byte(idata)) // we ignore the error } - //We can skip processConnection + // We can skip processConnection tx.ProcessRequestHeaders() _, _ = tx.ProcessRequestBody() tx.ProcessResponseHeaders(200, "HTTP/1.1") @@ -83,7 +83,7 @@ func (stage *ProfileTestStage) Start(waf *engine.Waf) error { log += fmt.Sprintf(" [id \"%d\"]", mr.Rule.Id) tr = append(tr, mr.Rule.Id) } - //now we evaluate tests + // now we evaluate tests if stage.Stage.Output.LogContains != "" { if !strings.Contains(log, stage.Stage.Output.LogContains) { return fmt.Errorf("log does not contain %s", stage.Stage.Output.LogContains) diff --git a/testing/engine_test.go b/testing/engine_test.go index c8a775130..3a5a5280b 100644 --- a/testing/engine_test.go +++ b/testing/engine_test.go @@ -34,7 +34,9 @@ func TestEngine(t *testing.T) { t.Error("failed to find test files") } waf := engine.NewWaf() - waf.SetLogLevel(5) + if err := waf.SetLogLevel(5); err != nil { + t.Error(err) + } for _, f := range files { profile, err := NewProfile(f) if err != nil { diff --git a/testing/plugins_test.go b/testing/plugins_test.go deleted file mode 100644 index 571f52e9e..000000000 --- a/testing/plugins_test.go +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright 2021 Juan Pablo Tosso -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package testing - -import ( - "strconv" - - "github.com/jptosso/coraza-waf/v2" -) - -/* -func init() { - actions.RegisterRuleAction("id15", func() coraza.RuleAction { - return &id15{} - }) -} - -// Test transformation, string to lowercase - -func transformationToLowercase(input string, _ coraza.RuleTransformationTools) string { - return strings.ToLower(input) -}*/ - -// Test action, set ID to 15 -/* -type id15 struct{} - -func (id15) Init(rule *coraza.Rule, _ string) error { - rule.Id = 15 - return nil -} - -func (id15) Evaluate(_ *coraza.Rule, _ *coraza.Transaction) {} - -func (id15) Type() coraza.RuleActionType { - return coraza.ActionTypeData -}*/ - -// Test operator, match if number is even - -type opEven struct{} - -func (opEven) Init(_ string) error { - return nil -} - -func (opEven) Evaluate(_ *coraza.Transaction, input string) bool { - i, _ := strconv.Atoi(input) - return i%2 == 0 -} - -// Tripwires -/* -var _ coraza.RuleTransformation = transformationToLowercase -var _ coraza.RuleAction = &id15{} -var _ coraza.RuleOperator = &opEven{} - -func TestPlugins(t *testing.T) { - waf := coraza.NewWaf() - parser, _ := seclang.NewParser(waf) - if err := parser.FromString("SecRule ARGS \"@even\" \"id15,log\""); err != nil { - t.Error(err) - } - - if waf.Rules.Count() == 0 { - t.Error("failed to create rules") - return - } - - if waf.Rules.GetRules()[0].Id != 15 { - t.Error("failed to set rule id to 15") - } - - if !waf.Rules.GetRules()[0].Operator.Operator.Evaluate(nil, "2") { - t.Error("failed to match operator even") - } - - if waf.Rules.GetRules()[0].Operator.Operator.Evaluate(nil, "1") { - t.Error("failed to match operator even") - } -} -*/ diff --git a/testing/profile.go b/testing/profile.go index 868568332..ee89b4d9a 100644 --- a/testing/profile.go +++ b/testing/profile.go @@ -55,9 +55,9 @@ type ProfileTestStageInnerInput struct { DestAddr string `yaml:"dest_addr"` Port int `yaml:"port"` Method string `yaml:"method"` - Uri string `yaml:"uri"` + URI string `yaml:"uri"` Version string `yaml:"version"` - Data interface{} `yaml:"data"` //Accepts array or string + Data interface{} `yaml:"data"` // Accepts array or string Headers map[string]string `yaml:"headers"` RawRequest string `yaml:"raw_request"` EncodedRequest string `yaml:"encoded_request"` @@ -73,7 +73,7 @@ type ProfileTestStageInnerOutput struct { Status interface{} `yaml:"status"` } -//NewProfile creates a new profile from a file +// NewProfile creates a new profile from a file func NewProfile(path string) (*Profile, error) { f, err := os.ReadFile(path) if err != nil { diff --git a/transaction.go b/transaction.go index 537c27449..085b63d8a 100644 --- a/transaction.go +++ b/transaction.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http:// www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -39,19 +39,19 @@ type Transaction struct { // If true the transaction is going to be logged, it won't log if IsRelevantStatus() fails Log bool - //Transaction Id + // Transaction Id Id string // Contains the list of matched rules and associated match information MatchedRules []MatchedRule - //True if the transaction has been disrupted by any rule + // True if the transaction has been disrupted by any rule Interruption *types.Interruption // Contains all collections, including persistent collections []*Collection - //Response data to be sent + // Response data to be sent Status int // This is used to store log messages @@ -134,7 +134,7 @@ func (tx *Transaction) MacroExpansion(data string) string { matchspl := strings.SplitN(match, ".", 2) col, err := variables.ParseVariable(matchspl[0]) if err != nil { - //Invalid collection + // Invalid collection continue } key := "" @@ -200,7 +200,7 @@ func (tx *Transaction) AddResponseHeader(key string, value string) { tx.GetCollection(variables.ResponseHeadersNames).AddUnique("", key) tx.GetCollection(variables.ResponseHeaders).Add(key, value) - //Most headers can be managed like that + // Most headers can be managed like that if key == "content-type" { spl := strings.SplitN(value, ";", 2) tx.GetCollection(variables.ResponseContentType).Set("", []string{spl[0]}) @@ -211,13 +211,13 @@ func (tx *Transaction) AddResponseHeader(key string, value string) { // that supports capture, like @rx func (tx *Transaction) CaptureField(index int, value string) { i := strconv.Itoa(index) - tx.GetCollection(variables.Tx).Set(i, []string{value}) + tx.GetCollection(variables.TX).Set(i, []string{value}) } // this function is used to control which variables are reset after a new rule is evaluated func (tx *Transaction) resetAfterRule() { - //We reset capture 0-9 - ctx := tx.GetCollection(variables.Tx) + // We reset capture 0-9 + ctx := tx.GetCollection(variables.TX) for i := 0; i < 10; i++ { si := strconv.Itoa(i) ctx.Set(si, []string{""}) @@ -298,7 +298,7 @@ func (tx *Transaction) MatchVariable(match MatchData) { matchedVars.Add("", match.Value) matchedVar.Set("", []string{match.Value}) - //fmt.Printf("%s: %s\n", match.VariableName, match.Value) + // fmt.Printf("%s: %s\n", match.VariableName, match.Value) matchedVarsNames.Add("", varname) matchedVarName.Set("", []string{varname}) @@ -309,7 +309,7 @@ func (tx *Transaction) MatchRule(mr MatchedRule) { tx.Waf.Logger.Debug("rule matched", zap.String("txid", tx.Id), zap.Int("rule", mr.Rule.Id)) /* if mr.Rule.Log && tx.Waf.ErrorLogger != nil { - //TODO log based on severity + // TODO log based on severity }*/ tx.MatchedRules = append(tx.MatchedRules, mr) @@ -396,9 +396,9 @@ func (tx *Transaction) GetCollection(variable variables.RuleVariable) *Collectio // savePersistentData save persistent collections to persistence engine func (tx *Transaction) savePersistentData() { - //TODO, disabled by now, maybe we should add persistent variables to the + // TODO, disabled by now, maybe we should add persistent variables to the // collection struct, something like col.Persist("key") - //pers := []byte{VARIABLE_SESSION, VARIABLE_IP} + // pers := []byte{VARIABLE_SESSION, VARIABLE_IP} /* pers := []byte{} for _, v := range pers { @@ -407,7 +407,7 @@ func (tx *Transaction) savePersistentData() { continue } data := col.Data() - //key := col.PersistenceKey + // key := col.PersistenceKey upc, _ := strconv.Atoi(data["UPDATE_COUNTER"][0]) upc++ ct, _ := strconv.ParseInt(data["CREATE_TIME"][0], 10, 64) @@ -424,7 +424,7 @@ func (tx *Transaction) savePersistentData() { // New version may have multiple collection types allowing us to identify this cases data["TIMEOUT"] = []string{timeout} data["LAST_UPDATE_TIME"] = []string{tss} - //tx.Waf.Persistence.Save(v, key, data) + // tx.Waf.Persistence.Save(v, key, data) } */ } @@ -459,7 +459,7 @@ func (tx *Transaction) RemoveRuleById(id int) { func (tx *Transaction) ProcessRequest(req *http.Request) (*types.Interruption, error) { var client string cport := 0 - //IMPORTANT: Some http.Request.RemoteAddr implementations will not contain port or contain IPV6: [2001:db8::1]:8080 + // IMPORTANT: Some http.Request.RemoteAddr implementations will not contain port or contain IPV6: [2001:db8::1]:8080 spl := strings.Split(req.RemoteAddr, ":") if len(spl) > 1 { client = strings.Join(spl[0:len(spl)-1], "") @@ -566,14 +566,14 @@ func (tx *Transaction) AddArgument(orig string, key string, value string) { func (tx *Transaction) ProcessUri(uri string, method string, httpVersion string) { tx.GetCollection(variables.RequestMethod).Set("", []string{method}) tx.GetCollection(variables.RequestProtocol).Set("", []string{httpVersion}) - tx.GetCollection(variables.RequestUriRaw).Set("", []string{uri}) + tx.GetCollection(variables.RequestURIRaw).Set("", []string{uri}) - //TODO modsecurity uses HTTP/${VERSION} instead of just version, let's check it out + // TODO modsecurity uses HTTP/${VERSION} instead of just version, let's check it out tx.GetCollection(variables.RequestLine).Set("", []string{fmt.Sprintf("%s %s %s", method, uri, httpVersion)}) var err error - //we remove anchors + // we remove anchors if in := strings.Index(uri, "#"); in != -1 { uri = uri[:in] } @@ -583,7 +583,7 @@ func (tx *Transaction) ProcessUri(uri string, method string, httpVersion string) if err != nil { tx.GetCollection(variables.UrlencodedError).Set("", []string{err.Error()}) path = uri - tx.GetCollection(variables.RequestUri).Set("", []string{uri}) + tx.GetCollection(variables.RequestURI).Set("", []string{uri}) /* tx.GetCollection(VARIABLE_URI_PARSE_ERROR).Set("", []string{"1"}) posRawQuery := strings.Index(uri, "?") @@ -598,7 +598,7 @@ func (tx *Transaction) ProcessUri(uri string, method string, httpVersion string) */ } else { tx.ExtractArguments("GET", parsedUrl.RawQuery) - tx.GetCollection(variables.RequestUri).Set("", []string{parsedUrl.String()}) + tx.GetCollection(variables.RequestURI).Set("", []string{parsedUrl.String()}) path = parsedUrl.Path query = parsedUrl.RawQuery } @@ -678,7 +678,7 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) { zap.String("bodyprocessor", rbp)) rbp = strings.ToLower(rbp) if rbp == "" { - //so there is no bodyprocessor, we don't want to generate an error + // so there is no bodyprocessor, we don't want to generate an error tx.Waf.Rules.Eval(types.PhaseRequestBody, tx) return tx.Interruption, nil } @@ -741,7 +741,7 @@ func (tx *Transaction) ProcessResponseHeaders(code int, proto string) *types.Int // This is used by webservers to choose whether tostream response buffers // directly to the client or write them to Coraza func (tx *Transaction) IsProcessableResponseBody() bool { - //TODO add more validations + // TODO add more validations ct := tx.GetCollection(variables.ResponseContentType).GetFirstString("") return utils.StringInSlice(ct, tx.Waf.ResponseBodyMimeTypes) } @@ -778,9 +778,9 @@ func (tx *Transaction) ProcessResponseBody() (*types.Interruption, error) { // delivered prior to the execution of this method. func (tx *Transaction) ProcessLogging() { // I'm not sure why but modsecurity won't log if RuleEngine is disabled - //if tx.RuleEngine == RULE_ENGINE_OFF { - // return - //} + // if tx.RuleEngine == RULE_ENGINE_OFF { + // return + // } defer func() { tx.savePersistentData() tx.RequestBodyBuffer.Close() @@ -802,7 +802,7 @@ func (tx *Transaction) ProcessLogging() { re := tx.Waf.AuditLogRelevantStatus status := tx.GetCollection(variables.ResponseStatus).GetFirstString("") if re != nil && !re.Match([]byte(status)) { - //Not relevant status + // Not relevant status tx.Waf.Logger.Debug("Transaction status not marked for audit logging", zap.String("tx", tx.Id), ) @@ -828,27 +828,27 @@ func (tx *Transaction) AuditLog() loggers.AuditLog { al := loggers.AuditLog{} parts := tx.AuditLogParts al.Messages = []loggers.AuditMessage{} - //YYYY/MM/DD HH:mm:ss + // YYYY/MM/DD HH:mm:ss ts := time.Unix(0, tx.Timestamp).Format("2006/01/02 15:04:05") al.Transaction = loggers.AuditTransaction{ Timestamp: ts, UnixTimestamp: tx.Timestamp, - Id: tx.Id, - ClientIp: tx.GetCollection(variables.RemoteAddr).GetFirstString(""), + ID: tx.Id, + ClientIP: tx.GetCollection(variables.RemoteAddr).GetFirstString(""), ClientPort: tx.GetCollection(variables.RemotePort).GetFirstInt(""), - HostIp: tx.GetCollection(variables.ServerAddr).GetFirstString(""), + HostIP: tx.GetCollection(variables.ServerAddr).GetFirstString(""), HostPort: tx.GetCollection(variables.ServerPort).GetFirstInt(""), - ServerId: tx.GetCollection(variables.ServerName).GetFirstString(""), //TODO check + ServerID: tx.GetCollection(variables.ServerName).GetFirstString(""), // TODO check Request: loggers.AuditTransactionRequest{ Method: tx.GetCollection(variables.RequestMethod).GetFirstString(""), Protocol: tx.GetCollection(variables.RequestProtocol).GetFirstString(""), - Uri: tx.GetCollection(variables.RequestUri).GetFirstString(""), - HttpVersion: tx.GetCollection(variables.RequestProtocol).GetFirstString(""), - //Body and headers are audit variables.RequestUriRaws + URI: tx.GetCollection(variables.RequestURI).GetFirstString(""), + HTTPVersion: tx.GetCollection(variables.RequestProtocol).GetFirstString(""), + // Body and headers are audit variables.RequestUriRaws }, Response: loggers.AuditTransactionResponse{ Status: tx.GetCollection(variables.ResponseStatus).GetFirstInt(""), - //body and headers are audit parts + // body and headers are audit parts }, } rengine := tx.RuleEngine.String() @@ -859,15 +859,15 @@ func (tx *Transaction) AuditLog() loggers.AuditLog { al.Transaction.Request.Headers = tx.GetCollection(variables.RequestHeaders).Data() case 'C': al.Transaction.Request.Body = tx.GetCollection(variables.RequestBody).GetFirstString("") - //TODO maybe change to: - //al.Transaction.Request.Body = tx.RequestBodyBuffer.String() + // TODO maybe change to: + // al.Transaction.Request.Body = tx.RequestBodyBuffer.String() case 'F': al.Transaction.Response.Headers = tx.GetCollection(variables.ResponseHeaders).Data() case 'G': al.Transaction.Response.Body = tx.GetCollection(variables.ResponseBody).GetFirstString("") case 'H': al.Transaction.Producer = loggers.AuditTransactionProducer{ - Connector: "unknown", //TODO maybe add connector variable to Waf + Connector: "unknown", // TODO maybe add connector variable to Waf Version: "unknown", Server: "", RuleEngine: rengine, @@ -884,11 +884,11 @@ func (tx *Transaction) AuditLog() loggers.AuditLog { * if you don’t want to have (often large) files stored in your audit logs. */ case 'J': - //upload data + // upload data files := []loggers.AuditTransactionRequestFiles{} al.Transaction.Request.Files = []loggers.AuditTransactionRequestFiles{} for i, name := range tx.GetCollection(variables.Files).Get("") { - //TODO we kind of assume there is a file_size for each file with the same index + // TODO we kind of assume there is a file_size for each file with the same index size, _ := strconv.ParseInt(tx.GetCollection(variables.FilesSizes).Get("")[i], 10, 64) ext := filepath.Ext(name) at := loggers.AuditTransactionRequestFiles{ @@ -909,7 +909,7 @@ func (tx *Transaction) AuditLog() loggers.AuditLog { Data: loggers.AuditMessageData{ File: mr.Rule.File, Line: mr.Rule.Line, - Id: r.Id, + ID: r.Id, Rev: r.Rev, Msg: mr.Message, Data: mr.Data, diff --git a/transaction_test.go b/transaction_test.go index 24176000d..9ef9b60cb 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -123,12 +123,8 @@ func TestTxResponse(t *testing.T) { */ } -func TestTxSetters2(t *testing.T) { - //TODO must be rebuilt -} - func TestTxGetField(t *testing.T) { - //GetField + // GetField } func TestTxMatch(t *testing.T) { @@ -152,7 +148,7 @@ func TestTxMatch(t *testing.T) { func TestRequestBody(t *testing.T) { urlencoded := "some=result&second=data" - //xml := "test" + // xml := "test" tx := wafi.NewTransaction() tx.AddRequestHeader("content-type", "application/x-www-form-urlencoded") if _, err := tx.RequestBodyBuffer.Write([]byte(urlencoded)); err != nil { @@ -180,10 +176,10 @@ func TestAuditLog(t *testing.T) { tx := makeTransaction() tx.AuditLogParts = []rune("ABCDEFGHIJK") al := tx.AuditLog() - if al.Transaction.Id != tx.Id { + if al.Transaction.ID != tx.Id { t.Error("invalid auditlog id") } - //TODO more checks + // TODO more checks } func TestResponseBody(t *testing.T) { @@ -215,14 +211,14 @@ func TestAuditLogFields(t *testing.T) { Id: tx.Id, MatchedData: MatchData{ VariableName: "UNIQUE_ID", - Variable: variables.UniqueId, + Variable: variables.UniqueID, }, }) if len(tx.MatchedRules) == 0 || tx.MatchedRules[0].Rule.Id != rule.Id { t.Error("failed to match rule for audit") } al := tx.AuditLog() - if len(al.Messages) == 0 || al.Messages[0].Data.Id != rule.Id { + if len(al.Messages) == 0 || al.Messages[0].Data.ID != rule.Id { t.Error("failed to add rules to audit logs") } if al.Transaction.Request.Headers == nil || al.Transaction.Request.Headers["test"][0] != "test" { @@ -237,7 +233,9 @@ func TestRequestStruct(t *testing.T) { req, _ := http.NewRequest("POST", "https://www.coraza.io/test", strings.NewReader("test=456")) waf := NewWaf() tx := waf.NewTransaction() - tx.ProcessRequest(req) + if _, err := tx.ProcessRequest(req); err != nil { + t.Error(err) + } if tx.GetCollection(variables.RequestMethod).GetFirstString("") != "POST" { t.Error("failed to set request from request object") } @@ -246,11 +244,11 @@ func TestRequestStruct(t *testing.T) { func TestResetCapture(t *testing.T) { tx := makeTransaction() tx.CaptureField(5, "test") - if tx.GetCollection(variables.Tx).GetFirstString("5") != "test" { + if tx.GetCollection(variables.TX).GetFirstString("5") != "test" { t.Error("failed to set capture field from tx") } tx.resetAfterRule() - if tx.GetCollection(variables.Tx).GetFirstString("5") != "" { + if tx.GetCollection(variables.TX).GetFirstString("5") != "" { t.Error("failed to reset capture field from tx") } } @@ -261,9 +259,9 @@ func TestRelevantAuditLogging(t *testing.T) { tx.GetCollection(variables.ResponseStatus).Set("", []string{"403"}) tx.AuditEngine = types.AuditEngineRelevantOnly tx.Log = false - //tx.Waf.auditLogger = loggers.NewAuditLogger() + // tx.Waf.auditLogger = loggers.NewAuditLogger() tx.ProcessLogging() - //TODO how do we check if the log was writen? + // TODO how do we check if the log was writen? } func TestLogCallback(t *testing.T) { @@ -277,7 +275,7 @@ func TestLogCallback(t *testing.T) { Rule: *NewRule(), MatchedData: MatchData{ VariableName: "UNIQUE_ID", - Variable: variables.UniqueId, + Variable: variables.UniqueID, }, }) if buffer == "" && strings.Contains(buffer, tx.Id) { diff --git a/transformations/base64decode.go b/transformations/base64decode.go index 6799c6bef..6f761809d 100644 --- a/transformations/base64decode.go +++ b/transformations/base64decode.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http:// www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -78,18 +78,18 @@ func doBase64decode(input string) string { if src[i] == '=' { j++ if j > 2 { - //ERROR + // ERROR return input } } if src[i] > 127 || base64_dec_map[src[i]] == 127 { - //ERROR + // ERROR return input } if base64_dec_map[src[i]] < 64 && j != 0 { - //ERROR + // ERROR return input } n++ @@ -97,6 +97,10 @@ func doBase64decode(input string) string { n = ((n * 6) + 7) >> 3 n -= j + if len(dst) == 0 || slen < n { + // ERROR + return input + } j = 3 n = 0 diff --git a/transformations/cmd_line.go b/transformations/cmd_line.go index 9bb26a123..0d3b34344 100644 --- a/transformations/cmd_line.go +++ b/transformations/cmd_line.go @@ -37,15 +37,16 @@ func cmdLine(data string, utils coraza.RuleTransformationTools) string { space := false ret := []byte{} for _, a := range data { - if a == '"' || a == '\'' || a == '\\' || a == '^' { + switch { + case a == '"' || a == '\'' || a == '\\' || a == '^': /* remove some characters */ - } else if a == ' ' || a == ',' || a == ';' || a == '\t' || a == '\r' || a == '\n' { + case a == ' ' || a == ',' || a == ';' || a == '\t' || a == '\r' || a == '\n': /* replace some characters to space (only one) */ if !space { ret = append(ret, ' ') space = true } - } else if a == '/' || a == '(' { + case a == '/' || a == '(': /* remove space before / or ( */ if space { ret = ret[:len(ret)-1] @@ -53,7 +54,7 @@ func cmdLine(data string, utils coraza.RuleTransformationTools) string { space = false ret = append(ret, byte(a)) - } else { + default: b := unicode.ToLower(a) ret = append(ret, byte(b)) space = false diff --git a/transformations/css_decode.go b/transformations/css_decode.go index 3763cab7a..903cb2307 100644 --- a/transformations/css_decode.go +++ b/transformations/css_decode.go @@ -24,21 +24,21 @@ func cssDecode(data string, utils coraza.RuleTransformationTools) string { } func cssDecodeInplace(input string) string { - //TODO the following shall be int64? + // TODO the following shall be int64? var c, i, j, count int d := []byte(input) - input_len := len(d) + inputLen := len(d) - for i < input_len { + for i < inputLen { /* Is the character a backslash? */ if input[i] == '\\' { /* Is there at least one more byte? */ - if i+1 < input_len { + if i+1 < inputLen { i++ /* We are not going to need the backslash. */ /* Check for 1-6 hex characters following the backslash */ j = 0 - for (j < 6) && (i+j < input_len) && (utils.ValidHex(input[i+j])) { + for (j < 6) && (i+j < inputLen) && (utils.ValidHex(input[i+j])) { j++ } @@ -106,7 +106,7 @@ func cssDecodeInplace(input string) string { } /* We must ignore a single whitespace after a hex escape */ - if (i+j < input_len) && isspace(input[i+j]) { + if (i+j < inputLen) && isspace(input[i+j]) { j++ } diff --git a/transformations/escape_seq_decode.go b/transformations/escape_seq_decode.go index e06ff924c..a85ff1b78 100644 --- a/transformations/escape_seq_decode.go +++ b/transformations/escape_seq_decode.go @@ -23,11 +23,11 @@ import ( func escapeSeqDecode(input string, tools coraza.RuleTransformationTools) string { var i, count, d int - input_len := len(input) + inputLen := len(input) data := []byte(input) - for i < input_len { - if (input[i] == '\\') && (i+1 < input_len) { + for i < inputLen { + if (input[i] == '\\') && (i+1 < inputLen) { c := -1 switch input[i+1] { @@ -63,7 +63,7 @@ func escapeSeqDecode(input string, tools coraza.RuleTransformationTools) string if c == -1 { if (input[i+1] == 'x') || (input[i+1] == 'X') { /* Hexadecimal. */ - if (i+3 < input_len) && (utils.IsXDigit(int(input[i+2]))) && (utils.IsXDigit(int(input[i+3]))) { + if (i+3 < inputLen) && (utils.IsXDigit(int(input[i+2]))) && (utils.IsXDigit(int(input[i+3]))) { /* Two digits. */ c = int(utils.X2c(input[i+2:])) i += 4 @@ -75,15 +75,15 @@ func escapeSeqDecode(input string, tools coraza.RuleTransformationTools) string buf := make([]byte, 4) j := 0 - for (i+1+j < input_len) && (j < 3) { + for (i+1+j < inputLen) && (j < 3) { buf[j] = input[i+1+j] j++ if (len(input) > (i + 1 + j)) && !utils.IsODigit(input[i+1+j]) { break } } - //buf[j] = '\x00' - //This line actually means that the string ends here so: + // buf[j] = '\x00' + // This line actually means that the string ends here so: buf = buf[:j] if j > 0 { diff --git a/transformations/js_decode.go b/transformations/js_decode.go index e89816976..fc33cf9e1 100644 --- a/transformations/js_decode.go +++ b/transformations/js_decode.go @@ -28,14 +28,16 @@ func jsDecode(data string, utils coraza.RuleTransformationTools) string { func doJsDecode(input string) string { d := []byte(input) - input_len := len(input) + inputLen := len(input) var i, c int - for i < input_len { + for i < inputLen { if input[i] == '\\' { /* Character is an escape. */ - if (i+5 < input_len) && (input[i+1] == 'u') && (utils.ValidHex(input[i+2])) && (utils.ValidHex(input[i+3])) && (utils.ValidHex(input[i+4])) && (utils.ValidHex(input[i+5])) { + switch { + + case (i+5 < inputLen) && (input[i+1] == 'u') && (utils.ValidHex(input[i+2])) && (utils.ValidHex(input[i+3])) && (utils.ValidHex(input[i+4])) && (utils.ValidHex(input[i+5])): /* \uHHHH */ /* Use only the lower byte. */ @@ -48,17 +50,17 @@ func doJsDecode(input string) string { c++ i += 6 - } else if (i+3 < input_len) && (input[i+1] == 'x') && utils.ValidHex(input[i+2]) && utils.ValidHex(input[i+3]) { + case (i+3 < inputLen) && (input[i+1] == 'x') && utils.ValidHex(input[i+2]) && utils.ValidHex(input[i+3]): /* \xHH */ d[c] = utils.X2c(input[i+2:]) c++ i += 4 - } else if (i+1 < input_len) && isodigit(input[i+1]) { + case (i+1 < inputLen) && isodigit(input[i+1]): /* \OOO (only one byte, \000 - \377) */ buf := make([]byte, 3) j := 0 - for (i+1+j < input_len) && (j < 3) { + for (i+1+j < inputLen) && (j < 3) { buf[j] = input[i+j] j++ if !isodigit(input[i+j]) { @@ -78,7 +80,7 @@ func doJsDecode(input string) string { c++ i += 1 + j } - } else if i+1 < input_len { + case i+1 < inputLen: /* \C */ cc := input[i+1] switch input[i+1] { @@ -104,9 +106,9 @@ func doJsDecode(input string) string { d[c] = cc c++ i += 2 - } else { + default: /* Not enough bytes */ - for i < input_len { + for i < inputLen { d[c] = input[i] c++ i++ diff --git a/transformations/normalise_path_win.go b/transformations/normalise_path_win.go index 5cb49e305..5c0873368 100644 --- a/transformations/normalise_path_win.go +++ b/transformations/normalise_path_win.go @@ -38,13 +38,7 @@ func normalisePathWin(data string, utils coraza.RuleTransformationTools) string } } -//We had to force the changes to the original library :( -const ( - PathSeparator = '\\' // OS-specific path separator - PathListSeparator = ';' // OS-specific path list separator -) - -//We overwrite the linux version manually +// We overwrite the linux version manually func clean(path string) string { originalPath := path volLen := volumeNameLen(path) diff --git a/transformations/remove_comments.go b/transformations/remove_comments.go index c65ecec13..90e4e6c88 100644 --- a/transformations/remove_comments.go +++ b/transformations/remove_comments.go @@ -44,19 +44,20 @@ func removeComments(value string, utils coraza.RuleTransformationTools) string { j++ } } else { - if (input[i] == '*') && (i+1 < inputLen) && (input[i+1] == '/') { + switch { + case (input[i] == '*') && (i+1 < inputLen) && (input[i+1] == '/'): incomment = false i += 2 input[j] = input[i] i++ j++ - } else if (input[i] == '-') && (i+1 < inputLen) && (input[i+1] == '-') && (i+2 < inputLen) && (input[i+2] == '>') { + case (input[i] == '-') && (i+1 < inputLen) && (input[i+1] == '-') && (i+2 < inputLen) && (input[i+2] == '>'): incomment = false i += 3 input[j] = input[i] i++ j++ - } else { + default: i++ } } diff --git a/transformations/remove_comments_char.go b/transformations/remove_comments_char.go index 4e5380aa6..51aea7de7 100644 --- a/transformations/remove_comments_char.go +++ b/transformations/remove_comments_char.go @@ -19,29 +19,28 @@ import "github.com/jptosso/coraza-waf/v2" func removeCommentsChar(data string, utils coraza.RuleTransformationTools) string { value := []byte(data) for i := 0; i < len(value); { - if value[i] == '/' && (i+1 < len(value)) && value[i+1] == '*' { + switch { + case value[i] == '/' && (i+1 < len(value)) && value[i+1] == '*': value = erase(value, i, 2) - } else if value[i] == '*' && - (i+1 < len(value)) && value[i+1] == '/' { + case value[i] == '*' && (i+1 < len(value)) && value[i+1] == '/': value = erase(value, i, 2) - } else if value[i] == '<' && + case value[i] == '<' && (i+1 < len(value)) && value[i+1] == '!' && (i+2 < len(value)) && value[i+2] == '-' && (i+3 < len(value)) && - value[i+3] == '-' { + value[i+3] == '-': value = erase(value, i, 4) - } else if value[i] == '-' && + case value[i] == '-' && (i+1 < len(value)) && value[i+1] == '-' && - (i+2 < len(value)) && value[i+2] == '>' { + (i+2 < len(value)) && value[i+2] == '>': value = erase(value, i, 3) - } else if value[i] == '-' && - (i+1 < len(value)) && value[i+1] == '-' { + case value[i] == '-' && (i+1 < len(value)) && value[i+1] == '-': value = erase(value, i, 2) - } else if value[i] == '#' { + case value[i] == '#': value = erase(value, i, 1) - } else { + default: i++ } } diff --git a/transformations/replace_comments.go b/transformations/replace_comments.go index 10313062e..93d651665 100644 --- a/transformations/replace_comments.go +++ b/transformations/replace_comments.go @@ -25,10 +25,10 @@ func doReplaceComments(value string) string { incomment := false input := []byte(value) - input_len := len(input) - for i < input_len { + inputLen := len(input) + for i < inputLen { if !incomment { - if (input[i] == '/') && (i+1 < input_len) && (input[i+1] == '*') { + if (input[i] == '/') && (i+1 < inputLen) && (input[i+1] == '*') { incomment = true i += 2 } else { @@ -37,7 +37,7 @@ func doReplaceComments(value string) string { j++ } } else { - if (input[i] == '*') && (i+1 < input_len) && (input[i+1] == '/') { + if (input[i] == '*') && (i+1 < inputLen) && (input[i+1] == '/') { incomment = false i += 2 input[j] = ' ' diff --git a/transformations/transformations_test.go b/transformations/transformations_test.go index 1b0a54ea7..4053eaf77 100644 --- a/transformations/transformations_test.go +++ b/transformations/transformations_test.go @@ -59,7 +59,7 @@ func TestTransformations(t *testing.T) { t.Error("Cannot parse test case") } for _, data := range cases { - //UNMARSHALL does not transform \u0000 to binary + // UNMARSHALL does not transform \u0000 to binary data.Input = strings.ReplaceAll(data.Input, `\u0000`, "\u0000") data.Output = strings.ReplaceAll(data.Output, `\u0000`, "\u0000") @@ -71,7 +71,7 @@ func TestTransformations(t *testing.T) { } trans, err := GetTransformation(data.Name) if err != nil { - //t.Error(err) + // t.Error(err) continue } tools := coraza.RuleTransformationTools{} diff --git a/transformations/url_decode.go b/transformations/url_decode.go index 58c3190d7..ee7f11fc6 100644 --- a/transformations/url_decode.go +++ b/transformations/url_decode.go @@ -20,24 +20,24 @@ import ( ) func urlDecode(data string, utils coraza.RuleTransformationTools) string { - res, _, _ := doUrlDecode(data) + res, _, _ := doURLDecode(data) return res } -//extracted from https://github.com/senghoo/modsecurity-go/blob/master/utils/urlencode.go -func doUrlDecode(input string) (string, bool, int) { +// extracted from https://github.com/senghoo/modsecurity-go/blob/master/utils/urlencode.go +func doURLDecode(input string) (string, bool, int) { d := []byte(input) - input_len := len(d) - var i, count, invalid_count, c int + inputLen := len(d) + var i, count, invalidCount, c int changed := false - for i < input_len { + for i < inputLen { if input[i] == '%' { /* Character is a percent sign. */ /* Are there enough bytes available? */ - if i+2 < input_len { + if i+2 < inputLen { c1 := input[i+1] c2 := input[i+2] if utils.ValidHex(c1) && utils.ValidHex(c2) { @@ -54,7 +54,7 @@ func doUrlDecode(input string) (string, bool, int) { c++ i++ count++ - invalid_count++ + invalidCount++ } } else { /* Not enough bytes available, copy the raw bytes. */ @@ -62,7 +62,7 @@ func doUrlDecode(input string) (string, bool, int) { c++ i++ count++ - invalid_count++ + invalidCount++ } } else { /* Character is not a percent sign. */ @@ -79,6 +79,6 @@ func doUrlDecode(input string) (string, bool, int) { } } - return string(d[0:c]), changed, invalid_count + return string(d[0:c]), changed, invalidCount } diff --git a/transformations/url_encode.go b/transformations/url_encode.go index 3597afc02..625540b96 100644 --- a/transformations/url_encode.go +++ b/transformations/url_encode.go @@ -17,20 +17,20 @@ package transformations import "github.com/jptosso/coraza-waf/v2" func urlEncode(data string, utils coraza.RuleTransformationTools) string { - return doUrlEncode(data) + return doURLEncode(data) } -func doUrlEncode(input string) string { - input_len := len(input) +func doURLEncode(input string) string { + inputLen := len(input) var i, count, c int - leng := input_len * 3 + leng := inputLen * 3 d := make([]byte, leng) d = append(d, []byte(input)...) c2xTable := []byte("0123456789abcdef") /* ENH Only encode the characters that really need to be encoded. */ - for i = 0; i < input_len; i++ { + for i = 0; i < inputLen; i++ { cc := input[i] if cc == ' ' { diff --git a/types/phase.go b/types/phase.go index 8d4d299e9..c5c551a1b 100644 --- a/types/phase.go +++ b/types/phase.go @@ -34,13 +34,14 @@ const ( // if the phase is invalid it will return an error func ParseRulePhase(phase string) (RulePhase, error) { i, err := strconv.Atoi(phase) - if phase == "request" { + switch { + case phase == "request": i = 2 - } else if phase == "response" { + case phase == "response": i = 4 - } else if phase == "logging" { + case phase == "logging": i = 5 - } else if err != nil || i > 5 || i < 1 { + case err != nil || i > 5 || i < 1: return 0, fmt.Errorf("invalid phase %s", phase) } return RulePhase(i), nil diff --git a/types/variables/variables.go b/types/variables/variables.go index e7991797c..cc101eea8 100644 --- a/types/variables/variables.go +++ b/types/variables/variables.go @@ -21,12 +21,16 @@ import ( // This file repeats the same content many times in order to make access // efficient for seclang and transactions +// RuleVariable is used for the rule to identify information +// Each RuleVariable is unique and represents a variable type RuleVariable byte const ( + // Unknown is the default value for a variable + // it's using for testing and error catching Unknown RuleVariable = RuleVariable(0) ResponseContentType RuleVariable = RuleVariable(1) - UniqueId RuleVariable = RuleVariable(2) + UniqueID RuleVariable = RuleVariable(2) ArgsCombinedSize RuleVariable = RuleVariable(3) AuthType RuleVariable = RuleVariable(4) FilesCombinedSize RuleVariable = RuleVariable(5) @@ -67,8 +71,8 @@ const ( RequestLine RuleVariable = RuleVariable(40) RequestMethod RuleVariable = RuleVariable(41) RequestProtocol RuleVariable = RuleVariable(42) - RequestUri RuleVariable = RuleVariable(43) - RequestUriRaw RuleVariable = RuleVariable(44) + RequestURI RuleVariable = RuleVariable(43) + RequestURIRaw RuleVariable = RuleVariable(44) ResponseBody RuleVariable = RuleVariable(45) ResponseContentLength RuleVariable = RuleVariable(46) ResponseProtocol RuleVariable = RuleVariable(47) @@ -105,194 +109,115 @@ const ( ArgsNames RuleVariable = RuleVariable(77) ArgsGetNames RuleVariable = RuleVariable(78) ArgsPostNames RuleVariable = RuleVariable(79) - Tx RuleVariable = RuleVariable(80) + TX RuleVariable = RuleVariable(80) Rule RuleVariable = RuleVariable(81) - Xml RuleVariable = RuleVariable(82) - Json RuleVariable = RuleVariable(83) + XML RuleVariable = RuleVariable(82) + JSON RuleVariable = RuleVariable(83) Env RuleVariable = RuleVariable(84) // Persisten storage kepy for compatibility - Ip RuleVariable = RuleVariable(85) + IP RuleVariable = RuleVariable(85) UrlencodedError RuleVariable = RuleVariable(86) ) +var rulemap = map[RuleVariable]string{ + Unknown: "UNKNOWN", + UrlencodedError: "URLENCODED_ERROR", + ResponseContentType: "RESPONSE_CONTENT_TYPE", + UniqueID: "UNIQUE_ID", + ArgsCombinedSize: "ARGS_COMBINED_SIZE", + AuthType: "AUTH_TYPE", + FilesCombinedSize: "FILES_COMBINED_SIZE", + FullRequest: "FULL_REQUEST", + FullRequestLength: "FULL_REQUEST_LENGTH", + InboundDataError: "INBOUND_DATA_ERROR", + MatchedVar: "MATCHED_VAR", + MatchedVarName: "MATCHED_VAR_NAME", + MultipartBoundaryQuoted: "MULTIPART_BOUNDARY_QUOTED", + MultipartBoundaryWhitespace: "MULTIPART_BOUNDARY_WHITESPACE", + MultipartCrlfLfLines: "MULTIPART_CRLF_LF_LINES", + MultipartDataAfter: "MULTIPART_DATA_AFTER", + MultipartDataBefore: "MULTIPART_DATA_BEFORE", + MultipartFileLimitExceeded: "MULTIPART_FILE_LIMIT_EXCEEDED", + MultipartHeaderFolding: "MULTIPART_HEADER_FOLDING", + MultipartInvalidHeaderFolding: "MULTIPART_INVALID_HEADER_FOLDING", + MultipartInvalidPart: "MULTIPART_INVALID_PART", + MultipartInvalidQuoting: "MULTIPART_INVALID_QUOTING", + MultipartLfLine: "MULTIPART_LF_LINE", + MultipartMissingSemicolon: "MULTIPART_MISSING_SEMICOLON", + MultipartStrictError: "MULTIPART_STRICT_ERROR", + MultipartUnmatchedBoundary: "MULTIPART_UNMATCHED_BOUNDARY", + OutboundDataError: "OUTBOUND_DATA_ERROR", + PathInfo: "PATH_INFO", + QueryString: "QUERY_STRING", + RemoteAddr: "REMOTE_ADDR", + RemoteHost: "REMOTE_HOST", + RemotePort: "REMOTE_PORT", + ReqbodyError: "REQBODY_ERROR", + ReqbodyErrorMsg: "REQBODY_ERROR_MSG", + ReqbodyProcessorError: "REQBODY_PROCESSOR_ERROR", + ReqbodyProcessorErrorMsg: "REQBODY_PROCESSOR_ERROR_MSG", + ReqbodyProcessor: "REQBODY_PROCESSOR", + RequestBasename: "REQUEST_BASENAME", + RequestBody: "REQUEST_BODY", + RequestBodyLength: "REQUEST_BODY_LENGTH", + RequestFilename: "REQUEST_FILENAME", + RequestLine: "REQUEST_LINE", + RequestMethod: "REQUEST_METHOD", + RequestProtocol: "REQUEST_PROTOCOL", + RequestURI: "REQUEST_URI", + RequestURIRaw: "REQUEST_URI_RAW", + ResponseBody: "RESPONSE_BODY", + ResponseContentLength: "RESPONSE_CONTENT_LENGTH", + ResponseProtocol: "RESPONSE_PROTOCOL", + ResponseStatus: "RESPONSE_STATUS", + ServerAddr: "SERVER_ADDR", + ServerName: "SERVER_NAME", + ServerPort: "SERVER_PORT", + Sessionid: "SESSIONID", + HighestSeverity: "HIGHEST_SEVERITY", + StatusLine: "STATUS_LINE", + InboundErrorData: "INBOUND_ERROR_DATA", + Duration: "DURATION", + ResponseHeadersNames: "RESPONSE_HEADERS_NAMES", + RequestHeadersNames: "REQUEST_HEADERS_NAMES", + Userid: "USERID", + Args: "ARGS", + ArgsGet: "ARGS_GET", + ArgsPost: "ARGS_POST", + FilesSizes: "FILES_SIZES", + FilesNames: "FILES_NAMES", + FilesTmpContent: "FILES_TMP_CONTENT", + MultipartFilename: "MULTIPART_FILENAME", + MultipartName: "MULTIPART_NAME", + MatchedVarsNames: "MATCHED_VARS_NAMES", + MatchedVars: "MATCHED_VARS", + Files: "FILES", + RequestCookies: "REQUEST_COOKIES", + RequestHeaders: "REQUEST_HEADERS", + ResponseHeaders: "RESPONSE_HEADERS", + Geo: "GEO", + RequestCookiesNames: "REQUEST_COOKIES_NAMES", + FilesTmpnames: "FILES_TMPNAMES", + ArgsNames: "ARGS_NAMES", + ArgsGetNames: "ARGS_GET_NAMES", + ArgsPostNames: "ARGS_POST_NAMES", + TX: "TX", + Rule: "RULE", + XML: "XML", + JSON: "JSON", + Env: "ENV", + IP: "IP", +} + +var rulemapRev = map[string]RuleVariable{} + // Name transforms a VARIABLE representation // into a string, it's used for audit and logging func (v RuleVariable) Name() string { - switch v { - case UrlencodedError: - return "URLENCODED_ERROR" - case ResponseContentType: - return "RESPONSE_CONTENT_TYPE" - case UniqueId: - return "UNIQUE_ID" - case ArgsCombinedSize: - return "ARGS_COMBINED_SIZE" - case AuthType: - return "AUTH_TYPE" - case FilesCombinedSize: - return "FILES_COMBINED_SIZE" - case FullRequest: - return "FULL_REQUEST" - case FullRequestLength: - return "FULL_REQUEST_LENGTH" - case InboundDataError: - return "INBOUND_DATA_ERROR" - case MatchedVar: - return "MATCHED_VAR" - case MatchedVarName: - return "MATCHED_VAR_NAME" - case MultipartBoundaryQuoted: - return "MULTIPART_BOUNDARY_QUOTED" - case MultipartBoundaryWhitespace: - return "MULTIPART_BOUNDARY_WHITESPACE" - case MultipartCrlfLfLines: - return "MULTIPART_CRLF_LF_LINES" - case MultipartDataAfter: - return "MULTIPART_DATA_AFTER" - case MultipartDataBefore: - return "MULTIPART_DATA_BEFORE" - case MultipartFileLimitExceeded: - return "MULTIPART_FILE_LIMIT_EXCEEDED" - case MultipartHeaderFolding: - return "MULTIPART_HEADER_FOLDING" - case MultipartInvalidHeaderFolding: - return "MULTIPART_INVALID_HEADER_FOLDING" - case MultipartInvalidPart: - return "MULTIPART_INVALID_PART" - case MultipartInvalidQuoting: - return "MULTIPART_INVALID_QUOTING" - case MultipartLfLine: - return "MULTIPART_LF_LINE" - case MultipartMissingSemicolon: - return "MULTIPART_MISSING_SEMICOLON" - case MultipartStrictError: - return "MULTIPART_STRICT_ERROR" - case MultipartUnmatchedBoundary: - return "MULTIPART_UNMATCHED_BOUNDARY" - case OutboundDataError: - return "OUTBOUND_DATA_ERROR" - case PathInfo: - return "PATH_INFO" - case QueryString: - return "QUERY_STRING" - case RemoteAddr: - return "REMOTE_ADDR" - case RemoteHost: - return "REMOTE_HOST" - case RemotePort: - return "REMOTE_PORT" - case ReqbodyError: - return "REQBODY_ERROR" - case ReqbodyErrorMsg: - return "REQBODY_ERROR_MSG" - case ReqbodyProcessorError: - return "REQBODY_PROCESSOR_ERROR" - case ReqbodyProcessorErrorMsg: - return "REQBODY_PROCESSOR_ERROR_MSG" - case ReqbodyProcessor: - return "REQBODY_PROCESSOR" - case RequestBasename: - return "REQUEST_BASENAME" - case RequestBody: - return "REQUEST_BODY" - case RequestBodyLength: - return "REQUEST_BODY_LENGTH" - case RequestFilename: - return "REQUEST_FILENAME" - case RequestLine: - return "REQUEST_LINE" - case RequestMethod: - return "REQUEST_METHOD" - case RequestProtocol: - return "REQUEST_PROTOCOL" - case RequestUri: - return "REQUEST_URI" - case RequestUriRaw: - return "REQUEST_URI_RAW" - case ResponseBody: - return "RESPONSE_BODY" - case ResponseContentLength: - return "RESPONSE_CONTENT_LENGTH" - case ResponseProtocol: - return "RESPONSE_PROTOCOL" - case ResponseStatus: - return "RESPONSE_STATUS" - case ServerAddr: - return "SERVER_ADDR" - case ServerName: - return "SERVER_NAME" - case ServerPort: - return "SERVER_PORT" - case Sessionid: - return "SESSIONID" - case HighestSeverity: - return "HIGHEST_SEVERITY" - case StatusLine: - return "STATUS_LINE" - case InboundErrorData: - return "INBOUND_ERROR_DATA" - case Duration: - return "DURATION" - case ResponseHeadersNames: - return "RESPONSE_HEADERS_NAMES" - case RequestHeadersNames: - return "REQUEST_HEADERS_NAMES" - case Userid: - return "USERID" - case Args: - return "ARGS" - case ArgsGet: - return "ARGS_GET" - case ArgsPost: - return "ARGS_POST" - case FilesSizes: - return "FILES_SIZES" - case FilesNames: - return "FILES_NAMES" - case FilesTmpContent: - return "FILES_TMP_CONTENT" - case MultipartFilename: - return "MULTIPART_FILENAME" - case MultipartName: - return "MULTIPART_NAME" - case MatchedVarsNames: - return "MATCHED_VARS_NAMES" - case MatchedVars: - return "MATCHED_VARS" - case Files: - return "FILES" - case RequestCookies: - return "REQUEST_COOKIES" - case RequestHeaders: - return "REQUEST_HEADERS" - case ResponseHeaders: - return "RESPONSE_HEADERS" - case Geo: - return "GEO" - case RequestCookiesNames: - return "REQUEST_COOKIES_NAMES" - case FilesTmpnames: - return "FILES_TMPNAMES" - case ArgsNames: - return "ARGS_NAMES" - case ArgsGetNames: - return "ARGS_GET_NAMES" - case ArgsPostNames: - return "ARGS_POST_NAMES" - case Tx: - return "TX" - case Rule: - return "RULE" - case Xml: - return "XML" - case Json: - return "JSON" - case Env: - return "ENV" - case Ip: - return "IP" + if name, ok := rulemap[v]; ok { + return name } return "UNKNOWN" } @@ -301,179 +226,15 @@ func (v RuleVariable) Name() string { // of a variable from a string // Returns error if there is no representation func ParseVariable(v string) (RuleVariable, error) { - switch strings.ToUpper(v) { - case "URLENCODED_ERROR": - return UrlencodedError, nil - case "RESPONSE_CONTENT_TYPE": - return ResponseContentType, nil - case "UNIQUE_ID": - return UniqueId, nil - case "ARGS_COMBINED_SIZE": - return ArgsCombinedSize, nil - case "AUTH_TYPE": - return AuthType, nil - case "FILES_COMBINED_SIZE": - return FilesCombinedSize, nil - case "FULL_REQUEST": - return FullRequest, nil - case "FULL_REQUEST_LENGTH": - return FullRequestLength, nil - case "INBOUND_DATA_ERROR": - return InboundDataError, nil - case "MATCHED_VAR": - return MatchedVar, nil - case "MATCHED_VAR_NAME": - return MatchedVarName, nil - case "MULTIPART_BOUNDARY_QUOTED": - return MultipartBoundaryQuoted, nil - case "MULTIPART_BOUNDARY_WHITESPACE": - return MultipartBoundaryWhitespace, nil - case "MULTIPART_CRLF_LF_LINES": - return MultipartCrlfLfLines, nil - case "MULTIPART_DATA_AFTER": - return MultipartDataAfter, nil - case "MULTIPART_DATA_BEFORE": - return MultipartDataBefore, nil - case "MULTIPART_FILE_LIMIT_EXCEEDED": - return MultipartFileLimitExceeded, nil - case "MULTIPART_HEADER_FOLDING": - return MultipartHeaderFolding, nil - case "MULTIPART_INVALID_HEADER_FOLDING": - return MultipartInvalidHeaderFolding, nil - case "MULTIPART_INVALID_PART": - return MultipartInvalidPart, nil - case "MULTIPART_INVALID_QUOTING": - return MultipartInvalidQuoting, nil - case "MULTIPART_LF_LINE": - return MultipartLfLine, nil - case "MULTIPART_MISSING_SEMICOLON": - return MultipartMissingSemicolon, nil - case "MULTIPART_STRICT_ERROR": - return MultipartStrictError, nil - case "MULTIPART_UNMATCHED_BOUNDARY": - return MultipartUnmatchedBoundary, nil - case "OUTBOUND_DATA_ERROR": - return OutboundDataError, nil - case "PATH_INFO": - return PathInfo, nil - case "QUERY_STRING": - return QueryString, nil - case "REMOTE_ADDR": - return RemoteAddr, nil - case "REMOTE_HOST": - return RemoteHost, nil - case "REMOTE_PORT": - return RemotePort, nil - case "REQBODY_ERROR": - return ReqbodyError, nil - case "REQBODY_ERROR_MSG": - return ReqbodyErrorMsg, nil - case "REQBODY_PROCESSOR_ERROR": - return ReqbodyProcessorError, nil - case "REQBODY_PROCESSOR_ERROR_MSG": - return ReqbodyProcessorErrorMsg, nil - case "REQBODY_PROCESSOR": - return ReqbodyProcessor, nil - case "REQUEST_BASENAME": - return RequestBasename, nil - case "REQUEST_BODY": - return RequestBody, nil - case "REQUEST_BODY_LENGTH": - return RequestBodyLength, nil - case "REQUEST_FILENAME": - return RequestFilename, nil - case "REQUEST_LINE": - return RequestLine, nil - case "REQUEST_METHOD": - return RequestMethod, nil - case "REQUEST_PROTOCOL": - return RequestProtocol, nil - case "REQUEST_URI": - return RequestUri, nil - case "REQUEST_URI_RAW": - return RequestUriRaw, nil - case "RESPONSE_BODY": - return ResponseBody, nil - case "RESPONSE_CONTENT_LENGTH": - return ResponseContentLength, nil - case "RESPONSE_PROTOCOL": - return ResponseProtocol, nil - case "RESPONSE_STATUS": - return ResponseStatus, nil - case "SERVER_ADDR": - return ServerAddr, nil - case "SERVER_NAME": - return ServerName, nil - case "SERVER_PORT": - return ServerPort, nil - case "SESSIONID": - return Sessionid, nil - case "HIGHEST_SEVERITY": - return HighestSeverity, nil - case "STATUS_LINE": - return StatusLine, nil - case "INBOUND_ERROR_DATA": - return InboundErrorData, nil - case "DURATION": - return Duration, nil - case "RESPONSE_HEADERS_NAMES": - return ResponseHeadersNames, nil - case "REQUEST_HEADERS_NAMES": - return RequestHeadersNames, nil - case "USERID": - return Userid, nil - case "ARGS": - return Args, nil - case "ARGS_GET": - return ArgsGet, nil - case "ARGS_POST": - return ArgsPost, nil - case "FILES_SIZES": - return FilesSizes, nil - case "FILES_NAMES": - return FilesNames, nil - case "FILES_TMP_CONTENT": - return FilesTmpContent, nil - case "MULTIPART_FILENAME": - return MultipartFilename, nil - case "MULTIPART_NAME": - return MultipartName, nil - case "MATCHED_VARS_NAMES": - return MatchedVarsNames, nil - case "MATCHED_VARS": - return MatchedVars, nil - case "FILES": - return Files, nil - case "REQUEST_COOKIES": - return RequestCookies, nil - case "REQUEST_HEADERS": - return RequestHeaders, nil - case "RESPONSE_HEADERS": - return ResponseHeaders, nil - case "GEO": - return Geo, nil - case "REQUEST_COOKIES_NAMES": - return RequestCookiesNames, nil - case "FILES_TMPNAMES": - return FilesTmpnames, nil - case "ARGS_NAMES": - return ArgsNames, nil - case "ARGS_GET_NAMES": - return ArgsGetNames, nil - case "ARGS_POST_NAMES": - return ArgsPostNames, nil - case "TX": - return Tx, nil - case "RULE": - return Rule, nil - case "XML": - return Xml, nil - case "JSON": - return Json, nil - case "ENV": - return Env, nil - case "IP": - return Ip, nil + if v, ok := rulemapRev[strings.ToUpper(v)]; ok { + return v, nil } return 0, fmt.Errorf("unknown variable %s", v) } + +func init() { + // we fill the rulemapRev with the reverse of rulemap + for k, v := range rulemap { + rulemapRev[v] = k + } +} diff --git a/types/variables/variables_test.go b/types/variables/variables_test.go index 1d5629d59..750039359 100644 --- a/types/variables/variables_test.go +++ b/types/variables/variables_test.go @@ -14,6 +14,7 @@ package variables import ( + "strings" "testing" ) @@ -26,3 +27,36 @@ func TestNameToVariable(t *testing.T) { } } } + +func TestVariableToName(t *testing.T) { + if len(rulemap) != len(rulemapRev) { + t.Error("rulemap and rulemapRev are not equal") + } + for i := 0; i < len(rulemap); i++ { + v := RuleVariable(i) + if v.Name() != rulemap[v] { + t.Error("failed to test variable " + v.Name()) + } + if rulemapRev[v.Name()] != v { + t.Error("failed to test variable " + v.Name()) + } + } +} + +func TestParseVariable(t *testing.T) { + for i := 0; i < len(rulemap); i++ { + v := RuleVariable(i) + name := v.Name() + // we create some lowercase cases + if i%3 == 0 { + name = strings.ToLower(name) + } + vv, err := ParseVariable(name) + if err != nil { + t.Errorf("failed to test variable %s", name) + } + if v != vv { + t.Errorf("failed to test variable %s", name) + } + } +} diff --git a/utils/strings.go b/utils/strings.go index f141cf199..d6745ec2f 100644 --- a/utils/strings.go +++ b/utils/strings.go @@ -27,7 +27,7 @@ var mu sync.Mutex func RandomString(length int) string { bytes := make([]byte, length) - //There is an entropy bug here with a lot of concurrency, so we need sync + // There is an entropy bug here with a lot of concurrency, so we need sync mu.Lock() _, err := rand.Read(bytes) diff --git a/utils/utils.go b/utils/utils.go index f3108f008..36f486185 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -17,7 +17,6 @@ package utils import ( "fmt" "io/ioutil" - "regexp" "strings" ) @@ -46,12 +45,3 @@ func OpenFile(path string, key string) ([]byte, error) { return ioutil.ReadFile(path) } } - -func ArgsToMap(args string) map[string]string { - a := map[string]string{} - re := regexp.MustCompile(`([\w\-_]+)=(.*?(?:\s|$))`) - for _, data := range re.FindAllStringSubmatch(args, -1) { - a[strings.TrimSpace(data[1])] = strings.TrimSpace(data[2]) - } - return a -} diff --git a/waf.go b/waf.go index b79672035..5ebe80fcd 100644 --- a/waf.go +++ b/waf.go @@ -168,7 +168,7 @@ func (w *Waf) NewTransaction() *Transaction { } // set capture variables - txvar := tx.GetCollection(variables.Tx) + txvar := tx.GetCollection(variables.TX) for i := 0; i <= 10; i++ { is := strconv.Itoa(i) txvar.Set(is, []string{""}) @@ -199,8 +199,8 @@ func (w *Waf) NewTransaction() *Transaction { variables.RequestBodyLength: "0", variables.Duration: "0", variables.HighestSeverity: "0", - variables.UniqueId: tx.Id, - //TODO single variables must be defaulted to empty string + variables.UniqueID: tx.Id, + // TODO single variables must be defaulted to empty string variables.RemoteAddr: "", } for v, data := range defaults { @@ -229,10 +229,15 @@ func (w *Waf) SetAuditLogger(engine string) error { return w.auditLogger.SetWriter(engine) } +// SetDebugLogPath sets the path for the debug log +// If the path is empty, the debug log will be disabled +// note: this is not thread safe func (w *Waf) SetDebugLogPath(path string) error { cfg := zap.NewProductionConfig() - cfg.OutputPaths = []string{ - path, + if path == "" { + cfg.OutputPaths = []string{} + } else { + cfg.OutputPaths = []string{path} } cfg.Level = *w.loggerAtomicLevel logger, err := cfg.Build() @@ -246,11 +251,15 @@ func (w *Waf) SetDebugLogPath(path string) error { // Logger returns the initiated loggers // Coraza supports unlimited loggers, so you can write for example // to syslog and a local drive at the same time -func (w *Waf) AuditLogger() loggers.Logger { - return *w.auditLogger +// AuditLogger() returns nil if the audit logger is not set +// Please try to use a nil logger... +func (w *Waf) AuditLogger() *loggers.Logger { + return w.auditLogger } // NewWaf creates a new WAF instance with default variables +// TODO there is much to fix here: +// - what are the default func NewWaf() *Waf { //default: us-ascii atom := zap.NewAtomicLevel() @@ -273,7 +282,7 @@ func NewWaf() *Waf { auditLogger: al, mux: &sync.RWMutex{}, RequestBodyInMemoryLimit: 131072, - RequestBodyLimit: 10000000, //10mb + RequestBodyLimit: 10000000, // 10mb ResponseBodyMimeTypes: []string{"text/html", "text/plain"}, ResponseBodyLimit: 524288, ResponseBodyAccess: false, @@ -291,7 +300,7 @@ func NewWaf() *Waf { // SetLogLevel changes the debug level of the Waf instance func (w *Waf) SetLogLevel(lvl int) error { - //setlevel is concurrent safe + // setlevel is concurrent safe switch lvl { case 0: w.loggerAtomicLevel.SetLevel(zapcore.FatalLevel)