refactor: setup CLI `open-dkim` (#4375)

Refactoring this `setup` CLI command as part of the effort to unify our DKIM feature support between OpenDKIM + Rspamd:
- Adds a `main()` method similar to other setup CLI commands.
- Help text more aligned with equivalent rspamd DKIM setup CLI command.
- DRY some repetition such as hard-coded paths to use variables.
- OpenDKIM config files are created / initialized early on now with `_create_opendkim_configs()`. `while` loop only needs to append entries, so is easier to grok.
- `_create_dkim_key()` to scope just the logic (_and additional notes_) to key generation via `opendkim-genkey`
- Now overall logic with the `while` loop of the script occurs in `_generate_dkim_keys()`:
  - Ownership fixes are now applied after the `while` loop as that seems more appropriate than per iteration.
  - Temporary VHOST config is now removed since it's no longer useful after running.
- Tests adjusted for one new log for adding of default trusted hosts content.

Overall this should be nicer to grok/maintain. Some of this logic will be reused for the unified DKIM generation command in future, which is more likely to shift towards all domains using the same keypair by default with rspamd/opendkim config generated at runtime rather than reliant upon DMS config volume to provide that (_still expected for private key_).

---------

Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>

CHANGELOG.md

@@ -6,6 +6,13 @@ All notable changes to this project will be documented in this file. The format
 
 > **Note**: Changes and additions listed here are contained in the `:edge` image tag. These changes may not be as stable as released changes.
 
+### Updates
+
+- **Documentation:**
+  - Added a compatibility note for a Dovecot + Solr 9.8 breaking change ([#4433](https://github.com/docker-mailserver/docker-mailserver/pull/4433))
+- **Internal:**
+  - Refactored `setup config dkim` (`open-dkim`) ([#4375](https://github.com/docker-mailserver/docker-mailserver/pull/4375))
+
 ## [v15.0.2](https://github.com/docker-mailserver/docker-mailserver/releases/tag/v15.0.2)
 
 ### Fixes

target/bin/open-dkim

@@ -12,9 +12,17 @@ if [[ -f /etc/dms-settings ]] && [[ $(_get_dms_env_value 'ENABLE_RSPAMD') -eq 1
   fi
 fi
 
-KEYSIZE=2048
+function _main() {
-SELECTOR=mail
+  # Default parameters (updated by `_parse_arguments()`):
-DOMAINS=
+  local KEYSIZE=2048
+  local SELECTOR=mail
+  local DMS_DOMAINS=
+
+  _require_n_parameters_or_print_usage 0 "${@}"
+  _parse_arguments "${@}"
+
+  _generate_dkim_keys
+}
 
 function __usage() {
   printf '%s' "${PURPLE}OPEN-DKIM${RED}(${YELLOW}8${RED})
@@ -57,122 +65,171 @@ ${ORANGE}EXAMPLES${RESET}
 
 ${ORANGE}EXIT STATUS${RESET}
     Exit status is 0 if command was successful. If wrong arguments are provided or arguments contain
-    errors, the script will exit early with exit status 2.
+    errors, the script will exit early with a non-zero exit status.
 
 "
 }
 
-_require_n_parameters_or_print_usage 0 "${@}"
+function _parse_arguments() {
-
+  # Parse the command args through iteration:
-while [[ ${#} -gt 0 ]]; do
+  while [[ ${#} -gt 0 ]]; do
     case "${1}" in
+
       ( 'keysize' )
-      if [[ -n ${2+set} ]]; then
+        if [[ -n ${2:-} ]]; then
           KEYSIZE="${2}"
-        shift
+          _log 'trace' "Keysize set to '${KEYSIZE}'"
-        shift
         else
           _exit_with_error "No keysize provided after 'keysize' argument"
         fi
         ;;
 
       ( 'selector' )
-      if [[ -n ${2+set} ]]; then
+        if [[ -n ${2:-} ]]; then
-        # shellcheck disable=SC2034
           SELECTOR="${2}"
-        shift
+          _log 'trace' "Selector set to '${SELECTOR}'"
-        shift
         else
           _exit_with_error "No selector provided after 'selector' argument"
         fi
         ;;
 
       ( 'domain' )
-      if [[ -n ${2+set} ]]; then
+        if [[ -n ${2:-} ]]; then
-        DOMAINS="${2}"
+          DMS_DOMAINS="${2}"
-        shift
+          _log 'trace' "Domain(s) set to '${DMS_DOMAINS}'"
-        shift
         else
           _exit_with_error "No domain(s) provided after 'domain' argument"
         fi
         ;;
 
-    ( * )
+      ( 'help' )
           __usage
-      _exit_with_error "Unknown options '${1}' ${2:+and \'${2}\'}"
+          exit 0
           ;;
 
+      ( * )
+        __usage
+        _exit_with_error "Unknown option(s) ${*}"
+        ;;
     esac
-done
 
+    # Discard these two args (option + value) now that they've been processed:
+    shift 2
+  done
+}
+
+function _generate_dkim_keys() {
+  _generate_domains_config
+  if [[ ! -s ${DATABASE_VHOST} ]]; then
+    _log 'warn' 'No entries found, no keys to make'
+    exit 0
+  fi
+
+  # Initialize OpenDKIM configs if necessary:
+  _create_opendkim_configs
+
+  # Generate a keypair per domain and add reference to OpenDKIM configs:
+  local ENTRY_KEY KEY_TABLE_ENTRY SIGNING_TABLE_ENTRY
+  while read -r DKIM_DOMAIN; do
+    _create_dkim_key "${DKIM_DOMAIN}"
+
+    # Create / Update OpenDKIM configs with new DKIM key:
+    ENTRY_KEY="${SELECTOR}._domainkey.${DKIM_DOMAIN}"
+    KEY_TABLE_ENTRY="${ENTRY_KEY} ${DKIM_DOMAIN}:${SELECTOR}:/etc/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private"
+    SIGNING_TABLE_ENTRY="*@${DKIM_DOMAIN} ${ENTRY_KEY}"
+
+    # If no existing entry, add one:
+    if ! grep -q "${KEY_TABLE_ENTRY}" "${KEY_TABLE_FILE}"; then
+      echo "${KEY_TABLE_ENTRY}" >> "${KEY_TABLE_FILE}"
+    fi
+    if ! grep -q "${SIGNING_TABLE_ENTRY}" "${SIGNING_TABLE_FILE}"; then
+      echo "${SIGNING_TABLE_ENTRY}" >> "${SIGNING_TABLE_FILE}"
+    fi
+  done < <(_get_valid_lines_from_file "${DATABASE_VHOST}")
+
+  # No longer needed, remove:
+  rm "${DATABASE_VHOST}"
+
+  # Ensure ownership is consistent for all content belonging to the base directory,
+  # During container startup, an internal copy will be made via `_setup_opendkim()`
+  # with ownership we expect, while this chown is for the benefit of the users ownership.
+  chown -R "$(stat -c '%U:%G' "${OPENDKIM_BASE_DIR}")" "${OPENDKIM_BASE_DIR}"
+}
+
+# Prepare a file with one domain per line (iterated via while loop as DKIM_DOMAIN):
+# Depends on methods from `scripts/helpers/postfix.sh`:
 DATABASE_VHOST='/tmp/vhost.dkim'
-# Prepare a file with one domain per line:
 function _generate_domains_config() {
   local TMP_VHOST='/tmp/vhost.dkim.tmp'
 
   # Generate the default vhost (equivalent to /etc/postfix/vhost),
-  # unless CLI arg DOMAINS provided an alternative list to use instead:
+  # unless CLI arg DMS_DOMAINS provided an alternative list to use instead:
-  if [[ -z ${DOMAINS} ]]; then
+  if [[ -z ${DMS_DOMAINS:-} ]]; then
     _obtain_hostname_and_domainname
     # uses TMP_VHOST:
     _vhost_collect_postfix_domains
   else
-    tr ',' '\n' <<< "${DOMAINS}" >"${TMP_VHOST}"
+    tr ',' '\n' <<< "${DMS_DOMAINS}" >"${TMP_VHOST}"
   fi
 
-  # uses DATABASE_VHOST + TMP_VHOST:
+  # Uses DATABASE_VHOST + TMP_VHOST:
   _create_vhost
 }
 
-_generate_domains_config
+# `opendkim-genkey` generates two files at the configured `--directory`:
-if [[ ! -s ${DATABASE_VHOST} ]]; then
+# - <selector>.private (Private key, PEM encoded)
-  _log 'warn' 'No entries found, no keys to make'
+# - <selector>.txt (Public key, formatted as a TXT record for a RFC 1035 DNS Zone file)
-  exit 0
+function _create_dkim_key() {
-fi
+  DKIM_DOMAIN=${1?Expected to be provided a domain}
 
-while read -r DKIM_DOMAIN; do
+  OPENDKIM_DOMAINKEY_DIR="${OPENDKIM_BASE_DIR}/keys/${DKIM_DOMAIN}"
-  mkdir -p "/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}"
+  mkdir -p "${OPENDKIM_DOMAINKEY_DIR}"
 
-  if [[ ! -f "/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private" ]]; then
+  DKIM_KEY_FILE="${OPENDKIM_DOMAINKEY_DIR}/${SELECTOR}.private"
-    _log 'info' "Creating DKIM private key '/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private'"
+  if [[ ! -f "${DKIM_KEY_FILE}" ]]; then
+    _log 'info' "Creating DKIM private key '${DKIM_KEY_FILE}'"
 
+    # NOTE:
+    # --domain only affects a comment in the generated DNS Zone file
+    # --subdomains is the default,
+    # --nosubdomains would add `t=s` to the DNS TXT record generated
+    # http://www.opendkim.org/opendkim-genkey.8.html
     opendkim-genkey \
       --bits="${KEYSIZE}" \
       --subdomains \
       --domain="${DKIM_DOMAIN}" \
       --selector="${SELECTOR}" \
-      --directory="/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}"
+      --directory="${OPENDKIM_DOMAINKEY_DIR}"
   fi
+}
 
-  # fix permissions to use the same user:group as /tmp/docker-mailserver/opendkim/keys
+OPENDKIM_BASE_DIR='/tmp/docker-mailserver/opendkim'
-  chown -R "$(stat -c '%U:%G' /tmp/docker-mailserver/opendkim/keys)" "/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}"
+KEY_TABLE_FILE="${OPENDKIM_BASE_DIR}/KeyTable"
+SIGNING_TABLE_FILE="${OPENDKIM_BASE_DIR}/SigningTable"
+TRUSTED_HOSTS_FILE="${OPENDKIM_BASE_DIR}/TrustedHosts"
+# Create configs if missing:
+function _create_opendkim_configs() {
+  mkdir -p "${OPENDKIM_BASE_DIR}"
+  local OPENDKIM_CONFIGS=(
+    "${KEY_TABLE_FILE}"
+    "${SIGNING_TABLE_FILE}"
+    "${TRUSTED_HOSTS_FILE}"
+  )
 
-  # write to KeyTable if necessary
+  # Only create if the file doesn't exist (avoids modifying mtime):
-  KEYTABLEENTRY="${SELECTOR}._domainkey.${DKIM_DOMAIN} ${DKIM_DOMAIN}:${SELECTOR}:/etc/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private"
+  for FILE in "${OPENDKIM_CONFIGS[@]}"; do
-  if [[ ! -f "/tmp/docker-mailserver/opendkim/KeyTable" ]]; then
+    if [[ ! -f "${FILE}" ]]; then
-    _log 'debug' 'Creating DKIM KeyTable'
+      _log 'debug' "Creating OpenDKIM config '${FILE}'"
-    echo "${KEYTABLEENTRY}" >/tmp/docker-mailserver/opendkim/KeyTable
+      touch "${FILE}"
-  else
-    if ! grep -q "${KEYTABLEENTRY}" "/tmp/docker-mailserver/opendkim/KeyTable"; then
-      echo "${KEYTABLEENTRY}" >>/tmp/docker-mailserver/opendkim/KeyTable
-    fi
     fi
+  done
 
-  # write to SigningTable if necessary
+  # If file exists but is empty, add default hosts to trust:
-  SIGNINGTABLEENTRY="*@${DKIM_DOMAIN} ${SELECTOR}._domainkey.${DKIM_DOMAIN}"
+  if [[ ! -s "${TRUSTED_HOSTS_FILE}" ]]; then
-  if [[ ! -f /tmp/docker-mailserver/opendkim/SigningTable ]]; then
+    _log 'debug' 'Adding default trust to OpenDKIM TrustedHosts config'
-    _log 'debug' 'Creating DKIM SigningTable'
+    echo "127.0.0.1" >  "${TRUSTED_HOSTS_FILE}"
-    echo "*@${DKIM_DOMAIN} ${SELECTOR}._domainkey.${DKIM_DOMAIN}" >/tmp/docker-mailserver/opendkim/SigningTable
+    echo "localhost" >> "${TRUSTED_HOSTS_FILE}"
-  else
-    if ! grep -q "${SIGNINGTABLEENTRY}" /tmp/docker-mailserver/opendkim/SigningTable; then
-      echo "${SIGNINGTABLEENTRY}" >>/tmp/docker-mailserver/opendkim/SigningTable
   fi
-  fi
+}
-done < <(_get_valid_lines_from_file "${DATABASE_VHOST}")
 
-# create TrustedHosts if missing
+_main "${@}"
-if [[ -d /tmp/docker-mailserver/opendkim ]] && [[ ! -f /tmp/docker-mailserver/opendkim/TrustedHosts ]]; then
-  _log 'debug' 'Creating DKIM TrustedHosts'
-  echo "127.0.0.1" >/tmp/docker-mailserver/opendkim/TrustedHosts
-  echo "localhost" >>/tmp/docker-mailserver/opendkim/TrustedHosts
-fi

target/scripts/helpers/error.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 function _exit_with_error() {
-  if [[ -n ${1+set} ]]; then
+  if [[ -n ${1:-} ]]; then
     _log 'error' "${1}"
   else
     _log 'error' "Call to '_exit_with_error' is missing a message to log"

target/scripts/helpers/log.sh

@@ -43,12 +43,12 @@ RESET=$(echo -ne   '\e[0m')
 # message is logged. Likewise when the second argument
 # is missing. Both failures will return with exit code '1'.
 function _log() {
-  if [[ -z ${1+set} ]]; then
+  if [[ -z ${1:-} ]]; then
     _log 'error' "Call to '_log' is missing a valid log level"
     return 1
   fi
 
-  if [[ -z ${2+set} ]]; then
+  if [[ -z ${2:-} ]]; then
     _log 'error' "Call to '_log' is missing a message to log"
     return 1
   fi
@@ -116,7 +116,7 @@ function _log() {
 # variables file. If this does not yield a value either,
 # use the default log level.
 function _get_log_level_or_default() {
-  if [[ -n ${LOG_LEVEL+set} ]]; then
+  if [[ -n ${LOG_LEVEL:-} ]]; then
     echo "${LOG_LEVEL}"
   elif [[ -e /etc/dms-settings ]] && grep -q -E "^LOG_LEVEL='[a-z]+'" /etc/dms-settings; then
     grep '^LOG_LEVEL=' /etc/dms-settings | cut -d "'" -f 2

target/scripts/helpers/utils.sh

@@ -131,9 +131,9 @@ function _reload_postfix() {
 # 1. No first and second argument is supplied
 # 2. The second argument is a path to a file that does not exist
 function _replace_by_env_in_file() {
-  if [[ -z ${1+set} ]]; then
+  if [[ -z ${1:-} ]]; then
     _dms_panic__invalid_value 'first argument unset' 'utils.sh:_replace_by_env_in_file'
-  elif [[ -z ${2+set} ]]; then
+  elif [[ -z ${2:-} ]]; then
     _dms_panic__invalid_value 'second argument unset' 'utils.sh:_replace_by_env_in_file'
   elif [[ ! -f ${2} ]]; then
     _dms_panic__invalid_value "file '${2}' does not exist" 'utils.sh:_replace_by_env_in_file'

target/scripts/startup/setup.d/dmarc_dkim_spf.sh

@@ -23,7 +23,11 @@ function _setup_opendkim() {
     # check if any keys are available
     if [[ -e /tmp/docker-mailserver/opendkim/KeyTable ]]; then
       cp -a /tmp/docker-mailserver/opendkim/* /etc/opendkim/
-      _log 'trace' "DKIM keys added for: $(find /etc/opendkim/keys/ -maxdepth 1 -type f -printf '%f ')"
+
+      local DKIM_DOMAINS
+      DKIM_DOMAINS=$(find /etc/opendkim/keys/ -maxdepth 1 -type f -printf '%f ')
+      _log 'trace' "DKIM keys added for: ${DKIM_DOMAINS}"
+
       chown -R opendkim:opendkim /etc/opendkim/
       chmod -R 0700 /etc/opendkim/keys/
     else

test/helper/common.bash

@@ -56,7 +56,7 @@ function __handle_container_name() {
   if [[ -n ${1:-} ]] && [[ ${1:-} =~ ^dms-test_ ]]; then
     printf '%s' "${1}"
     return 0
-  elif [[ -n ${CONTAINER_NAME+set} ]]; then
+  elif [[ -n ${CONTAINER_NAME:-} ]]; then
     printf '%s' "${CONTAINER_NAME}"
     return 0
   else

test/helper/setup.bash

@@ -17,7 +17,7 @@
 # This function is internal and should not be used in tests.
 function __initialize_variables() {
   function __check_if_set() {
-    if [[ ${!1+set} != 'set' ]]; then
+    if [[ -z ${!1:-} ]]; then
       echo "ERROR: (helper/setup.sh) '${1:?No variable name given to __check_if_set}' is not set" >&2
       exit 1
     fi

test/tests/serial/open_dkim.bats

@@ -62,7 +62,7 @@ function teardown() { _default_teardown ; }
 
   __init_container_without_waiting
 
-  __should_generate_dkim_key 6
+  __should_generate_dkim_key 7
   __assert_outputs_common_dkim_logs
 
   __should_have_tables_trustedhosts_for_domain
@@ -78,7 +78,7 @@ function teardown() { _default_teardown ; }
   # Only mount single config file (postfix-virtual.cf):
   __init_container_without_waiting "${PWD}/test/config/postfix-virtual.cf:/tmp/docker-mailserver/postfix-virtual.cf:ro"
 
-  __should_generate_dkim_key 5
+  __should_generate_dkim_key 6
   __assert_outputs_common_dkim_logs
 
   __should_have_tables_trustedhosts_for_domain
@@ -95,7 +95,7 @@ function teardown() { _default_teardown ; }
   # Only mount single config file (postfix-accounts.cf):
   __init_container_without_waiting "${PWD}/test/config/postfix-accounts.cf:/tmp/docker-mailserver/postfix-accounts.cf:ro"
 
-  __should_generate_dkim_key 5
+  __should_generate_dkim_key 6
   __assert_outputs_common_dkim_logs
 
   __should_have_tables_trustedhosts_for_domain
@@ -113,7 +113,7 @@ function teardown() { _default_teardown ; }
   __init_container_without_waiting '/tmp/docker-mailserver'
 
   # generate first key (with a custom selector)
-  __should_generate_dkim_key 4 '1024' 'domain1.tld' 'mailer'
+  __should_generate_dkim_key 5 '1024' 'domain1.tld' 'mailer'
   __assert_outputs_common_dkim_logs
   # generate two additional keys different to the previous one
   __should_generate_dkim_key 2 '1024' 'domain2.tld,domain3.tld'
@@ -183,15 +183,15 @@ function __assert_logged_dkim_creation() {
 
 function __assert_outputs_common_dkim_logs() {
   refute_output --partial 'No entries found, no keys to make'
-  assert_output --partial 'Creating DKIM KeyTable'
+  assert_output --partial "Creating OpenDKIM config '/tmp/docker-mailserver/opendkim/KeyTable'"
-  assert_output --partial 'Creating DKIM SigningTable'
+  assert_output --partial "Creating OpenDKIM config '/tmp/docker-mailserver/opendkim/SigningTable'"
-  assert_output --partial 'Creating DKIM TrustedHosts'
+  assert_output --partial "Creating OpenDKIM config '/tmp/docker-mailserver/opendkim/TrustedHosts'"
 }
 
 function __should_support_creating_key_of_size() {
   local EXPECTED_KEYSIZE=${1:-}
 
-  __should_generate_dkim_key 6 "${EXPECTED_KEYSIZE}"
+  __should_generate_dkim_key 7 "${EXPECTED_KEYSIZE}"
   __assert_outputs_common_dkim_logs
   __assert_logged_dkim_creation 'localdomain2.com'
   __assert_logged_dkim_creation 'localhost.localdomain'