[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apparmor-dev
Subject:    Re: [apparmor] [patch] Change aa.py to use SignalRule and SignalRuleset
From:       Kshitij Gupta <kgupta8592 () gmail ! com>
Date:       2015-11-18 22:43:38
Message-ID: CAMBXP52zfrYcHdZ8kQ-2NqZPH_MfnauhntyJg68vEgvGZbDYSw () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Accidentally hit reply instead of reply all.
Lets see if this goes through.

On Thu, Nov 19, 2015 at 3:50 AM, Kshitij Gupta <kgupta8592@gmail.com> wrote:

>
>
> On Fri, Oct 23, 2015 at 6:51 PM, Christian Boltz <apparmor@cboltz.de>
> wrote:
>
>> Hello,
>>
>> this patch changes aa.py to use SignalRule and SignalRuleset.
>>
>> This means:
>> - import the classes instead of RE_PROFILE_SIGNAL
>> - simplify signal rule parsing a lot
>> - drop the (now unused) functions parse_signal_rule() and
>> write_signal_rules()
>> - change write_signal() to use the SignalRuleset class
>>
>> Also drop the now unused Raw_Signal_Rule from rules.py.
>>
>> Finally, drop most parser signal tests from the "known wrong results"
>> blacklist in test-parser-simple-tests.py because those tests succeed
>> with SignalRule.
>>
>>
>> [ 09-use-SignalRule.diff ]
>>
>> === modified file ./utils/apparmor/aa.py
>> --- utils/apparmor/aa.py        2015-10-21 22:36:34.763596559 +0200
>> +++ utils/apparmor/aa.py        2015-10-22 23:42:25.162303886 +0200
>> @@ -47,7 +47,7 @@
>>                              RE_PROFILE_BARE_FILE_ENTRY,
>> RE_PROFILE_PATH_ENTRY,
>>                              RE_PROFILE_CHANGE_HAT,
>>                              RE_PROFILE_HAT_DEF, RE_PROFILE_DBUS,
>> RE_PROFILE_MOUNT,
>> -                            RE_PROFILE_SIGNAL, RE_PROFILE_PTRACE,
>> RE_PROFILE_PIVOT_ROOT,
>> +                            RE_PROFILE_PTRACE, RE_PROFILE_PIVOT_ROOT,
>>                              RE_PROFILE_UNIX, RE_RULE_HAS_COMMA,
>> RE_HAS_COMMENT_SPLIT,
>>                              strip_quotes, parse_profile_start_line,
>> re_match_include )
>>
>> @@ -57,6 +57,7 @@
>>  from apparmor.rule.change_profile import ChangeProfileRuleset,
>> ChangeProfileRule
>>  from apparmor.rule.network    import NetworkRuleset,    NetworkRule
>>  from apparmor.rule.rlimit     import RlimitRuleset,    RlimitRule
>> +from apparmor.rule.signal     import SignalRuleset,    SignalRule
>>  from apparmor.rule import parse_modifiers, quote_if_needed
>>
>>  from apparmor.yasti import SendDataToYast, GetDataFromYast, shutdown_yast
>> @@ -463,11 +464,11 @@
>>      profile['change_profile']   = ChangeProfileRuleset()
>>      profile['network']          = NetworkRuleset()
>>      profile['rlimit']           = RlimitRuleset()
>> +    profile['signal']           = SignalRuleset()
>>
>>      profile['allow']['path'] = hasher()
>>      profile['allow']['dbus'] = list()
>>      profile['allow']['mount'] = list()
>> -    profile['allow']['signal'] = list()
>>      profile['allow']['ptrace'] = list()
>>      profile['allow']['pivot_root'] = list()
>>
>> @@ -2919,27 +2921,11 @@
>>              mount_rules.append(mount_rule)
>>              profile_data[profile][hat][allow]['mount'] = mount_rules
>>
>> -        elif RE_PROFILE_SIGNAL.search(line):
>> -            matches = RE_PROFILE_SIGNAL.search(line).groups()
>> -
>> +        elif SignalRule.match(line):
>>              if not profile:
>>                  raise AppArmorException(_('Syntax Error: Unexpected
>> signal entry found in file: %(file)s line: %(line)s') % { 'file': file,
>> 'line': lineno + 1 })
>>
>> -            audit = False
>> -            if matches[0]:
>> -                audit = True
>> -            allow = 'allow'
>> -            if matches[1] and matches[1].strip() == 'deny':
>> -                allow = 'deny'
>> -            signal = matches[2].strip()
>> -
>> -            signal_rule = parse_signal_rule(signal)
>> -            signal_rule.audit = audit
>> -            signal_rule.deny = (allow == 'deny')
>> -
>> -            signal_rules =
>> profile_data[profile][hat][allow].get('signal', list())
>> -            signal_rules.append(signal_rule)
>> -            profile_data[profile][hat][allow]['signal'] = signal_rules
>> +
>> profile_data[profile][hat]['signal'].add(SignalRule.parse(line))
>>
>>          elif RE_PROFILE_PTRACE.search(line):
>>              matches = RE_PROFILE_PTRACE.search(line).groups()
>> @@ -3106,10 +3092,6 @@
>>      # XXX Do real parsing here
>>      return aarules.Raw_Mount_Rule(line)
>>
>> -def parse_signal_rule(line):
>> -    # XXX Do real parsing here
>> -    return aarules.Raw_Signal_Rule(line)
>> -
>>  def parse_ptrace_rule(line):
>>      # XXX Do real parsing here
>>      return aarules.Raw_Ptrace_Rule(line)
>> @@ -3312,22 +3294,10 @@
>>      data += write_mount_rules(prof_data, depth, 'allow')
>>      return data
>>
>> -def write_signal_rules(prof_data, depth, allow):
>> -    pre = '  ' * depth
>> -    data = []
>> -
>> -    # no signal rules, so return
>> -    if not prof_data[allow].get('signal', False):
>> -        return data
>> -
>> -    for signal_rule in prof_data[allow]['signal']:
>> -        data.append('%s%s' % (pre, signal_rule.serialize()))
>> -    data.append('')
>> -    return data
>> -
>>  def write_signal(prof_data, depth):
>> -    data = write_signal_rules(prof_data, depth, 'deny')
>> -    data += write_signal_rules(prof_data, depth, 'allow')
>> +    data = []
>> +    if prof_data.get('signal', False):
>> +        data = prof_data['signal'].get_clean(depth)
>>      return data
>>
>>  def write_ptrace_rules(prof_data, depth, allow):
>> === modified file ./utils/apparmor/rules.py
>> --- utils/apparmor/rules.py     2014-12-17 00:54:04.150444000 +0100
>> +++ utils/apparmor/rules.py     2015-10-22 23:39:29.592585653 +0200
>> @@ -71,9 +71,6 @@
>>  class Raw_Mount_Rule(_Raw_Rule):
>>      pass
>>
>> -class Raw_Signal_Rule(_Raw_Rule):
>> -    pass
>> -
>>  class Raw_Ptrace_Rule(_Raw_Rule):
>>      pass
>>
>> === modified file ./utils/test/test-parser-simple-tests.py
>> --- utils/test/test-parser-simple-tests.py      2015-10-20
>> 23:43:11.058010000 +0200
>> +++ utils/test/test-parser-simple-tests.py      2015-10-23
>> 01:09:18.228609114 +0200
>> @@ -134,27 +134,7 @@
>>      'ptrace/bad_07.sd',
>>      'ptrace/bad_08.sd',
>>      'ptrace/bad_10.sd',
>> -    'signal/bad_01.sd',
>> -    'signal/bad_02.sd',
>> -    'signal/bad_03.sd',
>> -    'signal/bad_04.sd',
>> -    'signal/bad_05.sd',
>> -    'signal/bad_06.sd',
>> -    'signal/bad_07.sd',
>> -    'signal/bad_08.sd',
>> -    'signal/bad_09.sd',
>> -    'signal/bad_10.sd',
>> -    'signal/bad_11.sd',
>> -    'signal/bad_12.sd',
>> -    'signal/bad_13.sd',
>> -    'signal/bad_14.sd',
>> -    'signal/bad_15.sd',
>> -    'signal/bad_16.sd',
>> -    'signal/bad_17.sd',
>> -    'signal/bad_18.sd',
>> -    'signal/bad_19.sd',
>> -    'signal/bad_20.sd',
>> -    'signal/bad_21.sd',
>> +    'signal/bad_21.sd',  # invalid regex
>>      'unix/bad_attr_1.sd',
>>      'unix/bad_attr_2.sd',
>>      'unix/bad_attr_3.sd',
>>
>> Its so nice to see much of this code finally removed, with the new class
> based rules.
>
> Thanks for the patch.
>
> Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
>
>
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>

>
>> Regards,
>>
>> Christian Boltz
>> --
>> Wir brauchen ein "postfixbuchconf"-Kommando, damit wir Autor und Version
>> bestimmen können... ;)        [Patrick Ben Koetter in postfixbuch-users]
>>
>>
>> --
>> AppArmor mailing list
>> AppArmor@lists.ubuntu.com
>> Modify settings or unsubscribe at:
>> https://lists.ubuntu.com/mailman/listinfo/apparmor
>>
>
>
>
> --
> Regards,
>
> Kshitij Gupta
>



-- 
Regards,

Kshitij Gupta

[Attachment #5 (text/html)]

<div dir="ltr">Accidentally hit reply instead of reply all.<br><div \
class="gmail_extra">Lets see if this goes through.<br><br></div><div \
class="gmail_extra"><div class="gmail_quote">On Thu, Nov 19, 2015 at 3:50 AM, Kshitij \
Gupta <span dir="ltr">&lt;<a href="mailto:kgupta8592@gmail.com" \
target="_blank">kgupta8592@gmail.com</a>&gt;</span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div \
class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Oct 23, \
2015 at 6:51 PM, Christian Boltz <span dir="ltr">&lt;<a \
href="mailto:apparmor@cboltz.de" target="_blank">apparmor@cboltz.de</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br> <br>
this patch changes aa.py to use SignalRule and SignalRuleset.<br>
<br>
This means:<br>
- import the classes instead of RE_PROFILE_SIGNAL<br>
- simplify signal rule parsing a lot<br>
- drop the (now unused) functions parse_signal_rule() and write_signal_rules()<br>
- change write_signal() to use the SignalRuleset class<br>
<br>
Also drop the now unused Raw_Signal_Rule from rules.py.<br>
<br>
Finally, drop most parser signal tests from the &quot;known wrong results&quot;<br>
blacklist in test-parser-simple-tests.py because those tests succeed<br>
with SignalRule.<br>
<br>
<br>
[ 09-use-SignalRule.diff ]<br>
<br>
=== modified file ./utils/apparmor/aa.py<br>
--- utils/apparmor/aa.py            2015-10-21 22:36:34.763596559 +0200<br>
+++ utils/apparmor/aa.py            2015-10-22 23:42:25.162303886 +0200<br>
@@ -47,7 +47,7 @@<br>
                                            RE_PROFILE_BARE_FILE_ENTRY, \
RE_PROFILE_PATH_ENTRY,<br>  RE_PROFILE_CHANGE_HAT,<br>
                                            RE_PROFILE_HAT_DEF, RE_PROFILE_DBUS, \
                RE_PROFILE_MOUNT,<br>
-                                          RE_PROFILE_SIGNAL, RE_PROFILE_PTRACE, \
RE_PROFILE_PIVOT_ROOT,<br> +                                          \
                RE_PROFILE_PTRACE, RE_PROFILE_PIVOT_ROOT,<br>
                                            RE_PROFILE_UNIX, RE_RULE_HAS_COMMA, \
                RE_HAS_COMMENT_SPLIT,<br>
                                            strip_quotes, parse_profile_start_line, \
re_match_include )<br> <br>
@@ -57,6 +57,7 @@<br>
  from apparmor.rule.change_profile import ChangeProfileRuleset, \
ChangeProfileRule<br>  from apparmor.rule.network      import NetworkRuleset,      \
NetworkRule<br>  from apparmor.rule.rlimit        import RlimitRuleset,      \
RlimitRule<br> +from apparmor.rule.signal        import SignalRuleset,      \
SignalRule<br>  from apparmor.rule import parse_modifiers, quote_if_needed<br>
<br>
  from apparmor.yasti import SendDataToYast, GetDataFromYast, shutdown_yast<br>
@@ -463,11 +464,11 @@<br>
        profile[&#39;change_profile&#39;]     = ChangeProfileRuleset()<br>
        profile[&#39;network&#39;]               = NetworkRuleset()<br>
        profile[&#39;rlimit&#39;]                 = RlimitRuleset()<br>
+      profile[&#39;signal&#39;]                 = SignalRuleset()<br>
<br>
        profile[&#39;allow&#39;][&#39;path&#39;] = hasher()<br>
        profile[&#39;allow&#39;][&#39;dbus&#39;] = list()<br>
        profile[&#39;allow&#39;][&#39;mount&#39;] = list()<br>
-      profile[&#39;allow&#39;][&#39;signal&#39;] = list()<br>
        profile[&#39;allow&#39;][&#39;ptrace&#39;] = list()<br>
        profile[&#39;allow&#39;][&#39;pivot_root&#39;] = list()<br>
<br>
@@ -2919,27 +2921,11 @@<br>
                    mount_rules.append(mount_rule)<br>
                    profile_data[profile][hat][allow][&#39;mount&#39;] = \
mount_rules<br> <br>
-            elif RE_PROFILE_SIGNAL.search(line):<br>
-                  matches = RE_PROFILE_SIGNAL.search(line).groups()<br>
-<br>
+            elif SignalRule.match(line):<br>
                    if not profile:<br>
                          raise AppArmorException(_(&#39;Syntax Error: Unexpected \
signal entry found in file: %(file)s line: %(line)s&#39;) % { &#39;file&#39;: file, \
&#39;line&#39;: lineno + 1 })<br> <br>
-                  audit = False<br>
-                  if matches[0]:<br>
-                        audit = True<br>
-                  allow = &#39;allow&#39;<br>
-                  if matches[1] and matches[1].strip() == &#39;deny&#39;:<br>
-                        allow = &#39;deny&#39;<br>
-                  signal = matches[2].strip()<br>
-<br>
-                  signal_rule = parse_signal_rule(signal)<br>
-                  signal_rule.audit = audit<br>
-                  signal_rule.deny = (allow == &#39;deny&#39;)<br>
-<br>
-                  signal_rules = \
                profile_data[profile][hat][allow].get(&#39;signal&#39;, list())<br>
-                  signal_rules.append(signal_rule)<br>
-                  profile_data[profile][hat][allow][&#39;signal&#39;] = \
signal_rules<br> +                  \
profile_data[profile][hat][&#39;signal&#39;].add(SignalRule.parse(line))<br> <br>
              elif RE_PROFILE_PTRACE.search(line):<br>
                    matches = RE_PROFILE_PTRACE.search(line).groups()<br>
@@ -3106,10 +3092,6 @@<br>
        # XXX Do real parsing here<br>
        return aarules.Raw_Mount_Rule(line)<br>
<br>
-def parse_signal_rule(line):<br>
-      # XXX Do real parsing here<br>
-      return aarules.Raw_Signal_Rule(line)<br>
-<br>
  def parse_ptrace_rule(line):<br>
        # XXX Do real parsing here<br>
        return aarules.Raw_Ptrace_Rule(line)<br>
@@ -3312,22 +3294,10 @@<br>
        data += write_mount_rules(prof_data, depth, &#39;allow&#39;)<br>
        return data<br>
<br>
-def write_signal_rules(prof_data, depth, allow):<br>
-      pre = &#39;   &#39; * depth<br>
-      data = []<br>
-<br>
-      # no signal rules, so return<br>
-      if not prof_data[allow].get(&#39;signal&#39;, False):<br>
-            return data<br>
-<br>
-      for signal_rule in prof_data[allow][&#39;signal&#39;]:<br>
-            data.append(&#39;%s%s&#39; % (pre, signal_rule.serialize()))<br>
-      data.append(&#39;&#39;)<br>
-      return data<br>
-<br>
  def write_signal(prof_data, depth):<br>
-      data = write_signal_rules(prof_data, depth, &#39;deny&#39;)<br>
-      data += write_signal_rules(prof_data, depth, &#39;allow&#39;)<br>
+      data = []<br>
+      if prof_data.get(&#39;signal&#39;, False):<br>
+            data = prof_data[&#39;signal&#39;].get_clean(depth)<br>
        return data<br>
<br>
  def write_ptrace_rules(prof_data, depth, allow):<br>
=== modified file ./utils/apparmor/rules.py<br>
--- utils/apparmor/rules.py        2014-12-17 00:54:04.150444000 +0100<br>
+++ utils/apparmor/rules.py        2015-10-22 23:39:29.592585653 +0200<br>
@@ -71,9 +71,6 @@<br>
  class Raw_Mount_Rule(_Raw_Rule):<br>
        pass<br>
<br>
-class Raw_Signal_Rule(_Raw_Rule):<br>
-      pass<br>
-<br>
  class Raw_Ptrace_Rule(_Raw_Rule):<br>
        pass<br>
<br>
=== modified file ./utils/test/test-parser-simple-tests.py<br>
--- utils/test/test-parser-simple-tests.py         2015-10-20 23:43:11.058010000 \
                +0200<br>
+++ utils/test/test-parser-simple-tests.py         2015-10-23 01:09:18.228609114 \
+0200<br> @@ -134,27 +134,7 @@<br>
        &#39;ptrace/<a href="http://bad_07.sd" rel="noreferrer" \
                target="_blank">bad_07.sd</a>&#39;,<br>
        &#39;ptrace/<a href="http://bad_08.sd" rel="noreferrer" \
                target="_blank">bad_08.sd</a>&#39;,<br>
        &#39;ptrace/<a href="http://bad_10.sd" rel="noreferrer" \
                target="_blank">bad_10.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_01.sd" rel="noreferrer" \
                target="_blank">bad_01.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_02.sd" rel="noreferrer" \
                target="_blank">bad_02.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_03.sd" rel="noreferrer" \
                target="_blank">bad_03.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_04.sd" rel="noreferrer" \
                target="_blank">bad_04.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_05.sd" rel="noreferrer" \
                target="_blank">bad_05.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_06.sd" rel="noreferrer" \
                target="_blank">bad_06.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_07.sd" rel="noreferrer" \
                target="_blank">bad_07.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_08.sd" rel="noreferrer" \
                target="_blank">bad_08.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_09.sd" rel="noreferrer" \
                target="_blank">bad_09.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_10.sd" rel="noreferrer" \
                target="_blank">bad_10.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_11.sd" rel="noreferrer" \
                target="_blank">bad_11.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_12.sd" rel="noreferrer" \
                target="_blank">bad_12.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_13.sd" rel="noreferrer" \
                target="_blank">bad_13.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_14.sd" rel="noreferrer" \
                target="_blank">bad_14.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_15.sd" rel="noreferrer" \
                target="_blank">bad_15.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_16.sd" rel="noreferrer" \
                target="_blank">bad_16.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_17.sd" rel="noreferrer" \
                target="_blank">bad_17.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_18.sd" rel="noreferrer" \
                target="_blank">bad_18.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_19.sd" rel="noreferrer" \
                target="_blank">bad_19.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_20.sd" rel="noreferrer" \
                target="_blank">bad_20.sd</a>&#39;,<br>
-      &#39;signal/<a href="http://bad_21.sd" rel="noreferrer" \
target="_blank">bad_21.sd</a>&#39;,<br> +      &#39;signal/<a href="http://bad_21.sd" \
                rel="noreferrer" target="_blank">bad_21.sd</a>&#39;,   # invalid \
                regex<br>
        &#39;unix/<a href="http://bad_attr_1.sd" rel="noreferrer" \
                target="_blank">bad_attr_1.sd</a>&#39;,<br>
        &#39;unix/<a href="http://bad_attr_2.sd" rel="noreferrer" \
                target="_blank">bad_attr_2.sd</a>&#39;,<br>
        &#39;unix/<a href="http://bad_attr_3.sd" rel="noreferrer" \
target="_blank">bad_attr_3.sd</a>&#39;,<br> <br></blockquote></div></div><div>Its so \
nice to see much of this code finally removed, with the new class based \
rules.<br><br>Thanks for the patch.<br><br>Acked-by: Kshitij Gupta &lt;<a \
href="mailto:kgupta8592@gmail.com" \
target="_blank">kgupta8592@gmail.com</a>&gt;<br><br></div></div></div></div></blockquote><div><br>Acked-by: \
Kshitij Gupta &lt;<a href="mailto:kgupta8592@gmail.com" \
target="_blank">kgupta8592@gmail.com</a>&gt; <br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div \
class="gmail_quote"><div></div><span class=""><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> <br>
Regards,<br>
<br>
Christian Boltz<br>
<span><font color="#888888">--<br>
Wir brauchen ein &quot;postfixbuchconf&quot;-Kommando, damit wir Autor und \
Version<br> bestimmen können... ;)            [Patrick Ben Koetter in \
postfixbuch-users]<br> <br>
<br>
--<br>
AppArmor mailing list<br>
<a href="mailto:AppArmor@lists.ubuntu.com" \
target="_blank">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> \
</font></span></blockquote></span></div><span class=""><font color="#888888"><br><br \
clear="all"><br>-- <br><div><div dir="ltr"><div>Regards,<br><br></div>Kshitij \
Gupta<br></div></div> </font></span></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></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