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

Allow setting a root fs.FS in a parser. #393

Merged
merged 12 commits into from
Sep 8, 2022
26 changes: 26 additions & 0 deletions internal/io/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package io

import (
"fmt"
"io/fs"
"os"
"path"
"path/filepath"
)

// ReadFirstFile looks for the file in all available paths
Expand All @@ -28,3 +30,27 @@ func ReadFirstFile(directories []string, filename string) ([]byte, error) {
}
return nil, fmt.Errorf("file %s not found", filename)
}

// OSFS implements fs.FS using methods on os to read from the system.
// Note that this implementation is not a compliant fs.FS, as they should only
// accept posix-style, relative paths, but as this is an internal implementation
// detail, we get the abstraction we need while being able to handle paths as
// the os package otherwise would.
// More context in: https://github.com/golang/go/issues/44279
type OSFS struct{}

func (OSFS) Open(name string) (fs.File, error) {
return os.Open(name)
}

func (OSFS) ReadFile(name string) ([]byte, error) {
return os.ReadFile(name)
}

func (OSFS) ReadDir(name string) ([]fs.DirEntry, error) {
return os.ReadDir(name)
}

func (OSFS) Glob(pattern string) ([]string, error) {
return filepath.Glob(pattern)
}
7 changes: 4 additions & 3 deletions operators/from_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@ package operators

import (
"errors"
"io/fs"
"os"
"path"
)

var errEmptyPaths = errors.New("empty paths")

func loadFromFile(filepath string, paths []string) ([]byte, error) {
func loadFromFile(filepath string, paths []string, root fs.FS) ([]byte, error) {
if path.IsAbs(filepath) {
return os.ReadFile(filepath)
return fs.ReadFile(root, filepath)
}

if len(paths) == 0 {
Expand All @@ -34,7 +35,7 @@ func loadFromFile(filepath string, paths []string) ([]byte, error) {
}

absFilepath := path.Join(p, filepath)
content, err = os.ReadFile(absFilepath)
content, err = fs.ReadFile(root, absFilepath)
if err != nil {
if os.IsNotExist(err) {
continue
Expand Down
25 changes: 21 additions & 4 deletions operators/from_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
"path"
"path/filepath"
"testing"
"testing/fstest"

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

const fileContent = "abc123"
Expand All @@ -24,14 +27,14 @@ func getTestFile(t *testing.T) (string, string) {
}

func TestLoadFromFileNoPaths(t *testing.T) {
_, err := loadFromFile("non-existing-file", nil)
_, err := loadFromFile("non-existing-file", nil, io.OSFS{})
if err == nil {
t.Errorf("expected error: %s", errEmptyPaths.Error())
}
}

func TestLoadFromFileNoExist(t *testing.T) {
content, err := loadFromFile("non-existing-file", []string{t.TempDir()})
content, err := loadFromFile("non-existing-file", []string{t.TempDir()}, io.OSFS{})
if err == nil {
t.Errorf("expected error: %s", os.ErrNotExist.Error())
}
Expand All @@ -44,7 +47,7 @@ func TestLoadFromFileNoExist(t *testing.T) {
func TestLoadFromFileAbsolutePath(t *testing.T) {
testDir, testFile := getTestFile(t)

content, err := loadFromFile(path.Join(testDir, testFile), nil)
content, err := loadFromFile(path.Join(testDir, testFile), nil, io.OSFS{})
if err != nil {
t.Error(err)
}
Expand All @@ -57,7 +60,7 @@ func TestLoadFromFileAbsolutePath(t *testing.T) {
func TestLoadFromFileRelativePath(t *testing.T) {
testDir, testFile := getTestFile(t)

content, err := loadFromFile(testFile, []string{"/does-not-exist", testDir})
content, err := loadFromFile(testFile, []string{"/does-not-exist", testDir}, io.OSFS{})
if err != nil {
t.Errorf("failed to load from file: %s", err.Error())
}
Expand All @@ -66,3 +69,17 @@ func TestLoadFromFileRelativePath(t *testing.T) {
t.Errorf("unexpected content, want %q, have %q", want, have)
}
}

func TestLoadFromCustomFS(t *testing.T) {
fs := fstest.MapFS{}
fs["animals/bear.txt"] = &fstest.MapFile{Data: []byte("pooh"), Mode: 0755}

content, err := loadFromFile("bear.txt", []string{"animals"}, fs)
if err != nil {
t.Errorf("failed to load from file: %s", err.Error())
}

if want, have := "pooh", string(content); want != have {
t.Errorf("unexpected content, want %q, have %q", want, have)
}
}
2 changes: 1 addition & 1 deletion operators/ip_match_from_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type ipMatchFromFile struct {
func (o *ipMatchFromFile) Init(options coraza.RuleOperatorOptions) error {
path := options.Arguments

data, err := loadFromFile(path, options.Path)
data, err := loadFromFile(path, options.Path, options.Root)
if err != nil {
return err
}
Expand Down
4 changes: 3 additions & 1 deletion operators/ip_match_from_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

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

func TestFromFile(t *testing.T) {
Expand All @@ -15,8 +16,9 @@ func TestFromFile(t *testing.T) {

ipm := &ipMatchFromFile{}
opts := coraza.RuleOperatorOptions{
Arguments: string("./testdata/op/netranges.dat"),
Arguments: "./testdata/op/netranges.dat",
Path: []string{"."},
Root: io.OSFS{},
}
if err := ipm.Init(opts); err != nil {
t.Error("Cannot init ipmatchfromfile operator")
Expand Down
3 changes: 2 additions & 1 deletion operators/operators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ func TestOperators(t *testing.T) {

opts := coraza.RuleOperatorOptions{
Arguments: data.Param,
Path: []string{"./testdata/op"},
Path: []string{"op"},
Root: os.DirFS("testdata"),
}
if err := op.Init(opts); err != nil {
t.Error(err)
Expand Down
2 changes: 1 addition & 1 deletion operators/pm_from_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type pmFromFile struct {
func (o *pmFromFile) Init(options coraza.RuleOperatorOptions) error {
path := options.Arguments

data, err := loadFromFile(path, options.Path)
data, err := loadFromFile(path, options.Path, options.Root)
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package coraza

import (
"fmt"
"io/fs"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -53,6 +54,9 @@ type RuleOperatorOptions struct {
// Path is used to store a list of possible data paths
Path []string

// Root is the root to resolve Path from.
Root fs.FS

// Datasets contains input datasets or dictionaries
Datasets map[string][]string
}
Expand Down
17 changes: 15 additions & 2 deletions seclang/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import (
"bufio"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"regexp"
"strings"

"github.com/corazawaf/coraza/v3"
"github.com/corazawaf/coraza/v3/internal/io"
"github.com/corazawaf/coraza/v3/types"
)

Expand All @@ -25,6 +27,7 @@ type Parser struct {
currentLine int
currentFile string
currentDir string
root fs.FS
includeCount int
}

Expand All @@ -37,7 +40,7 @@ func (p *Parser) FromFile(profilePath string) error {
files := []string{}
if strings.Contains(profilePath, "*") {
var err error
files, err = filepath.Glob(profilePath)
files, err = fs.Glob(p.root, profilePath)
if err != nil {
return err
}
Expand All @@ -52,7 +55,7 @@ func (p *Parser) FromFile(profilePath string) error {
p.currentFile = profilePath
lastDir := p.currentDir
p.currentDir = filepath.Dir(profilePath)
file, err := os.ReadFile(profilePath)
file, err := fs.ReadFile(p.root, profilePath)
if err != nil {
p.options.WAF.Logger.Error(err.Error())
return err
Expand Down Expand Up @@ -142,6 +145,7 @@ func (p *Parser) evaluate(data string) error {
p.options.Config.Set("last_profile_line", p.currentLine)
p.options.Config.Set("parser_config_file", p.currentFile)
p.options.Config.Set("parser_config_dir", p.currentDir)
p.options.Config.Set("parser_root", p.root)
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry don't quite follow the link, do you mean remove p.options.Config.Set/Get("working_dir") and have p.root default to working dir if user doesn't call SetRoot?

In that case, this won't work even if the user doesn't call SetRoot

coraza/waf.exe
config/rules.conf

Since .. wouldn't work. While we are discussing being OK with ../ not working with an explicit call to SetRoot, I'm not sure about when it hasn't been called at all, I guess it should be allowed though.

wd, err := os.Getwd()
if err != nil {
return err
Expand All @@ -165,6 +169,14 @@ func (p *Parser) SetCurrentDir(dir string) {
p.currentDir = dir
jcchavezs marked this conversation as resolved.
Show resolved Hide resolved
}

// SetRoot sets the root of the filesystem for resolving paths. If not set, the OS's
// filesystem is used. SetRoot with `embed.FS` can allow parsing Include and FromFile
// directives for an embedded set of rules, or zip.Reader can be used to work with
// an archive.
func (p *Parser) SetRoot(root fs.FS) {
p.root = root
}

// NewParser creates a new parser from a WAF instance
// Rules and settings will be inserted into the WAF
// rule container (RuleGroup).
Expand All @@ -175,6 +187,7 @@ func NewParser(waf *coraza.WAF) *Parser {
Config: make(types.Config),
Datasets: make(map[string][]string),
},
root: io.OSFS{},
}
return p
}
29 changes: 25 additions & 4 deletions seclang/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package seclang
import (
"bufio"
"context"
"embed"
"fmt"
"io/fs"
"os"
"path/filepath"
"testing"
Expand All @@ -15,6 +17,9 @@ import (
engine "github.com/corazawaf/coraza/v3"
)

//go:embed testdata
var testdata embed.FS

func TestInterruption(t *testing.T) {
waf := engine.NewWAF()
p := NewParser(waf)
Expand Down Expand Up @@ -65,8 +70,8 @@ func TestHardcodedSubIncludeDirective(t *testing.T) {
if err := p.FromString("Include ./testdata/includes/parent.conf"); err != nil {
t.Error(err)
}
if waf.Rules.Count() != 3 {
t.Error("Expected 3 rules loaded using include directive. Found: ", waf.Rules.Count())
if waf.Rules.Count() != 4 {
t.Error("Expected 4 rules loaded using include directive. Found: ", waf.Rules.Count())
}
}

Expand All @@ -78,8 +83,8 @@ func TestHardcodedSubIncludeDirectiveAbsolutePath(t *testing.T) {
if err := p.FromString("Include " + ruleFile); err != nil {
t.Error(err)
}
if waf.Rules.Count() != 3 {
t.Error("Expected 3 rules loaded using include directive. Found: ", waf.Rules.Count())
if waf.Rules.Count() != 4 {
t.Error("Expected 4 rules loaded using include directive. Found: ", waf.Rules.Count())
}
}

Expand Down Expand Up @@ -149,3 +154,19 @@ func TestChains(t *testing.T) {
t.Error("Chain over chain not created")
}*/
}

func TestEmbedFS(t *testing.T) {
waf := coraza.NewWAF()
p := NewParser(waf)
root, err := fs.Sub(testdata, "testdata")
if err != nil {
t.Error(err)
}
p.SetRoot(root)
if err := p.FromString("Include includes/parent.conf"); err != nil {
t.Error(err)
}
if waf.Rules.Count() != 4 {
t.Error("Expected 4 rules loaded using include directive. Found: ", waf.Rules.Count())
}
}
3 changes: 3 additions & 0 deletions seclang/rule_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ package seclang
import (
"errors"
"fmt"
"io/fs"
"regexp"
"strings"

"github.com/corazawaf/coraza/v3"
actionsmod "github.com/corazawaf/coraza/v3/actions"
"github.com/corazawaf/coraza/v3/internal/io"
utils "github.com/corazawaf/coraza/v3/internal/strings"
operators "github.com/corazawaf/coraza/v3/operators"
"github.com/corazawaf/coraza/v3/types"
Expand Down Expand Up @@ -199,6 +201,7 @@ func (p *RuleParser) ParseOperator(operator string) error {
p.options.Config.Get("parser_config_dir", "").(string),
p.options.Config.Get("working_dir", "").(string),
},
Root: p.options.Config.Get("parser_root", io.OSFS{}).(fs.FS),
}
err = opfn.Init(opts)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions seclang/testdata/includes/subinclude/pmFromFile-01.dat
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
abc
def
ghi
xxx yyy zzz
4 changes: 3 additions & 1 deletion seclang/testdata/includes/subinclude/rules3.conf
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
SecRule ARGS "@rx rule3" "id:300, phase:2, log, msg:'Rule 300', \
logdata:'Matched Data: %{TX.0} found within %{MATCHED_VAR}'"
logdata:'Matched Data: %{TX.0} found within %{MATCHED_VAR}'"

SecRule ARGS_NAMES "@pmFromFile pmFromFile-01.dat" "id:1,log"