Merge ~lvoytek/ubuntu/+source/clamav:clamav-sru-lp1925182-bionic into ubuntu/+source/clamav:ubuntu/bionic-devel

[Lena Voytek]

Cherry-picked from existing Debian fix for version 0.101.1+dfsg-1 at commit c284f2144560e840b6ce50eeb739dd036fec131f

  * Deploy apparmor profile before first start of freshclam daemon.
    - d/control: Add dh-apparmor as a build dependency
    - d/rules: Add dh install override to deploy apparmor profiles
    - d/clamav-daemon.postinst.in: Remove old apparmor profile deployment
    - d/clamav-freshclam.postinst.in: Remove old apparmor profile
      deployment
    Thanks to Sebastian Andrzej Siewior <email address hidden>.

PPA: ppa:lvoytek/clamav-fix-apparmor-profile-sru-bionic

Steps to test:

# lxc launch images:ubuntu/bionic test-failure
# lxc exec test-failure bash

# apt update
# apt dist-upgrade
# apt install -y apparmor apparmor-utils wget software-properties-common

- Install clamav packages of version 1 before current in bionic

# wget https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-freshclam_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-milter_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-testfiles_0.102.4+dfsg-0ubuntu0.18.04.1_all.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamdscan_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/libclamav-dev_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/libclamav9_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-daemon_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-docs_0.102.4+dfsg-0ubuntu0.18.04.1_all.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-base_0.102.4+dfsg-0ubuntu0.18.04.1_all.deb
# apt install -y ./*

- enforce apparmor profile for freshclam

# aa-enforce /usr/bin/freshclam

# apt update
# apt upgrade

- Check status of freshclam and notice that it was unable to restart

# systemctl status clamav-freshclam

clamav-freshclam.service - ClamAV virus database updater
   Loaded: loaded (/lib/systemd/system/clamav-freshclam.service; enabled; vendor preset: enabled)
  Drop-In: /run/systemd/system/clamav-freshclam.service.d
           └─zzz-lxc-service.conf
   Active: failed (Result: exit-code) since Mon 2021-11-15 20:48:40 UTC; 34s ago
     Docs: man:freshclam(1)
           man:freshclam.conf(5)
           https://www.clamav.net/documents
 Main PID: 8785 (code=exited, status=2)

Nov 15 20:48:40 test-failure systemd[1]: Started ClamAV virus database updater.
Nov 15 20:48:40 test-failure freshclam[8785]: WARNING: Ignoring deprecated option SafeBrowsing at /etc/clamav/freshclam.conf:22
Nov 15 20:48:40 test-failure freshclam[8785]: ERROR: Problem with internal logger (UpdateLogFile = /var/log/clamav/freshclam.log).
Nov 15 20:48:40 test-failure freshclam[8785]: ERROR: initialize: libfreshclam init failed.
Nov 15 20:48:40 test-failure freshclam[8785]: ERROR: Initialization error!
Nov 15 20:48:40 test-failure freshclam[8785]: ERROR: Can't open /var/log/clamav/freshclam.log in append mode (check permissions!).
Nov 15 20:48:40 test-failure systemd[1]: clamav-freshclam.service: Main process exited, code=exited, status=2/INVALIDARGUMENT
Nov 15 20:48:40 test-failure systemd[1]: clamav-freshclam.service: Failed with result 'exit-code'.

- Recreate the container but this time include the updated ppa version

# lxc launch images:ubuntu/bionic test-success
# lxc exec test-success bash

# apt update
# apt dist-upgrade
# apt install -y apparmor apparmor-utils wget software-properties-common

- Install clamav packages of version 1 before current in bionic

# wget https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-freshclam_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-milter_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-testfiles_0.102.4+dfsg-0ubuntu0.18.04.1_all.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamdscan_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/libclamav-dev_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/libclamav9_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-daemon_0.102.4+dfsg-0ubuntu0.18.04.1_amd64.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-docs_0.102.4+dfsg-0ubuntu0.18.04.1_all.deb https://launchpad.net/~ubuntu-security-proposed/+archive/ubuntu/ppa/+build/19629559/+files/clamav-base_0.102.4+dfsg-0ubuntu0.18.04.1_all.deb
# apt install -y ./*

- enforce apparmor profile for freshclam

# aa-enforce /usr/bin/freshclam

- Add ppa and update

# add-apt-repository ppa:lvoytek/clamav-fix-apparmor-profile-sru-bionic
# apt update
# apt upgrade

- Check status of freshclam and notice that it now starts properly

# systemctl status clamav-freshclam

clamav-freshclam.service - ClamAV virus database updater
   Loaded: loaded (/lib/systemd/system/clamav-freshclam.service; enabled; vendor preset: enabled)
  Drop-In: /run/systemd/system/clamav-freshclam.service.d
           └─zzz-lxc-service.conf
   Active: active (running) since Mon 2021-11-15 20:53:17 UTC; 1min 22s ago
     Docs: man:freshclam(1)
           man:freshclam.conf(5)
           https://www.clamav.net/documents
 Main PID: 9122 (freshclam)
    Tasks: 1 (limit: 19010)
   CGroup: /system.slice/clamav-freshclam.service
           └─9122 /usr/bin/freshclam -d --foreground=true

Nov 15 20:53:26 test-success freshclam[9122]: Mon Nov 15 20:53:26 2021 -> daily.cvd updated (version: 26354, sigs: 1945178, f-level: 90, builder: raynman)
Nov 15 20:53:26 test-success freshclam[9122]: Mon Nov 15 20:53:26 2021 -> main database available for download (remote version: 62)
Nov 15 20:53:43 test-success freshclam[9122]: Mon Nov 15 20:53:43 2021 -> Testing database: '/var/lib/clamav/tmp.18348c295d/clamav-33ed505d397b14992fffa075c959d4f5.tmp-main.cvd' ...
Nov 15 20:53:48 test-success freshclam[9122]: Mon Nov 15 20:53:48 2021 -> Database test passed.
Nov 15 20:53:48 test-success freshclam[9122]: Mon Nov 15 20:53:48 2021 -> main.cvd updated (version: 62, sigs: 6647427, f-level: 90, builder: sigmgr)
Nov 15 20:53:48 test-success freshclam[9122]: Mon Nov 15 20:53:48 2021 -> bytecode database available for download (remote version: 333)
Nov 15 20:53:48 test-success freshclam[9122]: Mon Nov 15 20:53:48 2021 -> Testing database: '/var/lib/clamav/tmp.18348c295d/clamav-4d4fb7100fb221e038730cde42d70c42.tmp-bytecode.cvd' ...
Nov 15 20:53:49 test-success freshclam[9122]: Mon Nov 15 20:53:49 2021 -> Database test passed.
Nov 15 20:53:49 test-success freshclam[9122]: Mon Nov 15 20:53:49 2021 -> bytecode.cvd updated (version: 333, sigs: 92, f-level: 63, builder: awillia2)

Package Test Results:

autopkgtest [14:18:17]: @@@@@@@@@@@@@@@@@@@@ summary
clamd PASS
client PASS
milter PASS

[Bryce Harrington (bryce)]

Looks great, and thanks for the good paint-by-numbers test case.

I verified the d/rules change in the upstream commit is not needed for bionic's version of the package, and that the postinst change matches exactly with upstream's change. I ran through the test case and verified it is accurate and I got the expected results.

Since this is all technically correct I'm going to mark this approved, however I have one recommendation on the changelog entry, and there's a couple other steps that should be done first before this lands.

With changelog entries for SRUs in particular, I like to mention what the problem was that is being solved, for two reasons. First, for users experiencing this problem it helps clue them in that this version should fix their problem (they may not know what apparmor or daemons are). Second, if someone upgrades their system and experiences some weird unrelated fault, it lets them know your change was probably not what caused it. In this case, probably could simply say "Fixes failure restarting the clamav-freshclam service during upgrades."

One step to do before we land this, is to get it landed in jammy. The SRU process will reject fixes that are missing from the current devel release (or any intervening stable release). We also don't need to deal with SRU paperwork for things landing in -devel.

But the other step is that for landing in bionic, we do need to deal with the SRU paperwork. While that's not technically _required_ in order to upload the package to bionic-proposed, it'll allow it to be reviewed as part of the MP review. Especially for your first few SRUs there's some conventions and tips to pick up in how to craft good SRUs that sail through review.

So, LGTM, approved, but before landing this MP:
1. Update the changelog to make mention of the problem being fixed
2. Create MPs with these same changes for jammy, focal, hirsute, and impish
3. Fill in the SRU template

review: Approve

[Lena Voytek (lvoytek)]

Added the info on the problem being fixed in the changelog. Is the formatting / amount of information on it alright?

[Bryce Harrington (bryce)]

Content looks great, I'd suggest moving the sentence earlier, maybe like:

  * d/clamav-freshclam.postinst.in: Deploy apparmor profile before first start
    of freshclam daemon. Fixes an error where clamav-freshclam fails to start
    automatically after an update when protected by apparmor. Thanks to
    Simon Déziel <email address hidden>.
    (LP: #1925182)

Everyone has a different style for where to put the LP #, I personally like having it at the end on a line by itself since it looks clean to me, but you'll see it done a variety of ways.

[Lena Voytek (lvoytek)]

Alright, got it updated. Thanks!

[Bryce Harrington (bryce)]

Looks good!

Grab me on mattermost if you'd like help or tips for filling in the SRU. And give me a ping once the jammy MP is ready to review.

review: Approve

[Lena Voytek (lvoytek)]

Found the location of the fix for this issue which was added alongside some other fixes after Bionic's release. Used that fix instead to match newer versions. Confirmed both the testing steps and autopkgtests still work

[Bryce Harrington (bryce)]

Thanks for identifying the better fix. Looks good, I've sponsored the upload:

Vcs-Git: https://git.launchpad.net/~bryce/ubuntu/+source/clamav
Vcs-Git-Commit: a1a2f8d4a90cd5e153cd41e6f1594f1a8e004a98
Vcs-Git-Ref: refs/heads/clamav-sru-lp1925182-bionic

$ dput ubuntu ../clamav_0.103.2+dfsg-0ubuntu0.18.04.3_source.changes
Checking signature on .changes
gpg: ../clamav_0.103.2+dfsg-0ubuntu0.18.04.3_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: ../clamav_0.103.2+dfsg-0ubuntu0.18.04.3.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading clamav_0.103.2+dfsg-0ubuntu0.18.04.3.dsc: done.
  Uploading clamav_0.103.2+dfsg-0ubuntu0.18.04.3.debian.tar.xz: done.
  Uploading clamav_0.103.2+dfsg-0ubuntu0.18.04.3_source.buildinfo: done.
  Uploading clamav_0.103.2+dfsg-0ubuntu0.18.04.3_source.changes: done.
Successfully uploaded packages.

Next step will be that it's accepted into the archive, and builds in -proposed. Keep an eye on https://launchpad.net/ubuntu/+source/clamav. After that, next thing to look at is migration issues: https://people.canonical.com/~ubuntu-archive/proposed-migration/bionic/update_excuses.html#clamav. Finally it'll go to sru review.

Most likely there won't be any problems. If there are give me a ping and we'll work through them.

diff --git a/debian/changelog b/debian/changelog
index dd27093..ba246bd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+clamav (0.103.2+dfsg-0ubuntu0.18.04.3) bionic; urgency=medium
+
+  * Deploy apparmor profile before first start of freshclam daemon.
+    - d/control: Add dh-apparmor as a build dependency
+    - d/rules: Add dh install override to deploy apparmor profiles
+    - d/clamav-daemon.postinst.in: Remove old apparmor profile deployment
+    - d/clamav-freshclam.postinst.in: Remove old apparmor profile deployment
+    Thanks to Sebastian Andrzej Siewior <[e-mail]>.
+    (LP: #1925182)
+
+ -- Lena Voytek <[e-mail]>  Mon, 29 Nov 2021 10:13:20 -0700
+
 clamav (0.103.2+dfsg-0ubuntu0.18.04.2) bionic-security; urgency=medium
 
   * SECURITY REGRESSION: clamdscan - MULTISCAN parameter causes
diff --git a/debian/clamav-daemon.postinst.in b/debian/clamav-daemon.postinst.in
index db408de..64652ed 100644
--- a/debian/clamav-daemon.postinst.in
+++ b/debian/clamav-daemon.postinst.in
@@ -543,31 +543,6 @@ EOF
   ;;
 esac
 
-# AppArmor integration
-if [ "$1" = "configure" ]; then
-    APP_PROFILE=/etc/apparmor.d/usr.sbin.clamd
-    if [ -f "$APP_PROFILE" ]; then
-        # Add the local/ include
-        LOCAL_APP_PROFILE=/etc/apparmor.d/local/usr.sbin.clamd
-
-        test -e "$LOCAL_APP_PROFILE" || {
-            tmp=`mktemp`
-        cat <<EOM > "$tmp"
-# Site-specific additions and overrides for usr.sbin.clamd.
-# For more details, please see /etc/apparmor.d/local/README.
-EOM
-            mkdir `dirname $LOCAL_APP_PROFILE` 2>/dev/null || true
-            mv -f "$tmp" "$LOCAL_APP_PROFILE"
-            chmod 644 "$LOCAL_APP_PROFILE"
-        }
-
-        # Reload the profile, including any abstraction updates
-        if aa-status --enabled 2>/dev/null; then
-            apparmor_parser -r -T -W "$APP_PROFILE" || true
-        fi
-    fi
-fi
-
 # dh_installdeb will replace this with shell code automatically
 # generated by other debhelper scripts.
 
diff --git a/debian/clamav-freshclam.postinst.in b/debian/clamav-freshclam.postinst.in
index 3efcce5..8add54a 100644
--- a/debian/clamav-freshclam.postinst.in
+++ b/debian/clamav-freshclam.postinst.in
@@ -33,6 +33,7 @@ FRESHCLAMLOGFILE=/var/log/clamav/freshclam.log
 DEBCONFFILE=/var/lib/clamav/freshclam.conf
 LOGROTFILE=/etc/logrotate.d/clamav-freshclam
 DEBROTFILE=/var/lib/clamav/clamav-freshclam
+DO_RUN_AS_EVALUATION=0
 
 case "$1" in
   configure)
@@ -333,6 +334,23 @@ EOF
     chown "$dbowner":adm $FRESHCLAMCONFFILE
   fi
   
+  DO_RUN_AS_EVALUATION=1
+  ;;
+  abort-upgrade|abort-remove|abort-deconfigure)
+  ;;
+  *)
+  echo "postinst called with unknown argument \`$1'" >&2
+  exit 1
+  ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+if [ $DO_RUN_AS_EVALUATION -eq 1 ]
+then
   if [ "$runas" = 'daemon' ]; then
     update-rc.d clamav-freshclam defaults >/dev/null
     invoke-rc.d clamav-freshclam start
@@ -349,43 +367,8 @@ EOF
     invoke-rc.d clamav-freshclam no-daemon || true
     update-rc.d -f clamav-freshclam disable > /dev/null 2>&1 || true
   fi
-  ;;
-  abort-upgrade|abort-remove|abort-deconfigure)
-  ;;
-  *)
-  echo "postinst called with unknown argument \`$1'" >&2
-  exit 1
-  ;;
-esac
-
-# AppArmor integration
-if [ "$1" = "configure" ]; then
-    APP_PROFILE=/etc/apparmor.d/usr.bin.freshclam
-    if [ -f "$APP_PROFILE" ]; then
-        # Add the local/ include
-        LOCAL_APP_PROFILE=/etc/apparmor.d/local/usr.bin.freshclam
 
-        test -e "$LOCAL_APP_PROFILE" || {
-            tmp=`mktemp`
-        cat <<EOM > "$tmp"
-# Site-specific additions and overrides for usr.bin.freshclam.
-# For more details, please see /etc/apparmor.d/local/README.
-EOM
-            mkdir `dirname $LOCAL_APP_PROFILE` 2>/dev/null || true
-            mv -f "$tmp" "$LOCAL_APP_PROFILE"
-            chmod 644 "$LOCAL_APP_PROFILE"
-        }
-
-        # Reload the profile, including any abstraction updates
-        if aa-status --enabled 2>/dev/null; then
-            apparmor_parser -r -T -W "$APP_PROFILE" || true
-        fi
-    fi
 fi
 
-# dh_installdeb will replace this with shell code automatically
-# generated by other debhelper scripts.
-
-#DEBHELPER#
 
 exit 0
diff --git a/debian/control b/debian/control
index 60b90a6..4bf2ebb 100644
--- a/debian/control
+++ b/debian/control
@@ -11,6 +11,7 @@ Uploaders: Michael Meskes <[e-mail]>,
 Build-Depends: automake,
                check,
                debhelper (>= 11),
+               dh-apparmor,
                dh-strip-nondeterminism,
                electric-fence,
                libbz2-dev,
diff --git a/debian/rules b/debian/rules
index 0f02706..6d03668 100755
--- a/debian/rules
+++ b/debian/rules
@@ -144,6 +144,11 @@ endif
 override_dh_strip:
 	dh_strip --dbgsym-migration=clamav-dbg
 
+override_dh_install:
+	dh_install
+	dh_apparmor -pclamav-freshclam --profile-name=usr.bin.freshclam
+	dh_apparmor -pclamav-daemon --profile-name=usr.sbin.clamd
+
 override_dh_installinit:
 	dh_installinit -pclamav-daemon
 	# Don't change the postinst/postrm scripts for clamav-freshclam, as they need non-standard code.