Merge ~sergiodj/ubuntu/+source/clamav:ifupdown-replacement into ubuntu/+source/clamav:ubuntu/devel

[Sergio Durigan Junior]

This MP implements support for the networkd-dispatcher utility on clamav.

This feature request has been file in bug #1718227 and opened for quite a while now. I submitted a Merge Request to Debian's clamav 2 years ago, but unfortunately it hasn't been reviewed/accepted yet. You can see it here:

https://salsa.debian.org/clamav-team/clamav/-/merge_requests/4

There's also a bug that proposes the same patch:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=999869

networkd-dispatcher is a modern (TM) replacement for the ifupdown scripts. It basically detects changes in the network configuration and executes certain scripts based on these changes. For clamav, we're only interested in running specific script snippets when the network is off, or when it becomes online (which, in networkd-dispatcher's parlance, means "routable").

I'm taking the opportunity to modernize the script a little bit, because it uses old and non-idiomatic shell expressions.

An easy way to test these scripts is to echo something when they start and then bring the ethernet interface down/up. You can monitor the "journalctl -u networkd-dispatcher" output and confirm that the messages show up. Don't forget to start the service beforehand. This has to be done inside a VM. In order to manipulate the network interface, you can use:

# ip link set <INTERFACE> [down|up]

There's a PPA with the proposed change here: https://launchpad.net/~sergiodj/+archive/ubuntu/clamav

[Sergio Durigan Junior (sergiodj)]

dep8 tests:

Results: (from http://autopkgtest.ubuntu.com/results/autopkgtest-mantic-sergiodj-clamav/?format=plain)
  clamav @ amd64:
    15.05.23 02:53:16 Log 🗒️ ✅ Triggers: clamav/0.103.8+dfsg-0ubuntu2~ppa1
  clamav @ arm64:
    12.05.23 21:41:02 Log 🗒️ ✅ Triggers: clamav/0.103.8+dfsg-0ubuntu2~ppa1
  clamav @ armhf:
    12.05.23 21:09:01 Log 🗒️ ✅ Triggers: clamav/0.103.8+dfsg-0ubuntu2~ppa1
  clamav @ ppc64el:
    12.05.23 21:13:01 Log 🗒️ ✅ Triggers: clamav/0.103.8+dfsg-0ubuntu2~ppa1
  clamav @ s390x:
    12.05.23 21:58:26 Log 🗒️ ✅ Triggers: clamav/0.103.8+dfsg-0ubuntu2~ppa1

[Bryce Harrington (bryce)]

I set up a vm and did the suggested test of adding debugging to clamav-freshclam:

set -e

echo "Running clamav-freshclam-ifupdown ${*}" >&2

[ -f /var/lib/clamav/interface ] || exit 0

if [ -d /run/systemd/system ]; then
    ...

I guess the test for the interface file causes the script to exit early so the debug statement seems to need to precede it, however it does appear to get called:

May 26 05:42:27 clamav-test-mantic systemd-timesyncd[11333]: No network connectivity, watching for changes.
May 26 05:42:33 clamav-test-mantic systemd-networkd[11337]: enp5s0: Link UP
May 26 05:42:33 clamav-test-mantic systemd-networkd[11337]: enp5s0: Gained carrier
May 26 05:42:33 clamav-test-mantic systemd-networkd[11337]: enp5s0: DHCPv4 address 10.69.244.74/24, gateway 10.69.244.1 acquired from 10.69.244.1
May 26 05:42:33 clamav-test-mantic systemd-timesyncd[11333]: Network configuration changed, trying to establish connection.
May 26 05:42:33 clamav-test-mantic networkd-dispatcher[15544]: Running clamav-freshclam-ifupdown
May 26 05:42:33 clamav-test-mantic systemd-timesyncd[11333]: Contacted time server 185.125.190.56:123 (ntp.ubuntu.com).
May 26 05:42:34 clamav-test-mantic systemd-networkd[11337]: enp5s0: Gained IPv6LL

I verified also the autopkgtests run fine, and did a cursory skim through the script's code. It all looks pretty straightforward and standardish, so +1 LGTM.

Regarding Steve's feedback to just make the script be networkd-only, honestly it may not be a terrible idea. Certainly a single codepath is going to be more testable and less code to maintain. If the goal is to avoid delta with debian, then an alternative might be to carry both versions of the script and then have the packaging system install the proper one based on the system configuration, rather than having the script heuristically guess which codepath to use. Anyway, I don't have a firm opinion on what would be better, either approach should work out ok.

review: Approve

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

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

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks for the review, Bryce.

I talked to Steve on #ubuntu-devel and he gave me a +1 there. I'm going to upload the package as is, and defer the discussion regarding the implementation of the feature for Debian.

Uploaded:

$ dput clamav_0.103.8+dfsg-0ubuntu2_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/clamav/clamav_0.103.8+dfsg-0ubuntu2_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/clamav/clamav_0.103.8+dfsg-0ubuntu2.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading clamav_0.103.8+dfsg-0ubuntu2.dsc: done.
  Uploading clamav_0.103.8+dfsg-0ubuntu2.debian.tar.xz: done.
  Uploading clamav_0.103.8+dfsg-0ubuntu2_source.buildinfo: done.
  Uploading clamav_0.103.8+dfsg-0ubuntu2_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 172f5e5..ec359bf 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,16 @@
+clamav (0.103.8+dfsg-0ubuntu2) mantic; urgency=medium
+
+  * Extend ifupdown script to support networkd-dispatcher.
+    - d/clamav-freshclam-ifupdown: Modernize some parts of
+      the script.  Implement support for networkd-dispatcher.
+    - d/clamav-freshclam.links: Install the
+      clamav-freshclam-ifupdown script inside the proper
+      /usr/lib/networkd-dispatcher/{off,routable}.d/
+      directories.
+    (LP: #1718227)
+
+ -- Sergio Durigan Junior <[e-mail]>  Fri, 12 May 2023 15:58:29 -0400
+
 clamav (0.103.8+dfsg-0ubuntu1) lunar; urgency=medium
 
   * Updated to version 0.103.8 to fix security issues.
diff --git a/debian/clamav-freshclam-ifupdown b/debian/clamav-freshclam-ifupdown
index 875c0cc..232fd79 100755
--- a/debian/clamav-freshclam-ifupdown
+++ b/debian/clamav-freshclam-ifupdown
@@ -1,16 +1,25 @@
 #!/bin/sh
 # 2004-01-25, Thomas Lamy <[e-mail]>
 # From Magnus Ekdahl's <[e-mail]> clamav-freshclam-handledaemon(8)
+# Adjust to be networkd-dispatcher compatible by
+# Sergio Durigan Junior <[e-mail]>
 
 set -e
 
-[ -e /var/lib/clamav/interface ] || exit 0
+[ -f /var/lib/clamav/interface ] || exit 0
+
+if [ -d /run/systemd/system ]; then
+    INIT='systemctl'
+    INIT_SUFFIX='clamav-freshclam'
+else
+    INIT='invoke-rc.d clamav-freshclam'
+    INIT_SUFFIX=''
+fi
 
-INIT=invoke-rc.d clamav-freshclam
 CLAMAV_CONF_FILE=/etc/clamav/clamd.conf
 FRESHCLAM_CONF_FILE=/etc/clamav/freshclam.conf
 
-INTERNETIFACE=`cat /var/lib/clamav/interface`
+INTERNETIFACE=$(cat /var/lib/clamav/interface)
 
 if grep -q freshclam /proc/*/stat 2>/dev/null; then
   IS_RUNNING=true
@@ -18,61 +27,104 @@ else
   IS_RUNNING=false
 fi
 
-# $IFACE is set by ifup/down, $PPP_IFACE by pppd 
-[ -n "$PPP_IFACE" ] && IFACE=$PPP_IFACE
+handle_ifupdown ()
+{
+    # $IFACE is set by ifup/down, $PPP_IFACE by pppd 
+    [ -n "$PPP_IFACE" ] && IFACE=$PPP_IFACE
 
-# This is sloppy - woody's pppd exports variables, while sid's passes them as 
-# arguments and exports them.
+    # This is sloppy - woody's pppd exports variables, while sid's passes them as 
+    # arguments and exports them.
 
-if [ "$1" = "$IFACE" ]; then # We're called by sid's pppd
-  shift 6                    # and we already know the interface
-fi                           # Dump the arguments passed.
+    if [ "$1" = "$IFACE" ]; then # We're called by sid's pppd
+	shift 6                    # and we already know the interface
+    fi                           # Dump the arguments passed.
+
+    if [ -z "$1" ]; then
+	case $(dirname "$0") in
+	    */if-up.d|*/ip-up.d)
+		# Short circuit and exit early if freshclam is already running
+		[ "$IS_RUNNING" = 'true' ] && exit 0
+		for interface in $INTERNETIFACE; do
+		    if [ "$interface" = "$IFACE" ]; then
+			FMODE=start
+			break
+		    else
+			FMODE=skip
+		    fi
+		done
+		;;
+	    */if-down.d|*/ip-down.d)
+		# Short circuit and exit early if freshclam is not already running
+		[ "$IS_RUNNING" = 'false' ] && exit 0
+		for interface in $INTERNETIFACE; do
+		    if [ "$interface" = "$IFACE" ]; then
+			FMODE=stop
+			break
+		    else
+			FMODE=skip
+		    fi
+		done
+		;;
+	    *)
+		FMODE=skip
+		;;
+	esac
+    else
+	FMODE="$1"
+    fi
+
+    case "$FMODE" in
+	start|stop)
+	    IFACE="$IFACE" $INIT $FMODE $INIT_SUFFIX
+	    ;;
+	skip)
+	    ;;
+	*)
+	    echo "Usage: $0 {start|stop|skip}" >&2
+	    exit 1
+	    ;;
+    esac
+}
+
+handle_networkd_dispatcher ()
+{
+    FOUND_IFACE=false
 
-if [ -z "$1" ]; then
-  case $(dirname "$0") in
-    */if-up.d|*/ip-up.d)
-    # Short circuit and exit early if freshclam is already running
-    [ "$IS_RUNNING" = 'true' ] && exit 0
-    for interface in $INTERNETIFACE; do
-      if [ "$interface" = "$IFACE" ]; then
-	FMODE=start
-	break
-      else
-	FMODE=skip
-      fi
-    done
-    ;;
-    */if-down.d|*/ip-down.d)
-    # Short circuit and exit early if freshclam is not already running
-    [ "$IS_RUNNING" = 'false' ] && exit 0
     for interface in $INTERNETIFACE; do
-      if [ "$interface" = "$IFACE" ]; then
-	FMODE=stop
-	break
-      else
-	FMODE=skip
-      fi
+	if [ "$interface" = "$IFACE" ]; then
+	    FOUND_IFACE=true
+	    break
+	fi
     done
-    ;;
-    *)
-    FMODE=skip
-    ;;
-  esac
+
+    [ "$FOUND_IFACE" = 'false' ] && return
+
+    FMODE=""
+
+    case "$STATE" in
+	"off")
+	    if [ "$IS_RUNNING" = 'true' ]; then
+		FMODE="stop"
+	    fi
+	    ;;
+	"routable")
+	    if [ "$IS_RUNNING" = 'false' ]; then
+		FMODE="start"
+	    fi
+	    ;;
+	*)
+	    return
+    esac
+
+    if [ -n "$FMODE" ]; then
+	IFACE="$IFACE" $INIT $FMODE $INIT_SUFFIX
+    fi
+}
+
+if [ -n "$STATE" ]; then
+    handle_networkd_dispatcher "$@"
 else
-  FMODE="$1"
+    handle_ifupdown "$@"
 fi
 
-case "$FMODE" in
-  start|stop)
-  IFACE="$IFACE" $INIT $FMODE
-  ;;
-  skip)
-  ;;
-  *)
-  echo "Usage: $0 {start|stop|skip}" >&2
-  exit 1
-  ;;
-esac
-
 exit 0
-
diff --git a/debian/clamav-freshclam.links b/debian/clamav-freshclam.links
index 04ad83d..3f0cfff 100644
--- a/debian/clamav-freshclam.links
+++ b/debian/clamav-freshclam.links
@@ -1,2 +1,4 @@
 /usr/share/doc/clamav-base/README.Debian.gz /usr/share/doc/clamav-freshclam/README.Debian.gz
 /usr/share/doc/clamav-base/NEWS.gz /usr/share/doc/clamav-freshclam/NEWS.gz
+/etc/network/if-up.d/clamav-freshclam-ifupdown /usr/lib/networkd-dispatcher/routable.d/clamav-freshclam
+/etc/network/if-down.d/clamav-freshclam-ifupdown /usr/lib/networkd-dispatcher/off.d/clamav-freshclam