Re: [apparmor] [patch] Add some simple_tests (dbus and bare file rules)

2016-01-07 Thread Christian Boltz
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()

2015-12-20 Thread Christian Boltz
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

2015-12-20 Thread Christian Boltz
 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()

2015-12-21 Thread Christian Boltz
):
 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

2015-12-21 Thread Christian Boltz
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

2015-12-25 Thread Christian Boltz
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

2015-12-26 Thread Christian Boltz
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

2015-12-26 Thread Christian Boltz
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

2015-12-26 Thread Christian Boltz
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

2015-12-26 Thread Christian Boltz
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()

2015-12-26 Thread Christian Boltz
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()

2015-12-26 Thread Christian Boltz
 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

2015-12-26 Thread Christian Boltz
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".

2015-12-18 Thread Christian Boltz
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

2015-12-20 Thread Christian Boltz
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()

2015-12-20 Thread Christian Boltz
 _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

2015-12-27 Thread Christian Boltz
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()

2015-12-27 Thread Christian Boltz
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

2015-12-27 Thread Christian Boltz
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

2015-12-27 Thread Christian Boltz
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

2015-12-27 Thread Christian Boltz
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

2015-12-27 Thread Christian Boltz
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

2015-12-27 Thread Christian Boltz
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

2015-12-27 Thread Christian Boltz
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()

2015-12-23 Thread Christian Boltz
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

2015-12-17 Thread Christian Boltz
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

2015-11-24 Thread Christian Boltz
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

2015-11-24 Thread Christian Boltz
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

2015-11-26 Thread Christian Boltz
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

2015-11-28 Thread Christian Boltz
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

2015-11-28 Thread Christian Boltz
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

2015-11-25 Thread Christian Boltz
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

2015-11-30 Thread Christian Boltz
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

2015-11-29 Thread Christian Boltz
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

2015-11-18 Thread Christian Boltz
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

2015-11-18 Thread Christian Boltz
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

2015-11-20 Thread Christian Boltz
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

2015-11-19 Thread Christian Boltz
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

2015-11-22 Thread Christian Boltz
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

2015-11-19 Thread Christian Boltz
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

2015-11-19 Thread Christian Boltz
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()

2016-01-12 Thread Christian Boltz
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

2016-06-05 Thread Christian Boltz
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]

2016-06-08 Thread Christian Boltz
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)

2016-05-31 Thread Christian Boltz
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)

2016-06-01 Thread Christian Boltz
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)

2016-06-01 Thread Christian Boltz
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'

2016-01-10 Thread Christian Boltz
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

2016-01-10 Thread Christian Boltz
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

2016-01-10 Thread Christian Boltz
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

2016-01-10 Thread Christian Boltz
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

2016-01-11 Thread Christian Boltz
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

2016-01-10 Thread Christian Boltz
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

2016-01-10 Thread Christian Boltz
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

2016-01-15 Thread Christian Boltz
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

2016-01-15 Thread Christian Boltz
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

2016-06-27 Thread Christian Boltz
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

2016-02-06 Thread Christian Boltz
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

2016-02-05 Thread Christian Boltz
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

2016-02-04 Thread Christian Boltz
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

2016-02-11 Thread Christian Boltz
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

2016-02-08 Thread Christian Boltz
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()

2016-02-01 Thread Christian Boltz
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

2016-02-01 Thread Christian Boltz
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

2016-02-01 Thread Christian Boltz
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

2016-01-31 Thread Christian Boltz
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

2016-02-01 Thread Christian Boltz
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()

2016-02-01 Thread Christian Boltz
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()

2016-01-25 Thread Christian Boltz
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

2016-01-20 Thread Christian Boltz
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

2016-01-19 Thread Christian Boltz
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

2016-01-19 Thread Christian Boltz
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

2016-01-27 Thread Christian Boltz
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'

2016-01-24 Thread Christian Boltz
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

2016-02-18 Thread Christian Boltz
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

2016-02-18 Thread Christian Boltz
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

2016-02-19 Thread Christian Boltz
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()

2016-02-20 Thread Christian Boltz
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[]

2016-02-21 Thread Christian Boltz
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

2016-02-21 Thread Christian Boltz
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'

2016-02-21 Thread Christian Boltz
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

2016-02-21 Thread Christian Boltz
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

2016-02-21 Thread Christian Boltz
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'

2016-02-21 Thread Christian Boltz
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

2016-02-21 Thread Christian Boltz
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()

2016-03-01 Thread Christian Boltz
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

2016-03-01 Thread Christian Boltz
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

2016-04-06 Thread Christian Boltz
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

2016-04-05 Thread Christian Boltz
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

2016-04-06 Thread Christian Boltz
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

2016-04-05 Thread Christian Boltz
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

2016-04-05 Thread Christian Boltz
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

2016-04-05 Thread Christian Boltz
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

2016-03-24 Thread Christian Boltz
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

2016-03-20 Thread Christian Boltz
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[]

2016-03-20 Thread Christian Boltz
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

2016-03-21 Thread Christian Boltz
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

2016-03-27 Thread Christian Boltz
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

2016-03-01 Thread Christian Boltz
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

2016-04-03 Thread Christian Boltz
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

<    5   6   7   8   9   10   11   12   13   14   >