Skip to content

Commit 64b0760

Browse files
jeffmahoneyjeffmahoney
committed
rule: Add support for removing individual rules
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.
1 parent 65ef49d commit 64b0760

File tree

4 files changed

+224
-29
lines changed

4 files changed

+224
-29
lines changed

‎audit_test.go‎

Lines changed: 128 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ func TestAuditClientSetImmutable(t *testing.T) {
797797
assert.EqualValues(t, 2, status.Enabled)
798798
}
799799

800-
func TestRuleParsing(t *testing.T) {
800+
func TestValidRuleParsing(t *testing.T) {
801801
var rules []string
802802
switch runtime.GOARCH {
803803
case "386":
@@ -833,6 +833,20 @@ func TestRuleParsing(t *testing.T) {
833833
"-a always,user -F uid=root",
834834
"-a always,task -F uid=root",
835835
"-a always,exit -S mount -F pid=1234",
836+
"-d always,exit -F arch=b64 -S execve,execveat -F key=exec",
837+
"-d never,exit -F arch=b64 -S connect,accept,bind -F key=external-access",
838+
"-W /etc/group -p wa",
839+
"-W /etc/passwd -p rx",
840+
"-W /etc/gshadow -p rwxa",
841+
"-W /tmp/test -p rwa",
842+
"-d always,exit -F arch=b64 -S open,truncate,ftruncate,creat,openat,open_by_handle_at -F exit=-EACCES -F key=access",
843+
"-d never,exit -F arch=b64 -S open,truncate,ftruncate,creat,openat,open_by_handle_at -F exit=-EPERM -F key=access",
844+
"-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",
845+
"-d always,exit -F arch=b64 -S open -F key=key -F uid=30000 -F gid=333 -F exit=-151111 -F filetype=fifo",
846+
"-d never,exclude -F msgtype=GRP_CHAUTHTOK",
847+
"-d always,user -F uid=root",
848+
"-d always,task -F uid=root",
849+
"-d always,exit -S mount -F pid=1234",
836850
}
837851
default:
838852
// Can't have multiple syscall testing as ordering of individual syscalls
@@ -851,6 +865,19 @@ func TestRuleParsing(t *testing.T) {
851865
"-a always,user -F uid=root",
852866
"-a always,task -F uid=root",
853867
"-a always,exit -S mount -F pid=1234",
868+
"-d always,exit -S execve -F key=exec",
869+
"-W /etc/group -p wa",
870+
"-W /etc/passwd -p rx",
871+
"-W /etc/gshadow -p rwxa",
872+
"-W /tmp/test -p rwa",
873+
"-d always,exit -S all -F exit=-EACCES -F key=access",
874+
"-d never,exit -S all -F exit=-EPERM -F key=access",
875+
"-d always,exit -S open -F key=admin -F uid=root -F gid=root -F exit=33 -F path=/tmp -F perm=rwxa",
876+
"-d always,exit -S open -F key=key -F uid=30000 -F gid=333 -F exit=-151111 -F filetype=fifo",
877+
"-d never,exclude -F msgtype=GRP_CHAUTHTOK",
878+
"-d always,user -F uid=root",
879+
"-d always,task -F uid=root",
880+
"-d always,exit -S mount -F pid=1234",
854881
}
855882
}
856883
t.Logf("checking %d rules", len(rules))
@@ -864,14 +891,113 @@ func TestRuleParsing(t *testing.T) {
864891
if err != nil {
865892
t.Fatal(err, msg)
866893
}
867-
cmdline, err := rule.ToCommandLine(data, true)
894+
addRule := true
895+
switch r.TypeOf() {
896+
case rule.DeleteFileWatchRuleType, rule.DeleteSyscallRuleType:
897+
addRule = false
898+
}
899+
cmdline, err := rule.ToCommandLineAddRemove(data, true, addRule)
868900
if err != nil {
869901
t.Fatal(err, msg)
870902
}
871903
assert.Equal(t, line, cmdline, msg)
872904
}
873905
}
874906

907+
func TestInvalidRuleParsing(t *testing.T) {
908+
rules := []string{
909+
"-D -a",
910+
"-D -A",
911+
"-D -d",
912+
913+
"-D -a always",
914+
"-D -A always",
915+
"-D -d always",
916+
917+
"-D -a never",
918+
"-D -A never",
919+
"-D -d never",
920+
921+
"-D -a always,task",
922+
"-D -A always,task",
923+
"-D -d always,task",
924+
925+
"-D -a always,task -w /foo/bar -p rw",
926+
"-D -a always,task -W /foo/bar -p rw",
927+
"-D -A always,task -w /foo/bar -p rw",
928+
"-D -A always,task -W /foo/bar -p rw",
929+
"-D -d always,task -w /foo/bar -p rw",
930+
"-D -d always,task -W /foo/bar -p rw",
931+
932+
"-D -a never,task",
933+
"-D -A never,task",
934+
"-D -d never,task",
935+
936+
"-D -w /foo/bar",
937+
"-D -W /foo/bar",
938+
"-D -w /foo/bar -p rw",
939+
"-D -W /foo/bar -p rw",
940+
"-D -w /foo/bar -p garbage",
941+
"-D -W /foo/bar -p garbage",
942+
943+
"-w /foo/bar -W /foo/bar",
944+
"-w /foo/bar -W /foo/bar -p rw",
945+
946+
"-w foo/bar",
947+
"-W foo/bar",
948+
"-w foo/bar -p rw",
949+
"-W foo/bar -p rw",
950+
951+
"-w foo/bar -S 42",
952+
"-W foo/bar -S 42",
953+
954+
"-w foo/bar -W foo/bar",
955+
"-w foo/bar -W foo/bar -p rw",
956+
957+
"-w foo/bar -W foo/bar -S 42",
958+
959+
"-w /foo/bar -F uid=100",
960+
"-W /foo/bar -F uid=100",
961+
962+
"-w /foo/bar -S 42",
963+
"-W /foo/bar -S 42",
964+
965+
"-w /foo/bar -F uid=100",
966+
"-W /foo/bar -F uid=100",
967+
968+
"-w /foo/bar -C auid!=uid",
969+
"-W /foo/bar -C auid!=uid",
970+
971+
"-a always,exit -w /foo/bar -p rw",
972+
"-a always,exit -W /foo/bar -p rw",
973+
"-A always,exit -w /foo/bar -p rw",
974+
"-A always,exit -W /foo/bar -p rw",
975+
976+
"-a always,exit -w /foo/bar",
977+
"-a always,exit -W /foo/bar",
978+
"-A always,exit -w /foo/bar",
979+
"-A always,exit -W /foo/bar",
980+
981+
}
982+
983+
t.Logf("checking %d rules", len(rules))
984+
for idx, line := range rules {
985+
r, err := flags.Parse(line)
986+
if err != nil {
987+
t.Log(err)
988+
continue
989+
}
990+
991+
_, err = rule.Build(r)
992+
if err != nil {
993+
t.Log(err)
994+
continue
995+
}
996+
997+
t.Errorf("parsing line #%d: `%s' should have failed", idx, line)
998+
}
999+
}
1000+
8751001
func extractDecimalNumber(s []int8, pos int) (value, nextPos int) {
8761002
for value = 0; ; pos++ {
8771003
c := s[pos]

‎rule/flags/flags.go‎

Lines changed: 75 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,19 @@ func Parse(s string) (rule.Rule, error) {
5858
Type: rule.DeleteAllRuleType,
5959
Keys: ruleFlagSet.Key,
6060
}
61-
case rule.FileWatchRuleType:
62-
r = &rule.FileWatchRule{
63-
Type: rule.FileWatchRuleType,
61+
case rule.FileWatchRuleType, rule.DeleteFileWatchRuleType:
62+
fileWatchRule := &rule.FileWatchRule{
63+
Type: ruleFlagSet.Type,
6464
Path: ruleFlagSet.Path,
6565
Permissions: ruleFlagSet.Permissions,
6666
Keys: ruleFlagSet.Key,
6767
}
68-
case rule.AppendSyscallRuleType, rule.PrependSyscallRuleType:
68+
r = fileWatchRule
69+
70+
if ruleFlagSet.Type == rule.DeleteFileWatchRuleType {
71+
fileWatchRule.Path = ruleFlagSet.DeletePath
72+
}
73+
case rule.AppendSyscallRuleType, rule.PrependSyscallRuleType, rule.DeleteSyscallRuleType:
6974
syscallRule := &rule.SyscallRule{
7075
Type: ruleFlagSet.Type,
7176
Filters: ruleFlagSet.Filters,
@@ -74,12 +79,16 @@ func Parse(s string) (rule.Rule, error) {
7479
}
7580
r = syscallRule
7681

77-
if ruleFlagSet.Type == rule.AppendSyscallRuleType {
82+
switch ruleFlagSet.Type {
83+
case rule.AppendSyscallRuleType:
7884
syscallRule.List = ruleFlagSet.Append.List
7985
syscallRule.Action = ruleFlagSet.Append.Action
80-
} else if ruleFlagSet.Type == rule.PrependSyscallRuleType {
86+
case rule.PrependSyscallRuleType:
8187
syscallRule.List = ruleFlagSet.Prepend.List
8288
syscallRule.Action = ruleFlagSet.Prepend.Action
89+
case rule.DeleteSyscallRuleType:
90+
syscallRule.List = ruleFlagSet.Delete.List
91+
syscallRule.Action = ruleFlagSet.Delete.Action
8392
}
8493
default:
8594
return nil, fmt.Errorf("unknown rule type: %v", ruleFlagSet.Type)
@@ -99,11 +108,13 @@ type ruleFlagSet struct {
99108
// Audit Rule
100109
Prepend addFlag // -A Prepend rule (list,action) or (action,list).
101110
Append addFlag // -a Append rule (list,action) or (action,list).
111+
Delete addFlag // [-d] delete single rule
102112
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]
103113
Syscalls stringList // -S Syscall name or number or "all". Value can be comma-separated.
104114

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

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

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

141154
func (r *ruleFlagSet) validate() error {
142155
var (
143-
deleteAll uint8
144-
fileWatch uint8
145-
syscall uint8
156+
deleteAll uint8
157+
fileWatchFlags uint8
158+
addFileWatch uint8
159+
deleteFileWatch uint8
160+
syscallFlags uint8
161+
addSyscall uint8
162+
deleteSyscall uint8
146163
)
147164

148165
r.flagSet.Visit(func(f *flag.Flag) {
149166
switch f.Name {
150167
case "D":
151168
deleteAll = 1
152-
case "w", "p":
153-
fileWatch = 1
154-
case "a", "A", "C", "F", "S":
155-
syscall = 1
169+
case "p":
170+
fileWatchFlags = 1
171+
case "w":
172+
addFileWatch = 1
173+
case "W":
174+
deleteFileWatch = 1
175+
case "S", "F", "C":
176+
syscallFlags = 1
177+
case "a", "A":
178+
addSyscall = 1
179+
case "d":
180+
deleteSyscall = 1
156181
}
157182
})
158183

159184
// Test for mutual exclusivity.
160-
switch deleteAll + fileWatch + syscall {
185+
switch deleteAll + addFileWatch + deleteFileWatch + addSyscall + deleteSyscall {
161186
case 0:
162187
return errors.New("missing an operation flag (add or delete rule)")
163188
case 1:
164189
switch {
165190
case deleteAll > 0:
166191
r.Type = rule.DeleteAllRuleType
167-
case fileWatch > 0:
192+
case addFileWatch > 0:
168193
r.Type = rule.FileWatchRuleType
169-
case syscall > 0:
194+
case deleteFileWatch > 0:
195+
r.Type = rule.DeleteFileWatchRuleType
196+
case addSyscall > 0:
170197
r.Type = rule.AppendSyscallRuleType
198+
case deleteSyscall > 0:
199+
r.Type = rule.DeleteSyscallRuleType
200+
default:
201+
return fmt.Errorf("unknown state")
171202
}
172203
default:
173204
ops := make([]string, 0, 3)
174205
if deleteAll > 0 {
175206
ops = append(ops, "delete all [-D]")
176207
}
177-
if fileWatch > 0 {
178-
ops = append(ops, "file watch [-w|-p]")
208+
if addFileWatch > 0 {
209+
ops = append(ops, "file watch [-w]")
210+
}
211+
if deleteFileWatch > 0 {
212+
ops = append(ops, "delete file watch [-W]")
179213
}
180-
if syscall > 0 {
181-
ops = append(ops, "audit rule [-a|-A|-S|-C|-F]")
214+
if addSyscall > 0 {
215+
ops = append(ops, "audit rule [-a|-A]")
182216
}
217+
if deleteSyscall > 0 {
218+
ops = append(ops, "delete audit rule [-d]")
219+
}
220+
183221
return fmt.Errorf("mutually exclusive flags uses together (%v)",
184222
strings.Join(ops, " and "))
185223
}
186224

187-
if syscall > 0 {
225+
if (addFileWatch+deleteFileWatch) > 0 && syscallFlags > 0 {
226+
return fmt.Errorf("audit rule [-F|-S|-C] flags cannot be used with file watches")
227+
}
228+
229+
if (addSyscall+deleteSyscall) > 0 && fileWatchFlags > 0 {
230+
return fmt.Errorf("file watch [-p] flags cannot be used with syscall rules")
231+
}
232+
233+
if deleteAll > 0 && syscallFlags > 0 {
234+
return fmt.Errorf("audit rule [-F|-S|-C] flags cannot be used when deleting all rules")
235+
}
236+
237+
if deleteAll > 0 && fileWatchFlags > 0 {
238+
return fmt.Errorf("file watch permission [-p] flags cannot be used when deleting all rules")
239+
}
240+
241+
if addSyscall > 0 {
188242
var zero addFlag
189243
if r.Prepend == zero && r.Append == zero {
190244
return errors.New("audit rules must specify either [-A] or [-a]")

‎rule/rule.go‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ func Build(rule Rule) (WireFormat, error) {
100100
return ard.toWireFormat(), nil
101101
}
102102

103+
func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
104+
return ToCommandLineAddRemove(wf, resolveIds, true)
105+
}
106+
103107
// ToCommandLine decodes a WireFormat into a command-line rule.
104108
// When resolveIds is set, it tries to resolve the argument to UIDs, GIDs,
105109
// file_type fields.
@@ -110,7 +114,7 @@ func Build(rule Rule) (WireFormat, error) {
110114
// auditctl always prints "b64" or "b32" even for architectures other than
111115
// the current machine. This is misleading, so this code will print the actual
112116
// architecture.
113-
func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
117+
func ToCommandLineAddRemove(wf WireFormat, resolveIds, addRule bool) (rule string, err error) {
114118
ar, err := fromWireFormat(wf)
115119
if err != nil {
116120
return "", fmt.Errorf("failed to parse wire format: %w", err)
@@ -162,7 +166,15 @@ func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
162166
}
163167
}
164168
if !extraFields {
165-
arguments := []string{"-w", path, "-p", permission(r.values[permIdx]).String()}
169+
addRemove := "-w"
170+
if !addRule {
171+
addRemove = "-W"
172+
}
173+
174+
arguments := []string{
175+
addRemove, path, "-p",
176+
permission(r.values[permIdx]).String(),
177+
}
166178
if len(key) > 0 {
167179
arguments = append(arguments, "-k", key)
168180
}
@@ -171,12 +183,13 @@ func ToCommandLine(wf WireFormat, resolveIds bool) (rule string, err error) {
171183
}
172184

173185
// Parse rule as syscall type
174-
175-
arguments := []string{
176-
"-a",
177-
fmt.Sprintf("%s,%s", act, list),
186+
addRemove := "-a"
187+
if !addRule {
188+
addRemove = "-d"
178189
}
179190

191+
arguments := []string{addRemove, fmt.Sprintf("%s,%s", act, list)}
192+
180193
// Parse arch field first, if present
181194
// Here there is a significant difference to what auditctl does.
182195
// Auditctl will allow to install a rule for a different platform

0 commit comments

Comments
 (0)