Hello intrigeri, On Mon, Dec 25, 2017 at 08:55:34AM +0100, [email protected] wrote: > Hi, > > I've upgraded thunderbird from 1:52.5.0-1 to 1:52.5.2-1 in my test sid > VM after double-checking that > /etc/apparmor.d/disable/usr.bin.thunderbird existed and the profile > was not loaded.
I just did testing that upgrading from the version in testing (52.4.0-1) and updating from the previous version in unstable (52.5.0-1) *with* re-enabled AA profile works as wanted. The case you have tested I've unfortunately didn't tested. It tooks me already two day to collect all thinks together with some testing over the x-mas days. It turns out without some more automatic testing this all isn't doable with the needed quality for going online with a package update. I'm feeling massively remembered to the de-branding of Icedove there testing all the possibilities take quite most of the time. > The upgrade removed /etc/apparmor.d/disable/usr.bin.thunderbird > (because it's not shipped as a file owned by the package anymore) and > thus loaded the profile in enforced mode. I think this is not what was > intended with commit 8c57218. No, this was not the intention of this commit. The origin for this all was report #884191 there a user was complaining about a always re-disabled AA profile on updates which he want's to have to be enabled all the time because it was working for him. > I'm setting RC severity because enabling the AppArmor profile breaks > too much functionality, which is why we've decided to disable it > by default. Migrating with this miss function would be not the right thing. So that's o.k to set this severity. Thunderbird isn't migrating to testing anyway right now, some packages in testing still depending on packages from previous thunderbird version which are transitional. > postinst got this added in 1:52.5.2-1: > > # Disable apparmor on new installations and when we're upgrading from > # a version that had it enabled by default > if test -z "$2" || dpkg --compare-versions "$2" le "1:52.5.0-1~"; then > mkdir -p /etc/apparmor.d/disable > ln -s /etc/apparmor.d/usr.bin.thunderbird > /etc/apparmor.d/disable/usr.bin.thunderbird > fi > > The buggy behavior I'm reporting is caused by: > > $ dpkg --compare-versions "1:52.5.0-1" le "1:52.5.0-1~" > $ echo $? > 1 > > … which might be surprising but it does make sense. > > While of course: > > $ dpkg --compare-versions "1:52.5.0-1" le "1:52.5.0-1" > $ echo $? > 0 > > … which is what the comparison should instead have been in the first > place, in the 1:52.5.2-1 upload. I guess the idea was to cover also package versions for stretch-security, jessie-security and also wheezy. And as my small local testing was working I didn't question this much. > But now that we got this wrong once, I think it'll be very hard to > recover and provide the intended behavior in _all_ cases. I think > we'll have to disable the profile on next upgrade again in many cases, > in order to revert the buggy change applied by this upgrade on sid > systems. I believe this could be achieved by replacing the test quoted > above with: > > test -z "$2" || dpkg --compare-versions "$2" le "1:52.5.2-1" > > … which is hopefully equivalent to "the package version we're > upgrading from did forcibly enable the AppArmor profile in at least > some cases when it should not have". > > Sadly, in some cases this will disable the profile even though the > administrator had opted-in and manually enabled it (i.e. > we reintroduce a one-time instance of #884191). I don't know how to > fix this, and IMO we should not block on it before we address the bug > I'm reporting here, but perhaps it's worth a NEWS.Debian entry? In preparation for 52.5.2 I was working on a solution for #884191 in a different way but Guido suggestion was much smaller and more readable than my solution so I was favoring Guidos two lines. Guido was pointing me to files in /etc are normally configuration files and dpkg should respect this also on files and symlinks in /etc/apparmor/disable.d For real files this will probably work and dpkg is detecting a removed file in this folder and will not reinstall a file, but symlinks are ignored right now. This is maybe a bug in debhelper which ignore to add also symlinks to the conffiles list. So we have probably multiple issues to solve, and this with partially workarounds. Back to my originally idea for solving the re-installation of /etc/apparmor/disable.d/usr.bin.thunderbird I was thinking of testing in the preinst of thunderbird for a file or symlink /etc/apparmor/disable.d/usr.bin.thunderbird and writing a marker files in case of this. Later in the postint I wanted to check for this file and also which version is going to be installed over which installed version to remove the symlink from the package if necessary. See added patch of my try. This was also working while I've tested this. I will try re-test this again. I guess we need to look at the reason why dh-installdeb isn't marking the symlinks in /etc not as conffiles and if this is not helping at all if the apparmor triggering can help her more. As we need to disable the enforcing off the AA profile with a new upload at all and we haven't packages of 52.5.2 in testing or any other release I'd skip a NEWS entry in a next upload. Right now the distribution name 'sid" is correct for thunderbird, it's partially broken now. Regards Carsten
>From 4f5c283b1161a407007669b9390e08ddce01e66b Mon Sep 17 00:00:00 2001 From: Carsten Schoenert <[email protected]> Date: Sat, 23 Dec 2017 21:07:03 +0100 Subject: [PATCH] don't disable AA profile on every package update We don't want to disable a re-enabled AA profile while the package update is installed. Closes: 884191 --- debian/thunderbird.postinst | 19 ++++++++++++++++++ debian/thunderbird.preinst | 47 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 debian/thunderbird.preinst diff --git a/debian/thunderbird.postinst b/debian/thunderbird.postinst index ffe5a7971b..24183dbccc 100644 --- a/debian/thunderbird.postinst +++ b/debian/thunderbird.postinst @@ -22,6 +22,23 @@ THUNDERBIRD_PREF=/etc/thunderbird/pref THUNDERBIRD_LIBDIR=/usr/lib/thunderbird TO_DELETE=0 +TB_NEW_INSTALLED_VERSION="$(dpkg -s thunderbird | awk '/^Version:/ {print $2}')" +TB_OLD_INSTALLED_VERSION="$2" + +check_tb_aa_profile_was_enabled() { + # Check if the user has re-enabled the AppArmor profile + # Only do something in case the old installed version is greater than 1:52.5.0-1~ + # and the new installed version is also greater or equal 1:52.5.0-1~ and + # we have found the marker file. + # If the user has installed 1:52.4.0-1 we want to disable the AA profile first! + if dpkg --compare-versions "$TB_OLD_INSTALLED_VERSION" gt "1:52.5.0-1~" && \ + dpkg --compare-versions "$TB_NEW_INSTALLED_VERSION" ge "1:52.5.0-1~" && \ + [ -e /var/run/thunderbird.apparmor_enabled ]; then + test -f /etc/apparmor.d/disable/usr.bin.thunderbird && rm /etc/apparmor.d/disable/usr.bin.thunderbird || true + fi + rm /var/run/thunderbird.apparmor_enabled || true +} + case "$1" in configure) # The users might have put some additional files/dirs into the old @@ -69,6 +86,8 @@ case "$1" in fi fi fi + # Call the check if the AA profile was active before the update + check_tb_aa_profile_was_enabled # Reload AppArmor profile if [ ! -f "/etc/apparmor.d/disable/usr.bin.thunderbird" ] && aa-status --enabled 2>/dev/null; then apparmor_parser -r -T -W "/etc/apparmor.d/usr.bin.thunderbird" || true diff --git a/debian/thunderbird.preinst b/debian/thunderbird.preinst new file mode 100644 index 0000000000..0bf32ad401 --- /dev/null +++ b/debian/thunderbird.preinst @@ -0,0 +1,47 @@ +#!/bin/sh +# preinst script for thunderbird +# +# see: dh_installdeb(1) + +set -e +#set -x + +# summary of how this script can be called: +# * <new-preinst> `install' +# * <new-preinst> `install' <old-version> +# * <new-preinst> `upgrade' <old-version> +# * <old-preinst> `abort-upgrade' <new-version> +# for details, see https://www.debian.org/doc/debian-policy/ or +# the debian-policy package + +check_tb_aa_profile_enabled() { + # check if the user has re-enabled the AppArmor profile + if [ ! -e "/etc/apparmor.d/disable/usr.bin.thunderbird" ]; then + echo "No file /etc/apparmor.d/disable/usr.bin.thunderbird" + # The user has removed the link to disable the AA profile for thunderbird. + # Setting a marker for the postinst script so it can take further action. + echo "This is a marker file for the postinst of thunderbird," > /var/run/thunderbird.apparmor_enabled + echo "it can be savely removed." >> /var/run/thunderbird.apparmor_enabled + fi +} + +case "$1" in + install|upgrade) + check_tb_aa_profile_enabled + ;; + + abort-upgrade) + ;; + + *) + echo "preinst called with unknown argument \`$1'" >&2 + exit 1 + ;; +esac + +# dh_installdeb will replace this with shell code automatically +# generated by other debhelper scripts. + +#DEBHELPER# + +exit 0 -- 2.15.1
