Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first persistence draft #866

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions collection/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,45 @@ type Keyed interface {
FindString(key string) []types.MatchData
}

type Editable interface {
Keyed

// Remove deletes the key from the CollectionMap
Remove(key string)

// Set will replace the key's value with this slice
Set(key string, values []string)

// TODO: in v4 this should contain setters for Map and Persistence
}

// Map are used to store VARIABLE data
// for transactions, this data structured is designed
// to store slices of data for keys
// Important: CollectionMaps ARE NOT concurrent safe
type Map interface {
Keyed
Editable
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a breaking change, as it still implements Keyed and it keeps the same functions (?)


// Add a value to some key
Add(key string, value string)

// Set will replace the key's value with this slice
Set(key string, values []string)

// SetIndex will place the value under the index
// If the index is higher than the current size of the CollectionMap
// it will be appended
SetIndex(key string, index int, value string)
}

// Remove deletes the key from the CollectionMap
Remove(key string)
// Persistent collections won't use arrays as values
// They are designed for collections that will be stored
type Persistent interface {
Editable

// Initializes the input as the collection key
Init(key string)

// Sum will add the value to the key
Sum(key string, sum int)

// SetOne will replace the key's value with this string
SetOne(key string, value string)
}
14 changes: 14 additions & 0 deletions experimental/plugins/persistence.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors
// SPDX-License-Identifier: Apache-2.0

package plugins

import (
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
"github.com/corazawaf/coraza/v3/internal/persistence"
)

// RegisterPersistenceEngine registers a new persistence engine
func RegisterPersistenceEngine(name string, engine plugintypes.PersistenceEngine) {
persistence.Register(name, engine)

Check warning on line 13 in experimental/plugins/persistence.go

View check run for this annotation

Codecov / codecov/patch

experimental/plugins/persistence.go#L12-L13

Added lines #L12 - L13 were not covered by tests
}
15 changes: 15 additions & 0 deletions experimental/plugins/plugintypes/persistence.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors
// SPDX-License-Identifier: Apache-2.0

package plugintypes

type PersistenceEngine interface {
Open(uri string, ttl int) error
Close() error
Sum(collectionName string, collectionKey string, key string, sum int) error
Get(collectionName string, collectionKey string, key string) (string, error)

All(collectionName string, collectionKey string) (map[string]string, error)
Set(collection string, collectionKey string, key string, value string) error
Remove(collection string, collectionKey string, key string) error
}
6 changes: 6 additions & 0 deletions experimental/plugins/plugintypes/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,10 @@ type TransactionVariables interface {
ArgsNames() collection.Collection
ArgsGetNames() collection.Collection
ArgsPostNames() collection.Collection
// TODO(v4: Add these)
// Session() collection.Persistent
// User() collection.Persistent
// IP() collection.Persistent
// Global() collection.Persistent
// Resource() collection.Persistent
}
60 changes: 32 additions & 28 deletions internal/actions/initcol.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@
package actions

import (
"fmt"
"strings"

"github.com/corazawaf/coraza/v3/experimental/plugins/macro"
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
"github.com/corazawaf/coraza/v3/internal/collections"
utils "github.com/corazawaf/coraza/v3/internal/strings"
"github.com/corazawaf/coraza/v3/types/variables"
)

// Action Group: Non-disruptive
Expand All @@ -23,9 +28,8 @@
// SecAction "phase:1,id:116,nolog,pass,initcol:ip=%{REMOTE_ADDR}"
// ```
type initcolFn struct {
collection string
variable byte
key string
collection variables.RuleVariable
key macro.Macro
}

func (a *initcolFn) Init(_ plugintypes.RuleMetadata, data string) error {
Expand All @@ -34,34 +38,34 @@
return ErrInvalidKVArguments
}

a.collection = col
a.key = key
a.variable = 0x0
c, err := variables.Parse(col)
if err != nil {
return fmt.Errorf("initcol: collection %s is not valid", col)
}

Check warning on line 44 in internal/actions/initcol.go

View check run for this annotation

Codecov / codecov/patch

internal/actions/initcol.go#L43-L44

Added lines #L43 - L44 were not covered by tests
// we validate if this is a persistent collection
persistent := []string{"USER", "SESSION", "IP", "RESOURCE", "GLOBAL"}
if !utils.InSlice(c.Name(), persistent) {
return fmt.Errorf("initcol: collection %s is not persistent", c.Name())
}

Check warning on line 49 in internal/actions/initcol.go

View check run for this annotation

Codecov / codecov/patch

internal/actions/initcol.go#L48-L49

Added lines #L48 - L49 were not covered by tests
a.collection = c
mkey, err := macro.NewMacro(key)
if err != nil {
return err
}

Check warning on line 54 in internal/actions/initcol.go

View check run for this annotation

Codecov / codecov/patch

internal/actions/initcol.go#L53-L54

Added lines #L53 - L54 were not covered by tests
a.key = mkey
return nil
}

func (a *initcolFn) Evaluate(_ plugintypes.RuleMetadata, _ plugintypes.TransactionState) {
// tx.DebugLogger().Error().Msg("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)
if data == nil {
ts := time.Now().UnixNano()
tss := strconv.FormatInt(ts, 10)
tsstimeout := strconv.FormatInt(ts+(int64(tx.WAF.CollectionTimeout)*1000), 10)
data = map[string][]string{
"CREATE_TIME": {tss},
"IS_NEW": {"1"},
"KEY": {key},
"LAST_UPDATE_TIME": {tss},
"TIMEOUT": {tsstimeout},
"UPDATE_COUNTER": {"0"},
"UPDATE_RATE": {"0"},
}
}
tx.GetCollection(a.variable).SetData(data)
tx.PersistentCollections[a.variable] = key
*/
func (a *initcolFn) Evaluate(_ plugintypes.RuleMetadata, txs plugintypes.TransactionState) {
col := txs.Collection(a.collection)
key := a.key.Expand(txs)
txs.DebugLogger().Debug().Str("collection", a.collection.Name()).Str("key", key).Msg("initcol: initializing collection")
c, ok := col.(*collections.Persistent)
if !ok {
txs.DebugLogger().Error().Str("collection", a.collection.Name()).Msg("initcol: collection is not a persistent collection")
return
}

Check warning on line 67 in internal/actions/initcol.go

View check run for this annotation

Codecov / codecov/patch

internal/actions/initcol.go#L65-L67

Added lines #L65 - L67 were not covered by tests
c.Init(key)
}

func (a *initcolFn) Type() plugintypes.ActionType {
Expand Down
31 changes: 20 additions & 11 deletions internal/actions/initcol_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,33 @@
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors
// SPDX-License-Identifier: Apache-2.0

package actions
package actions_test

import "testing"
import (
"testing"

"github.com/corazawaf/coraza/v3"
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
"github.com/corazawaf/coraza/v3/internal/actions"
)

func TestInitcolInit(t *testing.T) {
waf, _ := coraza.NewWAF(coraza.NewWAFConfig()) //nolint:errcheck
initcol, err := actions.Get("initcol")
if err != nil {
t.Error(err)
}
t.Run("invalid argument", func(t *testing.T) {
initcol := initcol()
err := initcol.Init(nil, "foo")
if err == nil {
t.Errorf("expected error")
if err := initcol.Init(&md{}, "test"); err == nil {
t.Error("expected error")
}
})

t.Run("passing argument", func(t *testing.T) {
initcol := initcol()
err := initcol.Init(nil, "foo=bar")
if err != nil {
t.Errorf("unexpected error: %s", err.Error())
t.Run("editable variable", func(t *testing.T) {
if err := initcol.Init(&md{}, "session=abcdef"); err != nil {
t.Error(err)
}
txs := waf.NewTransaction().(plugintypes.TransactionState)
initcol.Evaluate(&md{}, txs)
})
}
21 changes: 11 additions & 10 deletions internal/actions/setvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"strconv"
"strings"

utils "github.com/corazawaf/coraza/v3/internal/strings"

"github.com/corazawaf/coraza/v3/collection"
"github.com/corazawaf/coraza/v3/experimental/plugins/macro"
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
Expand Down Expand Up @@ -76,8 +78,10 @@
colKey, colVal, colOk := strings.Cut(key, ".")
// Right not it only makes sense to allow setting TX
// key is also required
if strings.ToUpper(colKey) != "TX" {
return errors.New("invalid arguments, expected collection TX")
available := []string{"TX", "USER", "GLOBAL", "RESOURCE", "SESSION", "IP"}
// we validate uppercase colKey is one of available
if !utils.InSlice(strings.ToUpper(colKey), available) {
return errors.New("setvar: invalid editable collection, available collections are: " + strings.Join(available, ", "))
}
if strings.TrimSpace(colVal) == "" {
return errors.New("invalid arguments, expected syntax TX.{key}={value}")
Expand Down Expand Up @@ -111,7 +115,7 @@
Str("var_key", key).
Str("var_value", value).
Int("rule_id", r.ID()).
Msg("Action evaluated")
Msg("Action SetVar evaluated")
a.evaluateTxCollection(r, tx, strings.ToLower(key), value)
}

Expand All @@ -120,17 +124,14 @@
}

func (a *setvarFn) evaluateTxCollection(r plugintypes.RuleMetadata, tx plugintypes.TransactionState, key string, value string) {
var col collection.Map
if c, ok := tx.Collection(a.collection).(collection.Map); !ok {
tx.DebugLogger().Error().Msg("collection in setvar is not a map")
// TODO for api breaking issues, we have to split this function in Map and Persistent
var col collection.Editable
if c, ok := tx.Collection(a.collection).(collection.Editable); !ok {
tx.DebugLogger().Error().Msg("collection in setvar is not editable")

Check warning on line 130 in internal/actions/setvar.go

View check run for this annotation

Codecov / codecov/patch

internal/actions/setvar.go#L130

Added line #L130 was not covered by tests
return
} else {
col = c
}
if col == nil {
tx.DebugLogger().Error().Msg("collection in setvar is nil")
return
}

if a.isRemove {
col.Remove(key)
Expand Down
37 changes: 37 additions & 0 deletions internal/actions/setvar_persistence_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors
// SPDX-License-Identifier: Apache-2.0

//go:build !tinygo
// +build !tinygo

package actions_test

import (
"testing"

"github.com/corazawaf/coraza/v3"
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
"github.com/corazawaf/coraza/v3/internal/actions"
"github.com/corazawaf/coraza/v3/types/variables"
)

func TestPersistenceSetvar(t *testing.T) {
a, err := actions.Get("setvar")
if err != nil {
t.Error("failed to get setvar action")
}
waf, err := coraza.NewWAF(coraza.NewWAFConfig().WithDirectives("SecPersistenceEngine default"))
if err != nil {
t.Fatal(err)
}
t.Run("SESSION should be set", func(t *testing.T) {
if err := a.Init(&md{}, "SESSION.test=test"); err != nil {
t.Errorf("unexpected error: %v", err)
}
tx := waf.NewTransaction()
txs := tx.(plugintypes.TransactionState)
a.Evaluate(&md{}, txs)
col := txs.Collection(variables.Session)
col.FindAll()
})
}
18 changes: 10 additions & 8 deletions internal/actions/setvar_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors
// SPDX-License-Identifier: Apache-2.0

package actions
package actions_test

import (
"testing"

"github.com/corazawaf/coraza/v3/internal/actions"
)

type md struct {
Expand All @@ -21,27 +23,27 @@ func (_ md) Status() int {
}

func TestSetvarInit(t *testing.T) {
a, err := actions.Get("setvar")
if err != nil {
t.Error("failed to get setvar action")
}
t.Run("no arguments", func(t *testing.T) {
a := setvar()
if err := a.Init(nil, ""); err == nil || err != ErrMissingArguments {
if err := a.Init(nil, ""); err == nil || err != actions.ErrMissingArguments {
t.Error("expected error ErrMissingArguments")
}
})
t.Run("non-map variable", func(t *testing.T) {
a := setvar()
if err := a.Init(&md{}, "PATH_INFO=test"); err == nil {
t.Error("expected error")
}
})
t.Run("TX set ok", func(t *testing.T) {
a := setvar()
if err := a.Init(&md{}, "TX.some=test"); err != nil {
t.Error(err)
}
})
t.Run("TX without key should fail", func(t *testing.T) {
a := setvar()
if err := a.Init(&md{}, "TX=test"); err == nil {
t.Run("SESSION without key should fail", func(t *testing.T) {
if err := a.Init(&md{}, "SESSION=test"); err == nil {
t.Error("expected error")
}
})
Expand Down
2 changes: 1 addition & 1 deletion internal/auditlog/concurrent_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestConcurrentWriterNoop(t *testing.T) {

func TestConcurrentWriterFailsOnInit(t *testing.T) {
config := NewConfig()
config.Target = "/unexisting.log"
config.Target = "/invalid/unexisting.log"
config.Dir = t.TempDir()
config.FileMode = fs.FileMode(0777)
config.DirMode = fs.FileMode(0777)
Expand Down
2 changes: 1 addition & 1 deletion internal/auditlog/serial_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestSerialLoggerSuccessOnInit(t *testing.T) {

func TestSerialWriterFailsOnInitForUnexistingFile(t *testing.T) {
config := NewConfig()
config.Target = "/unexisting.log"
config.Target = "/invalid/unexisting.log"
config.Dir = t.TempDir()
config.FileMode = fs.FileMode(0777)
config.DirMode = fs.FileMode(0777)
Expand Down
Loading
Loading