[apparmor] [PATCH] utils: Don't use access() to determine readability of profiles file

[Steve Beattie]

On Fri, Jun 19, 2015 at 04:00:19PM -0700, Steve Beattie wrote:
> Something goes horribly wrong with your patch applied, and I'm not sure
> why:
> 
> $ sudo ./aa-status --profiled
> 96
> $ sudo python3  ./aa-status --profiled
> 96
> $ quilt push
> Applying patch ../patches/aa-status-crash.patch
> patching file utils/aa-status
> 
> Now at patch ../patches/aa-status-crash.patch
> $ sudo ./aa-status --profiled
> Traceback (most recent call last):
>   File "./aa-status", line 201, in <module>
>     commands[cmd]()
>   File "./aa-status", line 22, in cmd_profiled
>     profiles = get_profiles()
>   File "./aa-status", line 99, in get_profiles
>     profiles[match.group(1)] = match.group(2)
> TypeError: 'unicode' object does not support item assignment
> $ sudo python3  ./aa-status --profiled
> Traceback (most recent call last):
>   File "./aa-status", line 201, in <module>
>     commands[cmd]()
>   File "./aa-status", line 22, in cmd_profiled
>     profiles = get_profiles()
>   File "./aa-status", line 99, in get_profiles
>     profiles[match.group(1)] = match.group(2)
> TypeError: 'str' object does not support item assignment
> 
> I'm trying to dig into it.

Alright, I tracked it down. In your patch you change the line:

> -    apparmor_profiles = os.path.join(apparmorfs, "profiles")
> +    profiles = os.path.join(apparmorfs, "profiles")

which is a problem, because the 'profiles' is already defined as the
dictionary to store the contents of the file. Reverting this smidgen of
a change fixes the breakage, i.e.:

> +    apparmor_profiles = os.path.join(apparmorfs, "profiles")
> +    try:
> +        f = open(apparmor_profiles)

So with that...

On Fri, Jun 19, 2015 at 11:05:16AM -0500, Tyler Hicks wrote:
> LSMs, such as AppArmor, aren't consulted when a program calls access(2).
> This can result in access(2) returning 0 but a subsequent open(2)
> failing.
> 
> The aa-status utility was doing the access() -> open() sequence and we
> became aware of a large number of tracebacks due to open() failing for
> lack of permissions. This patch catches any IOError exceptions thrown by
> open(). It continues to print the same error message as before when
> access() failed but also prints that error message when AppArmor blocks
> the open of the apparmorfs profiles file.
> 
> https://launchpad.net/bugs/1466768
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>

Acked-by: Steve Beattie <steve at nxnw.org> for trunk and 2.9 as long as the above is
fixed. Thanks!

-- 
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/20150619/06ffc3bd/attachment.pgp>