Merge ~ahasenack/ubuntu/+source/frr:kinetic-syslog-user-not-present into ubuntu/+source/frr:ubuntu/kinetic-devel

[Andreas Hasenack]

Don't take any action if the syslog user does not exist, as that is a strong indication of local configuration changes, and our chown (later on) would not work anyway.

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/frr-no-syslog/

The linked bug has the SRU details filled in, including test cases and other information like a justification.

Diff review hint: use "git show -w HEAD^" to ignore whitespace changes.

[Lucas Kanashiro (lucaskanashiro)]

Thanks for the fix Andreas. The SRU bug described well the issue and how to reproduce/test it. I am not sure what was your rationale to rule out the creation of syslog user in postinst which would be the other possible option, however, I agree with the proposed solution, being more cautious and checking if the user exists before using it seems reasonable to me.

I did test it in a kinetic container and the package in your PPA does fix the issue (following the Test Plan section of the bug). I am not testing all the others since the change is pretty much the same.

FWIW DEP-8 tests are also passing:

autopkgtest [16:23:45]: @@@@@@@@@@@@@@@@@@@@ summary
zebra-lo PASS
bgpd-snmp-rpki PASS
py-frr-reload PASS

review: Approve

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

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

review: Approve

[Andreas Hasenack (ahasenack)]

Uploaded:
Uploading frr_8.1-1ubuntu3.1.dsc
Uploading frr_8.1-1ubuntu3.1.debian.tar.xz
Uploading frr_8.1-1ubuntu3.1_source.buildinfo
Uploading frr_8.1-1ubuntu3.1_source.changes

diff --git a/debian/changelog b/debian/changelog
index 6a923e5..b3ab065 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+frr (8.1-1ubuntu3.1) kinetic; urgency=medium
+
+  * d/frr.postinst: don't change log ownership if the syslog user
+    doesn't exist. Thanks to Alessandro Ratti
+    <[e-mail]> for the fix (LP: #1991812).
+
+ -- Andreas Hasenack <[e-mail]>  Fri, 28 Oct 2022 11:37:23 -0300
+
 frr (8.1-1ubuntu3) kinetic; urgency=medium
 
   * SECURITY UPDATE: DoS via out-of-bounds read
diff --git a/debian/frr.postinst b/debian/frr.postinst
index 382edcb..7152e62 100644
--- a/debian/frr.postinst
+++ b/debian/frr.postinst
@@ -50,26 +50,28 @@ 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
+if getent passwd syslog > /dev/null; then
+    # 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
+fi
 
 # fix misconfigured vtysh.conf & frr.conf ownership caused by config save
 # mishandling in earlier FRR (and Quagga) versions