Skip to content

Commit

Permalink
rule: Add support for removing individual rules
Browse files Browse the repository at this point in the history 
We currently don't handle the '-d' or '-W' options that would remove
list rules or file watches.  This commit adds support to handle those
properly.  rule.ToCommandLine still returns the expected result, but
I've added a rule.ToCommandLineAddRemove that takes a bool indicating
whether the rule would be added or removed.  This was required to do
testing of deletion rules.
  • Loading branch information
@jeffmahoney
jeffmahoney committed Sep 8, 2023
1 parent 65ef49d commit 64b0760
Show file tree
Hide file tree
Showing 4 changed files with 224 additions and 29 deletions.
130 changes: 128 additions & 2 deletions audit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ func TestAuditClientSetImmutable(t *testing.T) {
assert.EqualValues(t, 2, status.Enabled)
}

func TestRuleParsing(t *testing.T) {
func TestValidRuleParsing(t *testing.T) {
var rules []string
switch runtime.GOARCH {
case "386":
Expand Down Expand Up @@ -833,6 +833,20 @@ func TestRuleParsing(t *testing.T) {
"-a always,user -F uid=root",
"-a always,task -F uid=root",
"-a always,exit -S mount -F pid=1234",
"-d always,exit -F arch=b64 -S execve,execveat -F key=exec",
"-d never,exit -F arch=b64 -S connect,accept,bind -F key=external-access",
"-W /etc/group -p wa",
"-W /etc/passwd -p rx",
"-W /etc/gshadow -p rwxa",
"-W /tmp/test -p rwa",
"-d always,exit -F arch=b64 -S open,truncate,ftruncate,creat,openat,open_by_handle_at -F exit=-EACCES -F key=access",
"-d never,exit -F arch=b64 -S open,truncate,ftruncate,creat,openat,open_by_handle_at -F exit=-EPERM -F key=access",
"-d always,exit -F arch=b32 -S open -F key=admin -F uid=root -F gid=root -F exit=33 -F path=/tmp -F perm=rwxa",
"-d always,exit -F arch=b64 -S open -F key=key -F uid=30000 -F gid=333 -F exit=-151111 -F filetype=fifo",
"-d never,exclude -F msgtype=GRP_CHAUTHTOK",
"-d always,user -F uid=root",
"-d always,task -F uid=root",
"-d always,exit -S mount -F pid=1234",
}
default:
// Can't have multiple syscall testing as ordering of individual syscalls
Expand All @@ -851,6 +865,19 @@ func TestRuleParsing(t *testing.T) {
"-a always,user -F uid=root",
"-a always,task -F uid=root",
"-a always,exit -S mount -F pid=1234",
"-d always,exit -S execve -F key=exec",
"-W /etc/group -p wa",
"-W /etc/passwd -p rx",
"-W /etc/gshadow -p rwxa",
"-W /tmp/test -p rwa",
"-d always,exit -S all -F exit=-EACCES -F key=access",
"-d never,exit -S all -F exit=-EPERM -F key=access",
"-d always,exit -S open -F key=admin -F uid=root -F gid=root -F exit=33 -F path=/tmp -F perm=rwxa",
"-d always,exit -S open -F key=key -F uid=30000 -F gid=333 -F exit=-151111 -F filetype=fifo",
"-d never,exclude -F msgtype=GRP_CHAUTHTOK",
"-d always,user -F uid=root",
"-d always,task -F uid=root",
"-d always,exit -S mount -F pid=1234",
}
}
t.Logf("checking %d rules", len(rules))
Expand All @@ -864,14 +891,113 @@ func TestRuleParsing(t *testing.T) {
if err != nil {
t.Fatal(err, msg)
}
cmdline, err := rule.ToCommandLine(data, true)
addRule := true
switch r.TypeOf() {
case rule.DeleteFileWatchRuleType, rule.DeleteSyscallRuleType:
addRule = false
}
cmdline, err := rule.ToCommandLineAddRemove(data, true, addRule)
if err != nil {
t.Fatal(err, msg)
}
assert.Equal(t, line, cmdline, msg)
}
}

func TestInvalidRuleParsing(t *testing.T) {
rules := []string{
"-D -a",
"-D -A",
"-D -d",

"-D -a always",
"-D -A always",
"-D -d always",

"-D -a never",
"-D -A never",
"-D -d never",

"-D -a always,task",
"-D -A always,task",
"-D -d always,task",

"-D -a always,task -w /foo/bar -p rw",
"-D -a always,task -W /foo/bar -p rw",
"-D -A always,task -w /foo/bar -p rw",
"-D -A always,task -W /foo/bar -p rw",
"-D -d always,task -w /foo/bar -p rw",
"-D -d always,task -W /foo/bar -p rw",

"-D -a never,task",
"-D -A never,task",
"-D -d never,task",

"-D -w /foo/bar",
"-D -W /foo/bar",
"-D -w /foo/bar -p rw",
"-D -W /foo/bar -p rw",
"-D -w /foo/bar -p garbage",
"-D -W /foo/bar -p garbage",

"-w /foo/bar -W /foo/bar",
"-w /foo/bar -W /foo/bar -p rw",

"-w foo/bar",
"-W foo/bar",
"-w foo/bar -p rw",
"-W foo/bar -p rw",

"-w foo/bar -S 42",
"-W foo/bar -S 42",

"-w foo/bar -W foo/bar",
"-w foo/bar -W foo/bar -p rw",

"-w foo/bar -W foo/bar -S 42",

"-w /foo/bar -F uid=100",
"-W /foo/bar -F uid=100",

"-w /foo/bar -S 42",
"-W /foo/bar -S 42",

"-w /foo/bar -F uid=100",
"-W /foo/bar -F uid=100",

"-w /foo/bar -C auid!=uid",
"-W /foo/bar -C auid!=uid",

"-a always,exit -w /foo/bar -p rw",
"-a always,exit -W /foo/bar -p rw",
"-A always,exit -w /foo/bar -p rw",
"-A always,exit -W /foo/bar -p rw",

"-a always,exit -w /foo/bar",
"-a always,exit -W /foo/bar",
"-A always,exit -w /foo/bar",
"-A always,exit -W /foo/bar",

}

t.Logf("checking %d rules", len(rules))
for idx, line := range rules {
r, err := flags.Parse(line)
if err != nil {
t.Log(err)
continue
}

_, err = rule.Build(r)
if err != nil {
t.Log(err)
continue
}

t.Errorf("parsing line #%d: `%s' should have failed", idx, line)
}
}

func extractDecimalNumber(s []int8, pos int) (value, nextPos int) {
for value = 0; ; pos++ {
c := s[pos]
Expand Down
96 changes: 75 additions & 21 deletions rule/flags/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,19 @@ func Parse(s string) (rule.Rule, error) {
Type: rule.DeleteAllRuleType,
Keys: ruleFlagSet.Key,
}
case rule.FileWatchRuleType:
r = &rule.FileWatchRule{
Type: rule.FileWatchRuleType,
case rule.FileWatchRuleType, rule.DeleteFileWatchRuleType:
fileWatchRule := &rule.FileWatchRule{
Type: ruleFlagSet.Type,
Path: ruleFlagSet.Path,
Permissions: ruleFlagSet.Permissions,
Keys: ruleFlagSet.Key,
}
case rule.AppendSyscallRuleType, rule.PrependSyscallRuleType:
r = fileWatchRule

if ruleFlagSet.Type == rule.DeleteFileWatchRuleType {
fileWatchRule.Path = ruleFlagSet.DeletePath
}
case rule.AppendSyscallRuleType, rule.PrependSyscallRuleType, rule.DeleteSyscallRuleType:
syscallRule := &rule.SyscallRule{
Type: ruleFlagSet.Type,
Filters: ruleFlagSet.Filters,
Expand All @@ -74,12 +79,16 @@ func Parse(s string) (rule.Rule, error) {
}
r = syscallRule

if ruleFlagSet.Type == rule.AppendSyscallRuleType {
switch ruleFlagSet.Type {
case rule.AppendSyscallRuleType:
syscallRule.List = ruleFlagSet.Append.List
syscallRule.Action = ruleFlagSet.Append.Action
} else if ruleFlagSet.Type == rule.PrependSyscallRuleType {
case rule.PrependSyscallRuleType:
syscallRule.List = ruleFlagSet.Prepend.List
syscallRule.Action = ruleFlagSet.Prepend.Action
case rule.DeleteSyscallRuleType:
syscallRule.List = ruleFlagSet.Delete.List
syscallRule.Action = ruleFlagSet.Delete.Action
}
default:
return nil, fmt.Errorf("unknown rule type: %v", ruleFlagSet.Type)
Expand All @@ -99,11 +108,13 @@ type ruleFlagSet struct {
// Audit Rule
Prepend addFlag // -A Prepend rule (list,action) or (action,list).
Append addFlag // -a Append rule (list,action) or (action,list).
Delete addFlag // [-d] delete single rule
Filters filterList // -F [n=v | n!=v | n<v | n>v | n<=v | n>=v | n&v | n&=v] OR -C [n=v | n!=v]
Syscalls stringList // -S Syscall name or number or "all". Value can be comma-separated.

// Filepath watch (can be done more expressively using syscalls)
Path string // -w Path for filesystem watch (no wildcards).
DeletePath string // -W Path for filesystem watch to remove (no wildcards).
Permissions fileAccessTypeFlags // -p [r|w|x|a] Permission filter.

Key stringList // -k Key(s) to associate with the rule.
Expand All @@ -120,11 +131,13 @@ func newRuleFlagSet() *ruleFlagSet {
rule.flagSet.BoolVar(&rule.DeleteAll, "D", false, "delete all")
rule.flagSet.Var(&rule.Append, "a", "append rule")
rule.flagSet.Var(&rule.Prepend, "A", "prepend rule")
rule.flagSet.Var(&rule.Delete, "d", "delete rule")
rule.flagSet.Var((*interFieldFilterList)(&rule.Filters), "C", "comparison filter")
rule.flagSet.Var((*valueFilterList)(&rule.Filters), "F", "filter")
rule.flagSet.Var(&rule.Syscalls, "S", "syscall name, number, or 'all'")
rule.flagSet.Var(&rule.Permissions, "p", "access type - r=read, w=write, x=execute, a=attribute change")
rule.flagSet.StringVar(&rule.Path, "w", "", "path to watch, no wildcards")
rule.flagSet.StringVar(&rule.DeletePath, "W", "", "path to unwatch, no wildcards")
rule.flagSet.Var(&rule.Key, "k", "key")

return rule
Expand All @@ -140,51 +153,92 @@ func (r *ruleFlagSet) Usage() string {

func (r *ruleFlagSet) validate() error {
var (
deleteAll uint8
fileWatch uint8
syscall uint8
deleteAll uint8
fileWatchFlags uint8
addFileWatch uint8
deleteFileWatch uint8
syscallFlags uint8
addSyscall uint8
deleteSyscall uint8
)

r.flagSet.Visit(func(f *flag.Flag) {
switch f.Name {
case "D":
deleteAll = 1
case "w", "p":
fileWatch = 1
case "a", "A", "C", "F", "S":
syscall = 1
case "p":
fileWatchFlags = 1
case "w":
addFileWatch = 1
case "W":
deleteFileWatch = 1
case "S", "F", "C":
syscallFlags = 1
case "a", "A":
addSyscall = 1
case "d":
deleteSyscall = 1
}
})

// Test for mutual exclusivity.
switch deleteAll + fileWatch + syscall {
switch deleteAll + addFileWatch + deleteFileWatch + addSyscall + deleteSyscall {
case 0:
return errors.New("missing an operation flag (add or delete rule)")
case 1:
switch {
case deleteAll > 0:
r.Type = rule.DeleteAllRuleType
case fileWatch > 0:
case addFileWatch > 0:
r.Type = rule.FileWatchRuleType
case syscall > 0:
case deleteFileWatch > 0:
r.Type = rule.DeleteFileWatchRuleType
case addSyscall > 0:
r.Type = rule.AppendSyscallRuleType
case deleteSyscall > 0:
r.Type = rule.DeleteSyscallRuleType
default:
return fmt.Errorf("unknown state")
}
default:
ops := make([]string, 0, 3)
if deleteAll > 0 {
ops = append(ops, "delete all [-D]")
}
if fileWatch > 0 {
ops = append(ops, "file watch [-w|-p]")
if addFileWatch > 0 {
ops = append(ops, "file watch [-w]")
}
if deleteFileWatch > 0 {
ops = append(ops, "delete file watch [-W]")
}
if syscall > 0 {
ops = append(ops, "audit rule [-a|-A|-S|-C|-F]")
if addSyscall > 0 {
ops = append(ops, "audit rule [-a|-A]")
}
if deleteSyscall > 0 {
ops = append(ops, "delete audit rule [-d]")
}

return fmt.Errorf("mutually exclusive flags uses together (%v)",
strings.Join(ops, " and "))
}

if syscall > 0 {
if (addFileWatch+deleteFileWatch) > 0 && syscallFlags > 0 {
return fmt.Errorf("audit rule [-F|-S|-C] flags cannot be used with file watches")
}

if (addSyscall+deleteSyscall) > 0 && fileWatchFlags > 0 {
return fmt.Errorf("file watch [-p] flags cannot be used with syscall rules")
}

if deleteAll > 0 && syscallFlags > 0 {
return fmt.Errorf("audit rule [-F|-S|-C] flags cannot be used when deleting all rules")
}

if deleteAll > 0 && fileWatchFlags > 0 {
return fmt.Errorf("file watch permission [-p] flags cannot be used when deleting all rules")
}

if addSyscall > 0 {
var zero addFlag
if r.Prepend == zero && r.Append == zero {
return errors.New("audit rules must specify either [-A] or [-a]")
Expand Down
25 changes: 19 additions & 6 deletions rule/rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ func Build(rule Rule) (WireFormat, error) {
return ard.toWireFormat(), nil
}

func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
return ToCommandLineAddRemove(wf, resolveIds, true)
}

// ToCommandLine decodes a WireFormat into a command-line rule.
// When resolveIds is set, it tries to resolve the argument to UIDs, GIDs,
// file_type fields.
Expand All @@ -110,7 +114,7 @@ func Build(rule Rule) (WireFormat, error) {
// auditctl always prints "b64" or "b32" even for architectures other than
// the current machine. This is misleading, so this code will print the actual
// architecture.
func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
func ToCommandLineAddRemove(wf WireFormat, resolveIds, addRule bool) (rule string, err error) {
ar, err := fromWireFormat(wf)
if err != nil {
return "", fmt.Errorf("failed to parse wire format: %w", err)
Expand Down Expand Up @@ -162,7 +166,15 @@ func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
}
}
if !extraFields {
arguments := []string{"-w", path, "-p", permission(r.values[permIdx]).String()}
addRemove := "-w"
if !addRule {
addRemove = "-W"
}

arguments := []string{
addRemove, path, "-p",
permission(r.values[permIdx]).String(),
}
if len(key) > 0 {
arguments = append(arguments, "-k", key)
}
Expand All @@ -171,12 +183,13 @@ func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
}

// Parse rule as syscall type

arguments := []string{
"-a",
fmt.Sprintf("%s,%s", act, list),
addRemove := "-a"
if !addRule {
addRemove = "-d"
}

arguments := []string{addRemove, fmt.Sprintf("%s,%s", act, list)}

// Parse arch field first, if present
// Here there is a significant difference to what auditctl does.
// Auditctl will allow to install a rule for a different platform
Expand Down
Loading

0 comments on commit 64b0760

Please sign in to comment.