Apparmor profile in 22.04 jammy - fails to start when printing enabled

[asdasda]

[Impact]

Users who have:
a) opted in to confining samba with apparmor (by installing apparmor-profiles); and
b) changed the usr.sbin.smbd and samba-bgqd apparmor profiles to enforce mode;

will experience an error in starting the smbd service in jammy:

[2022/08/25 16:04:05.848067, 0] ../../lib/util/become_daemon.c:119(exit_daemon)
  exit_daemon: daemon failed to start: Samba failed to init printing subsystem, error code 13

This "printing subsystem" is actually a new daemon called samba-bgqd. This errors prevents "smbd" from starting.

The reason it failed to start is that this binary is installed on a different path than what is allowed in the samba apparmor profiles, and as a result its execution is denied.

The chosen fix for this is to change the path of samba-bgqd in the samba apparmor profiles to match where it is actually being installed in the jammy packaging. Changing the actual path in the samba packaging would be a more invasive fix.

In kinetic and later, the installation path of samba-bgqd was changed instead, and requires no changes to the apparmor profiles.

However, once the path in the apparmor profiles was fixed for jammy, another error comes up which also requires an apparmor change. samba-bgqd is using locking when opening the *.tdb files in /run/samba, and that requires an extra "k" flag to apparmor rules that cover that directory and its tdb files.

This bug doesn't affect jammy samba users by default, as they have to complete steps (a) and (b) from above to be impacted. Therefore, on its own, this bug does not warrant an SRU, and we are using the block-proposed-jammy tag to prevent its release until such time when another more SRU-worthy apparmor bug is fixed for Jammy.

[Test Plan]

Make a container for testing:

$ lxc launch ubuntu-daily:jammy jammy-test
$ lxc shell jammy-test

Install the needed packages in this order, with two separate commands:
# apt update
# apt install apparmor-profiles apparmor-utils apparmor-profiles-extra
# apt install samba smbclient cups cups-client

Confirm that you have smbd and samba-bgqd processes confined and in complain mode (check first column):
# ps faxZ | grep -E "(smbd|bgqd)" | grep -v grep

smbd (complain) 2432 ? Ss 0:00 /usr/sbin/smbd --foreground --no-process-group
smbd (complain) 2434 ? S 0:00 \_ /usr/sbin/smbd --foreground --no-process-group
smbd (complain) 2435 ? S 0:00 \_ /usr/sbin/smbd --foreground --no-process-group
smbd//null-/usr/lib/x86_64-linux-gnu/samba/samba-bgqd (complain) 2436 ? S 0:00 \_ /usr/lib/x86_64-linux-gnu/samba/samba-bgqd

Change the samba profiles to enforce mode:
# aa-enforce /etc/apparmor.d/usr.sbin.smbd /etc/apparmor.d/samba-bgqd
Setting /etc/apparmor.d/usr.sbin.smbd to enforce mode.
Setting /etc/apparmor.d/samba-bgqd to enforce mode.

Restart smbd:
# systemctl restart smbd

systemctl won't complain, but smbd failed to start:
# ps faxZ | grep smbd | grep -v grep
#

# tail -2 /var/log/samba/log.smbd
[2022/09/09 18:20:35.200901, 0] ../../lib/util/become_daemon.c:119(exit_daemon)
  exit_daemon: daemon failed to start: Samba failed to init printing subsystem, error code 13

And dmesg on the *host* (not the container) will log a few DENIED messages like this:
[sex set 9 15:20:30 2022] audit: type=1400 audit(1662747635.194:10356): apparmor="DENIED" operation="exec" namespace="root//lxd-jammy-test_<var-snap-lxd-common-lxd>" profile="smbd" name="/usr/lib/x86_64-linux-gnu/samba/samba-bgqd" pid=994396 comm="smbd" requested_mask="x" denied_mask="x" fsuid=1000000 ouid=1000000

After installing the fixed package (and accepting the dpkg conf prompt changes), the new profile will be loaded in complain mode again. So let's put it in enforce mode one more time:

# aa-enforce /etc/apparmor.d/usr.sbin.smbd /etc/apparmor.d/samba-bgqd
Setting /etc/apparmor.d/usr.sbin.smbd to enforce mode.
Setting /etc/apparmor.d/samba-bgqd to enforce mode.

Restart:
# systemctl restart smbd

And confirm that smbd and samba-bgqd are running this time, and in enforce mode:

# ps faxZ | grep -E "(smbd|bgqd)" | grep -v grep
avahi-daemon (complain) 4363 ? Ss 0:00 avahi-daemon: running [j-samba-bgqd-apparmor.local]
smbd (enforce) 6734 ? Ss 0:00 /usr/sbin/smbd --foreground --no-process-group
smbd (enforce) 6736 ? S 0:00 \_ /usr/sbin/smbd --foreground --no-process-group
smbd (enforce) 6737 ? S 0:00 \_ /usr/sbin/smbd --foreground --no-process-group
samba-bgqd (enforce) 6738 ? S 0:00 \_ /usr/lib/x86_64-linux-gnu/samba/samba-bgqd --ready-signal-fd=45 --parent-watch-fd=11 --debuglevel=0 -F

Now that the bgqd daemon is running, let's create an actual printer and interact with it:

First, set a password for the samba "root" user:
# printf "root\nroot\n" | sudo smbpasswd -a root

Create a fake printer:
# lpadmin -p testprinter -E -v /dev/null

Check it's there:
# lpstat -l -p testprinter

Add this section to the end of /etc/samba/smb.conf:
[testprinter]
        browseable = No
        comment = All Printers
        create mask = 0700
        path = /var/spool/samba
        printable = Yes

Restart samba so we don't have to wait for it to pick up the changes:
systemctl restart smbd nmbd

Probe the printer via samba:
# rpcclient -Uroot%root localhost -c 'getprinter testprinter 2'
(some printer related output)

This is the test: confirm dmesg on the *host* has no apparmor DENIED events related to the rpcclient command above.

[Where problems could occur]
An apparmor update will impact all ubuntu users, regardless if they are using samba or not. One has to weigh this carefully with the importance of the bug that is being fixed.

This update will restart apparmor on the target system. All sorts of things can happen due to that:
- all apparmor profiles will be reloaded and reapplied
- if users have modified default profiles in /etc/apparmor.d/* (not inside local/*), they will get a dpkg conf prompt during this update
- in particular, users who have changed the samba profiles to be in enforce mode (via aa-enforce) will definitely get a dpkg conf prompt, because the samba profiles coming in via the apparmor-profiles package are in complain mode. This is good, actually, as it will raise awareness about the change the update is bringing
- if apparmor profile files have syntax mistakes, these will show up at this time, and might end up leaving a service that was confined before, unconfined after the update
- the "k" change is being done in abstractions/samba, instead of samba-bgqd specifically, because it already had a rule to allow "rw" access to *.tdb files in there. That abstraction is only included by other samba profiles at the moment, so the change seems contained, but one might argue that it would be best to add the explicit "k" rule to the samba-bgqd profile instead.

[Other information]

This fix alone does not warrant an apparmor SRU, therefore we are using the block-proposed-jammy tag so that the fix can be bundled with another future apparmor SRU.

Apparmor in Kinetic does not need the samba-bgqd path fix, but it might need the "k" locking one. We are waiting for an apparmor version update that will still happen in Kinetic to evaluate if some change will be needed there.

-------------------original report-------------------

See bug here:
https://bugzilla.opensuse.org/show_bug.cgi?id=1191532

Fix was backported, but the path to samba-bgqd is wrong on 22.04.

Currently apparmor profile has it like this:
/usr/lib*/samba/samba-bgqd

When in fact 22.04 has it on /usr/lib/x86_64-linux-gnu/samba/samba-bgqd

Moreover, the dmesg output failed and it has showed that the 'k' flag is required for the *.tdb files within /etc/apparmor.d/abstractions/samba.

[Paride Legovini (paride)]

Hello asdasda and thanks for this bug report. You are right: in Jammy there is a discrepancy between the samba-bgqd path and its path in the samba-bgqd apparmor profile (from apparmor-profiles). This issue is not present in Kinetic as samba-bgqd has been moved under /usr/libexec, see the following changelog entry:

samba (2:4.16.0+dfsg-1) experimental; urgency=medium

  * move helper programs from /usr/lib/$multiarch/ to /usr/libexec/
    where they belongs. This should not affect users.

While it's tempting to backport this change, from the SRU point of view it's probably more appropriate to fix the path in the apparmor profile, so I'm adding an apparmor task to this bug.

[Michał Małoszewski (michal-maloszewski99)]

I am about to create a branch of it and send the MP.
I have simply used grep to check where /usr/lib*/samba/samba-bgqd occurs when I did checkout to the ubuntu/jammy and I've seen that these files where it occurs are in apparmor profile. I have changed apparmor profile to adapt it to the actual location of the file in jammy. At the end samba-bgqd is in /usr/lib/x86_64-linux-gnu/samba/samba-bgqd.

[Andreas Hasenack (ahasenack)]

Initially I was also in favor of changing the apparmor profile instead of the samba packaging, but after I saw Michał's MP (with this approach), I'm kind of changing my mind, and here is my reasoning:

With the change to apparmor, all ubuntu users, regardless if they use samba or not, will get the update. All of them, because even though the change is in the bin:apparmor-profiles pacakge, it comes from src:apparmor, which builds many other apparmor binary packages and some of them are installed by default in ubuntu.

If instead we made the change in the samba packaging, fixing the binary path, then only samba users would get the update. We can still argue whether it's worth an sru, but at least we could stage the update so it goes out with other more important samba updates.

Paride, what do you think?

[Andreas Hasenack (ahasenack)]

That being said, it's also likely that all desktop users have some samba library installed :/

[Andreas Hasenack (ahasenack)]

The other argument I have against fixing this in apparmor, is that this fix is for jammy only. It's not present in kinetic. But the samba fix (correct path for that binary) is in jammy.

Then the question boils down to which update will cause the less amount of disruption for users...

a) apparmor: apparmor restart (reload all profiles), even for users who are not using samba services
b) samba: if only the libs are installed, maybe needrestart will ask for some restarts? If daemons are installed, then the usual samba restart, even for users who are not using the apparmor profile.

[Paride Legovini (paride)]

I see your point, and I find the one about "fixing it the Kinetic way" especially convincing, it may save us headaches in the future, and may help with backports. My concern is that other packages or scripts users wrote and tested on Jammy systems refer to the "wrong" path (/usr/lib/x86_64-linux-gnu/...). If we want to fix samba I think this is the commit to cherry-pick

https://salsa.debian.org/samba-team/samba/-/commit/3e8438ca59263aee8a2599b47a0dccf2d395e1f4

Note that speaking of binaries the commit message says

  Some of them are callable from other packages, and some
  others are specified in config files.

and that's what I'm a bit worried about. OTOH there may be packages in Ubuntu that got merged from Debian and that already refer to the "right" (libexec/...) path. So I think that to make fixing samba definitely safer/better than fixing apparmor we should

 - Check which binaries are affected by the path change
 - Check how they're meant to be user facing
 - Check if and how other packages are calling them.
   https://codesearch.debian.net/ may help here.

[Andreas Hasenack (ahasenack)]

Hm, I see, it's obviously not just samba-bgqd. Hmm...

[Robie Basak (racb)]

From an SRU perspective bug 1703821 is similar to this, except for dovecot in Bionic. In both cases, a workaround is trivially available since a user can safely modify the profile directly in /etc.

[Michał Małoszewski (michal-maloszewski99)]

Waiting for a review of MP

[Michał Małoszewski (michal-maloszewski99)]

Updated bug description with:

Moreover, the dmesg output failed and it has showed that the 'k' flag is required for the *.tdb files within /etc/apparmor.d/abstractions/samba.

It is required to close that bug in professional way, because the flag is also affected by samba-bgqd.

[Andreas Hasenack (ahasenack)]

Removing the samba task as there is nothing to do there for jammy.

[Andreas Hasenack (ahasenack)]

There is another ongoing apparmor sru for jammy[1] that is using the same version number. That one has been uploaded already for some time and the bug shows it's making progress. I'm not going to ask this change here to be included in that one, because it's further in the SRU process than this one is, and has more complicated considerations.

I will wait for that one to be accepted into proposed and processes, and then upload this one here with a version bump.

I'll remove the server todo tag, since I'm subscribed to the bugs and will be able to monitor the progress of #1994146.

1. https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1994146

[Andreas Hasenack (ahasenack)]

I'll check the current status