scripts: Rspamd stabilization pt. 1 (#3261)

* added checks whether OpenDKIM/OpenDMARC/policyd-spf are enabled
* added functions to check if VAR is 0/0 or an int

and also added tests.

I also adjusted the test file to not run in a container, because there
is no need. This also decreases test time, which, in turn, increases
maintainers' happiness.

* added more checks to Rspamd setup

I added the helpers from the previous commit to the Rspamd setup to make
the whole setup more robust, and indicate to the user that an ENV
variable's value is incorrect.

While we did not issues for this in the past, I believe it to be
worthwhile for the future.

* added canonical directory for users to place files in

This dir is canonical with DMS's optional configuration dirs, as it
lives in well-known volume mounts. Hence, users will not need to adjust
`/etc/rspamd/override.d` manually anymore, or mount a volume to this
place.

The docs explain this now, but the DKIM page needs a slight update on
this too I guess. I will follow-up here.

* misc minor improvements
* use variables for common directories

docs/content/config/advanced/optional-config.md

@@ -12,6 +12,7 @@ This is a list of all configuration files and directories which are optional or
 - **sieve-pipe:** directory for sieve pipe scripts. (Docs: [Sieve][docs-sieve])
 - **opendkim:** DKIM directory. Auto-configurable via [`setup.sh config dkim`][docs-setupsh]. (Docs: [DKIM][docs-dkim])
 - **ssl:** SSL Certificate directory if `SSL_TYPE` is set to `self-signed` or `custom`. (Docs: [SSL][docs-ssl])
+- **Rspamd:** Override directory for custom settings when using Rspamd (Docs: [Rspamd][docs-rspamd-override-d])
 
 ## Files
 
@@ -54,5 +55,6 @@ This is a list of all configuration files and directories which are optional or
 [docs-sieve]: ./mail-sieve.md
 [docs-setupsh]: ../../config/setup.sh.md
 [docs-ssl]: ../../config/security/ssl.md
+[docs-rspamd-override-d]: ../security/rspamd.md#manually
 [docs-rspamd-commands]: ../security/rspamd.md#with-the-help-of-a-custom-file
 [github-commit-setup-stack.sh-L411]: https://github.com/docker-mailserver/docker-mailserver/blob/941e7acdaebe271eaf3d296b36d4d81df4c54b90/target/scripts/startup/setup-stack.sh#L411

docs/content/config/security/rspamd.md

@@ -101,17 +101,20 @@ The [RBL module](https://rspamd.com/doc/modules/rbl.html) is enabled by default.
 
 ## Providing Custom Settings & Overriding Settings
 
+DMS brings sane default settings for Rspamd. They are located at `/etc/rspamd/local.d/` inside the container (or `target/rspamd/local.d/` in the repository).
+
 ### Manually
 
-DMS brings sane default settings for Rspamd. They are located at `/etc/rspamd/local.d/` inside the container (or `target/rspamd/local.d/` in the repository). If you want to change these settings and / or provide your own settings, you can
+If you want to overwrite the default settings and / or provide your own settings, you can place files at `docker-data/dms/config/rspamd/override.d/` (a directory that is linked to `/etc/rspamd/override.d/`, if it exists) to override Rspamd and DMS default settings.
 
-1. place files at `/etc/rspamd/override.d/` which will override Rspamd settings and DMS settings
-2. (re-)place files at `/etc/rspamd/local.d/` to override DMS settings and merge them with Rspamd settings
+!!! note "What is [`docker-data/dms/config/`][docs-dms-config-volume]?"
 
 !!! warning "Clashing Overrides"
 
     Note that when also [using the `rspamd-commands` file](#with-the-help-of-a-custom-file), files in `override.d` may be overwritten in case you adjust them manually and with the help of the file.
 
+[docs-dms-config-volume]: ../../faq.md#what-about-the-docker-datadmsmail-state-folder
+
 ### With the Help of a Custom File
 
 DMS provides the ability to do simple adjustments to Rspamd modules with the help of a single file. Just place a file called `rspamd-modules.conf` into the [local config directory `docker-data/dms/config/`][docs-volumes-config]. If this file is present, DMS will evaluate it. The structure is _very_ simple. Each line in the file looks like this:

target/scripts/helpers/utils.sh

@@ -114,3 +114,34 @@ function _replace_by_env_in_file
     sed -i -E "s#^${ESCAPED_KEY}[[:space:]]*=.*#${ESCAPED_KEY} =${ESCAPED_VALUE}#g" "${CONFIG_FILE}"
   done < <(env | grep "^${ENV_PREFIX}")
 }
+
+# Check if an environment variable's value is zero or one. This aids in checking variables
+# that act as "booleans" for enabling or disabling a service, configuration option, etc.
+#
+# This function will log a warning and return with exit code 1 in case the variable's value
+# is not zero or one.
+#
+# @param ${1} = name of the ENV variable to check
+function _env_var_expect_zero_or_one
+{
+  local ENV_VAR_NAME=${1:?ENV var name must be provided to _env_var_expect_zero_or_one}
+
+  [[ ${!ENV_VAR_NAME} =~ ^(0|1)$ ]] && return 0
+  _log 'warn' "The value of '${ENV_VAR_NAME}' is not zero or one ('${!ENV_VAR_NAME}'), but was expected to be"
+  return 1
+}
+
+# Check if an environment variable's value is an integer.
+#
+# This function will log a warning and return with exit code 1 in case the variable's value
+# is not an integer.
+#
+# @param ${1} = name of the ENV variable to check
+function _env_var_expect_integer
+{
+  local ENV_VAR_NAME=${1:?ENV var name must be provided to _env_var_expect_integer}
+
+  [[ ${!ENV_VAR_NAME} =~ ^-?[0-9][0-9]*$ ]] && return 0
+  _log 'warn' "The value of '${ENV_VAR_NAME}' is not an integer ('${!ENV_VAR_NAME}'), but was expected to be"
+  return 1
+}

target/scripts/startup/setup.d/security/rspamd.sh

@@ -3,10 +3,11 @@
 # Function called during global setup to handle the complete setup of Rspamd.
 function _setup_rspamd
 {
-  if [[ ${ENABLE_RSPAMD} -eq 1 ]]
+  if _env_var_expect_zero_or_one 'ENABLE_RSPAMD' && [[ ${ENABLE_RSPAMD} -eq 1 ]]
   then
     _log 'warn' 'Rspamd integration is work in progress - expect changes at any time'
     _log 'debug' 'Enabling and configuring Rspamd'
+    __rspamd__log 'trace' '----------  Setup started  ----------'
 
     __rspamd__run_early_setup_and_checks        # must run first
     __rspamd__setup_redis
@@ -18,7 +19,7 @@ function _setup_rspamd
     __rspamd__setup_hfilter_group
     __rspamd__handle_user_modules_adjustments   # must run last
 
-    _log 'trace' 'Rspamd setup finished'
+    __rspamd__log 'trace' '----------  Setup finished  ----------'
   else
     _log 'debug' 'Rspamd is disabled'
   fi
@@ -65,23 +66,61 @@ EOF
 # or checking for other anti-spam/anti-virus software.
 function __rspamd__run_early_setup_and_checks
 {
+  # Note: Variables not marked with `local` are
+  # used in other functions as well.
+  RSPAMD_LOCAL_D='/etc/rspamd/local.d'
+  RSPAMD_OVERRIDE_D='/etc/rspamd/override.d'
+  RSPAMD_DMS_D='/tmp/docker-mailserver/rspamd'
+  local RSPAMD_DMS_OVERRIDE_D="${RSPAMD_DMS_D}/override.d/"
+
   mkdir -p /var/lib/rspamd/
   : >/var/lib/rspamd/stats.ucl
 
+  if [[ -d ${RSPAMD_DMS_OVERRIDE_D} ]]
+  then
+    __rspamd__log 'debug' "Found directory '${RSPAMD_DMS_OVERRIDE_D}' - linking it to '${RSPAMD_OVERRIDE_D}'"
+    if rmdir "${RSPAMD_OVERRIDE_D}"
+    then
+      ln -s "${RSPAMD_DMS_OVERRIDE_D}" "${RSPAMD_OVERRIDE_D}"
+    else
+      __rspamd__log 'warn' "Could not remove '${RSPAMD_OVERRIDE_D}' - not linking '${RSPAMD_DMS_OVERRIDE_D}'"
+    fi
+  fi
+
   if [[ ${ENABLE_AMAVIS} -eq 1 ]] || [[ ${ENABLE_SPAMASSASSIN} -eq 1 ]]
   then
     __rspamd__log 'warn' 'Running Amavis/SA & Rspamd at the same time is discouraged'
   fi
+
+  if [[ ${ENABLE_OPENDKIM} -eq 1 ]]
+  then
+    __rspamd__log 'warn' 'Running OpenDKIM & Rspamd at the same time is discouraged - we recommend Rspamd for DKIM checks (enabled with Rspamd by default) & signing'
+  fi
+
+  if [[ ${ENABLE_OPENDMARC} -eq 1 ]]
+  then
+    __rspamd__log 'warn' 'Running OpenDMARC & Rspamd at the same time is discouraged - we recommend Rspamd for DMARC checks (enabled with Rspamd by default)'
+  fi
+
+  if [[ ${ENABLE_POLICYD_SPF} -eq 1 ]]
+  then
+    __rspamd__log 'warn' 'Running policyd-spf & Rspamd at the same time is discouraged - we recommend Rspamd for SPF checks (enabled with Rspamd by default)'
+  fi
+
+  if [[ ${ENABLE_POSTGREY} -eq 1 ]] && [[ ${RSPAMD_GREYLISTING} -eq 1 ]]
+  then
+    __rspamd__log 'warn' 'Running Postgrey & Rspamd at the same time is discouraged - we recommend Rspamd for greylisting'
+  fi
 }
 
 # Sets up Redis. In case the user does not use a dedicated Redis instance, we
 # supply a configuration for our local Redis instance which is started later.
 function __rspamd__setup_redis
 {
-  if [[ ${ENABLE_RSPAMD_REDIS} -eq 1 ]]
+  if _env_var_expect_zero_or_one 'ENABLE_RSPAMD_REDIS' && [[ ${ENABLE_RSPAMD_REDIS} -eq 1 ]]
   then
     __rspamd__log 'debug' 'Internal Redis is enabled, adding configuration'
-    cat >/etc/rspamd/local.d/redis.conf << "EOF"
+    cat >"${RSPAMD_LOCAL_D}/redis.conf" << "EOF"
 # documentation: https://rspamd.com/doc/configuration/redis.html
 
 servers = "127.0.0.1:6379";
@@ -120,10 +159,10 @@ function __rspamd__setup_postfix
 # If ClamAV is enabled, we will integrate it into Rspamd.
 function __rspamd__setup_clamav
 {
-  if [[ ${ENABLE_CLAMAV} -eq 1 ]]
+  if _env_var_expect_zero_or_one 'ENABLE_CLAMAV' && [[ ${ENABLE_CLAMAV} -eq 1 ]]
   then
     __rspamd__log 'debug' 'Enabling ClamAV integration'
-    sedfile -i -E 's|^(enabled).*|\1 = true;|g' /etc/rspamd/local.d/antivirus.conf
+    sedfile -i -E 's|^(enabled).*|\1 = true;|g' "${RSPAMD_LOCAL_D}/antivirus.conf"
     # Rspamd uses ClamAV's UNIX socket, and to be able to read it, it must be in the same group
     usermod -a -G clamav _rspamd
   else
@@ -165,7 +204,7 @@ function __rspamd__setup_default_modules
 #    from or to the "Junk" folder, and learning them as ham or spam.
 function __rspamd__setup_learning
 {
-  if [[ ${RSPAMD_LEARN} -eq 1 ]]
+  if _env_var_expect_zero_or_one 'RSPAMD_LEARN' && [[ ${RSPAMD_LEARN} -eq 1 ]]
   then
     __rspamd__log 'debug' 'Setting up intelligent learning of spam and ham'
 
@@ -210,10 +249,10 @@ EOF
 # https://rspamd.com/doc/modules/greylisting.html).
 function __rspamd__setup_greylisting
 {
-  if [[ ${RSPAMD_GREYLISTING} -eq 1 ]]
+  if _env_var_expect_zero_or_one 'RSPAMD_GREYLISTING' && [[ ${RSPAMD_GREYLISTING} -eq 1 ]]
   then
     __rspamd__log 'debug' 'Enabling greylisting'
-    sedfile -i -E "s|(enabled =).*|\1 true;|g" /etc/rspamd/local.d/greylist.conf
+    sedfile -i -E "s|(enabled =).*|\1 true;|g" "${RSPAMD_LOCAL_D}/greylist.conf"
   else
     __rspamd__log 'debug' 'Greylisting is disabled'
   fi
@@ -225,15 +264,12 @@ function __rspamd__setup_greylisting
 # succeeds.
 function __rspamd__setup_hfilter_group
 {
-  local MODULE_FILE='/etc/rspamd/local.d/hfilter_group.conf'
-  if [[ ${RSPAMD_HFILTER} -eq 1 ]]
+  local MODULE_FILE="${RSPAMD_LOCAL_D}/hfilter_group.conf"
+  if _env_var_expect_zero_or_one 'RSPAMD_HFILTER' && [[ ${RSPAMD_HFILTER} -eq 1 ]]
   then
     __rspamd__log 'debug' 'Hfilter (group) module is enabled'
     # Check if we received a number first
-    if [[ ! ${RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE} =~ ^[0-9][1-9]*$ ]]
-    then
-      __rspamd__log 'warn' "'RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE' is not a number (${RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE}) but was expected to be!"
-    elif [[ ${RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE} -ne 6 ]]
+    if _env_var_expect_integer 'RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE' && [[ ${RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE} -ne 6 ]]
     then
       __rspamd__log 'trace' "Adjusting score for 'HFILTER_HOSTNAME_UNKNOWN' in Hfilter group module to ${RSPAMD_HFILTER_HOSTNAME_UNKNOWN_SCORE}"
       sed -i -E \
@@ -275,7 +311,7 @@ function __rspamd__handle_user_modules_adjustments
     # remove possible whitespace at the end (e.g., in case ${ARGUMENT3} is empty)
     VALUE=${VALUE% }
 
-    local FILE="/etc/rspamd/override.d/${MODULE_FILE}"
+    local FILE="${RSPAMD_OVERRIDE_D}/${MODULE_FILE}"
     [[ -f ${FILE} ]] || touch "${FILE}"
 
     if grep -q -E "${OPTION}.*=.*" "${FILE}"

test/tests/parallel/set1/spam_virus/rspamd.bats

@@ -16,6 +16,8 @@ function setup_file() {
     --env ENABLE_RSPAMD=1
     --env ENABLE_OPENDKIM=0
     --env ENABLE_OPENDMARC=0
+    --env ENABLE_POLICYD_SPF=0
+    --env ENABLE_POSTGREY=0
     --env PERMIT_DOCKER=host
     --env LOG_LEVEL=trace
     --env MOVE_SPAM_TO_JUNK=1

test/tests/parallel/set3/scripts/helper_functions.bats

@@ -1,23 +1,63 @@
-load "${REPOSITORY_ROOT}/test/helper/setup"
 load "${REPOSITORY_ROOT}/test/helper/common"
 
-BATS_TEST_NAME_PREFIX='[Scripts] (helper functions inside container) '
-CONTAINER_NAME='dms-test_helper_functions'
+BATS_TEST_NAME_PREFIX='[Scripts] (helper functions) '
+SOURCE_BASE_PATH="${REPOSITORY_ROOT:?Expected REPOSITORY_ROOT to be set}/target/scripts/helpers"
 
-function setup_file() {
-  _init_with_defaults
-  _common_container_setup
+@test '(network.sh) _sanitize_ipv4_to_subnet_cidr' {
+  source "${SOURCE_BASE_PATH}/network.sh"
+
+  run _sanitize_ipv4_to_subnet_cidr '255.255.255.255/0'
+  assert_output '0.0.0.0/0'
+
+  run _sanitize_ipv4_to_subnet_cidr '192.168.255.14/20'
+  assert_output '192.168.240.0/20'
+
+  run _sanitize_ipv4_to_subnet_cidr '192.168.255.14/32'
+  assert_output '192.168.255.14/32'
 }
 
-function teardown_file() { _default_teardown ; }
+@test '(utils.sh) _env_var_expect_zero_or_one' {
+  source "${SOURCE_BASE_PATH}/log.sh"
+  source "${SOURCE_BASE_PATH}/utils.sh"
 
-@test "_sanitize_ipv4_to_subnet_cidr" {
-  _run_in_container_bash "source /usr/local/bin/helpers/index.sh; _sanitize_ipv4_to_subnet_cidr 255.255.255.255/0"
-  assert_output "0.0.0.0/0"
+  ZERO=0
+  ONE=1
+  TWO=2
 
-  _run_in_container_bash "source /usr/local/bin/helpers/index.sh; _sanitize_ipv4_to_subnet_cidr 192.168.255.14/20"
-  assert_output "192.168.240.0/20"
+  run _env_var_expect_zero_or_one ZERO
+  assert_success
 
-  _run_in_container_bash "source /usr/local/bin/helpers/index.sh; _sanitize_ipv4_to_subnet_cidr 192.168.255.14/32"
-  assert_output "192.168.255.14/32"
+  run _env_var_expect_zero_or_one ONE
+  assert_success
+
+  run _env_var_expect_zero_or_one TWO
+  assert_failure
+  assert_output --partial "The value of 'TWO' is not zero or one ('2'), but was expected to be"
+
+  run _env_var_expect_zero_or_one
+  assert_failure
+  assert_output --partial "ENV var name must be provided to _env_var_expect_zero_or_one"
+}
+
+@test '(utils.sh) _env_var_expect_integer' {
+  source "${SOURCE_BASE_PATH}/log.sh"
+  source "${SOURCE_BASE_PATH}/utils.sh"
+
+  INTEGER=1234
+  NEGATIVE=-${INTEGER}
+  NaN=not_an_integer
+
+  run _env_var_expect_integer INTEGER
+  assert_success
+
+  run _env_var_expect_integer NEGATIVE
+  assert_success
+
+  run _env_var_expect_integer NaN
+  assert_failure
+  assert_output --partial "The value of 'NaN' is not an integer ('not_an_integer'), but was expected to be"
+
+  run _env_var_expect_integer
+  assert_failure
+  assert_output --partial "ENV var name must be provided to _env_var_expect_integer"
 }