Merge ~michal-maloszewski99/ubuntu/+source/apparmor:fix-apparmor-jammy into ubuntu/+source/apparmor:ubuntu/jammy-devel

This MP is now correctly based on jammy-devel, but your branch itself is still based on jammy. Please rebase your branch onto jammy-devel, and then force push (no need to resubmit the MP).

> This MP is now correctly based on jammy-devel, but your branch itself is still
> based on jammy. Please rebase your branch onto jammy-devel, and then force
> push (no need to resubmit the MP).

Done. I've been doing it while you're writing that comment :D

SRU bug report to be evaluated.
https://docs.google.com/document/d/1uutUBJWCMGoLLgXPI7a3g-L0Js5GocbOVfUF6tnFQ_o/edit?usp=sharing

I've reviewed the MP and also the SRU suggestion.
=> (patch and SRU) needs fixing

I'll look at this

The SRU text needs some caring, I added comments to the google doc.

This commit message is confusing:
commit 34006daa61e223af4d21c4d6f0885a49ef140296
Author: Michal Maloszewski <email address hidden>
Date: Thu Aug 11 20:27:43 2022 +0200

    Fix the samba-bgqd apparmor profile

    Fix the apparmor profile. It's a minor issue,
    but help users to keep everything in order and
    helps visibility of maintainers. (LP: #1979879)

The fix is basically to adjust the path of the samba-bgqd binary in the samba-bgqd and smbd apparmor profiles to match what is shipped in the package.

From an AppArmor perspective, this LGTM.

Results: (from http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-michal-maloszewski99-apparmor-newppa-lp-1979879/?format=plain)
  apparmor @ amd64:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-michal-maloszewski99-apparmor-newppa-lp-1979879/jammy/amd64/a/apparmor/20220830_165327_6ccda@/log.gz
    30.08.22 16:53:27 ✅ Triggers: apparmor/3.0.4-2ubuntu2.2~ppa2
  apparmor @ arm64:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-michal-maloszewski99-apparmor-newppa-lp-1979879/jammy/arm64/a/apparmor/20220830_172108_49d35@/log.gz
    30.08.22 17:21:08 ✅ Triggers: apparmor/3.0.4-2ubuntu2.2~ppa2
  apparmor @ armhf:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-michal-maloszewski99-apparmor-newppa-lp-1979879/jammy/armhf/a/apparmor/20220830_170614_8966a@/log.gz
    30.08.22 17:06:14 ✅ Triggers: apparmor/3.0.4-2ubuntu2.2~ppa2
  apparmor @ ppc64el:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-michal-maloszewski99-apparmor-newppa-lp-1979879/jammy/ppc64el/a/apparmor/20220830_165357_49d35@/log.gz
    30.08.22 16:53:57 ✅ Triggers: apparmor/3.0.4-2ubuntu2.2~ppa2
  apparmor @ s390x:
    http://autopkgtest.ubuntu.com/results/autopkgtest-jammy-michal-maloszewski99-apparmor-newppa-lp-1979879/jammy/s390x/a/apparmor/20220830_165200_05164@/log.gz
    30.08.22 16:52:00 ✅ Triggers: apparmor/3.0.4-2ubuntu2.2~ppa2

I'm taking another look

I checked and in kinetic the startup of these extra daemons by samba has changed a bit. The reason you didn't see the need for the "k" flag in kinetic is that in the simple testing we have been doing so far (just starting the samba service), the bgqd daemon isn't started.

The moment we start it (after installing cups, cups-client, registering a fake printer, and calling rpcclient on it), we see a flood of warnings in kinetic (when the profiles are in complain mode):

[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.021:6583): apparmor="ALLOWED" operation="file_mmap" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-dcerpcd" name="/usr/libexec/samba/samba-dcerpcd" pid=720096 comm="samba-dcerpcd" requested_mask="r" denied_mask="r" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.045:6584): apparmor="ALLOWED" operation="open" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-dcerpcd" name="/usr/libexec/samba/" pid=720096 comm="samba-dcerpcd" requested_mask="r" denied_mask="r" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.045:6585): apparmor="ALLOWED" operation="file_lock" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-dcerpcd" name="/run/samba/names.tdb" pid=720096 comm="samba-dcerpcd" requested_mask="k" denied_mask="k" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.049:6586): apparmor="ALLOWED" operation="file_lock" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-dcerpcd" name="/run/samba/gencache.tdb" pid=720097 comm="samba-dcerpcd" requested_mask="k" denied_mask="k" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.053:6587): apparmor="ALLOWED" operation="file_lock" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-dcerpcd" name="/run/samba/epmdb.tdb" pid=720097 comm="samba-dcerpcd" requested_mask="k" denied_mask="k" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.053:6588): apparmor="ALLOWED" operation="file_mmap" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-rpcd-classic" name="/usr/libexec/samba/rpcd_classic" pid=720099 comm="rpcd_classic" requested_mask="r" denied_mask="r" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.053:6589): apparmor="ALLOWED" operation="file_mmap" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-rpcd" name="/usr/libexec/samba/rpcd_epmapper" pid=720100 comm="rpcd_epmapper" requested_mask="r" denied_mask="r" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.057:6590): apparmor="ALLOWED" operation="file_mmap" namespace="root//lxd-k-samba-apparmor_<var-snap-lxd-common-lxd>" profile="samba-rpcd" name="/usr/libexec/samba/rpcd_fsrvp" pid=720101 comm="rpcd_fsrvp" requested_mask="r" denied_mask="r" fsuid=1000000 ouid=1000000
[qui set 1 16:27:42 2022] audit: type=1400 audit(1662060465.065:6591): apparmor="ALLOWED" operatio...

For reference, these are the high level steps to get the bgqd service (and others) to react in the kinetic version of samba:

sudo -i
apt install cups cups-client
lpadmin -p testprinter -E -v /dev/null
smbpasswd -a root
systemctl start smbd
lpstat -l -p testprinter
rpcclient -Uroot localhost -c 'getprinter testprinter 2'

Ok, this is good. Can you please update the SRU bug with the SRU template filled out? If it needs changing, it's fine to do it there from now on.

Please update the package in the PPA (https://launchpad.net/~michal-maloszewski99/+archive/ubuntu/apparmor-newppa-lp-1979879). It does not contain what is in this branch. In particular, samba won't start with the apparmor profile that is in the ppa because it's lacking the "k" fix that this branch has.

> Please update the package in the PPA (https://launchpad.net/~michal-maloszewski99/+archive/ubuntu/apparmor-newppa-lp-1979879). It does not contain what is in this branch.

Sorry, my mistake. The rwk change is in abstractions/samba, which ships in the apparmor package, not apparmor-profiles as the rest of the samba profiles.

There is no need for a new upload.

Something that I missed until now is that the ubuntu patches are in their own directory, so I think the patch we are adding here should follow that pattern:

debian/patches/fix-samba-bgqd-apparmor-profile.patch

should be in

debian/patches/ubuntu/fix-samba-bgqd-apparmor-profile.patch

And the path for it in debian/patches/series updated too.

@Andreas, done but there is a problem where the diff with series is above the diff with the patch.
I don't know why it has happened. I have added files to staging area in the correct order. I didn't bump into something like that before. Moreover I think there is some problem because when I have used 'git reflog' to restore commits - I have seen that previous diffs which were correct are in reverse order now.

> @Andreas, done but there is a problem where the diff with series is above the diff with the patch.

This is fine and nothing to worry about.

This looks good, +1.

But let's park it for now, because the samba apparmor profiles in kinetic need work. I tested them there, and found out many changes are needed, and filed https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1990692 for it. Let's wait for the apparmor team to update the package there.

I'm going to approve this MP, but please move it back to "work in progress" while we wait for kinetic's apparmor to be updated. In that way we will have a record of the +1, but at the same time remove this MP from the status page for the time being.

Done, changed for 'work in progress'

I checked kinetic, found one more issue there[1], but I think it doesn't affect jammy, so we don't need to change anything here. I'll do one more pass on jammy with that in mind, though, just to be sure.

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

I'm afraid I found one more thing. Quite the rabbit hole this bug! :)

When actually trying to print something, with a real printer and such, got this DENIED:

[Thu Oct 20 21:26:04 2022] audit: type=1400 audit(1666301164.453:146): apparmor="DENIED" operation="open" profile="samba-bgqd" name="/var/cache/samba/printing/hpcolor.tdb" pid=3711 comm="samba-bgqd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0

So we need a rule like this in the samba-bgqd profile:

/var/cache/samba/printing/*.tdb rwk,

This is pretty much the same thing we need in kinetic[1], but the samba version there is different and the printing system changed such that it's another daemon that needs that rule in kinetic, and not samba-bgqd like is the case in focal.

Could you please add that rule above to the samba-bgqd profile? And adjust the patch description (DEP3 header).

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

Let's change the DEP3 description to this:
Description: Fix samba apparmor profiles wrt samba-bgqd
 The installation path of samba-bgqd in Ubuntu Jammy does not match the
 corresponding apparmor profiles where it's used. In Ubuntu Kinetic and later,
 that path was fixed in the samba packaging, so this patch is not needed there.
 .
 Once samba-bgqd was referenced in the correct path and allowed to start, more
 tweaks were needed for this profile to allow it to run properly:
 - locking ("k") of tdb files in /run/samba
 - read/write access to printer tdb files in /var/cache/samba/printing

For the commit message that adds the patch (currently dbc8179ae0d965b421b0ea277bec4fe9fa1d158d), I would suggest to use the same text as in d/changelog, which is generic enough. Then we leave the details in the dep3 header.

Done

Tested and all good, even on arm64 (so that @multiarch trick worked just fine). I hope by monday LL will be open, then we can make this same last change to the apparmor package in the devel release.