mir/apparmor
2
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-22 01:57:43 +00:00
Code Issues Releases Wiki Activity

Get rid of is_covered_aare_compat()

Browse Source 
This function was introduced as a temporary (ahem...) solution in
95404bb2f390ef743e289dc1da9a381bad168fbe but was never really correct.
It checked against other_value.regex (as a string!) and, while this was
somewhat generous in the results, could have unintended side effects.

Better error out on the safe side and add/keep a few superfluous rules
than having a wrong match in is_covered() and miss to add/keep a rule
that would be needed.

The perfect solution would be to really compare one AARE against the
other as the parser does. I'm not too keen to implement this in python,
and will wait until someone provides this function (which the parser
already has) via libapparmor ;-)
This commit is contained in:
Christian Boltz 2020-04-13 15:13:12 +02:00
parent acafe9de82
commit ef0d675824
No known key found for this signature in database
GPG Key ID: C6A682EA63C82F1C
7 changed files with 18 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
utils/apparmor/rule/__init__.py
View File

@ -192,15 +192,6 @@ class BaseRule(object):
# still here? -> then it is covered
return True
def _is_covered_aare_compat(self, self_value, self_all, other_value, other_all, cond_name):
'''check if other_* is covered by self_* - for AARE
Note: this function checks against other_value.regex, which is not really correct, but avoids overly strict results when matching one regex against another
'''
if type(other_value) == AARE:
other_value = other_value.regex
return self._is_covered_aare(self_value, self_all, other_value, other_all, cond_name)
def _is_covered_aare(self, self_value, self_all, other_value, other_all, cond_name):
'''check if other_* is covered by self_* - for AARE'''

14
utils/apparmor/rule/dbus.py
View File

@ -240,25 +240,25 @@ class DbusRule(BaseRule):
if not self._is_covered_list(self.access, self.all_access, other_rule.access, other_rule.all_access, 'access'):
return False
if not self._is_covered_aare_compat(self.bus, self.all_buses, other_rule.bus, other_rule.all_buses, 'bus'):
if not self._is_covered_aare(self.bus, self.all_buses, other_rule.bus, other_rule.all_buses, 'bus'):
return False
if not self._is_covered_aare_compat(self.path, self.all_paths, other_rule.path, other_rule.all_paths, 'path'):
if not self._is_covered_aare(self.path, self.all_paths, other_rule.path, other_rule.all_paths, 'path'):
return False
if not self._is_covered_aare_compat(self.name, self.all_names, other_rule.name, other_rule.all_names, 'name'):
if not self._is_covered_aare(self.name, self.all_names, other_rule.name, other_rule.all_names, 'name'):
return False
if not self._is_covered_aare_compat(self.interface, self.all_interfaces, other_rule.interface, other_rule.all_interfaces, 'interface'):
if not self._is_covered_aare(self.interface, self.all_interfaces, other_rule.interface, other_rule.all_interfaces, 'interface'):
return False
if not self._is_covered_aare_compat(self.member, self.all_members, other_rule.member, other_rule.all_members, 'member'):
if not self._is_covered_aare(self.member, self.all_members, other_rule.member, other_rule.all_members, 'member'):
return False
if not self._is_covered_aare_compat(self.peername, self.all_peernames, other_rule.peername, other_rule.all_peernames, 'peername'):
if not self._is_covered_aare(self.peername, self.all_peernames, other_rule.peername, other_rule.all_peernames, 'peername'):
return False
if not self._is_covered_aare_compat(self.peerlabel, self.all_peerlabels, other_rule.peerlabel, other_rule.all_peerlabels, 'peerlabel'):
if not self._is_covered_aare(self.peerlabel, self.all_peerlabels, other_rule.peerlabel, other_rule.all_peerlabels, 'peerlabel'):
return False
# still here? -> then it is covered

2
utils/apparmor/rule/ptrace.py
View File

@ -138,7 +138,7 @@ class PtraceRule(BaseRule):
if not self._is_covered_list(self.access, self.all_access, other_rule.access, other_rule.all_access, 'access'):
return False
if not self._is_covered_aare_compat(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'):
if not self._is_covered_aare(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'):
return False
# still here? -> then it is covered

2
utils/apparmor/rule/signal.py
View File

@ -188,7 +188,7 @@ class SignalRule(BaseRule):
if not self._is_covered_list(self.signal, self.all_signals, other_rule.signal, other_rule.all_signals, 'signal'):
return False
if not self._is_covered_aare_compat(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'):
if not self._is_covered_aare(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'):
return False
# still here? -> then it is covered

6
utils/test/test-dbus.py
View File

@ -606,9 +606,9 @@ class DbusCoveredTest_09(DbusCoveredTest):
('dbus,' , [ False , False , False , False ]),
('dbus send,' , [ False , False , False , False ]),
('dbus send member=/foo/bar,' , [ False , False , True , True ]),
('dbus send member=/foo/*,' , [ False , False , True , True ]),
('dbus send member=/**,' , [ False , False , True , True ]),
('dbus send member=/what/*,' , [ False , False , True , True ]),
('dbus send member=/foo/*,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('dbus send member=/**,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('dbus send member=/what/*,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('dbus member=/foo/bar,' , [ False , False , False , False ]),
('dbus send, # comment' , [ False , False , False , False ]),
('allow dbus send,' , [ False , False , False , False ]),

6
utils/test/test-ptrace.py
View File

@ -368,9 +368,9 @@ class PtraceCoveredTest_07(PtraceCoveredTest):
('ptrace,' , [ False , False , False , False ]),
('ptrace read,' , [ False , False , False , False ]),
('ptrace read peer=/foo/bar,' , [ False , False , True , True ]),
('ptrace read peer=/foo/*,' , [ False , False , True , True ]),
('ptrace read peer=/**,' , [ False , False , True , True ]),
('ptrace read peer=/what/*,' , [ False , False , True , True ]),
('ptrace read peer=/foo/*,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('ptrace read peer=/**,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('ptrace read peer=/what/*,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('ptrace peer=/foo/bar,' , [ False , False , False , False ]),
('ptrace read, # comment' , [ False , False , False , False ]),
('allow ptrace read,' , [ False , False , False , False ]),

6
utils/test/test-signal.py
View File

@ -416,9 +416,9 @@ class SignalCoveredTest_08(SignalCoveredTest):
('signal,' , [ False , False , False , False ]),
('signal send,' , [ False , False , False , False ]),
('signal send peer=/foo/bar,' , [ False , False , True , True ]),
('signal send peer=/foo/*,' , [ False , False , True , True ]),
('signal send peer=/**,' , [ False , False , True , True ]),
('signal send peer=/what/*,' , [ False , False , True , True ]),
('signal send peer=/foo/*,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('signal send peer=/**,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('signal send peer=/what/*,' , [ False , False , False , False ]), # TODO: wildcard vs. wildcard never matches in is_covered_aare()
('signal peer=/foo/bar,' , [ False , False , False , False ]),
('signal send, # comment' , [ False , False , False , False ]),
('allow signal send,' , [ False , False , False , False ]),