Hello, the attached file contains the review for r58 and also some bugs I found while testing.
Regards, Christian Boltz -- Stell dein cron auch deine Rechneruhr? Ja? Dann würde ich ihm nicht allzuviel mehr anvertrauen - er scheint leicht überlastet und strebt in Riesenschritten die Rente an ;-) [Matthias Houdek in suse-linux zu einer Mail aus der Zukunft]
------------------------------------------------------------ revno: 58 committer: Kshitij Gupta <kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Thu 2013-09-12 14:42:15 +0530 message: Updated __init_.py tested with de_DE and hi_IN translations using old apparmor-utils.mo file, not pushing remainder of files for their lack of beauty ### I've pushed the tested and fixed __init__.py file however an "early ### pressed" Enter also ended up pushing some other files. Try ignoring ### them for now. They probably won't make much sense. === modified file 'Testing/minitools_test.py' --- Testing/minitools_test.py 2013-08-31 12:18:40 +0000 +++ Testing/minitools_test.py 2013-09-12 09:12:15 +0000 @@ -2,74 +2,74 @@ +test_path = '/usr/sbin/ntpd' +local_profilename = None + +python_interpreter = 'python' +if sys.version_info >= (3,0): + python_interpreter = 'python3' def test_audit(self): #Set ntpd profile to audit mode and check if it was correctly set - subprocess.check_output('python ./../Tools/aa-audit -d ./profiles ntpd', shell=True) - local_profilename = apparmor.get_profile_filename(apparmor.get_full_path(apparmor.which('ntpd'))) + subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename + local_profilename = apparmor.get_profile_filename(test_path) # you should use the result for the aa-audit call ;-) # Besides that, local_profilename is already set in the global part of the script - no need to do it here again #Remove audit mode from ntpd profile and check if it was correctly removed + subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles -r ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename def test_complain(self): #Set ntpd profile to complain mode and check if it was correctly set + subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename #Set ntpd profile to enforce mode and check if it was correctly set + subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles -r ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename # Set audit flag and then complain flag in a profile + subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles ntpd'%python_interpreter, shell=True) + subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename #Remove complain flag first i.e. set to enforce mode + subprocess.check_output('%s ./../Tools/aa-complain -d ./profiles -r ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename #Remove audit flag - subprocess.check_output('python ./../Tools/aa-audit -d ./profiles -r ntpd', shell=True) + subprocess.check_output('%s ./../Tools/aa-audit -d ./profiles -r ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename def test_enforce(self): #Set ntpd profile to complain mode and check if it was correctly set + subprocess.check_output('%s ./../Tools/aa-enforce -d ./profiles -r ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename #Set ntpd profile to enforce mode and check if it was correctly set + subprocess.check_output('%s ./../Tools/aa-enforce -d ./profiles ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename def test_disable(self): #Disable the ntpd profile and check if it was correctly disabled + subprocess.check_output('%s ./../Tools/aa-disable -d ./profiles ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename #Enable the ntpd profile and check if it was correctly re-enabled + subprocess.check_output('%s ./../Tools/aa-disable -d ./profiles -r ntpd'%python_interpreter, shell=True) # this will still fail if ntpd is not installed - use the full path to ntpd or the profile filename === modified file 'Tools/aa-mergeprof' --- Tools/aa-mergeprof 2013-08-31 12:18:40 +0000 +++ Tools/aa-mergeprof 2013-09-12 09:12:15 +0000 # not reviewed, will wait for the next version === modified file 'apparmor/__init__.py' --- apparmor/__init__.py 2013-08-07 09:13:17 +0000 +++ apparmor/__init__.py 2013-09-12 09:12:15 +0000 @@ -5,10 +5,11 @@ def init_localisation(): locale.setlocale(locale.LC_ALL, '') #cur_locale = locale.getlocale() - filename = 'res/messages_%s.mo' % locale.getlocale()[0][0:2] + filename = '/usr/share/locale/%s/LC_MESSAGES/apparmor-utils.mo' % locale.getlocale()[0][0:2] # [0:2] means locales like pt_BR won't be used (only pt, which might be different). # I doubt this is intentional. === modified file 'apparmor/tools.py' --- apparmor/tools.py 2013-08-30 22:38:26 +0000 +++ apparmor/tools.py 2013-09-12 09:12:15 +0000 @@ -45,10 +45,14 @@ if which: program = apparmor.get_full_path(which) - if not program or not os.path.exists(program): - program = apparmor.UI_GetString(_('The given program cannot be found, please try with the fully qualified path name of the program: '), '') + if (not program or not os.path.exists(program)): + if not program.startswith('/'): + program = apparmor.UI_GetString(_('The given program cannot be found, please try with the fully qualified path name of the program: '), '') + else: + apparmor.UI_Info(_("%s does not exist, please double-check the path.")%program) + sys.exit(1) # better, but # - if a program does not exist, it should still be possible to disable etc. its profile # - in the "if not program" case, program.startswith can give funny messages ;-) @@ -104,12 +108,85 @@ def clean_profile(self, program, p): filename = apparmor.get_profile_filename(program) + self.delete_superluous_rules(program) # typo in the function name - should be super_f_luous - or did you consider the f superfluous? ;-) if filename: apparmor.write_profile_ui_feedback(program) apparmor.reload_base(program) else: raise apparmor.AppArmorException(_('The profile for %s does not exists. Nothing to clean.')%p) - + + def delete_superluous_rules(self, program): # typo in the function name - should be super_f_luous - or did you consider the f superfluous? ;-) # additional things noticed while testing: # aa-logprof: # (A)llow / [(D)eny] / (G)lob / Glob with (E)xtension / (N)ew / Abo(r)t / (F)inish / (M)ore / (I)gnore # # (I)gnore is misplaced, a better place would be after (D)eny # (M)ore throws an exception: # Traceback (most recent call last): # File "aa-logprof", line 29, in <module> # apparmor.do_logprof_pass(logmark) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 2201, in do_logprof_pass # ask_the_questions() # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 1781, in ask_the_questions # audit_toggle, owner_toggle = UI_ask_mode_toggles(audit_toggle, owner_toggle, allow_mode) # TypeError: 'NoneType' object is not iterable # (F)inish prints "FINISHING...", but should offer to save the profile (if anything was changed) # (my test with some acroread stuff just exited) # The following local profiles were changed. Would you like to save them? # # 1 - /usr/lib/Adobe/Reader9/Reader/intellinux/bin/acroread # [2 - /{usr/,}bin/ping] # 3 - /usr/sbin/ntpd # # (S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t # pressing "3" does not select the ntpd profile (cursor down works) # ("1" and "2" work - off-by-one error?) # (if I have more options, the last one can never be reached by typing the number - one more reason to guess off-by-one error) # (V)iew Changes results in: (only for some profiles! - in my case acroread - I'd say it happens only for changed profile with deny $file rules) # Traceback (most recent call last): # File "aa-logprof", line 29, in <module> # apparmor.do_logprof_pass(logmark) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 2209, in do_logprof_pass # save_profiles() # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 2290, in save_profiles # newprofile = serialize_profile_from_old_profile(aa[which], which, '') # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 3698, in serialize_profile_from_old_profile # if not write_prof_data[hat][allow]['path'][path].get('mode', False) & tmpmode: # TypeError: unsupported operand type(s) for &: 'bool' and 'set' # (View Changes b/w (C)lean profiles works) # writing a profile also fails: # Writing updated profile for /home/cb/bin/lj_make_galerie.sh. # Traceback (most recent call last): # File "aa-logprof", line 29, in <module> # apparmor.do_logprof_pass(logmark) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 2209, in do_logprof_pass # save_profiles() # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 2302, in save_profiles # write_profile_ui_feedback(profile_name) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 3834, in write_profile_ui_feedback # write_profile(profile) # File "/usr/lib/python2.7/site-packages/apparmor/aa.py", line 3858, in write_profile # os.rename(newprof.name, prof_filename) # OSError: [Errno 18] Invalid cross-device link # with some print() added, I get: # newprof.name: /tmp/tmpR8gvWs~ # prof_filename: /etc/apparmor.d/home.cb.bin.lj_make_galerie.sh # so the message is not too surprising ;-) # # hint: aa.py / write_profile(): # newprof = tempfile.NamedTemporaryFile('w', suffix='~' ,delete=False) # does not specify the directory # added rules are always prefixed with "allow" (at least in diff view) # IMHO you should default to not adding the "allow" keyword # but keep it when modifying an existing profile which already contains it # all tools: # if something goes wrong, they print the full exception # that's nice for debugging, but could shock normal users ;-) # please only print the real error message in cases where you "expect" an error message, for example "profile dir is not a dir" # (unexpected errors should still print the full exception) # apparmor/__init__.py # causes an exception when using "LANG=C" because "C" is shorter than what filename = ...[0:2] expects. # (see mailinglist 2013-09-13 for details) vim:ft=diff
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor