[apparmor] GSoC review r58
Christian Boltz
apparmor at cboltz.de
Sun Sep 15 14:59:16 UTC 2013
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]
-------------- next part --------------
------------------------------------------------------------
revno: 58
committer: Kshitij Gupta <kgupta8592 at 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
More information about the AppArmor
mailing list