Hello,
see the attachment for the r24 review.
Regards,
Christian Boltz
--
<Dominian> There is always room for improvement
<Dominian> to seek perfection is to drive yourself insane.
<Dominian> except suseROCKs, he's already insane.
[from #opensuse-project]
=== modified file 'apparmor/aa.py'
--- apparmor/aa.py 2013-07-19 22:49:07 +0000
+++ apparmor/aa.py 2013-07-22 23:05:51 +0000
@@ -449,7 +450,7 @@
local_profile[localfile]['include']['abstractions/python'] = True
elif 'ruby' in interpreter:
### hmm, just using "ruby" might lead to false positives
### better check for the full path or at least the full basename
local_profile[localfile]['include']['abstractions/ruby'] = True
- elif interpreter in ['/bin/bash', '/bin/dash', '/bin/sh']:
+ elif re.search('/bin/(bash|dash|sh)', interpreter):
### if you use a regex, don't forget ^...$ ;-)
+ # If profiled program executes itself only 'ix' option
+ if exec_target == profile:
+ options = 'i'
### ix is a good default, but there may be (rare) cases where a program executes itsself with different options etc.
### so I'd still allow the other x options
+ # Don't allow hats to cx?
+ options.replace('c', '')
### child profiles can't be nested, but one child profile can call another
### at the moment the workaround is to do (assuming we are in /bin/foo//bin/bar child profile) "Px -> /bin/foo//bin/baz"
### having an option "use another child profile" (which makes clear it won't be nested as /bin/foo//bin/bar//bin/baz) would be nice
+ # Add deny to options
+ options += 'd'
+ # Define the default option
+ default = None
+ if 'p' in options and os.path.exists(get_profile_filename(exec_target)):
+ default = 'CMD_px'
### while we are at it - displaying a note "target profile exists" would be helpful
+ elif ans == 'CMD_ux':
+ exec_mode = str_to_mode('ux')
+ ynans = UI_YesNo(gettext('Launching processes in an unconfined state is a very\n' +
+ 'dangerous operation and can cause serious security holes.\n\n' +
+ 'Are you absolutely certain you wish to remove all\n' +
+ 'AppArmor protection when executing :') + '%s ?' % exec_target, 'n')
### even if this question is absolutely correct, a "never ask again" option would be welcome ;-)
+ if exec_mode & str_to_mode('i'):
+ if 'perl' in exec_target:
+ aa[profile][hat]['include']['abstractions/perl'] = True
### oh, does executing /bin/perlentaucher need abstractions/perl?
### (the code will happily add it ;-)
+ elif '/bin/bash' in exec_path or '/bin/sh' in exec_path:
### similar question here for /usr/bin/bin/shorewall ;-)
### (in other words: using "in" can have quite some side effects)
+ if 'perl' in interpreter:
+ aa[profile][hat]['include']['abstractions/perl'] = True
+ elif '/bin/bash' in interpreter or '/bin/sh' in interpreter:
+ aa[profile][hat]['include']['abstractions/bash'] = True
### some more "in"...
### that all said - this is at least the second time where the code contains interpreter <-> abstraction information
### better store it at one place - for example like this:
### def abstraction_for_interpreter(interpreter)
### # TODO: interpreter_basename = remove ^(/usr)?/bin/ from interpreter
### if interpreter_basename == 'bash'
### return 'abstractions/bash'
### elif interpreter_basename == 'perl'
### [...]
=== modified file 'apparmor/severity.py'
--- apparmor/severity.py 2013-07-19 22:49:07 +0000
+++ apparmor/severity.py 2013-07-22 23:05:51 +0000
@@ -184,7 +184,7 @@
def load_variables(self, prof_path):
"""Loads the variables for the given profile"""
- regex_include = re.compile('include <(\S*)>')
+ regex_include = re.compile('^#?include\s*<(\S*)>')
### nearly correct ;-) - you should allow whitespace at the beginning of the line
if os.path.isfile(prof_path):
with open_file_read(prof_path) as f_in:
for line in f_in:
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
