aa-logprof does not include #include <tunables/global> in profiles

oops, forget to write:

and this aa-logprof behavior is , obviously, wrong and should be fixed, imho.

thank you!

In theory, the tunables/global include should always be added - and in practise it usually also is, because everything else would give us tons of bugreports ;-)

Now the question is: How did you create that profile without tunables/global - by using aa-genprof and/or aa-logprof, or was another tool or manual editing involved? Did you do something special?

It would be perfect if you know (and can explain) how to reproduce creating such a buggy profile from scratch, but I'm aware that this might be a bit hard to find out.

I created profile for mariadb , which is empty in Ubuntu 16.04 by using aa-logprof,
just added path and complain flag to it, and, after aa-logprof suggested to include includes like
abstractions/base I got above error.

I guess this is quite easy to reproduce...

Never had such problem before, so it I spent quite long time to find where problem is, so, I think, automation tools like aa-logprof have to generate working profiles.

Thank you!

I'm afraid this is _not_ easy to reproduce. (Well, the load failure when tunables/global is missing probably is, but but we need to find out _why_ tunables/global was missing in the profile.)

IIRC, you are the first one who noticed such a problem in all the years I use and work on AppArmor. I can't imagine what you did to trigger this, because all code I know (I just commited a *big* patch series for the aa-* tools, so I'd say I do know the code) would never forget to include tunables/global.

So - can you please provide an _exact_ way how to reproduce this? Ideally that means to start with a tarball "/etc/apparmor.d/ before adding the mariadb profile", and then continue with the exact steps you did (including a "screenshot" of the aa-logprof run - "aa-logprof | tee /tmp/logprof-screenshot" or saving the console history should do this).

If you can't provide all the details, it would at least be useful to know _how_ you created the profile and "just added path and complain flag" ;-) Which commands did you use? Any edits in $EDITOR or only using the aa-* tools?

Also, what do you mean with "profile for mariadb [...] is empty in Ubuntu 16.04"?

BTW: To make things even more funny, I'm not even using Ubuntu (I use openSUSE since 15 years).

well, about funny, I was first SLES user in my country, this is why I have such nickname ;-)
but I don't use SUSE anylonger, since it was bought by Novell it became nightmare.

Anyway, I'll try to reproduce it tomorrow and I'll provide you all step-by-step info, or , may be even today- I use 16.04 at home desktop, if I'll have time.

About mariadb profile in 16.04, well, it is installed from Ubuntu repo and it contains just:

cat /etc/apparmor.d/usr.sbin.mysqld
# This file is intensionally empty to disable apparmor by default for newer
# versions of MariaDB, while providing seamless upgrade from older versions
# and from mysql, where apparmor is used.
#
# By default, we do not want to have any apparmor profile for the MariaDB
# server. It does not provide much useful functionality/security, and causes
# several problems for users who often are not even aware that apparmor
# exists and runs on their system.
#
# Users can modify and maintain their own profile, and in this case it will
# be used.
#
# When upgrading from previous version, users who modified the profile
# will be promptet to keep or discard it, while for default installs
# we will automatically disable the profile.

That's simple :-D

OK, here it is

1. sudo apt-get install mariadb-server

2. /usr/sbin/mysqld {
}

3. systemctl reload apparmor

4. systemctl start mysql

5. sudo aa-logprof
Reading log entries from /var/log/syslog.
Updating AppArmor profiles in /etc/apparmor.d.
Enforce-mode changes:

Profile: /usr/sbin/mysqld
Path: /etc/ld.so.cache
Mode: r
Severity: 1

  1 - #include <abstractions/base>
  2 - #include <abstractions/evince>
  3 - #include <abstractions/gnome>
  4 - #include <abstractions/kde>
  5 - #include <abstractions/lightdm>
  6 - #include <abstractions/ubuntu-browsers.d/firefox>
  7 - #include <abstractions/ubuntu-browsers.d/kde>
  8 - #include <abstractions/ubuntu-browsers.d/mailto>
  9 - #include <abstractions/ubuntu-gnome-terminal>
  10 - #include <abstractions/ubuntu-konsole>
  11 - #include <abstractions/ubuntu-unity7-base>
 [12 - /etc/ld.so.cache]
(A)llow / [(D)eny] / (I)gnore / (G)lob / Glob with (E)xtension / (N)ew / Abo(r)t / (F)inish / (M)ore

Profile: /usr/sbin/mysqld
Path: /etc/ld.so.cache
Mode: r
Severity: 1

 [1 - #include <abstractions/base>]
  2 - #include <abstractions/evince>
  3 - #include <abstractions/gnome>
  4 - #include <abstractions/kde>
  5 - #include <abstractions/lightdm>
  6 - #include <abstractions/ubuntu-browsers.d/firefox>
  7 - #include <abstractions/ubuntu-browsers.d/kde>
  8 - #include <abstractions/ubuntu-browsers.d/mailto>
  9 - #include <abstractions/ubuntu-gnome-terminal>
  10 - #include <abstractions/ubuntu-konsole>
  11 - #include <abstractions/ubuntu-unity7-base>
  12 - /etc/ld.so.cache
(A)llow / [(D)eny] / (I)gnore / (G)lob / Glob with (E)xtension / (N)ew / Abo(r)t / (F)inish / (M)ore
Adding #include <abstractions/base> to profile.

= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /usr/sbin/mysqld]
(S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t
Writing updated profile for /usr/sbin/mysqld.

6.
sudo systemctl reload apparmor
Job for apparmor.service failed because the control process exited with error code. See "systemctl status apparmor.service" and "journalctl -xe" for details.

cat usr.sbin.mysqld
# Last Modified: Sun Oct 2 18:04:36 2016
# This file is intensionally empty to disable apparmor by default for newer
# versions of MariaDB, while providing seamless upgrade from older versions
# and from mysql, where apparmor is used.
#
# By default, we do not want to have any apparmor profile for the MariaDB
# server. It does not provide much useful functionality/security, and causes
# several problems for users who often are not even aware that apparmor
# exists and runs on their system.
#
# Users can modify and maintain their own profile, and in this case it will
# be used.
#
# When upgrading from previous version, users who modified the profile
# will be promptet to keep or discard it, while for default installs
# we will automatically disable the profile.

/usr/sbin/mysqld {
  #include <abstractions/base>

}

If
>In theory, the tunables/global include should always be added
it is not added by aa-logprof...

Yes, I remember that Novell had the "great" idea to force a pre-alpha package management ("Zenworks") into SLE (IIRC SLE 10) and openSUSE 10.1. On the positive side, they learned that this was a terrible idea, and SUSE developed libzypp and zypper - which turned the Zenworks desaster into the best package management openSUSE and SLE ever had :-) so in the end Zenworks improved things, even if it didn't happen in the way Novell had expected.

Back to the bugreport:

First, thanks for the exact reproducer! I finally understand what you did, and can reproduce the problem (using nearly-2.11 aa-logprof).

The problem is that you start with a hand-made empty profile in step 2 that does not include tunables/global, and then use aa-logprof to extend it. (Creating a profile with aa-genprof will always include tunables/global.)

aa-logprof doesn't check if variables that are used in an abstraction are defined in the profile file - and if the file doesn't have tunables/global, there are big chances that they aren't defined. The question is how we should handle this. Options I can imagine:

a) hardcode to always include tunables/global. This will annoy people who for some reason don't want it, so I don't like this idea too much - even if profiles without tunables/global are very rare.

b) when adding an include, check if all variables are defined. This is possible, but probably some work. It would also mean that aa-logprof must know where those variables are defined, and ask the user about including this file in the global area. That would be something totally new, because right now it only asks about things inside the profile (well, aa-mergeprof already asks about global includes).

c) declare this bugreport as user error - you broke it, so you own both parts ;-)

So: yes, this is a valid bugreport, but it's a corner case and not too high on my TODO list ;-)

No, it was far before..., SLES8 or 9, I reported bug in xfs filesystem to Novell which resulted in kernel crash, and to SGI, SGI fixed it :-),
I patched SUSE's kernel with SGI patch, compiled, and I informed Novell that bug is fixed...
Then, after several weeks, Novell updated kernel- and- surprise!- bug is here. Why?
they I claimed I did not waited for their binary kernel with fix to test, so fix was not included in newer kernels :-D
And only after I tested their binaries they included fix...
At least this is how I remember this - it was more then 10 years ago.
After that I decided that I have no reasons to use SLES , because it became too "enterprise" for me , i.e. too much useless efforts to use it in real life ;-)

>declare this bugreport as user error - you broke it

No, I didn't, not me shipped empty profile in mariadb-server package :-P

> It would also mean that aa-logprof must know where those variables are defined, and ask the user about including this file in the global area.

Well, when aa-logprof asks me about including some abstraction, it may know that this abstraction needs some includes, right? May be we need some dependency mechanism here? I.e. info in include which other includes it needs? Or, may be, just simple- if aa-logprof user decided to add standard include by aa-logprof advice, aa-logprof can also add tunables/global, if
>this will annoy people who for some reason don't want it
then aa-logrof may just warn about this, but, anyway including abstraction without tunables/global will result in broken profile...

Thank you!

For SLE - indeed, that sounds like a funny[tm] story...

> >declare this bugreport as user error - you broke it
>
> No, I didn't, not me shipped empty profile in mariadb-server package :-P

Well, but you added the empty
    /usr/sbin/mysqld {
    }
profile (see comment #6 step 2) to the comment-only file.

I assume you did this step using $EDITOR, right?

sles, thanks for the excellent reproducer.

Christian, I'd love the 'magic' version:
> b) when adding an include, check if all variables are defined.

Of course the user interface might be a bit awkward, especially if the intended use of the abstraction is for the profile author to provide the variable definition in the prologue.

THanks