samba smbd.service is missing ExecStartPre for update-apparmor-samba-profile

[Alex Murray]

In mantic, the smbd.service unit file contained the line:

ExecStartPre=/usr/share/samba/update-apparmor-samba-profile

As such, the associated AppArmor profile for smbd etc would be automatically updated to include permissions for the various shares etc on the local files system.

Since debian version 2:4.19.4+dfsg-1 this is not included anymore since we are not using the patched version of smb.service.in from packaging/systemd and instead are using one maintained directly in debian/samba.smbd.service - as such, the existing patch d/p/smbd.service-Run-update-apparmor-samba-profile-befor.patch should be dropped and instead the file debian/samba.smbd.service should be updated to include this ExecStartPre line.

ProblemType: Bug
DistroRelease: Ubuntu 24.04
Package: samba 2:4.19.5+dfsg-4ubuntu9
ProcVersionSignature: Ubuntu 6.8.0-22.22-generic 6.8.1
Uname: Linux 6.8.0-22-generic x86_64
ApportVersion: 2.28.1-0ubuntu2
Architecture: amd64
BothFailedConnect: Yes
CasperMD5CheckResult: unknown
CloudArchitecture: x86_64
CloudBuildName: server
CloudID: lxd
CloudName: lxd
CloudPlatform: lxd
CloudSerial: 20240407
CloudSubPlatform: LXD socket API v. 1.0 (/dev/lxd/sock)
Date: Mon Apr 22 06:30:04 2024
NmbdLog:

ProcEnviron:
 LANG=C.UTF-8
 PATH=(custom, no user)
 SHELL=/bin/bash
 TERM=xterm-256color
 XDG_RUNTIME_DIR=<set>
RebootRequiredPkgs: Error: path contained symlinks.
SambaServerRegression: Yes
SmbConfIncluded: Yes
SmbLog:

SourcePackage: samba
TestparmExitCode: 0
UpgradeStatus: No upgrade log present (probably fresh install)

[Alex Murray (alexmurray)]

Forwarded to debian in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069661

[Andreas Hasenack (ahasenack)]

Hm, this was introduced in this version, and I missed it in the subsequent merge:
samba (2:4.19.4+dfsg-1) unstable; urgency=medium

  * new upstream stable/bugfix release. See WHATSNEW.txt for details.
  * d/control: drop pkg.samba.nouring build profile: was needed
    for focal which we do not support anymore
  * remove /etc/cron.daily/samba: there's no reason to keep backing it up,
    most stuff is in ldb/tdb files these days.
  * d/samba.maintscript, d/winbind.maintscript:
    remove old rm_connfiles (pre-buster versions)
  * d/rules, d/*.service: provide .service files directly instead of renaming
    and patching upstream templates, and use dh_installsystemd to install them
    (partially Closes: #1059187)
  * d/rules: run dh_movetousr for libpam-winbind & libnss-winbind, if exists.
    This fixes remaining files in /lib (hopefully). In a search for better
    way to detect where to put system libs (/lib vs /usr/lib) as a configure
    option. Closes: #1059187

 -- Michael Tokarev <email address hidden> Mon, 08 Jan 2024 19:11:37 +0300

[Andreas Hasenack (ahasenack)]

Given that:
- samba's apparmor profile is not enabled by default. One has to install apparmor-profiles, and put the profile in enforce mode (it's in complain mode by default)
- we are in the release week, isos are being built. This could at most be a 0-day SRU
- we have been running without this fix since at least January, and enabling it now could be risky (every smbd start will invoke a shell script which could make the service fail

I think a normal SRU for noble is best. I could even add a set of DEP8 tests for this, to see how samba works with the profile from apparmor-profiles.

The only downside I see is that not applying the fix now would mean a release-upgrade regression for the users who are using an apparmored samba. We can release note it for now.

What do you think?

[Andreas Hasenack (ahasenack)]

I added a noble release notes entry for this: https://discourse.ubuntu.com/t/noble-numbat-release-notes/39890#samba-apparmor-profile-91

[Alex Murray (alexmurray)]

There should not be much risk of regression - this feature was only supported on samba in mantic, not jammy etc so not many users will upgrade from mantic to noble - and the current behaviour where this is broken in noble is the same behaviour as we have in jammy etc. And then even for users upgrading from mantic, this feature is about samba accurately reflecting changes in its configuration into the apparmor policy - so is only needed if you are making changes to the samba configuration to add new shares etc.

So I think this is fine for a post-release SRU - no need for it to land for the actual release.