[apparmor] Nested profile problem upgrading from apparmor-2.9.0 to apparmor-2.11.0

[Christian Boltz]

Hello,

Am Mittwoch, 20. Juni 2018, 08:06:44 CEST schrieb apparmor at raf.org:
> Christian Boltz wrote:
> > Am Dienstag, 19. Juni 2018, 07:53:32 CEST schrieb John Johansen:
> > > On 06/18/2018 09:21 PM, apparmor at raf.org wrote:

> Um, should those be triple forward slashes? or only double?

Depends ;-)

AppArmor uses doube slashes as profile separator.

If you see triple slashes, then it's a profile separator + a slash from 
a path, so
    /foo///bar///baz
basically means (adding some spacing between separators and profile 
names):
    /foo  //  /bar  //  /baz
(you'd obviously be less surprised if the profile name was /bin/true 
instead of /foo ;-)

An example with double slashes would be
    foo//bar//baz
which expands to
    foo  //  bar  //  baz

> > I already promised to enhance the tools to support deeper nested
> > profiles, but this will need quite some work, and I don't know yet
> > when I'll have time to do it.
> > 
> > Until then - what's your prefered way the tools should behave?
> > a) current status
> > b) error out if a deep nesting level is found (which would basically
> >    mean a better error message)
> > c) (any better idea?)
> 
> Thanks for identifying the problem. Ideally, any amount of nesting
> should work. 

Yes, as soon as I have time - or someone is faster and sends a big merge 
request ;-)

> For some reason I assumed that when a process called
> another, the profiles had to be nested similarly. I don't know
> why I assumed that.
> 
> However, it used to work (even if the syntax was clumsy) so it
> probably wasn't too crazy an assumption.
> 
> And apparmor_parser still loads it without error (but unloading it
> does give an error but works anyway). It's strange that aa-enforce
> and aa-complain fail but apparmor_parser works.

See John's explanation - the parser and the tools use completely 
different code to parse profiles.

> Given that it's a lot of work to support full nesting and, if the
> profiles don't need to be deeply nested to reflect process parentage,
> then I think that better error messages is probably good enough
> for now.

OK, I sent a merge request to make the list of pending changes even 
longer ;-)  - https://gitlab.com/apparmor/apparmor/merge_requests/136

> > > > But removing it with "apparmor_parser -R usr.sbin.apache2",
> > > > 
> > > > produces:
> > > >  apparmor_parser: Unable to remove
> > > >  "/usr/sbin/apache2//indexcgi//enscript".>
> > > >  
> > > >    Profile doesn't exist
> > > :
> > > :( , that should work. Looks like a bug
> > 
> > Not necessarily. AFAIK the parser unloads child profiles when
> > unloading the main profile:
> > 
> > # echo 'profile foo {
> > 
> >  profile bar {
> >  }
> >  }' |apparmor_parser
> > 
> > # aa-status |grep foo
> > 
> >    foo
> >    foo//bar
> > 
> > # echo 'profile foo {}' | apparmor_parser -R
> > # aa-status |grep foo
> > #
> > 
> > Your external child profiles are defined after the main profile,
> > therefore apparmor_parser tries to unload them after unloading the
> > main profile (including all child profiles).
> 
> Perhaps it could remember which profiles it has unloaded and
> suppress the error if it encounters it again. It's a harmless
> error message so silencing it would be nice.

Feel free to open a bugreport ;-)  (if you do so, please include my 
reproducer) - but I'm afraid it will take a while until we have time to 
fix such a harmless error message.

> > > > Is there something wrong with the above?
> > > > Has the syntax changed for nested profiles?
> > > 
> > > no it hasn't, the code has been slowly being cleaned up and
> > > you have found a regression
> > 
> > I wouldn't be surprised if 2.9 has a similar bug, but with different
> > results - in worst case writing only the first level of child
> > profiles. (Note: I didn't look at the 2.9 code for a long time, so
> > this is only a guess.)
> 
> With 2.9, aa-status does show all of the nested profiles so it
> looks like they were all loaded:

With "writing only the first level of child profiles", I meant saving 
the profile by aa-logprof after it was modified.

Loading profiles into the kernel (which is the base of what aa-status 
shows) is something completely different, and apparmor_parser doesn't 
have problems with deeply nested child profiles.

> > If you want to have the tools working *now*, a possible solution
> > would be to reduce the nesting level. For example, you could change> 
> >   profile /usr/sbin/apache2//indexcgi//mutt//exim4 {
> > to
> >   profile /usr/sbin/apache2//indexcgi--mutt--exim4 {
> > and adjust your Px targets accordingly.
> 
> Thanks. I've done that (but with underscores) and now aa-enforce
> and aa-complain and apparmor_parser all load without problem
> and unloading is fine too.

Now that you mention it - right, there's the problem that the parser 
doesn't like "-" in profile names. IIRC we even have a bugreport for 
that ;-)


Regards,

Christian Boltz
-- 
> > > Because we had feature freeze in January ;)
> > Which is why there were no new features added to YaST since January.
> Hey, we only did the usual bugfixing ;)
That's a bug, not a feature. :-D
[> Christoph Thiel and houghi in opensuse]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20180620/f70bafb9/attachment.sig>