Webmin conf improvements

conf/turnkey.d/webmin-conf-logging

@@ -0,0 +1,49 @@
+#!/bin/bash -e
+
+CONF=/etc/webmin/miniserv.conf
+LOG_DIR=/var/log/webmin
+
+update_or_add() {
+    key=$1
+    value=$2
+    if grep -q "$key" "$CONF"; then
+        sed -i "s|$key=.*|$key=$value|" "$CONF"
+    else
+        echo "$key=$value" >> "$CONF"
+    fi
+}
+
+update_or_add port 12321
+update_or_add listen 12321
+update_or_add keyfile /etc/ssl/private/cert.pem
+update_or_add certfile
+update_or_add cipher_list_def 0
+update_or_add error_handler_401 401.cgi
+update_or_add error_handler_404 404.cgi
+update_or_add error_handler_403 403.cgi
+update_or_add nolog '\/stats\.cgi\?xhr\-stats\=general'
+update_or_add no_tls1 1
+update_or_add no_tls1_1 1
+# TODO: Disable TLSv1.2 in a future release (i.e. append '1': 'no_tls1_2 1')
+update_or_add no_tls1_2
+update_or_add extracas
+update_or_add ssl_hsts 1
+update_or_add ssl_redirect 1
+# update logfile location
+update_or_add logfile "$LOG_DIR/miniserv.log"
+update_or_add errorlog "$LOG_DIR/miniserv.error"
+
+# Note: Updating Webmin config for it's own log file as below does not actually
+# work (continues to log to /var/webmin/webmin.log) but we'll work around that
+# via symlinks and update the config file to point to the actual log file
+# anyway.
+CONF=/etc/webmin/config
+update_or_add logfile "$LOG_DIR/webmin.log"
+
+# Prime log files and set permissions
+mkdir -p "$LOG_DIR"
+touch "$LOG_DIR"/{miniserv.log,miniserv.error,webmin.log}
+chmod 750 "$LOG_DIR"
+chmod 640 "$LOG_DIR"/*.log
+rm -f /var/webmin/webmin.log
+ln -sf /var/log/webmin/webmin.log /var/webmin/webmin.log

conf/turnkey.d/webmin-fw

@@ -1,10 +1,33 @@
-#!/bin/sh -e
+#!/bin/bash -e
 
-set ${WEBMIN_FW_TCP_INCOMING:=22 80 443 12321}
+# TODO: drop use of iptables-legacy and use nftables directly
 
-CONF=/etc/iptables.up.rules
+set "${WEBMIN_FW_TCP_INCOMING:=22 80 443 12321}"
 
-cat > $CONF <<EOF
+# Read into an array of sorted unique values
+# Note: lastpipe is enabled to work around race condition when combining
+# readarray directly with process substitution in bash scripts (job control
+# must be off)
+shopt -s lastpipe
+
+tr ' ' '\n' <<<"$WEBMIN_FW_TCP_INCOMING" \
+    | sort -un \
+    | readarray -t WEBMIN_FW_TCP_INCOMING
+
+# Disable lastpipe again to ensure no unexpected behavior later...
+shopt -u lastpipe
+
+for conf in /etc/iptables.up.rules /etc/ip6tables.up.rules; do
+    if [[ "$conf" == *"ip6"* ]]; then
+        # IPv6 should all accept all ICMPv6 types, not just echo-request
+        # ICMPv6 is essential for neighbour discovery (NDP), router
+        # advertisements, and path MTU - blocking it breaks IPv6 networking
+        # in ways that aren't obvious.
+        ICMP="-A INPUT -p ipv6-icmp -j ACCEPT"
+    else
+        ICMP="-A INPUT -p icmp -m icmp --icmp-type echo-request -j ACCEPT"
+    fi
+    cat > "$conf" <<EOF
 *nat
 :PREROUTING ACCEPT [0:0]
 :POSTROUTING ACCEPT [0:0]
@@ -24,31 +47,38 @@ COMMIT
 :INPUT DROP [0:0]
 :OUTPUT ACCEPT [0:0]
 -A INPUT -i lo -j ACCEPT
--A INPUT -p icmp -m icmp --icmp-type echo-request -j ACCEPT
+$ICMP
 -A INPUT -m state --state ESTABLISHED,RELATED -j ACCEPT
 EOF
-
-for port in $WEBMIN_FW_TCP_INCOMING; do
-    echo "-A INPUT -p tcp -m tcp --dport $port -j ACCEPT" >> $CONF
-done
-
-if [ "$WEBMIN_FW_UDP_INCOMING" ]; then
-    for port in $WEBMIN_FW_UDP_INCOMING; do
-        echo "-A INPUT -p udp -m udp --dport $port -j ACCEPT" >> $CONF
+    for port in "${WEBMIN_FW_TCP_INCOMING[@]}"; do
+        echo "-A INPUT -p tcp -m tcp --dport $port -j ACCEPT" >> "$conf"
     done
-fi
 
-if [ "$WEBMIN_FW_TCP_INCOMING_REJECT" ]; then
-    for port in $WEBMIN_FW_TCP_INCOMING_REJECT; do
-        echo "-A INPUT -p tcp -m tcp --dport $port -j REJECT" >> $CONF
-    done
-fi
+    if [[ "$WEBMIN_FW_UDP_INCOMING" ]]; then
+        readarray -t WEBMIN_FW_UDP_INCOMING \
+            < <(tr ' ' '\n' <<< "$WEBMIN_FW_UDP_INCOMING" | sort -un)
+        for port in "${WEBMIN_FW_UDP_INCOMING[@]}"; do
+            echo "-A INPUT -p udp -m udp --dport $port -j ACCEPT" >> "$conf"
+        done
+    fi
 
-echo "COMMIT" >> $CONF
+    if [ "$WEBMIN_FW_TCP_INCOMING_REJECT" ]; then
+        readarray -t WEBMIN_FW_TCP_INCOMING_REJECT \
+            < <(tr ' ' '\n' <<< "$WEBMIN_FW_TCP_INCOMING_REJECT" | sort -un)
+        for port in "${WEBMIN_FW_TCP_INCOMING_REJECT[@]}"; do
+            echo "-A INPUT -p tcp -m tcp --dport $port -j REJECT" >> "$conf"
+        done
+    fi
 
-sed -i "/^$/d" $CONF
+    echo "COMMIT" >> "$conf"
+    sed -i "/^$/d" "$conf"
+done
 
-# As of Buster, Debian uses nftables for firewall; but webmin only supports legacy
-# iptables - see https://github.com/webmin/webmin/issues/1097
+# Debian has been using nftables for firewall for some time; but historically
+# Webmin only supported legacy iptables. Webmin now supports nftables so as per
+# TODO at top of this file TKL should migrate to nftables, but for now we'll
+# continue to leverage legacy iptables functionality via 'iptables-legacy'.
+#
+# See https://github.com/webmin/webmin/issues/1097
 update-alternatives --set iptables /usr/sbin/iptables-legacy
 update-alternatives --set ip6tables /usr/sbin/ip6tables-legacy