From 450a16f730bc4f7c54c6cb370bffe485d7563cf3 Mon Sep 17 00:00:00 2001 From: Glop <13887307-gl0p@users.noreply.gitlab.com> Date: Fri, 3 Mar 2023 16:09:23 +0100 Subject: [PATCH 1/3] Destroy the temporary IP set in the cleanup function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the IP set capability tests, there is a race condition which might prevent the removal of the temporary IP set immediately after flushing the chain that uses this IP set: even though the rules which used the IP set were deleted, the IP set might still appear to be “in use by a kernel component.” In case this happens, we add an extra call to `ipset -X` in the `cleanup_iptables()` function, just to be sure that the temporary IP set is indeed destroyed when the compiler exits. --- Shorewall/Perl/Shorewall/Config.pm | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Shorewall/Perl/Shorewall/Config.pm b/Shorewall/Perl/Shorewall/Config.pm index ae3efcf4f..5d45a90c8 100644 --- a/Shorewall/Perl/Shorewall/Config.pm +++ b/Shorewall/Perl/Shorewall/Config.pm @@ -1507,7 +1507,7 @@ sub qt1( $ ) { } # -# Delete the test chains +# Delete the test chains and IP sets # sub cleanup_iptables() { qt1( "$iptables $iptablesw -F $sillyname" ); @@ -1530,6 +1530,12 @@ sub cleanup_iptables() { qt1( "$iptables $iptablesw -t raw -X $sillyname" ); } + my $ipset = $config{IPSET} || 'ipset'; + $ipset = which( $ipset ) unless $ipset =~ '/'; + if ( $ipset && -x $ipset ) { + qt( "$ipset -X $sillyname" ); + } + $sillyname = $sillyname1 = ''; } @@ -1574,7 +1580,7 @@ sub cleanup() { unlink ( $perlscriptname ), $perlscriptname = undef if $perlscriptname; unlink ( @tempfiles ), @tempfiles = () if @tempfiles; # - # Delete temporary chains + # Delete temporary chains and IP sets # cleanup_iptables if $sillyname; } From 5e8ce7d073b387066d2d73cd63592b8f68a4205b Mon Sep 17 00:00:00 2001 From: Jeremy Sowden Date: Mon, 4 Sep 2023 20:26:04 +0100 Subject: [PATCH 2/3] lib.cli-std: fix two shell errors when AUTOMAKE is false If `AUTOMAKE` is set to `no` in the config file, it is normalized to the empty string. This leads to two errors if `find` is provided by Busybox. There is a conditional where `$AUTOMAKE` is not quoted when compared to `recursive` leading to the following error: /usr/share/shorewall/lib.cli-std: line 398: [: =: unary operator expected In contrast to the non-Busybox case, we don't check for an empty `$AUTOMAKE` before passing it as an argument to `-maxdepth`, leading to: /usr/bin/find: Expected a positive decimal integer argument to -maxdepth, but got -type Refactor the conditionals to eliminate code duplication and fix these two bugs. Link: https://gitlab.com/shorewall/code/-/issues/10 Signed-off-by: Jeremy Sowden --- Shorewall/lib.cli-std | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/Shorewall/lib.cli-std b/Shorewall/lib.cli-std index a5a497674..4dd0e8149 100644 --- a/Shorewall/lib.cli-std +++ b/Shorewall/lib.cli-std @@ -381,36 +381,33 @@ uptodate() { [ -x $1 ] || return 1 local dir - local busybox local find + local quit + local maxdepth find=$(mywhich find) [ -n "${find}" ] || return 1 - [ -h "${find}" ] && busybox=Yes - find="${find} -L" + + if [ -h "${find}" ]; then + # + # 'Find' is provided by Busybox and doesn't support -quit. + # + quit= + else + quit=-quit + fi + + if [ "$AUTOMAKE" = recursive ]; then + maxdepth= + elif [ -z "$AUTOMAKE" ]; then + maxdepth="-maxdepth 1" + else + maxdepth="-maxdepth $AUTOMAKE" + fi for dir in $g_shorewalldir $(split $CONFIG_PATH); do - if [ -n "${busybox}" ]; then - # - # Busybox 'find' doesn't support -quit. - # - if [ $AUTOMAKE = recursive ]; then - if [ -n "$(${find} ${dir} -newer $1 -print)" ]; then - return 1; - fi - elif [ -n "$(${find} ${dir} -maxdepth $AUTOMAKE -type f -newer $1 -print)" ]; then - return 1; - fi - elif [ "$AUTOMAKE" = recursive ]; then - if [ -n "$(${find} ${dir} -newer $1 -print -quit)" ]; then - return 1; - fi - elif [ -z "$AUTOMAKE" ]; then - if [ -n "$(${find} ${dir} -maxdepth 1 -type f -newer $1 -print -quit)" ]; then - return 1; - fi - elif [ -n "$(${find} ${dir} -maxdepth $AUTOMAKE -type f -newer $1 -print -quit)" ]; then + if [ -n "$(${find} -L ${dir} ${maxdepth} -newer $1 -print ${quit})" ]; then return 1; fi done From badf2fc9f05c190d129634344afa92f0a149f914 Mon Sep 17 00:00:00 2001 From: Jeremy Sowden Date: Tue, 31 Jan 2023 21:52:42 +0000 Subject: [PATCH 3/3] Support `SAFESTOP` under systemd By default, in Debian and its derivatives, stopping the Shorewall service executes `/sbin/shorewall clear`. The `SAFESTOP` setting in /etc/default/shorewall is intended to stop the service by calling `/sbin/shorewall stop`. However, the systemd service files do not support this. Instead, install a shell-script that sources /etc/default/shorewall and honours `SAFESTOP` when stopping Shorewall and patch the service files to call it. Signed-off-by: Jeremy Sowden --- Shorewall-core/install.sh | 9 +++++++++ Shorewall-core/shorewallrc.debian.systemd | 1 + Shorewall-core/stop_service.debian | 19 +++++++++++++++++++ Shorewall-lite/shorewall-lite.service.debian | 2 +- Shorewall/shorewall.service.debian | 2 +- .../shorewall6-lite.service.debian | 2 +- Shorewall6/shorewall6.service.debian | 2 +- docs/starting_and_stopping_shorewall.xml | 18 +++++++++++------- 8 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 Shorewall-core/stop_service.debian diff --git a/Shorewall-core/install.sh b/Shorewall-core/install.sh index 24e22b340..1ce769b50 100755 --- a/Shorewall-core/install.sh +++ b/Shorewall-core/install.sh @@ -324,6 +324,15 @@ install_file wait4ifup ${DESTDIR}${LIBEXECDIR}/shorewall/wait4ifup 0755 echo echo "wait4ifup installed in ${DESTDIR}${LIBEXECDIR}/shorewall/wait4ifup" +# +# Install stop_service +# +if [ -n "${STOPSERVICEFILE}" ]; then + install_file ${STOPSERVICEFILE} ${DESTDIR}${LIBEXECDIR}/shorewall/stop_service 0755 + + echo + echo "${STOPSERVICEFILE} installed in ${DESTDIR}${LIBEXECDIR}/shorewall/stop_service" +fi # # Install the libraries diff --git a/Shorewall-core/shorewallrc.debian.systemd b/Shorewall-core/shorewallrc.debian.systemd index 41ba8b72e..390274638 100644 --- a/Shorewall-core/shorewallrc.debian.systemd +++ b/Shorewall-core/shorewallrc.debian.systemd @@ -22,3 +22,4 @@ SPARSE=Yes #If non-empty, only install $PRODUCT/$PRODUCT.conf in $CONFDIR VARLIB=/var/lib #Directory where product variable data is stored. VARDIR=${VARLIB}/$PRODUCT #Directory where product variable data is stored. DEFAULT_PAGER=/usr/bin/less #Pager to use if none specified in shorewall[6].conf +STOPSERVICEFILE=stop_service.debian #Name of script to stop systemd service that honours `SAFESTOP`. diff --git a/Shorewall-core/stop_service.debian b/Shorewall-core/stop_service.debian new file mode 100644 index 000000000..5301100da --- /dev/null +++ b/Shorewall-core/stop_service.debian @@ -0,0 +1,19 @@ +#!/bin/sh + +PRODUCT=$1 + +. /etc/default/${PRODUCT} + +if [ "$SAFESTOP" = 1 ]; then + COMMAND=stop +else + COMMAND=clear +fi + +if [ "${PRODUCT}" = shorewall6 ]; then + EXEC="/sbin/shorewall -6" +else + EXEC="/sbin/${PRODUCT}" +fi + +exec ${EXEC} ${OPTIONS} ${COMMAND} diff --git a/Shorewall-lite/shorewall-lite.service.debian b/Shorewall-lite/shorewall-lite.service.debian index 4eeaee8aa..47a68d51f 100644 --- a/Shorewall-lite/shorewall-lite.service.debian +++ b/Shorewall-lite/shorewall-lite.service.debian @@ -17,7 +17,7 @@ RemainAfterExit=yes EnvironmentFile=-/etc/default/shorewall-lite StandardOutput=syslog ExecStart=/sbin/shorewall-lite $OPTIONS start $STARTOPTIONS -ExecStop=/sbin/shorewall-lite $OPTIONS clear +ExecStop=/usr/share/shorewall/stop_service shorewall-lite ExecReload=/sbin/shorewall-lite $OPTIONS reload $RELOADOPTIONS [Install] diff --git a/Shorewall/shorewall.service.debian b/Shorewall/shorewall.service.debian index 7e4f04461..180cbce36 100644 --- a/Shorewall/shorewall.service.debian +++ b/Shorewall/shorewall.service.debian @@ -17,7 +17,7 @@ RemainAfterExit=yes EnvironmentFile=-/etc/default/shorewall StandardOutput=syslog ExecStart=/sbin/shorewall $OPTIONS start $STARTOPTIONS -ExecStop=/sbin/shorewall $OPTIONS clear +ExecStop=/usr/share/shorewall/stop_service shorewall ExecReload=/sbin/shorewall $OPTIONS reload $RELOADOPTIONS [Install] diff --git a/Shorewall6-lite/shorewall6-lite.service.debian b/Shorewall6-lite/shorewall6-lite.service.debian index e101cd204..00f0cf681 100644 --- a/Shorewall6-lite/shorewall6-lite.service.debian +++ b/Shorewall6-lite/shorewall6-lite.service.debian @@ -17,7 +17,7 @@ RemainAfterExit=yes EnvironmentFile=-/etc/default/shorewall6-lite StandardOutput=syslog ExecStart=/sbin/shorewall6-lite $OPTIONS start -ExecStop=/sbin/shorewall6-lite $OPTIONS clear +ExecStop=/usr/share/shorewall/stop_service shorewall6-lite ExecReload=/sbin/shorewall6-lite $OPTIONS reload [Install] diff --git a/Shorewall6/shorewall6.service.debian b/Shorewall6/shorewall6.service.debian index 1ad666a29..befa1c15b 100644 --- a/Shorewall6/shorewall6.service.debian +++ b/Shorewall6/shorewall6.service.debian @@ -18,7 +18,7 @@ RemainAfterExit=yes EnvironmentFile=-/etc/default/shorewall6 StandardOutput=syslog ExecStart=/sbin/shorewall -6 $OPTIONS start $STARTOPTIONS -ExecStop=/sbin/shorewall -6 $OPTIONS clear +ExecStop=/usr/share/shorewall/stop_service shorewall6 ExecReload=/sbin/shorewall -6 $OPTIONS reload $RELOADOPTIONS [Install] diff --git a/docs/starting_and_stopping_shorewall.xml b/docs/starting_and_stopping_shorewall.xml index 8c3cdf3ed..984a9b317 100644 --- a/docs/starting_and_stopping_shorewall.xml +++ b/docs/starting_and_stopping_shorewall.xml @@ -206,12 +206,12 @@
systemd - As with SysV init described in the preceeding section, the behavior - of systemctl commands differ from the Shorewall CLI commands on - Debian-based systems. To make systemctl stop shorewall[-lite] and - systemctl restart shorewall[-lite] behave like shorewall stop and - shorewall restart, use this workaround provided by J Cliff - Armstrong: + As with SysV init described in the preceeding section, the behavior of + systemctl commands differ from the Shorewall CLI commands on Debian-based + systems. In versions of Shorewall before 5.2.9, to make systemctl + stop shorewall and systemctl restart shorewall + behave like shorewall stop and shorewall + restart, use this workaround provided by J Cliff Armstrong: Type (as root): @@ -231,10 +231,14 @@ ExecStop=/sbin/shorewall $OPTIONS stop to activate the changes. This change will survive future updates of the shorewall package from apt repositories. The override file itself will - be saved to `/etc/systemd/system/shorewall.service.d/`. + be saved to /etc/systemd/system/shorewall.service.d/. The same workaround may be applied to the other Shorewall products (excluding Shorewall Init). + + From Shorewall 5.2.9 onwards, the systemd service files have been + updated to execute a shell script that obeys the SAFESTOP setting to stop + the firewall, and the workaround is no longer necessary.