[apparmor] [patch] is_known_rule(): check includes recursively

[Steve Beattie]

On Mon, Jun 22, 2015 at 10:14:01PM +0200, Christian Boltz wrote:
> Hello,
> 
> is_known_rule() in aa.py checked only direct includes, but not includes
> in the included files. As a result, aa-logprof asked about things that
> are already covered by an indirect include.
> 
> For example, the dovecot/auth profile includes abstractions/nameservice,
> and abstractions/nameservice includes abstractions/nis, which contains
> "capability net_bind_service,".
> Nevertheless, aa-logprof asked to add capability net_bind_service.
> 
> Reproducer: (asks for net_bind_service without this patch, should not
> ask for anything after applying the patch):
> python3 aa-logprof -d ../profiles/apparmor.d/ -f <(echo 'type=AVC msg=audit(1415403814.628:662): apparmor="ALLOWED" operation="capable" profile="/usr/lib/dovecot/auth" pid=15454 comm="auth" capability=13  capname="net_bind_service"')
> 
> The patch adds code to check include files included by other include
> files. Note that python doesn't allow to change a list while looping
> over it, therefore we have to use "while incname" as workaround. 
> 
> This fixes a regression for network rules (this patch is based on the
> old match_net_include() code). Funnily it "only" fixes capability rule
> handling (without the "regression" part) because the old
> match_cap_include() didn't do the recursive include handling.
> 
> 
> 
> [ 55-is_known_rule-check-includes-recursively.diff ]
> 
> === modified file utils/apparmor/aa.py
> --- utils/apparmor/aa.py        2015-06-21 19:02:13.797642711 +0200
> +++ utils/apparmor/aa.py        2015-06-22 21:56:15.601218138 +0200
> @@ -4087,11 +4087,28 @@
>          if profile[rule_type].is_covered(rule_obj, False):
>              return True
>  
> -    for incname in profile['include'].keys():
> +    includelist = list(profile['include'].keys())
> +    checked = []
> +
> +    if includelist:
> +        incname = includelist.pop(0)
> +
> +    while incname:

Why not restructure this as:

    while len(includelist) > 0:
        incname = includelist.pop(0)

It gets rid of the need for the pre-loop test, the awkward
if-then construction at the end of the loop *and* the need for the
59-is_known_rule-init-incname.diff patch.

> +        checked.append(incname)
> +
>          if include[incname][incname].get(rule_type, False):
>              if include[incname][incname][rule_type].is_covered(rule_obj, False):
>                  return True
>  
> +        for childinc in include[incname][incname]['include'].keys():
> +            if childinc not in checked:
> +                includelist += [childinc]
> +
> +        if len(includelist):
> +            incname = includelist.pop(0)
> +        else:
> +            incname = False
> +
>      return False
>  

-- 
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/20150706/d7f3ae66/attachment.pgp>