Re: [apparmor] Fwd: GSoC r13, r14, r15 review
happens if someone is not using utf-8? # The program's output needs to be utf-8 encoded else they get some weird stuff, on encoding look below. This means you have to set the encoding somewhere ;-) Or set LANG=C and you should have pure ASCII ;-) +ret = e.returncode +else: +ret = 0 +output = output.decode('utf-8').split('\n') +# Remove the extra empty string caused due to \n if present +if len(output) 1: +output.pop() +return (ret, output) +def get_reqs(file): +Returns a list of paths from ldd output +pattern1 = re.compile('^\s*\S+ = (\/\S+)') +pattern2 = re.compile('^\s*(\/\S+)') +reqs = [] +ret, ldd_out = get_output(ldd, file) ### ldd is None - you should fill it before using it ;-) ### also, I'm not sure if ldd needs to be a global variable # The setter method is yet to come. ;-) # Will fix it when refractoring the module later as I planned. :-) OK. === added file 'apparmor/common.py' --- apparmor/common.py1970-01-01 00:00:00 + +++ apparmor/common.py2013-07-03 23:34:04 + +def cmd_pipe(command1, command2): +'''Try to pipe command1 into command2.''' +try: +sp1 = subprocess.Popen(command1, stdout=subprocess.PIPE) +sp2 = subprocess.Popen(command2, stdin=sp1.stdout) +except OSError as ex: +return [127, str(ex)] + +if sys.version_info[0] = 3: +out = sp2.communicate()[0].decode('ascii', 'ignore') ### is ascii correct here, or should it be the system locale? # From aa-enforce, havent used it yet. However, utf-8 is default for Python3, so maybe we can use it? If we use it everywhere, then I'd say yes. +def open_file_read(path): +'''Open specified file read-only''' +try: +orig = codecs.open(path, 'r', UTF-8) # once more - is hardcoding utf-8 a good idea? # Again from aa-enforce, and we can discuss using utf-8 or system locale. Sounds like something we should discuss in the meeting later ;-) Regards, Christian Boltz -- Guten Tag, ich möchte gerne einen Tisch reservieren. Gerne, auf welchen Namen denn? 31337 /-/ /\ X0R! [Jens Benecke in suse-linux zum Thema Realnames] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] GSoC r13, r14, r15 review
Hello, Am Dienstag, 9. Juli 2013 schrieb Kshitij Gupta: I somehow doubt this is intentional - the test should catch this exception ;-) As it turns this issue has got something to do with the Python3's new feature called Exception Chaining. Its another of those Python2-3 problem. An additional line needs to be incorporated to check if Python2 or 3 and raise accordingly. :/ :-( It was inside the try except block of test, I got it out to try how exception looked. Turns out \n\t aren't working yet. :-\ Also note that the last line contains \n\t - this should become a real line break and tab in the output... I did not notice until later, but the AppArmorException uses repr() to print the description and hence its not possible to print the nicely formatted exception with the line number in new line. I noticed you removed repr() in r16 - does this fix the issue? (a short test tells me the answer is probably no) I feel I should use the error(), it will print something like this and terminate program: ERROR: No severity value present in file: severity_broken.db [Line 14]: CAP_SYS_MODULE What would you say? move to error() instead of raising AppArmorException? Can error() be catched with try/except like an exception? For example, we'll need this to display a nice error dialog in YaST. (If yes, then using error() is OK.) Regards, Christian Boltz -- The normal user is happy with openSUSE because: [...] - openSUSE isn't a religion (like a few others); What users blame us is for lack of customization :) [Nelson Marques in opensuse-factory] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] apparmor policy versioning
. With an additional copy of the profiles, we'll end up in a maintenance hell - and users will kill us because they have to update two profiles instead of one if they want to switch kernels. Regards, Christian Boltz [1] does this name remind you to something? ;-) [2] to be exact, cleanup of code duplication in PostfixAdmin -- Comic Sans Man möge mir verzeihen, aber ich möchte mich im Rahmen dieses Essays in erster Linie mit Schriften auseinandersetzen, nicht mit Krankheiten. [http://praegnanz.de/essays/136/typo-im-web-html-schriften-unter-der-lupe] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r17..22
Hello, the attached files contain my review notes for r17..22. In case you miss the files for r19 and r20: that's intentional, those commits look so good that I don't need to comment on them ;-) Regards, Christian Boltz -- Jungs. Mit dem Argument kann ich kaputte Autos verkaufen und dann erklären, daß Fahrradfahren eh viel gesünder ist. [Peer Heinlein in postfixbuch-users] === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-07-08 22:16:26 + +++ apparmor/aa.py 2013-07-17 15:08:13 + @@ -619,28 +624,36 @@ def set_profile_flags(prof_filename, newflags): Reads the old profile file and updates the flags accordingly regex_bin_flag = re.compile('^(\s*)((??\/.+???)|(profile\s+(??.+???)))\s+(flags=\(.+\)\s+)*\{\s*$/') -regex_hat_flag = re.compile('^(\s*\^\S+)\s+(flags=\(.+\)\s+)*\{\s*$') +regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)((\s+#\S*)*)\s*$') ### this looks superfluous because it's overwritten two lines below +a=re.compile('^([a-z]*)\s+([A-Z]*)((\s+#\S*)*)\s*$') ### a is not used anywhere in the function +regex_hat_flag = re.compile('^(\s*\^\S+)\s+(flags=\(.+\)\s+)*\{\s*(#*\S*)$') ### (regex_hat_flag overwritten here) if os.path.isfile(prof_filename): with open_file_read(prof_filename) as f_in: -with open_file_write(prof_filename + '.new') as f_out: +tempfile = tempfile.NamedTemporaryFile('w', prefix=prof_filename , delete=False, dir='/etc/apparmor.d/') ### Looks quite good. A small detail is missing - please add suffix='~' to make sure ### a profile in a tempfile is not loaded when someone calls rcapparmor reload @@ -654,3 +667,240 @@ +def sync_profile(): +def submit_created_profiles(new_profiles): +def submit_changed_profiles(changed_profiles): +def yast_select_and_upload_profiles(title, message, profiles_up): +def console_select_and_upload_profiles(title, message, profiles_up): +def set_profile_local_only(profs): ### those functions are about the profile repo and currently unused. ### a comment saying that would be nice ### (or move them to a separate file, maybe profilerepo.py) +def handle_children(profile, hat, root): +entries = root[:] +for entry in entries: + ### looks like the code to handle the children is still missing ;-) === added file 'apparmor/ui.py' --- apparmor/ui.py 1970-01-01 00:00:00 + +++ apparmor/ui.py 2013-07-17 15:08:13 + @@ -0,0 +1,255 @@ +def init_localisation(): +locale.setlocale(locale.LC_ALL, '') +#cur_locale = locale.getlocale() +filename = 'res/messages_%s.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. +def UI_Info(text): +if DEBUGGING: +debug_logger.info(text) ### debug_logger should check DEBUGGING itsself to avoid all the if DEBUGGING: conditions ### even if this means you'll need a small wrapper around debug_logger - but on the long term it makes the code more readable +def UI_YesNo(text, default): +if DEBUGGING: +debug_logger.debug('UI_YesNo: %s: %s %s' %(UI_mode, text, default)) +ans = default +if UI_mode == 'text': +yes = '(Y)es' +no = '(N)o' +usrmsg = 'PromptUser: Invalid hotkey for' +yeskey = 'y' +nokey = 'n' ### can you honor translations please? ### for example, in german it should be (j)a / (n)ein +sys.stdout.write('\n' + text + '\n') +if default == 'y': ### better: if default == yeskey - avoids problems when using translations +sys.stdout.write('\n[%s] / %s\n' % (yes, no)) +else: +sys.stdout.write('\n%s / [%s]\n' % (yes, no)) +ans = readkey() +if ans: +ans = ans.lower() +else: +ans = default +else: +SendDataTooYast({ + 'type': 'dialog-yesno', + 'question': text + }) +ypath, yarg = GetDataFromYast() +ans = yarg['answer'] +if not ans: +ans = default +return ans ### ans can be anything, not just yeskey and nokey ### if something else (= invalid key) is entered, the function should ask again ### (see UI_YesNoCancel which does exactly that) +def UI_YesNoCancel(text, default): +if DEBUGGING: +debug_logger.debug('UI_YesNoCancel: %s: %s %s' % (UI_mode, text, default)) + +if UI_mode == 'text': +yes = '(Y)es' +no = '(N)o' +cancel = '(C)ancel' +yeskey = 'y' +nokey = 'n' +cancelkey = 'c' ### should honor translations +CMDS = { +'CMD_ALLOW': '(A)llow', +'CMD_OTHER': '(M)ore', ### do all those options honor translations? === added file 'apparmor/yasti.py' --- apparmor/yasti.py 1970-01-01 00:00:00 + +++ apparmor/yasti.py 2013-07-17 15:08:13 + @@ -0,0 +1,82 @@ +def SendDataToYast(data): +for line in sys.stdin: [...] +Return
[apparmor] GSoC review r23
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 + +++ Testing/severity_test.py 2013-07-19 22:49:07 + 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 + +++ apparmor/aa.py 2013-07-19 22:49:07 + @@ -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 + +++ apparmor/severity.py 2013-07-19 22:49:07 + @@ -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
[apparmor] GSoC review r24
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 + +++ apparmor/aa.py 2013-07-22 23:05:51 + @@ -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 + +++ apparmor/severity.py 2013-07-22 23:05:51 + @@ -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
Re: [apparmor] [parser patch] fix apparmor cache tempfile location to use passed arg
Hello, Am Dienstag, 23. Juli 2013 schrieb Steve Beattie: The second issue is that it would generate the temporary file that it stores the cache file in [basedir]/cache even if an alternate cache location was specified on the command line. This causes a problem if [basedir]/cache is on a separate device than the alternate cache location, because the rename() of the tempfile into the final location would fail Looks like it was a good idea to package /etc/apparmor/cache as a symlink to /var/cache/apparmor on openSUSE ;-) (which the parser would not check the return code of). Nice[tm]. That said - your patch looks like something that should be backported to the 2.8 branch (even if it isn't needed for openSUSE thanks to the symlink). Regards, Christian Boltz -- Aren't most of SUSE-employed community members part of the ResearchDestroy department? [Sascha Peilicke in opensuse-project] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r26 and r27
Hello, see the attached file for r26 and r27 review notes. @John: I'm still waiting for your answer about # ix implies m, so we don't need to add m if ix is present I have some profiles that contain mrix (for example sbin.dhclient and usr.sbin.ntpd), so either the old logprof was buggy or the comment is wrong ;-) Regards, Christian Boltz -- [Grundrechte] Natürlich gibt's da auch das berühme Recht auf freie Entfaltung. Andererseits: setzt das nicht auch zwingend vorraus, daß man vorher auch gehörig zusammengefaltet wurde? ;-) [Gerard Jensen in suse-linux] === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-07-24 16:42:34 + +++ apparmor/aa.py 2013-07-27 10:02:12 + -elif 'python' in interpreter: +elif interpreter == 'python': local_profile[localfile]['include']['abstractions/python'] = True ### 'python3' should also be allowed +def ask_the_question(): [...] +elif ans == 'CMD_DENY': +aa[profile][hat]['deny']['capability'][capability]['set'] = True +changed[profile] = True ### sidenote: an additional ignore option would be nice to just ignore a log event ### without adding anything to the profile. ### (usecase: starting sshd the very first time will write /etc/ssh/ssh_host_key, but you probably don't want ### to have write permissions for it in the profile) (distro-shipped profile may be an exception of course) +# If we get an mmap request, check if we already have it in allow_mode +if mode AA_EXEC_MMAP: +# ix implies m, so we don't need to add m if ix is present ### hmm, some of my profiles disagree and have mrix... ### (John, any comments on this?) + +if not mode_contains(allow_mode, mode): +default_option = 1 +options = [] +newincludes = [] +include_valid = False + +for incname in include.keys(): +include_valid = False +# If already present skip +if aa[profile][hat][incname]: +continue + +if cfg['settings']['custom_includes']: +for incn in cfg['settings']['custom_includes'].split(): +if incn in incname: +include_valid = True + +if 'abstraction' in incname: +include_valid = True ### so I can add abstractions/doesnotexist and foobar/abstraction? ;-) ### (same for custom_includes) ### hints: ### - check if the file exists ### - use startswith 'abstractions/' instead of in +if not include_valid: +continue ### does this mean it's impossible to add an #include for files not in abstractions/ or custom_includes? ### I'd prefer not to forbid that ### (of course, check if the file exists etc.) +elif ans == 'CMD_ALLOW': +path = options[selected] +done = True +match = re.search('^#include\s+(.+)$', path) ### wrong regex: ### - space after #include is optional ### - fails with inline comments ### - if path is not trimmed, it fails with leading space ### (hint: a function re_match_include(path) would be a good idea, you already have the correct regex somewhere else) +elif ans == 'CMD_GLOB': +newpath = options[selected].strip() +if not newpath.startswith('#include'): +if newpath[-1] == '/': +if newpath[-4:] == '/**/' or newpath[-3:] == '/*/': +# collapse one level to /**/ +newpath = re.sub('/[^/]+/\*{1,2}$/', '/\*\*/', newpath) +else: +newpath = re.sub('/[^/]+/$', '/\*/', newpath) ### what happens with /foo/bar**/ ? ### (should glob to /foo/**/ - but the code looks like it will be globbed to /foo/*/ ) +else: +if newpath[-3:] == '/**' or newpath[-2:] == '/*': +newpath = re.sub('/[^/]+/\*{1,2}$', '/\*\*', newpath) +elif re.search('/\*\*[^/]+$', newpath): +newpath = re.sub
[apparmor] GSoC meeting
Hello, this week, the GSoC IRC meeting will be a day earlier than usual because I'll be away on tuesday. This means the meeting is on monday (= tomorrow) at 19:00 UTC. Besides the usual topics, we'll also discuss the to-be-written mergeprof. Regards, Christian Boltz -- AV is homeopathy for computers: This software was in contact with mal- ware in the past, so it'll recognize other malware in the future. $79 [https://twitter.com/thegrugq/status/297177182848049152] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r30
Hello, the review for r30 is attached - it had lots of new code (and interesting[tm] regexes) - therefore I have several notes about it ;-) @John: The review contains some questions for you - can you please answer them? Regards, Christian Boltz -- My calendar shows May 12th to be a Friday, not a Thursday? I meant 11th ;-(. With all the delays, perhaps mentioning the year would also be a good idea. ;-) [ Andreas Jaeger and houghi in opensuse] review-r30 Description: Binary data -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] GSoC review r30
Hello, John Johansen wrote: On 08/01/2013 02:59 PM, Christian Boltz wrote: ### a check if the hat already exists might be useful to avoid duplicate hat names (which might get merged on write, but I doubt that's intended behaviour) ### interestingly, the parser does _not_ complain about duplicate hats. ### John, is this a bug or intentional? That should fail, there is an explicit test for this in the parser. And in my quick testing I get Multiple definitions for hat foo in profile (null) exist,bailing out. so a bug in the output but the check worked, can you forward an example where it is not working correctly It happened with some echo $whatever | apparmor_parser -p /dev/stdin and checking my bash history showed I accidently deleted the pipe when I hit the so-called bug. In other words: it works as it should (and I get the correct error message for duplicate hat names) - sorry for the false alarm! +# Below is not required I'd say ### hmm, not sure - John? +if not do_include: +for hatglob in cfg['required_hats'].keys(): +for parsed_prof in sorted(parsed_profiles): +if re.search(hatglob, parsed_prof): +for hat in cfg['required_hats'][hatglob].split(): +if not profile_data[parsed_prof].get(hat, False): +profile_data[parsed_prof][hat] = hasher() err, I am going to have to get back to you on this one. I need to dive in and get more context first ;-) ### we should discuss if we want to keep writing in sorted() order (which can be helpful, but also annoying) ### or if we want to keep the original order of a profile whenever possible ### (see discussion about writing config files) ### - topic for the next meeting? I prefer original order when possible, possibly with an option to tell it to order and clean up the profile. Yes, that sounds like a good method, even if it means a bit more work. (Hint: we already do something similar when writing config files ;-) For the clean up option - don't read the old profile while writing the new one ;-) Basically it comes down to ordering destroys semantic/logical groupings and commenting. Yes. Regards, Christian Boltz -- Chance is irrelevant. We will succeed.-- Seven of Nine -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC r31 review
Hello, the GSoC review for r31 is attached. Regards, Christian Boltz -- My 2 cents, tja, Stundenlohn wird nach Eignung, Leistung und Befähigung gezahlt [ Claus Reheis und Detlef Reichelt in opensuse-de] review-r31 Description: Binary data -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r34
Hello, one more (quite harmless) review ;-) Regards, Christian Boltz -- [Windows krepiert nach Update] Habt Ihr eine Idee, was ich tun könnte? Vermutlich ein Computervirus. Besorg etwas Aciclovir aus der Apotheke, oeffne das Rechnergehaeuse und troepfle das Mittel auf alle roten oder geschwollenen Bauteile. [Mirko Liss] revno: 34 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Tue 2013-08-06 01:53:28 +0530 message: Some code for logprof === added directory 'Tools' === added file 'Tools/aa-logprof.py' --- Tools/aa-logprof.py 1970-01-01 00:00:00 + +++ Tools/aa-logprof.py 2013-08-05 20:23:28 + @@ -0,0 +1,13 @@ +os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin') # what's the reason for overriding $PATH ? === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-08-05 13:25:34 + +++ apparmor/aa.py 2013-08-05 20:23:28 + @@ -3361,17 +3365,25 @@ +def store_list_var(var, list_var, value, var_operation): ... +if var_operation == '=': +if not var.get(list_var, False): +var[list_var] = set(vlist) +else: +raise AppArmorException('An existing variable redefined') # better error message please - it's easy to include the variable name +else: +if var.get(list_var, False): +var[list_var] = set(var[list_var] + vlist) +else: +raise AppArmorException('An existing variable redefined') # better error message please - it's easy to include the variable name @@ -3829,8 +3841,8 @@ def get_include_data(filename): data = [] -if os.path.exists(profile_dir + '/' + filename): -with open_file_read(profile_dir + '/' + filename) as f_in: +if os.path.exists(filename): +with open_file_read(filename) as f_in: # this means get_include_data() needs the full path as parameter @@ -3844,6 +3856,7 @@ incfile = load_includeslist.pop(0) data = get_include_data(incfile) incdata = parse_profile_data(data, incfile, True) +print(incdata) # forgotten debug code? vim:ft=diff -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r35..39
Hallo, the reviews for r35..r39 are attached. I have no complaints about the revisions with even numbers ;-) Regards, Christian Boltz -- Aus der Beschreibung entnehme ich, daß deine Fonts nach Typ 3 konvertiert werden (Finger im Hals) und deine Bilder auf Screen- Qualität (Fuß zum Finger dazusteck...) [Ratti in suse-linux] revno: 35 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Wed 2013-08-07 14:43:17 +0530 message: certain fixes === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-08-05 20:23:28 + +++ apparmor/aa.py 2013-08-07 09:13:17 + @@ -1337,7 +1337,7 @@ if not os.path.exists(get_profile_filename(exec_target)): ynans = 'y' if exec_mode str_to_mode('i'): -ynans = UI_YesNo(_('A profile for ') + str(exec_target) + gettext(' doesnot exist.\nDo you want to create one?'), 'n') +ynans = UI_YesNo(_('A profile for ') + str(exec_target) + _(' doesnot exist.\nDo you want to create one?'), 'n') # this text should use %s instead of two split texts @@ -1596,6 +1596,7 @@ #last = None #event_type = None try: +print(filename) # forgotten debug code? log_open = open_file_read(filename) except IOError: raise AppArmorException('Can not read AppArmor logfile: ' + filename) @ -3371,7 +3376,9 @@ if not var.get(list_var, False): var[list_var] = set(vlist) else: -raise AppArmorException('An existing variable redefined') +print('Ignoring: New definition for variable for:',list_var,'=', value, 'operation was:',var_operation,'old value=', var[list_var]) +pass +#raise AppArmorException('An existing variable redefined') # variable redefinition should be an error, not a warning # (apparmor_parser also errors out IIRC) vim:ft=diff revno: 37 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Fri 2013-08-09 11:04:32 +0530 message: fixed debugglogger === modified file 'apparmor/common.py' --- apparmor/common.py 2013-08-05 13:25:34 + +++ apparmor/common.py 2013-08-09 05:34:32 + @@ -188,3 +189,22 @@ new_reg = new_reg + '$' return new_reg +class DebugLogger: +def __init__(self, module_name=__name__): +logging.basicConfig(filename='/var/log/apparmor/logprof.log') +self.logger = logging.getLogger(module_name) +self.debugging = True +if os.getenv('LOGPROF_DEBUG', False): +self.debugging = True # maybe we could make LOGPROF_DEBUG an integer (instead of on/off) to define the log level? # (0 = none, 1=error, 2=error+info, 3=error+info+debug +def error(self, msg): +if self.debugging: +self.logger.error(msg) +def info(self, msg): +if self.debugging: +self.logger.info(msg) +def debug(self, msg): +if self.debugging: +self.logger.debug(msg) +def shutdown(self): +logging.shutdown() +#logging.shutdown([self.logger]) vim:ft=diff revno: 39 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sat 2013-08-10 01:17:00 +0530 message: working logger === modified file 'apparmor/common.py' --- apparmor/common.py 2013-08-09 11:19:01 + +++ apparmor/common.py 2013-08-09 19:47:00 + @@ -191,16 +191,17 @@ class DebugLogger: def __init__(self, module_name=__name__): -#logging.basicConfig(filename='/var/log/apparmor/logprof.log') +logging.basicConfig(filename='/var/log/apparmor/logprof.log', level=logging.DEBUG, format='%(asctime)s - %(name)s - %(message)s\n') self.logger = logging.getLogger(module_name) -self.debugging = True +self.debugging = False if os.getenv('LOGPROF_DEBUG', False): self.debugging = True def error(self, msg): if self.debugging: +logging.error(msg) self.logger.error(msg) def info(self, msg): +logging.info(msg) if self.debugging: self.logger.info(msg) def debug(self, msg): # hmm, for info, logging.info() is always called, even if debugging is off - intentional? vim:ft=diff -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] GSoC review r34
Hello, Am Samstag, 10. August 2013 schrieb Christian Boltz: one more (quite harmless) review ;-) I noticed two additional small issues, see the [v2] in the updated review. Regards, Christian Boltz -- dU hAsT nAtUeRlIcH rEcHt. MaN mUsS sIcH bEiM lEsEn NuR dArAn GeWoEhNeN. mAcHt DaNn KeInEn UnTeRsChIeD mEhR. [Andreas Kneib in suse-linux] revno: 34 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Tue 2013-08-06 01:53:28 +0530 message: Some code for logprof === added directory 'Tools' === added file 'Tools/aa-logprof.py' --- Tools/aa-logprof.py 1970-01-01 00:00:00 + +++ Tools/aa-logprof.py 2013-08-05 20:23:28 + @@ -0,0 +1,13 @@ +os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin') # what's the reason for overriding $PATH ? === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-08-05 13:25:34 + +++ apparmor/aa.py 2013-08-05 20:23:28 + @@ -3361,17 +3365,25 @@ +def store_list_var(var, list_var, value, var_operation): ... +if var_operation == '=': +if not var.get(list_var, False): +var[list_var] = set(vlist) +else: +raise AppArmorException('An existing variable redefined') # better error message please - it's easy to include the variable name +else: # [v2] a comment saying else means += would be nice # [v2] or, better, explicitely check for += and add an else: that errors out saying invalid var_operation +if var.get(list_var, False): +var[list_var] = set(var[list_var] + vlist) +else: +raise AppArmorException('An existing variable redefined') # [v2] wrong error message, should be attemped to append to non-existing variable # better error message please - it's easy to include the variable name @@ -3829,8 +3841,8 @@ def get_include_data(filename): data = [] -if os.path.exists(profile_dir + '/' + filename): -with open_file_read(profile_dir + '/' + filename) as f_in: +if os.path.exists(filename): +with open_file_read(filename) as f_in: # this means get_include_data() needs the full path as parameter vim:ft=diff -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r40
Hello, the review for r40 is attached. I also included the r34 [v2] comments, so you can skip the mail with the updated r34 review ;-) @John: it also includes a question for you (the same I asked on IRC, but you didn't respond yet ;-) Regards, Christian Boltz -- Sagt mal ehrlich: Ist mein Rechner geisteskrank [Harald Katzer in suse-linux] revno: 40 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sat 2013-08-10 12:46:22 +0530 message: fixes from rev 32..39 and fixed(?) flags=(..)thing === modified file 'Testing/common_test.py' --- Testing/common_test.py 2013-08-05 13:25:34 + +++ Testing/common_test.py 2013-08-10 07:16:22 + @@ -4,72 +4,23 @@ +tests=apparmor.config.Config('ini') +tests.CONF_DIR = '.' +regex_tests = tests.read_config('regex_tests.ini') +for regex in regex_tests.sections(): +parsed_regex = re.compile(apparmor.common.convert_regexp(regex)) +for regex_testcase in regex_tests.options(regex): +self.assertEqual(bool(parsed_regex.search(regex_testcase)), eval(regex_tests[regex][regex_testcase]), 'Incorrectly Parsed regex') # the error message should print out which regex failed # besides that, this + regex_tests.ini are much better readable :-) === modified file 'Tools/aa-logprof.py' --- Tools/aa-logprof.py 2013-08-07 09:13:17 + +++ Tools/aa-logprof.py 2013-08-10 07:16:22 + @@ -7,9 +7,9 @@ import argparse if sys.version_info (3,0): -os.environ['PATH'] = '/bin/:/sbin/:/usr/bin/:/usr/sbin' +os.environ['AAPATH'] = '/bin/:/sbin/:/usr/bin/:/usr/sbin' else: -os.environb.putenv('PATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin') +os.environb.putenv('AAPATH', '/bin/:/sbin/:/usr/bin/:/usr/sbin') === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-08-09 11:19:01 + +++ apparmor/aa.py 2013-08-10 07:16:22 + @@ -215,7 +215,7 @@ def which(file): Returns the executable fullpath for the file, None otherwise -env_dirs = os.getenv('PATH').split(':') +env_dirs = os.getenv('AAPATH').split(':') # renaming the variable doesn't change much ;-) # the question is if we want to override PATH or if we should use the existing dirs in PATH # (which users might called expected behaviour, but could also include a small security risk) # John? for env_dir in env_dirs: env_path = env_dir + '/' + file # Test if the path is executable or not @@ -1751,6 +1748,7 @@ def ask_the_questions(): found = None +print(log_dict) # debug code? for aamode in sorted(log_dict.keys()): # Describe the type of changes if aamode == 'PERMITTING': @@ -2955,10 +2953,9 @@ repo_data = None parsed_profiles = [] initial_comment = '' -RE_PROFILE_START = re.compile('^((??\/.+???)|(profile\s+(??.+???)))\s+((flags=)?\((.+)\)\s+)*\{\s*(#.*)?$') +RE_PROFILE_START = re.compile('^((??\/.+???)|(profile\s+(??.+???)))\s+((flags=)?\((.+)\)\s+)?\{\s*(#.*)?$') ^ # regex_bin_flag does not have this \ - which of them is correct? ;-) # (it would be even better to share the regex) @@ -3373,12 +3361,12 @@ else: print('Ignored: New definition for variable for:',list_var,'=', value, 'operation was:',var_operation,'old value=', var[list_var]) pass -#raise AppArmorException('An existing variable redefined') +#raise AppArmorException('An existing variable redefined: %s' %list_var) # (r35 comment) # variable redefinition should be an error, not a warning # (apparmor_parser also errors out IIRC) else: # (r34 [v2] comment) # a comment saying else means += would be nice # or, better, explicitely check for += and add an else: that errors out saying invalid var_operation if var.get(list_var, False): var[list_var] = set(var[list_var] + vlist) else: -raise AppArmorException('An existing variable redefined') +raise AppArmorException('An existing variable redefined: %s' %list_var) # (r34 [v2] comment) # wrong error message, should be something like attemped to append to non-existing variable === modified file 'apparmor/common.py' --- apparmor/common.py 2013-08-09 19:47:00 + +++ apparmor/common.py 2013-08-10 07:16:22 + @@ -165,13 +165,12 @@ #new_reg = new_reg.replace('}', '}') #new_reg = new_reg.replace(',', '|') -while re.search('{.*,.*}', new_reg): -match = re.search('(.*){(.*),(.*)}(.*)', new_reg).groups() +while re.search('{[^}]*,[^}]*}', new_reg): +match = re.search('(.*){([^}]*)}(.*)', new_reg).groups() # I'd use the same regex for while and match (hint: use the regex from match) and store it in a variable # I'd also use ^...$ to be on the safe side @@ -190,18 +189,25 @@ class
Re: [apparmor] GSoC review r35..39
Hello, Am Samstag, 10. August 2013 schrieb Kshitij Gupta: Sorry, I was lost in deep thought about modes (read as: taking a nap ;) ) when you pinged me on IRC. No problem, and not surprising given the time ;-) I shamelessly made a few tiny pushes since you told me you extract out diff using a script and dont mind email spam ;) I even prefer tiny pushes, thanks for doing it this way! convert_regex thing regarding [^}] was useful i stumbled on a testcase which needed it to be used. :) ;-) Regards, Christian Boltz -- Was ist das, Nacht? Das ist der Zeitraum, in dem Du effektiv administrieren kannst. Weil anscheinend die User alle total faul sind, und sich ausgeloggt haben. [Wilfried Kramer] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] apparmor: add the ability to report a crypto hash of loaded policy
Hello, Am Donnerstag, 8. August 2013 schrieb John Johansen: Provide userspace the ability to validate what policy is loaded via an exported crypto hash value. Just a small issue: diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 80050b3..bb7acc7 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -66,6 +67,7 @@ struct aa_ext { u32 version; }; + /* audit callback for unpack fields */ static void audit_cb(struct audit_buffer *ab, void *va) { Was adding the second empty line intentional? Regards, Christian Boltz -- Ich hab da nochma ne Frage! :o) Was is eigentlich en DAU? Ich mein ihr sagt mir zwar die ganze Zeit das ich das bin, aber was das is wes ich ni! *heul* Ich rate ganz einfach ma!;o) Die Allercoolste Userin? Isses das? Ohhh danke danke! Du solltest doch nicht so viele von diesen bunten Pillen auf einmal schlucken... [Mickimausi und Axel Woelke in dag°] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [RFC] handling XDG user directories
Hello, your proposal looks very good. Nevertheless, let me add some notes (as usual ;-) Am Dienstag, 6. August 2013 schrieb John Johansen: On 08/05/2013 03:59 PM, Jamie Strandboge wrote: and users/admins can adjust /etc/apparmor.d/tunables/xdg-dirs or drop files into /etc/apparmor.d/tunables/xdg-dirs.d, providing a welcome convenience[2]. I know that people like the drop in dir bits, but quite bluntly I don't, for most things, its a way of papering over real problems (of course I consider treating profiles the way we do with packaging as a problems so ...) Tell us a better way ;-) This of course doesn't solve everything. Because users can modify theirrs ~/.config/user-dirs.dirs file at will and have it point anywhere, so we can't examine those files and do anything automatic there (when we have user policy we can revisit this). This proposal handles translations well though and use of This depends on what you are trying to enforce. If its a system level policy that is anything but advisory we can't ever let the user control the location. Exactly - otherwise things will become funny when a user sets XDG_DOWNLOAD_DIR=/ (wow, my browser can write everywhere! :-) If we allow the user config to be read, then it needs some restrictions: - only inside @{HOME} (and no ../ allowed) (even if I can guarantee you that users will complain about this ;-) - there must be a config option to disable reading ~/.config/, maybe we should even disable it by default If its a system level policy enforcing an advisory location for the user circumsribed by some larger control eg. @{HOME/** intersected with user defined @{HOME}/@{XDG_DESKTOP_DIR} yes, something like that - and also at least #include abstractions/private-files # or ...-strict If you mean that a user can set their translations sure, but again that doesn't currently reflect what system policy does. The sys admin can choose the set of local translations that are acceptable to system policy good point, see below * apparmor-xdg-dirs.py: this takes the output of 'locale -a' and I'm afraid this will result in a bit too much ;-) On my system, locale -a gives me 270 locales from aa_DJ to zu_ZA (and I even dropped suffixes like @euro or .utf-8 - with them, I get 460 locales) [1] In other words: this should be configurable: a) autogenerate for all installed languages (which would be a lot on my system) b) autogenerate for all languages in $config_option c) similar to b), but somehow automated (on openSUSE, you can choose to install for example all german translations in YaST - this should also add the german XDG dirs to apparmor) d) do not autogenerate anything Option a) might even result in too many permissions - I'm quite sure in one of the 270 locales I have, for example ~/downloads translates to a directory name I have, and that should not be accessible ;-) The perfect solution would be to only allow the directory names in each user's language (so the profile would have /home/cb/Dokumente/ and /home/english/documents/ for example) - but I know that's not really easy to implement ;-) Regards, Christian Boltz [1] I have no idea why this happens, the only thing I can imagine is that openSUSE has some *-lang packages that contains all non-english texts, and locale -a picks up all of the languages from them. -- [...] is currently down due to a failure in the NAS system. [...] your NAS (network attached storage) Oh. I thought it stood for Networked Adrian Schröter :D [ Adrian Schröter and Jean Delvare in opensuse-buildservice] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r41..45
Hello, the review for r41..45 is attached (merged into one review). BTW: Following the moved code was quite interesting[tm], but still easier than completely reviewing the new aamode.py and logparser.py ;-) Regards, Christian Boltz -- Seit einiger Zeit ist ftp://mirrors.mathematik.uni-bi*l*f*ld.de down Offenbar. [...] Aber warum machst du dir Sorgen? Bi*l*f*ld existiert nicht, wieso sollte es dort dann ein Ftp-Server geben? [ Al Bogner und David Haller in suse-linux] revno: 45 revno: 44 revno: 43 revno: 42 revno: 41 === modified file 'Testing/aa_test.py' --- Testing/aa_test.py 2013-08-09 11:19:01 + +++ Testing/aa_test.py 2013-08-11 13:00:01 + def test_modes_to_string(self): -MODE_TEST = {'x': apparmor.aa.AA_MAY_EXEC, ... +MODE_TEST = {'x': apparmor.aamode.AA_MAY_EXEC, ... def test_string_to_modes(self): -MODE_TEST = {'x': apparmor.aa.AA_MAY_EXEC, ... +MODE_TEST = {'x': apparmor.aamode.AA_MAY_EXEC, # I still think test_modes_to_string() and test_string_to_modes() should share MODE_TEST ;-) === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-08-10 07:16:22 + +++ apparmor/aa.py 2013-08-11 17:46:05 + @@ -244,7 +184,7 @@ return os.path.realpath(path) def find_executable(bin_path): -Returns the full executable path for the binary given, None otherwise +Returns the full executable path for the given, None otherwise # was this comment change intentional? # (hmm, maybe Returns the full path for the given executable given, None otherwise?) @@ -3296,7 +2747,7 @@ profile_data[profile][hat]['initial_comment'] = initial_comment initial_comment = '' if filelist[file]['profiles'][profile].get(hat, False): -raise AppArmorException('Error: Multiple definitions for hat %s in profile %s.' %(hat, profile)) +pass#raise AppArmorException('Error: Multiple definitions for hat %s in profile %s.' %(hat, profile)) filelist[file]['profiles'][profile][hat] = True # huh? what's wrong with raising an exception here? @@ -3831,6 +3284,7 @@ def get_include_data(filename): data = [] +filename = profile_dir + '/' + filename if os.path.exists(filename): # includes do not have to be inside the profile_dir # see the IRC log at the end of this file for details # short version: there's also # #include foo- relative to base dir (typically /etc/apparmor.d) # #include /path/to/foo - absolute path # (both with optional quotes around the filename) === added file 'apparmor/aamode.py' --- apparmor/aamode.py 1970-01-01 00:00:00 + +++ apparmor/aamode.py 2013-08-11 17:46:05 + @@ -0,0 +1,268 @@ +AA_MAY_EXEC = set('x') ... +AA_EXEC_UNSAFE = set('z') # 'z' is currently unused, but nevertheless there's a small risk here (it will break if we ever introduce 'z' permissions, whatever that will be) +AA_EXEC_INHERIT = set('i') +AA_EXEC_UNCONFINED = set('U') +AA_EXEC_PROFILE = set('P') +AA_EXEC_CHILD = set('C') # I'd directly use ix/Ux/Px/Cx, so you don't need to always add AA_MAY_EXEC +AA_EXEC_NT = set('N') # similar problem as with 'z' +AA_LINK_SUBSET = set('ls') # similar problem (for the 's' part) as with 'z' === modified file 'apparmor/common.py' --- apparmor/common.py 2013-08-10 07:16:22 + +++ apparmor/common.py 2013-08-11 17:46:35 + @@ -194,13 +195,21 @@ self.debug_level = logging.DEBUG if os.getenv('LOGPROF_DEBUG', False): self.debugging = os.getenv('LOGPROF_DEBUG') +try: +self.debugging = int(self.debugging) +except: +self.debugging = False +if self.debugging not in range(1,4): +sys.stderr.out('Environment Variable: LOGPROF_DEBUG contains invalid value: %s' %os.getenv('LOGPROF_DEBUG')) # I'd also allow 0 (for people who fail to unset the variable and set it to 0 instead) ;-) === added file 'apparmor/logparser.py' --- apparmor/logparser.py 1970-01-01 00:00:00 + +++ apparmor/logparser.py 2013-08-11 09:52:07 + @@ -0,0 +1,379 @@ +def parse_event(self, msg): ... +# Map c (create) to a and d (delete) to w, logprof doesn't support c and d +if rmask: +rmask = rmask.replace('c', 'a') +rmask = rmask.replace('d', 'w') +if not validate_log_mode(hide_log_mode(rmask)): +pass#fatal_error(_('Log contains unknown mode %s') % rmask) # I'd at least make that a debug notice or warning +if dmask: +dmask = dmask.replace('c', 'a') +dmask = dmask.replace('d', 'w') +if not validate_log_mode(hide_log_mode(dmask)): +pass #fatal_error(_('Log contains unknown mode %s') % dmask) # I'd at least make
[apparmor] GSoC r46..47 review
Hello, see /dev/null for the r46 and r47 review. (In other words: looks good, I don't have anything to complain ;-) Regards, Christian Boltz -- cat /inhalt/der/mail | mail -s mein subject [...] Ist der Useless Use of Cat Award diese Woche schon vergeben? ;-) [ Andreas Feile und Martin Schmitz in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC - remaining parts of old reviews
Hello, the attached files contain the remaining parts of old reviews. I always remove things that are done, and also checked the remaining parts in the r40 code (that's why most files have checked-in-r40) - but even with this suffix, they should be up to date with r47. I hope the number of files doesn't shock you too much - most of the files are quite small ;-) If you have questions or think some things need to be discussed, just ask ;-) Regards, Christian Boltz -- und *echte* Männer benutzen Linux -- wegen der langen Kommandozeilen (Meine ist länger als deine!). Dann muss man nicht mehr Krieg spielen, um zu zeigen, wie hart man ist. [S. Lauterkorn in suse-talk] review of r13, r14 and r15 === added file 'apparmor/aa.py' --- apparmor/aa.py 1970-01-01 00:00:00 + +++ apparmor/aa.py 2013-07-06 13:27:06 + +CONFDIR = '/etc/apparmor' ### It's just a path, but you should keep it at _one_ place (config.py). ### (besides that, CONFDIR is unused in aa.py) +unimplemented_warning = False ### set this to true in debugging mode? ### (right now, unimplemented_warning is unused) +aa = dict() # Profiles originally in sd, replace by aa +original_aa = dict() ### aa and original_aa aren't very self-explaining variable names - can you give them a better name? # They basically contain the list of profiles present version and # the one at the time of loading # ### Nice, but the variable name should represent that. What about profile_list and original_profile_list? +def get_profile_filename(profile): +Returns the full profile name +filename = profile +if filename.startswith('/'): +# Remove leading / +filename = filename[1:] +else: +filename = profile_ + filename +filename.replace('/', '.') ### should we replace more chars? ### just imagine someone has ~/bin/bad $$ script ? for * stars ### (ok, extreme example - but you want a profile for this for sure ;-) ### ### I'm not sure about backward compability if we change this (John?) ### but people could also have a profile for /bin/foo in /etc/apparmor.d/mybar ### so we can't really expect the /bin/foo profile in bin.foo #(r45) get_profile_filename is now in logparser.py +def complain(path): +Sets the profile to complain mode if it exists +filename, name = name_to_prof_filename(path) ### see above - you can't rely on having the proifle for /bin/foo in bin.foo ### (IIRC there's even an open bugreport about this) +def enforce(path): +Sets the profile to complain mode if it exists +filename, name = name_to_prof_filename(path) ### same here +def head(file): +Returns the first/head line of the file +first = '' +if os.path.isfile(file): +with open_file_read(file) as f_in: +first = f_in.readline().rstrip() +return first ### should head() raise an error if file doesn't exist? ### (or does open_file_read() do this already?) +def get_output(params): +Returns the return code output by running the program with the args given in the list +program = params[0] +args = params[1:] +ret = -1 +output = [] +# program is executable +if os.access(program, os.X_OK): +try: +# Get the output of the program +output = subprocess.check_output(params) +except OSError as e: +raise AppArmorException(Unable to fork: %s\n\t%s %(program, str(e))) +# If exit-codes besides 0 +except subprocess.CalledProcessError as e: +output = e.output +output = output.decode('utf-8').split('\n') ### what happens if someone is not using utf-8? +ret = e.returncode +else: +ret = 0 +output = output.decode('utf-8').split('\n') +# Remove the extra empty string caused due to \n if present +if len(output) 1: +output.pop() +return (ret, output) +def get_reqs(file): +Returns a list of paths from ldd output +pattern1 = re.compile('^\s*\S+ = (\/\S+)') +pattern2 = re.compile('^\s*(\/\S+)') +reqs = [] +ret, ldd_out = get_output(ldd, file) ### ldd is None - you should fill it before using it ;-) ### also, I'm not sure if ldd needs to be a global variable # The setter method is yet to come. ;-) # Will fix it when refractoring the module later as I planned. :-) === added file 'apparmor/common.py' --- apparmor/common.py 1970-01-01 00:00:00 + +++ apparmor/common.py 2013-07-03 23:34:04 + +def cmd_pipe(command1, command2): +'''Try to pipe command1 into command2.''' +try: +sp1 = subprocess.Popen(command1, stdout=subprocess.PIPE) +sp2 = subprocess.Popen(command2, stdin=sp1.stdout) +except OSError as ex: +return [127, str(ex)] + +if sys.version_info[0] = 3: +out = sp2.communicate()[0].decode('ascii', 'ignore') ### is ascii correct here, or should it be the system locale
[apparmor] GSoC review r48..51
Hello, the review for r48, 49, 50 and 51 is attached. Of course feedback to all the code is always welcome (I don't have a monopoly on reviewing GSoC code ;-) but there's a detail I'd like to discuss: aa-genprof.py has: if os.path.exists('/var/log/audit/audit.log'): syslog = False I'm not sure if audit.log exists is the best way to choose the logfile but I have to admit that I don't have a better method ;-) Does someone have any better ideas? Or is the current way ok? Regards, Christian Boltz -- But you are probably also complaining if local root exploits in the kernel are fixed, because now you no longer can use that to become root easily... [Stefan Seyfried in opensuse-factory] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r52 and r53
Hello, the review for r52 and r53 is attached. For the flags handling, I have an interesting question: Should we prefer to add the flags (for example complain) to the profile as we do now, or should we prefer to create a symlink in /etc/apparmor.d/force-complain/ ? The flags=(...) in the profile has the advantage that people are used to it, OTOH creating a symlink means we don't need to modify the profile. Opinions? (We'll have to contunue supporting both ways, the question is what aa-complain, aa-audit etc. should do.) Regards, Christian Boltz -- Der fünfte apokalyptische Reiter der Digitalen Inkompatibilität wird dich mit den hornigen Hufen des Workarounds niedertrampeln, und niemand wird in der Nacht deine Fehlermeldungen lesen. Wir aber werden an einem warmen Feuer sitzen, in dem deine Sourcen verbrennen, und uns der Kommandozeile erfreuen. So. [Ratti in fontlinge-devel] revno: 52 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Mon 2013-08-26 00:23:59 +0530 message: Merged aa-audit, aa-autodep, aa-complain, aa-disable, aa-enforce to share the common code into a tools.py module. Added -r/--remove feature to aa-complain, aa-enforce, aa-audit and -r/--revert feature to aa-disable. Some other fixes from review 48..51 revno: 53 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Mon 2013-08-26 00:41:15 +0530 message: aa-cleanprof tool === modified file 'Tools/aa-audit.py' --- Tools/aa-audit.py 2013-08-21 05:56:09 + +++ Tools/aa-audit.py 2013-08-25 18:53:59 + +audit.check_profile_dir() # $tool.check_profile_dir should be part of apparmor/tools.py # besides that, I like the new mini-tools :-) === modified file 'Tools/aa-autodep.py' --- Tools/aa-autodep.py 2013-08-21 05:56:09 + +++ Tools/aa-autodep.py 2013-08-25 18:53:59 + -parser = argparse.ArgumentParser(description='Disable the profile for the given programs') +parser = argparse.ArgumentParser(description='') # a description would be nice ;-) +autodep.check_profile_dir() # $tool.check_profile_dir should be part of apparmor/tools.py === added file 'Tools/aa-cleanprof.py' --- Tools/aa-cleanprof.py 1970-01-01 00:00:00 + +++ Tools/aa-cleanprof.py 2013-08-25 18:53:59 + # aa-cleanprof should also use apparmor/tools.py # (the code looks quite similar, it even has the wrong error message if the profiledir doesn't exist ;-) +if os.path.exists(program): # This check means aa-cleanprof works only if you have the program on your computer - # you can't cleanup unused profiles. # That's not the best idea ;-) # you should check for profile for program exists instead. === modified file 'Tools/aa-complain.py' --- Tools/aa-complain.py2013-08-21 05:56:09 + +++ Tools/aa-complain.py2013-08-25 18:53:59 + +complain.check_profile_dir() # $tool.check_profile_dir should be part of apparmor/tools.py === modified file 'Tools/aa-disable.py' --- Tools/aa-disable.py 2013-08-21 05:56:09 + +++ Tools/aa-disable.py 2013-08-25 18:53:59 + +disable.check_profile_dir() +disable.check_disable_dir() # $tool.check_profile_dir should be part of apparmor/tools.py # (and also check_disable_dir() (wrapped with an if condition) to have all code at the same place) === modified file 'Tools/aa-enforce.py' --- Tools/aa-enforce.py 2013-08-21 05:56:09 + +++ Tools/aa-enforce.py 2013-08-25 18:53:59 + +parser.add_argument('-r', '--remove', action='store_true', help='remove enforce mode') # maybe switch to complain mode would be a better help text +enforce.check_profile_dir() # $tool.check_profile_dir should be part of apparmor/tools.py # (did I already mention that I love copypaste reviews? ;-) +args = parser.parse_args() + +enforce = aa_tools('enforce', args) # enforce is not a real flag - basically it translates to not complain. # live would be easier in apparmor/tools.py if you do # # args.remove = not args.remove # invert the remove flag # enforce = aa_tools('complain', args) === modified file 'Tools/aa-unconfined.py' --- Tools/aa-unconfined.py 2013-08-21 05:56:09 + +++ Tools/aa-unconfined.py 2013-08-25 18:53:59 + @@ -48,22 +48,22 @@ if not attr: -if re.search('^(/usr)?/bin/(python|perl|bash)', prog): -cmdline = re.sub('\0', ' ', cmdline) +if re.search('^(/usr)?/bin/(python|perl|bash|sh)', prog): # this still doesn't cover the etc. part of sh etc. ;-) # (hint: apparmor/aa.py contains some more shells, and it would be a good idea to have the list of shells at _one_ place) # besides that, the regex will also cover /bin/pythonhunter and /usr/bin/shit ;-) +cmdline = re.sub('\x00', ' ', cmdline) cmdline = re.sub('\s+$', '', cmdline).strip() +if 'perl' in cmdline
Re: [apparmor] [patch] make __init__.py GSoC-ready
Hello, Am Donnerstag, 12. September 2013 schrieb Christian Boltz: to make testing Kshitij's new tools easier, I propose to merge his code in utils/apparmor/__init__.py - that's the only filename conflict (at least in the 2.8 branch). If we do this, we can ship his new tools in a testing package that can be installed on top of the 2.8.x packages without problems [1]. The following patch is for trunk and the 2.8 branch: As usual ;-) I found a way to break it - using LANG=C caused an exception because C is shorter than what filename = ...[0:2] expects. Here's an updated patch. Changes: - filename = ... moved into the try block - catch all exceptions, not only IOError I know it's not perfect, but worst thing that can happen is to get the non-translated interface. That's much better than an exception ;-) === modified file 'utils/apparmor/__init__.py' --- utils/apparmor/__init__.py 2012-05-08 05:37:48 + +++ utils/apparmor/__init__.py 2013-09-12 15:10:50 + @@ -1,9 +1,25 @@ # -- # #Copyright (C) 2011-2012 Canonical Ltd. +#Copyright (C) 2013 Kshitij Gupta # #This program is free software; you can redistribute it and/or #modify it under the terms of version 2 of the GNU General Public #License published by the Free Software Foundation. # # -- + +import gettext +import locale + +def init_localisation(): +locale.setlocale(locale.LC_ALL, '') +#cur_locale = locale.getlocale() +try: +filename = '/usr/share/locale/%s/LC_MESSAGES/apparmor-utils.mo' % locale.getlocale()[0][0:2] +trans = gettext.GNUTranslations(open( filename, 'rb')) +except: # IOError: +trans = gettext.NullTranslations() +trans.install() + +init_localisation() Note that the Canonical copyright header in __init.py__ was basically for the empty file - I'm not sure if it makes sense to keep it ;-) (but I won't remove it unless explicitely asked to do so to avoid Canonical will sue me for stealing an (besides the copyright header) empty file ;-) Also note that trunk also needs changes in utils/apparmor/common.py - Kshitij added several functions there. But that's another story and not too urgent. However we should do it before the 3.0 release. [1] We'll need to re-add the .py suffix for the tools in the testing package, but that's another story ;-) BTW: I'll also commit this patch to openSUSE Factory which has feature freeze today ;-) Regards, Christian Boltz -- Jan, I can write scripts too :) rebuild_logic=coolo [ Stephan Kulow and Marcus Meissner in opensuse-factory] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] utils/po/de.po update
Hello, after seeing a very strange translation, I did a round of proofreading on utils/po/de.po. The most strange translation was: msgid Reading log entries from %s. -msgstr %s Mailserver-Domains werden eingelesen... +msgstr Protokolleinträge von %s werden eingelesen. but there were more things that were mistranslated (but not as bad as the one quoted above ;-) or needed minor adjustments. I also marked some translations as fuzzy because I'm not too happy with them, but need to think about better alternatives. The most interesting question is if capability should be translated to Funktion - I somehow doubt... See the attached patch for all changes. I propose this patch for trunk and the 2.8 tree. Regards, Christian Boltz PS: Note to myself: avoid to use lokalize - it changes unmodified texts to multiline format even if I didn't touch them, which results in an unreadable diff :-/ The attached patch is of course cleaned up ;-) -- Erstes Gesetz WWW: Du mögest trennen die Spinnen und Indianer von den Usern und jedem sein eigen Grund und Heim zuteilen auf das der eine nicht neidisch werde auf den anderen und begehre dessen Heim und Gut. *lach* [Thomas Templin in suse-linux] === modified file 'utils/po/de.po' --- utils/po/de.po 2011-02-09 00:29:59 + +++ utils/po/de.po 2013-09-13 19:12:39 + @@ -1,19 +1,23 @@ # Copyright (C) 2006 SuSE Linux Products GmbH, Nuernberg +# Copyright (C) 2013 Christian Boltz # This file is distributed under the same license as the package. # msgid msgstr Project-Id-Version: apparmor-utils\n Report-Msgid-Bugs-To: apparmor-gene...@forge.novell.com\n POT-Creation-Date: 2008-09-22 22:56-0700\n -PO-Revision-Date: 2009-02-05 13:38\n -Last-Translator: Novell Language langu...@novell.com\n +PO-Revision-Date: 2013-09-13 21:05+0200\n +Last-Translator: Christian Boltz appar...@cboltz.de\n Language-Team: Novell Language langu...@novell.com\n MIME-Version: 1.0\n Content-Type: text/plain; charset=UTF-8\n Content-Transfer-Encoding: 8bit\n +Language: de\n +Plural-Forms: nplurals=2; plural=(n != 1);\n #: ../genprof:69 +#, fuzzy msgid Please enter the program to profile: msgstr Geben Sie das Programm fÃŒr das Profil ein: @@ -52,12 +57,12 @@ #: ../logprof:72 #, perl-format msgid usage: %s [ -d /path/to/profiles ] [ -f /path/to/logfile ] [ -m \mark in log to start processing after\ -msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ -f /pfad/zu/protokolldatei ] [ -m \markierng im protokoll, nach der die verarbeitung gestartet werden soll\ +msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ -f /pfad/zu/protokolldatei ] [ -m \Markierng im Protokoll, nach der die Verarbeitung gestartet werden soll\ #: ../autodep:63 #, perl-format msgid Can't find AppArmor profiles in %s. -msgstr In %s wurden keine UnterdomÀnenprofile gefunden. +msgstr In %s wurden keine AppArmor-Profile gefunden. #: ../autodep:71 msgid Please enter the program to create a profile for: @@ -86,7 +91,7 @@ #: ../audit:131 #, perl-format msgid usage: %s [ -d /path/to/profiles ] [ program to switch to audit mode ] -msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ programm, das in den prÃŒfmodus versetzt werden soll ] +msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ Programm, das in den PrÃŒfmodus versetzt werden soll ] #: ../complain:64 msgid Please enter the program to switch to complain mode: @@ -100,7 +105,7 @@ #: ../complain:131 #, perl-format msgid usage: %s [ -d /path/to/profiles ] [ program to switch to complain mode ] -msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ programm, das in den meldungsmodus versetzt werden soll ] +msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ Programm, das in den Meldungsmodus versetzt werden soll ] #: ../enforce:64 msgid Please enter the program to switch to enforce mode: @@ -109,12 +114,12 @@ #: ../enforce:105 ../AppArmor.pm:592 #, perl-format msgid Setting %s to enforce mode. -msgstr Einstellungen %s fÃŒr Erwzingungsmodus +msgstr %s wird in den Erwzingen-Modus versetzt. #: ../enforce:131 #, perl-format msgid usage: %s [ -d /path/to/profiles ] [ program to switch to enforce mode ] -msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ programm, das in den erzwingen-modus versetzt werden soll ] +msgstr Syntax: %s [ -d /pfad/zu/profilen ] [ Programm, das in den Erzwingen-Modus versetzt werden soll ] #: ../unconfined:50 #, perl-format @@ -193,7 +198,7 @@ #: ../AppArmor.pm:1159 msgid Select which of the changed profiles you would like to upload\nto the repository -msgstr WÀhlen Sie die geÀnderten Profile aus, die Sie an das Repository \nhochladen möchten +msgstr WÀhlen Sie die geÀnderten Profile aus, die Sie in das Repository \nhochladen möchten #: ../AppArmor.pm:1161 msgid Changed profiles @@ -210,7 +215,7 @@ #: ../AppArmor.pm:1236 ../AppArmor.pm:1316 #, perl-format msgid WARNING: An error occured while
[apparmor] GSoC review r58
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 + +++ Testing/minitools_test.py 2013-09-12 09:12:15 + @@ -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
[apparmor] [patch] ntpd needs read access to openssl.cnf
Hello, I just received the following patch and propose it for 2.8 and trunk: Patch-Author: Stefan Seyfried seife+...@b1-systems.com After this change in ntp: * Mo Aug 19 2013 crrodrig...@opensuse.org - Build with -DOPENSSL_LOAD_CONF , ntp must respect and use the system's openssl configuration. we need to read openssl.cnf or starting of ntpd will fail silently(!) Patch v2 by Christian Boltz: use abstractions/openssl instead of allowing /etc/ssl/openssl.cnf directly === modified file 'profiles/apparmor.d/usr.sbin.ntpd' --- profiles/apparmor.d/usr.sbin.ntpd 2011-08-08 20:16:06 + +++ profiles/apparmor.d/usr.sbin.ntpd 2013-09-16 20:28:39 + @@ -14,6 +14,7 @@ /usr/sbin/ntpd { #include abstractions/base #include abstractions/nameservice + #include abstractions/openssl #include abstractions/xad capability dac_override, Regards, Christian Boltz -- No need to use Windows -- it's easier to go through the door. [author unknown] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] ntpd needs read access to openssl.cnf
Hello, Am Montag, 16. September 2013 schrieb Steve Beattie: On Mon, Sep 16, 2013 at 10:39:13PM +0200, Christian Boltz wrote: I just received the following patch and propose it for 2.8 and trunk: Patch-Author: Stefan Seyfried seife+...@b1-systems.com After this change in ntp: * Mo Aug 19 2013 crrodrig...@opensuse.org - Build with -DOPENSSL_LOAD_CONF , ntp must respect and use the system's openssl configuration. we need to read openssl.cnf or starting of ntpd will fail silently(!) Though ntpd failing silently sounds like an ntpd bug to me. Indeed - I'll ask Seife if he can open a bugreport about it (probably in ntp upstream). Regards, Christian Boltz -- Ich bin beeindruckt! Windows startet nicht mehr - Problem gelöst. Ich wünschte, ich könnte meine Probleme auch so befriedigend lösen. [Sandy Drobic in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] Revert r1225 mistranslations (utils/po/*.po)
Hello, during the last days, we (as in: the usual people in #apparmor) discovered that the r1225 translation update introduced _lots_ of mistranslations in various languages, even for texts that had correct translations before. Also, most of the changes I commited for de.po recently fixed things that were introduced in r1225. Therefore I propose to completely [1] revert r1225 in trunk and also in the 2.8 branch. Note that r1225 is a commit from 2009, so it's likely that we'll get some merge conflicts. We can easily solve conflicts by removing the translated text so that translators get a second chance. The alternative is to mark _all_ texts in _all_ languages as fuzzy, so that translators see them as please check this. The disadvantages of this way are - we keep the mistranslations until translators have checked the translations - not the best idea because some translations are really bad (and I can only judge on en_* and de - maybe the other languages are even more funny[tm]) - it's probably more work for translators than re-translating a few texts that are a victim of revert/merge conflicts The only advantage of marking everything as fuzzy is that we'll get proofreading for all texts which might also catch mistranslations from other commits. Opinions? Objections? (if you want to see the patch for this proposal: bzr diff -r1224..1225, then swap + and -) Regards, Christian Boltz [1] except de.po because I fixed it already -- Zwar sind CSS-Bugs kein Alleinstellungsmerkmal des Internet Explorers, jedoch beansprucht Microsoft seit vielen Jahren die Marktführerschaft. [Dirk Jesse auf http://www.highresolution.info/spotlight/entry/was_sie_ueber_css-frameworks_wissen_sollten/] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] utils/*.pod: fix broken URL
Hello, Am Donnerstag, 19. September 2013 schrieb Steve Beattie: On Thu, Sep 19, 2013 at 08:52:19PM +0200, Christian Boltz wrote: the following patch fixes broken URLs in various utils/*.pod files. (The broken URLs were introduced in r1582.) I propose this patch for trunk and for the 2.8 branch. Acked-by: Steve Beattie st...@nxnw.org for trunk and 2.8 There are a bunch of other instances to fix as well. I haven't verified if all are available in 2.8, but a similar sed -i conversion should happen there, too. Acked-by: Christian Boltz appar...@cboltz.de for trunk and 2.8 I should really use bzr log -v -r$rev when checking regressions to find the full breakage ;-) As discussed on #apparmor, I included your patch in my commit. BTW: your patch also worked in the 2.8 branch, with the exception that apparmor.vim.pod still lives in parser/ there. Regards, Christian Boltz -- [tables vs. css layout] please - we should not start another religious war here, unless the GNOME vs KDE and emacs vs vi wars are fought out ;-)). [Frank Sundermeyer in opensuse-wiki] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r66 and r67
Hello, the review for r67 is attached. It looks big, but mostly contains minor text changes ;-) r66 looks good - no need for a review file. Regards, Christian Boltz -- [submit-request #65647 declined by saschpe:] description is 400 lines, too long :-) Where is a limit documented? [Stephan Kulow in opensuse-packaging] revno: 67 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sat 2013-09-21 01:08:34 +0530 message: fixes from rev65 === modified file 'Translate/README' --- Translate/README2013-09-20 13:50:41 + +++ Translate/README2013-09-20 19:38:34 + # might need another update to use the easier available ;-) xgettext # # Syntax: # xgettext --language=Python --keyword=_ --output=Translate/messages.pot apparmor/*.py Tools/aa-* # (run from the main directory, not inside Translate) # xgettext will also give you a list of all texts containing multiple %s ;-) === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-09-20 13:50:41 + +++ apparmor/aa.py 2013-09-20 19:38:34 + @@ -340,10 +343,12 @@ Modifies the profile to add the requirements reqs_processed = dict() reqs = get_reqs(path) +print(reqs) # debug code? @@ -2572,7 +2575,7 @@ +raise AppArmorException(_('%s profile in %s contains syntax errors in line: %s.') % (profile, file, lineno+1)) # line %s (without : ) @@ -2624,7 +2627,7 @@ elif RE_PROFILE_END.search(line): # If profile ends and we're not in one if not profile: -raise AppArmorException('Syntax Error: Unexpected End of Profile reached in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected End of Profile reached in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) # while at it, you should use %(file)s and %(lineno + 1)s - not sure if %(lineno + 1)s works... # (also in the following copypaste review notes, even if I didn't paste this note everywhere ;-) @@ -2639,7 +2642,7 @@ matches = RE_PROFILE_CAP.search(line).groups() if not profile: -raise AppArmorException('Syntax Error: Unexpected capability entry found in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected capability entry found in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) @@ -2658,7 +2661,7 @@ matches = RE_PROFILE_LINK.search(line).groups() if not profile: -raise AppArmorException('Syntax Error: Unexpected link entry found in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected link entry found in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) @@ -2686,7 +2689,7 @@ matches = RE_PROFILE_CHANGE_PROFILE.search(line).groups() if not profile: -raise AppArmorException('Syntax Error: Unexpected change profile entry found in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected change profile entry found in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) @@ -2708,7 +2711,7 @@ matches = RE_PROFILE_RLIMIT.search(line).groups() if not profile: -raise AppArmorException('Syntax Error: Unexpected rlimit entry found in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected rlimit entry found in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) @@ -2719,7 +2722,7 @@ matches = RE_PROFILE_BOOLEAN.search(line) if not profile: -raise AppArmorException('Syntax Error: Unexpected boolean definition found in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected boolean definition found in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) @@ -2759,7 +2762,7 @@ matches = RE_PROFILE_PATH_ENTRY.search(line).groups() if not profile: -raise AppArmorException('Syntax Error: Unexpected path entry found in file: %s line: %s' % (file, lineno+1)) +raise AppArmorException(_('Syntax Error: Unexpected path entry found in file: %s line: %s') % (file, lineno+1)) # file %s line %s (without : ) @@ -2783,10 +2786,10 @@ try: re.compile(p_re) except: -raise AppArmorException('Syntax Error: Invalid Regex %s in file: %s line: %s' % (path, file, lineno+1
[apparmor] GSoC review r68 and r69
Hello, the review for r68 is attached - it contains some small issues and a set of fresh bugs I found while doing some tests. r69 passed my review without complaints ;-) @John (and whoever else wants to answer) I also have an interesting question about behaviour of aa-audit /bin/ping The profile starts with /{usr/,}bin/ping { so it's, strictly speaking, two profiles in one. The interesting question is what the correct behaviour is because besides setting the profile for /bin/ping to audit mode (expected) it would also set the profile for /usr/bin/ping to audit mode (not expected) OTOH, the current behaviour of aa-audit (error message saying that /bin/ping does not exist) also is not expected and will terribly confuse users. What should aa-audit /bin/ping do in this case? (aa-audit is just an example - the same question applies to all aa-* tools that change profile flags.) Regards, Christian Boltz -- which camera is this? Marcus, this is my bug :) [Marcus Meissner and Stephan Kulow in https://bugzilla.novell.com/show_bug.cgi?id=217731] revno: 68 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sat 2013-09-21 12:36:51 +0530 message: Fixed flag reader and writer to be able to set unset flag for a specific target program also fixed tests for mini tools to be independent of existence of ntpd === modified file 'Tools/aa-complain' --- Tools/aa-complain 2013-09-19 05:02:19 + +++ Tools/aa-complain 2013-09-21 07:06:51 + @@ -11,5 +11,5 @@ complain = apparmor.tools.aa_tools('complain', args) - +print(args) # debugging code? === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-09-20 19:38:34 + +++ apparmor/aa.py 2013-09-21 07:06:51 + -def get_profile_flags(filename): +def get_profile_flags(filename, program): # To-Do # XXX If more than one profile in a file then second one is being ignored XXX # Do we return flags for both or # with the change below, the To-Do seems solved # you'll also need to fix # Tools/aa-genprof:apparmor.helpers[program] = apparmor.get_profile_flags(profile_filename) # to use two parameters @@ -564,13 +564,17 @@ with open_file_read(filename) as f_in: for line in f_in: if RE_PROFILE_START.search(line): -flags = RE_PROFILE_START.search(line).groups()[6] -return flags +matches = RE_PROFILE_START.search(line).groups() +profile = matches[1] or matches[3] +flags = matches[6] +if profile == program: +return flags raise AppArmorException(_('%s contains no profile')%filename) # with the above change, the error message should probably be # '%(filename)s contains no profile for %(program)s' -def change_profile_flags(filename, flag, set_flag): -old_flags = get_profile_flags(filename) +def change_profile_flags(filename, program, flag, set_flag): +old_flags = get_profile_flags(filename, program) +print(old_flags) # debugging code? # Testing results: # logprof doesn't expand aliases correctly: # (sorry for the german parts, but I'm quite sure you'll understand what is happening ;-) Profil: /{usr/,}bin/ping Pfad:/home/sys-var/run/nscd/dbVSXZwf Modus: r Schweregrad: 4 [1 - /home/sys-var/run/nscd/dbVSXZwf] 2 - /home/*/run/nscd/dbVSXZwf (A)llow / [(D)eny] / (I)gnore / (G)lob / Glob with (E)xtension / (N)ew / Abo(r)t / (F)inish / (M)ore Neuen Pfad eingeben: /var/run/nscd/db* The specified path does not match this log entry: Log Entry: /home/sys-var/run/nscd/dbVSXZwf Entered Path: /var/run/nscd/db* Do you really want to use this path? (J)a / [(N)ein] Profil: /{usr/,}bin/ping Pfad:/home/sys-var/run/nscd/dbVSXZwf Modus: r Schweregrad: 4 1 - /home/sys-var/run/nscd/dbVSXZwf 2 - /home/*/run/nscd/dbVSXZwf [3 - /var/run/nscd/db*] (A)llow / [(D)eny] / (I)gnore / (G)lob / Glob with (E)xtension / (N)ew / Abo(r)t / (F)inish / (M)ore Adding /var/run/nscd/db* r to profile Profil: /{usr/,}bin/ping Pfad:/home/sys-var/run/nscd/dbwvkZpy Modus: r Schweregrad: 4 # # cat /etc/apparmor.d/tunables/alias # alias /var/ - /home/sys-var/, # alias /tmp/ - /home/sys-tmp/, # # - logprof should not ask /home/sys-var/run/nscd/dbwvkZpy because it's covered by /var/run/nscd/db* (with the alias applied) # # in fact, it should not ask ask for /home/sys-var/run/nscd/db* at all because the /bin/ping profile includes abstractions/nameservice # which already contains /{,var/}run/nscd/db* rmix, # let's write one of the changed profiles... = Changed Local Profiles = The following local profiles were changed. Would you like to save them? [1 - /home/cb/bin/lj_make_galerie.sh] 2 - /usr/lib/Adobe/Reader9/Reader/intellinux/bin/acroread 3 - /{usr/,}bin/ping (S)ave Changes / Save
[apparmor] GSoC - updated reviews
Hello, I just reviewed all old reviews. The attached tar.gz contains everything that is still not fixed. (If you disagree with something, you can of course just tell me that I'm wrong ;-) To make things more interesting, I added some small issues that are related to the old reviews ;-) The attached review-r69 (I needed a filename ;-) contains another small bug - it's just a missing space, but causes invalid profiles ;-) Regards, Christian Boltz -- Look at Debian... its stable, works on a variety of platforms and development is racing along at the speed of a turtle with 3 broken legs. [Joseph M. Gaffney in opensuse] reviews.tar.gz Description: application/compressed-tar # (not really related to r69, but I needed a filename ;-) (apparmor/aa.py) def write_path_rules(prof_data, depth, allow): ... while user or other: ownerstr = '' tmpmode = 0 tmpaudit = False if user - other: # if no other mode set ownerstr = 'owner' # causes broken profiles - should be 'owner ' (space added) tmpmode = user - other tmpaudit = user_audit user = user - tmpmode else: if user_audit - other_audit user: ownerstr = 'owner ' -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r70..72
Hello, the (quite small) reviews for r70 and r72 are attached. The r70 review also contains two bugs I noticed. For r71, I have no reason to complain ;-) Regards, Christian Boltz -- Das Autofahrersyndrom: Prüft Euren Ton. *anschlag* *bonk* Stimmt, der Ton ist nicht sonderlich... [ Peter Lipp und David Haller in suse-linux] revno: 70 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sun 2013-09-22 15:01:34 +0530 message: So that closes the first proper version of aa-cleanprof with testcases added, fixed profile writer to work on multiple profiles at once, please use the view clean changes option in logprof and genprof, the comment preserver version needs tweaking that version wont be written anyways. Plus a few other changes === modified file 'Tools/aa-cleanprof' --- Tools/aa-cleanprof 2013-09-19 05:02:19 + +++ Tools/aa-cleanprof 2013-09-22 09:31:34 + @@ -7,6 +7,7 @@ parser = argparse.ArgumentParser(description='Cleanup the profiles for the given programs') parser.add_argument('-d', '--dir', type=str, help='path to profiles') parser.add_argument('program', type=str, nargs='+', help='name of program') +parser.add_argument('-s', '--silent', action='store_true', help='Silently over-write with cleanprofile') # ...with clean profile === modified file 'Tools/manpages/aa-cleanprof.pod' --- Tools/manpages/aa-cleanprof.pod 2013-09-20 19:38:34 + +++ Tools/manpages/aa-cleanprof.pod 2013-09-22 09:31:34 + +B-s --silent + + Silently over-writes the profile without user prompt. # overwrites # cleanprof test results (cleaning up the nptd profile) # network inet stream was removed, but network inet6 stream was not (both covered by abstractions/nameservice, so both should have been removed) # besides that, the cleaned profile looks good # (well, it shouldn't contain the allow keyword in all lines, but that's a known issue in the write method) # from testing logprof: # I used -d ./profiles, but when saving a profile (with the Save Selec(t)ed Profile option), it was written to /etc/apparmor.d/ vim:ft=diff revno: 72 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sun 2013-09-22 15:25:20 +0530 message: Added help messages to translate strings and a few other minor fixes === modified file 'Tools/aa-cleanprof' --- Tools/aa-cleanprof 2013-09-22 09:31:34 + +++ Tools/aa-cleanprof 2013-09-22 09:55:20 + -parser.add_argument('-s', '--silent', action='store_true', help='Silently over-write with cleanprofile') +parser.add_argument('-s', '--silent', action='store_true', help=_('Silently over-write with a clean profile')) # overwrite === modified file 'Tools/aa-mergeprof' --- Tools/aa-mergeprof 2013-09-19 05:02:19 + +++ Tools/aa-mergeprof 2013-09-22 09:55:20 + ##parser.add_argument('profiles', type=str, nargs=3, help='MINE BASE OTHER') -parser.add_argument('-auto', action='store_true', help='Automatically merge profiles, exits incase of *x conflicts') +parser.add_argument('-auto', action='store_true', help=_('Automatically merge profiles, exits incase of *x conflicts')) # should it be '--auto'? vim:ft=diff -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r75
Hello, the review for r75 is attached, with two bugs and a To-Do note included. Regards, Christian Boltz -- you are spending too much time in web forums or with apache guys if you are using +1 and -1 :-) [Stefan Seyfried in opensuse-factory] revno: 75 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Mon 2013-09-23 02:14:11 +0530 message: Fixed the netrule persistence issue in cleanprof, some elementary work for mergeprof === modified file 'Tools/aa-mergeprof' --- Tools/aa-mergeprof 2013-09-22 18:19:19 + +++ Tools/aa-mergeprof 2013-09-22 20:44:11 + @ -17,13 +16,12 @@ profiles = [args.mine, args.base, args.other] print(profiles) # debugging code? # bugs noticed: (from aa-cleanprof /usr/sbin/ntpd) [23:08:29] cboltz I'm afraid there is a real bug in cleanprof [23:08:35] cboltz it said Deleted 4 rules [23:08:47] cboltz but manually diffing the profile shows that it removed 5 rules ;-) [23:08:56] kshitij8 damn! you noticed that :P [23:09:35] kshitij8 I noticed that after the commit. # python aa-mergeprof /etc/apparmor.d/usr.sbin.ntpd ./profiles/usr.sbin.ntpd /dev/null ['/etc/apparmor.d/usr.sbin.ntpd', 'profiles/usr.sbin.ntpd', '/dev/null'] Traceback (most recent call last): File aa-mergeprof, line 72, in module main() File aa-mergeprof, line 24, in main mergeprofiles.clear_common() File aa-mergeprof, line 56, in clear_common user_other = cleanprofile.CleanProf(False, user, other) NameError: global name 'user' is not defined [23:22:53] cboltz oh, mergeprof can now at least print --help output (no syntax error anymore ;-) [23:23:24] cboltz it seems to enforce 3 parameters [23:23:46] cboltz I'd like to also have a way to merge only 2 profiles [23:25:32] kshitij8 that shouldn't be hard. I'll make the third param optional. [23:26:01] kshitij8 and other changes about it ofcourse. vim:ft=diff -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GSoC review r76..79
Hello, the reviews for r76..79 are attached. (No complaints about r76 and r78.) Regards, Christian Boltz -- Microsoft-Compatible Spongiforme Encephalitis? Setzt das nicht Hirn voraus? Irgendwo müssen doch all die Beschwörungsformeln hin, die man als MCSE auswendig lernen muß. Ein schwammförmiges Gehirn scheint mir dafür durchaus geeignet ... [Aran Kuntze, Gerhard Schromm und Martin Bienwald in dasr] revno: 79 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Mon 2013-09-23 21:00:36 +0530 message: (no message) === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-09-23 14:02:25 + +++ apparmor/aa.py 2013-09-23 15:30:36 + @@ -3030,8 +3030,12 @@ def set_allow_str(allow): if allow == 'deny': return 'deny ' -else: +elif allow == 'allow': return 'allow ' +elif allow == '': +return '' +else: +raise AppArmorException(_(Invalid allow string: %(allow)s)) # looks good (in theory), but in practise it seems all function calls pass in 'allow', not '' # which means we still get all lines prefixed with 'allow ' :-( # # Please change it to elif allow == 'allow': return '' # (better no allow than having allow everywhere ;-) vim:ft=diff revno: 77 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Mon 2013-09-23 19:32:25 +0530 message: Added first version of aa-mergeprof, does not include the check for conflicting ix rules yet === modified file 'apparmor/tools.py' --- apparmor/tools.py 2013-09-22 17:21:30 + +++ apparmor/tools.py 2013-09-23 14:02:25 + @@ -123,7 +123,7 @@ q = apparmor.hasher() q['title'] = 'Changed Local Profiles' q['headers'] = [] -q['explanation'] = _('The following local profiles were changed. Would you like to save them?') +q['explanation'] = _('The local profile for %s in file %s was changed. Would you like to save it?') %(program, filename) # good idea, but # - you should use %(program)s and %(filename)s instead of %s # - a \n wouldn't hurt ;-) # review of aa-mergeprof postponed until ask_the_question() is re-integrated into aa.py (as discussed on IRC) # # in other words: ignore everything below this line ;-) === modified file 'Tools/aa-mergeprof' --- Tools/aa-mergeprof 2013-09-22 22:17:15 + +++ Tools/aa-mergeprof 2013-09-23 14:02:25 + @@ -16,39 +18,70 @@ def main(): mergeprofiles = Merge(profiles) #Get rid of common/superfluous stuff mergeprofiles.clear_common() +if not args.auto: +mergeprofiles.ask_the_questions('other') + +mergeprofiles.clear_common() +print(base) +mergeprofiles.ask_the_questions('base') + +q = apparmor.aa.hasher() +q['title'] = 'Changed Local Profiles' +q['headers'] = [] +q['explanation'] = _('The following local profiles were changed. Would you like to save them?') +q['functions'] = ['CMD_SAVE_CHANGES', 'CMD_VIEW_CHANGES', 'CMD_ABORT'] +q['default'] = 'CMD_VIEW_CHANGES' +q['options'] = [] +q['selected'] = 0 +p =None +ans = '' +arg = None +programs = list(mergeprofiles.user.aa.keys()) +program = programs[0] +while ans != 'CMD_SAVE_CHANGES': +ans, arg = apparmor.aa.UI_PromptUser(q) +if ans == 'CMD_SAVE_CHANGES': +apparmor.aa.write_profile_ui_feedback(program) +apparmor.aa.reload_base(program) +elif ans == 'CMD_VIEW_CHANGES': +for program in programs: +apparmor.aa.original_aa[program] = apparmor.aa.deepcopy(apparmor.aa.aa[program]) +#oldprofile = apparmor.serialize_profile(apparmor.original_aa[program], program, '') +newprofile = apparmor.aa.serialize_profile(mergeprofiles.user.aa[program], program, '') + apparmor.aa.display_changes_with_comments(mergeprofiles.user.filename, newprofile) + @@ -64,453 +97,526 @@ +def ask_the_questions(self, other): +if other == 'other': +other = self.other +else: +other = self.base +#print(other.aa) + +#Add the file-wide includes from the other profile to the user profile +done = False +options = list(map(lambda inc: '#include %s' %inc, sorted(other.filelist[other.filename]['include'].keys( +q = apparmor.aa.hasher() +q['options'] = options +default_option = 1 +q['selected'] = default_option - 1 +q['headers'] = [_('File includes'), _('Select the ones you wish to add')] +q['functions'] = ['CMD_ALLOW', 'CMD_IGNORE_ENTRY', 'CMD_ABORT', 'CMD_FINISHED
[apparmor] GSoC review r80..84
Hello, the review for r80 is attached. Maybe I'll add some comments on the UI later after actually testing aa-mergeprof ;-) r81..84 look fine :-) Regards, Christian Boltz -- http://www1.giga.de/gigahelp/index_gigahelp/0,3597,,00.html | Leider scheint Euer Browser den Aufbau von Frames zu unterstützen ... *Leider?* :) Tut Lynx doch gar nicht. :) [Andreas Kneib in suse-linux] revno: 80 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Mon 2013-09-23 23:05:25 +0530 message: Fixes netrule deletion for includes === modified file 'Tools/aa-mergeprof' --- Tools/aa-mergeprof 2013-09-23 14:02:25 + +++ Tools/aa-mergeprof 2013-09-23 17:35:25 + @@ -97,6 +97,36 @@ base_other = cleanprofile.CleanProf(False, self.base, self.other) deleted += user_base.compare_profiles() +def conflict_mode(self, profile, hat, allow, path, mode, new_mode, old_mode): +conflict_modes = set('uUpPcCiIxX') # uppercase I should never appear (but it can't hurt to check for it nevertheless ;-) # also, I'm not aware of uppercase X +conflict_x= (old_mode | mode) conflict_modes +if conflict_x: +#We may have conflicting x modes +if conflict_x set('x'): +conflict_x.remove('x') +if conflict_x set('X'): +conflict_x.remove('X') +if len(conflict_x) 1: +q = apparmor.aa.hasher() +q['headers'] = [_('Path'), path] +q['headers'] += [_('Select the appropriate mode'), ''] +options = [] +options.append('%s: %s' %(mode, path, apparmor.aa.mode_to_str_user(apparmor.aa.flatten_mode((old_mode | new_mode) - (old_mode conflict_x) +options.append('%s: %s' %(mode, path, apparmor.aa.mode_to_str_user(apparmor.aa.flatten_mode((old_mode | new_mode) - (new_mode conflict_x) +q['options'] = options +q['functions'] = ['CMD_ALLOW', 'CMD_ABORT'] # I'll probably add a comment for the user interface after testing it, but it looks ok for now +done = False +while not done: +ans, selected = apparmor.aa.UI_PromptUser(q) +if ans == 'CMD_ALLOW': +if selection == 0: +self.user.aa[profile][hat][allow][path][mode] = (old_mode | new_mode) - (old_mode conflict_x) +elif selection == 1: +self.user.aa[profile][hat][allow][path][mode] = (old_mode | new_mode) - (new_mode conflict_x) +else: +raise apparmor.aa.AppArmorException(_('Unknown selection')) +done = True vim:ft=diff -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] force-complain symlinks break cache?
Hello, it seems using force-complain/ symlinks breaks cache usage: root@beta:/etc/apparmor.d/force-complain time rcapparmor reload redirecting to systemctl reload apparmor real0m0.791s user0m0.004s sys 0m0.000s Now let's create force-complain symlinks for all profiles: root@beta:/etc/apparmor.d/force-complain ln -s ../* . root@beta:/etc/apparmor.d/force-complain time rcapparmor reload redirecting to systemctl reload apparmor real0m17.267s user0m0.000s sys 0m0.004s root@beta:/etc/apparmor.d/force-complain time rcapparmor reload redirecting to systemctl reload apparmor real0m17.250s user0m0.000s sys 0m0.004s This is a server with openSUSE 13.1 beta with AppArmor 2.8.2. Regards, Christian Boltz -- Hier gibt es zB eine Adress-DB für einige Leute und allein schon die gleichzeitige Verwendung dieser DB ist eher die Ausnahme. Wahrscheinlich verdienen die Datenbanken hier die Bezeichnung gar nicht. Wenn du willst, kannst du auch dazu gemeinsam genutzter strukturierter Notizzettel sagen. [Al Bogner in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [Patch] cleanup usr.sbin.ntpd profile
Hello, this patch removes some rules from the ntpd profile that are already covered by abstractions: - the network rules are in abstractions/nameservice - /etc/gai.conf is also in abstractions/nameservice - @{PROC}/sys/kernel/ngroups_max is in abstractions/base I found those superfluous rules with aa-cleanup :-) but merged the changes manually to keep comments and rule sorting. @Kshitij: it would be nice if aa-cleanup would have an option to only delete superfluous rules _without_ removing comments and sorting the remaining rules ;-) === modified file 'profiles/apparmor.d/usr.sbin.ntpd' --- profiles/apparmor.d/usr.sbin.ntpd 2013-09-16 22:23:32 + +++ profiles/apparmor.d/usr.sbin.ntpd 2013-09-30 15:36:51 + @@ -27,10 +27,6 @@ capability sys_time, capability sys_nice, - network inet dgram, - network inet stream, - network inet6 stream, - /drift/ntp.drift rwl, /drift/ntp.drift.TEMP rwl, /etc/ntp.conf r, @@ -39,7 +35,6 @@ /etc/ntp/step-tickers r, /etc/ntpd.conf r, /etc/ntpd.conf.tmp r, - /etc/gai.conf r, /tmp/ntp* rwl, /usr/sbin/ntpd rmix, @@ -60,7 +55,6 @@ /{,var/}run/ntpd.pid w, /var/tmp/ntp* rwl, @{PROC}/@{pid}/net/if_inet6 r, - @{PROC}/sys/kernel/ngroups_max r, # allow access for when chrooted /var/lib/ntp/@{PROC}/@{pid}/net/if_inet6 r, Regards, Christian Boltz -- [GUI vs. Command-Line] Einen ähnlichen Streit wird es in 20 Jahren auch geben, wenn die 2D-Screenfanatiker auf die VR Fans losgehen und wieder ein Streit vom Zaun bricht der an Sinnfreiheit kaum zu überbieten ist. [Phillip Richdale in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 05/13] parser - rewrite caching tests in python unittest
Hello, [sorry for the slightly broken quoting - KMail needs some improvement when quoting overlong lines ;-) ] Am Donnerstag, 10. Oktober 2013 schrieb Steve Beattie: --- /dev/null +++ b/parser/tst/caching.py +from optparse import OptionParser# deprecated, should move to argparse eventually I love it when a patch introduces a new file that already comes with a deprecated notice. What about just switching to argparse? ;-) Lots of tests (all?) have +rc, report = testlib.run_cmd(cmd) +self.assertEquals(rc, 0, Got return code %d, expected 0\nOutput: %s % (rc, report)) testlib.run_cmd should have a parameter expected_returncode (default 0) and fail the test if it doesn't match. If you don't want to mix execution and result testing in one function, a separate function run_cmd_checkstatus() that calls testlib.run_cmd() and checks the return code also works. Even if it only saves one line per test - you have lots of tests ;-) It would remove some noise from all tests and makes sure the returncode is checked in every test. (With the current way, I wouldn't be too surprised if one of the tests forgets to check the returncode.) +def test_cache_not_loaded_when_features_differ(self): +'''test cache is not loaded when features file differs''' + +self._generate_cache_file() + +with open(os.path.join(self.cache_dir, '.features'), 'w+') as f: +f.write('monkey\n') It seems you have lots of bananas ;-) - adding the monkeys to the features file is also worth its own function IMHO. An even better alternative is to add an optional monkey parameter to _generate_cache_file() After reading the second half of the patch, I noticed that you also add monkeys to other files, so maybe (untested!) def add_monkey(filename): banana = os.path.join(self.cache_dir, filename) with open(banana, 'w+') as f: f.write('monkey') would be another good solution. +def test_cache_writing_updates_cache_file(self): ... +with open(cache_file, 'rb') as f: +cache_contents = f.read() +new_size = os.fstat(f.fileno()).st_size +# We check sizes here rather than whether the string monkey is +# in cache_contents because of the difficulty coercing cache +# file bytes into strings in python3 +self.assertNotEquals(orig_size, new_size, 'Expected cache file to be updated, got: \n%s' % cache_contents) Wait a minute - first you are afraid of diffing the file content because of handling binary stuff in python, and then you dump cache_contents (which contains the binary cache of the profile) to the terminal if the test fails. I'm quite sure nobody wants to have his terminal filled up with binary data, and I'm also sure nobody can read the binary dump without using tools. Instead, you should print both file sizes or just file size differs. This is the only critical thing - everything else is just cleanup which can go into a follow-up patch. With dumping the binary stuff to the terminal removed, and a promise to do the cleanups in a follow-up patch [1], Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz [1] you know I'm good at reminding you ;-) -- Ugly doesn't even begin to describe the knoppix init script system. [..] Some people should just be strung up by their short hairs and made to walk in the steps of those who must follow them before being allowed to code such monstrosities again. [Robison, Jonathon (M.)] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 13/13] parser - update README information
Hello, Am Donnerstag, 10. Oktober 2013 schrieb Steve Beattie: The README in the parser directory was woefully out of date; this patch updates the information to contain the current mail list, wiki, and bug tracking locations. That was an easy one to proofread ;-) Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Aber doch ... Woast Bub, ich denk bei sowas immer willkürlich an den Worst-case. Nämlich das das nicht ein Gscheidle wie du macht, sondern daß das irgendeiner hier oder in irgendeinem Forum aufschnappt. [David Haller in opensuse-de] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 03/13] parser - add simple file deny rule tests
Hello, Am Donnerstag, 10. Oktober 2013 schrieb Steve Beattie: Our simple language tests did not include any file deny rule tests. This patch adds a few simple ones. Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Oder kannst du dir ein AUto vorstellen das erst mit einem Benzinmotor fabriziert wird und dann wenn du es mit Diesel betankst auch noch fährt. *lach* [Thomas Templin in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 05/13] parser - rewrite caching tests in python unittest
Hello, Am Dienstag, 15. Oktober 2013 schrieb Steve Beattie: On Fri, Oct 11, 2013 at 10:08:51PM +0200, Christian Boltz wrote: We'll see if you still like this in some months... While I reserve the right to flake out^W^W change my mind, I help ;-) maintain and improve other codebases that don't get reviews before commits... and they sure could use it. ;-) Maybe it would be even easier with break_file instead of add_monkey as function name. OTOH, I'm quite sure people _will_ check what add_monkey() does, but nobody will read the code of break_file() ;-) The thing is, I often need the full path for the subsequent verification check as well, so pushing the os.path.join(self.cache_dir, ...) call into the helper function both limits the generality of the helper function (in some cases, that's okay) and doesn't save me very much because I need to do it again later. So in this case, I created a write_file() function that takes a path and a string and writes that string to the path. It's more general but means the path join occurs in the test function. What about this? def write_file(directory, filename, contents): '''write contents to path''' path = os.path.join(directory, path) with open(path, 'w+') as f: f.write(contents) return path This makes the tests a bit more readable, and if you need the filename later, you can use filename = write_file(...) Though, what I'd really like is to somehow set self.do_cleanup to False when any test fails, so that for test cases that fail, the temporary directory is left behind, to make diagnosing why it failed easier to do. I'll think about whether there's a reasonable way to do that. Looks like it isn't really nice or easy, but at least possible: http://stackoverflow.com/questions/4414234/getting-pythons-unittest- results-in-a-teardown-method http://www.piware.de/2012/10/python-unittest-show-log-on-test-failur e/ (the comments are also interesting) Thanks. A lot of those solutions are specific to adding information to the logged output, which is fine, but not what I'm after; I want to be able to potentially re-run the test manually to see why it's failing with a minimum of effort. That said, I was able to make the decorator function approach work. Patch history: v1: - initial version v2: - create template base class - add keep_on_fail() decorator to keep temporary test files around after a test fails I'd prefer if you can do this on a global level instead of having it on every test. google says it's possible, see http://stackoverflow.com/questions/6695854/writing-a-class-decorator-that-applies-a-decorator-to-all-methods and http://stackoverflow.com/questions/3467526/attaching-a-decorator-to-all-functions-within-a-class and some more, see https://www.google.com/search?q=python+decorator+all+functionsie=UTF-8 - create run_cmd_check wrapper to run_cmd that adds an assertion check based on whether return code matches the expected rc - similarly, add a check to run_cmd_check for verifying the output contains a specific string, also simplifying many of the caching tests. Looks good :-) I'd say commit the patch now and then do a follow-up patch for the things listed above. That's easier to review ;-) Assuming the patch consists of the previous patch + the changes listed in your next mail, Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Was ist das, Nacht? Das ist der Zeitraum, in dem Du effektiv administrieren kannst. Weil anscheinend die User alle total faul sind, und sich ausgeloggt haben. [Wilfried Kramer] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] updated usr.sbin.smbd profile
Hello, looks like the patch needs one additional line (inserted below), see https://bugzilla.novell.com/show_bug.cgi?id=845867#c4 Am Dienstag, 15. Oktober 2013 schrieb Christian Boltz: Am Dienstag, 15. Oktober 2013 schrieb Christian Boltz: some samba *.dat files were moved, and a new library needs to be loaded by smbd. It turns out more changes are needed for samba, also in the nmbd and winbindd profile. The reason is probably a major version update - openSUSE 13.1 ships samba 4.1, while 12.3 came with samba 3.6. Also fix /usr/lib*/samba/{lowercase,upcase,valid}.dat r, which should be lowcase instead of lowercase. Google didn't find any samba-related lowercase.dat and my ARCHIVES.gz archive shows that openSUSE 11.4 already used lowcase.dat, so removing lowercase shouldn't cause any problems. Nevertheless, I'll not remove lowercase in the 2.8 branch to be on the safe side. References: https://bugzilla.novell.com/show_bug.cgi?id=845867 References: https://bugzilla.novell.com/show_bug.cgi?id=846054 I propose this patch for trunk and the 2.8 branch, with the little difference for lowercase mentioned above. I also noticed that the winbindd profile does not use abstractions/samba (which would simplify the profile a lot), but that's something for another patch ;-) === modified file 'profiles/apparmor.d/abstractions/samba' --- profiles/apparmor.d/abstractions/samba 2011-08-26 23:52:27 + +++ profiles/apparmor.d/abstractions/samba 2013-10-15 19:54:07 + @@ -11,6 +11,7 @@ /etc/samba/* r, /usr/share/samba/*.dat r, + /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r, /var/lib/samba/**.tdb rwk, /var/log/samba/cores/ rw, /var/log/samba/cores/** rw, === modified file 'profiles/apparmor.d/usr.sbin.nmbd' --- profiles/apparmor.d/usr.sbin.nmbd 2013-01-02 23:31:01 + +++ profiles/apparmor.d/usr.sbin.nmbd 2013-10-15 19:54:34 + @@ -12,6 +12,7 @@ /usr/sbin/nmbd mr, /var/{cache,lib}/samba/browse.dat* rw, + /var/{cache,lib}/samba/gencache.dat rw, /var/{cache,lib}/samba/wins.dat* rw, /var/{cache,lib}/samba/smb_krb5/ rw, /var/{cache,lib}/samba/smb_krb5/krb5.conf* rw, === modified file 'profiles/apparmor.d/usr.sbin.smbd' --- profiles/apparmor.d/usr.sbin.smbd 2013-10-09 20:42:41 + +++ profiles/apparmor.d/usr.sbin.smbd 2013-10-15 19:54:27 + @@ -29,7 +29,8 @@ /usr/lib*/samba/vfs/*.so mr, /usr/lib*/samba/charset/*.so mr, /usr/lib*/samba/auth/script.so mr, - /usr/lib*/samba/{lowercase,upcase,valid}.dat r, + /usr/lib*/samba/pdb/*.so mr, + /usr/lib*/samba/{lowcase,upcase,valid}.dat r, /usr/sbin/smbd mr, /usr/sbin/smbldap-useradd Px, /var/cache/samba/** rwk, @@ -38,6 +39,7 @@ /{,var/}run/cups/cups.sock rw, /{,var/}run/dbus/system_bus_socket rw, /{,var/}run/samba/** rk, + /{,var/}run/samba/ncalrpc/ rw, + /{,var/}run/samba/ncalrpc/** rw, /{,var/}run/samba/smbd.pid rw, /var/spool/samba/** rw, === modified file 'profiles/apparmor.d/usr.sbin.winbindd' --- profiles/apparmor.d/usr.sbin.winbindd 2012-11-06 22:19:46 + +++ profiles/apparmor.d/usr.sbin.winbindd 2013-10-15 19:56:45 + @@ -1,4 +1,3 @@ -# Last Modified: Mon Mar 26 20:28:18 2012 #include tunables/global /usr/sbin/winbindd { @@ -13,6 +12,8 @@ /usr/lib*/samba/idmap/*.so mr, /usr/lib*/samba/nss_info/*.so mr, /usr/sbin/winbindd mr, + /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r, + /var/cache/samba/netsamlogon_cache.tdb rw, /var/lib/samba/account_policy.tdb rwk, /var/lib/samba/gencache.tdb rwk, /var/lib/samba/gencache_notrans.tdb rwk, @@ -20,7 +21,7 @@ /var/lib/samba/messages.tdb rwk, /var/lib/samba/netsamlogon_cache.tdb rwk, /var/lib/samba/serverid.tdb rwk, - /var/lib/samba/winbindd_cache.tdb rwk, + /var/lib/samba/winbindd_cache.tdb* rwk, /var/lib/samba/winbindd_privileged/pipe w, /var/log/samba/cores/ rw, /var/log/samba/cores/winbindd/ rw, @@ -28,6 +29,7 @@ /var/log/samba/log.wb-* w, /var/log/samba/log.winbindd rw, /{var/,}run/samba/winbindd.pid rwk, + /{var/,}run/samba/winbindd/ rw, # Site-specific additions and overrides. See local/README for details. #include local/usr.sbin.winbindd Regards, Christian Boltz -- Die Borg sind einfach eine Allegorie auf M$: gross, toll und voller endloser Featuritis - aber wenn es ernst wird, sterben sie an einer Schutzverletzung. [Andreas Pohlke in drsst] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 1/8] parser caching tests - remove unused value
Hello, Am Mittwoch, 23. Oktober 2013 schrieb Steve Beattie: Remove unused report value where it's not used. Signed-off-by: Steve Beattie st...@nxnw.org --- parser/tst/caching.py | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- $ rpm -q --queryformat %{name}-%{version} %{buildtime:date} mod_php mod_php-3.0.11 Fri 23 Jul 1999 03:25:43 PM CEST -dn'*SCNR*'h Jaja. | grep root /etc/aliases kaiser_willem:root [ David Haller und Ratti in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 8/8] parser testlib - use metaclass to mark all test functions keep_on_fail
Hello, Am Mittwoch, 23. Oktober 2013 schrieb Steve Beattie: This patch adds a python metaclass to wrap the test methods in the subclasses of the template class AATestTemplate with the keep_on_fail function, which sets the do_cleanup attribute to False when a testcase failure occurs (i.e. an Exception is raised), and removes the manually applied decorators to the caching tests that made use of this. The downside to this approach is that the way metaclasses are declared changed between python 2 and python 3 in an incompatible way. Since python 3 is The Futureâ¢, I chose that approach and made the caching and valgrind tests which use testlib be python3 (until this change, they would have worked under either python 2 or python 3). I think requiring py3 for the parser tests is OK because it doesn't restrict what we test[1], and a BuildRequires: python3 is acceptable when it makes things easier on the programming side. Therefore: Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz [1] Things would be different if you'd test _python code_ only with py3, even if it's written to run with py2 also -- Ich glaube nicht, daß es je einem Briefkasten gelungen ist durch Namenslosigkeit oder Pseudonym den Werbeblättchen des Supermarkts auf der grünen Wiese oder denen einer politischen Partei direkt vor Wahl verschont zu bleiben. [Helga Fischer in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] parser/po/de.po fixes
Hello, this patch fixes some minor issues in parser/po/de.po I propose this patch for trunk only - fortunately nothing is critical enough for a backport to 2.8;-) === modified file 'parser/po/de.po' --- parser/po/de.po 2009-02-07 12:16:03 + +++ parser/po/de.po 2013-10-26 22:01:40 + @@ -235,7 +235,7 @@ #: ../parser_misc.c:473 ../parser_misc.c:480 ../parser_misc.c:487 #: ../parser_misc.c:484 ../parser_misc.c:491 msgid Conflict 'a' and 'w' perms are mutually exclusive. -msgstr Konfliktive permanente Werte für 'a' und 'W' schließen sich gegenseitig aus. +msgstr Berechtigungskonflikt - 'a' und 'w' schließen sich gegenseitig aus. #: ../parser_misc.c:497 ../parser_misc.c:504 ../parser_misc.c:508 msgid Exec qualifier 'i' invalid, conflicting qualifier already specified @@ -335,16 +335,16 @@ #: parser_yacc.y:640 parser_yacc.y:637 msgid Assert: 'local_profile rule' returned NULL. -msgstr Assert: 'llocal_profile rulel' hat NULL zurückgegeben. +msgstr Assert: 'local_profile rule' hat NULL zurückgegeben. #: parser_yacc.y:772 #, c-format msgid Unset boolean variable %s used in if-expression -msgstr In Bedingungssatz verwendete Boolesche Variable '%s' deaktivieren +msgstr In Bedingungssatz verwendete Boolsche Variable '%s' deaktivieren #: parser_yacc.y:830 msgid subset can only be used with link rules. -msgstr Teilmenge kann nur mit Link-Regeln verwendet werden. +msgstr subset kann nur mit Link-Regeln verwendet werden. #: parser_yacc.y:832 msgid link and exec perms conflict on a file rule using - @@ -356,7 +356,7 @@ #: parser_yacc.y:850 msgid unsafe rule missing exec permissions -msgstr Unsichere Exec-Berechtigungen bei fehlender Regel +msgstr Fehlende Exec-Berechtigungen bei unsicherer Regel #: parser_yacc.y:853 msgid link perms are not allowed on a named profile transtion.\n @@ -374,7 +374,7 @@ #: parser_yacc.y:1064 parser_yacc.y:1072 parser_yacc.y:1053 parser_yacc.y:1061 #, c-format msgid Invalid capability %s. -msgstr Ungültige Funktion %s. +msgstr Ungültige Capability %s. #: parser_yacc.y:1089 parser_yacc.y:1078 #, c-format @@ -389,7 +389,7 @@ #: ../parser_regex.c:283 #, c-format msgid %s: Illegal open {, nesting groupings not allowed\n -msgstr %s: Öffnen ungültig {, verschachtelte Gruppierungen sind nicht zulässig\n +msgstr %s: Ungültige öffnende {, verschachtelte Gruppierungen sind nicht zulässig\n #: ../parser_regex.c:303 #, c-format @@ -454,7 +454,7 @@ #: ../parser_policy.c:298 ../parser_policy.c:304 #, c-format msgid ERROR expanding variables for profile %s, failed to load\n -msgstr FEHLER bei der Erweiterung von Variablen für Profil '%s'. Fehler beim Laden\n +msgstr FEHLER beim Expandieren von Variablen für Profil '%s'. Fehler beim Laden\n #: ../parser_policy.c:481 ../parser_policy.c:486 #, c-format Regards, Christian Boltz -- Wieviele Leute braucht es, um Windows zu installieren? - Sieben! Einen, der sich die CD leisten kann, drei die ausdiskutieren, wie man das wohl macht, zwei Priester, die beten, dass es funktioniert und einen Psychiater, der die Nervenzusammenbrüche behandelt!!! -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] Revert r1225 mistranslations (utils/po/*.po)
Hello, (summing up an IRC discussion from some hours ago for those who missed it) Am Dienstag, 17. September 2013 schrieb Christian Boltz: during the last days, we (as in: the usual people in #apparmor) discovered that the r1225 translation update introduced _lots_ of mistranslations in various languages, even for texts that had correct translations before. Also, most of the changes I commited for de.po recently fixed things that were introduced in r1225. Therefore I propose to completely [1] revert r1225 in trunk and also in the 2.8 branch. Note that r1225 is a commit from 2009, so it's likely that we'll get some merge conflicts. Kshitij tried to revert r1225 today, and got about 30% rejects (line count of patch vs. line count of *.rej files). Additionally he noticed that hi.po contains some mistranslations from r1114. To make things even more interesting, the infamous Mailserver- Domains mistranslation in de.po was initially introduced in r198 (de.po was added in this revision), fixed in r1114 and broken again in a later revision. In other words: It looks like lots of revisions potentially introduced mistranslations, so... The alternative is to mark _all_ texts in _all_ languages as fuzzy, so that translators see them as please check this. ... this is the only way to be sure everything gets checked and fixed. I'd even add a notice (or send a mail) to the translators to warn them to do the fuzzy checking extra carefully because we know there are some mistranslations. I also propose to completely drop the en_GB and en_US translations because I see no real value in them, but several bugs. utils/po/en_GB.po basically contains: msgid Abort msgstr Aborted msgid (C)ancel msgstr Cancel msgid Severity msgstr Security and utils/po/en_US.po contains (only quoting the best stuff): msgid Can't find AppArmor profiles in msgstr Updating AppArmor profiles in %s. msgid Are you sure you want to exit? msgstr Do you really want to use this path? msgid # Filter: %s, Value: %s\n \n msgstr # Period: %s-%s\n msgid Setting %s to audit mode. msgstr Setting %s to complain mode. (and probably some more - the above were enough fun for today ;-) The parser/po/en_*.po files are not that funny, but I'd also consider them superfluous. (If there are real differences between en_GB and en_US that we should keep, please speak up ;-) Regards, Christian Boltz -- weitere Indizien deuten ja auf KMail2: - [...] - KMail2 ist immer kaputt, warum nicht auch hier? ;) [Roman Fietze in opensuse-de] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] dnsmasq profile - update for libvirt files
Hello, dnsmasq needs read access to more files in /var/lib/libvirt/dnsmasq/ (at least *.conf and *.addnhosts) Since this directory contains only files that are intended for dnsmasq (also confirmed by Jim Fehlig, the SUSE libvirt maintainer), the best way is to just allow /var/lib/libvirt/dnsmasq/* r, References: https://bugzilla.novell.com/show_bug.cgi?id=848215 I propose this patch for trunk and the 2.8 branch. === modified file 'profiles/apparmor.d/usr.sbin.dnsmasq' --- profiles/apparmor.d/usr.sbin.dnsmasq2013-08-20 22:52:22 +++ profiles/apparmor.d/usr.sbin.dnsmasq2013-10-30 19:33:18 @@ -43,10 +43,10 @@ @{TFTP_DIR}/ r, @{TFTP_DIR}/** r, - # libvirt lease and hosts files for dnsmasq + # libvirt config, lease and hosts files for dnsmasq /var/lib/libvirt/dnsmasq/r, + /var/lib/libvirt/dnsmasq/*r, /var/lib/libvirt/dnsmasq/*.leases rw, - /var/lib/libvirt/dnsmasq/*.hostsfile r, # libvirt pid files for dnsmasq /{,var/}run/libvirt/network/ r, Regards, Christian Boltz -- Die Borg sind einfach eine Allegorie auf M$: gross, toll und voller endloser Featuritis - aber wenn es ernst wird, sterben sie an einer Schutzverletzung. [Andreas Pohlke in drsst] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] ntpd profile update
Hello, ntpd needs access to /var/lib/ntp/drift/driftfile and /var/lib/ntp/drift/driftfile.TEMP References: https://bugzilla.novell.com/show_bug.cgi?id=850374 I propose this patch for 2.8 and trunk. === modified file 'profiles/apparmor.d/usr.sbin.ntpd' --- profiles/apparmor.d/usr.sbin.ntpd 2013-10-03 13:35:56 + +++ profiles/apparmor.d/usr.sbin.ntpd 2013-11-14 20:36:47 + @@ -40,6 +40,8 @@ /usr/sbin/ntpd rmix, /var/lib/ntp/drift rwl, /var/lib/ntp/drift.TEMP rwl, + /var/lib/ntp/drift/driftfile rw, + /var/lib/ntp/drift/driftfile.TEMP rw, /var/lib/ntp/drift/ntp.drift rw, /var/lib/ntp/drift/ntp.drift.TEMP rw, /var/lib/ntp/etc/* r, Regards, Christian Boltz -- Subscribers don't receive messages from authors, they receive messages from listservs. I've never seen a list server write a message :-) [Felix Miata and jdd in opensuse-factory] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Update samba profiles for samba 4.x
Hello, this is an updated version (actually v4) of my patches for smbd and nmbd which I sent some weeks ago ([patch] updated usr.sbin.smbd profile), and is already included in the packages for the just released openSUSE 13.1. The patch includes changes needed for Samba 4.x, which also includes some small abstraction updates. References: https://bugzilla.novell.com/show_bug.cgi?id=845867 References: https://bugzilla.novell.com/show_bug.cgi?id=846054 I propose the patch for 2.8 and trunk (the patch is for 2.8, but it should apply to trunk without problems) Note: I'm intentionally not including the winbindd profile in this mail. I received another bugreport for it today, so I'll wait some days and will then hopefully be able to send a more complete patch ;-) === modified file 'profiles/apparmor.d/abstractions/samba' --- profiles/apparmor.d/abstractions/samba 2011-08-26 23:52:27 + +++ profiles/apparmor.d/abstractions/samba 2013-10-15 20:36:33 + @@ -11,6 +11,7 @@ /etc/samba/* r, /usr/share/samba/*.dat r, + /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r, /var/lib/samba/**.tdb rwk, /var/log/samba/cores/ rw, /var/log/samba/cores/** rw, === modified file 'profiles/apparmor.d/abstractions/kerberosclient' --- profiles/apparmor.d/abstractions/kerberosclient.orig2011-03-23 20:24:11.0 +0100 +++ profiles/apparmor.d/abstractions/kerberosclient 2013-11-02 15:04:27.267448981 +0100 @@ -20,7 +20,7 @@ /usr/lib/@{multiarch}/krb5/plugins/preauth/ r, /usr/lib/@{multiarch}/krb5/plugins/preauth/* mr, - /etc/krb5.keytabr, + /etc/krb5.keytabrk, /etc/krb5.conf r, # config files found via strings on libs === modified file 'profiles/apparmor.d/usr.sbin.nmbd' --- profiles/apparmor.d/usr.sbin.nmbd 2011-08-27 18:50:42 + +++ profiles/apparmor.d/usr.sbin.nmbd 2013-10-20 11:54:48 + @@ -11,7 +11,9 @@ /usr/sbin/nmbd mr, + /var/cache/samba/gencache.tdb rwk, /var/{cache,lib}/samba/browse.dat* rw, + /var/{cache,lib}/samba/gencache.dat rw, /var/{cache,lib}/samba/wins.dat* rw, /var/{cache,lib}/samba/smb_krb5/ rw, /var/{cache,lib}/samba/smb_krb5/krb5.conf* rw, === modified file 'profiles/apparmor.d/usr.sbin.smbd' --- profiles/apparmor.d/usr.sbin.smbd 2012-01-10 18:06:24 + +++ profiles/apparmor.d/usr.sbin.smbd 2013-10-15 20:36:33 + @@ -29,16 +29,21 @@ /usr/lib*/samba/vfs/*.so mr, /usr/lib*/samba/charset/*.so mr, /usr/lib*/samba/auth/script.so mr, - /usr/lib*/samba/{lowercase,upcase,valid}.dat r, + /usr/lib*/samba/pdb/*.so mr, + /usr/lib*/samba/{lowercase,lowcase,upcase,valid}.dat r, # [1] /usr/sbin/smbd mr, /usr/sbin/smbldap-useradd Px, /var/cache/samba/** rwk, /var/cache/samba/printing/printers.tdb mrw, /var/lib/samba/** rwk, /var/lib/samba/printers/** rw, + /var/lib/sss/mc/passwd r, + /var/lib/sss/pubconf/kdcinfo.* r, /{,var/}run/cups/cups.sock rw, /{,var/}run/dbus/system_bus_socket rw, /{,var/}run/samba/** rk, + /{,var/}run/samba/ncalrpc/ rw, + /{,var/}run/samba/ncalrpc/** rw, /{,var/}run/samba/smbd.pid rw, /var/log/samba/cores/smbd/ rw, /var/log/samba/cores/smbd/** rw, [1] for trunk, this line will be + /usr/lib*/samba/{lowcase,upcase,valid}.dat r, # [1] because (quoting myself from Oct 15th): Also fix /usr/lib*/samba/{lowercase,upcase,valid}.dat r, which should be lowcase instead of lowercase. Google didn't find any samba-related lowercase.dat and my ARCHIVES.gz archive shows that openSUSE 11.4 already used lowcase.dat, so removing lowercase shouldn't cause any problems. Nevertheless, I'll not remove lowercase in the 2.8 branch to be on the safe side. Regards, Christian Boltz -- .domain.intern smpt:[mx.domain.intern] Du meinst sicher smtp und nicht smpt. :-) Du kennst den Senseless Mailinglist Protocol Typo nicht? ;-) [ Michael Neufing und () Gregor Hermens in postfixbuch-users] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Update samba profiles for samba 4.x
Hello, Am Dienstag, 19. November 2013 schrieb Seth Arnold: On Tue, Nov 19, 2013 at 10:28:28PM +0100, Christian Boltz wrote: === modified file 'profiles/apparmor.d/usr.sbin.nmbd' --- profiles/apparmor.d/usr.sbin.nmbd 2011-08-27 18:50:42 + +++ profiles/apparmor.d/usr.sbin.nmbd 2013-10-20 11:54:48 + @@ -11,7 +11,9 @@ /usr/sbin/nmbd mr, + /var/cache/samba/gencache.tdb rwk, The missing {,lib} on the first line makes my OCD tingle; is this correct? abstractions/samba contains /var/lib/samba/**.tdb rwk, so it's (more or less) correct that the nmbd profile doesn't include {,lib}. We might need to include the /var/cache/ path in abstractions/samba, but that's something for another patch ;-) (The smbd/nmbd/winbind profiles need a bit of cleanup anyway.) Regards, Christian Boltz -- [package naming] Another funny example is libreoffice, it actually collides with our shared library policy ;-) [Sascha Peilicke in opensuse-packaging] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] abstractions/ssl_certs update
Hello, add /var/lib/ca-certificates/ to abstractions/ssl_certs. update-ca-certificates (from ca-certificates-1_201310161709-1.1.noarch) stores certs in this directory now. References: https://bugzilla.novell.com/show_bug.cgi?id=852018 I propose this patch for trunk and the 2.8 branch. === modified file 'profiles/apparmor.d/abstractions/ssl_certs' --- profiles/apparmor.d/abstractions/ssl_certs 2011-08-08 20:22:03 +++ profiles/apparmor.d/abstractions/ssl_certs 2013-11-24 16:14:18 @@ -17,3 +17,5 @@ /usr/share/ssl/certs/ca-bundle.crt r, /usr/local/share/ca-certificates/ r, /usr/local/share/ca-certificates/** r, + /var/lib/ca-certificates/ r, + /var/lib/ca-certificates/** r, Regards, Christian Boltz -- Wenn das Teil unter Windows CE oder Pocket PC 2000 läuft, ist Synce Dein Fall. Zu finden auf Sourceforge, wenn ich mich nicht irre, und ich irre mich nie wenn ich mich nicht irre.[Michael Karges in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 1/4] security: add security_path_chdir hook
Hello, Am Donnerstag, 28. November 2013 schrieb Seth Arnold: On Tue, Nov 05, 2013 at 05:34:58AM -0800, John Johansen wrote: diff --git a/fs/open.c b/fs/open.c index d420331..9505fc5 100644 --- a/fs/open.c +++ b/fs/open.c @@ -387,6 +387,10 @@ retry: if (error) goto out; + error = security_path_chdir(path); + if (error) + goto dput_and_out; + error = inode_permission(path.dentry-d_inode, MAY_EXEC | MAY_CHDIR); if (error) goto dput_and_out; Hmm, does that mean that first the AppArmor permissions are checked and then the file/directory permissions? I reported some time ago that the audit.log contains stuff that would be denied by file/directory permissions anyway (which also means logging it more confusing than useful ;-) and the answer was that this (IMHO buggy) behaviour is caused by the kernel. It might be a good idea to check the file/directory permissions first, and, _if_ access would be allowed, ask AppArmor via the security hook. @@ -419,6 +423,10 @@ SYSCALL_DEFINE1(fchdir, unsigned int, fd) if (!S_ISDIR(inode-i_mode)) goto out_putf; + error = security_path_chdir(f.file-f_path); + if (error) + goto out_putf; + error = inode_permission(inode, MAY_EXEC | MAY_CHDIR); Same here. Regards, Christian Boltz -- Machen wir einen Club utf-8 geplagte Perl-Programmierer auf? [Bernhard Walle in suse-programming] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 3/4] security: add security_path_access hook
Hello, basically what we are just discussing in [PATCH 1/4] security: add security_path_chdir hook also applies here: Am Donnerstag, 28. November 2013 schrieb Seth Arnold: On Tue, Nov 05, 2013 at 05:35:00AM -0800, John Johansen wrote: diff --git a/fs/open.c b/fs/open.c index 9505fc5..f3e276e 100644 --- a/fs/open.c +++ b/fs/open.c @@ -343,6 +343,10 @@ retry: goto out_path_release; } + res = security_path_access(path, mode | MAY_ACCESS); + if (res) + goto out_path_release; + res = inode_permission(inode, mode | MAY_ACCESS); /* SuS v2 requires we report a read only fs too */ if (res || !(mode S_IWOTH) || special_file(inode-i_mode)) Please insert the hook _after_ checking the file/directory permissions, not before. Regards, Christian Boltz -- Ich hab letztens nen Film gesehen, in dem sich zwei Irre unterhalten haben. Da hat der eine den anderen auch nicht verstanden. Stimmt, hast Recht. Wann haben wir übrigens wieder Freigang? ;) [ Martin Borchert und Bernd Brodesser in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 02/12] parser: mark valgrind test target as phony
Hello, Am Dienstag, 3. Dezember 2013 schrieb Steve Beattie: Signed-off-by: Steve Beattie st...@nxnw.org --- parser/tst/Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/parser/tst/Makefile === --- a/parser/tst/Makefile +++ b/parser/tst/Makefile @@ -13,7 +13,7 @@ endif all: tests -.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality +.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality valgrind tests: error_output caching minimize equality parser_sanity GEN_TRANS_DIRS=simple_tests/generated_x/ simple_tests/generated_perms_leading/ simple_tests/generated_perms_safe/ simple_tests/generated_dbus Acked-By: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Früher habe ich auch gerne CDs gekauft [...] Aber ich habe gelernt, daß ich damit nicht Musiker fördere, sondern nur koksende Sony-Spacken, die mir zum Dank für meine Investition [...] ein Rootkit auf meine Karre installieren, gleich neben den Staatstrojaner. [http://blog.koehntopp.de/archives/3154-Nicht-Urheberrecht-ist-das-Kernthema.html] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 08/12] parser: add test case for empty character class regex
Hello, Am Dienstag, 3. Dezember 2013 schrieb Steve Beattie: This patch adds a test that verifies the parser considers an emty character class regex as a parse arror. Signed-off-by: Steve Beattie st...@nxnw.org --- parser/tst/simple_tests/file/bad_re_brace_1.sd |8 1 file changed, 8 insertions(+) Index: b/parser/tst/simple_tests/file/bad_re_brace_1.sd === --- /dev/null +++ b/parser/tst/simple_tests/file/bad_re_brace_1.sd @@ -0,0 +1,8 @@ +# +#=DESCRIPTION regex with empty character class (brace) +#=EXRESULT FAIL +# +/usr/bin/foo { + /alpha/[]beta rw, +} + Good idea! Acked-By: Christian Boltz appar...@cboltz.de BTW: Do we already have a similar test for empty alternations, like /foo{}/bar rw, ? Regards, Christian Boltz -- The kernel will stay the same between SUSE Linux 10.1 and SLE10 - it just might be that we release them at different days, Good. Let the SLED customers test it for us first ;) [ Andreas Jaeger and Martin Schlander in opensuse] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 10/12] parser: add basic alternation tests, along with their file and owner equivalents. (v2)
Hello, Am Dienstag, 3. Dezember 2013 schrieb Steve Beattie: This patch verifies basic alternation usage. Patch history: v1: initial revision v2: mark nested alternation tests as passing, as it was deemed a bug that the parser didn't support them. Signed-off-by: Steve Beattie st...@nxnw.org --- parser/tst/simple_tests/file/file/ok_alternations_1.sd |7 +++ parser/tst/simple_tests/file/file/ok_alternations_2.sd | 7 +++ parser/tst/simple_tests/file/ok_alternations_1.sd | 7 +++ parser/tst/simple_tests/file/ok_alternations_2.sd | 7 +++ parser/tst/simple_tests/file/owner/ok_alternations_1.sd | 7 +++ parser/tst/simple_tests/file/owner/ok_alternations_2.sd |7 +++ 6 files changed, 42 insertions(+) Acked-By: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Ach so, ein 64-Bit-System... Tja, es gibt da wohl zwei Tendenzen: die einen benutzen /lib für 64 Bit (weil es ein 64-Bit-System ist) und /lib32 für den alten Kram, die anderen /lib für 32 Bit (weil das schon immer so war) und /lib64 für das neue Zeugs. Mir reichen schon 2 Bit ;-) [Werner Flamme in postfixbuch-users] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] fix broken english in parser_yacc.y
Hello, I think the patch (and $SUBJECT) speaks for itsself ;-) === modified file 'parser/parser_yacc.y' --- parser/parser_yacc.y2013-09-28 00:26:39 + +++ parser/parser_yacc.y2013-12-06 18:52:41 + @@ -657,7 +657,7 @@ rules: rules opt_prefix mnt_rule { if ($2.owner) - yyerror(_(owner prefix not allow on mount rules)); + yyerror(_(owner prefix not allowed on mount rules)); if ($2.deny $2.audit) { $3-deny = 1; } else if ($2.deny) { @@ -674,7 +674,7 @@ rules: rules opt_prefix dbus_rule { if ($2.owner) - yyerror(_(owner prefix not allow on dbus rules)); + yyerror(_(owner prefix not allowed on dbus rules)); if ($2.deny $2.audit) { $3-deny = 1; } else if ($2.deny) { @@ -701,7 +701,7 @@ rules: rules opt_prefix capability { if ($2.owner) - yyerror(_(owner prefix not allow on capability rules)); + yyerror(_(owner prefix not allowed on capability rules)); if ($2.deny) $1-caps.deny |= $3; Regards, Christian Boltz -- Am Besten wäre natürlich, den Owner von /dev/usbkabel ;-) zu überprüfen *g* Dieses Device ist IMHO aber erst im neuen Kernel vorgesehen. Hast Du da etwa schon einen Patch für den SuSE-Kernel? ;-) [ Christian Boltz und Harald Krause in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] monthly meeting
Hello, as I already mentioned in the last IRC meeting, I won't be online on tuesday for the monthly meeting. I'll let it up to you if we move it [1] or if you do the meeting without me ;-) Regards, Christian Boltz [1] I'm also away on wednesday and saturday -- Nochmal: Insgesamt macht das PDF einen guten Eindruck! Gell? Bin ja auch stolz wie Oskar :=) Zu Recht! Oh, danke! *erroet* *verbeug* [ Christian Boltz und David Haller in suse-linux-faq] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] can ?not fix
Hello, Am Donnerstag, 5. Dezember 2013 schrieb Seth Arnold: On Thu, Dec 05, 2013 at 10:50:56PM +0100, Christian Boltz wrote: as discussed on #apparmor yesterday, here's the most important patch we've ever seen ;-) References: https://bugzilla.novell.com/show_bug.cgi?id=853661 Ha! The best part about this is that the entire section needs to be re-written, as it is several years out of date: Well, it's the first time I touched this file. You know what this means? ;-) So, while the patch itself looks good, there's bigger problems that need to be fixed. :) I was afraid of that ;-) Here's an updated (and much bigger) patch that - removes the note about can ?not mknod - also removes mount and umount from the can ?not list which are covered by mount rules now (are the remaining parts still valid?) - updates the example audit.log lines to the current log format - updates the description of the log format BTW: Is the (Nameis in quotes, because the process name is limited to 15 bytes; [...] part still valid? === modified file 'parser/apparmor.pod' --- parser/apparmor.pod 2010-12-20 20:29:10 + +++ parser/apparmor.pod 2013-12-08 14:32:51 + @@ -6,6 +6,9 @@ #Copyright (c) 2010 #Canonical Ltd. (All rights reserved) # +#Copyright (c) 2013 +#Christian Boltz (All rights reserved) +# #This program is free software; you can redistribute it and/or #modify it under the terms of version 2 of the GNU General Public #License published by the Free Software Foundation. @@ -89,43 +92,46 @@ cannot call the following system calls: create_module(2) delete_module(2) init_module(2) ioperm(2) - iopl(2) mount(2) umount(2) ptrace(2) reboot(2) setdomainname(2) + iopl(2) ptrace(2) reboot(2) setdomainname(2) sethostname(2) swapoff(2) swapon(2) sysctl(2) -A confined process can not call mknod(2) to create character or block devices. - =head1 ERRORS When a confined process tries to access a file it does not have permission to access, the kernel will report a message through audit, similar to: - audit(1148420912.879:96): REJECTING x access to /bin/uname - (sh(6646) profile /tmp/sh active /tmp/sh) - - audit(1148420912.879:97): REJECTING r access to /bin/uname - (sh(6646) profile /tmp/sh active /tmp/sh) - - audit(1148420944.837:98): REJECTING access to capability - 'dac_override' (sh(6641) profile /tmp/sh active /tmp/sh) - - -The permissions requested by the process are immediately after -REJECTING. The name and process id of the running program are reported, -as well as the profile name and any hat that may be active. (Name + audit(1386511672.612:238): apparmor=DENIED operation=exec + parent=7589 profile=/tmp/sh name=/bin/uname pid=7605 + comm=sh requested_mask=x denied_mask=x fsuid=0 ouid=0 + + audit(1386511672.613:239): apparmor=DENIED operation=open + parent=7589 profile=/tmp/sh name=/bin/uname pid=7605 + comm=sh requested_mask=r denied_mask=r fsuid=0 ouid=0 + + audit(1386511772.804:246): apparmor=DENIED operation=capable + parent=7246 profile=/tmp/sh pid=7589 comm=sh pid=7589 + comm=sh capability=2 capname=dac_override + +The permissions requested by the process are described in the operation= +and denied_mask= (for files - capabilities etc. use a slightly different +log format). +The name and process id of the running program are reported, +as well as the profile name including any hat that may be active, +separated by //. (Name is in quotes, because the process name is limited to 15 bytes; it is the -same as reported through the Berkeley process accounting.) If no hat is -active (see aa_change_hat(2)) then the profile name is printed for active. +same as reported through the Berkeley process accounting.) For confined processes running under a profile that has been loaded in complain mode, enforcement will not take place and the log messages reported to audit will be of the form: - audit(1146868287.904:237): PERMITTING r access to - /etc/apparmor.d/tunables (du(3811) profile /usr/bin/du active - /usr/bin/du) + audit(1386512577.017:275): apparmor=ALLOWED operation=open + parent=8012 profile=/usr/bin/du name=/etc/apparmor.d/tunables/ + pid=8049 comm=du requested_mask=r denied_mask=r fsuid=1000 ouid=0 - audit(1146868287.904:238): PERMITTING r access to /etc/apparmor.d - (du(3811) profile /usr/bin/du active /usr/bin/du) + audit(1386512577.017:276): apparmor=ALLOWED operation=open + parent=8012 profile=/usr/bin/du name=/etc/apparmor.d/tunables/ + pid=8049 comm=du requested_mask=r denied_mask=r fsuid=1000 ouid=0 If the userland auditd is not running, the kernel will send audit events Regards, Christian Boltz -- Was spricht gegen einen Punkt im Expertenmodus: [ ] Ich weiß nicht, was eine Partition ist. Wenn
[apparmor] dovecot profiles
Hello, attached to this mail is a set of profiles for dovecot 2.x. Some are already in bzr in an older version (see the diff file for the changes), others are completely new. I'm also introducing tunables/dovecot which contains @{DOVECOT_MAILSTORE} containing the location of the mailboxes, which is needed in several profiles (and replaces quite some lines in the already existing dovecot profiles.) References: https://bugzilla.novell.com/show_bug.cgi?id=851984 I doubt if this is the final version, but nevertheless I'd welcome comments ;-) (I'll propose the profiles to be added to profiles/apparmor.d/ when they are finished, and also release them as update for at least openSUSE 13.1.) Note: some profiles don't have the #include local/... - that's on my TODO list. Also the paperwork (copyright headers) is still missing. Regards, Christian Boltz -- * mrdocs wonders when darix sleeps sshaw mrdocs: robots don't need sleep [from #opensuse-buildservice] === modified file 'profiles/apparmor.d/usr.lib.dovecot.deliver' --- profiles/apparmor.d/usr.lib.dovecot.deliver 2012-01-06 16:34:44 + +++ profiles/apparmor.d/usr.lib.dovecot.deliver 2013-12-15 14:56:00 + @@ -1,6 +1,8 @@ # Author: Dulmandakh Sukhbaatar dulmand...@gmail.com #include tunables/global +#include tunables/dovecot + /usr/lib/dovecot/deliver { #include abstractions/base #include abstractions/nameservice @@ -8,20 +10,16 @@ capability setgid, capability setuid, + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, + # http://www.postfix.org/SASL_README.html#server_dovecot /etc/dovecot/dovecot.conf r, /etc/dovecot/{auth,conf}.d/*.conf r, /etc/dovecot/dovecot-postfix.conf r, @{HOME} r, - @{HOME}/Maildir/ rw, - @{HOME}/Maildir/** klrw, - @{HOME}/mail/ rw, - @{HOME}/mail/* klrw, - @{HOME}/mail/.imap/** klrw, /usr/lib/dovecot/deliver mr, - /var/mail/* klrw, - /var/spool/mail/* klrw, # Site-specific additions and overrides. See local/README for details. #include local/usr.lib.dovecot.deliver === modified file 'profiles/apparmor.d/usr.lib.dovecot.imap' --- profiles/apparmor.d/usr.lib.dovecot.imap 2011-08-26 23:12:10 + +++ profiles/apparmor.d/usr.lib.dovecot.imap 2013-12-13 12:48:02 + @@ -1,6 +1,8 @@ # Author: Kees Cook k...@ubuntu.com #include tunables/global +#include tunables/dovecot + /usr/lib/dovecot/imap { #include abstractions/base #include abstractions/nameservice @@ -8,18 +10,11 @@ capability setgid, capability setuid, - @{HOME} r, - @{HOME}/Maildir/ rw, - @{HOME}/Maildir/** klrw, - @{HOME}/Mail/ rw, - @{HOME}/Mail/* klrw, - @{HOME}/Mail/.imap/** klrw, - @{HOME}/mail/ rw, - @{HOME}/mail/* klrw, - @{HOME}/mail/.imap/** klrw, + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, + + @{HOME} r, # ??? /usr/lib/dovecot/imap mr, - /var/mail/* klrw, - /var/spool/mail/* klrw, # Site-specific additions and overrides. See local/README for details. #include local/usr.lib.dovecot.imap === modified file 'profiles/apparmor.d/usr.lib.dovecot.pop3' --- profiles/apparmor.d/usr.lib.dovecot.pop3 2011-08-26 23:12:10 + +++ profiles/apparmor.d/usr.lib.dovecot.pop3 2013-12-13 12:49:33 + @@ -1,6 +1,8 @@ # Author: Kees Cook k...@ubuntu.com #include tunables/global +#include tunables/dovecot + /usr/lib/dovecot/pop3 { #include abstractions/base #include abstractions/nameservice @@ -8,13 +10,10 @@ capability setgid, capability setuid, - /var/mail/* klrw, - /var/spool/mail/* klrw, - @{HOME} r, - @{HOME}/mail/* klrw, - @{HOME}/mail/.imap/** klrw, - @{HOME}/Maildir/ rw, - @{HOME}/Maildir/** klrw, + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, + + @{HOME} r, # ??? /usr/lib/dovecot/pop3 mr, # Site-specific additions and overrides. See local/README for details. === modified file 'profiles/apparmor.d/usr.sbin.dovecot' --- profiles/apparmor.d/usr.sbin.dovecot 2013-01-02 23:34:38 + +++ profiles/apparmor.d/usr.sbin.dovecot 2013-12-13 12:56:25 + @@ -1,6 +1,8 @@ # Author: Kees Cook k...@ubuntu.com #include tunables/global +#include tunables/dovecot + /usr/sbin/dovecot { #include abstractions/authentication #include abstractions/base @@ -9,29 +11,42 @@ #include abstractions/ssl_keys capability chown, + capability dac_override, + capability fsetid, + capability kill, capability net_bind_service, capability setgid, capability setuid, capability sys_chroot, - capability fsetid, + + + + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, /etc/dovecot/** r, /etc/mtab r, /etc/lsb-release r, /etc/SuSE-release r, @{PROC}/@{pid}/mounts r, + /usr/bin/doveconf rix, + /usr/lib/dovecot/anvil Px, + /usr/lib/dovecot/auth Px, + /usr/lib/dovecot/config Px, /usr/lib/dovecot/dovecot-auth Pxmr, /usr/lib/dovecot/imap Pxmr, /usr/lib/dovecot/imap-login Pxmr, + /usr/lib/dovecot/log Px, + /usr/lib/dovecot/managesieve Px, + /usr/lib
Re: [apparmor] [PATCH] profiles: rw file perms are now needed on AF_UNIX socket files
Hello, Am Donnerstag, 19. Dezember 2013 schrieb Tyler Hicks: The AppArmor kernel now checks for both read and write permissions when a process calls connect() on a UNIX domain socket. The patch updates a four abstractions that were found to be needing changes after the kernel change. Does this affect all sockets? There are some more candidates I found while grepping through the profiles: # grep -r ' w,' . |grep -v '/ w,' # pid files, logs etc. manually removed from the list ./abstractions/nameservice: /{,var/}run/avahi-daemon/socket w, ./abstractions/base: /dev/log w, ./abstractions/mdns: /{,var/}run/mdnsd w, ./abstractions/apparmor_api/change_profile:@{PROC}/@{tid}/attr/{current,exec} w, ./abstractions/apache2-common: @{PROC}/@{pid}/attr/current w, ./abstractions/X: /tmp/.X11-unix/* w, ./usr.lib.dovecot.dovecot-auth: /var/spool/postfix/private/dovecot-auth w, ./usr.sbin.winbindd: /var/lib/samba/winbindd_privileged/pipe w, ./usr.sbin.winbindd: /var/log/samba/log.winbindd-idmap w, ./usr.sbin.winbindd: /{var/,}run/samba/winbindd/pipe w, ./sbin.syslogd: /dev/tty* w, ./sbin.syslog-ng: /dev/log w, ./sbin.syslog-ng: /dev/syslog w, ./sbin.syslog-ng: @{CHROOT_BASE}/var/lib/*/dev/log w, ./usr.sbin.nscd.orig: /{,var/}run/avahi-daemon/socket w, ./usr.sbin.dovecot: /var/spool/postfix/private/* w, ./usr.sbin.avahi-daemon: /{,var/}run/avahi-daemon/socket w, Do you think some of them need to be changed from w to rw? If yes, which ones? Regards, Christian Boltz -- Gegen nachhaltige Zweifel, ob die SSL-Verschlüsselung in Windows wirklich noch den erwarteten Schutz vor unerwünschten Lauschern bieten kann, hilft damit letztlich nur der Wechsel des Betriebssystems. [http://www.heise.de/ct/artikel/Microsofts-Hintertuer-1921730.html] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] allow samba to create /var/run/samba/
Hello, samba (nmbd and smbd) need to create /var/run/samba at startup (at least on systems where /var/run is on a tmpfs) References: https://bugzilla.novell.com/show_bug.cgi?id=856651 I propose this patch for trunk and 2.8 === modified file 'profiles/apparmor.d/abstractions/samba' --- profiles/apparmor.d/abstractions/samba 2013-11-20 00:11:01 + +++ profiles/apparmor.d/abstractions/samba 2013-12-22 15:50:18 + @@ -16,5 +16,6 @@ /var/log/samba/cores/ rw, /var/log/samba/cores/** rw, /var/log/samba/log.* w, + /{,var/}run/samba/ w, /{,var/}run/samba/*.tdb rw, Regards, Christian Boltz -- Du kannst dir einen Kernel so geschwaetzig eingestellt kompilieren, dass die HDD kaum noch mit dem loggen hinterherkommt (was wiederum Bugs im HDD-Treiber ausloesen koennte ;)) [David Haller in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] allow samba to create /var/run/samba/
Hello, Am Sonntag, 22. Dezember 2013 schrieb Christian Boltz: samba (nmbd and smbd) need to create /var/run/samba at startup (at least on systems where /var/run is on a tmpfs) It also needs to create /var/cache/samba/ References: https://bugzilla.novell.com/show_bug.cgi?id=856651 I propose this patch for trunk and 2.8 Updated patch: === modified file 'profiles/apparmor.d/abstractions/samba' --- profiles/apparmor.d/abstractions/samba 2013-11-20 00:11:01 +++ profiles/apparmor.d/abstractions/samba 2013-12-23 12:28:06 @@ -12,9 +12,11 @@ /etc/samba/* r, /usr/share/samba/*.dat r, /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r, + /var/cache/samba/ w, /var/lib/samba/**.tdb rwk, /var/log/samba/cores/ rw, /var/log/samba/cores/** rw, /var/log/samba/log.* w, + /{,var/}run/samba/ w, /{,var/}run/samba/*.tdb rw, Regards, Christian Boltz -- Wieso ich, ich habe 1,5 Mbit anbindung, ständig. Du? Oder dein Rechner? :-) Was meinst du wie ich nach 1,500,000 Bit aussehe, glaubst du, dann könnte ich noch schreiben, geschweige denn programmieren? [ Ratti und Gerald Goebel in fontlinge-devel] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] AppArmor profile for LibreOffice
Hello, Am Mittwoch, 25. Dezember 2013 schrieb Jonathan Davies: On 25/12/2013 16:23, Christian Boltz wrote: Am Mittwoch, 25. Dezember 2013 schrieb Jonathan Davies: I have created an AppArmor profile for LibreOffice and I would like to see it placed into the 14.04 packages. I had a short look at it. Some notes: audit deny network bluetooth, It seems this isn't allowed by any abstractions. What's the reason to explicitely deny it? I didn't want LibreOffice to talk on bluetooth, and it seems to open up a service there by default. Sounds reasonable - and leaves me with the question if audit makes sense. (You already know it wants to do that, and you deny it - so why fill the logs?) # abstractions/private-files-strict is in force from above. owner @{HOME}/** rwk, The usual problem of having an application with a save as... dialog ;-) I know there's some work done on a file dialog helper going (to avoid the need for such rules), but I don't know the details and if it's useable already. I don't see an issue here - I'm allowing full access to the home folder of the user, while private-files-strict is disallowing access to places such as ~/.{ssh,gnupg,mozilla}/*, etc. Trying opening or saving a file there and you'll find that access is denied. The issue is that it allows full access to the home (with the private- files-strict exceptions). It's the best we can do currently - I just wanted to mention that there might be a better solution in the future. deny @{HOME}/.exec* rwmx, What's the reason for this denial? Should it be part of an abstraction instead of having it in the profile? LibreOffice seems to try to write to these files but does nothing with them - so I decided to block it. Ah, ok. /usr/bin/bluetooth-sendto rmUx, /usr/bin/lpr rmUx, /usr/bin/paperconfrmix, /usr/bin/xdg-open rmUx, I'd recommend rmPUx instead of rmUx - if someone has a profile for one of them, it should be used. Someone needs to update the manpage, it says that this kind of mode mixing is incompatible. PUx means: if a profile exists, use it (so Px) - but if no profile exists, fall back to Ux. You are right - the apparmor.d manpage doesn't explain those fallback modes yet :-( I just submitted https://bugs.launchpad.net/apparmor/+bug/1264178 to make sure it doesn't get lost in the holiday season ;-) Regards, Christian Boltz -- Nicht das ich frei von Paranoia Schueben waere ;), aber wenn Dir das passiert spiel sofort Lotto, bei dem Glueck bekommst Du bestimmt 4 Wochen den 6er mit Superzahl. [Maik Holtkamp in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] add FIPS support to abstractions/openssl
Hello, patch description stolen from Lars Vogdt The /proc/sys/crypto/fips_enabled r, should IMHO be integrated in the upstream abstractions/openssl as this is not critical if you run without FIPS, but it will produce a lot of log entries on systems like SLES that are FIPS aware. /stolen patch description References: https://bugzilla.novell.com/show_bug.cgi?id=857122#c2 === modified file 'profiles/apparmor.d/abstractions/openssl' --- profiles/apparmor.d/abstractions/openssl2011-08-08 20:22:03 +++ profiles/apparmor.d/abstractions/openssl2014-01-03 18:07:23 @@ -10,4 +10,5 @@ /etc/ssl/openssl.cnf r, /usr/share/ssl/openssl.cnf r, + @{PROC}/sys/crypto/fips_enabled r, Regards, Christian Boltz -- I wonder how we ended up with baseurl and extra_url, now we are missing one with a - like data-dir to violate consistency and the principle of least surprise in all possible ways. [Duncan Mac-Vicar Prett in bnc#449842] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] aa-logprof doesn't check if user is root
Hello, Am Mittwoch, 15. Januar 2014 schrieb Aaron Lewis: aa-logprof doesn't check if user is root Can someone add the verification please? just like aa-status and others Well, not always ;-) aa-logprof doesn't always need root permissions (well, except for reloading the profiles). You can easily run it as user when using -f /my/logfile -d /path/to/profiles/ (assuming the user has access to both /my/logfile and /path/to/profiles/). I know this isn't the typical usecase, but still something that should be possible. (However, maybe we should think about having the root check enabled by default, and add an option --no-profile-reload that also skips the root check.) That said - feel free to test the rewritten tools available at https://code.launchpad.net/apparmor-profile-tools Regards, Christian Boltz -- Weißt Du, man soll ja eigentlich keine Leute auf öffentlichen Mailinglisten beschimpfen, sie kratzen oder ihnen Tiernamen geben. Aber die traumwandlerische Sicherheit, mit der Du den relevanten Teil des Logs weggeschnitten hast, ist schon beeindruckend. Also, Du Hängebauchschwein, fühl Dich beschimpft und gekratzt ;-) [Stefan Förster in postfixbuch-users] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 12/18] parser: add rlimit language acceptance tests
Hello,, Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie: The parser was lacking language tests for rlimits. This test adds several, one for each rlimit type. Signed-off-by: Steve Beattie st...@nxnw.org Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Erstes Gesetz WWW: Du mögest trennen die Spinnen und Indianer von den Usern und jedem sein eigen Grund und Heim zuteilen auf das der eine nicht neidisch werde auf den anderen und begehre dessen Heim und Gut. *lach* [Thomas Templin in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 13/18] parser: add rttime rlimit support
Hello, Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie: This patch adds support for the rttime rlimit (aka RLIMIT_RTTIME), available since the 2.6.25 kernel, according to the getrlimit(2) man page; see that man page for more details on this rlimit. An acceptance test is also added. Index: b/parser/tst/simple_tests/rlimits/ok_rlimit_18.sd === +profile rlimit { + set rlimit rttime = 60minutes, +} Does this also need an addition for apparmor.vim.in? (and BTW, did you test if apparmor.vim displays all tests from 12/18 correctly?) Regards, Christian Boltz -- [SuSE 9.1] Und utf-8 saugt tote Hamster durch Strohhalme, selbst wenn es funktioniert. [...] Und das alles nur, damit ich Klingonisch native verarbeiten kann in meinem Rechner. [http://blog.koehntopp.de/archives/317_Die+schlimmste+aller+Susen.html] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 15/18] utils: remove unneeded imports from a-easyprof and aa-sandbox
Hello, Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie: Found by running pyflakes on these scripts. Signed-off-by: Steve Beattie st...@nxnw.org Acked-by: Christian Boltz appar...@cboltz.de (assuming pyflakes was right - and even if not, we'll notice the failures quickly ;-) Regards, Christian Boltz -- Ich dachte schon vor 2 Jahren, das Niveau ließe sich nicht mehr weiter senken, doch offensichtlich geht es tatsächlich auch unterirdisch noch weiter ... Du hast den Nullpunkt flshca angesetzt. [ Ralf Corsepius und Florian Gross in suse-linux über die Liste] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 16/18] utils: address pep8 complaints
Hello, Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie: This patch eliminates the complaints from running: pep8 --ignore=E501 aa-easyprof vim/ (E501 is 'line too long', which I'm not too chuffed about. ) Mostly, it's a lot of whitespace touchups, with a few conversions from '==' to 'is'. Signed-off-by: Steve Beattie st...@nxnw.org --- utils/aa-easyprof|3 utils/vim/create-apparmor.vim.py | 133 +++ 2 files changed, 68 insertions(+), 68 deletions(-) Index: b/utils/vim/create-apparmor.vim.py === --- a/utils/vim/create-apparmor.vim.py +++ b/utils/vim/create-apparmor.vim.py @@ -73,28 +74,28 @@ for af_pair in af_pairs: ... aa_regex_map = { 'FILENAME': filename, -'FILE': r'\v^\s*(audit\s+)?(deny\s+|allow\s+)?(owner\s+)?' + filename + r'\s+', # Start of a file rule +'FILE': r'\v^\s*(audit\s+)?(deny\s+|allow\s+)?(owner\s+)?' + filename + r'\s+', # Start of a file rule # (whitespace_+_, owner etc. flag_?_, filename pattern, whitespace_+_) -'DENYFILE': r'\v^\s*(audit\s+)?deny\s+(owner\s+)?' + filename + r'\s+', # deny, otherwise like FILE +'DENYFILE': r'\v^\s*(audit\s+)?deny\s+(owner\s+)?' + filename + r'\s+', # deny, otherwise like FILE 'auditdenyowner': r'(audit\s+)?(deny\s+|allow\s+)?(owner\s+)?', - 'audit_DENY_owner': r'(audit\s+)?deny\s+(owner\s+)?', # must include deny, otherwise like auditdenyowner +'audit_DENY_owner': r'(audit\s+)?deny\s+(owner\s+)?', # must include deny, otherwise like auditdenyowner 'auditdeny': r'(audit\s+)?(deny\s+|allow\s+)?', -'EOL': r'\s*,(\s*$|(\s*#.*$)\@=)', # End of a line (whitespace_?_, comma, whitespace_?_ comment.*) +'EOL': r'\s*,(\s*$|(\s*#.*$)\@=)', # End of a line (whitespace_?_, comma, whitespace_?_ comment.*) 'TRANSITION': r'(\s+-\\s+\S+)?', Sorry for the terrible quoting, anyway: Does it really make sense to have two spaces in front of # ? +#syn match sdEntryM /@@DENYFILE@@(r|mk|x)+@@EOL@@/ contains=sdGlob,sdComment nextgroup=@sdEntry,sdComment,sdError,sdInclude -#syn match sdEntryM /@@DENYFILE@@(r|m|k|x)+@@EOL@@/ contains=sdGlob,sdComment nextgroup=@sdEntry,sdComment,sdError,sdInclude NAK - you broke the regex here (first search for the difference yourself, then have a look at [1] ;-) (and yes, I'd like to have this un-broken even if it's only a comment) With that fixed, Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz [1] It probably becomes more obvious with the lines shortened: +#syn match sdEntryM /@@DENYFILE@@(r|mk|x)+@@EOL@@/ -#syn match sdEntryM /@@DENYFILE@@(r|m|k|x)+@@EOL@@/ ^^^ -- ...womit die Geschichte wieder von vorn losgeht... Jaha, aber nun ist das eine vollkommen andere Situation: Jetzt hast Du nämlich eine von Suse generierte kdmrc für ein von Suse geliefertes KDE, und KDE ist eine vom Installationssupport abgedeckte Komponente. Also ist das ein meldefähiger und supportberechtigter Bug! :-) [ Gerald Martin und Kristian Köhntopp in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] utils: fix apparmor.vim rlimits support (was Re: [patch 13/18] parser: add rttime rlimit support)
Hello, Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie: On Fri, Jan 17, 2014 at 12:45:27AM +0100, Christian Boltz wrote: (and BTW, did you test if apparmor.vim displays all tests from 12/18 correctly?) Apparently I missed all the incorrect highlighting vim gave me while creating those test cases, because no, apparmor.vim does not display many of them correctly. The following is a patch to address the shortcomings I found: Subject: utils: fix apparmor.vim rlimits support The rlimits syntax checking support in apparmor.vim was broken in various unhelpful ways: [...] Thanks! Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Yeah, life always gets in the way of the important stuff :-) [Per Jessen in opensuse-project] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] update winbindd profile
Hello, this patch includes several updates for the winbindd profile that the openSUSE package collected over the last months. - add abstractions/samba to usr.sbin.winbindd profile (and cleanup things that are included in the abstraction - the cleanup part is not in the openSUSE package) - add capabilities ipc_lock and setuid to usr.sbin.winbindd profile (bnc#851131) - updates for samba 4.x and kerberos (bnc#846586#c12 and #c15, bnc#845867, bnc#846054) - drop always-outdated Last Modified comment References: see the bnc# above (they are bug numbers at bugzilla.novell.com) === modified file 'profiles/apparmor.d/usr.sbin.winbindd' --- profiles/apparmor.d/usr.sbin.winbindd 2012-11-06 22:19:46 +++ profiles/apparmor.d/usr.sbin.winbindd 2014-01-19 15:56:00 @@ -1,33 +1,32 @@ -# Last Modified: Mon Mar 26 20:28:18 2012 #include tunables/global /usr/sbin/winbindd { #include abstractions/base #include abstractions/nameservice - - /etc/samba/dhcp.conf r, + #include abstractions/samba + + deny capability block_suspend, + + capability ipc_lock, + capability setuid, + /etc/samba/passdb.tdb rwk, /etc/samba/secrets.tdb rwk, @{PROC}/sys/kernel/core_pattern r, /tmp/.winbindd/ w, + /tmp/krb5cc_* rwk, /usr/lib*/samba/idmap/*.so mr, /usr/lib*/samba/nss_info/*.so mr, + /usr/lib*/samba/pdb/*.so mr, /usr/sbin/winbindd mr, - /var/lib/samba/account_policy.tdb rwk, - /var/lib/samba/gencache.tdb rwk, - /var/lib/samba/gencache_notrans.tdb rwk, - /var/lib/samba/group_mapping.tdb rwk, - /var/lib/samba/messages.tdb rwk, - /var/lib/samba/netsamlogon_cache.tdb rwk, - /var/lib/samba/serverid.tdb rwk, - /var/lib/samba/winbindd_cache.tdb rwk, - /var/lib/samba/winbindd_privileged/pipe w, - /var/log/samba/cores/ rw, - /var/log/samba/cores/winbindd/ rw, - /var/log/samba/cores/winbindd/** rw, - /var/log/samba/log.wb-* w, + /var/cache/samba/*.tdb rwk, + /var/lib/samba/smb_krb5/krb5.conf.* rw, + /var/lib/samba/smb_tmp_krb5.* rw, + /var/lib/samba/winbindd_cache.tdb* rwk, /var/log/samba/log.winbindd rw, /{var/,}run/samba/winbindd.pid rwk, + /{var/,}run/samba/winbindd/ rw, + /{var/,}run/samba/winbindd/pipe w, # Site-specific additions and overrides. See local/README for details. #include local/usr.sbin.winbindd Regards, Christian Boltz -- auf meinem Rechen Suse 8.2 KDE 3.1.1, [...] Hey, man kann SuSE inzwischen sogar auf einem Rechen installieren? Wow, da muss ich morgen mal im Garten vorbei schauen... :-)) [ Bernhard Schimanski und Thomas Hertweck in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] profiles/Makefile: make sure all profiles have #include local/...
Hello, the following patch makes sure all profiles have a line #include local/... BTW: All profiles pass this test :-) but having an additional check can never hurt. === modified file 'profiles/Makefile' --- profiles/Makefile 2013-01-05 00:33:41 + +++ profiles/Makefile 2014-01-19 19:22:56 + @@ -44,6 +44,7 @@ for profile in ${TOPLEVEL_PROFILES}; do \ fn=$$(basename $$profile); \ echo # Site-specific additions and overrides for '$$fn' ${PROFILES_SOURCE}/local/$$fn; \ + grep include\\s\\s*local/$$fn $$profile /dev/null || { echo $$profile doesn't contain #include local/$$fn ; exit 1; } ; \ done; \ .PHONY: install Regards, Christian Boltz -- 116: Programm Sobald eine Datei von einem Virus infiziert werden kann, ist es ein Programm. (Markus Kuhn) -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 1/3] dovecot profiles: introduce tunables/dovecot
Hello, Am Donnerstag, 23. Januar 2014 schrieb John Johansen: On 01/19/2014 08:58 AM, Christian Boltz wrote: this patch introduces tunables/dovecot (with @{DOVECOT_MAILSTORE}) and replaces the mail storage location in various dovecot-related profiles with this variable. It also adds nice copyright headers (I hope I got the bzr log right ;-) a few comments inline === added file 'profiles/apparmor.d/tunables/dovecot' --- profiles/apparmor.d/tunables/dovecot1970-01-01 00:00:00 + +++ profiles/apparmor.d/tunables/dovecot2014-01-19 16:08:06 ... +# @{DOVECOT_MAILSTORE} is a space-separated list of all directories +# where dovecot is allowed to store and read mails +# +# The default value is quite broad to avoid breaking existing setups. +# Please change @{DOVECOT_MAILSTORE} to (only) contain the directory +# you use, and remove everything else. + +@{DOVECOT_MAILSTORE}=@{HOME}/Maildir/ @{HOME}/mail/ @{HOME}/Mail/ /var/vmail/ /var/mail/ /var/spool/mail/ === modified file 'profiles/apparmor.d/usr.lib.dovecot.imap' --- profiles/apparmor.d/usr.lib.dovecot.imap2011-08-26 23:12:10 + +++ profiles/apparmor.d/usr.lib.dovecot.imap 2014-01-19 ... - @{HOME} r, - @{HOME}/Maildir/ rw, - @{HOME}/Maildir/** klrw, - @{HOME}/Mail/ rw, - @{HOME}/Mail/* klrw, - @{HOME}/Mail/.imap/** klrw, - @{HOME}/mail/ rw, - @{HOME}/mail/* klrw, - @{HOME}/mail/.imap/** klrw, + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, so this is slightly wider perms than - @{HOME}/{m,M}ail/* klrw, - @{HOME}/{m,M}ail/.imap/** klrw, is this what we want? The idea of @{DOVECOT_MAILSTORE} is to allow the directories that were allowed in the old profile, and getting all profiles in sync so that for example IMAP and POP3 allow access to the same directory. I know the list got quite long - but that's what you get from checking the current dovecot-related profiles. (Maybe also a location from a bugreport on bnc sneaked in, I'd have to check that ;-) I'd happily shorten the list to just /var/vmail/, but I'm sure users would kill me for doing it ;-) The perfect solution would be to auto-generate @{DOVECOT_MAILSTORE} from the dovecot config, but unfortunately that isn't as easy as it looks. (Proposals and scripts welcome ;-) + @{HOME} r, # ??? why the ???, not sure if this rule is required above, you'll find - @{HOME} r, I'm also not sure if it's required (that's why I added ???), but wanted to keep it for backwards compability (there must be a reason why it's there ;-) (If you are sure we can remove it, this should be a separate patch titled break the profile or so ;-) /usr/lib/dovecot/imap mr, - /var/mail/* klrw, - /var/spool/mail/* klrw, again a slight widening of permissions Yes, see above. === modified file 'profiles/apparmor.d/usr.lib.dovecot.pop3' --- profiles/apparmor.d/usr.lib.dovecot.pop32011-08-26 23:12:10 + +++ profiles/apparmor.d/usr.lib.dovecot.pop3 2014-01-19 16:08:30 + @@ -1,6 +1,18 @@ ... - @{HOME} r, - @{HOME}/mail/* klrw, - @{HOME}/mail/.imap/** klrw, - @{HOME}/Maildir/ rw, - @{HOME}/Maildir/** klrw, + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, + + @{HOME} r, # ??? again the change in allowed permissions again see above ;-) Regards, Christian Boltz -- how to use the -b parameter ? You... type it in. [ Jun Hu and Jan Engelhardt in opensuse-packaging] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 01/11] mod_apparmor: fix logging [v3]
Hello, Am Donnerstag, 23. Januar 2014 schrieb Steve Beattie: On Thu, Jan 23, 2014 at 03:04:53AM -0800, John Johansen wrote: Looks good, though I did find myself wishing for a patch to rename immunix to apparmor. Yeah, as well as a patch to fix up some of the whitespace quirks (lots of trailing whitespace for one). But I wanted functional code changes That, and also a funny mix of tabs and spaces in several lines. to land first before doing any of that, to make it easier to merge to 2.8, if need be. Personally, I'd like to have the fixes applied[1] to 2.8 ;-) (maybe except the change to using aa_change_hatv to be very sure nothing breaks?) Nevertheless, I'll probably take the risk and test 2.8 with the latest mod_apparmor.c as soon as you commit your patches to trunk. (I want one big patch, not copypaste from 11 mails all changing the same file ;-) BTW: will the updated mod_apparmor also need 2.8 r2111? (libapparmor: fix aa_change_hat token format string) That all said - how many lines are _not_ touched by your patch series? ;-) Regards, Christian Boltz [1] no need to write backport - the patches should apply without problems ;-) -- Grundsaetzlich ist es so, dass was unter Linux als Alpha Bezeichnet wird, einem Release unter Windows entspricht, die Beta Versionen ungefaehrt Service Pack 2 entsprechen und Stabil unter linux keinen Gegenpart in der Windows Welt hat .. :) [Stefan Onken in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 09/11] mod_apparmor: add logging for AAHatName/AADefaultHatName policy misconfig
Hello, Am Donnerstag, 23. Januar 2014 schrieb Steve Beattie: It kind of points to a minor deficiency in aa_change_hatv()'s interface, in that you know you successfully changed to hat or not, but not which one. That sounds like we should find a way to change that ;-) Does aa_change_hatv internally know to which hat it switched? If yes, I'd propose to add another function similar to aa_change_hatv that indicates the selected hat in its return value. (aa_change_hat(2) says the return value is zero on success, so changing the return code of the existing function isn't a real option.) Regards, Christian Boltz -- So... Hm... ich bin etwas aufgeschmissen. How to troubleshoot without trouble? [Ratti in fontlinge-devel] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 1/3] dovecot profiles: introduce tunables/dovecot
Hello, Am Donnerstag, 23. Januar 2014 schrieb John Johansen: On 01/23/2014 06:37 AM, Christian Boltz wrote: Am Donnerstag, 23. Januar 2014 schrieb John Johansen: On 01/19/2014 08:58 AM, Christian Boltz wrote: this patch introduces tunables/dovecot (with @{DOVECOT_MAILSTORE}) and replaces the mail storage location in various dovecot-related profiles with this variable. It also adds nice copyright headers (I hope I got the bzr log right ;-) a few comments inline === added file 'profiles/apparmor.d/tunables/dovecot' --- profiles/apparmor.d/tunables/dovecot 1970-01-01 00:00:00 +++ profiles/apparmor.d/tunables/dovecot 2014-01-19 16:08:06 ... +# @{DOVECOT_MAILSTORE} is a space-separated list of all directories +# where dovecot is allowed to store and read mails +# +# The default value is quite broad to avoid breaking existing setups. +# Please change @{DOVECOT_MAILSTORE} to (only) contain the directory +# you use, and remove everything else. + +@{DOVECOT_MAILSTORE}=@{HOME}/Maildir/ @{HOME}/mail/ @{HOME}/Mail/ /var/vmail/ /var/mail/ /var/spool/mail/ === modified file 'profiles/apparmor.d/usr.lib.dovecot.imap' --- profiles/apparmor.d/usr.lib.dovecot.imap 2011-08-26 23:12:10 + +++ profiles/apparmor.d/usr.lib.dovecot.imap2014-01-19 ... - @{HOME} r, - @{HOME}/Maildir/ rw, - @{HOME}/Maildir/** klrw, - @{HOME}/Mail/ rw, - @{HOME}/Mail/* klrw, - @{HOME}/Mail/.imap/** klrw, - @{HOME}/mail/ rw, - @{HOME}/mail/* klrw, - @{HOME}/mail/.imap/** klrw, + @{DOVECOT_MAILSTORE}/ rw, + @{DOVECOT_MAILSTORE}/** rwkl, so this is slightly wider perms than - @{HOME}/{m,M}ail/* klrw, - @{HOME}/{m,M}ail/.imap/** klrw, is this what we want? The idea of @{DOVECOT_MAILSTORE} is to allow the directories that were allowed in the old profile, and getting all profiles in sync so that for example IMAP and POP3 allow access to the same directory. I know the list got quite long - but that's what you get from checking the current dovecot-related profiles. (Maybe also a location from a bugreport on bnc sneaked in, I'd have to check that ;-) I'd happily shorten the list to just /var/vmail/, but I'm sure users would kill me for doing it ;-) The perfect solution would be to auto-generate @{DOVECOT_MAILSTORE} from the dovecot config, but unfortunately that isn't as easy as it looks. (Proposals and scripts welcome ;-) Not quite what I meant by widening of the perms (though I would love to have the list generated from the config too). The list for @{DOVECOT_MAILSTORE} is fine. What I meant was that for the case of @{HOME}/mail/ and @{HOME}/Mail/ we used to have @{HOME}/{m,M}ail/* klrw, @{HOME}/{m,M}ail/.imap/** klrw, but we now have @{HOME}/{m,M}ail/** klrw, the difference being that we only allowed the recursive ** for the .imap dir. I just wanted to make sure this widening of permissions was intentional More or less ;-) I'd call it the price we have to pay to get it configurable in @{DOVECOT_MAILSTORE} - which also means permissions for ~/{m,M}ail can easily be removed. Besides that, only allowing the .imap/** subdirectory doesn't match the other allowed directories, especially ~/Maildir/** which we already have in the profile. Regards, Christian Boltz -- if ( ! ifdef $root ) { [...] } ifdef? Da hat einer zusammengerollte Makefiles geraucht... [ Christian Boltz und Ratti in fontlinge-devel] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] /usr/lib/dovecot/auth and mysql
Hello, this patch is an interesting one - /usr/lib/dovecot/auth reads the mysql config files, which is not covered by abstractions/mysql. Now the interesting question is where we should add this. a) add it to abstractions/mysql because it belongs to mysql even if /usr/lib/dovecot/auth is the only one that needs it b) add it to usr.lib.dovecot.auth because only /usr/lib/dovecot/auth is the only one that needs it At the moment, I tend to b) to avoid superfluous permissions for other programs with abstractions/mysql, but I'd like to hear your opinions ;-) === modified file 'profiles/apparmor.d/usr.lib.dovecot.auth' --- profiles/apparmor.d/usr.lib.dovecot.auth2014-01-26 21:46:51 +++ profiles/apparmor.d/usr.lib.dovecot.auth2014-01-26 22:36:47 @@ -23,6 +23,10 @@ capability setgid, capability setuid, + /etc/my.cnf r, + /etc/my.cnf.d/ r, + /etc/my.cnf.d/*.cnf r, + /etc/dovecot/dovecot-database.conf.ext r, /etc/dovecot/dovecot-sql.conf.ext r, /usr/lib/dovecot/auth mr, Regards, Christian Boltz -- chliEßlichle sendi emeiSt Enleut ehier mehralsdreIpo Stingsa Mtag sOd Asesdoch et. Waserm üdentwärdenkahnimmerrattentsumÜßenw aßIrge nDeinezUs Ahmäst ell unkvonbU chst, abensagenw iel ;-) [Tilman Ahr in dcoulm zum Thema Rechtschreibfehler stoeren doch nicht] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] usr.bin.dovecot profile
Hello, after testing the dovecot profiles on a new server, I noticed /usr/sbin/dovecot needs some more permissions: -mysql access - execution permissions for /usr/lib/dovecot/dict and lmtp - write access to some postfix sockets, used to - provide SMTP Auth via dovecot - deliver mails to dovecot via LMTP - and read access to /proc/filesystems === modified file 'profiles/apparmor.d/usr.sbin.dovecot' --- profiles/apparmor.d/usr.sbin.dovecot2014-01-26 21:48:02 + +++ profiles/apparmor.d/usr.sbin.dovecot2014-01-26 23:18:44 + @@ -15,6 +15,7 @@ /usr/sbin/dovecot { #include abstractions/authentication #include abstractions/base + #include abstractions/mysql #include abstractions/nameservice #include abstractions/ssl_certs #include abstractions/ssl_keys @@ -33,13 +34,16 @@ /etc/lsb-release r, /etc/SuSE-release r, @{PROC}/@{pid}/mounts r, + @{PROC}/filesystems r, /usr/bin/doveconf rix, /usr/lib/dovecot/anvil Px, /usr/lib/dovecot/auth Px, /usr/lib/dovecot/config Px, + /usr/lib/dovecot/dict Px, /usr/lib/dovecot/dovecot-auth Pxmr, /usr/lib/dovecot/imap Pxmr, /usr/lib/dovecot/imap-login Pxmr, + /usr/lib/dovecot/lmtp Px, /usr/lib/dovecot/log Px, /usr/lib/dovecot/managesieve Px, /usr/lib/dovecot/managesieve-login Pxmr, @@ -50,6 +54,8 @@ /usr/sbin/dovecot mrix, /var/lib/dovecot/ w, /var/lib/dovecot/* rwkl, + /var/spool/postfix/private/auth w, + /var/spool/postfix/private/dovecot-lmtp w, /{,var/}run/dovecot/ rw, /{,var/}run/dovecot/** rw, link /{,var/}run/dovecot/** - /var/lib/dovecot/**, Regards, Christian Boltz -- Sorry, mit java kenne ich mich gar nicht aus, das ist mir einfach zu unportabel. [Thorsten Kukuk in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] new profile tools - handling of (F)inish
Hello, currently, selecting (F)inish in the new profile tools basically means aborting without saving anything. However, we already have Abo(r)t for that ;-) (F)inish should ask the user if he wants to save the changed profiles before exiting. The attached patch does this (except for two places, where I just added a TODO instead of real code ;-) - probably solvable by copypaste) I don't really like that I had to remove the central handling of (F)inish in ui.py and add it to several places in aa.py. OTOH calling confirm_and_finish() from ui.py a) isn't that easy and b) would mix program logic into ui.py - I'm not sure if this is a good idea. Hmm, maybe we should also move the handling for Abo(r)t to aa.py and make ui.py totally dump to be consistent, even if this means to add some duplicated lines? Or add a small function to aa.py as wrapper for UI_PromptUser that - always adds (F)inish and Abo(r)t - handles those two - and returns any other answer to the calling function ? This function could then be used for all user prompts, with the exception of the save profile / view diff prompt. Opinions? That all said: Kshitij, this is your chance to write an evil review about a patch from me - don't waste it ;-) (In the unlikely case that you like my patch, you can of course commit it ;-) Regards, Christian Boltz -- Werbung lügt, Corporate Design sagt die Wahrheit. Naja, alle _guten_ Komponenten der Wahrheit. :-) [Ratti] === modified file 'Testing/runtests-py2.sh' (properties changed: -x to +x) === modified file 'Testing/runtests-py3.sh' (properties changed: -x to +x) === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-12-19 21:42:58 + +++ apparmor/aa.py 2014-01-27 21:52:05 + @@ -492,6 +492,7 @@ ans = '' while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans: +# TODO: handle FINISHED ans, arg = UI_PromptUser(q) p = profile_hash[options[arg]] q['selected'] = options.index(options[arg]) @@ -975,6 +979,8 @@ ans = UI_PromptUser(q) +# TODO: handle FINISHED + transitions[context] = ans if ans == 'CMD_ADDHAT': @@ -1236,6 +1242,11 @@ q['functions'] += build_x_functions(default, options, exec_toggle) ans = '' continue + +if ans == 'CMD_FINISHED': +save_profiles() +return; + if ans == 'CMD_nx' or ans == 'CMD_nix': arg = exec_target ynans = 'n' @@ -1535,6 +1546,11 @@ done = False while not done: ans, selected = UI_PromptUser(q) + +if ans == 'CMD_FINISHED': +save_profiles() +return; + # Ignore the log entry if ans == 'CMD_IGNORE_ENTRY': done = True @@ -1779,6 +1795,10 @@ ans, selected = UI_PromptUser(q) +if ans == 'CMD_FINISHED': +save_profiles() +return; + if ans == 'CMD_IGNORE_ENTRY': done = True break @@ -1930,6 +1950,11 @@ done = False while not done: ans, selected = UI_PromptUser(q) + +if ans == 'CMD_FINISHED': +save_profiles() +return; + if ans == 'CMD_IGNORE_ENTRY': done = True break @@ -2224,7 +2249,7 @@ pass finishing = False -# Check for finished +# TODO: Check for finished save_profiles() ##if not repo_cfg['repository'].get('upload', False) or repo['repository']['upload'] == 'later': === modified file 'apparmor/ui.py' --- apparmor/ui.py 2013-12-29 09:42:30 + +++ apparmor/ui.py 2014-01-27 22:00:57 + @@ -288,10 +288,10 @@ if cmd == 'CMD_ABORT': confirm_and_abort() cmd = 'XXXINVALIDXXX' -elif cmd == 'CMD_FINISHED': -if not params: -confirm_and_finish() -cmd = 'XXXINVALIDXXX' +#elif cmd == 'CMD_FINISHED': +#if not params: +#confirm_and_finish() +#cmd = 'XXXINVALIDXXX' return (cmd, arg) def confirm_and_abort(): @@ -319,9 +319,9 @@ }) ypath, yarg = GetDataFromYast() -def confirm_and_finish(): -sys.stdout.write(_('FINISHING...\n')) -sys.exit(0) +#def confirm_and_finish(): +#sys.stdout.write(_('FINISHING...\n
[apparmor] systemd AppArmorProfile=
Hello, I was just pointed to https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg15717.html which contains a set of patches that seem to implement aa-exec-like behaviour in systemd (and a note that there will be a fresh patch soon). @Michael: I hope the mail address I found is the right one. If not, sorry for disturbing ;-) Feel free to send your (updated) patches to the AppArmor mailinglist (in CC) to get a review of the AppArmor side of your patches. BTW: It looks like your patch requires the profiles to be loaded already. Do you have any plans for loading, reloading or removing profiles via systemd? @all: Can someone have a look at those patches, please? (Even if it's clear that there will be a v2 ;-) Regards, Christian Boltz -- Manfred, Du solltest so spaet keine Emails mehr schreiben :-) Danke für die Berichtigung, werd mir den Tipp hinter die Ohren schreiben und nur noch Mailen, wenn ich die Augen zumindestens zu einem drittel aufkriege. [ Thomas Hertweck und Manfred Tremmel in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] review r93..95
Hello, the review for r93..95 of the new profile tools is attached. Your commits fixed several bugs, but there are also cases where you replaced one bug with another ;-) I also answered your question about trace handling in the aa-* tools, even if my answer where to handle it won't be too surprising ;-) Regards, Christian Boltz -- Well, I guess, Stephan knows very well, what the fuzz is about: it's about hundreds of patches, which will have to be regenerated, done as an employment-creation measure for this lazy gang of packagers. [Hans-Peter Jansen in opensuse-packaging] revno: 93 committer: Kshitij Gupta kgupta8...@gmail.com branch nick: apparmor-profile-tools timestamp: Sat 2014-02-01 06:14:05 +0530 message: Some bugfixes for UIYesNo to deny invalid keys, fix autodep when creating new profiles === modified file 'Tools/aa-audit' --- Tools/aa-audit 2013-10-21 21:36:23 + +++ Tools/aa-audit 2014-02-01 00:44:05 + #[merged review for aa-audit r93..95] # # r95 message: # Fixed the sample --trace feature. Opinions on using it? and should it be implemented in every tool separately? # not shocking users with a backtrace is a good idea. # maybe you should only hide AppArmor exceptions (they could be called expected, and the error message in them is usually enough) # and still display other (unexpected) exceptions (because we want to know where something fails unexpectedly) # This should of course ;-) be handled in a central place (tools.py?) instead of adding it to every aa-* === modified file 'apparmor/aa.py' --- apparmor/aa.py 2013-12-19 21:42:58 + +++ apparmor/aa.py 2014-02-01 00:44:05 + @ -418,7 +418,8 @@ local_profile[hat]['flags'] = 'complain' created.append(localfile) - +changed.append(localfile) # looks good, but... # python Tools/aa-genprof ~cb/linuxtag/apparmor/scripts/hello # Traceback (most recent call last): # File Tools/aa-genprof, line 102, in module # apparmor.autodep(program) # File /usr/lib/python2.7/site-packages/apparmor/aa.py, line 569, in autodep # profile_data = create_new_profile(pname) # File /usr/lib/python2.7/site-packages/apparmor/aa.py, line 421, in create_new_profile # changed.append(localfile) # AttributeError: 'dict' object has no attribute 'append' # hint (aa.py around line 90) # changed = dict() # created = [] @@ -865,34 +869,42 @@ def build_x_functions(default, options, exec_toggle): ret_list = [] +fallback_toggle = False if exec_toggle: if 'i' in options: ret_list.append('CMD_ix') if 'p' in options: ret_list.append('CMD_pix') -ret_list.append('CMD_EXEC_IX_OFF') +fallback_toggle = True -elif 'c' in options: +if 'c' in options: ret_list.append('CMD_cix') -ret_list.append('CMD_EXEC_IX_OFF') +fallback_toggle = True -elif 'n' in options: +if 'n' in options: ret_list.append('CMD_nix') +fallback_toggle = True +if fallback_toggle: ret_list.append('CMD_EXEC_IX_OFF') # placing CMD_EXEC_IX_OFF here has a small disadvantage - it will be offered left of CMD_ux (but should be right of it) -elif 'u' in options: +if 'u' in options: ret_list.append('CMD_ux') === modified file 'apparmor/common.py' --- apparmor/common.py 2013-12-29 09:42:30 + +++ apparmor/common.py 2014-02-01 00:44:05 + @@ -201,9 +201,16 @@ +def user_perm(prof_dir): # the function name is not too helpful # what about is_writeable()? # (or must_be_writeable() and let it raise an exception if prof_dir is not writeable, see below) +if not os.access(prof_dir, os.R_OK): # [should check for W_OK, already fixed in r94] +sys.stdout.write(Cannot write to profile directory.\n + + Please run as a user with appropriate permissions. ) # please make the error message translatable # I'd also mention prof_dir in the message. # To make the function useable for everything (not just profile dir), change the message to Cannot write to %s class DebugLogger(object): def __init__(self, module_name=__name__): ... +try: +logging.basicConfig(filename=self.logfile, level=self.debug_level, +format='%(asctime)s - %(name)s - %(message)s\n') +except OSError: # looks good, but fails with py2 # - use except IOError: instead, that works with py2 and py3 +# Unable to open the default logfile, so create a temporary logfile and tell use about it # ... tell use_r_ about it +import tempfile +templog = tempfile.NamedTemporaryFile('w', prefix='apparmor', suffix='.log' ,delete=False
Re: [apparmor] systemd AppArmorProfile=
Hello, Am Sonntag, 2. Februar 2014 schrieb Michael Scherer: Le samedi 01 février 2014 à 18:18 +0100, Christian Boltz a écrit : BTW: It looks like your patch requires the profiles to be loaded already. Do you have any plans for loading, reloading or removing profiles via systemd? I had plan to look on how Suse is doing this, but the only way i found after a quick look was t run a external binary, and I think that's something that should be avoided at least in systemd. I also didn't found a potential C library to do that. The current way is the /etc/init.d/boot.apparmor initscript, which calls code in /lib/apparmor/rc.apparmor.functions, which finally loads the profiles using apparmor_parser. AppArmor 3.0 (not released yet) will make it a bit easier - apparmor_parser will be able to load all profiles in /etc/apparmor.d/ at once, instead of having to load one profile after the other. This means (re)loading all profiles can be done with apparmor_parser -r /etc/apparmor.d/ Maybe you need some additional options, but you should get the point. Also note that this way didn't get much testing yet. I slightly ;-) doubt if it's a good idea to re-invent apparmor_parser inside systemd, and calling it as external binary doesn't sound too bad to me. (Hey, it worked without problems for the last 10 years ;-) If you really want a library, the best way is probably to convert most of apparmor_parser into a library. However, I'm afraid this will need some[tm] time. Well, I have the v2 already, i just didn't found time to really test it with a VM before sending it. Ah, the usual ENOTIME ;-) Regards, Christian Boltz -- Please resolve this as NOT A BUG and USER SHOULD HAVE MORE COFFEE BEFORE FILING BUGS. I apologize for taking up valuable developer time! [Jon Nelson in https://bugzilla.novell.com/show_bug.cgi?id=776271#c2] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Branch ~apparmor-dev/apparmor/master] Rev 2363: Move short_options next to long_options to make them easier to keep in sync
Hello, Am Mittwoch, 5. Februar 2014 schrieben Sie: revno: 2363 committer: John Johansen john.johan...@canonical.com branch nick: apparmor timestamp: Tue 2014-02-04 20:56:17 -0500 message: Move short_options next to long_options to make them easier to keep in sync === modified file 'parser/parser_main.c' [...] === modified file 'parser/parser_regex.c' --- parser/parser_regex.c 2014-01-24 18:47:42 + +++ parser/parser_regex.c 2014-02-05 01:56:17 + @@ -493,8 +493,6 @@ if ((entry-mode AA_USER_SHIFT) AA_EXEC_INHERIT) entry-mode |= AA_EXEC_MMAP AA_USER_SHIFT; - /* relying on ptrace and change_profile not getting merged earlier */ - /* the link bit on the first pair entry should not get masked * out by a deny rule, as both pieces of the link pair must * match. audit info for the link is carried on the second @@ -556,19 +554,6 @@ if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, index, vec, dfaflags)) return FALSE; } - if (entry-mode (AA_USER_PTRACE | AA_OTHER_PTRACE)) { - int mode = entry-mode (AA_USER_PTRACE | AA_OTHER_PTRACE); - if (entry-ns) { - const char *vec[2]; - vec[0] = entry-ns; - vec[1] = entry-name; - if (!aare_add_rule_vec(dfarules, 0, mode, 0, 2, vec, dfaflags)) - return FALSE; - } else { - if (!aare_add_rule(dfarules, entry-name, 0, mode, 0, dfaflags)) - return FALSE; - } - } return TRUE; } This part doesn't look related to short options ;-) Regards, Christian Boltz -- Henne, did you actually test this before closing the bug as invalid? of course i did not test it. do you think i'm bored? [ Christian Boltz and Hendrik Vogelsang in https://bugzilla.novell.com/show_bug.cgi?id=420972] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] new profile tools: preserve full initial comment
Hello, while playing with aa-cleanprof, I noticed only the last line of the initial comment was preserved. This patch - preserves the complete initial comment - makes sure whitespace inside the comment is kept (except leading whitespace - line.trim() is still applied). - no longer removes the # vim:syntax line Note: I didn't test if handling the REPOSITORY line still works (in theory it should), but without a working repo, I don't care too much ;-) BTW: It might be a good idea to use a different variable name for the result of line.split() to avoid confusion. I also noticed you didn't commit my patch for handling (F)inish I sent a week ago. Kshitij, if you like my patches, please commit them - I'd like to have a more readable bzr diff again soon ;-) === modified file 'apparmor/aa.py' --- apparmor/aa.py 2014-02-01 00:44:05 + +++ apparmor/aa.py 2014-02-05 22:30:08 + @@ -2937,10 +2966,12 @@ elif line[0] == '#': # Handle initial comments if not profile: -if line.startswith('# vim:syntax') or line.startswith('# Last Modified:'): +#if line.startswith('# vim:syntax') or line.startswith('# Last Modified:'): +if line.startswith('# Last Modified:'): continue -line = line.split() -if len(line) 1 and line[1] == 'REPOSITORY:': +elif line.startswith('# REPOSITORY'): # TODO: allow any number of spaces/tabs +#if len(line) 1 and line[1] == 'REPOSITORY:': +line = line.split() if len(line) == 3: repo_data = {'neversubmit': True} elif len(line) == 5: @@ -2948,7 +2979,7 @@ 'user': line[3], 'id': line[4]} else: -initial_comment = ' '.join(line) + '\n' +initial_comment = initial_comment + line + '\n' else: raise AppArmorException(_('Syntax Error: Unknown line found in file: %s line: %s') % (file, lineno+1)) Regards, Christian Boltz -- cboltz jjohansen: you are making it too easy for kshitij8 ;-) jjohansen cboltz: oops sorry, now I'll have to come up with a new task to make him suffer :) sarnold review the c++11 conversion? :) * sarnold runs jjohansen haha, sarnold I said suffer, not drive him to commit suicide [from #apparmor] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 1/8] chromium-browser profile
Hello, Am Dienstag, 11. Februar 2014 schrieb Seth Arnold: Author: Jamie Strandboge ja...@canonical.com Description: chromium-browser profile Forwarded: yes --- profiles/apparmor.d/usr.bin.chromium-browser | 221 Just to make sure I understand this correct - you propose to add this profile to bzr trunk to the set of default profiles, right? Short summary: The profile contains some restrictions that will result in quite some annoyed users (especially the restriction to ~/Public and ~/Downloads). Therefore I'm not sure if it should be in the set of profiles that are enabled by default. I'm thinking about introducing an apparmor-profiles-paranoid package (with a big warning that it _will_ break what a typical user often does) since some time - maybe this profile would be a reason to finally do it ;-) See below for more details. Index: b/profiles/apparmor.d/usr.bin.chromium-browser === --- /dev/null +++ b/profiles/apparmor.d/usr.bin.chromium-browser @@ -0,0 +1,221 @@ +# Author: Jamie Strandboge ja...@canonical.com +#include tunables/global + +# We need 'flags=(attach_disconnected)' in newer chromium versions +/usr/lib/chromium-browser/chromium-browser flags=(attach_disconnected) { + #include abstractions/audio + #include abstractions/cups-client + #include abstractions/dbus-session just curious - would dbus-session-strict be enough? + #include abstractions/gnome + #include abstractions/ibus + #include abstractions/nameservice + #include abstractions/user-tmp + + # This include specifies which ubuntu-browsers.d abstractions to use. Eg, if + # you want access to productivity applications, adjust the following file + # accordingly. + #include abstractions/ubuntu-browsers.d/chromium-browser Users of other distributions will *love* ubuntu-browsers.d ;-) I know that it's only a name, nevertheless it would be a good idea to rename it (not the most urgent problem, but... ;-) [...] + # Default profile allows downloads to ~/Downloads and uploads from ~/Public This comment is wrong - uploads are allowed from ~/Public/ and ~/Downloads/ ;-) That said: yes, I know this setup is very secure, but I'm also sure it will cause some ;-) bugreports like I can't download files to ~/coolstuff The perfect solution would be to wait for the content helper - what's the current status there? + owner @{HOME}/ r, + owner @{HOME}/Public/ r, + owner @{HOME}/Public/* r, + owner @{HOME}/Downloads/ r, + owner @{HOME}/Downloads/* rw, + + # Helpers + /usr/bin/xdg-open ixr, + /usr/bin/gnome-open ixr, + /usr/bin/gvfs-open ixr, + # TODO: kde, xfce Oh nice - this TODO will result in the next flood of bugreports (according to a survey 70% of the openSUSE users use KDE as their desktop - guess how many annoyed users and bugreports that means...) Hint: For KDE, it is probably /usr/bin/kde-open + profile xdgsettings { [...] +# Setting the default browser [...] +owner @{HOME}/.local/share/applications/ w, Hmm, why write permissions for the directory? +owner @{HOME}/.local/share/applications/mimeapps.list* rw, Personally, I'd say a browser should never be allowed to change the default browser (and I'd even forbid to check if it is the current default browser - I'm not the biggest fan of the hey, I'm not your default browser warnings ;-) Additionally, there's a chance that malicious code changes the default application for a file the user just downloaded, which could in theory cause some delayed remote code execution (somewhat similar to stored XSS) + } + + # Site-specific additions and overrides. See local/README for details. + #include local/usr.bin.chromium-browser Hiding this #include between two child profiles is, hmm, interesting ;-) Can you move it to a more visible place, please? (like the end of the main profile, above the child profiles) +profile chromium_browser_sandbox { [...] +# *Sigh* +capability sys_ptrace, Nice comment, but not too useful for the average user... Regards, Christian Boltz -- Graphisch??? Wie meinen? Hast du zuviel Fleisch von zu gluecklichen Rindern gefuttert? *scnr* Wozu zum Henker sollte man sowas brauchen? Logo ginge auch per ASCII :) (Logo? welches Logo? Wozu ueberhaupt?) [David Haller in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] update abstractions/winbind
Hello, abstractions/winbind needs an update: - some *.dat files live in a different directory nowadays (at least in openSUSE) - the openSUSE smb.conf includes the (autogenerated) dhcp.conf, so this file also needs to be readable. References: https://bugzilla.novell.com/show_bug.cgi?id=863226 I also propose this patch for 2.8 === modified file 'profiles/apparmor.d/abstractions/winbind' --- profiles/apparmor.d/abstractions/winbind2011-11-01 17:35:29 +++ profiles/apparmor.d/abstractions/winbind2014-02-14 18:34:15 @@ -13,7 +13,9 @@ /tmp/.winbindd/pipe rw, /var/{lib,run}/samba/winbindd_privileged/pipe rw, /etc/samba/smb.conf r, + /etc/samba/dhcp.confr, /usr/lib*/samba/valid.dat r, /usr/lib*/samba/upcase.dat r, /usr/lib*/samba/lowcase.dat r, + /usr/share/samba/codepages/{lowcase,upcase,valid}.dat r, Regards, Christian Boltz -- Planung ist der Ersatz des Zufalls durch den Irrtum. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 0/2] Preliminary XDG user dir support
Hello, Am Freitag, 14. Februar 2014 schrieb Jamie Strandboge: Add basic support for XDG user dirs: 1. Update profiles/apparmor.d/tunables/global to include xdg-user-dirs. 2. Create the xdg-user-dirs tunable using the default 'C' xdg-user-dirs values: $ cat profiles/apparmor.d/tunables/xdg-user-dirs # Define the common set of XDG user directories (usually defined in # /etc/xdg/user-dirs.defaults) @{XDG_DESKTOP_DIR}=Desktop @{XDG_DOWNLOAD_DIR}=Downloads @{XDG_TEMPLATES_DIR}=Templates @{XDG_PUBLICSHARE_DIR}=Public @{XDG_DOCUMENTS_DIR}=Documents @{XDG_MUSIC_DIR}=Music @{XDG_PICTURES_DIR}=Pictures @{XDG_VIDEOS_DIR}=Videos # Also, include files in tunables/xdg-user-dirs.d for site-specific adjustments # to the various XDG directories #include tunables/xdg-user-dirs.d 3. Add profiles/apparmor.d/tunables/xdg-user-dirs.d/site.local with commented out examples on how to use the directory. Acked-By: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Weil es sehr weit verbreitet ist, eingespielt und überall drauf. Die weite Verbreitung ist allenfalls geeignet, die kaputte Syntax auszugleichen, ein Erfordernis also, kein Pluspunkt. [ Ratti und Thorsten Haude in suse-linux zur Frage Warum procmail?] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] dnsmasq profile - NetworkManager integration
Hello, the dnsmasq profile needs some more permissions for NetworkManager integration. This is an updated version of the previous dnsmasq profile patch, again from develop7 [at] develop7.info I propose this patch for trunk and the 2.8 branch. === modified file 'profiles/apparmor.d/usr.sbin.dnsmasq' --- profiles/apparmor.d/usr.sbin.dnsmasq2014-01-17 19:58:21 +++ profiles/apparmor.d/usr.sbin.dnsmasq2014-02-17 21:31:53 @@ -29,6 +29,8 @@ /etc/dnsmasq.d/ r, /etc/dnsmasq.d/* r, /etc/ethers r, + /etc/NetworkManager/dnsmasq.d/ r, + /etc/NetworkManager/dnsmasq.d/* r, /usr/sbin/dnsmasq mr, @@ -56,6 +58,7 @@ /{,var/}run/nm-dns-dnsmasq.conf r, /{,var/}run/sendsigs.omit.d/*dnsmasq.pid w, /{,var/}run/NetworkManager/dnsmasq.conf r, + /{,var/}run/NetworkManager/dnsmasq.pid w, # Site-specific additions and overrides. See local/README for details. #include local/usr.sbin.dnsmasq Regards, Christian Boltz -- |#|Die drei wichtigsten Tugenden eines Programmierers: |#| Faulheit, Ungeduld und Selbstüberschätzung -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] new profile tools - review of merging branch
Hello, Am Sonntag, 16. Februar 2014 schrieb Kshitij Gupta: Sorry for the delayed response. I wasn't well and have been barely crawling in and out of bed. That's why laptops were invented - you can move them next to your bed ;-)) I hope you get well soon! I'm starting to catch up all the mails and patches. Please bear with me. Btw @cboltz, you have the commit rights I suppose. Always feel free to commit patches. (They can always be reverted ;) ) AFAIK, I don't have write access to the apparmor-profile-tools repo - but that doesn't matter too much. In the meantime, Steve merged your code into the official repo, with some changes and fixes applied [1]. This means the latest code is in the official repo (in other words: the apparmor-profile-tools repo is outdated ;-) and the development should continue in the apparmor repo. (I'd propose that you add a notice about this to the apparmor-profile- tools repo's README.md and then switch over to the apparmor repo.) This also means that everybody with commit access can commit (after following the usual review policy - send the patch to the mailinglist and wait for someone to send an ACK before you commit it). Speaking about commit access - it would be a good idea to give you commit access to the apparmor repo ;-) @Steve or John: can you do that, please? Regards, Christian Boltz [1] some of Steve's changes were quite big, like several whitespace changes to make PEP8 happy. This also means you'll get an unreadable diff ;-) bzr log -r 2383 -n0 | less will tell you what Steve did (commit messages), and something like bzr diff -r2368.1.21..2368.1.22 will show you what exactly he changed between the two specified revisions. -- ich habe mir gerade eine DVD mit meinen Daten geschrieben, scheint aber nicht lesbar zu sein. Wie genau hast du sie denn geschrieben? - Mit einem Stift: Lösung: Hm. In ca 30 cm vor die Nase halten und lesen. [ Anca Tibor Attila und Martin Ereth in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] new profile tools - review of merging branch
Hello, Am Freitag, 14. Februar 2014 schrieb Steve Beattie: On Sat, Feb 15, 2014 at 12:36:03AM +0100, Christian Boltz wrote: I also noticed my patches - new profile tools: preserve full initial comment - new profile tools - handling of (F)inish are not included yet. Can you please review and (hopefully) merge them? I'm hoping to, yes, but it requires a bit more deep understanding of the issues and code then I've got at the moment. Sorry. Hmm, I thought you read all the code you merge, didn't you? ;-) You can check both issues by testing (with and without the patches applied) - and my patch descriptions should explain what to test ;-) +++ utils/apparmor/translations.py [snip] + +__apparmor_gettext__ = None + +def init_translation(): +global __apparmor_gettext__ +if __apparmor_gettext__ is None: +t = gettext.translation(TRANSLATION_DOMAIN, fallback=True) +__apparmor_gettext__ = t.gettext +return __apparmor_gettext__ # what's the reason to make __apparmor_gettext__ global? In python, globals are a bit different than other languages. Globals are local to the module (i.e. have the scope of the file/module, translations.py in this case). Also, you can read their value freely, if there's no variable with the same name declared in a scope more local than the module (i.e. declared in the current function). But in order to modify the value and have that change be reflected outside of the scope of the function, it needs the global declaration; otherwise the change stays local to the function and is lost when the function ends. Thanks for explaining the details. Only needing the global declaration for writing sounds like an interesting[tm] design decision (even PHP is better here - it requires global for read and write access to global variables) - but I'm afraid we won't get that changed in python ;-) This still doesn't explain why you didn't make __apparmor_gettext__ local to the function. The only reason I can imagine is a bit of performance tuning if more than one script or module needs translations (the second one will get a cached instance of t.gettext). +++ /home/cb/apparmor/sbeattie-gsoc-pre-merge/pre-merger-cleanups/Testi ng/easyprof.conf 2014-02-11 18:48:23.035073000 +0100 @@ -1,5 +1,5 @@ # Location of system policygroups -POLICYGROUPS_DIR=./policygroups +POLICYGROUPS_DIR=/usr/share/apparmor/easyprof/policygroups # Location of system templates -TEMPLATES_DIR=./templates +TEMPLATES_DIR=/usr/share/apparmor/easyprof/templates # I'd understand this change for the system-wide config, but in /test/ ? I think you're a bit confused because of the wonky way merging has happened, but that's how it was on kshitij's branch: http://bazaar.launchpad.net/~kgupta8592/apparmor-profile-tools/trunk/v iew/head:/Testing/easyprof.conf Please note that I reverted that particular change when I merged in Kshitij's branch into my to-be-merged-into-trunk branch, which from the VCS point of view ended up being a NOP: Right, in the merged result it is ./ so everything is fine :-) === modified file 'utils/apparmor/aa.py' snipped - I'll let Kshitij answer the technical details ;-) Regards, Christian Boltz -- wie jeder weiß ist Debian auf ISDN die langsamste bekannte Methode Selbstmord zu begehen (Selbstmord durch Erosion) [http://blog.koehntopp.de/archives/113-Debian-ist-doch-schlecht..html] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] new profile tools - handling of (F)inish
Hello, [patch v2, see below] Am Montag, 27. Januar 2014 schrieb Christian Boltz: currently, selecting (F)inish in the new profile tools basically means aborting without saving anything. However, we already have Abo(r)t for that ;-) (F)inish should ask the user if he wants to save the changed profiles before exiting. The attached patch does this (except for two places, where I just added a TODO instead of real code ;-) - probably solvable by copypaste) Ignore the except ... part - that's fixed in the v2 patch ;-) I don't really like that I had to remove the central handling of (F)inish in ui.py and add it to several places in aa.py. OTOH calling confirm_and_finish() from ui.py a) isn't that easy and b) would mix program logic into ui.py - I'm not sure if this is a good idea. Hmm, maybe we should also move the handling for Abo(r)t to aa.py and make ui.py totally dump to be consistent, even if this means to add some duplicated lines? Or add a small function to aa.py as wrapper for UI_PromptUser that - always adds (F)inish and Abo(r)t - handles those two - and returns any other answer to the calling function ? This function could then be used for all user prompts, with the exception of the save profile / view diff prompt. Opinions? Here's an updated version of the patch that applies to current bzr trunk. === modified file 'utils/apparmor/aa.py' --- utils/apparmor/aa.py2014-02-24 18:20:11 + +++ utils/apparmor/aa.py2014-02-24 18:42:03 + @@ -498,6 +498,10 @@ ans = '' while 'CMD_USE_PROFILE' not in ans and 'CMD_CREATE_PROFILE' not in ans: +if ans == 'CMD_FINISHED': +save_profiles() +return; + ans, arg = aaui.UI_PromptUser(q) p = profile_hash[options[arg]] q['selected'] = options.index(options[arg]) @@ -990,6 +994,10 @@ ans = aaui.UI_PromptUser(q) +if ans == 'CMD_FINISHED': +save_profiles() +return; + transitions[context] = ans if ans == 'CMD_ADDHAT': @@ -1251,6 +1259,11 @@ q['functions'] += build_x_functions(default, options, exec_toggle) ans = '' continue + +if ans == 'CMD_FINISHED': +save_profiles() +return; + if ans == 'CMD_nx' or ans == 'CMD_nix': arg = exec_target ynans = 'n' @@ -1550,6 +1563,11 @@ done = False while not done: ans, selected = aaui.UI_PromptUser(q) + +if ans == 'CMD_FINISHED': +save_profiles() +return; + # Ignore the log entry if ans == 'CMD_IGNORE_ENTRY': done = True @@ -1794,6 +1812,10 @@ ans, selected = aaui.UI_PromptUser(q) +if ans == 'CMD_FINISHED': +save_profiles() +return; + if ans == 'CMD_IGNORE_ENTRY': done = True break @@ -1945,6 +1967,11 @@ done = False while not done: ans, selected = aaui.UI_PromptUser(q) + +if ans == 'CMD_FINISHED': +save_profiles() +return; + if ans == 'CMD_IGNORE_ENTRY': done = True break === modified file 'utils/apparmor/ui.py' --- utils/apparmor/ui.py2014-02-13 18:01:03 + +++ utils/apparmor/ui.py2014-02-24 18:32:40 + @@ -288,10 +288,6 @@ if cmd == 'CMD_ABORT': confirm_and_abort() cmd = 'XXXINVALIDXXX' -elif cmd == 'CMD_FINISHED': -if not params: -confirm_and_finish() -cmd = 'XXXINVALIDXXX' return (cmd, arg) def confirm_and_abort(): @@ -317,9 +313,6 @@ }) ypath, yarg = GetDataFromYast() -def confirm_and_finish(): -sys.stdout.write(_('FINISHING...\n')) -sys.exit(0) def Text_PromptUser(question): title = question['title'] Regards, Christian Boltz -- Wenn ich mir ein System installiere, welches dieses oder jenes oder welches für mich tut (gegen mich?), dann kann ich gleich Windows nehmen. Das fährt sich so gut, wie ein elektronischer Chauffeur das eben kann: Nämlich bis die Elektronik ein Dixi-Klo mit 'ner Garage verwechselt: Eckig und die Tür war offen. [Ratti in suse-linux
[apparmor] [patch] common.py: add debugging, py2 compat fix
Hello, this patch fixes two (unrelated) issues in common.py: - it adds some debug logging in valid_path() - it fixes a py2 incompability in DebugLogger.__init__ === modified file 'utils/apparmor/common.py' --- utils/apparmor/common.py2014-02-12 23:54:00 + +++ utils/apparmor/common.py2014-02-24 18:55:21 + @@ -112,6 +112,7 @@ return False if '' in path: # We double quote elsewhere +debug(%s (contains quote) % (m)) return False try: @@ -228,7 +229,7 @@ try: logging.basicConfig(filename=self.logfile, level=self.debug_level, format='%(asctime)s - %(name)s - %(message)s\n') -except OSError: +except IOError: # Unable to open the default logfile, so create a temporary logfile and tell use about it import tempfile templog = tempfile.NamedTemporaryFile('w', prefix='apparmor', suffix='.log', delete=False) Regards, Christian Boltz -- *pieps* Die Verkehrshinweise: Im Netzwerkkabel von Marc 100 MB Stau wegen einer Vollsperrung der Ausfahrt Festplatte. Bitte warten Sie auf dem Rasthof FTP-Server. *pieps* [Christian Boltz, gefunden von D. Haller] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] libapparmor README
Hello, this patch updates the bugtracker link in the libapparmor README. === modified file 'libraries/libapparmor/README' --- libraries/libapparmor/README2008-05-19 22:48:31 + +++ libraries/libapparmor/README2014-02-24 20:45:19 + @@ -1,1 +1,3 @@ -What little documentation exists is in src/aalogparse.h. Please file bugs using http://bugzilla.novell.com under the AppArmor product. +What little documentation exists is in src/aalogparse.h. + +Please file bugs using https://bugs.launchpad.net/apparmor/+filebug Regards, Christian Boltz -- By the way, it's a sign of how good the distribution is that we're arguing about the name and not dealing with problems in the essence of the thing. Even the overloaded servers are what an old boss of mine would have called a success problem. [Randall Schulz in opensuse] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] utils: fix cmd reference in apparmor/tools.py
Hello, Am Montag, 24. Februar 2014 schrieb Steve Beattie: This patch fixes up the parser command invocation via apparmor/common.py:cmd(), as it handles stdout/stderr redirection, and the redirection that was being attempted were being handed as arguments to the parser. Nice, good catch! (As an aside, we generally try to avoid invoking the shell when running external commands, to avoid shell quoting issues.) Signed-off-by: Steve Beattie st...@nxnw.org --- utils/apparmor/tools.py |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Index: b/utils/apparmor/tools.py === --- a/utils/apparmor/tools.py +++ b/utils/apparmor/tools.py @@ -16,7 +16,7 @@ import sys import apparmor.aa as apparmor import apparmor.ui as aaui -from apparmor.common import user_perm +from apparmor.common import user_perm, cmd # setup module translations from apparmor.translations import init_translation @@ -118,8 +118,7 @@ class aa_tools: # One simply does not walk in here! raise apparmor.AppArmorException('Unknown tool: %s' % self.name) -cmd_info = apparmor.cmd([apparmor.parser, filename, '-I%s' % apparmor.profile_dir, '-R 21', '1/dev/null']) - #cmd_info = apparmor.cmd(['cat', filename, '|', apparmor.parser, '-I%s'%apparmor.profile_dir, '-R 21', '1/dev/null']) +cmd_info = cmd([apparmor.parser, '-I%s' % apparmor.profile_dir, '-R', filename]) if cmd_info[0] != 0: raise apparmor.AppArmorException(cmd_info[1]) Acked-by: Christian Boltz appar...@cboltz.de Regards, Christian Boltz -- Früher habe ich auch gerne CDs gekauft [...] Aber ich habe gelernt, daß ich damit nicht Musiker fördere, sondern nur koksende Sony-Spacken, die mir zum Dank für meine Investition [...] ein Rootkit auf meine Karre installieren, gleich neben den Staatstrojaner. [http://blog.koehntopp.de/archives/3154-Nicht-Urheberrecht-ist-das-Kernthema.html] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] complain flag is enough, no symlink needed
Hello, let me compile 20 minutes of discussions into the addition of a # ;-) Change aa-complain / set_complain() to (only) add the complain flag. We don't need to additionally create a force-complain symlink. === modified file 'utils/apparmor/aa.py' --- utils/apparmor/aa.py2014-02-24 19:56:28 + +++ utils/apparmor/aa.py2014-02-24 23:11:32 + @@ -257,7 +257,8 @@ def set_complain(filename, program): Sets the profile to complain mode aaui.UI_Info(_('Setting %s to complain mode.') % program) -create_symlink('force-complain', filename) +# a force-complain symlink is more packaging-friendly, but breaks caching +# create_symlink('force-complain', filename) change_profile_flags(filename, program, 'complain', True) def set_enforce(filename, program): Regards, Christian Boltz -- Ich habe ein update für 2.0.1 released, welches die Änderung im Makefile auf die alte Version zurückportiert (Was für ein großes Wort für zwei Anführungsstriche). [Ratti in fontlinge-devel - nach Änderung von zwei »'« zu »« in einem Makefile] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [Bug 1180230] Re: glob in aa-genprof repeats same option
This patch was commited to 2.8 branch and trunk, and later changed to use grep instead of ~~~. AppArmor 2.8.3 contains the fix. ** Changed in: apparmor Status: Fix Committed = Fix Released -- You received this bug notification because you are a member of AppArmor Developers, which is a bug assignee. https://bugs.launchpad.net/bugs/1180230 Title: glob in aa-genprof repeats same option Status in AppArmor Linux application security framework: Fix Released Bug description: When using glob, the glob does not check if the entries mentioned previously is repeated or not. Using a simple check to match against the previous entry will solve this and prevent such long pointless lists. | [(A)llow] / (D)eny / (G)lob / Glob w/(E)xt / (N)ew / Abo(r)t / (F)inish / (O)pts | | Profile: /usr/sbin/mtr | Path: /etc/gai.conf | Mode: r | Severity: unknown | | | 1 - #include abstractions/apache2-common | 2 - #include abstractions/nameservice | 3 - /etc/gai.conf | 4 - /etc/* | 5 - /** | 6 - /** | 7 - /** | 8 - /** | 9 - /** | 10 - /** | 11 - /** | 12 - /** | 13 - /** | 14 - /** | [15 - /**] | | [(A)llow] / (D)eny / (G)lob / Glob w/(E)xt / (N)ew / Abo(r)t / (F)inish / (O)pts I'm assuming this is a problem with the AppArmor library based in Perl. I'm trying to work a way to fix it. (BTW, my first bug report ever.) To manage notifications about this bug go to: https://bugs.launchpad.net/apparmor/+bug/1180230/+subscriptions -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor