[apparmor] missing 'mr' on binary for usage on containers

[Simon Déziel]

[Impact]

 * rsyslog ships with a (Default disable) apparmor profile.
 * Security sensitive users are in general encouraged to enable such
   profiles but unfortunately due to slightly new behavior of the program
   the profile prevents its usage.
 * Allow the program to map/read its binary to get this working again

[Test Case]

1) Create a 'eoan' container called rs1 here:
  lxc launch ubuntu-daily:e rs1
2) Enter the container
  lxc shell rs1
3) Enable apparmor profile
  rm /etc/apparmor.d/disable/usr.sbin.rsyslogd
  apparmor_parser -r -T -W /etc/apparmor.d/usr.sbin.rsyslogd
  systemctl restart rsyslog
4) notice rsyslog failed to start
  systemctl status rsyslog

[Regression Potential]

 * This is just opening up the apparmor profile a bit. Therefore the only
   regression it could cause IMHO is a security issue. But then what it
   actually allows is reading (not writing!) its own binary which should
   be very safe.
 * Thinking further it came to my mind that package updates (independent
   to the change) might restart services and that means if there is any
   issue e.g. in a local config that worked but now fails (not by this
   change but in general) then the upgrade will not cause, but trigger
   this. This is a general regression risk for any upload, but in this
   case worth to mention as it is about log handling - which if broken -
   makes large scale systems hard to debug.

[Other Info]

 * n/a

---

Issue description:

Enabling the rsyslog (disabled by default) Apparmor profile causes rsyslog to fail to start when running *inside a container*.

Steps to reproduce:

1) Create a 'eoan' container called rs1 here:
  lxc launch ubuntu-daily:e rs1
2) Enter the container
  lxc shell rs1
3) Enable apparmor profile
  rm /etc/apparmor.d/disable/usr.sbin.rsyslogd
  apparmor_parser -r -T -W /etc/apparmor.d/usr.sbin.rsyslogd
  systemctl restart rsyslog
4) notice rsyslog failed to start
  systemctl status rsyslog

Workaround:

  echo ' /usr/sbin/rsyslogd mr,' >> /etc/apparmor.d/local/usr.sbin.rsyslogd
  apparmor_parser -r -T -W /etc/apparmor.d/usr.sbin.rsyslogd
  systemctl restart rsyslog

Additional information:

root@rs1:~# uname -a
Linux rs1 4.15.0-48-generic #51-Ubuntu SMP Wed Apr 3 08:28:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
root@rs1:~# lsb_release -rd
Description: Ubuntu Eoan EANIMAL (development branch)
Release: 19.10
root@rs1:~# dpkg -l| grep -wE 'apparmor|rsyslog'
ii apparmor 2.13.2-9ubuntu6 amd64 user-space parser utility for AppArmor
ii rsyslog 8.32.0-1ubuntu7 amd64 reliable system and kernel logging daemon

ProblemType: Bug
DistroRelease: Ubuntu 19.10
Package: rsyslog 8.32.0-1ubuntu7
ProcVersionSignature: Ubuntu 4.15.0-48.51-generic 4.15.18
Uname: Linux 4.15.0-48-generic x86_64
ApportVersion: 2.20.10-0ubuntu27
Architecture: amd64
Date: Wed May 1 17:36:29 2019
ProcEnviron:
 TERM=xterm-256color
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: rsyslog
UpgradeStatus: No upgrade log present (probably fresh install)

[Christian Ehrhardt  (paelzer)]

This actually is a perfect bug:
- simple case
- solution on a silver plate
- only changing d/* content
- already ubuntu Delta

I feel bad that this hung around so log, but today I saw it and gave it a review.
This is building in Eoan now.

[Simon Déziel (sdeziel)]

On 2019-07-03 10:47 a.m., Christian Ehrhardt  wrote:
> I feel bad that this hung around so log, but today I saw it and gave it a review.
> This is building in Eoan now.

No worries for the delay, I know where to find you if something more
critical is taking too long to my taste ;) Thank you Christian!

Regards,
Simon

[Launchpad Janitor (janitor)]

This bug was fixed in the package rsyslog - 8.1901.0-1ubuntu2

---------------
rsyslog (8.1901.0-1ubuntu2) eoan; urgency=medium

  [ Simon Deziel ]
  * d/usr.sbin.rsyslogd: allow reading/mmap'ing rsyslog binary
    This is required for usage inside containers (LP: #1827253)

 -- Christian Ehrhardt <email address hidden> Wed, 03 Jul 2019 16:34:41 +0200

[Simon Déziel (sdeziel)]

I'm hitting the same problem when using a Bionic host with a Bionic container when using the 5.0 HWE kernel.

@paelzer, I'd appreciate if this could be SRU'ed to Bionic, please :)

[Christian Ehrhardt  (paelzer)]

The fix is trivial, so this should be fine.
But I'm just back from PTO and a bit swamped by TODOs right now, so bear with me as I'll need some time :-)

[Christian Ehrhardt  (paelzer)]

I prepped the two MPs for the SRU as this isn't the biggest impact fix, but OTOH patch-on-a-platter and also easy to review for code/impact.

[Christian Ehrhardt  (paelzer)]

MPs got reviewed and uploaded to D/B SRU queue

[Łukasz Zemczak (sil2100)]

Reviewing this, it feels to me like something nice to have, but low-importance enough that I'd be worried about users getting an upgrade with no impact for them (and, as per the regression potential, some low-chance but still regression risk). The SRU policy is a bit conflicted there: on one side we should only do high-impact bugs *or* low-risk bugs in case of applications. But this is rsyslog, so something a bit more low-level, so it's up to some interpretation as to what the policy thinks about it.

One option would be to accept it into -proposed pockets and set the block-proposed-* tags to not make it migrate, only including it in case some additional SRU gets uploaded on top. Would that be fine? The only problem I see is that rsyslog doesn't seem to have an active SRU history looking a disco and bionic, so the chances this would go out into -updates would be low...

[Christian Ehrhardt  (paelzer)]

Thanks for the review Łukasz,
I somewhat agree with your summary. The reason I went on with it was because it was very low hanging fruit and Simon (Bug reporter) is like "The Ubuntu community good bug reports role model".
But I agree that the update for everyone (it is always installed) might be too much.

I'd be ok getting it only to proposed, we have done so in the past for other things and since it can be changed by the user prio is low anyway.

@Simon - opinions on proposed / abort / convince ?

[Simon Déziel (sdeziel)]

Thanks Łukasz and Christian. I find the block-proposed-* tags idea interesting if that's not too much work on your side.

[Robie Basak (racb)]

Hello Simon, or anyone else affected,

Accepted rsyslog into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/rsyslog/8.32.0-1ubuntu7.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Robie Basak (racb)]

Hello Simon, or anyone else affected,

Accepted rsyslog into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/rsyslog/8.32.0-1ubuntu4.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Simon Déziel (sdeziel)]

I feel really bad now :/

The initial commit that went in doesn't even fix the problem due to a typo/confusion. The proposed manual workaround was OK but the merge proposal was not.

 "/usr/sbin/rsyslog mr," != "/usr/sbin/rsyslogd mr,"

I'm failing the verification and have proposed a new MP. Sorry.

[Christian Ehrhardt  (paelzer)]

We have a new MP that fixes things for real in Focal.

The former SRUs already have been set to verification-failed and can be cancelled from -proposed if it helps to clear up things.

[Launchpad Janitor (janitor)]

This bug was fixed in the package rsyslog - 8.2001.0-1ubuntu1

---------------
rsyslog (8.2001.0-1ubuntu1) focal; urgency=medium

  [ Christian Ehrhardt ]
  * Merge with Debian unstable (LP: #1862762). Remaining changes:
    - debian/00rsyslog.conf Install tmpfiles.d snippet to ensure that the
      syslog group can write into /var/log/.
    - debian/50-default.conf: set of default rules for syslog
      + debian/50-default.conf: separated default rules
      + d/rsyslog.install: install default rules
      + d/rsyslog.postrm: clear default rules on purge
      + d/rsyslog.postrm: remove conf file in postrm on purge. manage with ucf
      + d/rsyslog.postinst: Adapt script to use ucf for Ubuntu's config files
      + debian/control: Add Depends for ucf
    - debian/rsyslog.conf:
      + enable $RepeatedMsgReduction to avoid bloating the syslog file.
      + enable $KLogPermitNonKernelFacility for non-kernel klog messages
      + Run as rsyslog:rsyslog, set $FileOwner to syslog
      + Remove rules moved to 50-default.conf
    - Add disabled by default AppArmor profile, debian/usr.sbin.rsyslogd
      + d/rsyslog.install: install apparmor rule
      + d/rules: use dh_apparmor to install profile before rsyslog is started
      + d/control: suggests apparmor (>= 2.3)
      + d/contrl: Build-Depends on dh-apparmor
      + debian/rsyslog.dirs: install /etc/apparmor.d/force-complain,
        /etc/apparmor.d/disable and /etc/apparmor.d/local
      + d/usr.sbin.rsyslogd apparmor profile for rsyslogd
      + debian/rsyslog.preinst: disable profile on clean installs.
    - d/rules: Fix LDFLAGS to avoid segfault on receipt of first message
    - Drop mmnormalize module, which depends on liblognorm from universe.
      + d/rules: drop --enable-mmnormalize
      + d/control: drop build dependency on liblognorm-dev
    - run as user syslog
      + d/rsyslog.postinst: fix ownership of /var/spool/rsyslog.
      + d/rsyslog.postinst: Create syslog user and add it to adm group
      + d/rsyslog.postinst: Adapt privileges for /var/log
      + debian/control: Add Depends for adduser
    - debian/dmesg.service: provide /var/log/dmesg.log as non log-rotated
      log for boot-time kernel messages.
    - debian/clean: Delete some files left over by the test suite
  * Dropped Changes:
    - d/control: drop rsyslog-mongodb package from suggests
      [ This part was forgotten to be droped in 8.32.0-1ubuntu1 ]
    - d/rules: Build with --disable-silent-rules to get useful build logs.
      [ was a no-op as verbose is the default ]
    - d/rsyslog.postinst: Clean up temporary syslog.service symlink
      [ Formerly missing in Changelog, now gone in Debian as well ]

  [ Simon Deziel ]
  * d/usr.sbin.rsyslogd: apparmor: fix typo in rule for (LP: #1827253).

rsyslog (8.2001.0-1) unstable; urgency=medium

  * New upstream version 8.2001.0
  * Set PYTHON=/usr/bin/python3 in debian/rules
  * Cherry-pick upstream patches which fix a couple of imfile issues
  * Add missing test files

rsyslog (8.1911.0-1) unstable; urgency=medium

  * New upstream version 8.1911.0
  * Follow DEP-14 naming
  * Rebase patches
  * Bump Standards-Version to 4.4.1

rsyslog (8.1910.0-2) unstable; ur...

[Simon Déziel (sdeziel)]

The version in focal-proposed works well, thanks Christian! I hadn't anticipated the additional roadblocks so I really appreciate you pushing it forward nevertheless!

[Rafael David Tinoco (rafaeldtinoco)]

Simon,

Could you please help identifying the current status of this bug ? Your original merge requests were postponed to the next SRU for rsyslog in Bionic, Disco and Eoan, correct ?

During verification time you discovered the difference between "bin" and "sbin" for the apparmor rule, is that it ? Than the verifications have been marked as failed.

From @paelzers comments I see that Focal is good. Should we re-try a SRU for *at least* Bionic here in this bug ? Or just let it hang for documentation purposes if others face this (since SRU is too small to be accepted directly) ?

Any thoughts ?

[Simon Déziel (sdeziel)]

The 1st SRU for Bionic failed because I typo'ed the path to the binary (rsyslog != rsyslogd).
Focal is fixed and Bionic is left with a 'bad' package in bionic-proposed. I don't think redoing the SRU for Bionic is worth it, it's a default *disabled* profile after all.

I'd leave things as is or maybe delete the bionic-proposed package as cleanup if that's easy.

Thanks Rafael!

[Łukasz Zemczak (sil2100)]

The version of rsyslog in the proposed pocket of Bionic that was purported to fix this bug report has been removed because one or more bugs that were to be fixed by the upload have failed verification and been in this state for more than 10 days.

[Christian Ehrhardt  (paelzer)]

I want to look at this again ...

[Christian Ehrhardt  (paelzer)]

Let us kill the SRU - given what happened last time and that this is a
a) disabled by default profile
and
b) user fixable by editing a conffile

If it is ok with you I'd better focus on merging the latest version + your improvement in https://code.launchpad.net/~sdeziel/ubuntu/+source/rsyslog/+git/rsyslog/+merge/382345

[Simon Déziel (sdeziel)]

@Christian, https://code.launchpad.net/~sdeziel/ubuntu/+source/rsyslog/+git/rsyslog/+merge/382345 was a 'drive-by' merge proposal not associated with any LP (is that OK?). As such, I don't consider it related to this bug which can be closed now AFAICT.

[Christian Ehrhardt  (paelzer)]

Yeah I have considered it as nice drive-by improvement unrelated to this bug - and thanks for doing so.
I just had sorted out some lose ends on rsyslog and they both where part of it.

Since this didn't work last time it is not closed yet, but open at low prio.
As explained TBH I think we won't do it (this bug) as SRU a second time.
Oh yeah lets set Won't Fix to reflect that as well.