Merge ~ahasenack/ubuntu/+source/frr:jammy-frr-logging into ubuntu/+source/frr:ubuntu/jammy-devel

[Andreas Hasenack]

SRU of the kinetic fix[1]

The linked bug[2] has the SRU template filled out and contains a test case.

1. https://code.launchpad.net/~ahasenack/ubuntu/+source/frr/+git/frr/+merge/424952
2. https://bugs.launchpad.net/bugs/1958162

[Bryce Harrington (bryce)]

Confirmed this is the same exact changes as landed for kinetic (https://code.launchpad.net/~ahasenack/ubuntu/+source/frr/+git/frr/+merge/424952).

I checked if the egrep->grep -E change might be worth including, but I gather the deprecation warning is new and doesn't appear to affect jammy or earlier.

The SRU text looks very thorough and specific. I especially like the thought you give to Where Problems May Occur. Regarding the concern about breaking users who fixed logging in some bespoke fashion, I'd suggest such a type of user would also both have the skills to re/un-fix, and the awareness that their system is non-standard and thus liable to require further attention during upgrade.

Anyway, all looks great, no suggested changes. +1

review: Approve

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

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

review: Approve

[Andreas Hasenack (ahasenack)]

Thanks, uploaded

Uploading frr_8.1-1ubuntu1.1.dsc
Uploading frr_8.1-1ubuntu1.1.debian.tar.xz
Uploading frr_8.1-1ubuntu1.1_source.buildinfo
Uploading frr_8.1-1ubuntu1.1_source.changes

diff --git a/debian/changelog b/debian/changelog
index 27f82a0..a9817b1 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+frr (8.1-1ubuntu1.1) jammy; urgency=medium
+
+  * Fix logging with Ubuntu's unprivileged rsyslog (LP: #1958162):
+    - d/frr.postinst: change log files ownership
+    - d/frr.logrotate: change rotated log file ownership
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 19 Jul 2022 17:36:23 -0300
+
 frr (8.1-1ubuntu1) jammy; urgency=medium
 
   * SECURITY UPDATE: overflow via input packet length
diff --git a/debian/frr.logrotate b/debian/frr.logrotate
index a56a908..f49da46 100644
--- a/debian/frr.logrotate
+++ b/debian/frr.logrotate
@@ -4,7 +4,7 @@
         missingok
         compress
         rotate 14
-        create 640 frr frrvty
+        create 640 syslog adm
 
         postrotate
             pid=$(lsof -t -a -c /syslog/ /var/log/frr/* 2>/dev/null)
diff --git a/debian/frr.postinst b/debian/frr.postinst
index 505ff8e..382edcb 100644
--- a/debian/frr.postinst
+++ b/debian/frr.postinst
@@ -32,7 +32,6 @@ quaggagid=`id -g quagga 2>/dev/null || echo 0`
 
 find \
 	/etc/frr \
-	/var/log/frr \
 		\( -uid 0 -o -uid $quaggauid \) -a \
 		\( -gid 0 -o -gid $quaggauid \) | \
 	while read filename; do
@@ -48,6 +47,30 @@ find \
 	fi
 done
 
+# fix logging for Ubuntu, which does not run rsyslog as root (LP: #1958162),
+# and upgrades from quagga (what the block above used to do also for /var/log)
+
+# frr user was created above, this really shouldn't fail
+frruid=`getent passwd frr | cut -d : -f 3`
+frrgid=`getent group frr | cut -d : -f 3`
+
+find \
+    /var/log/frr \
+        \( -uid 0 -o -uid $quaggauid -o -uid $frruid \) -a \
+        \( -gid 0 -o -gid $quaggauid -o -gid $frrgid \) | \
+    while read filename; do
+
+    # don't chown anything that has ACLs (but don't fail if we don't
+    # have getfacl)
+    if { getfacl -c "$filename" 2>/dev/null || true; } \
+        | egrep -q -v '^((user|group|other)::|$)'; then
+        :
+    else
+        chown syslog:adm "$filename"
+        chmod o-rwx "$filename"
+    fi
+done
+
 # fix misconfigured vtysh.conf & frr.conf ownership caused by config save
 # mishandling in earlier FRR (and Quagga) versions
 find /etc/frr -maxdepth 1 \( -name vtysh.conf -o -name frr.conf \) \