From 851ee38b52874cd801db1433c7d9ba0bdfde38e2 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:17:49 +0000 Subject: [PATCH 1/7] chore(deps): pin actions/checkout action to 11bd719 in .github/workflows/regression.yml (#1216) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/regression.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index dc3a78d8d..3b0be03c0 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -19,7 +19,7 @@ jobs: tags: ${{ steps.generate.outputs.tags }} steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 - name: Generate tag combinations id: generate From 12d554b7d61a6ed1ac84024ab413dec189a96aa7 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:18:13 +0000 Subject: [PATCH 2/7] fix(deps): update module github.com/corazawaf/coraza-coreruleset/v4 to v4.7.0 in testing/coreruleset/go.mod (#1217) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- testing/coreruleset/go.mod | 2 +- testing/coreruleset/go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/testing/coreruleset/go.mod b/testing/coreruleset/go.mod index 796da5f37..08625fd88 100644 --- a/testing/coreruleset/go.mod +++ b/testing/coreruleset/go.mod @@ -4,7 +4,7 @@ go 1.22.3 require ( github.com/bmatcuk/doublestar/v4 v4.7.1 - github.com/corazawaf/coraza-coreruleset/v4 v4.6.0 + github.com/corazawaf/coraza-coreruleset/v4 v4.7.0 github.com/corazawaf/coraza/v3 v3.2.1 github.com/coreruleset/albedo v0.0.16 github.com/coreruleset/go-ftw v1.1.1 diff --git a/testing/coreruleset/go.sum b/testing/coreruleset/go.sum index 8a47f7b1a..28e197171 100644 --- a/testing/coreruleset/go.sum +++ b/testing/coreruleset/go.sum @@ -10,6 +10,8 @@ github.com/corazawaf/coraza-coreruleset v0.0.0-20240226094324-415b1017abdc h1:Ol github.com/corazawaf/coraza-coreruleset v0.0.0-20240226094324-415b1017abdc/go.mod h1:7rsocqNDkTCira5T0M7buoKR2ehh7YZiPkzxRuAgvVU= github.com/corazawaf/coraza-coreruleset/v4 v4.6.0 h1:VGlMw3QMuKaV7XgifPgcqCm66K+HRSdM4d9PRh1nD50= github.com/corazawaf/coraza-coreruleset/v4 v4.6.0/go.mod h1:1FQt1p+JSQ6tYrafMqZrEEdDmhq6aVuIJdnk+bM9hMY= +github.com/corazawaf/coraza-coreruleset/v4 v4.7.0 h1:j02CDxQYHVFZfBxbKLWYg66jSLbPmZp1GebyMwzN9Z0= +github.com/corazawaf/coraza-coreruleset/v4 v4.7.0/go.mod h1:1FQt1p+JSQ6tYrafMqZrEEdDmhq6aVuIJdnk+bM9hMY= github.com/corazawaf/libinjection-go v0.2.2 h1:Chzodvb6+NXh6wew5/yhD0Ggioif9ACrQGR4qjTCs1g= github.com/corazawaf/libinjection-go v0.2.2/go.mod h1:OP4TM7xdJ2skyXqNX1AN1wN5nNZEmJNuWbNPOItn7aw= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= From a5cc4f09827777c6e758b4efbd81e3a257e4bb0c Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Thu, 14 Nov 2024 10:19:34 +0100 Subject: [PATCH 3/7] perf: reduces transformArg allocation without multimatch (#1198) opt 1 Co-authored-by: Juan Pablo Tosso --- internal/corazawaf/rule.go | 82 ++++++++++++++++++--------------- internal/corazawaf/rule_test.go | 18 ++++---- internal/corazawaf/rulegroup.go | 2 +- 3 files changed, 55 insertions(+), 47 deletions(-) diff --git a/internal/corazawaf/rule.go b/internal/corazawaf/rule.go index cd6cd6269..4e04ff8de 100644 --- a/internal/corazawaf/rule.go +++ b/internal/corazawaf/rule.go @@ -242,8 +242,17 @@ func (r *Rule) doEvaluate(logger debuglog.Logger, phase types.RulePhase, tx *Tra } vLog.Debug().Msg("Expanding arguments for rule") + args := make([]string, 1) + var errs []error + var argsLen int for i, arg := range values { - args, errs := r.transformArg(arg, i, cache) + if r.MultiMatch { + args, errs = r.transformMultiMatchArg(arg) + argsLen = len(args) + } else { + args[0], errs = r.transformArg(arg, i, cache) + argsLen = 1 + } if len(errs) > 0 { vWarnLog := vLog.Warn() if vWarnLog.IsEnabled() { @@ -255,7 +264,7 @@ func (r *Rule) doEvaluate(logger debuglog.Logger, phase types.RulePhase, tx *Tra } // args represents the transformed variables - for _, carg := range args { + for _, carg := range args[:argsLen] { evalLog := vLog. Debug(). Str("operator_function", r.operator.Function). @@ -381,42 +390,41 @@ func (r *Rule) doEvaluate(logger debuglog.Logger, phase types.RulePhase, tx *Tra return matchedValues } -func (r *Rule) transformArg(arg types.MatchData, argIdx int, cache map[transformationKey]*transformationValue) ([]string, []error) { - if r.MultiMatch { - // TODOs: - // - We don't need to run every transformation. We could try for each until found - // - Cache is not used for multimatch - return r.executeTransformationsMultimatch(arg.Value()) - } else { - switch { - case len(r.transformations) == 0: - return []string{arg.Value()}, nil - case arg.Variable().Name() == "TX": - // no cache for TX - arg, errs := r.executeTransformations(arg.Value()) - return []string{arg}, errs - default: - // NOTE: See comment on transformationKey struct to understand this hacky code - argKey := arg.Key() - argKeyPtr := unsafe.StringData(argKey) - key := transformationKey{ - argKey: argKeyPtr, - argIndex: argIdx, - argVariable: arg.Variable(), - transformationsID: r.transformationsID, - } - if cached, ok := cache[key]; ok { - return cached.args, cached.errs - } else { - ars, es := r.executeTransformations(arg.Value()) - args := []string{ars} - errs := es - cache[key] = &transformationValue{ - args: args, - errs: es, - } - return args, errs +func (r *Rule) transformMultiMatchArg(arg types.MatchData) ([]string, []error) { + // TODOs: + // - We don't need to run every transformation. We could try for each until found + // - Cache is not used for multimatch + return r.executeTransformationsMultimatch(arg.Value()) +} + +func (r *Rule) transformArg(arg types.MatchData, argIdx int, cache map[transformationKey]*transformationValue) (string, []error) { + switch { + case len(r.transformations) == 0: + return arg.Value(), nil + case arg.Variable().Name() == "TX": + // no cache for TX + arg, errs := r.executeTransformations(arg.Value()) + return arg, errs + default: + // NOTE: See comment on transformationKey struct to understand this hacky code + argKey := arg.Key() + argKeyPtr := unsafe.StringData(argKey) + key := transformationKey{ + argKey: argKeyPtr, + argIndex: argIdx, + argVariable: arg.Variable(), + transformationsID: r.transformationsID, + } + if cached, ok := cache[key]; ok { + return cached.arg, cached.errs + } else { + ars, es := r.executeTransformations(arg.Value()) + errs := es + cache[key] = &transformationValue{ + arg: ars, + errs: es, } + return ars, errs } } } diff --git a/internal/corazawaf/rule_test.go b/internal/corazawaf/rule_test.go index 06d9c219e..d999bb0c8 100644 --- a/internal/corazawaf/rule_test.go +++ b/internal/corazawaf/rule_test.go @@ -506,23 +506,23 @@ func TestTransformArgSimple(t *testing.T) { rule := NewRule() _ = rule.AddTransformation("AppendA", transformationAppendA) _ = rule.AddTransformation("AppendB", transformationAppendB) - args, errs := rule.transformArg(md, 0, transformationCache) + arg, errs := rule.transformArg(md, 0, transformationCache) if errs != nil { t.Fatalf("Unexpected errors executing transformations: %v", errs) } - if args[0] != "/testAB" { - t.Errorf("Expected \"/testAB\", got \"%s\"", args[0]) + if arg != "/testAB" { + t.Errorf("Expected \"/testAB\", got \"%s\"", arg) } if len(transformationCache) != 1 { t.Errorf("Expected 1 transformations in cache, got %d", len(transformationCache)) } // Repeating the same transformation, expecting still one element in the cache (that means it is a cache hit) - args, errs = rule.transformArg(md, 0, transformationCache) + arg, errs = rule.transformArg(md, 0, transformationCache) if errs != nil { t.Fatalf("Unexpected errors executing transformations: %v", errs) } - if args[0] != "/testAB" { - t.Errorf("Expected \"/testAB\", got \"%s\"", args[0]) + if arg != "/testAB" { + t.Errorf("Expected \"/testAB\", got \"%s\"", arg) } if len(transformationCache) != 1 { t.Errorf("Expected 1 transformations in cache, got %d", len(transformationCache)) @@ -538,12 +538,12 @@ func TestTransformArgNoCacheForTXVariable(t *testing.T) { } rule := NewRule() _ = rule.AddTransformation("AppendA", transformationAppendA) - args, errs := rule.transformArg(md, 0, transformationCache) + arg, errs := rule.transformArg(md, 0, transformationCache) if errs != nil { t.Fatalf("Unexpected errors executing transformations: %v", errs) } - if args[0] != "testA" { - t.Errorf("Expected \"testA\", got \"%s\"", args[0]) + if arg != "testA" { + t.Errorf("Expected \"testA\", got \"%s\"", arg) } if len(transformationCache) != 0 { t.Errorf("Expected 0 transformations in cache, got %d. It is not expected to cache TX variable transformations", len(transformationCache)) diff --git a/internal/corazawaf/rulegroup.go b/internal/corazawaf/rulegroup.go index 81b3f0c7b..c03da9afa 100644 --- a/internal/corazawaf/rulegroup.go +++ b/internal/corazawaf/rulegroup.go @@ -260,6 +260,6 @@ type transformationKey struct { } type transformationValue struct { - args []string + arg string errs []error } From 832c9ae0bcdf931a3029d13d143a73f5aa23bbe2 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 15 Nov 2024 00:07:26 +0000 Subject: [PATCH 4/7] fix(deps): update all non-major dependencies in testing/coreruleset/go.mod (#1218) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/codeql-analysis.yml | 6 +++--- testing/coreruleset/go.mod | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index a11d235df..6f4a372f9 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -13,12 +13,12 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 - name: Initialize CodeQL - uses: github/codeql-action/init@396bb3e45325a47dd9ef434068033c6d5bb0d11a # v3 + uses: github/codeql-action/init@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f # v3 with: languages: go - name: Autobuild - uses: github/codeql-action/autobuild@396bb3e45325a47dd9ef434068033c6d5bb0d11a # v3 + uses: github/codeql-action/autobuild@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f # v3 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@396bb3e45325a47dd9ef434068033c6d5bb0d11a # v3 + uses: github/codeql-action/analyze@ea9e4e37992a54ee68a9622e985e60c8e8f12d9f # v3 diff --git a/testing/coreruleset/go.mod b/testing/coreruleset/go.mod index 08625fd88..ae5f4d734 100644 --- a/testing/coreruleset/go.mod +++ b/testing/coreruleset/go.mod @@ -5,7 +5,7 @@ go 1.22.3 require ( github.com/bmatcuk/doublestar/v4 v4.7.1 github.com/corazawaf/coraza-coreruleset/v4 v4.7.0 - github.com/corazawaf/coraza/v3 v3.2.1 + github.com/corazawaf/coraza/v3 v3.2.2 github.com/coreruleset/albedo v0.0.16 github.com/coreruleset/go-ftw v1.1.1 github.com/rs/zerolog v1.33.0 From 1e2da9fcc4c2e9ead9170e3ed7b8ad47b703f7be Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 15 Nov 2024 00:07:32 +0000 Subject: [PATCH 5/7] chore(deps): update codecov/codecov-action action to v5 in .github/workflows/regression.yml (#1219) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/regression.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index 3b0be03c0..6b68f5537 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -48,28 +48,28 @@ jobs: export BUILD_TAGS=${{ matrix.build-flag }} go run mage.go coverage - name: "Codecov: General" - uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238 # v4 + uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 if: ${{ matrix.go-version == '1.22.x' }} with: files: build/coverage.txt flags: default,${{ matrix.build-flag }} token: ${{ secrets.CODECOV_TOKEN }} - name: "Codecov: Examples" - uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238 # v4 + uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 if: ${{ matrix.go-version == '1.22.x' }} with: files: build/coverage-examples.txt flags: examples+${{ matrix.build-flag }} token: ${{ secrets.CODECOV_TOKEN }} - name: "Codecov: FTW" - uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238 # v4 + uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 if: ${{ matrix.go-version == '1.22.x' }} with: files: build/coverage-ftw.txt flags: ftw,${{ matrix.build-flag }} token: ${{ secrets.CODECOV_TOKEN }} - name: "Codecov: Tinygo" - uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238 # v4 + uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 # only if coverage-tinygo.txt exists if: ${{ matrix.go-version == '1.22.x' && hashFiles('build/coverage-tinygo.txt') != '' }} with: From bbadbb6ad368b8d7e0179ddc90bb1fb3d2239a3c Mon Sep 17 00:00:00 2001 From: Namburi Soujanya <54130357+soujanyanmbri@users.noreply.github.com> Date: Fri, 15 Nov 2024 19:51:25 +0530 Subject: [PATCH 6/7] Perf: Improve Performance, reduce heap allocations (#1202) * Reduce heap allocation #1 * Remove append, use simple allocation * Remove append, use simple allocation * Add benchmark + Get * Improve test * Minor improvments * Minor fix * Minor fix * Minor fix * Minor fix * Minor fix * Minor fix - allocations * Minor fix - allocations * Minor fix - allocations * Minor fix - remove append --------- Co-authored-by: Matteo Pace Co-authored-by: Juan Pablo Tosso --- internal/collections/map.go | 26 ++++++++++++++++++-------- internal/collections/map_test.go | 22 ++++++++++++++++++++++ internal/collections/named.go | 8 ++++---- internal/corazarules/rule_match.go | 12 ++++++------ 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/internal/collections/map.go b/internal/collections/map.go index 069c8e6a9..ba6adf35e 100644 --- a/internal/collections/map.go +++ b/internal/collections/map.go @@ -47,11 +47,15 @@ func (c *Map) Get(key string) []string { if !c.isCaseSensitive { key = strings.ToLower(key) } - var values []string - for _, a := range c.data[key] { - values = append(values, a.value) + values := c.data[key] + if len(values) == 0 { + return nil + } + result := make([]string, len(values)) + for i, v := range values { + result[i] = v.value } - return values + return result } // FindRegex returns all map elements whose key matches the regular expression. @@ -120,16 +124,22 @@ func (c *Map) Add(key string, value string) { c.data[key] = append(c.data[key], aVal) } -// Set sets the value of a key with the array of strings passed. If the key already exists, it will be overwritten. +// Sets the value of a key with the array of strings passed. If the key already exists, it will be overwritten. func (c *Map) Set(key string, values []string) { originalKey := key if !c.isCaseSensitive { key = strings.ToLower(key) } - c.data[key] = make([]keyValue, 0, len(values)) - for _, v := range values { - c.data[key] = append(c.data[key], keyValue{key: originalKey, value: v}) + dataSlice, exists := c.data[key] + if !exists || cap(dataSlice) < len(values) { + dataSlice = make([]keyValue, len(values)) + } else { + dataSlice = dataSlice[:len(values)] // Reuse existing slice with the same length + } + for i, v := range values { + dataSlice[i] = keyValue{key: originalKey, value: v} } + c.data[key] = dataSlice } // SetIndex sets the value of a key at the specified index. If the key already exists, it will be overwritten. diff --git a/internal/collections/map_test.go b/internal/collections/map_test.go index 68bb83c42..7c47d29c5 100644 --- a/internal/collections/map_test.go +++ b/internal/collections/map_test.go @@ -106,3 +106,25 @@ func TestNewCaseSensitiveKeyMap(t *testing.T) { } } + +func BenchmarkTxSetGet(b *testing.B) { + keys := make(map[int]string, b.N) + for i := 0; i < b.N; i++ { + keys[i] = fmt.Sprintf("key%d", i) + } + c := NewCaseSensitiveKeyMap(variables.RequestHeaders) + + b.Run("Set", func(b *testing.B) { + b.ResetTimer() + for i := 0; i < b.N; i++ { + c.Set(keys[i], []string{"value2"}) + } + }) + b.Run("Get", func(b *testing.B) { + b.ResetTimer() + for i := 0; i < b.N; i++ { + c.Get(keys[i]) + } + }) + b.ReportAllocs() +} diff --git a/internal/collections/named.go b/internal/collections/named.go index 88d9166fd..855566750 100644 --- a/internal/collections/named.go +++ b/internal/collections/named.go @@ -61,11 +61,11 @@ func (c *NamedCollection) Len() int { // Data is an internal method used for serializing to JSON func (c *NamedCollection) Data() map[string][]string { - result := map[string][]string{} + result := make(map[string][]string, len(c.data)) for k, v := range c.data { - result[k] = make([]string, 0, len(v)) - for _, a := range v { - result[k] = append(result[k], a.value) + result[k] = make([]string, len(v)) + for i, a := range v { + result[k][i] = a.value } } return result diff --git a/internal/corazarules/rule_match.go b/internal/corazarules/rule_match.go index 67f8f7dc1..5b6700ed4 100644 --- a/internal/corazarules/rule_match.go +++ b/internal/corazarules/rule_match.go @@ -33,27 +33,27 @@ type MatchData struct { var _ types.MatchData = (*MatchData)(nil) -func (m *MatchData) Variable() variables.RuleVariable { +func (m MatchData) Variable() variables.RuleVariable { return m.Variable_ } -func (m *MatchData) Key() string { +func (m MatchData) Key() string { return m.Key_ } -func (m *MatchData) Value() string { +func (m MatchData) Value() string { return m.Value_ } -func (m *MatchData) Message() string { +func (m MatchData) Message() string { return m.Message_ } -func (m *MatchData) Data() string { +func (m MatchData) Data() string { return m.Data_ } -func (m *MatchData) ChainLevel() int { +func (m MatchData) ChainLevel() int { return m.ChainLevel_ } From 3bc5ef6f7b9c9b9146e2d39205bbbc6c162e3e32 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sat, 16 Nov 2024 00:26:03 +0000 Subject: [PATCH 7/7] chore(deps): update codecov/codecov-action digest to 5c47607 in .github/workflows/regression.yml (#1222) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/regression.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index 6b68f5537..7fe896311 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -48,28 +48,28 @@ jobs: export BUILD_TAGS=${{ matrix.build-flag }} go run mage.go coverage - name: "Codecov: General" - uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 + uses: codecov/codecov-action@5c47607acb93fed5485fdbf7232e8a31425f672a # v5 if: ${{ matrix.go-version == '1.22.x' }} with: files: build/coverage.txt flags: default,${{ matrix.build-flag }} token: ${{ secrets.CODECOV_TOKEN }} - name: "Codecov: Examples" - uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 + uses: codecov/codecov-action@5c47607acb93fed5485fdbf7232e8a31425f672a # v5 if: ${{ matrix.go-version == '1.22.x' }} with: files: build/coverage-examples.txt flags: examples+${{ matrix.build-flag }} token: ${{ secrets.CODECOV_TOKEN }} - name: "Codecov: FTW" - uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 + uses: codecov/codecov-action@5c47607acb93fed5485fdbf7232e8a31425f672a # v5 if: ${{ matrix.go-version == '1.22.x' }} with: files: build/coverage-ftw.txt flags: ftw,${{ matrix.build-flag }} token: ${{ secrets.CODECOV_TOKEN }} - name: "Codecov: Tinygo" - uses: codecov/codecov-action@968872560f81e7bdde9272853e65f2507c0eca7c # v5 + uses: codecov/codecov-action@5c47607acb93fed5485fdbf7232e8a31425f672a # v5 # only if coverage-tinygo.txt exists if: ${{ matrix.go-version == '1.22.x' && hashFiles('build/coverage-tinygo.txt') != '' }} with: