Re: [apparmor] [patch] Add some simple_tests (dbus and bare file rules)
Hello, Am Donnerstag, 7. Januar 2016 schrieb Steve Beattie: > On Thu, Jan 07, 2016 at 09:54:40PM +0100, Christian Boltz wrote: > > [ more-simple_tests.diff ] > > > > === added file 'parser/tst/simple_tests/dbus/ok_bind_2.sd' > > --- parser/tst/simple_tests/dbus/ok_bind_2.sd 1970-01-01 00:00:00 > > + +++ parser/tst/simple_tests/dbus/ok_bind_2.sd 2015-10-27 > > 22:55:01 + @@ -0,0 +1,7 @@ > > +# > > +#=DESCRIPTION simple dbus implicit bind acceptance test with deny > > keyword +#=EXRESULT PASS > > + > > +profile a_profile { > > + deny dbus name=(SomeService), > > +} > > Hrm, I'm surprised the autogenerated dbus tests don't cover this, > since they exercise deny pretty excessively. But grepping recursively > for 'deny dbus name' doesn't find anything. IIRC I noticed this by missing code coverage, so... ;-) > > === added file 'parser/tst/simple_tests/file/ok_bare_1.sd' > > --- parser/tst/simple_tests/file/ok_bare_1.sd 1970-01-01 00:00:00 > > + +++ parser/tst/simple_tests/file/ok_bare_1.sd 2015-10-27 > > 22:50:19 + @@ -0,0 +1,7 @@ > > +# > > +#=Description bare file rule > > +#=EXRESULT PASS > > +# > > +/usr/bin/foo { > > + file, > > +} > > Covered by parser/tst/simple_tests/file/file/ok_2.sd; note that the > file/file/ subdirectory covers use of the file keyword with file > pathnames. I'm okay with okay with renaming/replacing that one with > ok_bare_1.sd, but keeping it in the file/file/ subdirectory. I don't really care about the filename, so I'll just remove ok_bare_1.sd from the patch (and keep ok_2.sd unchanged). This also means I'll rename ok_bare_2.sd to ok_bare_1.sd to avoid someone wonders why ok_bare_1.sd doesn't exist ;-) > > === added file 'parser/tst/simple_tests/file/ok_bare_2.sd' > > --- parser/tst/simple_tests/file/ok_bare_2.sd 1970-01-01 00:00:00 > > + +++ parser/tst/simple_tests/file/ok_bare_2.sd 2015-10-27 > > 22:50:36 + @@ -0,0 +1,7 @@ > > +# > > +#=Description bare file rule > > +#=EXRESULT PASS > > +# > > +/usr/bin/foo { > > + deny file, > > +} > > Yep, that's not covered by existing tests. > > Acked-by: Steve Beattie <st...@nxnw.org>, as long as the duplication > between file/ok_bare_1.sd and file/file/ok_2.sd is resolved. Regards, Christian Boltz -- ...als ich letztens an der Elbe saß, mein Astra-was-sonst nuckelte und die Aufschriften auf den Containerschiffen studierte, beschloss ich, daß die Einheit "Kilopunkt" (kPt) für Fonts durchaus praxisnah sei. Die Überprüfung meiner Erkenntnisse ("Dasch verdammte Wort is minstens 5 Medder hoch, wennichsogar...") muß ich leider bis zum Erwerb eines Schlauchbootes zurückstellen. Wissenschaft fordert Opfer. [Ratti in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] split off _aare_or_all()
Hello, Am Sonntag, 20. Dezember 2015 schrieb Christian Boltz: > we need to check a rule part if it is *Rule.ALL or a string at various > places. Therefore split off the checks in PtraceRule's and > SignalRule's __init__() to the new _aare_or_alll() function in > BaseRule. > > This also makes the *Rule __init__() much more readable because we now > have one line to set self.foo and self.all_foo instead of 10 lines of > nested if conditions. I missed that we need the is_path flag, so here's v2: [ 39-split-off-aare_or_all.diff ] === modified file ./utils/apparmor/rule/__init__.py --- utils/apparmor/rule/__init__.py 2015-12-20 19:28:07.516487665 +0100 +++ utils/apparmor/rule/__init__.py 2015-12-20 19:30:40.931501227 +0100 @@ -13,6 +13,7 @@ # # -- +from apparmor.aare import AARE from apparmor.common import AppArmorBug, type_is_str # setup module translations @@ -50,6 +51,30 @@ # Set only in the parse() class method self.raw_rule = None +def _aare_or_all(self, rulepart, partname, is_path, log_event): +'''checks rulepart and returns + - (AARE, False) if rulepart is a (non-empty) string + - (None, True) if rulepart is all_obj (typically *Rule.ALL) + - raises AppArmorBug if rulepart is an empty string or has a wrong type + + Parameters: + - rulepart: the rule part to check (string or *Rule.ALL object) + - partname: the name of the rulepart (for example 'peer', used for exception messages) + - is_path (passed through to AARE) + - log_event (passed through to AARE) + ''' + +if rulepart == self.ALL: +return None, True +elif type_is_str(rulepart): +if len(rulepart.strip()) == 0: +raise AppArmorBug('Passed empty %(partname)s to %(classname)s: %(rulepart)s' % +{'partname': partname, 'classname': self.__class__.__name__, 'rulepart': str(rulepart)}) +return AARE(rulepart, is_path=is_path, log_event=log_event), False +else: +raise AppArmorBug('Passed unknown %(partname)s to %(classname)s: %(rulepart)s' +% {'partname': partname, 'classname': self.__class__.__name__, 'rulepart': str(rulepart)}) + def __repr__(self): classname = self.__class__.__name__ try: === modified file ./utils/apparmor/rule/ptrace.py --- utils/apparmor/rule/ptrace.py 2015-12-20 19:28:07.516487665 +0100 +++ utils/apparmor/rule/ptrace.py 2015-12-20 19:27:22.024780366 +0100 @@ -14,9 +14,8 @@ import re -from apparmor.aare import AARE from apparmor.regex import RE_PROFILE_PTRACE, RE_PROFILE_NAME -from apparmor.common import AppArmorBug, AppArmorException, type_is_str +from apparmor.common import AppArmorBug, AppArmorException from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, parse_modifiers, quote_if_needed # setup module translations @@ -63,18 +62,7 @@ if unknown_items: raise AppArmorException(_('Passed unknown access keyword to PtraceRule: %s') % ' '.join(unknown_items)) -# XXX same as in SignalRule - move to _init_peer() function! -self.peer = None -self.all_peers = False -if peer == PtraceRule.ALL: -self.all_peers = True -elif type_is_str(peer): -if len(peer.strip()) == 0: -raise AppArmorBug('Passed empty peer to PtraceRule: %s' % str(peer)) -self.peer = AARE(peer, False, log_event=log_event) -else: -raise AppArmorBug('Passed unknown object to PtraceRule: %s' % str(peer)) - +self.peer, self.all_peers = self._aare_or_all(peer, 'peer', is_path=False, log_event=log_event) @classmethod def _match(cls, raw_rule): === modified file ./utils/apparmor/rule/signal.py --- utils/apparmor/rule/signal.py 2015-12-20 19:28:07.516487665 +0100 +++ utils/apparmor/rule/signal.py 2015-12-20 19:27:09.752859340 +0100 @@ -14,9 +14,8 @@ import re -from apparmor.aare import AARE from apparmor.regex import RE_PROFILE_SIGNAL, RE_PROFILE_NAME -from apparmor.common import AppArmorBug, AppArmorException, type_is_str +from apparmor.common import AppArmorBug, AppArmorException from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, parse_modifiers, quote_if_needed # setup module translations @@ -92,17 +91,7 @@ else: raise AppArmorException(_('Passed unknown signal keyword to SignalRule: %s') % item) -self.peer = None -self.all_peers = False -if peer == SignalRule.ALL: -self.all_peers = True -elif type_is_str(peer): -if len(peer.strip()) == 0: -raise AppArmorBug('Passed empty peer to SignalRule: %s' % str(peer)) -self.peer = AAR
[apparmor] [patch] split off _is_covered_*() helper functions
specified in other signal rule') +if not self._is_covered_plain(self.signal, self.all_signals, other_rule.signal, other_rule.all_signals, 'signal'): +return False -if not self.all_access: -if other_rule.all_access: -return False -if other_rule.access != self.access: -return False - -if not self.all_signals: -if other_rule.all_signals: -return False -if other_rule.signal != self.signal: -return False - -if not self.all_peers: -if other_rule.all_peers: -return False -if not self.peer.match(other_rule.peer.regex): -return False +if not self._is_covered_aare(self.peer, self.all_peers, other_rule.peer, other_rule.all_peers, 'peer'): +return False # still here? -> then it is covered return True Regards, Christian Boltz -- I have the ideal solution for you to speed up the writing of the manuals: http://www.lipsum.com/ - I am sure almost nobody will notice the difference. ;-) [houghi in opensuse-wiki] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Use list check in PtraceRule and SignalRule is_covered_localvars()
): def test_borked_obj_is_covered_1(self): obj = PtraceRule.parse('ptrace read peer=/foo,') === modified file ./utils/test/test-signal.py --- utils/test/test-signal.py 2015-12-12 13:34:40.549997194 +0100 +++ utils/test/test-signal.py 2015-12-20 23:47:40.041531733 +0100 @@ -433,6 +433,41 @@ ('deny signal send,' , [ False , False , False , False ]), ] +class SignalCoveredTest_09(SignalCoveredTest): +rule = 'signal (send, receive) set=(int, quit),' + +tests = [ +# rule equal strict equal covered covered exact +('signal,', [ False , False , False , False ]), +('signal send,' , [ False , False , False , False ]), +('signal send set=int,' , [ False , False , True , True ]), +('signal receive set=quit,' , [ False , False , True , True ]), +('signal (receive,send) set=int,' , [ False , False , True , True ]), +('signal (receive,send) set=(int quit),',[True, False , True , True ]), +('signal send set=(quit int),', [ False , False , True , True ]), +('signal send peer=/foo/bar,' , [ False , False , False , False ]), +('signal send peer=/foo/*,' , [ False , False , False , False ]), +('signal send peer=/**,' , [ False , False , False , False ]), +('signal send peer=/what/*,' , [ False , False , False , False ]), +('signal peer=/foo/bar,' , [ False , False , False , False ]), +('signal send, # comment' , [ False , False , False , False ]), +('allow signal send,' , [ False , False , False , False ]), +('allow signal send peer=/foo/bar,' , [ False , False , False , False ]), +('signalsend,', [ False , False , False , False ]), +('signalsend peer=/foo/bar,' , [ False , False , False , False ]), +('signalsend peer=/what/ever,', [ False , False , False , False ]), +('signal send set=quit,' , [ False , False , True , True ]), +('signal send set=int peer=/foo/bar,' , [ False , False , True , True ]), +('audit signal send peer=/foo/bar,' , [ False , False , False , False ]), +('audit signal,' , [ False , False , False , False ]), +('signal receive,', [ False , False , False , False ]), +('signal set=int,', [ False , False , False , False ]), +('audit deny signal send,', [ False , False , False , False ]), +('deny signal send,' , [ False , False , False , False ]), +] + + + class SignalCoveredTest_Invalid(AATest): def test_borked_obj_is_covered_1(self): obj = SignalRule.parse('signal send peer=/foo,') Regards, Christian Boltz -- ist eine recht interessante rechnung: 3,5kg linux + bücher für €79,90 180g windows xp home ohne bücher €229,- kennt jemand den feinunzenpreis von gold? er müßte kanpp unter dem von windows liegen [Wilhelm Feichter in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Improve __repr__() for *Ruleset
Hello, if a *Ruleset is empty, let __repr__() print/return instead of I propose this patch for trunk and 2.10. [ 42-improve-repr-empty-ruleset.diff ] === modified file ./utils/apparmor/rule/__init__.py --- utils/apparmor/rule/__init__.py 2015-12-21 00:42:28.521222690 +0100 +++ utils/apparmor/rule/__init__.py 2015-12-21 22:57:03.421574167 +0100 @@ -291,7 +291,10 @@ def __repr__(self): classname = self.__class__.__name__ -return '<%s>\n' % classname + '\n'.join(self.get_raw(1)) + '' % classname +if self.rules: +return '<%s>\n' % classname + '\n'.join(self.get_raw(1)) + '' % classname +else: +return '<%s (empty) />' % classname def add(self, rule): '''add a rule object''' Regards, Christian Boltz -- > Kann mir jemand sagen, wie unter der neuen SuSE der inetd gestartet > wird? [...] Ich such mir hier einen Affen... hmm, hier? Wo ist hier? Und wenn Du einen findest, was dann? Was willst Du mit einem Affen? Welchen denn? Wieso hier? Wenn Du einen findest, kannst ihn ja mal fragen... Vielleicht hilfts Dir, wenn Du mal in /etc/init.d /etc/syconfig /etc/rc.config.d usw. suchst. Achso ja, Affen hab ich da noch keinen gesehen;) [> Andreas Meyer und Bernd Obermayr in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Change log_dict to use profile_storage() and simplify log translation
allow_mode = set() allow_audit = set() @@ -2490,6 +2463,8 @@ for profile in prelog[aamode].keys(): for hat in prelog[aamode][profile].keys(): +log_dict[aamode][profile][hat] = profile_storage(profile, hat, 'collapse_log()') + for path in prelog[aamode][profile][hat]['path'].keys(): mode = prelog[aamode][profile][hat]['path'][path] @@ -2506,35 +2481,37 @@ combinedmode |= match_prof_incs_to_path(aa[profile][hat], 'allow', path)[0] if not combinedmode or not mode_contains(combinedmode, mode): -if log_dict[aamode][profile][hat]['path'].get(path, False): -mode |= log_dict[aamode][profile][hat]['path'][path] +if log_dict[aamode][profile][hat]['allow']['path'].get(path, False): +mode |= log_dict[aamode][profile][hat]['allow']['path'][path] -log_dict[aamode][profile][hat]['path'][path] = mode +log_dict[aamode][profile][hat]['allow']['path'][path] = mode for cap in prelog[aamode][profile][hat]['capability'].keys(): -# If capability not already in profile -# XXX remove first check when we have proper profile initialisation -if aa[profile][hat].get('capability', False) and not aa[profile][hat]['capability'].is_covered(CapabilityRule(cap, log_event=True)): -log_dict[aamode][profile][hat]['capability'][cap] = True +cap_event = CapabilityRule(cap, log_event=True) +if not is_known_rule(aa[profile][hat], 'capability', cap_event): + log_dict[aamode][profile][hat]['capability'].add(cap_event) nd = prelog[aamode][profile][hat]['netdomain'] for family in nd.keys(): for sock_type in nd[family].keys(): -if not is_known_rule(aa[profile][hat], 'network', NetworkRule(family, sock_type, log_event=True)): - log_dict[aamode][profile][hat]['netdomain'][family][sock_type] = True +net_event = NetworkRule(family, sock_type, log_event=True) +if not is_known_rule(aa[profile][hat], 'network', net_event): + log_dict[aamode][profile][hat]['network'].add(net_event) ptrace = prelog[aamode][profile][hat]['ptrace'] for peer in ptrace.keys(): for access in ptrace[peer].keys(): -if not is_known_rule(aa[profile][hat], 'ptrace', PtraceRule(access, peer, log_event=True)): - log_dict[aamode][profile][hat]['ptrace'][peer][access] = True +ptrace_event = PtraceRule(access, peer, log_event=True) +if not is_known_rule(aa[profile][hat], 'ptrace', ptrace_event): + log_dict[aamode][profile][hat]['ptrace'].add(ptrace_event) sig = prelog[aamode][profile][hat]['signal'] for peer in sig.keys(): for access in sig[peer].keys(): for signal in sig[peer][access].keys(): -if not is_known_rule(aa[profile][hat], 'signal', SignalRule(access, signal, peer, log_event=True)): - log_dict[aamode][profile][hat]['signal'][peer][access][signal] = True +signal_event = SignalRule(access, signal, peer, log_event=True) +if not is_known_rule(aa[profile][hat], 'signal', signal_event): + log_dict[aamode][profile][hat]['signal'].add(signal_event) PROFILE_MODE_RE = re.compile('^(r|w|l|m|k|a|ix|ux|px|pux|cx|pix|cix|cux|Ux|Px|PUx|Cx|Pix|Cix|CUx)+$') Regards, Christian Boltz -- Nicht nur Schoenheit, sondern auch Schweinkram liegt ausschliesslich im Auge des Betrachters. [Kristian Koehntopp zur Aussage "frauen sind gut zu voegeln" in http://groups.google.com/groups?selm=3ejajb$e...@picard.toppoint.de] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Fix aa-mergeprof crash with files containing multiple profiles
Hello, if a profile file contains multiple profiles, aa-mergeprof crashes on saving in write_profile() because the second profile in the file is not listed in 'changed'. This patch first checks if 'changed' contains the profile before pop()ing it. Reproducer: copy utils/test/cleanprof_test.in to your profile directory and run aa-mergeprof utils/test/cleanprof_test.out. Then just press 's' to save the profile. I can reproduce this with trunk, 2.10 and 2.9 and therefore propose this patch for all these branches. [ 47-fix-multi-profile-mergeprof-crash.diff ] --- utils/apparmor/aa.py2015-12-26 16:47:30.614839586 +0100 +++ utils/apparmor/aa.py2015-12-26 17:27:36.376228122 +0100 @@ -4039,7 +4039,11 @@ os.rename(newprof.name, prof_filename) -changed.pop(profile) +if profile in changed: +changed.pop(profile) +else: +debug_logger.error("%s written, but not listed in 'changed' list" % profile) + original_aa[profile] = deepcopy(aa[profile]) def matchliteral(aa_regexp, literal): Regards, Christian Boltz -- [Glaskugel?] Ich habe früher Aufsicht im Rechnerpool an der Uni gemacht. Irgendwie hat es die User beeindruckt, wenn ich Ihnen (ohne den Monitor einsehen zu können und ohne dass die User etwas gesagt hätten) erklärt habe, dass Word abstürzt, wenn man erst die Diskette entfernt und dann Word schließt. Das Laufwerksknurpsgeräusch und der Gesichtsausdruck der User war eindeutig genug... [Antje M. Bendrich in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Better error message on unknown profile lines
Hello, when hitting an unknown line while parsing a profile, it's a good idea to include that line in the error message ;-) I propose this patch for trunk and 2.10 (2.9 would print a literal \n because it doesn't have apparmor.fail, so if we want that patch in 2.9, I'll have to s/\n //') [ 49-parse-unknown-line-better-error-msg.diff ] --- utils/apparmor/aa.py2015-12-26 17:35:16.273569178 +0100 +++ utils/apparmor/aa.py2015-12-26 21:19:36.910261844 +0100 @@ -3056,7 +3056,7 @@ else: lastline = line else: -raise AppArmorException(_('Syntax Error: Unknown line found in file: %(file)s line: %(line)s') % { 'file': file, 'line': lineno + 1 }) +raise AppArmorException(_('Syntax Error: Unknown line found in file %(file)s line %(lineno)s:\n%(line)s') % { 'file': file, 'lineno': lineno + 1, 'line': line }) # Below is not required I'd say if not do_include: Regards, Christian Boltz -- Lesson learned: Web service APIs can drive you crazy especially if you mix it with asynchronous network connections [David Williams in opensuse-project] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Add more ruletypes to the cleanprof test profiles
Hello, to ensure aa-cleanprof works as expected (and writing the rules works as expected), add some rules for every rule class to the cleanprof.in and cleanprof.out test profiles. [ 48-add-more-ruletypes-to-cleanprof-test.diff ] === modified file ./utils/test/cleanprof_test.in --- utils/test/cleanprof_test.in2015-12-12 13:34:40.549997194 +0100 +++ utils/test/cleanprof_test.in2015-12-26 17:11:27.034328693 +0100 @@ -4,12 +4,32 @@ /usr/bin/a/simple/cleanprof/test/profile { # Just for the heck of it, this comment wont see the day of light #include + +capability sys_admin, +audit capability, + +change_profile -> /bin/foo, +change_profile, + +network inet stream, +network stream, + #Below rule comes from abstractions/base allow /usr/share/X11/locale/** r, allow /home/*/** r, +ptrace tracedby peer=/bin/strace, +ptrace tracedby, unix (receive) type=dgram, +set rlimit nofile <= 256, +set rlimit nofile <= 64, + +signal set=(hup int quit ill trap abrt) + set=(bus,fpe,,,kill,usr1) + set=segv set=usr2 set=pipe set=alrm set=term set=stkflt set=chld, +signal set=(hup int quit), + ^foo { /etc/fstab r, capability dac_override, === modified file ./utils/test/cleanprof_test.out --- utils/test/cleanprof_test.out 2015-12-12 13:34:40.549997194 +0100 +++ utils/test/cleanprof_test.out 2015-12-26 17:14:06.105337830 +0100 @@ -6,11 +6,23 @@ /usr/bin/a/simple/cleanprof/test/profile { #include + set rlimit nofile <= 256, + + audit capability, + + network stream, + + signal set=(abrt alrm bus chld fpe hup ill int kill pipe quit segv stkflt term trap usr1 usr2), + + ptrace tracedby, + unix (receive) type=dgram, /home/*/** r, /home/foo/** w, + change_profile, + ^foo { capability dac_override, Regards, Christian Boltz -- > > of course, now everybody will claim how bad it is to fix bugs which > > people rely on; > No, I wont claim that, in fact I would argue against keeping any bug > on which people relies on (known as "backwards compatibility") I should have excluded you from the list of everybody... [> Cristian Rodríguez and (>>) Dominique Leuenberger in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Dear Santa
Dear Santa, I know I'm late, but - Can you please add the python developer(s) responsible for http://bugs.python.org/issue10076 to your black book? Having to add a workaround for a 5 years old bug, which even has a patch attached to the bugreport, is not nice :-/ [patch] Implement __deepcopy__() for aare Thanks to http://bugs.python.org/issue10076, we need to implement this ourself :-/ Also add some tests to ensure __deepcopy__() works as expected. I found this bug while testing the next patch series, which crashed aa-cleanprof with TypeError: cannot deepcopy this pattern object [ 50-aare-deepcopy.diff ] --- utils/apparmor/aare.py 2015-12-26 21:37:04.534751923 +0100 +++ utils/apparmor/aare.py 2015-12-26 21:34:48.575725530 +0100 @@ -46,6 +46,13 @@ '''returns a "printable" representation of AARE''' return "AARE('%s')" % self.regex +def __deepcopy__(self, memo): +# thanks to http://bugs.python.org/issue10076, we need to implement this ourself +if self.orig_regex: +return AARE(self.orig_regex, is_path=False, log_event=True) +else: +return AARE(self.regex, is_path=False) + def match(self, expression): '''check if the given expression (string or AARE) matches the regex''' --- utils/test/test-aare.py 2015-12-26 21:37:04.534751923 +0100 +++ utils/test/test-aare.py 2015-12-26 21:37:14.302681989 +0100 @@ -13,6 +13,7 @@ import unittest from common_test import AATest, setup_all_loops +from copy import deepcopy import re from apparmor.common import convert_regexp, AppArmorBug, AppArmorException from apparmor.aare import AARE, convert_expression_to_aare @@ -223,6 +224,25 @@ obj = AARE('/foo', True) self.assertEqual(str(obj), "AARE('/foo')") +class TestAAREDeepcopy(AATest): +tests = [ +# regex is path?log event expected (dummy value) +(AARE('/foo', False) , True), +(AARE('/foo', False, True) , True), +(AARE('/foo', True) , True), +(AARE('/foo', True, True) , True), +] + +def _run_test(self, params, expected): +dup = deepcopy(params) + +self.assertTrue(params.match('/foo')) +self.assertTrue(dup.match('/foo')) + +self.assertEqual(params.regex, dup.regex) +self.assertEqual(params.orig_regex, dup.orig_regex) +self.assertEqual(params.orig_regex, dup.orig_regex) + setup_all_loops(__name__) if __name__ == '__main__': Regards, Christian Boltz -- Wir waren vor einiger Zeit schonmal "soweit fertig". Dann kam Gerald, fand 1000 Sachen Scheisse, hat 500 Sachen nicht begriffen und 250 falsch gemacht. :-)) [Ratti in fontlinge-devel] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Fix wrong usage of write_prof_data in serialize_profile_from_old_profile()
Hello, write_prof_data[hat] is correct (it only contains one profile, see also bug 1528139), write_prof_data[profile][hat] is not and returns an empty (sub)hasher. This affects RE_PROFILE_START and RE_PROFILE_BARE_FILE_ENTRY. I propose this patch for trunk, 2.10 and 2.9. [ 46-serialize_profile_from_old_profile-fix-wrong-access-to-write_prof_data.diff ] === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py2015-12-06 19:36:00.818745321 +0100 +++ utils/apparmor/aa.py2015-12-08 18:59:09.625261162 +0100 @@ -3718,7 +3718,7 @@ if RE_PROFILE_START.search(line): (profile, hat, attachment, flags, in_contained_hat, correct) = serialize_parse_profile_start( -line, prof_filename, None, profile, hat, write_prof_data[profile][hat]['profile'], write_prof_data[profile][hat]['external'], correct) +line, prof_filename, None, profile, hat, write_prof_data[hat]['profile'], write_prof_data[hat]['external'], correct) if not write_prof_data[hat]['name'] == profile: correct = False @@ -3954,7 +3954,7 @@ if matches[0]: audit = mode -path_rule = write_prof_data[profile][hat][allow]['path'][ALL] +path_rule = write_prof_data[hat][allow]['path'][ALL] if path_rule.get('mode', set()) & mode and \ (not audit or path_rule.get('audit', set()) & audit) and \ path_rule.get('file_prefix', set()): Regards, Christian Boltz -- programmers' biggest strength is that they're lazy bastards. [Claudio Freire in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Split off logprof_value_or_all()
apparmor.common import AppArmorBug, AppArmorException -from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, parse_modifiers, quote_if_needed +from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, logprof_value_or_all, parse_modifiers, quote_if_needed # setup module translations from apparmor.translations import init_translation @@ -160,15 +160,8 @@ return True def logprof_header_localvars(self): -if self.all_access: -access = _('ALL') -else: -access = ' '.join(sorted(self.access)) - -if self.all_peers: -peer = _('ALL') -else: -peer = self.peer.regex +access = logprof_value_or_all(self.access,self.all_access) +peer = logprof_value_or_all(self.peer, self.all_peers) return [ _('Access mode'), access, === modified file ./utils/apparmor/rule/signal.py --- utils/apparmor/rule/signal.py 2015-12-23 21:50:28.363843471 +0100 +++ utils/apparmor/rule/signal.py 2015-12-26 22:33:12.363974781 +0100 @@ -16,7 +16,7 @@ from apparmor.regex import RE_PROFILE_SIGNAL, RE_PROFILE_NAME from apparmor.common import AppArmorBug, AppArmorException -from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, parse_modifiers, quote_if_needed +from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, logprof_value_or_all, parse_modifiers, quote_if_needed # setup module translations from apparmor.translations import init_translation @@ -214,20 +214,9 @@ return True def logprof_header_localvars(self): -if self.all_access: -access = _('ALL') -else: -access = ' '.join(sorted(self.access)) - -if self.all_signals: -signal = _('ALL') -else: -signal = ' '.join(sorted(self.signal)) - -if self.all_peers: -peer = _('ALL') -else: -peer = self.peer.regex +access = logprof_value_or_all(self.access, self.all_access) +signal = logprof_value_or_all(self.signal, self.all_signals) +peer= logprof_value_or_all(self.peer, self.all_peers) return [ _('Access mode'), access, Regards, Christian Boltz -- Java Call Stack Tief in der Scheisse [Patrick Schaaf, FdI#545] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [7/7] Add support for ptrace log events to aa-logprof
Hello, Am Samstag, 26. Dezember 2015 schrieb John Johansen: > On 12/08/2015 11:40 AM, Christian Boltz wrote: > > $subject. > > > > In detail, this means: > > - handle ptrace events in logparser.py > > - "translate" those events in aa.py - from log (logparser.py > > readlog())> > > to prelog (handle_children()) to log_dict (collapse_log()) to > > log_obj (ask_the_questions()) > > (yes, really! :-/ - needless to say that this is ugly...) [1] ... > With [1] noted, my eyes, my poor eyes! ;-) I'm quite sure you'll love 45-change-log_dict-to-profile_storage.diff which simplifies the log "translation" by removing one step. > Acked-by: John Johansen <john.johan...@canonical.com> Thanks for reviewing the ptrace series and the "Dear Santa" patch! (I knew that subject would catch some attention ;-) > > [1] I already said that when adding signal support, and making it > > less ugly is on my TODO list ;-) Regards, Christian Boltz -- > Das ist uebrigens genau der "Trick" den reiserfs verwendet! ...ich dachte immer, der "Trick" von reiserfs sei: if ($stromausfall) rm /* -r [> David Haller und Ratti in fontlinge-devel] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] AppArmor profile: requested_mask and denied_mask = "c", "x".
Hello, Am Freitag, 18. Dezember 2015 schrieb daniel curtis: > I would like to ask about AppArmor profile and a system log files > such as, for example, /var/log/syslog etc. Let say, that I wrote a > profile for an application, which 'audit' entries in log files > contains something like this (of course, I omitted the whole > 'audit'): > > * requested_mask="c" denied_mask="c" > > I have to say, that it is "DENIED" action for 'mkdir' operation in > /home/user/.app/ directory. But that is not the point. So, what "c" > exactly means? If I would like to add a rule to the AppArmor > profile what should I use? I mean: 'r', 'w', 'x', or maybe 'l', 'k', > 'm'? Or maybe something completely different, like: c means "create file/directory". For a directory, you'll need a rule like /the/directory/ w, For files, a (append) permissions might be enough, but depending on how the application opens the file, you might need the more permissive w. > * /usr/bin/xyz Cx -> sanitized_helper, > > Generally: what does "c" and "x" exactly means? (In AppArmor > audit messages). In conclusion: what rules should I use in an > application profile, if in log files there is, for example, 'audit' > messages like this one: > > 1/ operation="mkdir", requested_mask="c", denied_mask="c" See above. > 2/ operation="exec", requested_mask=x", denied_mask="x" That means executing another binary. Depending on what gets executed, you can choose ix (inherit = use the same profile), Cx (use a child profile), Px (use a standalone profile) or Ux (unconfined = execute without AppArmor restrictions). Hint: Avoid Px for things like /bin/bash ;-) > So, how a correct entries, in the profile, should look like? If in doubt, use aa-logprof - it will give you a matching proposal. Also have a look at man apparmor.d which explains all rule types and permissions. Finally, I can recommend my "AppArmor Crash Course". You can find (slightly outdated) slides at blog.cboltz.de (search for AppArmor). If slides aren't enough, check the DebConf15 video archives - I gave that talk there. Regards, Christian Boltz -- [CVS] Es gibt auch ein grafisches Frontend (nein, nicht das kranke Cervisia, beim Programieren war da wohl zuviel Cervessa im Spiel) [Gerald Goebel in fontlinge-devel] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] GPG signature
Hello, starting today, I'll sign all mails to the AppArmor mailinglist with my GPG key to ensure the mail integrity, which is especially important when sending or acking patches. My GPG key is: pub rsa4096/63C82F1C 2005-10-06 [expires: 2016-12-31] Key fingerprint = 70CA A060 DE04 2AAE B1B1 5196 C6A6 82EA 63C8 2F1C uid [ultimate] Christian Boltz (www.cboltz.de) <g...@cboltz.de> The key is available on the keyserver network. If you prefer to receive it as mail attachment, just ask (off-list). Regards, Christian Boltz -- >gehe zu 'http://www.linuxiso.org/' Zu Fuß? [> Michael Meyer und Thorsten Haude in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] split off _aare_or_all()
_match(cls, raw_rule): Regards, Christian Boltz -- Non-understandable error messages are trademark of someone else, so SUSE is not allowed to submit them. ;-)) [Eberhard Moenkeberg in https://bugzilla.novell.com/show_bug.cgi?id=209354] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] [3/9] Add DbusRule and DbusRuleset classes
Hello, $subject. Those classes will be used to parse and handle dbus rules. They understand the syntax of dbus rules. Note that get_clean() doesn't output superfluos things, so dbus ( send ), will become dbus send, Note: r, read, w, write, rw are not documented in apparmor.d.pod. [ 54-add-DbusRule.diff ] --- utils/apparmor/rule/dbus.py 2015-12-27 00:13:37.990086206 +0100 +++ utils/apparmor/rule/dbus.py 2015-12-27 00:13:07.714299658 +0100 @@ -0,0 +1,327 @@ +# -- +#Copyright (C) 2015 Christian Boltz <appar...@cboltz.de> +# +#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 as published by the Free Software Foundation. +# +#This program is distributed in the hope that it will be useful, +#but WITHOUT ANY WARRANTY; without even the implied warranty of +#MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +#GNU General Public License for more details. +# +# -- + +import re + +from apparmor.regex import RE_PROFILE_DBUS, RE_PROFILE_NAME, strip_parenthesis, strip_quotes +from apparmor.common import AppArmorBug, AppArmorException +from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, logprof_value_or_all, parse_modifiers, quote_if_needed + +# setup module translations +from apparmor.translations import init_translation +_ = init_translation() + + +message_keywords = ['send', 'receive', 'r', 'read', 'w', 'write', 'rw'] +access_keywords = [ 'bind', 'eavesdrop' ] + message_keywords + +# XXX joint_access_keyword and RE_ACCESS_KEYWORDS exactly as in SignalRule - move to function? +joint_access_keyword = '(' + '(\s|,)*' + '(' + '|'.join(access_keywords) + ')(\s|,)*' + ')' +RE_ACCESS_KEYWORDS = ( joint_access_keyword + # one of the access_keyword or + '|' + # or + '\(' + '(\s|,)*' + joint_access_keyword + '?' + '(' + '(\s|,)+' + joint_access_keyword + ')*' + '\)' # one or more access_keyword in (...) + ) + + +RE_FLAG = '(?P<%s>(\S+|"[^"]+"|\(\s*\S+\s*\)|\(\s*"[^"]+"\)\s*))'# string without spaces, or quoted string, optionally wrapped in (...). %s is the match group name +# plaintext version: | * | "* " | (*) | ( " * " )| + +RE_DBUS_DETAILS = re.compile( +'^' + +'(\s+(?P' + RE_ACCESS_KEYWORDS + '))?' + # optional access keyword(s) +'(\s+(bus\s*=\s*' + RE_FLAG % 'bus' + '))?' + # optional bus= system | session | AARE, (...) optional +'(\s+(path\s*=\s*' + RE_FLAG % 'path' + '))?' + # optional path=AARE, (...) optional +'(\s+(name\s*=\s*' + RE_FLAG % 'name' + '))?' + # optional name=AARE, (...) optional +'(\s+(interface\s*=\s*' + RE_FLAG % 'interface' + '))?' + # optional interface=AARE, (...) optional +'(\s+(member\s*=\s*'+ RE_FLAG % 'member'+ '))?' + # optional member=AARE, (...) optional +'(\s+(peer\s*=\s*\((,|\s)*'+ # optional peer=( name=AARE and/or label=AARE ), (...) required +'(' + +'(' + '(,|\s)*' + ')' + # empty peer=() +'|' # or +'(' + 'name\s*=\s*' + RE_PROFILE_NAME % 'peername1' + ')' + # only peer name (match group peername1) +'|' # or +'(' + 'label\s*=\s*' + RE_PROFILE_NAME % 'peerlabel1' + ')' + # only peer label (match group peerlabel1) +'|' # or +'(' + 'name\s*=\s*' + RE_PROFILE_NAME % 'peername2' + '(,|\s)+' + 'label\s*=\s*' + RE_PROFILE_NAME % 'peerlabel2' + ')' + # peer name + label (match name peername2/peerlabel2) +'|' # or +'(' + 'label\s*=\s*' + RE_PROFILE_NAME % 'peerlabel3' + '(,|\s)+' + 'name\s*=\s*' + RE_PROFILE_NAME % 'peername3' + ')' + # peer label + name (match name peername3/peerlabel3) +')' +'(,|\s)*\)))?' +'\s*$') + + +class DbusRule(BaseRule): +'''Class to handle and store a single dbus rule''' + +# Nothing external should reference this class, all external users +# should reference the class field DbusRule.ALL +class __DbusAll(object): +pass + +ALL = __DbusAll + +rule_name = 'dbus' + +def __init__(self, access, bus, path, name, interface, member, peername, peerlabel, +audit=False, deny=False, allow_keyword=False, comment='', log_event=None): + +super(DbusRule, self).__init__(audit=audit, deny=deny, + allow_keyword=allow_keyword, + comment=comment, + log_event=log_event) + +self.access,
[apparmor] [patch] [4/9] Add support for dbus events in parse_event()
Hello, this patch adds the dbus-specific details to the event data returned by parse_event(). [ 55-handle-dbus-events-in-parse_event.diff ] === modified file ./utils/apparmor/logparser.py --- utils/apparmor/logparser.py 2015-12-21 00:13:57.211799567 +0100 +++ utils/apparmor/logparser.py 2015-12-25 16:43:56.958718997 +0100 @@ -142,6 +142,12 @@ ev['peer'] = event.peer elif ev['operation'] and ev['operation'] == 'ptrace': ev['peer'] = event.peer +elif ev['operation'] and ev['operation'].startswith('dbus_'): +ev['peer_profile'] = event.peer_profile +ev['bus'] = event.dbus_bus +ev['path'] = event.dbus_path +ev['interface'] = event.dbus_interface +ev['member'] = event.dbus_member LibAppArmor.free_record(event) Regards, Christian Boltz -- > Das ist uebrigens genau der "Trick" den reiserfs verwendet! ...ich dachte immer, der "Trick" von reiserfs sei: if ($stromausfall) rm /* -r [> David Haller und Ratti in fontlinge-devel] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] [9/9] Add support for dbus events to aa-logprof
Hello, $subject. In detail, this means: - handle ptrace events in logparser.py - "translate" those events in aa.py - from log (logparser.py readlog()) to prelog (handle_children()) to log_dict (collapse_log())) - finally ask the user about the ptrace in ask_the_questions() (no code change needed there) Note that these changes are not covered by tests, however they worked in a manual test with the log examples in the libapparmor testsuite. Unfortunately there's no example log for eavesdrop, so it might be a good idea to a) add such a log line and b) test with it [ 60-add-logprof-support-for-dbus-events.diff ] === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py2015-12-27 13:13:48.245063269 +0100 +++ utils/apparmor/aa.py2015-12-27 15:06:10.149844921 +0100 @@ -1155,6 +1155,16 @@ continue prelog[aamode][profile][hat]['capability'][capability] = True +elif typ == 'dbus': +# If dbus then we (should) have pid, profile, hat, program, mode, access, bus, name, path, interface, member, peer_profile +pid, p, h, prog, aamode, access, bus, path, name, interface, member, peer_profile = entry +if not regex_nullcomplain.search(p) and not regex_nullcomplain.search(h): +profile = p +hat = h +if not profile or not hat: +continue + prelog[aamode][profile][hat]['dbus'][access][bus][path][name][interface][member][peer_profile] = True + elif typ == 'ptrace': # If ptrace then we (should) have pid, profile, hat, program, mode, access and peer pid, p, h, prog, aamode, access, peer = entry @@ -2489,6 +2499,28 @@ if not is_known_rule(aa[profile][hat], 'capability', cap_event): log_dict[aamode][profile][hat]['capability'].add(cap_event) +dbus = prelog[aamode][profile][hat]['dbus'] +for access in dbus: +for bus in dbus[access]: +for path in dbus[access][bus]: +for name in dbus[access][bus][path]: +for interface in dbus[access][bus][path][name]: +for member in dbus[access][bus][path][name][interface]: +for peer_profile in dbus[access][bus][path][name][interface][member]: +# Depending on the access type, not all parameters are allowed. +# Ignore them, even if some of them appear in the log. +# Also, the log doesn't provide a peer label, therefore always use ALL. +if access in ['send', 'receive']: +dbus_event = DbusRule(access, bus, path,DbusRule.ALL, interface, member,peer_profile, DbusRule.ALL, log_event=True) +elif access == 'bind': +dbus_event = DbusRule(access, bus, DbusRule.ALL,name, DbusRule.ALL, DbusRule.ALL, DbusRule.ALL, DbusRule.ALL, log_event=True) +elif access == 'eavesdrop': +dbus_event = DbusRule(access, bus, DbusRule.ALL,DbusRule.ALL, DbusRule.ALL, DbusRule.ALL, DbusRule.ALL, DbusRule.ALL, log_event=True) +else: +raise AppArmorBug('unexpected dbus access: %s') + + log_dict[aamode][profile][hat]['dbus'].add(dbus_event) + nd = prelog[aamode][profile][hat]['netdomain'] for family in nd.keys(): for sock_type in nd[family].keys(): === modified file ./utils/apparmor/logparser.py --- utils/apparmor/logparser.py 2015-12-27 13:13:48.245063269 +0100 +++ utils/apparmor/logparser.py 2015-12-27 15:08:57.024735157 +0100 @@ -377,6 +377,9 @@ elif e['operation'] == 'signal': return(e['pid'], e['parent'], 'signal', [profile, hat, prog, aamode, e['denied_mask'], e['signal'], e['peer']]) +elif e['operation'].startswith('dbus_'): +return(e['pid'], e['parent'], 'dbus', + [profile, hat, prog, aamode, e['denied_mask'], e['bus'], e['path'], e['name'], e['interface'], e['member'], e['peer_profile']]) else: self.debug_logger.debug('UNHANDLED: %s' % e) Regards, Christian Boltz -- programmers' bigges
[apparmor] [patch] [8/9] Add support for handling dbus rules everywhere
Hello, $subject. "Everywhere" means aa-mergeprof and aa-cleanprof. In theory also aa-logprof, but that needs some code that parses dbus log events ;-) Also add some dbus rules to the aa-cleanprof test profiles to ensure superfluous dbus rules get deleted. [ 59-enable-DbusRule-everywhere.diff ] --- utils/apparmor/aa.py2015-12-26 16:24:40.246989550 +0100 +++ utils/apparmor/aa.py2015-12-26 16:25:29.090656074 +0100 @@ -62,7 +62,7 @@ from apparmor.rule.signal import SignalRuleset,SignalRule from apparmor.rule import parse_modifiers, quote_if_needed -ruletypes = ['capability', 'change_profile', 'network', 'ptrace', 'rlimit', 'signal'] +ruletypes = ['capability', 'change_profile', 'dbus', 'network', 'ptrace', 'rlimit', 'signal'] from apparmor.yasti import SendDataToYast, GetDataFromYast, shutdown_yast === modified file ./utils/test/cleanprof_test.in --- utils/test/cleanprof_test.in2015-12-26 17:39:09.224196858 +0100 +++ utils/test/cleanprof_test.in2015-12-26 21:16:59.623391061 +0100 @@ -22,6 +22,9 @@ ptrace tracedby, unix (receive) type=dgram, +dbus send bus=session, +dbus send bus=session peer=(label=foo), + set rlimit nofile <= 256, set rlimit nofile <= 64, === modified file ./utils/test/cleanprof_test.out --- utils/test/cleanprof_test.out 2015-12-26 17:39:09.224196858 +0100 +++ utils/test/cleanprof_test.out 2015-12-26 18:13:19.051300600 +0100 @@ -12,6 +12,8 @@ network stream, + dbus send bus=session, + signal set=(abrt alrm bus chld fpe hup ill int kill pipe quit segv stkflt term trap usr1 usr2), ptrace tracedby, Regards, Christian Boltz -- [tgz Datei entpacken] tar xzf Für weitere Informationen lesen Sie bitte die Manpage oder Ihren Admin. [Torsten Hallmann in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] [7/9] Remove the DBUS_Rule class
Hello, DBUS_Rule (in rules.py) was added in r2424 as a "this is how it should look like" proof of concept, but was never used. We have a "real" class for dbus rules now, so we can drop the proof of concept class. Also remove a commented, old version of RE_DBUS_ENTRY from aa.py [ 58-delete-DBUS_Rule-class.diff ] --- utils/apparmor/rules.py 2015-12-26 15:10:02.313819739 +0100 +++ utils/apparmor/rules.py 2015-12-26 15:11:59.149032447 +0100 @@ -8,42 +8,6 @@ # # -- -class DBUS_Rule(object): -actions = set() -busses = set() -names = set() -paths = set() -interfaces = set() -members = set() -peer_names = set() -peer_labels = set() - -audit = False -deny = False - -def __init__(self, actions=[], busses=[], names=[], paths=[], interfaces=[], - members=[], peer_names=[], peer_labels=[]): -self.actions = set(actions) -self.busses = set(busses) -self.names = set(names) -self.paths = set(paths) -self.interfaces = set(interfaces) -self.members = set(members) -self.peer_name = set(peer_names) -self.peer_labels = set(peer_labels) - -def serialize(self): -out = "%s%s%s" % ('audit ' if self.audit else '', - 'deny ' if self.deny else '', - 'dbus') -if len(self.actions) > 0: -if len(self.actions) == 1: -out += ' %s' % self.actions[0] -else: -out += ' (%s)' % (', '.join(self.actions)) -out += ',' -return out - class _Raw_Rule(object): audit = False deny = False === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py2015-12-27 15:37:32.297470567 +0100 +++ utils/apparmor/aa.py2015-12-27 15:24:39.034431798 +0100 @@ -3131,9 +3131,6 @@ return profile_data -# RE_DBUS_ENTRY = re.compile('^dbus\s*()?,\s*$') -# use stuff like '(?P(send|write|w|receive|read|r|rw))' - def parse_mount_rule(line): # XXX Do real parsing here return aarules.Raw_Mount_Rule(line) Regards, Christian Boltz -- > ich wollte wohl eigentlich sagen / demonstrieren, dass > Updateritis heilbar sein kann... Das mag sein, aber der Entwöhnungsprozess kann dauern... [> David Haller und Michael Höhne in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] [5/9] Add tests for DbusRule and DbusRuleset
Hello, $subject. The tests include the two tests from test-dbus_parse.py, therefore delete this file. As usual, we have 100% coverage :-) Also addd an explicit str() conversion to common_test.py to avoid TypeError: not all arguments converted during string formatting [ 56-add-test-dbus.diff ] --- utils/test/test-dbus.py 2015-12-27 00:13:43.570046876 +0100 +++ utils/test/test-dbus.py 2015-12-27 00:10:59.171207134 +0100 @@ -0,0 +1,864 @@ +#!/usr/bin/env python +# -- +#Copyright (C) 2015 Christian Boltz <appar...@cboltz.de> +# +#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 as published by the Free Software Foundation. +# +#This program is distributed in the hope that it will be useful, +#but WITHOUT ANY WARRANTY; without even the implied warranty of +#MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +#GNU General Public License for more details. +# +# -- + +import unittest +from collections import namedtuple +from common_test import AATest, setup_all_loops + +from apparmor.rule.dbus import DbusRule, DbusRuleset +from apparmor.rule import BaseRule +from apparmor.common import AppArmorException, AppArmorBug +from apparmor.logparser import ReadLog +from apparmor.translations import init_translation +_ = init_translation() + +exp = namedtuple('exp', ['audit', 'allow_keyword', 'deny', 'comment', +'access', 'all_access', 'bus', 'all_buses', 'path', 'all_paths', 'name', 'all_names', 'interface', 'all_interfaces', 'member', 'all_members', 'peername', 'all_peernames', 'peerlabel', 'all_peerlabels']) + +# --- tests for single DbusRule --- # + +class DbusTest(AATest): +def _compare_obj(self, obj, expected): +self.assertEqual(obj.allow_keyword, expected.allow_keyword) +self.assertEqual(obj.audit, expected.audit) +self.assertEqual(obj.deny, expected.deny) +self.assertEqual(obj.comment, expected.comment) + +self.assertEqual(obj.access, expected.access) +self._assertEqual_aare(obj.bus, expected.bus) +self._assertEqual_aare(obj.path, expected.path) +self._assertEqual_aare(obj.name, expected.name) +self._assertEqual_aare(obj.interface, expected.interface) +self._assertEqual_aare(obj.member, expected.member) +self._assertEqual_aare(obj.peername, expected.peername) +self._assertEqual_aare(obj.peerlabel, expected.peerlabel) + +self.assertEqual(obj.all_access, expected.all_access) +self.assertEqual(obj.all_buses, expected.all_buses) +self.assertEqual(obj.all_paths, expected.all_paths) +self.assertEqual(obj.all_names, expected.all_names) +self.assertEqual(obj.all_interfaces, expected.all_interfaces) +self.assertEqual(obj.all_members, expected.all_members) +self.assertEqual(obj.all_peernames, expected.all_peernames) +self.assertEqual(obj.all_peerlabels, expected.all_peerlabels) + +def _assertEqual_aare(self, obj, expected): +if obj: +self.assertEqual(obj.regex, expected) +else: +self.assertEqual(obj, expected) + +class DbusTestParse(DbusTest): +tests = [ +# DbusRule object audit allow deny commentaccessall?bus all?path all?nameall?interface all?member all?peername all?peerlabel all? +('dbus,', exp(False, False, False, '',None , True , None, True, None, True, None, True, None, True, None, True, None, True, None, True)), +('dbus ( ),' , exp(False, False, False, '',None , True , None, True, None, True, None, True, None, True, None, True, None, True, None, True)), +('dbus ( , ),' , exp(False, False, False, '',None , True , None, True, None, True, None, True, None, True, None, True, None, True, None, True)), +('dbus send,' , exp(False, False, False, '',{'send'}, False, None, True, None, True, None, True, None, True, None, True, None, True, None, True)), +('dbus (send, receive),', exp(False, False, False, '',{'send', 'receive'}, False, None, True, None, True, None, True, None, True, None, True, None, True,
[apparmor] [patch] [2/9] Add strip_parenthesis() to regex.py
Hello, some dbus rule conditionals come with optional parenthesis. Instead of making the regex even more complicated, use a small function to strip those parenthesis. Also add some tests for strip_parenthesis() to test-regex.py. [ 53-add-strip_parenthesis.diff ] === modified file ./utils/apparmor/regex.py --- utils/apparmor/regex.py 2015-12-21 00:13:57.207799592 +0100 +++ utils/apparmor/regex.py 2015-12-24 01:19:47.916978461 +0100 @@ -128,6 +128,15 @@ return matches.group('magicpath') +def strip_parenthesis(data): +'''strips parenthesis from the given string and returns the strip()ped result. + The parenthesis must be the first and last char, otherwise they won't be removed. + Even if no parenthesis get removed, the result will be strip()ped. + ''' +if data[0] + data[-1] == '()': +return data[1:-1].strip() +else: +return data.strip() def strip_quotes(data): if data[0] + data[-1] == '""': === modified file ./utils/test/test-regex_matches.py --- utils/test/test-regex_matches.py2015-12-21 00:13:57.207799592 +0100 +++ utils/test/test-regex_matches.py2015-12-24 00:56:50.382751188 +0100 @@ -14,7 +14,7 @@ from common_test import AATest, setup_all_loops from apparmor.common import AppArmorBug, AppArmorException -from apparmor.regex import strip_quotes, parse_profile_start_line, re_match_include, RE_PROFILE_START, RE_PROFILE_CAP, RE_PROFILE_PTRACE, RE_PROFILE_SIGNAL +from apparmor.regex import strip_parenthesis, strip_quotes, parse_profile_start_line, re_match_include, RE_PROFILE_START, RE_PROFILE_CAP, RE_PROFILE_PTRACE, RE_PROFILE_SIGNAL class AARegexTest(AATest): @@ -501,6 +501,24 @@ re_match_include(params) +class TestStripParenthesis(AATest): +tests = [ +('foo', 'foo' ), +('(foo)', 'foo' ), +('( foo )','foo' ), +('(foo','(foo' ), +('foo )', 'foo )'), +('foo ()', 'foo ()'), +('()', '' ), +('( )','' ), +('(())','()'), +(' (foo)', '(foo)'), # parenthesis not first char, whitespace stripped nevertheless +('(foo) ', '(foo)'), # parenthesis not last char, whitespace stripped nevertheless +] + +def _run_test(self, params, expected): +self.assertEqual(strip_parenthesis(params), expected) + class TestStripQuotes(AATest): def test_strip_quotes_01(self): self.assertEqual('foo', strip_quotes('foo')) Regards, Christian Boltz -- > Und fuer die Jahre-Hiersein finde ich die zwei Ergebnisse > (unechte Mini-FAQ und Etikette) recht duenn Ich glaub es hackt. Du kannst ja das Geld zurück verlangen, wenn es Dir nicht paßt. [> toRBEN pOLLmann und Bernd Brodesser in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] [0/9] add dbus rule support to the tools
Hello, just in case someone is bored and doesn't have enough code to review... ;-) This patch series adds full support for dbus rules to the tools, including handling dbus log events in aa-logprof, de-duplication in aa-cleanprof and handling in aa-mergeprof. diffstat summary for all patches: utils/apparmor/aa.py | 19 - utils/apparmor/logparser.py|9 utils/apparmor/regex.py| 11 utils/apparmor/rule/dbus.py| 327 utils/apparmor/rules.py| 33 - utils/test/cleanprof_test.in |3 utils/test/cleanprof_test.out |2 utils/test/common_test.py |2 utils/test/test-dbus.py| 864 + utils/test/test-dbus_parse.py | 28 - utils/test/test-parser-simple-tests.py | 25 utils/test/test-regex_matches.py | 33 + 12 files changed, 1289 insertions(+), 147 deletions(-) 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] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] split off _is_equal_aare()
Hello, checking if two AARE objects are equal is not hard, but also not a one-liner. Since we need to do this more than once (and even more often in other outstanding rule classes), split that code into an _is_equal_aare() function and change PtraceRule and SignalRule to use it. To make things even more easier, the parameters to use match the _is_covered_aare() syntax. [ 44-split-off-_is_equal_aare.diff ] === modified file ./utils/apparmor/rule/__init__.py --- utils/apparmor/rule/__init__.py 2015-12-21 23:03:28.883275939 +0100 +++ utils/apparmor/rule/__init__.py 2015-12-23 21:36:16.194205414 +0100 @@ -213,6 +213,21 @@ return self.is_equal_localvars(rule_obj) +def _is_equal_aare(self, self_value, self_all, other_value, other_all, cond_name): +'''check if other_* is the same as self_* - for AARE''' + +if not other_value and not other_all: +raise AppArmorBug('No %(cond_name)s specified in other %(rule_name)s rule' % {'cond_name': cond_name, 'rule_name': self.rule_name}) + +if self_all != other_all: +return False + +if self_value and not self_value.is_equal(other_value): +return False + +# still here? -> then it is equal +return True + # @abstractmethod FIXME - uncomment when python3 only def is_equal_localvars(self, other_rule): '''compare if rule-specific variables are equal''' === modified file ./utils/apparmor/rule/ptrace.py --- utils/apparmor/rule/ptrace.py 2015-12-21 22:03:04.895715006 +0100 +++ utils/apparmor/rule/ptrace.py 2015-12-23 21:37:38.485590731 +0100 @@ -154,10 +154,7 @@ or self.all_access != rule_obj.all_access): return False -if self.all_peers != rule_obj.all_peers: -return False - -if self.peer and not self.peer.is_equal(rule_obj.peer): +if not self._is_equal_aare(self.peer, self.all_peers, rule_obj.peer, rule_obj.all_peers, 'peer'): return False return True === modified file ./utils/apparmor/rule/signal.py --- utils/apparmor/rule/signal.py 2015-12-21 22:03:04.895715006 +0100 +++ utils/apparmor/rule/signal.py 2015-12-23 21:39:47.048630569 +0100 @@ -208,10 +208,7 @@ or self.all_signals != rule_obj.all_signals): return False -if self.all_peers != rule_obj.all_peers: -return False - -if self.peer and not self.peer.is_equal(rule_obj.peer): +if not self._is_equal_aare(self.peer, self.all_peers, rule_obj.peer, rule_obj.all_peers, 'peer'): return False return True Regards, Christian Boltz -- Ich lasse ihn lieber im Glauben, er sei kein Idiot, damit es besser wirkt, wenn ich ihn mal wieder Idiot nenne. :-) [Frank Toennes in dag°] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Adjust test-aa.py for python2
Hello, Am Donnerstag, 17. Dezember 2015 schrieb Steve Beattie: > trunk (and 2.10) still build on precise, see > https://launchpad.net/~apparmor-upload/+archive/ubuntu/apparmor-devel/ > +packages (In order to even get to the point of being uploaded to that > ppa, local builds have to succeed first.) > > That said, my local automated builds haven't historically run > 'make check' on the utils/ directory due to the tests failing when > /etc/apparmor/ conf files didn't exist. Did that get addressed? Looks like you are really busy with other stuff and don't read patches or commit messages. Yes, that's fixed since about two months (r3263) :-) Regards, Christian Boltz -- > "Quite low" is 1 in 4 billion. Murphy could make me believe you saw it > once, but not twice. You could plausibly see it in a stress test rig This _is_ Christian :) he has a knack for finding bugs no one else can.. [> Crispin Cowan and Seth Arnold in apparmor-general] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Add realtime signal example to the apparmor.d manpage
Hello, $subject. I propose this patch for trunk, 2.10 and 2.9 (assuming real-time signals were supported in those versions). [ apparmor.d.pod-add-signal-rtmin-example.diff ] === modified file 'parser/apparmor.d.pod' --- parser/apparmor.d.pod 2015-08-25 11:27:18 + +++ parser/apparmor.d.pod 2015-11-24 21:50:22 + @@ -964,6 +964,9 @@ # Allow us to signal ourselves using the built-in @{profile_name} variable signal peer=@{profile_name}, +# Allow two real-time signals +signal set=(rtmin+0 rtmin+32), + =head2 DBus rules AppArmor supports DBus mediation. The mediation is performed in conjunction Regards, Christian Boltz -- > Ich werde gerne als Frau Fischer angesprochen, Herr Fischer macht > sich da nicht so gut. Warum denn nicht. Vielleicht kannst Du dann ja mit der Flugbereitschaft der Bundeswehr auf die Melediven fliegen ;o) [> Helga Fischer und Thorsten Körner in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] aa-enabled
Hello, Am Dienstag, 24. November 2015 schrieb John Johansen: > On 11/22/2015 07:20 AM, Christian Boltz wrote: > > To allow a smooth transition, I propose to add a little aa-enabled > > tool to 2.9 and 2.10 which just does > > > > #!/bin/sh > > exec aa-status --enabled > > hrmmm, this certainly doesn't solve the problem, and hardly seems Depends how you define the problem ;-) IMHO it is "calling aa-status --enabled" (especially when done from dh_apparmor and similar packaging helpers), and that can be solved by such a wrapper. (Getting dh_apparmor changed might be harder than getting a new AppArmor version in.) However... > worth doing as a backport. If we were to pull anything back to 2.9 or > 2.10 I think I would rather the bash script or ideally a simple C > program so there are no interpreter dependencies to worry about. ... I won't object if we backport the "real" aa-enabled ;-) > > This means Debian and Ubuntu can switch dh_apparmor etc. to use > > aa-enabled instead of aa-status --enabled _now_ (assuming it fits > > for > > them) instead of having to wait for a major AppArmor release. This > > allows a longer migration period. > > sure they could switch now, but such a change isn't going to show up > in the current releases. It will only be dropped into new ones Of course it will only be changed for new releases, but I'd guess getting a minor release in is easier shortly before a distribution release. > > For trunk, I propose aa-enabled should actually do the work itsself > > - > > see the "Re: [apparmor] [patch] utils: make aa-status(8) function > > without python3-apparmor" mail for a proposal. > > I agree it should do it itself, and I counter with the following C > program > > --- > > #include > #include > #include > > int main(int argc, char **argv) > { > if (aa_is_enabled()) { > printf("Y"); > return 0; > } > printf("N"); > return errno; > } Nice trick - you are using libapparmor to hide most of the code ;-) (that's not really bad because it avoids code duplication, but makes the comparison a bit unfair ;-) Oh, and the C code has a bug - like aa-status --enabled, aa-enabled should only set the exitcode, but not print anything. Anyway, I can live with both solutions as long as we get aa-enabled added ;-) Regards, Christian Boltz -- Über den Autor Marcus Meissner: Marcus Meissner entwickelt seit über 10 Jahren Opensource Entwickler. [gefunden auf http://www.linuxtag.org/2007/de/conf/events/vp-mittwoch/vortragsdetails.html?talkid=40] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Add support for signal log events to aa-logprof
Hello, Am Donnerstag, 26. November 2015 schrieb John Johansen: > > this patch adds support for signal log events to aa-logprof. > > > > In other words: this is the first new feature in aa-logprof since > > I'm working on the rule classes :-) > > > > In detail, this means: > > - handle signal events in logparser.py > > - "translate" those events in aa.py - from log (logparser.py > > readlog()) to prelog (handle_children()) to log_dict > > (collapse_log()) to log_obj (ask_the_questions()) > > (yes, really! :-/ - needless to say that this is ugly...) > > you weren't kidding I know :-/ The final goal is to have a SignalRuleset with the rules to add (if the user wants them), so the interesting question is if we can switch to SignalRuleset earlier. Ideally we would already do it in logparser.py. "Unfortunately" it has a good reasons to use a hasher() because it is a cheap way to do de- duplication of log events - it "just" overwrites a value in the hasher with another (actually the same) value if an event happens multiple times. (In other words: using SignalRuleset here already would probably have some performance impact.) I'll have to check if it makes sense to switch to SignalRuleset in handle_children(). It doesn't check against the existing profile, so the question is if a SignalRule needs more RAM than an element in a hasher. A quick measurement with http://code.activestate.com/recipes/546530/ shows that a SignalRule uses about 2150 bytes, while a hasher containing hasher['/bin/foo']['send']['int']=True needs about 1070 bytes (always assuming that this script knows how to calculate the memory usage correctly ;-) I'm quite sure we can switch to SignalRuleset in collapse_log() (which also checks against the existing profile and ignores already-covered events) so that at least ask_the_question() wouldn't need to convert it. Actually we already create a temporary SignalRule in collapse_log() to check against the profile, so storing it in a SignalRuleset wouldn't be hard (and shouldn't be a real problem for memory usage). This probably won't be the next patch I'll send, but I'll do it one day ;-) > I'm not fond of this. The translation is really ugly and the dict > stuff and hand_children() make me want to cry (bad memories of the > perl code). I already wanted to cry when I waded through the code to find out where I need to add something for the signal log events. That's also the reason why I added the details to the patch description - now we have an in-bzr manual explaining how to add handling for other rule types ;-) Regards, Christian Boltz -- > Meine Fonts füllen die komplette Wand, also könnte ich auch kein > größeres Poster brauchen. :-) Ich verwende für die Wände immer Tapete ;-) [> Ratti und Christian Boltz] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] parser: add basic support for parallel compiles and loads
Hello, Am Samstag, 28. November 2015 schrieb John Johansen: > v2 > switch to default --jobs=auto > check and bail on bad debug argument > update comments on work_sync_one() > check for an invalid number of maximum jobs() > put an upper limit on the maximum number of jobs to 8*# of cpus > +static void setup_parallel_compile(void) > +{ > + /* jobs_count and paralell_max set by default, config or args */ > + long n = sysconf(_SC_NPROCESSORS_ONLN); > + if (jobs_count == JOBS_AUTO) > + jobs_count = n; > + if (jobs_max == JOBS_AUTO) > + jobs_max = n; > + /* jobs_max will be used internally */ > + jobs_max = min(jobs_count, jobs_max); > + if (jobs_max > 8*n) { > + PERROR("%s: Invalid maximum number of jobs '%ld' > 8 * # of > cpus", > +progname, jobs_max); > + exit(1); > + } So the parser will error out if a too big job number is given _and_ if there are enough profiles to load (otherwise the number gets reduced to the number of available profiles/jobs). In other words: I can use -j 1 for quite a while, but it will break after I add another profile to my system. This sounds like surprising behaviour to me (especially because it depends on the number of profiles to load), and I'm not sure if it's critical enough to exit(). I'd prefer to reduce jobs_max to 8*n and print a warning. In C-like Pseudocode: + if (jobs_max > 8*n) { + WARN("%s: Invalid maximum number of jobs '%ld' > 8 * # of cpus, reducing to %ld", + progname, jobs_max, n); + jobs_max = 8*n; + } Regards, Christian Boltz -- Who is General Failure and why is he reading my disk? -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] aa-enabled
Hello, Am Samstag, 28. November 2015 schrieb John Johansen: > v3 > > change conflicting/unknown option warning message slightly > output error string on failure > add binutils dir > add manpage > add makefile > add pot file > === added file 'binutils/Makefile' > --- binutils/Makefile 1970-01-01 00:00:00 + > +++ binutils/Makefile 2015-11-28 18:18:25 + It looks like you copied large parts of parser/Makefile. Would it make sense to split those common parts off to a separate file, like common/Make-c.rules? (That's nothing that should stop you from adding aa-enabled, so feel free to do that as a follow-up patch.) BTW: It seems you never commited the parser/Makefile cleanup patch series you sent a while ago. Is there a special reason, or did you just forget it? (Also, does binutils/Makefile need some similar cleanups, or are they already integrated?) > === added file 'binutils/aa-enabled.c' > --- binutils/aa-enabled.c 1970-01-01 00:00:00 + > +++ binutils/aa-enabled.c 2015-11-28 17:34:45 + ... > +#ifndef PACKAGE > +#define PACKAGE "" > +#define LOCALEDIR "" > +#endif Now that we have a nice Makefile, is this still needed? > === added file 'binutils/po/aa-enabled.pot' > --- binutils/po/aa-enabled.pot1970-01-01 00:00:00 + > +++ binutils/po/aa-enabled.pot2015-11-28 18:23:11 + > @@ -0,0 +1,67 @@ > +# SOME DESCRIPTIVE TITLE. > +# Copyright (C) YEAR Canonical Ltd > +# This file is distributed under the same license as the PACKAGE > package. + > # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR. That's a very informative copyright header, especially the uppercase parts ;-) That said: The patch looks good to me (with the questions answered or addressed), but I'll leave acking it for someone who understands C better. Regards, Christian Boltz -- Diese Signatur ist vorübergehend nicht erreichbar. Versuchen Sie es später noch einmal oder hinterlassen Sie eine Nachricht vor dem Signaturtrenner. Piep. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] aa-enabled
Hello, Am Dienstag, 24. November 2015 schrieb John Johansen: > v3 containing all suggested changes + copyright and ifdef around > packaging defines for PACKAGE and LOCALEDIR, at least until make file > packaging can be worked out Looks good, I noticed just some minor details. Feel free to include easy to fix things in your commit, otherwise commit as acked by Seth and address this with a follow-up patch. > #ifndef PACKAGE > #define PACKAGE "" > #define LOCALEDIR "" > #endif > setlocale(LC_MESSAGES, ""); > bindtextdomain(PACKAGE, LOCALEDIR); > textdomain(PACKAGE); I interpret this as not using translations for now, right? If that is easy to change, do it now - otherwise commit as is. > if (argc > 2) { > printf(_("unknown options\n")); This could also mean someone used aa-enabled --quiet --help which doesn't sound too useful, but will still result in a "wrong" error message. Adding a more detailed check just to print the "right" error message would be overmuch, so what about changing the error message to something like unknown or incompatible options ? > case EACCES: > printf(_("Maybe - insufficient permissions to determine > availability.\n")); break; Do we need a "run aa-enabled as root" hint here? >default: >printf(_("No\n")); How likely is it to hit this "no"? If "not very likely" - would it make sense to print out err to make it easier to find out what caused the "no"? Regards, Christian Boltz -- > > Moin Moin,> Wann stehst Du denn üblicherwiese auf ;-) So grüßt man sich z. B. in Hamburg von 0 bis ca. 23:59:59 Uhr. Faulpelze und Rucksack-Fischköppe wie ich sagen nur einmal *Moin* :-) P.S.: Wer jetzt fragt, wie man sich hier in der restlichen Zeit grüßt, ist doof ;-) [>> Mathias Bölke, > Manfred Tremmel und Jan Trippler 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: Don't check for existence of abstraction files in aa-easyprof
Hello, Am Sonntag, 29. November 2015 schrieb Tyler Hicks: > aa-easyprof is used to generate profiles and the lack of an > abstraction file during profile generation should not be an error > condition. > > Leave the handling of the abstraction file for the parser. It will > fail if the file does not exist when the profile is being compiled. > > https://launchpad.net/bugs/1521031 I understand why you want this change, and agree that raising an exception in aa-easyprof is too heavy. OTOH, I don't like to silently ignore missing abstractions. What about printing a warning message? It won't hurt in build environments (I'd guess nobody reads warnings - if something is serious, it's an error ;-) but if someone calls aa-easyprof manually, it can still be helpful. > --- a/utils/apparmor/easyprof.py > +++ b/utils/apparmor/easyprof.py > @@ -507,8 +507,6 @@ class AppArmorEasyProfile: > def gen_abstraction_rule(self, abstraction): > '''Generate an abstraction rule''' > p = os.path.join(self.aa_topdir, "abstractions", abstraction) > -if not os.path.exists(p): > -raise AppArmorException("%s does not exist" % p) better: warn("%s does not exist" % p) You'll need to import warn() from apparmor.common. Actually apparmor.easyprof also has a warn() function - I seriously recommend to de-duplicate it and import it from apparmor.common. Speaking about de-duplicating - there are quite some functions in apparmor.easyprof that are also in apparmor.common. Often with nearly the same code, but it seems apparmor.commen received more bugfixes ;-) (comparing both files with meld is the easiest way to see the common code) Regards, Christian Boltz -- Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Adjust type(x) == str checks in the rule classes for py2
r/rule/rlimit.py 2015-07-17 00:19:58.574811968 +0200 +++ utils/apparmor/rule/rlimit.py 2015-11-29 21:18:18.101563810 +0100 @@ -16,7 +16,7 @@ import re from apparmor.regex import RE_PROFILE_RLIMIT, strip_quotes -from apparmor.common import AppArmorBug, AppArmorException +from apparmor.common import AppArmorBug, AppArmorException, type_is_str from apparmor.rule import BaseRule, BaseRuleset, parse_comment, quote_if_needed # setup module translations @@ -57,7 +57,7 @@ if audit or deny or allow_keyword: raise AppArmorBug('The audit, allow or deny keywords are not allowed in rlimit rules.') -if type(rlimit) == str: +if type_is_str(rlimit): if rlimit in rlimit_all: self.rlimit = rlimit else: @@ -70,7 +70,7 @@ self.all_values = False if value == RlimitRule.ALL: self.all_values = True -elif type(value) == str: +elif type_is_str(value): if not value.strip(): raise AppArmorBug('Empty value in rlimit rule') === modified file ./utils/apparmor/rule/signal.py --- utils/apparmor/rule/signal.py 2015-11-23 22:15:45.411432694 +0100 +++ utils/apparmor/rule/signal.py 2015-11-29 21:18:38.877542865 +0100 @@ -16,7 +16,7 @@ from apparmor.aare import AARE from apparmor.regex import RE_PROFILE_SIGNAL, RE_PROFILE_NAME -from apparmor.common import AppArmorBug, AppArmorException +from apparmor.common import AppArmorBug, AppArmorException, type_is_str from apparmor.rule import BaseRule, BaseRuleset, check_and_split_list, parse_modifiers, quote_if_needed # setup module translations @@ -96,7 +96,7 @@ self.all_peers = False if peer == SignalRule.ALL: self.all_peers = True -elif type(peer) == str: +elif type_is_str(peer): if len(peer.strip()) == 0: raise AppArmorBug('Passed empty peer to SignalRule: %s' % str(peer)) self.peer = AARE(peer, False, log_event=log_event) === modified file ./utils/test/test-common.py --- utils/test/test-common.py 2015-11-29 21:58:47.320305689 +0100 +++ utils/test/test-common.py 2015-11-29 21:13:39.121644329 +0100 @@ -0,0 +1,32 @@ +#! /usr/bin/env python +# -- +# +#Copyright (C) 2015 Christian Boltz <appar...@cboltz.de> +# +#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 unittest +from common_test import AATest, setup_all_loops + +from apparmor.common import type_is_str + +class TestIs_str_type(AATest): +tests = [ +('foo', True), +(u'foo',True), +(42,False), +(True, False), +([],False), +] + +def _run_test(self, params, expected): +self.assertEqual(type_is_str(params), expected) + + +setup_all_loops(__name__) +if __name__ == '__main__': +unittest.main(verbosity=2) Regards, Christian Boltz -- [BILD] Als langjährig tätiger Strafverteidiger (und Fan von Volker Pispers) muß ich jedoch dringend davor warnen, stinkende tote Fische in dieses Freiexemplar der sogenannten "Zeitung" einzuwickeln. Weil das ein Strafverfahren wegen Beleidigung zulasten des Fisches nach sich ziehen könnte. [http://www.kanzlei-hoenig.de/2012/keine-stinkende-tote-fische-im-briefkasten/] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] AppArmor 2.10 branch created
Hello, Am Mittwoch, 18. November 2015 schrieb Steve Beattie: > After the discussion in the IRC meeting yesterday and in preparation > for upcoming AppArmor 2.10.1 and 2.11 releases, I have created the > apparmor 2.10 branch on launchpad: I hereby nominate all my pending patches for 2.10 ;-) (Yes, that includes the signal rule handling, even if that is a new feature ;-) @John: You acked the two patches add a named match group to RE_PROFILE_SIGNAL Add debug info to profile_storage() an hour after Steve created the 2.10 branch. Please tell me if that ack was for trunk only or if you also want those patches in 2.10. Regards, Christian Boltz -- >> sorry, Zitat stammt von William Shakespeare (Hamlet) >> das Posting sollte keinesfalls als Plagiat entlarft werden ;) > Jaja, sonst ist dein Doktor-Titel weg. Siehe Schawan und Andere. An welcher Uni kann man denn mit einem Posting auf der openSUSE Liste promovieren;-) [>> Christian Meseberg, > Bernhard Junk und Ralf Arndt auf opensuse-de] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] fix-abstraction-for-python3.5.patch
Hello, Am Mittwoch, 18. November 2015 schrieb Jamie Strandboge: > Description: update python abstraction for python 3.5 > > Signed-off-by: Jamie Strandboge <ja...@canonical.com> I'd prefer a more future-proof version, which means to use python{2.[4-7],3.[0-9]} in all rules ;-) With or without this change, Acked-by: Christian Boltz <appar...@cboltz.de> for trunk, 2.10 and 2.9 (Damn, my logprof.conf update also didn't include python3.5 :-/ ) Regards, Christian Boltz -- Du solltest mal deine sHIFT-taSTE und die ordogravieh in Ordnung bringen. Das hilft in jedem Falle dabei, hier qualifizierte Antworten zu bekommen. [Axel Heinrici 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 missing variables to the apparmor.d manpage
Hello, @{pids} and @{apparmorfs} was not mentioned in the apparmor.d manpage. I propose this patch for trunk, 2.10 and 2.9 (all have those variables defined in tunables/*) [ apparmor.d.pod-add-missing-variables.diff ] === modified file ./parser/apparmor.d.pod --- parser/apparmor.d.pod 2015-11-19 17:42:26.329879090 +0100 +++ parser/apparmor.d.pod 2015-11-20 20:23:27.042844698 +0100 @@ -1230,8 +1230,10 @@ @{HOMEDIRS} @{multiarch} @{pid} + @{pids} @{PROC} @{securityfs} + @{apparmorfs} @{sys} @{tid} @{XDG_DESKTOP_DIR} 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] [patch] Map c (create) log events to w instead of a
Hello, creating a file is in theory covered by the 'a' permission, however discussion on IRC brought up that depending on the open flags it might not be enough (real-world example: creating the apache pid file). Therefore change the mapping to 'w' permissions - that might allow more than needed in some cases, but makes sure the profile always works. I propose this patch for 2.9, 2.10 and trunk [ 23-map-create-to-w.diff ] === modified file ./utils/apparmor/logparser.py --- utils/apparmor/logparser.py 2015-11-19 17:42:26.333879063 +0100 +++ utils/apparmor/logparser.py 2015-11-19 20:51:49.139296808 +0100 @@ -296,15 +296,15 @@ self.debug_logger.debug('UNHANDLED (missing request_mask): %s' % e) return None -# Map c (create) to a and d (delete) to w (logging is more detailed than the profile language) +# Map c (create) and d (delete) to w (logging is more detailed than the profile language) rmask = e['request_mask'] -rmask = rmask.replace('c', 'a') +rmask = rmask.replace('c', 'w') rmask = rmask.replace('d', 'w') if not validate_log_mode(hide_log_mode(rmask)): raise AppArmorException(_('Log contains unknown mode %s') % rmask) dmask = e['denied_mask'] -dmask = dmask.replace('c', 'a') +dmask = dmask.replace('c', 'w') dmask = dmask.replace('d', 'w') if not validate_log_mode(hide_log_mode(dmask)): raise AppArmorException(_('Log contains unknown mode %s') % dmask) Regards, Christian Boltz -- Warum nochmal benutzen alle Procmail? Das ist eine Art Quiz, oder? Wer die unleserlichtste Regel erstellt, bekommt einen Preis? [Thorsten Haude in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] aa-enabled
Hello, aa-status --enabled (and only --enabled AFAIK) is used in dh_apparmor, which means we have to keep the dependencies as small as possible, especially "import apparmor.$something". Needless to say that this makes improving aa-status quite difficult. We discussed splitting off an "aa-enabled" tool that just does what aa-status --enabled does with minimal dependencies. To allow a smooth transition, I propose to add a little aa-enabled tool to 2.9 and 2.10 which just does #!/bin/sh exec aa-status --enabled This means Debian and Ubuntu can switch dh_apparmor etc. to use aa-enabled instead of aa-status --enabled _now_ (assuming it fits for them) instead of having to wait for a major AppArmor release. This allows a longer migration period. For trunk, I propose aa-enabled should actually do the work itsself - see the "Re: [apparmor] [patch] utils: make aa-status(8) function without python3-apparmor" mail for a proposal. Regards, Christian Boltz -- Und da Du mit der Installation so wenig Probleme hast, könntest Du doch die gesparte Zeit in ein paar Großbuchstaben investieren. Liest sich besser am Bildschirm. Danke. [Helga Fischer 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] Add SignalRule and SignalRuleset classes
Hello, [scroll down for an add-on patch that addresses Kshitij's comments] Am Donnerstag, 19. November 2015 schrieb Kshitij Gupta: > On Fri, Oct 23, 2015 at 6:30 PM, Christian Boltz wrote: > > this patch adds the SignalRule and SignalRuleset classes > > [ 07-add-SignalRule-and-SignalRuleset.diff ] > > > > === modified file ./utils/apparmor/rule/signal.py > > --- utils/apparmor/rule/signal.py 2015-10-23 > > 01:17:21.579245521 +0200 +++ utils/apparmor/rule/signal.py > > 2015-10-23 01:08:01.149132984 +0200 @@ -0,0 +1,300 @@ ... > > + 'io', 'pwr', 'sys', 'emt', 'exists'] > > +RE_SIGNAL_REALTIME = > > re.compile('^rtmin\+0*([0-9]|[12][0-9]|3[0-2])$') # > > rtmin+0..rtmin+32, number may have leading zeros > > I do not like this regex. > Its far too complicated for when its only saying: rtmin+x such that x > maybe 2digit and is in [0,32] and possibly return x. Plus Its > confusing whether rtmin+032 is allowed or not (regex suggests it is). It is allowed. Even rtmin+00032 is allowed ;-) > Maybe its easier(more readably) done in a function with some int() and > boundary tests. We have more interesting regexes, and that one is not too terrible IMHO. With your proposal, the regex would probably be ^rtmin\+([0-9]+)$ and a boundary check at another place. I slightly doubt that makes the code more readable ;-) > > +joint_access_keyword = '\s*(' + '|'.join(access_keywords) + ')\s*' > > Better[tm] written as: > joint_access_keyword = ''\s*(%s)\s*' % ('|'.join(access_keywords)) If it would be the only regex in the file, I'd fully agree. However the other regexes nearby are composed using + to keep them readable. Therefore I tend to keep using + also here ;-) > > +RE_ACCESS_KEYWORDS = ( joint_access_keyword + # one of the > > access_keyword or > > + '|' + > ># or > > + '\(' + joint_access_keyword + '(' + > > '(\s|,)+' + joint_access_keyword + ')*' + '\)' # one or more > > signal_keyword in (...) + ) > > Thats some beast! Thanks to the syntax of signal rules - I don't like some details, but obviously have to support all of them. (You missed some funny rants about that on IRC ;-) > > allow_keyword=allow_keyword, + > >comment=comment, + > > log_event=log_event) + > > +self.access, self.all_accesss, unknown_items = > > check_and_split_list(access, access_keywords, SignalRule.ALL, > > 'SignalRule', 'access') > > all_accesss (three s's) ;-) > I hope that was intentional. Not really, but at least it's consistent across signal.py (thanks autocompletion!) and therefore "just" a strange variable name without causing bugs. Nevertheless, I'll remove the middle s. > > +if RE_SIGNAL_REALTIME.match(item): > > +self.signal.add(item) > > +else: > > +raise AppArmorException('Passed unknown signal > > keyword to SignalRule: %s' % item) > > Missing _(). > AppArmorExceptions are expected to have translations while for > AppArmorBug we dont, right? Right, I'll change that (here and some lines below) > > +if details.group('access'): > > +access = details.group('access') > > +access = ' '.join(access.split(',')) # split by > > ',' or whitespace > > Is it expected to split strings separated by a , or whitespace? This > part will only split strings separated by comma. > It can't do both(which the comment confused me to believe until I read > on and saw the space split ;-) ). Actually the comment describes what the code does, but I changed the code after writing the comment ;-) This line splits at the comma and re-joins with a space as delimiter. After looking at it again, .replace() should be enough (and probably faster). Also, replacing the comma and split() should be done nearby. I'll change that to a single line so that the comment fits again ;-) > > +if access.startswith('(') and access.endswith(')'): > > +access = access[1:-1] > > +access = access.split() > > There's the space separated split. Yes, after the (sometimes optional) "(...)" wrapper was removed. And that's also the place where the comma split/replacement should and will happen. > > +if not self.all_peers: > > +if other_rule.all_peers: > > +return False > > +if other_rule.peer != self.peer: # XXX use AARE > >
[apparmor] [patch] Also add python 3.5 to logprof.conf
Hello, $subject (as pointed out by Jamie's python abstraction update) I propose this patch for 2.9, 2.10 and trunk. [ logprof-conf-python3.5.diff ] === modified file 'utils/logprof.conf' --- utils/logprof.conf 2015-11-18 12:39:07 + +++ utils/logprof.conf 2015-11-19 17:43:10 + @@ -112,6 +112,7 @@ /usr/bin/python3 = icn /usr/bin/python3.3= icn /usr/bin/python3.4= icn + /usr/bin/python3.5= icn /usr/bin/tr = icn [required_hats] Regards, Christian Boltz -- > oder das absolut berauschende ;-)) > [ -d "/test/" ] || echo mkd Danke, zum Glück muß ich heute nicht mehr mit dem Auto fahren :-) [> Thomas Preissler und Al Bogner in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [RFC PATCH 1/1] libapparmor: Create man page for aa_stack_profile()/aa_stack_onexec()
Hello, Am Dienstag, 12. Januar 2016 schrieb Tyler Hicks: > More boilerplate from aa_change_profile(2). That's what I get for > copying from a man page that is incorrect. :) Please send a patch to fix aa_change_profile.pod to ensure you get a correct copy next time ;-) Regards, Christian Boltz -- Ja, aber Popcorn over IP (PoIP) ist noch nicht so ganz ausgereift. Ich habe schon versucht, das mit RFC1149 zu koppeln, aber die blöden Viecher fressen die Popcorn immer selber :-( [Peter J. Holzer [ais] <slrnbhqmta.h3c.hjp-use...@teal.hjp.at>] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Add a note about still enforcing deny rules to aa-complain manpage
Hello, $subject. This behaviour makes sense (for example to force the confined program to use a fallback path), but is probably surprising for users, so we should document it. References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=826218#37 I propose this patch for trunk, 2.10 and 2.9 [ 02-aa-complain-deny-note.diff ] === modified file 'utils/aa-complain.pod' --- utils/aa-complain.pod 2014-09-15 18:30:47 + +++ utils/aa-complain.pod 2016-06-05 16:17:23 + @@ -41,6 +41,8 @@ In this mode security policy is not enforced but rather access violations are logged to the system log. +Note that 'deny' rules will be enforced even in complain mode. + =head1 BUGS If you find any bugs, please report them at Regards, Christian Boltz -- When a device looks like a printer, acts like a printer, and sounds like a printer, that device could be a computer. [Johannes Meixner in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [utils] Refactor Severity module [Refactor series]
Hello, Am Donnerstag, 2. Juni 2016, 03:02:43 CEST schrieb Kshitij Gupta: > The following patch: > 1. Refactors Severity module in hopes of making it more readable and > simplifying up some of the terse logic. (This resulted in re-write of > some segments) > 2. Updates relevant modules/tools accordingly (checked with grep for > usages) > > Side notes: > 1. The tests were ultra-useful while refactoring, hopefully they cover > sufficient stuff. Use make coverage-html and have a look at htmlcov/* to get an exact picture of test coverage ;-) > 2. I missed not working with git (but then I am not familiar with bzr) Whenever I read an introduction to git, I'm scared by all the commands and options it offers. Therefore I still prefer something simple like bzr ;-) (nevertheless I won't object to switching to git if I'm the only one who thinks so) > 3. Comments are welcome regarding bugs and style > === modified file 'utils/aa-mergeprof' > --- utils/aa-mergeprof2016-05-10 12:32:46 + > +++ utils/aa-mergeprof2016-06-01 21:16:50 + > @@ -18,14 +18,18 @@ > import os > > import apparmor.aa > -from apparmor.aa import available_buttons, combine_name, delete_duplicates, > get_profile_filename, is_known_rule, match_includes > import apparmor.aamode > -from apparmor.common import AppArmorException > -from apparmor.regex import re_match_include > + > import apparmor.severity > import apparmor.cleanprofile as cleanprofile > import apparmor.ui as aaui > > +from apparmor.aa import (available_buttons, combine_name, delete_duplicates, superfluous trailing space > + get_profile_filename, is_known_rule, match_includes) > +from apparmor.common import AppArmorException > +from apparmor.regex import re_match_include The imports reordering makes sense, even if it will probably make it a bit more interesting for my FileRule series (whoever commits last will need to adjust the patch ;-) To ensure a defined commit order, maybe I should just let you win the race and add Acked-by: Christian Boltz <appar...@cboltz.de> for the import reordering above and in rule/capability.py. > @@ -660,7 +664,7 @@ > > # Load variables into sev_db? Not needed/used > for capabilities and network rules. > severity = rule_obj.severity(sev_db) > -if severity != sev_db.NOT_IMPLEMENTED: > +if severity != apparmor.severity.NOT_IMPLEMENTED: I'm not sure if I like this change. sev_db gets passed into *Rule as function parameter, so it feels more natural to check against sev_db.NOT_IMPLEMENTED instead of checking against something that's imported in a totally different way. > === modified file 'utils/apparmor/aa.py' > --- utils/apparmor/aa.py 2016-05-10 12:32:46 + > +++ utils/apparmor/aa.py 2016-05-29 18:40:31 + > @@ -1644,7 +1644,7 @@ > > # Load variables into sev_db? Not needed/used > for capabilities and network rules. > severity = rule_obj.severity(sev_db) > -if severity != sev_db.NOT_IMPLEMENTED: > +if severity != apparmor.severity.NOT_IMPLEMENTED: (same as above) > === modified file 'utils/apparmor/rule/__init__.py' > --- utils/apparmor/rule/__init__.py 2016-01-25 22:48:34 + > +++ utils/apparmor/rule/__init__.py 2016-05-29 18:42:17 + > @@ -13,6 +13,8 @@ > # > # -- > > +import apparmor.severity > + > from apparmor.aare import AARE > from apparmor.common import AppArmorBug, type_is_str > > @@ -237,9 +239,9 @@ > '''return severity of this rule, which can be: > - a number between 0 and 10, where 0 means harmless and 10 means > critical, > - "unknown" (to be exact: the value specified for "unknown" as > set when loading the severity database), or > - - sev_db.NOT_IMPLEMENTED if no severity check is implemented for > this rule type. > + - apparmor.severity.NOT_IMPLEMENTED if no severity check is > implemented for this rule type. > sev_db must be an apparmor.severity.Severity object.''' > -return sev_db.NOT_IMPLEMENTED > +return apparmor.severity.NOT_IMPLEMENTED (same as above) > === modified file 'utils/apparmor/rule/capability.py' > --- utils/apparmor/rule/capability.py 2016-01-25 22:48:34 + > +++ utils/apparmor/rule/capability.py 2016-06-01 21:17:54 + > @@ -13,10 +13,11 @@ > # > # -- > > +import
[apparmor] [patch] aa-genprof: ask about profiles in extra dir (again)
RE_ENTRY', 'CMD_GLOB', 'CMD_GLOBEXT', 'CMD_NEW', 'CMD_AUDIT_OFF', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py available_buttons() with CMD_AUDIT_OFF (['CMD_ALLOW', 'CMD_DENY', 'CMD_IGNORE_ENTRY', 'CMD_GLOB', 'CMD_GLOBEXT', 'CMD_NEW', 'CMD_AUDIT_NEW', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py available_buttons() with CMD_AUDIT_NEW (['CMD_SAVE_CHANGES', 'CMD_SAVE_SELECTED', 'CMD_VIEW_CHANGES', 'CMD_VIEW_CHANGES_CLEAN', 'CMD_ABORT'], True), # aa.py save_profiles() -(['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', 'CMD_CREATE_PROFILE', 'CMD_ABORT', 'CMD_FINISHED'],True), # aa.py get_profile() +(['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', 'CMD_CREATE_PROFILE', 'CMD_ABORT'],True), # aa.py get_profile() (['CMD_UPLOAD_CHANGES', 'CMD_VIEW_CHANGES', 'CMD_ASK_LATER', 'CMD_ASK_NEVER', 'CMD_ABORT'], True), # aa.py console_select_and_upload_profiles() (['CMD_ix', 'CMD_pix', 'CMD_cix', 'CMD_nix', 'CMD_EXEC_IX_OFF', 'CMD_ux', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py build_x_functions() with exec_toggle (['CMD_ix', 'CMD_cx', 'CMD_px', 'CMD_nx', 'CMD_ux', 'CMD_EXEC_IX_ON', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py build_x_functions() without exec_toggle Regards, Christian Boltz -- > > Vielen Dank, daß du dir die Zeit nimmst, dran rumzutesten. > Wenn Du es nicht gemerkt hast: Ich empfehle Dir jetzt die Features, > die ich gern hätte. Geht schneller, als es selbst zu bauen *g* Verdammt. Ich wurde reingelegt! ;-)) [Ratti zu > $me beim Testen seiner Fontlinge] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] aa-genprof: ask about profiles in extra dir (again)
Hello, Am Mittwoch, 1. Juni 2016, 23:06:41 CEST schrieb Kshitij Gupta: > On Wed, Jun 1, 2016 at 5:37 AM, Christian Boltz <appar...@cboltz.de> wrote: > > thanks to reading the wrong directory in read_inactive_profiles() > > (profile_dir instead of extra_profile_dir), aa-genprof never asked > > about using a profile from the extra_profile_dir. > > > > Sounds like an easy fix, right? ;-) ... > > I propose this patch for 2.9, 2.10 and trunk > > (test-translations.py is only in trunk, therefore this part of the > > patch is obviously trunk-only.) > > > > > > [ 01-genprof-ask-for-extra-dir.diff ] > > > > === modified file ./utils/apparmor/aa.py > > --- utils/apparmor/aa.py2016-05-30 23:16:05.713448348 +0200 > > +++ utils/apparmor/aa.py2016-06-01 01:25:31.242505830 +0200 > > @@ -578,8 +578,11 @@ > > > > inactive_profile[prof_name][prof_name].pop('filename') > > profile_hash[uname]['username'] = uname > > profile_hash[uname]['profile_type'] = 'INACTIVE_LOCAL' > > > > -profile_hash[uname]['profile'] = > > serialize_profile(inactive_profile[prof_name], prof_name) > > +profile_hash[uname]['profile'] = > > serialize_profile(inactive_profile[prof_name], prof_name, None) > > Wouldn't it be better to then make the parameter optional with default > value as None, I have two reasons: > 1. I see the two parameter form littered around in aa.py *hides* I noticed that and intentionally didn't fix it because it points out dead code ;-) Well, at least to someone who knows about the function parameters, so "me yesterday" ;-) > 2. The function has built in handling for cases when the options are > not present and sort of makes sense to have an option free serialiser > with a default > 3. The None there looks absolutely meaningless and confusing (to me > and explains why I above said two reasonsand not 3) Sounds like good arguments - patches welcome ;-) > > profile_hash[uname]['profile_data'] = inactive_profile > > + > > +existing_profiles.pop(prof_name) # remove profile filename > > from list to force storing in /etc/apparmor.d/ instead of > > extra_profile_dir > force storing in profile_dir you mean? Yes, of course. I just used /etc/apparmor.d to make the command easier to understand ;-) > > +proc.communicate(p['profile'].encode()) > > space before encode [comment ignored as discussed on IRC] > > @@ -683,6 +682,7 @@ > > > > if not profile_data: > > profile_data = create_new_profile(pname) > > > > file = get_profile_filename(pname) > > > > +profile_data[pname][pname]['filename'] = None # will be stored > > in /etc/apparmor.d when saving, so it shouldn't carry the > > profile_dir? see above ;-) > I agree to the sentiment that this is essentially a bugfix for a > long broken feature. > > Acked-by: Kshitij Gupta <kgupta8...@gmail.com> Thanks! Regards, Christian Boltz -- Die drei Todfeinde des Programmieres: Sonnenlicht, frische Luft und das unerträgliche Gebrüll der Vögel. signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] aa-genprof: ask about profiles in extra dir (again)
Hello, Am Dienstag, 31. Mai 2016, 17:22:16 CEST schrieb Seth Arnold: > On Wed, Jun 01, 2016 at 02:07:10AM +0200, Christian Boltz wrote: > > thanks to reading the wrong directory in read_inactive_profiles() > > (profile_dir instead of extra_profile_dir), aa-genprof never asked > > about using a profile from the extra_profile_dir. > > > > Sounds like an easy fix, right? ;-) > > > > After fixing this (last chunk), several other errors popped up, one > > after the other: ... > > I propose this patch for 2.9, 2.10 and trunk > > Acked-by: Seth Arnold <seth.arn...@canonical.com> > for trunk > > Nice set of fixes; however I'm uncomfortable with making that large a > change on 2.9, 2.10. I know it's only a wee tiny little code change > but the semantic idea of the change is quite a bit larger, and it > doesn't feel appropriate to me to make the change anywhere except > trunk. (I won't object if someone else ACKs it for 2.9, 2.10.. I just > don't feel comfortable myself.) Well, it introduces a behaviour change if a profile exists in extra, (asking if the user wants to use this profile as starting point), but I'd argue that this fixes a regression that was introduced in 2.9 during the rewrite to python ;-) BTW: I tested the patch on top of trunk and the 2.10 and 2.9 bzr branches, and it works everywhere. @John: is your ACK also valid for 2.10 and 2.9? > I'm also confused by the removal of the CMD_FINISHED; how do you end > an aa-genprof or aa-logprof session? CMD_FINISHED is only removed in the question that asks if you want to use the profile in extra or if you want to create a new one. And you still have Abo(r)t as an option: # python3 aa-genprof /usr/bin/skype Profile: /usr/bin/skype [1 - Inactive local profile for /usr/bin/skype] [(V)iew Profile] / (U)se Profile / (C)reate New Profile / Abo(r)t<--- (F)inish removed here Writing updated profile for /usr/bin/skype. Setting /usr/bin/skype to complain mode. Before you begin, [...] Profiling: /usr/bin/skype [(S)can system log for AppArmor events] / (F)inish [...] Without my patch, there also was (F)inish at the first question. However, it behaved exactly like (C)reate New Profile, and continued with the (S)can log / (F)inish question. This means it was a) useless and b) confusing. > And, final question: > > +profile_data[pname][pname]['filename'] = None # will be stored > > in /etc/apparmor.d when saving, so it shouldn't carry the > > extra_profile_dir filename > Is this correct? Should it instead be: > > del profile_data[pname][pname]['filename'] Both work. Setting it to None has the advantage that the data structure stays complete ('filename' is still there, just empty/None) and can be used without a .get('filename') safeguard. > I'm not sure what else may depend upon this being present-but-None vs > not-even-present. (Half of why I'm excited about the death of the > hasher.. :) I'm using a POC "strict storage" patch since a while which changes profile_storage() to return a dict() with a specified set of sub-items (so I can notice access to funny[tm] uninitialized places without breaking it for users - and actually found some of those places, so I don't want to merge this patch yet ;-) I can of course post this patch in case someone wants to play with it. The code expects 'filename' to be in the data structure - all checks basically use if ...['filename']: so setting it to None doesn't cause any problems. I'm not aware of any is None or == '' checks for it. TL;DR: I'm quite sure something would explode when using "del" ;-) as soon as my "strict storage" patch enters the stage. In theory I could set it to the final filename instead of None - but write_profile() basically does this itsself (using get_profile_filename()) if 'filename' is empty, so why should I do superfluous work? ;-) That all said - hasher is not dead yet ;-) Regards, Christian Boltz -- and devs
[apparmor] [patch] apparmor.d.pod: document 'deny x'
Hello, deny rules don't allow ix, Px, Ux etc. - only 'deny /foo x,' is allowed. (Well, mostly - see https://bugs.launchpad.net/apparmor/+bug/1532578 ) I propose this patch for trunk and 2.10 (it doesn't apply on the 2.9 apparmor.d.pod, and I'm too lazy to backport it ;-) [ apparmor.d.pod-deny-x.diff ] === modified file ./parser/apparmor.d.pod --- parser/apparmor.d.pod 2016-01-10 18:02:11.060675379 +0100 +++ parser/apparmor.d.pod 2016-01-10 18:00:49.985190030 +0100 @@ -251,7 +251,7 @@ B = ( 'r' | 'w' | 'a' | 'l' | 'k' | 'm' | I )+ (not all combinations are allowed; see below.) -B = ( 'ix' | 'ux' | 'Ux' | 'px' | 'Px' | 'cx' | 'Cx' | 'pix' | 'Pix' | 'cix' | 'Cix' | 'pux' | 'PUx' | 'cux' | 'CUx' ) +B = ( 'ix' | 'ux' | 'Ux' | 'px' | 'Px' | 'cx' | 'Cx' | 'pix' | 'Pix' | 'cix' | 'Cix' | 'pux' | 'PUx' | 'cux' | 'CUx' | 'x' ) ('x' is only allowed in rules with the deny qualifier, everything else only without the deny qualifier) B = name (requires I specified) @@ -366,6 +366,10 @@ - transition to subprofile on execute with fallback to unconfined -- scrub the environment +=item B + +- disallow execute (in rules with the deny qualifier) + =item B - allow PROT_EXEC with mmap(2) calls @@ -428,7 +432,7 @@ run unconfined and LD_PRELOAD must be used. Any profile using this mode provides negligible security. Use at your own risk. -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -442,7 +446,7 @@ Use this mode only if the child absolutely must be run unconfined. Use at your own risk. -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -454,7 +458,7 @@ LD_PRELOAD; as a result, the calling domain may have an undue amount of influence over the callee. -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -463,7 +467,7 @@ the environment, similar to setuid programs. (See ld.so(8) for some information on setuid/setgid environment scrubbing.) -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -475,7 +479,7 @@ LD_PRELOAD; as a result, the calling domain may have an undue amount of influence over the callee. -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -484,7 +488,7 @@ the environment, similar to setuid programs. (See ld.so(8) for some information on setuid/setgid environment scrubbing.) -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -498,7 +502,7 @@ version to scrub the environment because 'ix' executions don't change privileges. -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -512,7 +516,7 @@ 'Cix' == 'Cx' with fallback to 'ix' 'cix' == 'cx' with fallback to 'ix' -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. =item B @@ -527,7 +531,14 @@ 'CUx' == 'Cx' with fallback to 'Ux' 'cux' == 'cx' with fallback to 'ux' -Incompatible with other exec transition modes. +Incompatible with other exec transition modes and the deny qualifier. + +=item B + +For rules including the deny modifier, only 'x' is allowed to deny execute. + +The 'ix', 'Px', 'px', 'Cx', 'cx' and the fallback modes conflict with the deny +modifier. =item B Regards, Christian Boltz -- Dann siehst du nämlich ganz genau, daß der Cursor blinkt, und er hat feuerrote tote Augen, mit denen er dich anstarrt und brüllt: ".. UND WENN DU DICH VERTIPPST, DANN FRESSE ICH DICH MITSAMT DEINEM MAUSZEIGER"[Ratti in suse-programming] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] Add compressed dfa matching routines to library, and a base test program
umber of + * potential NFA states to track in parallel is known in > advanced and + * of a fixed number. > + * > + * Atm this code only implements the DFA portion of the match and > does + * not allow for NFA states nor the extended work memory. > + */ I don't say that I completely understand this, but a short and simplified summary seems to be that this code does matching like the kernel does when deciding if an action is allowed/denied. Right? Does this also mean this code is "stolen" from the kernel code? > +/** > + * aa_hfa_can_fail - evaluates whether @state could potentially fail > for @hfa + * @hfa: hfa to consider > + * @hfa: state struct to be paired with @hfa > + * > + * Returns: true if the combination can fail, else false > + */ > +bool aa_hfa_can_fail(struct aa_hfa *hfa, struct aa_state *state) > +{ > + return false; > +} That a) looks very optimistic ("nothing will fail") or b) misses at least a TODO note ;-) > +/** > + * aa_hfa_matchn_cont - traverse @hfa from @state to find state @str I can't find aa_hfa_matchn_cont in your patch or the existing code. Wrong/superfluous comment? Or did you just change the function name after writing the comment? > stops at + * @hfa: the hfa to match @str against (NOT NULL) > + * @state: the state of the hfa to start matching in > + * @str: the string of bytes to match against the hfa (NOT NULL) > + * @len: length of the string of bytes to match > + * > + * aa_hfa_matchn will match @str against the hfa and return the state > it + * finished matching in. The final state can be used to look up > the accepting + * label, or as the start state of a continuing match. > + * > + * This function will happily match again the 0 byte and only match again_st_? > finishes + * when @len input is consumed. > + * > + * Returns: 0 on success else error. > + * > + * Note: most matches are guarenteed to succeed, see aa_hfa_reset, > and + * aa_hfa_can_fail. > + */ > +int aa_hfa_continuen(struct aa_hfa *hfa, struct aa_state *state, > + const char *str, size_t len) > +/** > + * aa_hfa_matchn - reset and traverse @hfa to find state @str stops > at + * @hfa: the hfa to match @str against (NOT NULL) > + * @state: the state of the hfa to start matching in > + * @str: the string of bytes to match against the hfa (NOT NULL) > + * @len: length of the string of bytes to match > + * > + * aa_hfa_matchn will match @str against the hfa and return the state > it + * finished matching in. The final state can be used to look up > the accepting + * label, or as the start state of a continuing match. > + * > + * This function will happily match again the 0 byte and only match again_st_? > finishes + * when @len input is consumed. > + * > + * Returns: 0 on success else error. > + * > + * Note: most matches are guarenteed to succeed, see aa_hfa_reset, > and + * aa_hfa_can_fail. > + */ > +int aa_hfa_matchn(struct aa_hfa *hfa, struct aa_state *state, const > char *str, +size_t len) > diff --git a/libraries/libapparmor/src/hfa_unpack.c > b/libraries/libapparmor/src/hfa_unpack.c new file mode 100644 > index 000..c23f214 > --- /dev/null > +++ b/libraries/libapparmor/src/hfa_unpack.c > +static u32 remap_accept_to_idx(u32 nstates, char *accept1, char > *accept2, + struct accept64 **accept, u16 **idx) ... > + UNPACK_TO_STRUCT(tmp_accept, accept2, nstates, u32, be32_to_cpu, > + accept2); > + /* set up an state table to sort and do merge based off of */ set up _a_ state table > diff --git a/libraries/libapparmor/swig/SWIG/libapparmor.i > b/libraries/libapparmor/swig/SWIG/libapparmor.i index > 69b4cc2..47213a1 100644 > --- a/libraries/libapparmor/swig/SWIG/libapparmor.i > +++ b/libraries/libapparmor/swig/SWIG/libapparmor.i > +extern void aa_dfa_free(struct aa_dfa *dfa); > +extern struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int > flags); > +extern unsigned int aa_dfa_match_len(struct aa_dfa *dfa, > unsigned int start, + const char *str, int len); > +extern unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int > start, + const char *str); > +extern unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int > state, + const char c); > +extern unsigned int aa_dfa_accept(struct aa_dfa *dfa, unsigned int > state); It's nice to see more library functions exported towards the swig bindings. Now to the practical question - are some of those functions useful to, let's say, handle the AARE matching in the tools? Regards, Christian Boltz -- Versuchst du mal bitte zu formulieren, was du eigentlich möchtest? Mit diesem Posting hast du gute Chancen auf den "Marcel Stein Award". [Christian Paul in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 4/4] dconf patch
Hello, Am Montag, 14. Dezember 2015 schrieb William Hua: > Just made one minor change to make dconf rules more consistent with > other rules (parsing permissions after paths). Some notes about 0004-Add-support-for-dconf-confinement.patch: > --- a/parser/parser_lex.l > +++ b/parser/parser_lex.l > +{ > + r(ead)? { RETURN_TOKEN(TOK_READ); } > + w(rite)?{ RETURN_TOKEN(TOK_WRITE); } > + (rw|wr) { > RETURN_TOKEN(TOK_READWRITE); } > + ({PATHNAME}|{QPATHNAME}){ That's much better than the lng list of possible keywords in the last round. I still wonder if we really need "read" and "write" or if "r" and "w" would be enough ;-) (Yes, I know we allow "read" and "write" at other places, but we don't need to repeat that error ;-) > --- a/parser/parser_yacc.y > +++ b/parser/parser_yacc.y > +dconf_perm: TOK_READ { $$ = AA_DCONF_READ; } > + | TOK_WRITE { $$ = AA_DCONF_READWRITE; /* writable implies readable > */ } > + | TOK_READWRITE { $$ = AA_DCONF_READWRITE; } I still don't like the idea to implicitely grant read permissions if something has write permissions. This needs *at least* a very clear note in the documentation (BTW: did I overlook the apparmor.d.pod patch?). The more strict and IMHO better way would be to error out if only write is allowed in a profile. Also, can you please add a parser/tst/simple_tests/dconf/ directory with some example profiles (some with valid, some with invalid syntax)? Regards, Christian Boltz -- Bugzilla beißt nicht und ist viel, viel netter als ich. ;) [Lars Müller in opensuse-de] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] apparmor.d.pod: add details about append and creating files
Hello, $subject. I hope my description is correct. If not, please give me a better text ;-) I propose this patch for trunk, 2.10 and 2.9. [ apparmor.d.pod-explain-append.diff ] === modified file ./parser/apparmor.d.pod --- parser/apparmor.d.pod 2016-01-07 20:41:32.738787547 +0100 +++ parser/apparmor.d.pod 2016-01-10 18:00:49.985190030 +0100 @@ -403,7 +407,10 @@ Append mode will prevent an application from opening the file for write unless it passes the O_APPEND parameter flag on open. -The mode conflicts with Write mode. +Append mode also allows to create a file using creat() or open() with the +O_APPEND flag. However, you'll need 'w' when using open() with O_RW | O_CREATE. + +This mode conflicts with Write mode. =item B Regards, Christian Boltz -- > > Ooooch, nu sei doch nicht gleich gnidderig. :-) > Bin ich doch gar nicht :-))) Dann ist ja gut. (Bratpfanne unauffällig fallenlass und mit dem Fuß wegstubs...) [> Christian Boltz und Ratti in fontlinge-devel] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] Add compressed dfa matching routines to library, and a base test program
Hello, Am Montag, 11. Januar 2016 schrieb John Johansen: > On 01/10/2016 07:22 AM, Christian Boltz wrote: > > Am Freitag, 8. Januar 2016 schrieb John Johansen: > >> diff --git a/devtools/Makefile b/devtools/Makefile > >> new file mode 100644 > >> index 000..b0cd26e > >> --- /dev/null > >> +++ b/devtools/Makefile > >> +print_hfa: print_hfa.o > >> + $(CC) ${CFLAGS} -o $@ $^ -L ../libraries/libapparmor/src/.libs/ > >> -static -lapparmor > > > > Should this -L depend on USE_SYSTEM? > > (I'm not sure which files are needed, so maybe I'm wrong ;-) > > well, not yet. Eventually we will get there but this being the first > pass I actually wanted to restrict it to the in tree builds atm I'd still use a variable and set it to "-L ../libraries/..." in both branches of the "if USE_SYSTEM" to make it obvious that we build in-tree only. > >> diff --git a/devtools/expr.txt b/devtools/expr.txt > >> new file mode 100644 > >> index 000..4b7d12b > >> --- /dev/null > >> +++ b/devtools/expr.txt > >> @@ -0,0 +1,772 @@ > >> +1 / > >> +1 /([^\])*\.[Bb][Mm][Pp] > >> +1 /([^\])*\.[Bb][Zz]2 > >> +1 /([^\])*\.[Cc][Bb][7RZrz] > >> +1 /([^\])*\.[Dd][Jj][Vv][Uu] > >> +1 /([^\])*\.[Dd][Vv][Ii] > > > > What's the reason for using that syntax (looks like "real" regexes > > or > > PCRE) instead of AARE? > > because it is a PCRE syntax which is more powerful than what AARE can > express and what is actually used internally on the backend. > > > I'd prefer to feed the devtools with AARE, since this is what we use > > in the profiles. A requirement "variables must be expanded" would > > be ok. > Nope, and nope. > > This is targeting testing the backend dev work. It allows more and > will also start allowing "kernel" vars etc which won't be expanded. Indeed, kernel vars are a good argument to not expect _all_ variables expanded ;-) It might still be ok to expect non-kernel vars expanded. (At least for now - a separate parameter containing variable values might be even better, but one step after the other ;-) > Eventually we may open up some of the PCRE syntax to the front end > profiles, certainly if not the syntax some of the capabilities. > > We do have some unit tests for converting between the two, but more > tests are always good There are some tests (including expected matches) in the python tests (test-aare.py TestConvert_regexpAndAAREMatch) that test the python AARE implementation. If you provide a way to convert them (or even to run everything from python), we can run the same set of tests against the libapparmor/future kernel AARE checking. Yes, I know that won't happen in the next days. Just keep it in mind as something to do in the future. > >> diff --git a/devtools/print_hfa.c b/devtools/print_hfa.c > >> new file mode 100644 > >> index 000..c5a5b65 > >> --- /dev/null > >> +++ b/devtools/print_hfa.c > >> > >> +const char *short_options = ""; > >> +struct option long_options[] = { > >> + {NULL, 0, 0, 0}, > >> +}; > >> + > >> +static int process_args(int argc, char *argv[]) > > Also, how can the error condition (case 0) happen? From reading > > getopt_long(3), I'd guess it happens if a long option is matched - > > but it's very unlikely that NULLL will ever match ;-) > > (Also, why do we need that NULL in long_options at all?) > > the null is a terminator, without it you can get strange option > processing based on random junk in memory, and yes we had a bug > related to this once Sounds interesting[tm] ;-) (It also sounds like a bug in getopt_long, but if the workaround is that easy, we should of course keep it as safety net. Nevertheless, I hope this is or gets fixed in getopt_long.) > > Reading a file into a buffer doesn't look that easy in C ;-) > > > > Is there really no function available somewhere that shortens all > > this to something like > > > > buffer = read_file(filename) > > > > + checking errno? > > meh, there are alternatives, like using fopen and fread, mmap, ... > > I'm not sure why I through in a more low level approach here. It can > be changed That would be a good idea - I already found two double-close(), and someone who really understands C might find even more interesting things. > >> +char *load_file(const char *path) > > > > That looks like the same load_file() as in print_hfa.c, therefore > > - please avoid copy prog
[apparmor] IRC meeting
Hello, the next IRC meeting is scheduled for next wednesday (Jan 13) at 20:00 UTC. I have an offline meeting that day and might be late (not sure how long it takes), therefore options are a) just ignore this and hope that I'm not too late b) move the meeting - thursday and friday would work for me, also most days of the following week except wednesday and thursday I have only the usual topics for the meeting, which are - rewiew my patches! (including several bugfixes) [1] - get 2.9.3 and 2.10.1 released ASAP so it's not too bad if I really miss the meeting ;-) Regards, Christian Boltz [1] I have only ;-) 29 pending patches: ==> 38-more-useful-logparser-failure-reports.diff <== More useful logparser failure reports ==> 39-split-off-aare_or_all.diff <== split off _aare_or_all() ==> 40-split-off-_is_covered-helpers.diff <== split off _is_covered_*() helper functions ==> 41-ptrace-signal-use-list-in-is_covered.diff <== Use list check in PtraceRule and SignalRule is_covered_localvars() ==> 42-improve-repr-empty-ruleset.diff <== Improve __repr__() for *Ruleset ==> 43-prevent-crash-by-serialize_profile_from_old_profile.diff <== Prevent crash caused by serialize_profile_from_old_profile() ==> 44-split-off-_is_equal_aare.diff <== split off _is_equal_aare() ==> 45-change-log_dict-to-profile_storage.diff <== Change log_dict to use profile_storage() and simplify log translation ==> 46-serialize_profile_from_old_profile-fix-wrong-access-to-write_prof_data.diff <== Fix wrong usage of write_prof_data in serialize_profile_from_old_profile() ==> 47-fix-multi-profile-mergeprof-crash.diff <== Fix aa-mergeprof crash with files containing multiple profiles ==> 48-add-more-ruletypes-to-cleanprof-test.diff <== Add more ruletypes to the cleanprof test profiles ==> 49-parse-unknown-line-better-error-msg.diff <== Better error message on unknown profile lines ==> 51-split-off-logprof_value_or_all.diff <== Split off logprof_value_or_all() ==> 52-add-match-group-to-RE_PROFILE_DBUS.diff <== [1/9] add a named match group to RE_PROFILE_DBUS ==> 53-add-strip_parenthesis.diff <== [2/9] Add strip_parenthesis() to regex.py ==> 54-add-DbusRule.diff <== [3/9] Add DbusRule and DbusRuleset classes ==> 55-handle-dbus-events-in-parse_event.diff <== [4/9] Add support for dbus events in parse_event() ==> 56-add-test-dbus.diff <== [5/9] Add tests for DbusRule and DbusRuleset ==> 57-use-DbusRule.diff <== [6/9] Use DbusRule and DbusRuleset ==> 58-delete-DBUS_Rule-class.diff <== [7/9] Remove the DBUS_Rule class ==> 59-enable-DbusRule-everywhere.diff <== [8/9] Add support for handling dbus rules everywhere ==> 60-add-logprof-support-for-dbus-events.diff <== [9/9] Add support for dbus events to aa-logprof ==> 61-autodep-remove-pname-bin_name-mapping.diff <== Remove pname to bin_name mapping in autodep() ==> 62-ptrace-peer-strip-quotes.diff <== Handle quoted peers when parsing ptrace rules ==> apparmor.d.pod-deny-x.diff <== apparmor.d.pod: document 'deny x' ==> apparmor.d.pod-explain-append.diff <== apparmor.d.pod: add details about append and creating files ==> document-empty-quotes-in-variables.diff <== Document empty quotes ("") as empty value of a variable ==> profiles-dovecot-lda.diff <== dovecot-lda profile: allow tempfiles and executing sendmail ==> update-sshd-profile.diff <== Update the sshd profile (might be obsoleted by Simon's merge request) That's a total of 24 files changed, 1698 insertions(+), 395 deletions(-) The biggest part is the dbus series: 12 files changed, 1289 insertions(+), 147 deletions(-) -- > I'll be happy to fix the wording or Germanglish :D And shift it to Netherlangish? ;) [> Jos Poortvliet and Lars Müller in opensuse-project] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] www.apparmor.net broken
Hello, http://www.apparmor.net/ -> "Forbidden" http://apparmor.net/ -> "Service Unavailable - Guru Meditation: XID: 1530364709" Can someone check and fix that, please? (Shouldn't be too hard since it's just a redirect to the wiki.) At least wiki.apparmor.net works for people who know the wiki subdomain. Regards, Christian Boltz -- > Ich hätte auch nie geglaubt, das es 10 Minuten dauern kann, bis jemand > ohne Fehler einmal ein lspci -v fehlerfrei eingegeben hat. Hast Du auch erwähnt, daß man anschließend die RETURN-Taste drücken muß? [> Torsten Hallmann und Bernd Brodesser in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Fix a missing comma in parser_misc.c capnames
Hello, the capnames list missed a comma, which lead to the funny "mac_overridesyslog" capability name. __debug_capabilities() seems to be the only user of capnames, which might explain why this bug wasn't noticed earlier. I propose this patch for trunk, 2.10 and 2.9. BTW: Do we really need capnames or could the code be changed to use the list from cap_names.h? [ parser-fix-missing-comma.diff ] === modified file 'parser/parser_misc.c' --- parser/parser_misc.c2015-07-11 01:16:09 + +++ parser/parser_misc.c2016-01-15 12:15:53 + @@ -724,7 +724,7 @@ "audit_write", "audit_control", "setfcap", - "mac_override" + "mac_override", "syslog", }; Regards, Christian Boltz -- Durr, shouldn't send emails before having my morning coffee. [Steve Beattie in apparmor] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] Fix: segfault when processing directories
Hello, Am Donnerstag, 14. Januar 2016 schrieb John Johansen: > On 01/14/2016 04:57 PM, Steve Beattie wrote: > > On Thu, Jan 14, 2016 at 04:46:36PM -0800, John Johansen wrote: > >> Patch -r 2952 switched over to using the library kernel interface, > >> and added a kernel_interface parameter to the dir_cb struct, that > >> is used to process directories. > >> > >> Unfortunately kernel_interface parameter of the dir_cb struct is > >> not being properly initialized resulting in odd failures and > >> sefaults when the parser is processing directories. > >> > >> Signed-off-by: John Johansen <john.johan...@canonical.com> > > > > Acked-by: Steve Beattie <st...@nxnw.org> for both trunk and 2.10. > > > > Is there a bug report open for this? > > It does now. > http://bugs.launchpad.net/bugs/1534405 > > I ran into it today when trying to give Jamie some newer timings for > the parallel jobs patch. > > It wasn't caught previously because most of my testing used -Q to skip > the kernel load, and what kernel loads I did do where on a version > pre the kernel_interface patch I tend to disagree ;-) About a month ago (2015-12-07), I told you about the 2.10 parser segfaulting when loading a directory on IRC (and sent you the core dump). I just tested by compiling the parser before and after running bzr up (using trunk). Before, it segfaulted - and after bzr up, it doesn't segfault anymore. So it seems this patch fixes "my" segfault. Therefore I officially claim the credits for reporting this bug first ;-)) BTW: Even if it looks like this patch fixes my issue, it might be a good idea to give my core dump a quick check if it really hit the same bug. Regards, Christian Boltz -- Was glaubst Du, wie oft ich fluche, daß diese Windowskisten erst ein explizites 'Nun speichere auch endlich in die Zwischenablage' wünschen und ich immer erst ins Leere klicke, wenn ich's eilig habe, nur weil ich ein strg-c vergessen habe? Menno, können die sich nicht an den üblichen *nix-Standard halten? [Helga Fischer in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] utils: Handle the safe/unsafe change_profile exec modes
150,10 +170,12 @@ class ChangeProfileRule(BaseRule): > return True > > def logprof_header_localvars(self): > +execmode_txt= logprof_value_or_all(self.execmode, > None) Same again ;-) Also, did you really test this in all variants? I'd guess not specifying an exec mode will result in Exec Mode: or even Exec Mode: None 'safe' / 'unsafe' would be better choices ;-) Also, you don't need to use logprof_value_or_all() - with None as second parameter, it will just return the first parameter (self.execmode) so you can use that directly. (There's some special handling for sets, lists and tuples, but that doesn't matter here.) So basically you should do if self.execmode: execmode_txt = self.execmode else execmode_txt = 'safe' > @@ -144,20 +153,20 @@ class ChangeProfileFromInit(ChangeProfileTest): > class InvalidChangeProfileInit(AATest): I'm not quoting your changes because KMail made them unreadable by changing the linebreaks ;-) Nevertheless, please add another test that includes an invalid execmode, for example (['maybe', '/foo', '/bar' ], AppArmorBug), # invalid execmode (should be one line, but I won't do manual linebreaks in the whole mail just to get it right here ;-) > @@ -248,6 +257,8 @@ class > ChangeProfileCoveredTest_01(ChangeProfileCoveredTest): > +(' change_profile safe /foo,' , [ False , > False , False , False ]), After handling 'safe' and None/'' as equal, you'll need to adjust this test ;-) > ChangeProfileCoveredTest_02(ChangeProfileCoveredTest): ( > + ( 'change_profile safe /foo -> /bar,' , [ False , False , False , False ]), Same here. > +class ChangeProfileCoveredTest_06(ChangeProfileCoveredTest): > +rule = 'change_profile safe /foo,' > > +tests = [ > +# rule equal > strict equalcovered covered exact +( 'deny > change_profile /foo,' , [ False , False , False > , False ]), +('audit deny change_profile /foo,' , > [ False , False , False , False ]), +( > 'change_profile /foo,' , [ False , False , > False , False ]), # XXX should covered be true here? + > ( 'deny change_profile /bar,' , [ False , False > , False , False ]), +( 'deny change_profile,' >, [ False , False , False , False ]), + > ] and here ;-) > @@ -333,7 +356,7 @@ class ChangeProfileCoveredTest_Invalid(AATest): > def test_borked_obj_is_covered_2(self): You might want to add another testcase that sets an invalid testobj.execmode. > class ChangeProfileLogprofHeaderTest(AATest): > tests = [ > + ('change_profile,', [ > _('Exec Mode'), None, _('Exec Condition'), _('ALL'), _('Target Profile'), _('ALL'), ]), Ah, here you confirm that Exec Condition: None will be displayed in aa-logprof or aa-mergeprof. As I already wrote in the comment about logprof_headers_localvars(), you should avoid that ;-) (and adjust the tests afterwards) > diff --git a/utils/test/test-parser-simple-tests.py > b/utils/test/test-parser-simple-tests.py index 304ff98..66b77ab > 100644 > --- a/utils/test/test-parser-simple-tests.py > +++ b/utils/test/test-parser-simple-tests.py > @@ -47,6 +47,11 @@ exception_not_raised = [ > 'capability/bad_3.sd', > 'capability/bad_4.sd', > 'change_hat/bad_parsing.sd', > + > +# The tools don't detect conflicting change_profile exec modes > +'change_profile/onx_conflict_unsafe1.sd', > +'change_profile/onx_conflict_unsafe2.sd', OK for now, but having conflict detection would be nice. Regards, Christian Boltz -- [Evolution - Message-ID] Oh ja... Apropos: die libcamel (die fuer diesen Muell verantwortlich ist) ist, aehm. "interessant" zu lesen... Und NEIN! Ich habe keine Lust, den Muell zu fixen. Es sei denn, man zahlt mir Schmerzensgeld. [David Haller in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] Pending patches
Helau! Several of my patches didn't get a review since (much) more than a week. Will someone review them, or should I just commit them as Acked-by ? a) apparmor.d.pod changes (also for 2.10 and 2.9) ==> apparmor.d.pod-deny-x.diff <== apparmor.d.pod: document 'deny x' ==> apparmor.d.pod-explain-append.diff <== apparmor.d.pod: add details about append and creating files ==> document-empty-quotes-in-variables.diff <== Document empty quotes ("") as empty value of a variable b) profile updates (also for 2.10 and 2.9) ==> profiles-dovecot-lda.diff <== dovecot-lda profile: allow tempfiles and executing sendmail c) various utils fixes (some of them also for 2.10 and 2.9) ==> 43-prevent-crash-by-serialize_profile_from_old_profile.diff <== Prevent crash caused by serialize_profile_from_old_profile() ==> 45-change-log_dict-to-profile_storage.diff <== Change log_dict to use profile_storage() and simplify log translation ==> 46-serialize_profile_from_old_profile-fix-wrong-access-to-write_prof_data.diff <== Fix wrong usage of write_prof_data in serialize_profile_from_old_profile() ==> 47-fix-multi-profile-mergeprof-crash.diff <== Fix aa-mergeprof crash with files containing multiple profiles ==> 48-add-more-ruletypes-to-cleanprof-test.diff <== Add more ruletypes to the cleanprof test profiles ==> 61-autodep-remove-pname-bin_name-mapping.diff <== Remove pname to bin_name mapping in autodep() ==> 62-ptrace-peer-strip-quotes.diff <== Handle quoted peers when parsing ptrace rules ==> 66-add-tests-for-get_output-and-get_reqs.diff <== Add tests for aa.py get_output() and get_reqs() ==> 67-get_output-dont-ignore-non-executable.diff <== aa.py get_output(): raise exception on non-executable or non-existing programs ==> 68-logparser-check-sanity-of-all-file-events.diff <== logparser.py: do sanity check for all file events ==> 69-error-out-on-dir-exec.diff <== Error out if the log contains an exec event for a directory (this patch is quite new, but I'm sure it will time out before someone reviews all the other patches *eg*) d) the DBus series ==> 52-add-match-group-to-RE_PROFILE_DBUS.diff <== [1/9] add a named match group to RE_PROFILE_DBUS ==> 53-add-strip_parenthesis.diff <== [2/9] Add strip_parenthesis() to regex.py ==> 54-add-DbusRule.diff <== [3/9] Add DbusRule and DbusRuleset classes ==> 55-handle-dbus-events-in-parse_event.diff <== [4/9] Add support for dbus events in parse_event() ==> 56-add-test-dbus.diff <== [5/9] Add tests for DbusRule and DbusRuleset ==> 57-use-DbusRule.diff <== [6/9] Use DbusRule and DbusRuleset ==> 58-delete-DBUS_Rule-class.diff <== [7/9] Remove the DBUS_Rule class ==> 59-enable-DbusRule-everywhere.diff <== [8/9] Add support for handling dbus rules everywhere ==> 60-add-logprof-support-for-dbus-events.diff <== [9/9] Add support for dbus events to aa-logprof Any comments or reviews on these patches? If nobody objects, I'll commit it on Thursday as Acked-by (except patch 69, which times out on Friday ;-) Regards, Christian 'Viking for four days' Boltz -- OMG I'm not perfect! Please don't tell anyone! [Henne Vogelsang in opensuse-wiki] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Drop unused suggest_incs_for_path() in aa.py
Hello, $subject ;-) [ 70-drop-unused-suggest_incs_for_path.diff ] === modified file 'utils/apparmor/aa.py' --- utils/apparmor/aa.py2016-01-26 06:54:53 + +++ utils/apparmor/aa.py2016-02-05 21:52:09 + @@ -4252,29 +4252,6 @@ return combinedmode, combinedaudit, matches -def suggest_incs_for_path(incname, path, allow): -combinedmode = set() -combinedaudit = set() -matches = [] - -includelist = [incname] -while includelist: -inc = includelist.pop(0) -cm, am, m = rematchfrag(include[inc][inc], 'allow', path) -if cm: -combinedmode |= cm -combinedaudit |= am -matches += m - -if include[inc][inc]['allow']['path'].get(path, False): -combinedmode |= include[inc][inc]['allow']['path'][path]['mode'] -combinedaudit |= include[inc][inc]['allow']['path'][path]['audit'] - -if include[inc][inc]['include'].keys(): -includelist += include[inc][inc]['include'].keys() - -return combinedmode, combinedaudit, matches - def check_qualifiers(program): if cfg['qualifiers'].get(program, False): if cfg['qualifiers'][program] != 'p': Regards, Christian Boltz -- > Aber sorry, habe die Schnauze voll mit Linux Da gehört's eindeutig nicht hin. Nimm's lieber wieder raus. [> Juergen Jaeckel und Bernd Glueckert in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Error out if the log contains an exec event for a directory
Hello, according to the discussion with John on IRC, exec log events for directories should never happen, therefore let handle_children() raise an exception. [ 69-error-out-on-dir-exec.diff ] --- utils/apparmor/aa.py2016-02-04 01:21:33.010848414 +0100 +++ utils/apparmor/aa.py2016-02-04 17:49:00.985255184 +0100 @@ -1208,8 +1203,7 @@ if mode & str_to_mode('x'): if os.path.isdir(exec_target): -mode = mode - apparmor.aamode.ALL_AA_EXEC_TYPE -mode = mode | str_to_mode('ix') +raise AppArmorBug('exec permissions requested for directory %s. This should not happen - please open a bugreport!' % exec_target) else: do_execute = True Regards, Christian Boltz -- > > "Frontpage" is a M$ WYSIWYG web page creation program. Would you like some Wine with that ActiveX? You must have a different Outlook(tm) on things, I thought it was an Excel(tm)lent Word(tm). [>> Carl Hartung and Peter Flodin in opensuse] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Fix aa-mergeprof crash with files containing multiple profiles
Hello, Am Freitag, 12. Februar 2016, 03:26:36 CET schrieb Kshitij Gupta: > On Sat, Dec 26, 2015, Christian Boltz <appar...@cboltz.de> wrote: > > if a profile file contains multiple profiles, aa-mergeprof crashes > > on > > saving in write_profile() because the second profile in the file is > > not listed in 'changed'. > > > > This patch first checks if 'changed' contains the profile before > > pop()ing it. > > Assuming, this is because the second profile is actually not changing Exactly. (I should have mentioned this in the patch description.) > )looking at the in and out files, only comment was removed)/ > I guess ideally we should probably have two lists of profiles for a > file(changed and unchanged), to maintain that the profile is always > processed and we may show this info elsewhere. Not really - the list of changed profiles should stay exactly what it is, and we already make sure that we keep all profiles in a file. One day, we'll need to support nested child profiles, and when we implement that, we'll probably also have to adjust several of our internal storage methods (aa[profile][hat] won't work anymore if you allow deeper nesting, and several other things that use a similar pattern will also explode. Oh, and external hats already explode with the current code.) While adjusting that, we can also address the file <-> profile relationship storage. > > Reproducer: copy utils/test/cleanprof_test.in to your profile > > directory and run aa-mergeprof utils/test/cleanprof_test.out. > > Then just press 's' to save the profile. > > > > > > I can reproduce this with trunk, 2.10 and 2.9 and therefore propose > > this patch for all these branches. > > > > > > [ 47-fix-multi-profile-mergeprof-crash.diff ] > > > > --- utils/apparmor/aa.py2015-12-26 16:47:30.614839586 +0100 > > +++ utils/apparmor/aa.py2015-12-26 17:27:36.376228122 +0100 > > @@ -4039,7 +4039,11 @@ > > > > os.rename(newprof.name, prof_filename) > > > > -changed.pop(profile) > > +if profile in changed: > > +changed.pop(profile) > > +else: > > +debug_logger.error("%s written, but not listed in 'changed' > > list" % profile) > > + > > We may rephrase it to something like: "Unchanged/unlisted profile %s > written to file" with a log level of warn? It will only be visible in the debug log (so it's unlikely someone sees it in practise), but I'll change the message to Unchanged profile written: %s I won't switch to the warn loglevel because DebugLogger doesn't support that ;-) - thinking about it, info is probably the better choice. > With above nit-pick considered: > > Acked-by: Kshitij Gupta <kgupta8...@gmail.com> > > for trunk, 2.10 and 2.9 Thanks! 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!!! signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] Pending patches
Helau! Am Sonntag, 7. Februar 2016, 05:35:25 CET schrieb Kshitij Gupta: > On 07-Feb-2016 3:54 am, "Christian Boltz" <appar...@cboltz.de> wrote: > > Helau! > > Interesting greeting! Some Viking warcry? No, see below ;-) > > Any comments or reviews on these patches? > > I'll try to review some of them today or this week. :-) > > Christian 'Viking for four days' Boltz > > I need to know the story behind this! Well, it's carnival time again - and this year, we (the rural youth of Insheim) chose to be Vikings. We are on tour on some carnival parades (the first one was on saturday, the last one is tomorrow), which sums up to four days ;-) To come back to your first question - "Helau" is a "Narrenruf" (literally "Jester Cry") and is something we shout a lot these days. It's typically split to He-lau, and the people watching the carnival parades (and other carnival events) are expected to shout the "-lau" part). See also http://paste.opensuse.org/a3dd4fb0 for quite some Vikings ;-) > And what it entails!!! Basically we follow the openSUSE /etc/motd - Have a lot of fun... ;-) Regards, Christian Boltz -- > Ich versuchs mal so zu sagen: Ich versuche gerade, dich lngsam > ins kalte Wasser zu schubsen. ich mag kein kaltes Wasser, las uns die weiteren Tests nach Playa de Santiago verlegen, da dürfte das Wasser jetzt ca. 20° C haben. [> Ratti und Gerald Goebel in fontlinge-devel] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Add tests for aa.py get_output() and get_reqs()
Hello, Am Montag, 1. Februar 2016, 11:50:49 CET schrieb Seth Arnold: > On Mon, Feb 01, 2016 at 07:35:07PM +0100, Christian Boltz wrote: > > --- utils/test/test-aa.py 2016-01-26 22:22:14.660008000 +0100 > > +++ utils/test/test-aa.py 2016-02-01 18:53:10.085684909 +0100 > > +import apparmor.aa as aa # needed to set global vars in some tests > Most of this looked good but I'm a bit worried about the global > variables. Could they be referenced via apparmor.aa. > instead? I think that'd lead to better comprehension two years from > now.. Yes, that's possible ;-) - it makes the import a bit shorter ("as aa" gets removed) and the usage a bit longer ("apparmor.aa.cfg...") Here's v2 with this changed: [ 66-add-tests-for-get_output-and-get_reqs.diff ] --- utils/test/test-aa.py 2016-01-26 22:22:14.660008000 +0100 +++ utils/test/test-aa.py 2016-02-01 18:53:10.085684909 +0100 @@ -14,8 +14,9 @@ from common_test import read_file, write_file import os -from apparmor.aa import (check_for_apparmor, get_interpreter_and_abstraction, create_new_profile, +import apparmor.aa # needed to set global vars in some tests +from apparmor.aa import (check_for_apparmor, get_output, get_reqs, get_interpreter_and_abstraction, create_new_profile, get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir, parse_profile_start, parse_profile_data, separate_vars, store_list_var, write_header, var_transform, serialize_parse_profile_start) @@ -72,6 +73,26 @@ mounts = write_file(self.tmpdir, 'mounts', self.MOUNTS_WITH_SECURITYFS % self.tmpdir) self.assertEqual('%s/security/apparmor' % self.tmpdir, check_for_apparmor(filesystems, mounts)) +class AATest_get_output(AATest): +tests = [ +(['./fake_ldd', '/AATest/lib64/libc-2.22.so'], (0, [' /AATest/lib64/ld-linux-x86-64.so.2 (0x556858473000)', ' linux-vdso.so.1 (0x7ffe98912000)'] )), +(['./fake_ldd', '/tmp/aa-test-foo'],(0, ['not a dynamic executable'] )), +(['./fake_ldd', 'invalid'], (1, [''] )), # stderr is not part of output +] +def _run_test(self, params, expected): +self.assertEqual(get_output(params), expected) + +class AATest_get_reqs(AATest): +tests = [ +('/AATest/bin/bash',['/AATest/lib64/libreadline.so.6', '/AATest/lib64/libtinfo.so.6', '/AATest/lib64/libdl.so.2', '/AATest/lib64/libc.so.6', '/AATest/lib64/ld-linux-x86-64.so.2']), +('/tmp/aa-test-foo',[]), +] + +def _run_test(self, params, expected): +apparmor.aa.cfg['settings']['ldd'] = './fake_ldd' + +self.assertEqual(get_reqs(params), expected) + class AaTest_create_new_profile(AATest): tests = [ # file content expected interpreterexpected abstraction (besides 'base') --- utils/test/fake_ldd 2016-02-01 19:22:59.738008345 +0100 +++ utils/test/fake_ldd 2016-02-01 19:23:58.781645883 +0100 @@ -0,0 +1,56 @@ +#!/usr/bin/python3 + +import sys + +if len(sys.argv) != 2: +raise Exception('wrong number of arguments in fake_ldd') + +if sys.argv[1] == '/AATest/bin/bash': +print('linux-vdso.so.1 (0x7ffcf97f4000)') +print('libreadline.so.6 => /AATest/lib64/libreadline.so.6 (0x7f2c41324000)') +print('libtinfo.so.6 => /AATest/lib64/libtinfo.so.6 (0x7f2c410f9000)') +print('libdl.so.2 => /AATest/lib64/libdl.so.2 (0x7f2c40ef5000)') +print('libc.so.6 => /AATest/lib64/libc.so.6 (0x7f2c40b5)') +print('/AATest/lib64/ld-linux-x86-64.so.2 (0x55782c473000)') + +elif sys.argv[1] == '/AATest/lib64/ld-2.22.so': +print('linux-vdso.so.1 (0x7ffcf97f4000)') + +elif sys.argv[1] == '/AATest/lib64/libc-2.22.so': +print('/AATest/lib64/ld-linux-x86-64.so.2 (0x556858473000)') +print('linux-vdso.so.1 (0x7ffe98912000)') + +elif sys.argv[1] == '/AATest/lib64/libdl.so.2': +print('linux-vdso.so.1 (0x7ffec2538000)') +print('libc.so.6 => /AATest/lib64/libc.so.6 (0x7f8865346000)') +print('/AATest/lib64/ld-linux-x86-64.so.2 (0x560c3bcee000)') + +elif sys.argv[1] == '/AATest/lib64/libtinfo.so.6': +print('linux-vdso.so.1 (0x7fff30518000)') +print('libc.so.6 => /AATest/lib64/libc.so.6 (0x7fb6f2ea3000)') +print('/AATest/lib64/ld-linux-x86-64.so.2 (0x5631fe8d3000)') + +elif sys.argv[1] == '/AATest/lib64/libreadline.so.6': +print('linux-vdso.so.1 (0x7ffcb5b62000)') +print('libtinfo.so.6 => /AATest/lib64/libtinfo.so.6 (0x7f2a4ed07000)') +
Re: [apparmor] [Merge] lp:~sdeziel/apparmor-profiles/unbound-remove-unneeded-caps into lp:apparmor-profiles
Review: Approve Thanks! Acked and merged ;-) -- https://code.launchpad.net/~sdeziel/apparmor-profiles/unbound-remove-unneeded-caps/+merge/284673 Your team AppArmor Developers is subscribed to branch lp:apparmor-profiles. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] logparser.py: do sanity check for all file events
Hello, most probably-file log events can also be network events. Therefore check for request_mask in all events, not only file_perm, file_inherit and (from the latest bugreport) file_receive. References: https://bugs.launchpad.net/apparmor/+bug/1540562 I propose this patch for trunk, 2.10 and 2.9. [ 68-logparser-check-sanity-of-all-file-events.diff ] --- utils/apparmor/logparser.py 2016-02-01 21:31:56.439302830 +0100 +++ utils/apparmor/logparser.py 2016-02-01 22:38:40.519637878 +0100 @@ -300,10 +300,10 @@ 'rename_dest', 'unlink', 'rmdir', 'symlink_create', 'link', 'sysctl', 'getattr', 'setattr', 'xattr'] ): -# for some reason, we get file_perm and file_inherit log events without request_mask, see -# https://bugs.launchpad.net/apparmor/+bug/1466812/ and https://bugs.launchpad.net/apparmor/+bug/1509030 +# for some kernel-side reason, we get file-related log events without request_mask, see +# https://bugs.launchpad.net/apparmor/+bug/1466812/, https://bugs.launchpad.net/apparmor/+bug/1509030 and https://bugs.launchpad.net/apparmor/+bug/1540562 # request_mask can also be '', see https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1525119 -if e['operation'] in ['file_perm', 'file_inherit'] and not e['request_mask']: +if not e['request_mask']: self.debug_logger.debug('UNHANDLED (missing request_mask): %s' % e) return None Regards, Christian Boltz -- Linux - und dein PC macht nie wieder blau. signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] adjust unbound profile for openSUSE
Hello, I just replaced my self-made unbound profile with the latest Ubuntu profile. It needs exactly one change [1] to work on openSUSE, and that's the pid file location. Additionally, I prefer to use abstractions/openssl instead of /etc/ssl/openssl.cnf. As a sidenote - the capabilities fowner, fsetid and sys_chroot are not needed on openSUSE. sys_chroot obviously depends on the confi. I wonder about the difference for fowner and fsetid (they were added by Simon's patch, so I assume they are needed on Ubuntu ;-) - are those also depending on the config, or is there some other difference? === modified file 'ubuntu/16.04/usr.sbin.unbound' --- ubuntu/16.04/usr.sbin.unbound 2016-01-12 21:30:36 + +++ ubuntu/16.04/usr.sbin.unbound 2016-01-31 16:45:45 + @@ -5,6 +5,7 @@ /usr/sbin/unbound { #include #include + #include # needlessly chown'ing the PID, for details see: # https://www.nlnetlabs.nl/bugs-script/show_bug.cgi?id=734 @@ -37,11 +39,9 @@ audit deny /var/lib/unbound/**/unbound_control.{key,pem} rw, audit deny /var/lib/unbound/**/unbound_server.key w, - /etc/ssl/openssl.cnf r, - /usr/sbin/unbound mr, - /{,var/}run/unbound.pid rw, + /{,var/}run/{unbound/,}unbound.pid rw, # Unix control socket /{,var/}run/unbound.ctl rw, Regards, Christian Boltz [1] well, the two "deny capability" rules also cause failures, but that's a known issue and will fix itsself when openSUSE gets the next unbound release -- Inactive upstream is often just a sign of well engineered software, which works for many years without continuous bugfixing and which is feature complete. Something the CADT crowd of today probably just cannot imagine anymore. [Stefan Seyfried in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] adjust unbound profile for openSUSE
Hello, Am Montag, 1. Februar 2016, 15:23:46 CET schrieb Simon Deziel: > On 2016-01-31 11:56 AM, Christian Boltz wrote: > > As a sidenote - the capabilities fowner, fsetid and sys_chroot are > > not needed on openSUSE. > > sys_chroot is needed but fowner/fsetid should be dropped. I just > tested this on Ubuntu and they are not used any more. I audited all > the other capabilities and they are used by Unbound 1.5.7. > > The fowner/fsetid are probably leftovers from the initial profile I > had created for Ubuntu Precise. > > Should I send a follow-up patch or you'll drop fowner/fsetid when > committing? Your mail was a bit late (I already commited my patch), so please send a patch or merge request ;-) Regards, Christian Boltz -- > [1] Schmerzen wg. einer Zerrung > -- > Nicht alles, was hinkt, ist ein Vergleich. In manchen Fällen ist es auch ein David Haller... *SCNR* [> David Haller und Mario van der Linde in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Add tests for aa.py get_output() and get_reqs()
ith('/tmp/aa-test-'): # test file generated by test-aa.py +print('not a dynamic executable') + +elif sys.argv[1] == 'TEMPLATE': +print('') +print('') +print('') + +else: +raise Exception('unknown parameter in fake_ldd: %s' % sys.argv[1]) Regards, Christian Boltz -- "Der wahrscheinlich ärgerlichste Aspekt eines Computerprogrammes ist die Art und Weise, in der es auf Ihre Fehler reagiert" [L. Lamport, LaTeX-Handbuch] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Prevent crash caused by by serialize_profile_from_old_profile()
Hello, Am Dienstag, 22. Dezember 2015 schrieb Christian Boltz: > if a profile file contains multiple profiles and one of those profiles > contains a rule managed by a *Ruleset class, > serialize_profile_from_old_profile() crashes with an AttributeError. ... > [ 43-prevent-crash-by-serialize_profile_from_old_profile.diff ] To answer Steve's comment at the "right" place: | (Yes, I know I missed 43. It requires a little more thought/effort | than I have time for at the moment.) Well, the TL;DR version is that we can choose between a) catch errors in serialize_profile_from_old_profile() and handle them in a as-sane-as-possible way (that's what this patch does) b) crash aa-logprof if serialize_profile_from_old_profile() fails The long version: serialize_profile_from_old_profile() has some known breakage [1] and needs a full rewrite, but that needs quite some work and thought (and will _really_ be a patch that requires "a little more thought/effort" ;-) (I have an idea how to implement it, but I'm not yet sure if and how good it will work.) That said - I'll probably do more cleanup before touching serialize_profile_from_old_profile() because the cleanup will hopefully make it easier to rewrite this beast ;-) So: Yes, I know this patch isn't perfect, but IMHO it is an improvement, but for now, I'd prefer to display an error message with a hint to use "view (C)lean changes" over crashing aa-logprof ;-) The good thing is that this will only happen in corner cases (one file containing multiple profiles), so users won't see that message too often. On the long term, serialize_profile_from_old_profile() needs to be replaced by something that is easier to maintain, has less bugs and doesn't crash ;-) Regards, Christian Boltz [1] without looking at the bugtracker or the code, I know that - it fails to handle hats and child profiles (the whole hats will appear as removed lines in the diff, which makes the diff view useless for them) - it crashes if a file contains more than one (main) profile (prevented by this patch) - it sometimes adds new rules at not-so-perfect places or, to sum it up, it feels like the (random!) sig ;-) -- wodim is based on a cdrecord from September 2004 with additional bugs added by Debian and with the DVD support code ripped off and replaced by something that works on weekends wit full-moon. [Jörg Schilling in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] AARE: escape exclamation mark
Hello, '!' is a reserved symbol and needs to be escaped in AARE. Sidenote: This bug went unnoticed since years. I noticed it on a server with AppArmor 2.8.4 where aa-logprof created an invalid profile after someone uploaded a file with an explanation mark in the filename. I propose the aa.py part of this patch for trunk, 2.10 and 2.9. aare.py only exists in trunk, therefore this part is trunk-only. [ 64-aare-escape-exclamation-mark.diff ] --- utils/apparmor/aa.py2016-01-07 21:50:43.035415000 +0100 +++ utils/apparmor/aa.py2016-01-20 20:16:19.478996074 +0100 @@ -1205,6 +1205,7 @@ detail = detail.replace('*', '\*') detail = detail.replace('{', '\{') detail = detail.replace('}', '\}') +detail = detail.replace('!', '\!') # Give Execute dialog if x access requested for something that's not a directory # For directories force an 'ix' Path dialog --- utils/apparmor/aare.py 2015-12-27 16:06:12.635071663 +0100 +++ utils/apparmor/aare.py 2016-01-20 19:53:40.902819126 +0100 @@ -83,7 +83,7 @@ def convert_expression_to_aare(expression): '''convert an expression (taken from audit.log) to an AARE string''' -aare_escape_chars = ['\\', '?', '*', '[', ']', '{', '}', '"'] +aare_escape_chars = ['\\', '?', '*', '[', ']', '{', '}', '"', '!'] for char in aare_escape_chars: expression = expression.replace(char, '\\' + char) Regards, Christian Boltz -- > I'm running SUPER. I've a USB mouse attached. The mouse is too > sensitive, the cursor is moving too fast which is out of my control. Even the mouse is performance enhanced, wow! [> Qingjia Zhu and Peter Flodin in opensuse] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] lp:~intrigeri/apparmor-profiles/pidgin-vs-gstreamer-1.6 into lp:apparmor-profiles
Review: Approve I never used pidgin, but the changes look sane and valid, therefore merged into r154. I tend to think that all gstreamer-related rules should go into abstractions/gstreamer, but a) that's something for a separate cleanup patch and b) I don't really know gstreamer, so I'm not sure if all applications using gstreamer also need those permissions (if yes, moving to the abstraction makes sense) -- https://code.launchpad.net/~intrigeri/apparmor-profiles/pidgin-vs-gstreamer-1.6/+merge/277499 Your team AppArmor Developers is subscribed to branch lp:apparmor-profiles. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] lp:~cameronnemo/apparmor/gnome-abstraction into lp:apparmor
It seems your merge request somehow got lost, sorry for that! I'd like to see your added line + owner @{HOME}/.config/gtk-3.0/** r, merged with the existing owner @{HOME}/.config/gtk-2.0/** r, which would result in owner @{HOME}/.config/gtk-[23].0/** r, If you expect the .0 to change, maybe also [23].[0-9]. BTW: I noticed owner @{HOME}/.config/gtk-2.0/gtkfilechooser.ini* rw, Is writing that file also needed with gtk 3? If yes, please also change it to [23].0 or [23].[0-9]. -- https://code.launchpad.net/~cameronnemo/apparmor/gnome-abstraction/+merge/261320 Your team AppArmor Developers is requested to review the proposed merge of lp:~cameronnemo/apparmor/gnome-abstraction into lp:apparmor. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 1/3] libapparmor: Remove incorrect statement in aa_change_profile man page
Hello, Am Dienstag, 26. Januar 2016 schrieb Tyler Hicks: > The statement was meant to convey the difference between > aa_change_hat() and aa_change_profile(). Unfortunately, it read as if > there was something preventing a program from using > aa_change_profile() twice to move from profile A to profile B and > back to profile A, even if profiles A and B contained the necessary > rules. > > Signed-off-by: Tyler Hicks <tyhi...@canonical.com> > Reported-by: Seth Arnold <seth.arn...@canonical.com> > --- > libraries/libapparmor/doc/aa_change_profile.pod | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/libraries/libapparmor/doc/aa_change_profile.pod > b/libraries/libapparmor/doc/aa_change_profile.pod index > e5ac0be..6457c33 100644 > --- a/libraries/libapparmor/doc/aa_change_profile.pod > +++ b/libraries/libapparmor/doc/aa_change_profile.pod > @@ -40,14 +40,13 @@ An AppArmor profile applies to an executable > program; if a portion of the program needs different access > permissions than other portions, the program can "change profile" to > a different profile. To change into a new profile, it can use the > aa_change_profile() function to do so. It passes -in a pointer to the > I to transition to. Transitioning to another -profile via > aa_change_profile() is permanent and the process is not -permitted to > transition back to the original profile. Confined programs -wanting > to use aa_change_profile() need to have rules permitting changing -to > the named profile. See apparmor.d(8) for details. > +in a pointer to the I to transition to. Confined programs > wanting to +use aa_change_profile() need to have rules permitting What about mentioning the rule name to make things clear? ... need to have *change_profile* rules permitting... > changing to the named +profile. See apparmor.d(8) for details. > > If a program wants to return out of the current profile to the > -original profile, it should use aa_change_hat(2) instead. > +original profile, it may use aa_change_hat(2). Otherwise, the two > profiles must +have rules permitting changing between the two > profiles. Same here - ...must have *change_profile* rules permitting... With or without that changed, Acked-by: Christian Boltz <appar...@cboltz.de> for trunk, 2.10 and 2.9 (they all have the same aa_change_profile.pod and therefore all need this fix) The other patches in this series should also be applied to the 2.9 and 2.10 branch once they are acked. However, I'll leave someone else (who knows the technical details of aa_change_profile better) review them ;-) Regards, Christian Boltz -- > rpmdb: PANIC: fatal region error detected; run recovery Du wohnst nicht zufällig in Bielefeld? [> Cornelia Böttge und Michael Raab in opensuse-de] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] print test filenames in 'make check' and 'make coverage'
Hello, $subject. This makes it easier to find the file that contains a failing test. I propose this patch for trunk and 2.10. [ 65-utils-test-Makefile-print-test-name.diff ] === modified file 'utils/test/Makefile' --- utils/test/Makefile 2015-11-18 12:44:45 + +++ utils/test/Makefile 2016-01-24 21:46:09 + @@ -62,10 +62,10 @@ rm -rf __pycache__/ .coverage htmlcov check: __libapparmor - export PYTHONPATH=$(PYTHONPATH) ; export LD_LIBRARY_PATH=$(LD_LIBRARY_PATH) ; $(foreach test, $(wildcard test-*.py), $(call pyalldo, $(test))) + export PYTHONPATH=$(PYTHONPATH) ; export LD_LIBRARY_PATH=$(LD_LIBRARY_PATH) ; $(foreach test, $(wildcard test-*.py), echo ; echo === $(test) === ; $(call pyalldo, $(test))) .coverage: $(wildcard ../aa-* ../apparmor/*.py test-*.py) __libapparmor - export PYTHONPATH=$(PYTHONPATH) ; export LD_LIBRARY_PATH=$(LD_LIBRARY_PATH); $(COVERAGE_IGNORE_FAILURES_CMD) ; $(foreach test, $(wildcard test-*.py), $(PYTHON) -m coverage run --branch -p $(test); ) + export PYTHONPATH=$(PYTHONPATH) ; export LD_LIBRARY_PATH=$(LD_LIBRARY_PATH); $(COVERAGE_IGNORE_FAILURES_CMD) ; $(foreach test, $(wildcard test-*.py), echo ; echo === $(test) === ; $(PYTHON) -m coverage run --branch -p $(test); ) $(PYTHON) -m coverage combine coverage: .coverage Regards, Christian Boltz -- Software and cathedrals are much the same - first we build them, then we pray. [Sam Redwine] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] parser: Allow AF_UNSPEC family in network rules
Hello, Am Mittwoch, 17. Februar 2016, 22:51:01 CET schrieb Tyler Hicks: > https://launchpad.net/bugs/1546455 > > Don't filter out AF_UNSPEC from the list of valid protocol families so > that the parser will accept rules such as 'network unspec,'. > > There are certain syscalls, such as socket(2), where the LSM hooks are > called before the protocol family is validated. In these cases, > AppArmor was emitting denials even though socket(2) will eventually > fail. There may be cases where AF_UNSPEC sockets are accepted and we > need to make sure that we're mediating those appropriately. Whenever you change something in the parser simple_tests or libapparmor test_multi testsuite, please also run the utils testsuite which also runs against those testcases. Long story short: Your addition of the 'unspec' keyword breaks the utils testsuite. To un-break it, we need... [patch] Add 'unspec' to NetworkRule keyword list I propose this patch for trunk and 2.10 (assuming the parser patch for AF_UNSPEC gets applied to both) [ 71-network-unspec.diff ] --- utils/apparmor/rule/network.py 2016-02-12 22:11:21.078578660 +0100 +++ utils/apparmor/rule/network.py 2016-02-18 18:09:26.482597013 +0100 @@ -27,7 +27,7 @@ network_domain_keywords = [ 'unix', 'inet', 'ax25', 'ipx', 'appletalk', 'netrom', 'bridge', 'atmpvc', 'x25', 'inet6', 'rose', 'netbeui', 'security', 'key', 'netlink', 'packet', 'ash', 'econet', 'atmsvc', 'rds', 'sna', 'irda', 'pppox', 'wanpipe', 'llc', 'can', 'tipc', 'bluetooth', 'iucv', 'rxrpc', 'isdn', 'phonet', - 'ieee802154', 'caif', 'alg', 'nfc', 'vsock', 'mpls', 'ib' ] + 'ieee802154', 'caif', 'alg', 'nfc', 'vsock', 'mpls', 'ib', 'unspec' ] network_type_keywords = ['stream', 'dgram', 'seqpacket', 'rdm', 'raw', 'packet'] network_protocol_keywords = ['tcp', 'udp', 'icmp'] Feel free to commit this together with your parser patch ;-) Regards, Christian Boltz -- > Bei Mutt oder Gnus landet ohnehin jeder früher oder später, > Du kannst also abkürzen gleich damit anfangen. ;) Nein, diese Aussage ist schlicht falsch. Denn in einem kleinen Dorf im Nordwesten Galliens... [> Andreas Kneib und Thomas Hertweck in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Add simple_tests/profile/profile_ns_bad8.sd to utils test exception list
Hello, parser/tst/simple_tests/profile/profile_ns_bad8.sd was added in r3376 (trunk) / r3312 (2.10 branch) and contains the profile name ':ns/t' which misses the terminating ':' for the namespace. Unfortunately the tools don't understand namespaces yet and just use the full profile name. This also means this test doesn't fail as expected when tested against the utils code. This patch adds profile_ns_bad8.sd to the exception list of test-parser-simple-tests.py. I propose this patch for trunk and 2.10. [ 72-parser-tests-namespace-profile-name.diff ] === modified file 'utils/test/test-parser-simple-tests.py' --- utils/test/test-parser-simple-tests.py 2015-12-27 00:20:37 + +++ utils/test/test-parser-simple-tests.py 2016-02-18 22:25:44 + @@ -125,6 +125,7 @@ 'profile/flags/flags_bad_debug_3.sd', 'profile/flags/flags_bad_debug_4.sd', 'profile/simple_bad_no_close_brace4.sd', +'profile/profile_ns_bad8.sd', # 'profile :ns/t' without terminating ':' 'ptrace/bad_05.sd', # actually contains a capability rule with invalid (ptrace-related) keyword 'ptrace/bad_06.sd', # actually contains a capability rule with invalid (ptrace-related) keyword 'ptrace/bad_10.sd', # peer with invalid regex Regards, Christian Boltz -- If something is red you should always worry. That's way it is red. [Thorsten Kukuk] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] dovecot-lda profile: allow tempfiles and executing sendmail
Hello, Am Freitag, 19. Februar 2016, 15:54:02 CET schrieb Seth Arnold: > The contents of the profile are fine but there's a _huge_ amount of > trailing spaces that are being added here. It seems that was caused by copy from Konsole. I have no idea why it happens, but it isn't the first time - but this time I didn't notice it in the mail :-/ Unfortunately it only happens sometimes, which makes it hard to find a good reproducer for a bugreport. My *.diff file didn't have the extra spaces, so there was nothing to fix ;-) Regards, Christian Boltz -- > Write the code like you are going to lose your memory in six months. Most people would say I write code like I've already lost my mind. Is that the same thing? [Randal L. Schwartz] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Prevent crash caused by by serialize_profile_from_old_profile()
Hello, Am Freitag, 19. Februar 2016, 16:12:23 CET schrieb Seth Arnold: > On Tue, Dec 22, 2015 at 12:17:40AM +0100, Christian Boltz wrote: > > Therefore this patch wraps the serialize_profile_from_old_profile() > > call in try/except. If it fails, the diff will include an error > > message and recommend to use 'View Changes b/w (C)lean profiles' > > instead, which is known to work. > > What does "b/w" mean? beziehungsweise? between (no idea why it's shortened this way, I just copied the original option name from aa-logprof to stay consistent) > > Note: I know using an error message as 'newprofile' isn't an usual > > way to display an error message, but I found it more intuitive than > > displaying it as a warning (without $PAGER). > > > > > > References: > > https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139 > > > > > > I propose this patch for trunk and 2.10 > > (2.9 "just" displays a wrong diff, but doesn't crash.) > > Well, uh, begrudging ack for trunk and 2.10. Not-crashing is better > than crashing.. Indeed ;-) I already wrote that the long-term solution is a complete rewrite of serialize_profile_from_old_profile(), but that has to wait until we have rule classes for everything. > Acked-by: Seth Arnold <seth.arn...@canonical.com> Regards, Christian Boltz -- I guess part of the question then becomes what the ultimate purpose of the board is. I've always been under the impression that they guide the project hendersj,"What's the meaning of life" is a much less complex question :-D [from #opensuse-project] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Don't store exec modes in transtions[]
Hello, exec choices are stored in transitions[], but that's never used (and I don't see a need for it), therefore stop storing it. [ 73-exec-transitions.diff ] === modified file 'utils/apparmor/aa.py' --- utils/apparmor/aa.py2016-02-20 12:32:36 + +++ utils/apparmor/aa.py2016-02-21 13:50:24 + @@ -1205,7 +1205,6 @@ context_new = context_new + '^%s' % hat context_new = context_new + ' -> %s' % exec_target -# ans_new = transitions.get(context_new, '') # XXX ans meant here? combinedmode = set() combinedaudit = set() ## Check return Value Consistency @@ -1415,7 +1414,6 @@ exec_mode = exec_mode - (apparmor.aamode.AA_EXEC_UNSAFE | AA_OTHER(apparmor.aamode.AA_EXEC_UNSAFE)) else: ans = 'INVALID' -transitions[context_new] = ans regex_options = re.compile('CMD_(ix|px|cx|nx|pix|cix|nix)') if regex_options.search(ans): Regards, Christian Boltz -- > Ich moechte gern einige User die ihre Mails ueber einen Mailserver > (sendmail bevorzugt, postfix auch moeglich) scannen. Dafür reicht ein Kopierer. Hosen runter, User draufsetzen und "Copy" drücken! [> Ralf Thomas und Sandy Drobic in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Drop unused function split_name() in aa.py
Hello, $subject. [ 74-drop-unused-split_name.diff ] === modified file 'utils/apparmor/aa.py' --- utils/apparmor/aa.py2016-02-20 12:32:36 + +++ utils/apparmor/aa.py2016-02-21 14:43:21 + @@ -4317,12 +4317,6 @@ else: return '%s^%s' % (name1, name2) -def split_name(name): -names = name.split('^') -if len(names) == 1: -return name, name -else: -return names[0], names[1] def commonprefix(new, old): match = re.search(r'^([^\0]*)[^\0]*(\0\1[^\0]*)*$', '\0'.join([new, old])) if match: Regards, Christian Boltz -- NO' and 'YES' are short words which need long thoughts. Most of the troubles in life are the result of saying 'YES' too soon or 'NO' too late !!! signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Make sure 'x' log events always come with type 'exec'
Hello, according to a discussion with John on IRC, denied_mask="x" can only happen for 'exec' log events. This patch raises an exception if John is wrong ;-) [ 75-x-but-not-exec-exception.diff ] === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py2016-02-21 15:43:58.021985441 +0100 +++ utils/apparmor/aa.py2016-02-21 16:06:41.744595751 +0100 @@ -1210,6 +1210,8 @@ if mode & str_to_mode('x'): if os.path.isdir(exec_target): raise AppArmorBug('exec permissions requested for directory %s. This should not happen - please open a bugreport!' % exec_target) +elif typ != 'exec': +raise AppArmorBug('exec permissions requested for %i(exec_target)s, but mode is %(mode)s instead of exec. This should not happen - please open a bugreport!' % {'exec_target': exec_target, 'mode':mode}) else: do_execute = True 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?"] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] handle_binfmt: resolve symlinks in library paths
Hello, $subject. This should happen rarely, but nevertheless it can happen - and since AppArmor needs the symlink target in the profile, we have to resolve any symlink. [ 76-handle_binfmt-resolve-symlinks.diff ] === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py2016-02-21 17:14:28.444520585 +0100 +++ utils/apparmor/aa.py2016-02-21 16:06:41.744595751 +0100 @@ -386,6 +388,7 @@ reqs = get_reqs(path) while reqs: library = reqs.pop() +library = get_full_path(library) # resolve symlinks if not reqs_processed.get(library, False): if get_reqs(library): reqs += get_reqs(library) Regards, Christian Boltz -- The "Well" was referring to my role: as a product manager, I am not even entitled to review the "validity of implementation details", ... I sometime cannot resist and do nevertheless, ...;-) [Matthias G. Eckermann in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] handle_binfmt: resolve symlinks in library paths
Hello, Am Montag, 22. Februar 2016, 00:02:09 CET schrieb Kshitij Gupta: > On Sun, Feb 21, 2016, Christian Boltz <appar...@cboltz.de> wrote: > > $subject. > > > > This should happen rarely, but nevertheless it can happen - and > > since > > AppArmor needs the symlink target in the profile, we have to resolve > > any symlink. > > > > > > [ 76-handle_binfmt-resolve-symlinks.diff ] > > > > === modified file ./utils/apparmor/aa.py > > --- utils/apparmor/aa.py2016-02-21 17:14:28.444520585 +0100 > > +++ utils/apparmor/aa.py2016-02-21 16:06:41.744595751 +0100 > > @@ -386,6 +388,7 @@ > > > > reqs = get_reqs(path) > > > > while reqs: > > library = reqs.pop() > > > > +library = get_full_path(library) # resolve symlinks > > How about inlining the get_full_path with the pop? I know it would make the code shorter, but I prefer an additional line if it makes it more readable ;-) > Also, is the comment above adding any value and worth it? I think so ;-) People might wonder why get_full_path() is called because get_reqs() only returns full library paths (the regexes only match '/.*'), so explaining that it's about symlinks makes sense IMHO. > Acked-by: Kshitij Gupta <kgupta8...@gmail.com> Thanks for the review! Regards, Christian Boltz -- general rule: if Olaf reports a bug, it is a valid bug. [Olaf Hering while reopening https://bugzilla.novell.com/show_bug.cgi?id=168595] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Make sure 'x' log events always come with type 'exec'
Hello, Am Sonntag, 21. Februar 2016, 23:53:40 CET schrieb Kshitij Gupta: > On Sun, Feb 21, 2016 at 9:48 PM, Christian Boltz wrote: > > according to a discussion with John on IRC, denied_mask="x" can only > > happen for 'exec' log events. This patch raises an exception if John > > is wrong ;-) > > > > > > [ 75-x-but-not-exec-exception.diff ] > > > > === modified file ./utils/apparmor/aa.py > > --- utils/apparmor/aa.py2016-02-21 15:43:58.021985441 +0100 > > +++ utils/apparmor/aa.py2016-02-21 16:06:41.744595751 +0100 > > +elif typ != 'exec': > > +raise AppArmorBug('exec permissions > > requested for %i(exec_target)s, but mode is %(mode)s instead of > > exec. This > Is that "%i(exec_target)s: above containing the "%i" what you were > aiming for? Nice catch - it should be %(...), not %i(...) ;-) Updated patch: [ 75-x-but-not-exec-exception.diff ] === modified file ./utils/apparmor/aa.py --- utils/apparmor/aa.py2016-02-21 15:43:58.021985441 +0100 +++ utils/apparmor/aa.py2016-02-21 16:06:41.744595751 +0100 @@ -1210,6 +1210,8 @@ if mode & str_to_mode('x'): if os.path.isdir(exec_target): raise AppArmorBug('exec permissions requested for directory %s. This should not happen - please open a bugreport!' % exec_target) +elif typ != 'exec': +raise AppArmorBug('exec permissions requested for %(exec_target)s, but mode is %(mode)s instead of exec. This should not happen - please open a bugreport!' % {'exec_target': exec_target, 'mode':mode}) else: do_execute = True Regards, Christian Boltz -- There is only so much everybody can do. We suffer from hour-shortage on the day I guess :)[Dominique Leuenberger in opensuse-factory] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Change log_dict to use profile_storage() and simplify log translation
Hello, Am Montag, 22. Februar 2016, 02:07:42 CET schrieb Kshitij Gupta: > On Fri, Dec 25, 2015 at 8:57 PM, Christian Boltz wrote: > > [ 45-change-log_dict-to-profile_storage.diff ] > > > > === modified file ./utils/apparmor/aa.py > > --- utils/apparmor/aa.py2015-12-25 15:10:26.931746576 +0100 > > +++ utils/apparmor/aa.py2015-12-25 15:12:17.323014813 +0100 > > for ruletype in ruletypes: > > -# XXX aa-mergeprof also has this code - if you > > change it, keep aa-mergeprof in sync! > > -for rule_obj in > > log_obj[profile][hat][ruletype].rules: - > > -if rule_obj.log_event != aamode: # XXX > > does it really make sense to handle enforce and complain mode > > changes in different rounds? > > -continue > > +for rule_obj in > > log_dict[aamode][profile][hat][ruletype].rules: > > +# XXX aa-mergeprof also has this code - if > > you change it, keep aa-mergeprof in sync! > > sure it still does after the above change? Plus what does the *this > code* even refer to? To the code starting at this comment, until... > > if is_known_rule(aa[profile][hat], > > ruletype, > > > > rule_obj): > > continue > > > > @@ -1789,8 +1762,8 @@ > > > > # END of code (mostly) shared with aa-mergeprof ... this comment. The patch didn't change anything in between, so it should still be in sync. For the records: remaining pending "old" patches are: - 46-serialize_profile_from_old_profile-fix-wrong-access-to-write_prof_data.msg Fix wrong usage of write_prof_data in serialize_profile_from_old_profile() - the DBUS series (except 1/9) - document-empty-quotes-in-variables.msg Document empty quotes ("") as empty value of a variable - profiles-smbd-cap-sys_admin.msg smbd profile needs capability sys_admin Regards, Christian Boltz -- Ich bezweifle, dass jeder 1984 gelesen hat. Denn dann wüsten die Kommentatoren, dass das Gros der Bürger gar nicht überwacht, sondern einfach nur verdummt wurde. Privatfernsehen wurde übrigens in Deutschland zum 1. Januar 1984 eingeführt. [Peter Brülls zu http://blog.koehntopp.de/archives/3237-Kleine-Kinder-spielen-verstecken.html] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Fix wrong usage of write_prof_data in serialize_profile_from_old_profile()
Hello, Am Montag, 22. Februar 2016, 02:16:28 CET schrieb Kshitij Gupta: > On Sat, Dec 26, 2015 at 9:07 PM, Christian Boltz wrote: > > write_prof_data[hat] is correct (it only contains one profile, see > > also bug 1528139), write_prof_data[profile][hat] is not and returns > > an empty (sub)hasher. > > Hmm... > > Reading the comments near the initialisation of write_prof_data: > # XXX this will explode if a file contains multiple profiles, see > https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139 > # XXX fixing this needs lots of write_prof_data[hat] -> > write_prof_data[profile][hat] changes (and of course also a change in > the calling code) > > So, basically a part of the logic below is correct in that it accessed > the hat from the profile, which will again need to be added once > write_prof_data supports multiple profiles I'm guessing? Well, it's a very small part of the function that uses write_prof_data[profile][hat]. In theory that would be better, but doing this everywhere would also mean we have to do lots of changes in the function and also in the calling code. serialize_profile_from_old_profile() is (IMHO) broken beyond repair, so I don't want to spend too much time on it - but this patch is needed to a) fix the rare places where [profile][hat] is accidently used and b) get towards a more strict profile_storage() > Why not copy write_prof_data[hat] to write_prof_data[profile][hat] for > the time-being? > That might seem hack-ish though. As discussed on IRC some days ago, it would also lead to funny[tm] problems if someone has a hat named ^capability or ^network or something like that ;-) So - how do you like the patch as is? ;-) > > This affects RE_PROFILE_START and RE_PROFILE_BARE_FILE_ENTRY. > > > > I propose this patch for trunk, 2.10 and 2.9. > > > > > > [ > > 46-serialize_profile_from_old_profile-fix-wrong-access-to-write_prof > > _data.diff ] > > > > === modified file ./utils/apparmor/aa.py > > --- utils/apparmor/aa.py2015-12-06 19:36:00.818745321 +0100 > > +++ utils/apparmor/aa.py2015-12-08 18:59:09.625261162 +0100 > > @@ -3718,7 +3718,7 @@ > > > > if RE_PROFILE_START.search(line): > > (profile, hat, attachment, flags, in_contained_hat, > > > > correct) = serialize_parse_profile_start( > > -line, prof_filename, None, profile, hat, > > write_prof_data[profile][hat]['profile'], > > write_prof_data[profile][hat]['external'], correct) > > +line, prof_filename, None, profile, hat, > > write_prof_data[hat]['profile'], write_prof_data[hat]['external'], > > correct)> > > if not write_prof_data[hat]['name'] == profile: > > correct = False > > > > @@ -3954,7 +3954,7 @@ > > > > if matches[0]: > > audit = mode > > > > -path_rule = > > write_prof_data[profile][hat][allow]['path'][ALL] > > +path_rule = > > write_prof_data[hat][allow]['path'][ALL] > > > > if path_rule.get('mode', set()) & mode and \ > > > > (not audit or path_rule.get('audit', set()) & > > audit) > > > > and \ > > > > path_rule.get('file_prefix', set()): Regards, Christian Boltz -- In A.D. 1582 Pope Gregory XIII [...] changed the rules about calculating leap years to account for this. Similarly, in A.D. 2013 Rockchip hardware engineers found that the new Gregorian calendar still contained flaws, and that the month of November should be counted up to 31 days instead. [https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/ commit/?id=f076ef44a44d02ed91543f820c14c2c7dff53716] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] fix missing import in 2.9 test-aa.py
Hello, since 2.9 r2978, test-aa.py fails thanks to a missing import of 'var_transform'. This patch adds the missing import. === modified file 'utils/test/test-aa.py' --- utils/test/test-aa.py 2015-12-12 12:02:06 + +++ utils/test/test-aa.py 2016-03-01 20:30:11 + @@ -16,7 +16,7 @@ import tempfile from common_test import read_file, write_file -from apparmor.aa import check_for_apparmor, get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir, parse_profile_start, separate_vars, store_list_var, write_header, serialize_parse_profile_start +from apparmor.aa import check_for_apparmor, get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir, parse_profile_start, separate_vars, store_list_var, write_header, serialize_parse_profile_start, var_transform from apparmor.common import AppArmorException, AppArmorBug class AaTestWithTempdir(AATest): 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] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] parser: Fix dependency in Makefile
Hello, Am Mittwoch, 6. April 2016, 11:58:42 CEST schrieb Tyler Hicks: > parser 'make install' failed if 'make' wasn't run before. This patch > adds the missing dependency 'install-indep: indep'. That's what we get for copy programming ;-) > Signed-off-by: Tyler Hicks <tyhi...@canonical.com> > --- > parser/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/parser/Makefile b/parser/Makefile > index 2b22ea9..35c9bd4 100644 > --- a/parser/Makefile > +++ b/parser/Makefile > @@ -368,7 +368,7 @@ install-arch: $(INSTALLDEPS) > install -m 755 ${TOOLS} $(DESTDIR)/sbin > > .PHONY: install-indep > -install-indep: > +install-indep: indep > install -m 755 -d $(INSTALL_CONFDIR) > install -m 644 subdomain.conf $(INSTALL_CONFDIR) > install -m 644 parser.conf $(INSTALL_CONFDIR) Acked-by: Christian Boltz <appar...@cboltz.de> Regards, Christian Boltz -- Wer Linux an Windows misst, misst Mist. [David Haller in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Move binutils manpages to section 1
Hello, the binutils (aa-enabled and aa-exec) get installed into /usr/bin/ and are meant to be used by non-root users. Therefore the manpages should be in section 1 instead of 8 (which is for sysadmin commands). [ move-binutils-manpages-to-section-1.diff ] === modified file 'binutils/Makefile' --- binutils/Makefile 2015-12-18 01:19:23 + +++ binutils/Makefile 2016-04-05 18:02:48 + @@ -20,7 +20,7 @@ DESTDIR=/ BINDIR=${DESTDIR}/usr/bin LOCALEDIR=/usr/share/locale -MANPAGES=aa-enabled.8 aa-exec.8 +MANPAGES=aa-enabled.1 aa-exec.1 WARNINGS = -Wall EXTRA_WARNINGS = -Wsign-compare -Wmissing-field-initializers -Wformat- security -Wunused-parameter Regards, Christian Boltz -- >> Wo finde ich das log von Cyrus bei Opensuse 10.3. > Hinter der Festplatte links? Ich habe nachgesehen, dort ist das Log nicht, was nun? [>> "Info Beilfuss", > Patrick Ben Koetter und Sandy Drobic in postfixbuch-users] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] dovecot/auth: allow access to /var/run/dovecot/stats-user
Hello, $subject. Since the latest Tumbleweed update (dovecot 2.2.21 -> 2.2.22), dovecot/auth writes to /var/run/dovecot/stats-user. I propose this patch for trunk, 2.10 and 2.9. [ profiles-dovecot-auth-stats-user.diff ] === modified file 'profiles/apparmor.d/usr.lib.dovecot.auth' --- profiles/apparmor.d/usr.lib.dovecot.auth2015-03-19 12:56:41 + +++ profiles/apparmor.d/usr.lib.dovecot.auth2016-04-06 21:14:45 + @@ -38,6 +38,7 @@ /var/tmp/smtp_* rw, /{var/,}run/dovecot/auth-token-secret.dat{,.tmp} rw, + /{var/,}run/dovecot/stats-user w, # Site-specific additions and overrides. See local/README for details. #include Regards, Christian Boltz -- Alles wird gut. Nichts wird besser. :-) [Ratti in fontlinge-devel] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Fix dependency in binutils Makefile
Hello, binutils 'make install' failed if 'make' wasn't run before. This patch adds the missing dependency 'install-indep: indep' [ binutils-makefile-fix-deps.diff ] === modified file ./binutils/Makefile --- binutils/Makefile 2016-04-05 20:05:20.141427055 +0200 +++ binutils/Makefile 2016-04-05 20:59:54.839688746 +0200 @@ -141,7 +141,7 @@ install -m 755 ${TOOLS} ${BINDIR} .PHONY: install-indep -install-indep: +install-indep: indep $(MAKE) -C po install NAME=${NAME} DESTDIR=${DESTDIR} $(MAKE) install_manpages DESTDIR=${DESTDIR} Regards, Christian Boltz -- Teeren und federn und aus der Stadt jagen die dafür Verantwortlichen! Permanente wiederkehrende Geschlechtskrankheiten für die nächsten zehn Jahre! Mit mir eine Woche jeden Tag acht Stunden einen Raum teilen! Fallen wem noch härtere Strafen ein? ;) [Lars Müller in opensuse-de] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] [parser] Fix jobs not scaling up to meet available resources when cpus are brought online during compilation
Hello, Am Dienstag, 5. April 2016, 13:22:19 CEST schrieb Seth Arnold: > On Tue, Apr 05, 2016 at 12:37:07PM -0700, John Johansen wrote: > > Enable dynamically scaling max jobs if new resources are brought > > online > > > > BugLink: http://bugs.launchpad.net/bugs/1566490 > > > > This patch enables to parser to scale the max jobs if new resources > > are being brought online by the scheduler. > > > > It only enables the scaling check if there is a difference between > > the maximum number of cpus (CONF) and the number of online (ONLN) > > cpus. > > > > Instead of checking for more resources regardless, of whether the > > online cpu count is increasing it limits its checking to a maximum > > of MAX CPUS + 1 - ONLN cpus times. With each check coming after > > fork spawns a new work unit, giving the scheduler a chance to bring > > new cpus online before the next check. The +1 ensures the checks > > will be done at least once after the scheduling task sleeps waiting > > for its children giving the scheduler an extra chance to bring cpus > > online. Will it also reduce the number of processes if some CPUs are sent to vacation? > > Signed-off-by: John Johansen <john.johan...@canonical.com> > > This feels more complicated than it could be but I must admit I can't > suggest any modifications to the algorithm to simplify it. It sounds too simple, and it might start too many jobs in some cases, but - why not use the total number of CPUs from the beginning instead of the currently online CPUs? The only possible disadvantage is running "too many" jobs - would that do any harm? Regards, Christian Boltz -- >Gibt es hier in dieser Liste eigentlich ausser mir noch jemanden ?? Nein, aber es laufen einige Robots, die Traffic vortäuschen. Ich bin auch einer davon. [Tobias Korb und Thorsten Haude in suse-programming] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] [parser] Fix jobs not scaling up to meet available resources when cpus are brought online during compilation
Hello, Am Dienstag, 5. April 2016, 14:16:01 CEST schrieb John Johansen: > On 04/05/2016 01:51 PM, Christian Boltz wrote: > > Am Dienstag, 5. April 2016, 13:22:19 CEST schrieb Seth Arnold: > >> On Tue, Apr 05, 2016 at 12:37:07PM -0700, John Johansen wrote: > >>> Enable dynamically scaling max jobs if new resources are brought > >>> online > >>> > >>> BugLink: http://bugs.launchpad.net/bugs/1566490 > >>> > >>> This patch enables to parser to scale the max jobs if new > >>> resources > >>> are being brought online by the scheduler. > >>> > >>> It only enables the scaling check if there is a difference between > >>> the maximum number of cpus (CONF) and the number of online (ONLN) > >>> cpus. > >>> > >>> Instead of checking for more resources regardless, of whether the > >>> online cpu count is increasing it limits its checking to a maximum > >>> of MAX CPUS + 1 - ONLN cpus times. With each check coming after > >>> fork spawns a new work unit, giving the scheduler a chance to > >>> bring > >>> new cpus online before the next check. The +1 ensures the checks > >>> will be done at least once after the scheduling task sleeps > >>> waiting > >>> for its children giving the scheduler an extra chance to bring > >>> cpus > >>> online. > > > > Will it also reduce the number of processes if some CPUs are sent to > > vacation? > > It does not. This is specifically addressing the case of the hotplug > governor (and a few other ones in use on mobile devices), which > offlines cpus when load is low, and then brings them back online as > load ramps up. > > I was also trying to minimize the cost of the check, by limiting the > number of times we call out to check how many cpus are available. Its > extra overhead that really isn't needed on the devices where we are > seeing this problem. So the simple solution of just check every > time isn't ideal. > > The reverse case of cpus going offline while load is high seems some > what degenerate, and is a case where I am willing to live with a few > extra processes hanging around. Hopefully its not a common case > and would only result in one or two extra processes. Agreed. If you switch off CPUs, you want things to become slower ;-) > >>> Signed-off-by: John Johansen <john.johan...@canonical.com> > >> > >> This feels more complicated than it could be but I must admit I > >> can't > >> suggest any modifications to the algorithm to simplify it. > > > > It sounds too simple, and it might start too many jobs in some > > cases, > > but - why not use the total number of CPUs from the beginning > > instead of the currently online CPUs? > > > > The only possible disadvantage is running "too many" jobs - would > > that do any harm? > > it does, too many jobs actually slows things down. I am however > willing to revisit this when we manage to convert to true threading > instead of the fork model we are using today. Then we could > preallocate all possible threads and just not use them if it would > cause contention. > > Note, also this patch does not deal with cgroups and actual number > of cpus available to be used, which could be less than what is > online. I need to spend some time evaluating the best solution > for doing this. I wonder if we really need to implement this ourself - finding out how many threads should be used / how many CPUs are actually available sounds like something that has been done before ;-) > We could use pthread_getaffinity_np() which is probably the best > solution and we are already linking against pthreads because of the > library, but we want to go directly to sched_getaffinity(), or maybe > there is something else I haven't hit yet. Sounds like the "has been done before" code I was looking for. I'd recommend to swith to one of those functions instead of improving our re-invented wheel even more ;-) - but that shouldn't stop you from commiting this patch. Regards, Christian Boltz -- Linux: und wo bitte ist mein blauer Bildschirm? signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] tests: Adjust stacking tests build check for 2.11 Beta 1 release
Hello, Am Donnerstag, 24. März 2016, 10:31:55 CET schrieb Tyler Hicks: > I configured the stacking test binary to only be built when > libapparmor 2.11 is present. The versioning of the 2.11 Beta 1 > release (2.10.95) causes that check to fail and the stacking tests to > not be used. > > This patch adjusts the libapparmor version check to be aware of the > 2.11 Beta 1 versioning. > > Signed-off-by: Tyler Hicks <tyhi...@canonical.com> Makes sense ;-) Acked-by: Christian Boltz <appar...@cboltz.de> Regards, Christian Boltz -- Die Anbieter von Schrauben sollen davon ausgehen, daß die Anwender keine passenden Schraubenzieher haben, sondern nur einen Hammer benutzen wollen? [Erhard Schwenk in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] smbd profile needs capability sys_admin
Hello, smbd stores ACLS in the security.NTACL namespace, which means it needs capability sys_admin. References: https://bugzilla.opensuse.org/show_bug.cgi?id=964971 http://samba-technical.samba.narkive.com/eHtOW8DE/nt-acls-using-the-security-namespace-for-ntacl-considered-improper I propose this patch for trunk, 2.10 and 2.9. [ profiles-smbd-cap-sys_admin.diff ] === modified file 'profiles/apparmor.d/usr.sbin.smbd' --- profiles/apparmor.d/usr.sbin.smbd 2015-02-28 20:35:18 + +++ profiles/apparmor.d/usr.sbin.smbd 2016-02-11 17:51:14 + @@ -17,6 +17,7 @@ capability net_bind_service, capability setgid, capability setuid, + capability sys_admin, # needed to store ACLS in the security.NTACL namespace capability sys_resource, capability sys_tty_config, Regards, Christian Boltz -- > Genaugenommen kann es DAUs (also Mehrzahl) gar nicht geben ;-) Stimmt. Aber die werden ja gezuechtet, es gibt staendig einen neuen DAU, ergo hat man den aktuellen DAU und die nicht ganz aktuellen... [> Manfred Tremmel und David Haller in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Don't store exec modes in transtions[]
Hello, Am Samstag, 19. März 2016, 11:55:09 CET schrieb Steve Beattie: > On Sun, Feb 21, 2016 at 03:00:06PM +0100, Christian Boltz wrote: > > exec choices are stored in transitions[], but that's never used > > (and I don't see a need for it), therefore stop storing it. > > > > > > [ 73-exec-transitions.diff ] > > > > === modified file 'utils/apparmor/aa.py' > > --- utils/apparmor/aa.py2016-02-20 12:32:36 + > > +++ utils/apparmor/aa.py2016-02-21 13:50:24 + > > @@ -1205,7 +1205,6 @@ > > > > context_new = context_new + '^%s' % hat > > > > context_new = context_new + ' -> %s' % > > exec_target > > > > -# ans_new = transitions.get(context_new, '') # > > XXX ans meant here?> > > combinedmode = set() > > combinedaudit = set() > > ## Check return Value Consistency > > > > @@ -1415,7 +1414,6 @@ > > > > exec_mode = exec_mode - > > (apparmor.aamode.AA_EXEC_U > > NSAFE | > > AA_OTHER(apparmor.aamode.A > > A_EXEC_UNSAFE))> > > > > else: > > ans = 'INVALID' > > > > -transitions[context_new] = ans > > > > regex_options = > > re.compile('CMD_(ix|px|cx|nx|pix|cix|nix)' > > ) > > > if regex_options.search(ans): > Are you sure about that? I see in handle_children(): > > > http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/view/head:/ > utils/apparmor/aa.py#L1075 > > 1075ans = transitions.get(context, 'XXXINVALIDXXX') > 1076 > 1077 while ans not in ['CMD_ADDHAT', 'CMD_USEDEFAULT', > 'CMD_DENY']: > > and transitions is a global hasher() object. > > But I've only looked at this cursorily, so don't claim any real > understanding of what's going on (or not going on) here. 'transitions' is currently used for storing two not-too-related things: The code you quoted is about hats, and it's looking for 'CMD_ADDHAT', 'CMD_USEDEFAULT' and 'CMD_DENY'. That part is useful and will stay. (Maybe we should rename 'transitions' to a better name (hat_choices?), but that's a cosmetic issue.) The code I want to remove stores the exec choice (CMD_ix, CMD_px etc.) - and that information isn't used anywhere (= storing it is superfluous). Instead, profile_known_exec() is used to check if we need to ask for adding an exec rule or if we already have one. Regards, Christian Boltz -- Möglicherweise laufe ich sogar mit fliegenden Fahnen von Gnome zu KDE über. Jedenfalls, bis sich das Gnome-Projekt dazu entschliesst, Nautilus durch /irgendwas/ zu ersetzen. Notfalls eine Parkuhr oder einen Bratenwender. Aber nicht dieses "Ding". [Ratti in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] nscd profile: allow paranoia mode
Hello, in /etc/nscd.conf there is an option allowing to restart nscd after a certain time. However, this requires reading /proc/self/cmdline - otherwise nscd will disable paranoia mode. References: https://bugzilla.opensuse.org/show_bug.cgi?id=971790 I propose this patch for trunk, 2.10 and 2.9 [ profiles-nscd-paranoia.diff ] === modified file 'profiles/apparmor.d/usr.sbin.nscd' --- profiles/apparmor.d/usr.sbin.nscd 2014-12-01 22:44:13 + +++ profiles/apparmor.d/usr.sbin.nscd 2016-03-21 19:57:03 + @@ -31,6 +31,7 @@ /{var/cache,var/run,run}/nscd/{passwd,group,services,hosts,netgroup} rw, /{,var/}run/{nscd/,}nscd.pid rwl, /var/log/nscd.log rw, + @{PROC}/@{pid}/cmdline r, @{PROC}/@{pid}/fd/ r, @{PROC}/@{pid}/fd/* r, @{PROC}/@{pid}/mounts r, Regards, Christian Boltz -- Wenn's eine kaputte Platte ist: Entsorgen, Backup zurückspielen. Wenn's kein Backup gibt - nennt sich das ganze "lernen" ;-) [Arno Lehmann in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Update abstractions/ssl_* for acmetool-generated certificates
Hello, acmetool is an alternative client for Let's Encrypt. (https://github.com/hlandau/acme/) It stores the certificates etc. in the following directory layout: /var/lib/acme/live/ -> ../certs/ /var/lib/acme/certs//cert /var/lib/acme/certs//chain /var/lib/acme/certs//privkey -> ../../keys//privkey /var/lib/acme/certs//url /var/lib/acme/certs//fullchain /var/lib/acme/keys//privkey This patch adds the needed permissions to the ssl_certs and ssl_keys abstractions so that the certificates can be used. I propose this patch for trunk, 2.10 and 2.9. [ abstractions-ssl-acmetool.diff ] === modified file 'profiles/apparmor.d/abstractions/ssl_certs' --- profiles/apparmor.d/abstractions/ssl_certs 2015-01-31 15:51:17 + +++ profiles/apparmor.d/abstractions/ssl_certs 2016-03-27 16:28:03 + @@ -23,3 +23,7 @@ /usr/local/share/ca-certificates/** r, /var/lib/ca-certificates/ r, /var/lib/ca-certificates/** r, + + # acmetool + /var/lib/acme/certs/*/chain r, + /var/lib/acme/certs/*/cert r, === modified file 'profiles/apparmor.d/abstractions/ssl_keys' --- profiles/apparmor.d/abstractions/ssl_keys 2010-12-20 20:29:10 + +++ profiles/apparmor.d/abstractions/ssl_keys 2016-03-27 16:32:32 + @@ -16,3 +16,7 @@ /etc/ssl/ r, /etc/ssl/** r, + # acmetool + /var/lib/acme/live/* r, + /var/lib/acme/certs/** r, + /var/lib/acme/keys/** r, Regards, Christian Boltz -- das Gerät ist doch am USB-Port angeschlossen, also verfolge einfach das Kabel von USB-Anschluss, am Ende solltest du dein Gerät wiederfinden (vielleicht ist es ja nur vom Schreibtisch gefallen) [Kai Lindenberg in suse-linux] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] Reminder: IRC meeting today
Hello, just as a reminder - we have an IRC meeting scheduled for today 20:00 UTC (that's in about 3 hours). Regards, Christian Boltz -- They ARE right, you CAN secure an IIS against intrusion. First you turn off all the services and other hooks that lets it do all the things they brag on that makes IIS "better", then you imbed an axe in the machine, then you unplug it, encase it in glass, put it in a concrete vault, cover that with urethane, and toss the whole thing into the depest part of the ocean, cover it with 150 meters of silt, create a concrete sarcophogus over the silt pile, and liberally seed the area with spent nuclear fuel to kill anything that gets too close to it. No problem. [random in asr] signature.asc Description: This is a digitally signed message part. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] Add a test to check for hotkey conflicts
Hello, this test builds and installs the apparmor-utils translations into a tempdir, and then checks if there's any hotkey conflict in one of the languages. This is based on a manually maintained list of "buttons" that are displayed at the same time. To make things a bit easier to test, add CMD_CANCEL to ui.py CMDS[]. Also replace hardcoded usage of '(Y)es', '(N)o' and '(C)ancel' with CMDS['CMD_YES'], CMDS['CMD_NO'] and CMDS['CMD_CANCEL']. Note: you'lll get hotkey conflicts for the german translations. I fixed them on lp already, so importing the latest translations should help ;-) [ 77-check-hotkey-conflicts.diff ] --- utils/apparmor/ui.py2014-11-17 20:18:13.810933000 +0100 +++ utils/apparmor/ui.py2016-04-03 16:21:17.013134733 +0200 @@ -77,8 +77,8 @@ default = default.lower() ans = None if UI_mode == 'text': -yes = _('(Y)es') -no = _('(N)o') +yes = CMDS['CMD_YES'] +no = CMDS['CMD_NO'] yeskey = get_translated_hotkey(yes).lower() nokey = get_translated_hotkey(no).lower() ans = 'XXXINVALIDXXX' @@ -121,9 +121,9 @@ default = default.lower() ans = None if UI_mode == 'text': -yes = _('(Y)es') -no = _('(N)o') -cancel = _('(C)ancel') +yes = CMDS['CMD_YES'] +no = CMDS['CMD_NO'] +cancel = CMDS['CMD_CANCEL'] yeskey = get_translated_hotkey(yes).lower() nokey = get_translated_hotkey(no).lower() @@ -274,6 +274,7 @@ 'CMD_ASK_LATER': _('Ask Me (L)ater'), 'CMD_YES': _('(Y)es'), 'CMD_NO': _('(N)o'), +'CMD_CANCEL': _('(C)ancel'), 'CMD_ALL_NET': _('Allow All (N)etwork'), 'CMD_NET_FAMILY': _('Allow Network Fa(m)ily'), 'CMD_OVERWRITE': _('(O)verwrite Profile'), --- utils/test/test-translations.py 2016-04-03 18:00:50.303549877 +0200 +++ utils/test/test-translations.py 2016-04-03 18:00:03.555820261 +0200 @@ -0,0 +1,67 @@ +#! /usr/bin/env python +# -- +# +#Copyright (C) 2016 Christian Boltz <appar...@cboltz.de> +# +#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 unittest +from common_test import AATest, setup_all_loops + +import gettext +import os +import subprocess + +from apparmor.ui import CMDS, get_translated_hotkey + +class TestHotkeyConflicts(AATest): +# check if there are any hotkey conflicts in one of the apparmor-utils translations +tests = [ +(['CMD_ALLOW', 'CMD_DENY', 'CMD_IGNORE_ENTRY', 'CMD_GLOB', 'CMD_GLOBEXT', 'CMD_NEW', 'CMD_AUDIT_OFF', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py available_buttons() with CMD_AUDIT_OFF +(['CMD_ALLOW', 'CMD_DENY', 'CMD_IGNORE_ENTRY', 'CMD_GLOB', 'CMD_GLOBEXT', 'CMD_NEW', 'CMD_AUDIT_NEW', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py available_buttons() with CMD_AUDIT_NEW +(['CMD_SAVE_CHANGES', 'CMD_SAVE_SELECTED', 'CMD_VIEW_CHANGES', 'CMD_VIEW_CHANGES_CLEAN', 'CMD_ABORT'], True), # aa.py save_profiles() +(['CMD_VIEW_PROFILE', 'CMD_USE_PROFILE', 'CMD_CREATE_PROFILE', 'CMD_ABORT', 'CMD_FINISHED'],True), # aa.py get_profile() +(['CMD_UPLOAD_CHANGES', 'CMD_VIEW_CHANGES', 'CMD_ASK_LATER', 'CMD_ASK_NEVER', 'CMD_ABORT'], True), # aa.py console_select_and_upload_profiles() +(['CMD_ix', 'CMD_pix', 'CMD_cix', 'CMD_nix', 'CMD_EXEC_IX_OFF', 'CMD_ux', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py build_x_functions() with exec_toggle +(['CMD_ix', 'CMD_cx', 'CMD_px', 'CMD_nx', 'CMD_ux', 'CMD_EXEC_IX_ON', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py build_x_functions() without exec_toggle +(['CMD_ADDHAT', 'CMD_USEDEFAULT', 'CMD_DENY', 'CMD_ABORT', 'CMD_FINISHED'], True), # aa.py handle_children() +(['CMD_YES', 'CMD_NO', 'CMD_CANCEL'], True), # ui.py UI_YesNo() and UI_YesNoCancel +] + +def _run_test(self, params, expected): +self.createTmpdir() + +subprocess.call("make -C ../po >/dev/null", shell=True) +subprocess.call("DESTDIR=%s NAME=apparmor-utils make -C ../po install >/dev/null" % self.tmpdir, shell=True) + +self.localedir = '%s/usr/share/locale' % self.tmpdir + +self.languages = os.listdir(self.localedir) + +# make sure we found all translations +if len(self.languages) < 15: +raise Exception('None or not al