Use go-shellquote to split rules into arguments (#115)

The rule implementation naively split rules using strings.Fields, but this did not
handle arguments that were quoted in cases where the arguments themselves
contain spaces.

Now go-shellquote is used to split the arguments before parsing.

Fixes #114

CHANGELOG.md

@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Fix change in behaviour that causes error when unmarshaling `AuditStatus` with a short buffer. [#110](https://github.com/elastic/go-libaudit/pull/110)
 - Reduce heap allocations when parsing and enriching auditd events. [#111](https://github.com/elastic/go-libaudit/pull/111)
 - Relax short buffer requirement further to allow for kernels that do not support the backlog wait feature. [#113](https://github.com/elastic/go-libaudit/pull/113)
+- Fix parsing of audit rules where arguments are quoted (like file paths containing spaces). [#115](https://github.com/elastic/go-libaudit/pull/115)
 - Fix minimum `AuditStatus` length so that library can support kernels from 2.6.32. [#119](https://github.com/elastic/go-libaudit/pull/119)
 
 ### Removed

go.mod

@@ -3,6 +3,7 @@ module github.com/elastic/go-libaudit/v2
 go 1.16
 
 require (
+	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
 	github.com/stretchr/testify v1.7.0
 	go.uber.org/multierr v1.7.0
 	golang.org/x/sys v0.0.0-20200615200032-f1bc736245b1

go.sum

@@ -1,6 +1,8 @@
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
+github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

rule/flags/flags.go

@@ -28,15 +28,22 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/kballard/go-shellquote"
+
 	"github.com/elastic/go-libaudit/v2/rule"
 )
 
 // Parse parses an audit rule specified using flags. It can parse delete all
 // commands (-D), file watch rules (-w), and syscall rules (-a or -A).
-func Parse(args string) (rule.Rule, error) {
+func Parse(s string) (rule.Rule, error) {
+	args, err := shellquote.Split(s)
+	if err != nil {
+		return nil, err
+	}
+
 	// Parse the flags.
 	ruleFlagSet := newRuleFlagSet()
-	if err := ruleFlagSet.flagSet.Parse(strings.Fields(args)); err != nil {
+	if err := ruleFlagSet.flagSet.Parse(args); err != nil {
 		return nil, err
 	}
 	if err := ruleFlagSet.validate(); err != nil {

rule/gen_testdata_test.go

@@ -30,10 +30,10 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"testing"
 
+	"github.com/kballard/go-shellquote"
 	"gopkg.in/yaml.v2"
 
 	"github.com/elastic/go-libaudit/v2"
@@ -145,9 +145,8 @@ func auditctlExec(t testing.TB, command string) (string, []byte) {
 	defer deleteRules(t, client)
 
 	// Replace paths with ones in a temp dir for test environment consistency.
-	command = makePaths(t, tempDir, command)
+	args := makePaths(t, tempDir, command)
 
-	args := strings.Fields(command)
 	_, err = exec.Command("auditctl", args...).Output()
 	if err != nil {
 		var exitErr *exec.ExitError
@@ -166,39 +165,60 @@ func auditctlExec(t testing.TB, command string) (string, []byte) {
 		t.Fatalf("expected 1 rule but got %d", len(rules))
 	}
 
-	return command, rules[0]
+	return shellquote.Join(args...), rules[0]
 }
 
 // makePaths extracts any paths from the command, creates the path as either
 // a regular file or directory, then updates the paths to point to the one
-// created for the test. It returns the updated command that contains the test
-// paths.
-func makePaths(t testing.TB, tmpDir, rule string) string {
-	re := regexp.MustCompile(`(-w |dir=|path=)/(\S+)`)
-	matches := re.FindAllStringSubmatch(rule, -1)
-	for _, match := range matches {
-		path := match[2]
-		realPath := filepath.Join(tmpDir, path)
+// created for the test. It returns the updated command arguments which contain
+// the test paths.
+func makePaths(t testing.TB, tmpDir, rule string) []string {
+	args, err := shellquote.Split(rule)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for i, arg := range args {
+		var prefix, path string
+		switch {
+		case arg == "-w":
+			path = args[i+1]
+		case strings.HasPrefix(arg, "dir="):
+			prefix = "dir="
+			path = strings.TrimPrefix(arg, prefix)
+		case strings.HasPrefix(arg, "path="):
+			prefix = "path="
+			path = strings.TrimPrefix(arg, prefix)
+		default:
+			continue
+		}
+
+		testPath := filepath.Join(tmpDir, path)
 
 		if strings.HasSuffix(path, "/") {
 			// Treat paths with trailing slashes as a directory to monitor.
-			if err := os.MkdirAll(realPath, 0o700); err != nil {
+			if err := os.MkdirAll(testPath, 0o700); err != nil {
 				t.Fatal(err)
 			}
 		} else {
 			// Touch a file.
-			dir := filepath.Dir(realPath)
+			dir := filepath.Dir(testPath)
 			if err := os.MkdirAll(dir, 0o700); err != nil {
 				t.Fatal(err)
 			}
-			if err := ioutil.WriteFile(realPath, nil, 0o600); err != nil {
+			if err := ioutil.WriteFile(testPath, nil, 0o600); err != nil {
 				t.Fatal(err)
 			}
 		}
+
+		if prefix == "" {
+			args[i+1] = testPath
+		} else {
+			args[i] = prefix + testPath
+		}
 	}
 
-	substitution := "$1" + filepath.Join(tmpDir, "$2")
-	return re.ReplaceAllString(rule, substitution)
+	return args
 }
 
 func deleteRules(t testing.TB, client *libaudit.AuditClient) {

rule/testdata/01-file-watch-spaces.rules

@@ -0,0 +1,3 @@
+-w "/folder with space" -p wa -k path-double-quoted
+
+-w '/folder with space' -p wa -k path-single-quoted

rule/testdata/01-file-watch-spaces.rules.golden.yml

@@ -0,0 +1,49 @@
+# Linux 7fa0ce96f3a7 5.10.76-linuxkit #1 SMP PREEMPT Mon Nov 8 11:22:26 UTC 2021 aarch64 GNU/Linux
+# auditctl version 3.0
+rules:
+- flags: -w '/tmp/audit-test/folder with space' -p wa -k path-double-quoted
+  bytes: !!binary |
+    BAAAAAIAAAADAAAA//////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    ////8AAGkAAABqAAAA0gAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAh
+    AAAACgAAABIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAE
+    AAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADMAAAAvdG1wL2F1ZGl0
+    LXRlc3QvZm9sZGVyIHdpdGggc3BhY2VwYXRoLWRvdWJsZS1xdW90ZWQA
+- flags: -w '/tmp/audit-test/folder with space' -p wa -k path-single-quoted
+  bytes: !!binary |
+    BAAAAAIAAAADAAAA//////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    //////////////////////////////////////////////////////////////////////
+    ////8AAGkAAABqAAAA0gAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAh
+    AAAACgAAABIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAE
+    AAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
+    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADMAAAAvdG1wL2F1ZGl0
+    LXRlc3QvZm9sZGVyIHdpdGggc3BhY2VwYXRoLXNpbmdsZS1xdW90ZWQA