[prev in list] [next in list] [prev in thread] [next in thread]
List: apparmor-dev
Subject: Re: [apparmor] [patch] Add SignalRule and SignalRuleset classes
From: Kshitij Gupta <kgupta8592 () gmail ! com>
Date: 2015-11-19 20:36:30
Message-ID: CAMBXP50MEVBbJeav0POJuK=YOTR07Yz5u2iF_r9ADvJnJL+71Q () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Looks good.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
On Thu, Nov 19, 2015 at 11:11 PM, Christian Boltz <apparmor@cboltz.de>
wrote:
> 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+000000000000000000000000000000000000000000032
> 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
> > > + return False
> > > +
> > > + # still here? -> then it is covered
> >
> > code seems surprised seeing one here ;-)
>
> Well, it had enough changes to hit a "return False" in all the checks
> above ;-) - and the comment is needed to explain the "return True" in
> the next line.
>
> > > + return True
>
>
> > > === modified file ./utils/test/test-signal.py
> > > --- utils/test/test-signal.py 2015-10-23 01:17:35.102452075 +0200
> > > +++ utils/test/test-signal.py 2015-10-23 13:29:07.054183515 +0200
> > > @@ -0,0 +1,576 @@
> ...
> > > + ('deny signal send,' , [ False , False
> > > , False , False ]),
> > > + ]
> > > +
> > > +
> > > +
> > > +
> >
> > Too much white-space
>
> Indeed. I'll remove whitespace here and at some more places.
>
>
> > The tests mostly look nice and comprehensive(I've superficially
> > looked at them). Yay for the awesome coverage!
> >
> > Looks good.
> >
> > I can understand the patch being pending for so long due to its sheer
> > size.
>
> Come on, signal.py has only 300 lines - that's not too much for a
> totally new feature ;-) but yes, I know it needs more review time than
> a one-line change.
>
>
>
> Here's the patch to address your comments.
> Since you have already acked those changes in advance in your next mail,
> I'll assume it's acked if nobody complains within a day ;-)
>
>
> [patch] Some adjustments for SignalRule based on Kshitij's comments
>
> - add a missing comment to explain RE_SIGNAL_DETAILS
> - rename self.all_accesss to self.all_access (also in the tests)
> - make all AppArmorExceptions translatable
> - make splitting the access keywords one line to fit the comment
> - remove some superfluous empty lines from test-signal.py
>
>
> I'll commit this together with 07-add-SignalRule-and-SignalRuleset.diff
>
> (Pre-)Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
>
>
> [ 22-signal-rule-adjustments.diff ]
>
> === modified file ./utils/apparmor/rule/signal.py
> --- utils/apparmor/rule/signal.py 2015-11-19 17:42:26.325879118 +0100
> +++ utils/apparmor/rule/signal.py 2015-11-19 18:06:46.376422794 +0100
> @@ -53,7 +53,7 @@
> '^' +
> '(\s+(?P<access>' + RE_ACCESS_KEYWORDS + '))?' + # optional access
> keyword(s)
> '(?P<signal>' + '(\s+(' + RE_SIGNAL_KEYWORDS + '))+' + ')?' + #
> optional signal set(s)
> - '(\s+(peer=' + RE_PROFILE_NAME % 'peer' + '))?' +
> + '(\s+(peer=' + RE_PROFILE_NAME % 'peer' + '))?' + # optional peer
> '\s*$')
>
>
> @@ -80,9 +80,9 @@
> comment=comment,
> log_event=log_event)
>
> - self.access, self.all_accesss, unknown_items =
> check_and_split_list(access, access_keywords, SignalRule.ALL, 'SignalRule',
> 'access')
> + self.access, self.all_access, unknown_items =
> check_and_split_list(access, access_keywords, SignalRule.ALL, 'SignalRule',
> 'access')
> if unknown_items:
> - raise AppArmorException('Passed unknown access keyword to
> SignalRule: %s' % ' '.join(unknown_items))
> + raise AppArmorException(_('Passed unknown access keyword to
> SignalRule: %s') % ' '.join(unknown_items))
>
> self.signal, self.all_signals, unknown_items =
> check_and_split_list(signal, signal_keywords, SignalRule.ALL, 'SignalRule',
> 'signal')
> if unknown_items:
> @@ -90,7 +90,7 @@
> if RE_SIGNAL_REALTIME.match(item):
> self.signal.add(item)
> else:
> - raise AppArmorException('Passed unknown signal
> keyword to SignalRule: %s' % item)
> + raise AppArmorException(_('Passed unknown signal
> keyword to SignalRule: %s') % item)
>
> self.peer = None
> self.all_peers = False
> @@ -129,10 +129,9 @@
>
> if details.group('access'):
> access = details.group('access')
> - access = ' '.join(access.split(',')) # split by ',' or
> whitespace
> if access.startswith('(') and access.endswith(')'):
> access = access[1:-1]
> - access = access.split()
> + access = access.replace(',', ' ').split() # split by ','
> or whitespace
> else:
> access = SignalRule.ALL
>
> @@ -162,7 +161,7 @@
>
> space = ' ' * depth
>
> - if self.all_accesss:
> + if self.all_access:
> access = ''
> elif len(self.access) == 1:
> access = ' %s' % ' '.join(self.access)
> @@ -192,7 +191,7 @@
> def is_covered_localvars(self, other_rule):
> '''check if other_rule is covered by this rule object'''
>
> - if not other_rule.access and not other_rule.all_accesss:
> + if not other_rule.access and not other_rule.all_access:
> raise AppArmorBug('No access specified in other signal rule')
>
> if not other_rule.signal and not other_rule.all_signals:
> @@ -201,8 +200,8 @@
> if not other_rule.peer and not other_rule.all_peers:
> raise AppArmorBug('No peer specified in other signal rule')
>
> - if not self.all_accesss:
> - if other_rule.all_accesss:
> + if not self.all_access:
> + if other_rule.all_access:
> return False
> if other_rule.access != self.access:
> return False
> @@ -229,7 +228,7 @@
> raise AppArmorBug('Passed non-signal rule: %s' %
> str(rule_obj))
>
> if (self.access != rule_obj.access
> - or self.all_accesss != rule_obj.all_accesss):
> + or self.all_access != rule_obj.all_access):
> return False
>
> if (self.signal != rule_obj.signal
> @@ -245,7 +244,7 @@
> return True
>
> def logprof_header_localvars(self):
> - if self.all_accesss:
> + if self.all_access:
> access = _('ALL')
> else:
> access = ' '.join(sorted(self.access))
> === modified file ./utils/test/test-signal.py
> --- utils/test/test-signal.py 2015-11-19 17:42:26.329879090 +0100
> +++ utils/test/test-signal.py 2015-11-19 17:46:14.472311782 +0100
> @@ -25,7 +25,7 @@
> _ = init_translation()
>
> exp = namedtuple('exp', ['audit', 'allow_keyword', 'deny', 'comment',
> - 'access', 'all_accesss', 'signal', 'all_signals', 'peer',
> 'all_peers'])
> + 'access', 'all_access', 'signal', 'all_signals', 'peer',
> 'all_peers'])
>
> # --- tests for single SignalRule --- #
>
> @@ -39,7 +39,7 @@
> self.assertEqual(expected.peer, obj.peer.regex)
> else:
> self.assertEqual(expected.peer, obj.peer)
> - self.assertEqual(expected.all_accesss, obj.all_accesss)
> + self.assertEqual(expected.all_access, obj.all_access)
> self.assertEqual(expected.all_signals, obj.all_signals)
> self.assertEqual(expected.all_peers, obj.all_peers)
> self.assertEqual(expected.deny, obj.deny)
> @@ -125,7 +125,6 @@
>
> self.assertEqual(obj.get_raw(1), ' signal send set=term
> peer=/usr/bin/pulseaudio///usr/lib/pulseaudio/pulse/gconf-helper,')
>
> -
> class SignalFromInit(SignalTest):
> tests = [
> # SignalRule object
> audit allow deny comment access all? signal
> all? peer all?
> @@ -140,7 +139,6 @@
> def _run_test(self, obj, expected):
> self._compare_obj(obj, expected)
>
> -
> class InvalidSignalInit(AATest):
> tests = [
> # init params expected exception
> @@ -177,7 +175,6 @@
> with self.assertRaises(TypeError):
> SignalRule('r', 'int')
>
> -
> class InvalidSignalTest(AATest):
> def _check_invalid_rawrule(self, rawrule):
> obj = None
> @@ -309,7 +306,6 @@
> ('signal receive,' , [ False , False ,
> False , False ]),
> ]
>
> -
> class SignalCoveredTest_03(SignalCoveredTest):
> rule = 'signal send set=quit,'
>
> @@ -437,9 +433,6 @@
> ('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
> --
> Jaaaa! Und am besten den Rest des Desktops auch noch mit DHTML
> nachprogrammieren, ach was sag ich, JavaScript ist ja /die/ Sprache,
> um das ganze Betriebsystem neu zu entwickeln.
> [Ratti in fontlinge-devel]
>
>
> --
> AppArmor mailing list
> AppArmor@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
--
Regards,
Kshitij Gupta
[Attachment #5 (text/html)]
<div dir="ltr">Looks good.<br><br>Acked-by: Kshitij Gupta <<a \
href="mailto:kgupta8592@gmail.com" \
target="_blank">kgupta8592@gmail.com</a>><br></div><div \
class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 19, 2015 at 11:11 PM, \
Christian Boltz <span dir="ltr"><<a href="mailto:apparmor@cboltz.de" \
target="_blank">apparmor@cboltz.de</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Hello,<br> <br>
[scroll down for an add-on patch that addresses Kshitij's comments]<br>
<br>
Am Donnerstag, 19. November 2015 schrieb Kshitij Gupta:<br>
<span class="">> On Fri, Oct 23, 2015 at 6:30 PM, Christian Boltz wrote:<br>
> > this patch adds the SignalRule and SignalRuleset classes<br>
<br>
</span><span class="">> > [ 07-add-SignalRule-and-SignalRuleset.diff ]<br>
> ><br>
> > === modified file ./utils/apparmor/rule/signal.py<br>
> > --- utils/apparmor/rule/signal.py 2015-10-23<br>
> > 01:17:21.579245521 +0200 +++ utils/apparmor/rule/signal.py<br>
> > 2015-10-23 01:08:01.149132984 +0200 @@ -0,0 +1,300 @@<br>
</span>...<br>
<span class="">> > + 'io', \
'pwr', 'sys', 'emt', 'exists']<br> > > \
+RE_SIGNAL_REALTIME =<br> > > \
re.compile('^rtmin\+0*([0-9]|[12][0-9]|3[0-2])$') #<br> > > \
rtmin+0..rtmin+32, number may have leading zeros<br> ><br>
> I do not like this regex.<br>
> Its far too complicated for when its only saying: rtmin+x such that x<br>
> maybe 2digit and is in [0,32] and possibly return x. Plus Its<br>
> confusing whether rtmin+032 is allowed or not (regex suggests it is).<br>
<br>
</span>It is allowed. Even rtmin+000000000000000000000000000000000000000000032<br>
is allowed ;-)<br>
<span class=""><br>
> Maybe its easier(more readably) done in a function with some int() and<br>
> boundary tests.<br>
<br>
</span>We have more interesting regexes, and that one is not too terrible IMHO.<br>
<br>
With your proposal, the regex would probably be ^rtmin\+([0-9]+)$ and a<br>
boundary check at another place. I slightly doubt that makes the code<br>
more readable ;-)<br>
<span class=""><br>
> > +joint_access_keyword = '\s*(' + '|'.join(access_keywords) \
+ ')\s*'<br> ><br>
> Better[tm] written as:<br>
> joint_access_keyword = ''\s*(%s)\s*' % \
('|'.join(access_keywords))<br> <br>
</span>If it would be the only regex in the file, I'd fully agree. However \
the<br> other regexes nearby are composed using + to keep them readable.<br>
Therefore I tend to keep using + also here ;-)<br>
<span class=""><br>
> > +RE_ACCESS_KEYWORDS = ( joint_access_keyword + # one of the<br>
> > access_keyword or<br>
> > + '|' +<br>
> > # or<br>
> > + '\(' + joint_access_keyword + \
'(' +<br> > > '(\s|,)+' + joint_access_keyword + ')*' + \
'\)' # one or more<br> > > signal_keyword in (...) + \
)<br> ><br>
> Thats some beast!<br>
<br>
</span>Thanks to the syntax of signal rules - I don't like some details, but<br>
obviously have to support all of them. (You missed some funny rants<br>
about that on IRC ;-)<br>
<span class=""><br>
> > allow_keyword=allow_keyword, +<br>
> > comment=comment, +<br>
> > log_event=log_event) +<br>
> > + self.access, self.all_accesss, unknown_items =<br>
> > check_and_split_list(access, access_keywords, SignalRule.ALL,<br>
> > 'SignalRule', 'access')<br>
><br>
> all_accesss (three s's) ;-)<br>
> I hope that was intentional.<br>
<br>
</span>Not really, but at least it's consistent across signal.py (thanks<br>
autocompletion!) and therefore "just" a strange variable name without<br>
causing bugs.<br>
<br>
Nevertheless, I'll remove the middle s.<br>
<span class=""><br>
> > + if RE_SIGNAL_REALTIME.match(item):<br>
> > + self.signal.add(item)<br>
> > + else:<br>
> > + raise AppArmorException('Passed unknown \
signal<br> > > keyword to SignalRule: %s' % item)<br>
><br>
> Missing _().<br>
> AppArmorExceptions are expected to have translations while for<br>
> AppArmorBug we dont, right?<br>
<br>
</span>Right, I'll change that (here and some lines below)<br>
<span class=""><br>
> > + if details.group('access'):<br>
> > + access = details.group('access')<br>
> > + access = ' \
'.join(access.split(',')) # split by<br> > > ',' or \
whitespace<br> ><br>
> Is it expected to split strings separated by a , or whitespace? This<br>
> part will only split strings separated by comma.<br>
> It can't do both(which the comment confused me to believe until I read<br>
> on and saw the space split ;-) ).<br>
<br>
</span>Actually the comment describes what the code does, but I changed the<br>
code after writing the comment ;-)<br>
<br>
This line splits at the comma and re-joins with a space as delimiter.<br>
After looking at it again, .replace() should be enough (and probably<br>
faster). Also, replacing the comma and split() should be done nearby.<br>
<br>
I'll change that to a single line so that the comment fits again ;-)<br>
<span class=""><br>
> > + if access.startswith('(') and \
access.endswith(')'):<br> > > + access = \
access[1:-1]<br> > > + access = access.split()<br>
><br>
> There's the space separated split.<br>
<br>
</span>Yes, after the (sometimes optional) "(...)" wrapper was removed.<br>
And that's also the place where the comma split/replacement should and<br>
will happen.<br>
<span class=""><br>
> > + if not self.all_peers:<br>
> > + if other_rule.all_peers:<br>
> > + return False<br>
> > + if other_rule.peer != self.peer: # XXX use AARE<br>
> > + return False<br>
> > +<br>
> > + # still here? -> then it is covered<br>
><br>
> code seems surprised seeing one here ;-)<br>
<br>
</span>Well, it had enough changes to hit a "return False" in all the \
checks<br> above ;-) - and the comment is needed to explain the "return \
True" in<br> the next line.<br>
<br>
> > + return True<br>
<span class=""><br>
<br>
> > === modified file ./utils/test/test-signal.py<br>
> > --- utils/test/test-signal.py 2015-10-23 01:17:35.102452075 +0200<br>
> > +++ utils/test/test-signal.py 2015-10-23 13:29:07.054183515 +0200<br>
> > @@ -0,0 +1,576 @@<br>
</span>...<br>
<span class="">> > + ('deny signal send,' \
, [ False , False<br> > > , False , False ]),<br>
> > + ]<br>
> > +<br>
> > +<br>
> > +<br>
> > +<br>
><br>
> Too much white-space<br>
<br>
</span>Indeed. I'll remove whitespace here and at some more places.<br>
<span class=""><br>
<br>
> The tests mostly look nice and comprehensive(I've superficially<br>
> looked at them). Yay for the awesome coverage!<br>
><br>
> Looks good.<br>
><br>
> I can understand the patch being pending for so long due to its sheer<br>
> size.<br>
<br>
</span>Come on, signal.py has only 300 lines - that's not too much for a<br>
totally new feature ;-) but yes, I know it needs more review time than<br>
a one-line change.<br>
<br>
<br>
<br>
Here's the patch to address your comments.<br>
Since you have already acked those changes in advance in your next mail,<br>
I'll assume it's acked if nobody complains within a day ;-)<br>
<br>
<br>
[patch] Some adjustments for SignalRule based on Kshitij's comments<br>
<br>
- add a missing comment to explain RE_SIGNAL_DETAILS<br>
- rename self.all_accesss to self.all_access (also in the tests)<br>
- make all AppArmorExceptions translatable<br>
- make splitting the access keywords one line to fit the comment<br>
- remove some superfluous empty lines from test-signal.py<br>
<span class=""><br>
<br>
I'll commit this together with 07-add-SignalRule-and-SignalRuleset.diff<br>
<br>
</span>(Pre-)Acked-by: Kshitij Gupta <<a \
href="mailto:kgupta8592@gmail.com">kgupta8592@gmail.com</a>><br> <br>
<br>
[ 22-signal-rule-adjustments.diff ]<br>
<br>
=== modified file ./utils/apparmor/rule/signal.py<br>
--- utils/apparmor/rule/signal.py 2015-11-19 17:42:26.325879118 +0100<br>
+++ utils/apparmor/rule/signal.py 2015-11-19 18:06:46.376422794 +0100<br>
@@ -53,7 +53,7 @@<br>
'^' +<br>
<span class=""> '(\s+(?P<access>' + RE_ACCESS_KEYWORDS + \
'))?' + # optional access keyword(s)<br> </span><span class=""> \
'(?P<signal>' + '(\s+(' + RE_SIGNAL_KEYWORDS + '))+' + \
')?' + # optional signal set(s)<br> </span>- '(\s+(peer=' + \
RE_PROFILE_NAME % 'peer' + '))?' +<br> + '(\s+(peer=' + \
RE_PROFILE_NAME % 'peer' + '))?' + # optional peer<br> \
'\s*$')<br> <br>
<br>
@@ -80,9 +80,9 @@<br>
\
comment=comment,<br>
\
log_event=log_event)<br> <br>
- self.access, self.all_accesss, unknown_items = \
check_and_split_list(access, access_keywords, SignalRule.ALL, 'SignalRule', \
'access')<br> + self.access, self.all_access, unknown_items = \
check_and_split_list(access, access_keywords, SignalRule.ALL, 'SignalRule', \
'access')<br> if unknown_items:<br>
- raise AppArmorException('Passed unknown access keyword to \
SignalRule: %s' % ' '.join(unknown_items))<br> + raise \
AppArmorException(_('Passed unknown access keyword to SignalRule: %s') % \
' '.join(unknown_items))<br> <span class=""><br>
self.signal, self.all_signals, unknown_items = \
check_and_split_list(signal, signal_keywords, SignalRule.ALL, 'SignalRule', \
'signal')<br> </span> if unknown_items:<br>
@@ -90,7 +90,7 @@<br>
if RE_SIGNAL_REALTIME.match(item):<br>
self.signal.add(item)<br>
else:<br>
- raise AppArmorException('Passed unknown signal \
keyword to SignalRule: %s' % item)<br> + raise \
AppArmorException(_('Passed unknown signal keyword to SignalRule: %s') % \
item)<br> <br>
self.peer = None<br>
self.all_peers = False<br>
@@ -129,10 +129,9 @@<br>
<br>
if details.group('access'):<br>
<span class=""> access = details.group('access')<br>
</span>- access = ' '.join(access.split(',')) \
# split by ',' or whitespace<br> <span class=""> if \
access.startswith('(') and access.endswith(')'):<br> </span><span \
class=""> access = access[1:-1]<br> </span>- \
access = access.split()<br> + access = \
access.replace(',', ' ').split() # split by ',' or \
whitespace<br> else:<br>
access = SignalRule.ALL<br>
<br>
@@ -162,7 +161,7 @@<br>
<br>
space = ' ' * depth<br>
<br>
- if self.all_accesss:<br>
+ if self.all_access:<br>
access = ''<br>
elif len(self.access) == 1:<br>
<span class=""> access = ' %s' % ' \
'.join(self.access)<br> </span>@@ -192,7 +191,7 @@<br>
def is_covered_localvars(self, other_rule):<br>
<span class=""> '''check if other_rule is covered by this \
rule object'''<br> <br>
</span>- if not other_rule.access and not other_rule.all_accesss:<br>
+ if not other_rule.access and not other_rule.all_access:<br>
<span class=""> raise AppArmorBug('No access specified in \
other signal rule')<br> <br>
</span><span class=""> if not other_rule.signal and not \
other_rule.all_signals:<br> </span>@@ -201,8 +200,8 @@<br>
<span class=""> if not other_rule.peer and not other_rule.all_peers:<br>
</span><span class=""> raise AppArmorBug('No peer specified in \
other signal rule')<br> <br>
</span>- if not self.all_accesss:<br>
- if other_rule.all_accesss:<br>
+ if not self.all_access:<br>
+ if other_rule.all_access:<br>
return False<br>
if other_rule.access != self.access:<br>
return False<br>
@@ -229,7 +228,7 @@<br>
<span class=""> raise AppArmorBug('Passed non-signal rule: \
%s' % str(rule_obj))<br> <br>
</span> if (self.access != rule_obj.access<br>
- or self.all_accesss != rule_obj.all_accesss):<br>
+ or self.all_access != rule_obj.all_access):<br>
return False<br>
<br>
if (self.signal != rule_obj.signal<br>
@@ -245,7 +244,7 @@<br>
return True<br>
<br>
def logprof_header_localvars(self):<br>
- if self.all_accesss:<br>
+ if self.all_access:<br>
<span class=""> access = _('ALL')<br>
</span> else:<br>
access = ' '.join(sorted(self.access))<br>
=== modified file ./utils/test/test-signal.py<br>
--- utils/test/test-signal.py 2015-11-19 17:42:26.329879090 +0100<br>
+++ utils/test/test-signal.py 2015-11-19 17:46:14.472311782 +0100<br>
@@ -25,7 +25,7 @@<br>
_ = init_translation()<br>
<span class=""><br>
exp = namedtuple('exp', ['audit', 'allow_keyword', \
'deny', 'comment',<br> </span>- 'access', \
'all_accesss', 'signal', 'all_signals', 'peer', \
'all_peers'])<br> + 'access', 'all_access', \
'signal', 'all_signals', 'peer', 'all_peers'])<br> \
<span class=""><br> # --- tests for single SignalRule --- #<br>
<br>
</span>@@ -39,7 +39,7 @@<br>
self.assertEqual(expected.peer, obj.peer.regex)<br>
else:<br>
self.assertEqual(expected.peer, obj.peer)<br>
- self.assertEqual(expected.all_accesss, obj.all_accesss)<br>
+ self.assertEqual(expected.all_access, obj.all_access)<br>
self.assertEqual(expected.all_signals, obj.all_signals)<br>
self.assertEqual(expected.all_peers, obj.all_peers)<br>
self.assertEqual(expected.deny, obj.deny)<br>
@@ -125,7 +125,6 @@<br>
<br>
self.assertEqual(obj.get_raw(1), ' signal send set=term \
peer=/usr/bin/pulseaudio///usr/lib/pulseaudio/pulse/gconf-helper,')<br> <br>
-<br>
class SignalFromInit(SignalTest):<br>
tests = [<br>
<span class=""> # SignalRule object \
audit allow deny comment access all? signal \
all? peer all?<br> </span>@@ -140,7 +139,6 @@<br>
def _run_test(self, obj, expected):<br>
self._compare_obj(obj, expected)<br>
<br>
-<br>
class InvalidSignalInit(AATest):<br>
tests = [<br>
<span class=""> # init params expected \
exception<br> </span>@@ -177,7 +175,6 @@<br>
with self.assertRaises(TypeError):<br>
<span class=""> SignalRule('r', 'int')<br>
<br>
</span>-<br>
class InvalidSignalTest(AATest):<br>
def _check_invalid_rawrule(self, rawrule):<br>
obj = None<br>
@@ -309,7 +306,6 @@<br>
<span class=""> ('signal receive,' , [ \
False , False , False , False ]),<br> </span> \
]<br> <br>
-<br>
class SignalCoveredTest_03(SignalCoveredTest):<br>
<span class=""> rule = 'signal send set=quit,'<br>
<br>
</span>@@ -437,9 +433,6 @@<br>
<span class=""> ('deny signal send,' , \
[ False , False , False , False ]),<br> </span> \
]<br> <br>
-<br>
-<br>
-<br>
class SignalCoveredTest_Invalid(AATest):<br>
def test_borked_obj_is_covered_1(self):<br>
<span class=""> obj = SignalRule.parse('signal send \
peer=/foo,')<br> <br>
<br>
<br>
<br>
</span>Regards,<br>
<br>
Christian Boltz<br>
<span class="HOEnZb"><font color="#888888">--<br>
Jaaaa! Und am besten den Rest des Desktops auch noch mit DHTML<br>
nachprogrammieren, ach was sag ich, JavaScript ist ja /die/ Sprache,<br>
um das ganze Betriebsystem neu zu entwickeln.<br>
[Ratti in fontlinge-devel]<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
--<br>
AppArmor mailing list<br>
<a href="mailto:AppArmor@lists.ubuntu.com">AppArmor@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a \
href="https://lists.ubuntu.com/mailman/listinfo/apparmor" rel="noreferrer" \
target="_blank">https://lists.ubuntu.com/mailman/listinfo/apparmor</a><br> \
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div \
class="gmail_signature"><div dir="ltr"><div>Regards,<br><br></div>Kshitij \
Gupta<br></div></div> </div>
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic