[apparmor] [patch] write_net_rules() fixes, part 3

[Steve Beattie]

On Fri, Apr 10, 2015 at 09:51:34PM +0200, Christian Boltz wrote:
> Hello,
> 
> thanks to the used data structure, write_net_rules() replaces bare
> 'network,' rules with the invalid 'network all,' when saving a profile.
> This patch makes sure a correct 'network,' rule is written.
> 
> Also reset 'audit' to avoid all (remaining) rules get the audit flag
> after writing an audit network rule.
> 
> Note: The first section of the function (that claims to be responsible
> for bare 'network,' rules) is probably never hit - but I'm not too keen
> to remove it and try it out ;-)
> 
> I propose this patch for trunk and 2.9.

Acked-by: Steve Beattie <steve at nxnw.org> for trunk and 2.9. Thanks.

> That's the 3rd fix for write_net_rules() in two days. In the meantime,
> I'm slightly surprised that it worked at all ;-) and I'm even more
> surprised we didn't get any bugreport about those issues.
> 
> Even with those fixes, handling of network rules is still buggy. For
> example, I noticed deny rules loose the audit flag (which might be a
> case of "last rule wins", and that sounds even worse), and I also found
> parse_profile_data() crashing on a profile with
>     network inet6,
>         network inet6 raw,
> because these two (overlapping) rules collide in the hasher data
> structure. Needless to say that fixing this would need quite some
> changes.
> 
> In case you are interested - I tested with this dummy profile:
> 
> /usr/bin/ping {
>   audit network,
>   audit deny network bluetooth,
>   network inet6,
>   network inet6 raw,
>   deny network bluetooth,
> }
> 
> 
> That said - I'll focus on writing the NetworkRule class instead of
> hunting more bugs in the current, obviously broken code. That sounds
> more reasonable and probably comes with less bugs than the current
> implementation.
> 
> 
> [ 38-fix-write_net_rules-network-all.diff ]
> 
> === modified file utils/apparmor/aa.py
> --- utils/apparmor/aa.py        2015-04-09 13:26:40.395241456 +0200
> +++ utils/apparmor/aa.py        2015-04-10 21:16:26.773608745 +0200
> @@ -3399,10 +3399,14 @@
>              data.append('%s%snetwork,' % (pre, audit))
>          else:
>              for fam in sorted(prof_data[allow]['netdomain']['rule'].keys()):
> +                audit = ''
>                  if prof_data[allow]['netdomain']['rule'][fam] is True:
>                      if prof_data[allow]['netdomain']['audit'][fam]:
>                          audit = 'audit '
> -                    data.append('%s%s%snetwork %s,' % (pre, audit, allowstr, fam))
> +                    if fam == 'all':
> +                        data.append('%s%s%snetwork,' % (pre, audit, allowstr))
> +                    else:
> +                        data.append('%s%s%snetwork %s,' % (pre, audit, allowstr, fam))
>                  else:
>                      for typ in sorted(prof_data[allow]['netdomain']['rule'][fam].keys()):
>                          if prof_data[allow]['netdomain']['audit'][fam].get(typ, False):
> 
> 
> 
> Regards,
> 
> Christian Boltz
> -- 
> Hm, mich hat Frust in meiner Linuxanfangszeit doch eher beflügelt, 
> ich hab mir gedacht, dem Schrotthaufen Code zeig ich mal, wer die 
> Hosen anhat. Wobei, ich zappel wohl hier immer noch eher in einem 
> Strampelanzug herum ;) [Thorsten von Plotho-Kettner in suse-linux]
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150410/a215a2d3/attachment-0001.pgp>