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

[Tyler Hicks]

On 2015-06-19 17:09:26, Steve Beattie wrote:
> 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!

Thanks for tracking down this mistake!

Tyler

> 
> -- 
> 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/20150622/6ad87f36/attachment.pgp>