Merge ~ahasenack/ubuntu/+source/tomcat9:kinetic-tomcat9-logging-fix into ubuntu/+source/tomcat9:ubuntu/devel

[Andreas Hasenack]

This fixes tomcat9's logging via ubuntu's unprivileged rsyslog.

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/tomcat9-logging/

# Context

The package sets /var/log/tomcat9 to be 2770 tomcat:adm, so newly created files will inherit the adm group ownership.

There are 4 entities writing to /var/log/tomcat9 and they all need to agree on permissions and ownership:
- rsyslog: catalina.out only, runs as syslog:adm. It can write to /var/log/tomcat9 due to the adm group permission
- logrotate: catalina.out and its rotated logs only. Runs as root, can set any permission/ownership it wants
- java (tomcat9 itself): localhost*, catalina.<date>.log: runs as tomcat:tomcat, manages its own rotation of these logs. The log files end up being owned by group adm because of the group sticky bit in the /var/log/tomcat9 directory
- tomcat9's maintainer scripts (postinst): chown -Rh tomcat:adm /var/log/tomcat9

With the above in mind, consider that there are only two groups of files in /var/log/tomcat9:
- logs produced by the java process directly, which runs as tomcat:tomcat
- single log file written to by rsyslogd, which runs as syslog:adm

Our main problem is with the rsyslogd-produced log file: /var/log/tomcat9/catalina.out

I chose to alter 3 of these writers:
- rsyslog: drop the ownership setting of catalina.out. We run unprivileged and cannot set the owner to tomcat, which is what debian does
- logrotate: switch to syslog:adm. Without this, rsyslog won't be able to write to catalina.out. Here I could have just dropped the "create" and "su" lines, because logrotate will keep the permissions and ownership of the file it is rotating in the new empty file it creates. I also don't think we need to use "copytruncate", this seems to be old history from when catalina.out was produced by tomcat itself, and not via rsyslog. But one thing at a time.
- postinst: add another chown, specific to catalina.out*, to adjust the permissions back to syslog:adm. I could have replaced debian's chown -R with something more elaborate, that would loop over the logs and skip catalina.out*, or chown it differently, but here I wanted to keep our delta simple.

The above changes survive logrotation events ("logrotate -f /etc/logrotate.conf" can be used to test), package upgrade/reinstall events, and even a full rm -rf /var/log/tomcat9/* (i.e., it doesn't rely on pre-creating of log files with correct permissions before a daemon (rsyslog or java) can write to them). It also "fixes" previous installations which will start working wrt logging.

For testing, an easy way to trigger writing to /var/log/tomcat9/catalina.out (which is via rsyslogd) is to install or purge the tomcat9-admin package.

[Christian Ehrhardt  (paelzer)]

Wow there are really a lot of combinations of different actions and their order to consider here.
AFAICS you covered them all well and I approve +1

I have one minimal comment to enhance an explanation that you have made, but that is up to you if you agree.

For Kinetic this really is step forward.

I'm a bit scared of SRUing this despite seeming absolutely correct.
Changing permissions/ownership always has an odd feel of "what if someone manually adapted for this problem".
We have had this in the past and depending on the uncertainty level we guarded the changes by evaluating the situation if it matches the expected one (in this case perm/ownership) but back then all changes were in maintainer scripts, here with conf files and such this would not work.
Did you discuss the SRUability of this with anyone on the SRU team already?

review: Approve

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: ahasenack, paelzer
Uploaders: ahasenack, paelzer
MP auto-approved

review: Approve

[Andreas Hasenack (ahasenack)]

> Changing permissions/ownership always has an odd feel of "what if someone manually adapted for
> this problem".

I had no discussion yet about an SRU, and I share this concern. The biggest issue I think is the maintainer script, which even without our changes (pristine debian) is already forcing a bunch of chown/chmod everytime. Even if an admin changed config files, the maintainer script would still be running chmod/chown on /var/log/tomcat9. I don't really know how someone would have adapted and fixed their system in the meantime, unless they changed the logging directory to be somewhere else entirely. Then all the remaining changes are in config files, which a package upgrade wouldn't override.

I could refrain from my chown in postinst if I detect something changed, but then I would have to include debian's chown/chmod in that if/then/else clause.

165edec... by Andreas Hasenack on 2022-06-27

Expand comment on changing ownership of catalina.out and the rotated
logs

[Andreas Hasenack (ahasenack)]

> back then all changes were in maintainer scripts, here with conf files and such this would not
> work.

Indeed, I was thinking of ways to check the config files, maybe a simple grep looking for the settings I'm changing (fileOwner in rsyslog snippet, and "create" in logrotate snippet), or even md5sums, but the biggest problem is that this are config file *snippets*, i.e., they could be named anything in a .d directory, or the user could have changed the main config file even (logrotate.conf or rsyslog.conf).

[Andreas Hasenack (ahasenack)]

Uploaded:

Uploading tomcat9_9.0.64-2ubuntu1.dsc
Uploading tomcat9_9.0.64-2ubuntu1.debian.tar.xz
Uploading tomcat9_9.0.64-2ubuntu1_source.buildinfo
Uploading tomcat9_9.0.64-2ubuntu1_source.changes

[Andreas Hasenack (ahasenack)]

This migrated. It wasn't merged automatically because of the empty directory problem:

ERROR: Empty directories exist and will disappear on commit, causing
extraneous unintended changes:

webapps/examples/WEB-INF/lib

diff --git a/debian/changelog b/debian/changelog
index 8709e15..db50166 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,16 @@
+tomcat9 (9.0.64-2ubuntu1) kinetic; urgency=medium
+
+  * Fix logging for unprivileged rsyslogd (LP: #1964881):
+    - d/logrotate.template: use syslog:adm for log rotation so that
+      rsyslog can write to the file
+    - d/rsyslog/tomcat9.conf: drop "fileOwner" as it cannot be set by an
+      unprivileged rsyslogd
+    - d/tomcat9.postinst: adjust ownership of catalina.out so that
+      rsyslogd can write to it. Also change the rotated log files for
+      consistency.
+
+ -- Andreas Hasenack <[e-mail]>  Thu, 23 Jun 2022 18:02:52 -0300
+
 tomcat9 (9.0.64-2) unstable; urgency=medium
 
   * Fallback to the default log formatter when systemd isn't used
diff --git a/debian/control b/debian/control
index 7d593a8..27968d7 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: tomcat9
 Section: java
 Priority: optional
-Maintainer: Debian Java Maintainers <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian Java Maintainers <[e-mail]>
 Uploaders:
  tony mancill <[e-mail]>,
  Emmanuel Bourg <[e-mail]>
diff --git a/debian/logrotate.template b/debian/logrotate.template
index 1cf0315..3d76318 100644
--- a/debian/logrotate.template
+++ b/debian/logrotate.template
@@ -5,6 +5,6 @@
   compress
   delaycompress
   missingok
-  create 640 tomcat adm
-  su tomcat adm
+  create 640 syslog adm
+  su syslog adm
 }
diff --git a/debian/rsyslog/tomcat9.conf b/debian/rsyslog/tomcat9.conf
index f09f4f9..0ed099e 100644
--- a/debian/rsyslog/tomcat9.conf
+++ b/debian/rsyslog/tomcat9.conf
@@ -2,6 +2,6 @@
 $template TomcatFormat,"[%timegenerated:::date-year%-%timegenerated:::date-month%-%timegenerated:::date-day% %timegenerated:::date-hour%:%timegenerated:::date-minute%:%timegenerated:::date-second%] [%syslogseverity-text%]%msg%\n"
 
 :programname, startswith, "tomcat9" {
-  action(type="omfile" file="/var/log/tomcat9/catalina.out" Template="TomcatFormat" fileOwner="tomcat" fileCreateMode="0640")
+  action(type="omfile" file="/var/log/tomcat9/catalina.out" Template="TomcatFormat" fileCreateMode="0640")
   stop
 }
diff --git a/debian/tomcat9.postinst b/debian/tomcat9.postinst
index 7e4c9ba..2dad7e2 100644
--- a/debian/tomcat9.postinst
+++ b/debian/tomcat9.postinst
@@ -64,6 +64,9 @@ case "$1" in
 
 	# Grant read/write access to tomcat to the log and cache directories
 	chown -Rh $TOMCAT_USER:adm /var/log/tomcat9/
+	# catalina.out is written to by rsyslogd, which runs as the "syslog" user in Ubuntu
+	# also include rotated catalina.out files, usually with an extension of [0-9]+[.gz]
+	chown syslog:adm /var/log/tomcat9/catalina.out* 2>/dev/null || : # file might not exist yet
 	chmod 2770 /var/log/tomcat9/
 	chown -Rh $TOMCAT_USER:$TOMCAT_GROUP /var/cache/tomcat9/
 	chmod 750 /var/cache/tomcat9/