[prev in list] [next in list] [prev in thread] [next in thread]
List: apparmor-dev
Subject: Re: [apparmor] aa.py: fix netrules_access_check()
From: Kshitij Gupta <kgupta8592 () gmail ! com>
Date: 2014-10-20 19:46:45
Message-ID: CAMBXP52hwmUNk96k-K3GP=V=zKep8CBSAP0OLpPmtvzk_yHFWA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hello,
On Tue, Oct 21, 2014 at 12:19 AM, Christian Boltz <apparmor@cboltz.de>
wrote:
> Hello,
>
> Am Sonntag, 19. Oktober 2014 schrieb Christian Boltz:
> > netrules_access_check() in aa.py checks if
> > type(netrules['rule'][family]) == dict
> > however this check always returns false (at least with py3, I didn't
> > test with py2).
> >
> > This broken type check is the reason for
> > https://bugs.launchpad.net/apparmor/+bug/1380368
> > aa-logprof doesn't propose abstractions for network rules
> > and
> > https://bugs.launchpad.net/apparmor/+bug/1380367
> > aa-logprof asks for already existing network rules
> >
> > The following patch fixes both bugs:
>
>
> Thanks for tracing down relevant bug-reports. Please update the
bug-reports. :-)
> > So the type check (or another check) is still needed. Any ideas how to
> > implement it in a way that works?
>
> After some discussion with Kshitij on #apparmor here's the working
> patch.
>
> Note that the type check itsself is (nearly?) always true, but it's
needed as a safety net because in theory netrules['rule'][family] could
> be boolean True - see line 2994.
>
> Update: cboltz found the type check did avert a crash in case of a rule
like "network inet".
The sock_type in .....keys() check is there to ensure the hasher doesn't
> automagically add an empty sub-dict, which caused the regression in the
> first version of my patch.
>
> ReasonsWhyShouldWeGetRidOfHasherMagicAndMoveToClasses++;
>
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py 2014-10-20 18:07:24 +0000
> +++ utils/apparmor/aa.py 2014-10-20 18:40:53 +0000
> @@ -4460,7 +4460,8 @@
> if netrules['rule'].get(family, False) is True:
> all_net_family = True
> if (netrules['rule'].get(family, False) and
> - type(netrules['rule'][family]) == dict and
> + type(netrules['rule'][family]) == type(hasher()) and
> + sock_type in netrules['rule'][family].keys() and
> netrules['rule'][family][sock_type]):
> net_family_sock = True
>
>
>
>
Thanks for the patch.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>.
Regards,
Kshitij Gupta
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
>
[Attachment #5 (text/html)]
<div dir="ltr">Hello,<br><div class="gmail_extra"><br><div class="gmail_quote">On \
Tue, Oct 21, 2014 at 12:19 AM, 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:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br> <br>
Am Sonntag, 19. Oktober 2014 schrieb Christian Boltz:<br>
<span class="">> netrules_access_check() in aa.py checks if<br>
> type(netrules['rule'][family]) == dict<br>
> however this check always returns false (at least with py3, I didn't<br>
> test with py2).<br>
><br>
> This broken type check is the reason for<br>
> <a href="https://bugs.launchpad.net/apparmor/+bug/1380368" \
target="_blank">https://bugs.launchpad.net/apparmor/+bug/1380368</a><br> > \
aa-logprof doesn't propose abstractions for network rules<br> > and<br>
> <a href="https://bugs.launchpad.net/apparmor/+bug/1380367" \
target="_blank">https://bugs.launchpad.net/apparmor/+bug/1380367</a><br> > \
aa-logprof asks for already existing network rules<br> ><br>
> The following patch fixes both bugs:<br>
<br>
<br></span></blockquote><div>Thanks for tracing down relevant bug-reports. Please \
update the bug-reports. :-)<br> <br></div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><span class=""> </span><span class="">> So the \
type check (or another check) is still needed. Any ideas how to<br> > implement it \
in a way that works?<br> <br>
</span>After some discussion with Kshitij on #apparmor here's the working<br>
patch.<br>
<br>
Note that the type check itsself is (nearly?) always true, but it's \
</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> needed as a safety \
net because in theory netrules['rule'][family] could<br> be boolean True - \
see line 2994.<br> <br></blockquote><div>Update: cboltz found the type check did \
avert a crash in case of a rule like "network \
inet".<br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> The sock_type in \
.....keys() check is there to ensure the hasher doesn't<br> automagically add an \
empty sub-dict, which caused the regression in the<br> first version of my patch.<br>
<br></blockquote><div>ReasonsWhyShouldWeGetRidOfHasherMagicAndMoveToClasses++;<br> \
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
=== modified file 'utils/apparmor/aa.py'<br>
--- utils/apparmor/aa.py 2014-10-20 18:07:24 +0000<br>
+++ utils/apparmor/aa.py 2014-10-20 18:40:53 +0000<br>
@@ -4460,7 +4460,8 @@<br>
<span class=""> if netrules['rule'].get(family, False) is True:<br>
all_net_family = True<br>
if (netrules['rule'].get(family, False) and<br>
- type(netrules['rule'][family]) == dict and<br>
</span>+ type(netrules['rule'][family]) == type(hasher()) \
and<br> + sock_type in netrules['rule'][family].keys() \
and<br> <span class=""> \
netrules['rule'][family][sock_type]):<br> net_family_sock = True<br>
<br>
<br>
<br></span></blockquote><div><br>Thanks for the patch.<br>
<br>
Acked-by: Kshitij Gupta <<a \
href="mailto:kgupta8592@gmail.com">kgupta8592@gmail.com</a>>.<br> <br>
Regards,<br>
<br>
Kshitij Gupta<br></div><div><br></div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><span class=""> </span>Regards,<br>
<br>
Christian Boltz<br>
<span class=""><font color="#888888">--<br>
Programming today is a race between software engineers striving to build<br>
bigger and better idiot-proof programs, and the Universe trying to<br>
produce bigger and better idiots. So far, the Universe is winning.<br>
</font></span><div class=""><div><br>
<br></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px \
0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div \
class="h5">
--<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" \
target="_blank">https://lists.ubuntu.com/mailman/listinfo/apparmor</a><br> \
</div></div></blockquote></div><br></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