Hello,
see the attachment for r23 review. The commit looks quite good, but I
found some small issues nevertheless ;-)
Regards,
Christian Boltz
--
> I don't really know how nor why, but if a spellchecker is
> enabled on the wiki server, the edit wiki windows do
> colorize the mispelled words and this is very handy.
I have mixed feelings about using a spill chicken...
[> jdd and Peter Flodin in opensuse-wiki]
=== modified file 'Testing/severity_test.py'
--- Testing/severity_test.py 2013-07-18 19:14:55 +0000
+++ Testing/severity_test.py 2013-07-19 22:49:07 +0000
def testRank_Test(self):
z = severity.Severity()
### (not from this commit, nevertheless)
### "z" seems to be unused - remove it?
### BTW:
### # python3 severity_test.py
### severity_test.py:59: ResourceWarning: unclosed file <_io.BufferedReader name='severity_broken.db'>
### [...]
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py 2013-07-17 23:59:54 +0000
+++ apparmor/aa.py 2013-07-19 22:49:07 +0000
@@ -434,40 +434,30 @@
hashbang = head(localfile)
if hashbang.startswith('#!'):
interpreter = get_full_path(hashbang.lstrip('#!').strip())
- try:
- local_profile[localfile]['allow']['path'][interpreter]['audit'] |= 0
- except TypeError:
- local_profile[localfile]['allow']['path'][interpreter]['audit'] = 0
+
+ local_profile[localfile]['allow']['path'][localfile]['mode'] = local_profile[localfile]['allow']['path'][localfile].get('mode', str_to_mode('r')) | str_to_mode('r')
### that looks already better than the try/except
### nevertheless it will cause some superfluous typing
### the ideal syntax would be something like
### local_profile.add('allow', 'path', '/bin/foo', 'r', no_audit) # no_audit could be an alias of "false"
### (then let all the magic happen inside local_profile, where you need the code only once)
@@ -894,5 +885,183 @@
def handle_children(profile, hat, root):
entries = root[:]
### the code uses entry[:3], entry[:5] etc. quite often, so a comment explaining the structure
### (or pointing to the explanation elsewhere) would be very useful
+ regex_nullcomplain = re.compile('null(-complain)*-profile')
for entry in entries:
+ elif type == 'unknown_hat':
[...]
+ if ans == 'CMD_ADDHAT':
+ hat = uhat
+ aa[profile][hat]['flags'] = aa[profile][profile]['flags']
+ elif ans == 'CMD_USEDEFAULT':
+ hat = default_hat
+ elif ans == 'CMD_DENY':
+ return None
### shouldn't CMD_DENY add a "deny" rule? (or is this handled elsewhere?)
+ elif type == 'path' or type == 'exec':
[...]
+ # Give Execute dialog if x access requested for something that's not a directory
+ # For directories force an 'ix' Path dialog
+ do_execute = False
+ exec_target = detail
+
+ if mode & str_to_mode('x'):
+ if os.path.isdir(exec_target):
### this test might fail if someone sends you his log and you try to update a profile based on the log
### (does the log contain directories as "/foo/bar/"? If yes, testing for the trailing "/" might work)
+ if mode & AA_MAY_LINK:
+ regex_link = re.compile('^from (.+) to (.+)$')
### I can imagine some funny filenames to break that regex ;-)
### for example "/home/cb/I'm from germany and go to the next train station.txt"
### (not sure if this is too relevant in real world, and what's the best way to prevent it)
=== modified file 'apparmor/severity.py'
--- apparmor/severity.py 2013-07-18 19:14:55 +0000
+++ apparmor/severity.py 2013-07-19 22:49:07 +0000
@@ -184,16 +184,20 @@
def load_variables(self, prof_path):
"""Loads the variables for the given profile"""
+ regex_include = re.compile('include <(\S*)>')
### looks better, but could get false positives and false negatives, for example
### /foo/bar r, # instead of #include <abstractions/foobar> <--- regex will match comment
### #include <abstractions/foo> <--- will not match because there's more than one space after "#include"
### a better regex would be
### ^\s*#?include\s+<(\S*)>
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
