From 23bb1c8e50dad1462c645b8a9cf50aeee8bc2625 Mon Sep 17 00:00:00 2001 From: Brennan Kinney <5098581+polarathene@users.noreply.github.com> Date: Mon, 31 Mar 2025 22:27:28 +1300 Subject: [PATCH] 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 Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> --- CHANGELOG.md | 7 + target/bin/open-dkim | 223 +++++++++++------- target/scripts/helpers/error.sh | 2 +- target/scripts/helpers/log.sh | 6 +- target/scripts/helpers/utils.sh | 4 +- .../scripts/startup/setup.d/dmarc_dkim_spf.sh | 6 +- test/helper/common.bash | 2 +- test/helper/setup.bash | 2 +- test/tests/serial/open_dkim.bats | 16 +- 9 files changed, 168 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b5cea3..90f2fd3f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/target/bin/open-dkim b/target/bin/open-dkim index 9410ecfa..60927635 100755 --- a/target/bin/open-dkim +++ b/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 -SELECTOR=mail -DOMAINS= +function _main() { + # Default parameters (updated by `_parse_arguments()`): + 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 + case "${1}" in -while [[ ${#} -gt 0 ]]; do - case "${1}" in - ( 'keysize' ) - if [[ -n ${2+set} ]]; then - KEYSIZE="${2}" - shift - shift - else - _exit_with_error "No keysize provided after 'keysize' argument" - fi - ;; + ( 'keysize' ) + if [[ -n ${2:-} ]]; then + KEYSIZE="${2}" + _log 'trace' "Keysize set to '${KEYSIZE}'" + else + _exit_with_error "No keysize provided after 'keysize' argument" + fi + ;; - ( 'selector' ) - if [[ -n ${2+set} ]]; then - # shellcheck disable=SC2034 - SELECTOR="${2}" - shift - shift - else - _exit_with_error "No selector provided after 'selector' argument" - fi - ;; + ( 'selector' ) + if [[ -n ${2:-} ]]; then + SELECTOR="${2}" + _log 'trace' "Selector set to '${SELECTOR}'" + else + _exit_with_error "No selector provided after 'selector' argument" + fi + ;; - ( 'domain' ) - if [[ -n ${2+set} ]]; then - DOMAINS="${2}" - shift - shift - else - _exit_with_error "No domain(s) provided after 'domain' argument" - fi - ;; + ( 'domain' ) + if [[ -n ${2:-} ]]; then + DMS_DOMAINS="${2}" + _log 'trace' "Domain(s) set to '${DMS_DOMAINS}'" + else + _exit_with_error "No domain(s) provided after 'domain' argument" + fi + ;; - ( * ) - __usage - _exit_with_error "Unknown options '${1}' ${2:+and \'${2}\'}" - ;; + ( 'help' ) + __usage + exit 0 + ;; - esac -done + ( * ) + __usage + _exit_with_error "Unknown option(s) ${*}" + ;; + esac + # 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: - if [[ -z ${DOMAINS} ]]; then + # unless CLI arg DMS_DOMAINS provided an alternative list to use instead: + 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 -if [[ ! -s ${DATABASE_VHOST} ]]; then - _log 'warn' 'No entries found, no keys to make' - exit 0 -fi +# `opendkim-genkey` generates two files at the configured `--directory`: +# - .private (Private key, PEM encoded) +# - .txt (Public key, formatted as a TXT record for a RFC 1035 DNS Zone file) +function _create_dkim_key() { + DKIM_DOMAIN=${1?Expected to be provided a domain} -while read -r DKIM_DOMAIN; do - mkdir -p "/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}" + OPENDKIM_DOMAINKEY_DIR="${OPENDKIM_BASE_DIR}/keys/${DKIM_DOMAIN}" + mkdir -p "${OPENDKIM_DOMAINKEY_DIR}" - if [[ ! -f "/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private" ]]; then - _log 'info' "Creating DKIM private key '/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private'" + DKIM_KEY_FILE="${OPENDKIM_DOMAINKEY_DIR}/${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 - chown -R "$(stat -c '%U:%G' /tmp/docker-mailserver/opendkim/keys)" "/tmp/docker-mailserver/opendkim/keys/${DKIM_DOMAIN}" +OPENDKIM_BASE_DIR='/tmp/docker-mailserver/opendkim' +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 - KEYTABLEENTRY="${SELECTOR}._domainkey.${DKIM_DOMAIN} ${DKIM_DOMAIN}:${SELECTOR}:/etc/opendkim/keys/${DKIM_DOMAIN}/${SELECTOR}.private" - if [[ ! -f "/tmp/docker-mailserver/opendkim/KeyTable" ]]; then - _log 'debug' 'Creating DKIM KeyTable' - echo "${KEYTABLEENTRY}" >/tmp/docker-mailserver/opendkim/KeyTable - else - if ! grep -q "${KEYTABLEENTRY}" "/tmp/docker-mailserver/opendkim/KeyTable"; then - echo "${KEYTABLEENTRY}" >>/tmp/docker-mailserver/opendkim/KeyTable + # Only create if the file doesn't exist (avoids modifying mtime): + for FILE in "${OPENDKIM_CONFIGS[@]}"; do + if [[ ! -f "${FILE}" ]]; then + _log 'debug' "Creating OpenDKIM config '${FILE}'" + touch "${FILE}" fi - fi + done - # write to SigningTable if necessary - SIGNINGTABLEENTRY="*@${DKIM_DOMAIN} ${SELECTOR}._domainkey.${DKIM_DOMAIN}" - if [[ ! -f /tmp/docker-mailserver/opendkim/SigningTable ]]; then - _log 'debug' 'Creating DKIM SigningTable' - echo "*@${DKIM_DOMAIN} ${SELECTOR}._domainkey.${DKIM_DOMAIN}" >/tmp/docker-mailserver/opendkim/SigningTable - else - if ! grep -q "${SIGNINGTABLEENTRY}" /tmp/docker-mailserver/opendkim/SigningTable; then - echo "${SIGNINGTABLEENTRY}" >>/tmp/docker-mailserver/opendkim/SigningTable - fi + # If file exists but is empty, add default hosts to trust: + if [[ ! -s "${TRUSTED_HOSTS_FILE}" ]]; then + _log 'debug' 'Adding default trust to OpenDKIM TrustedHosts config' + echo "127.0.0.1" > "${TRUSTED_HOSTS_FILE}" + echo "localhost" >> "${TRUSTED_HOSTS_FILE}" fi -done < <(_get_valid_lines_from_file "${DATABASE_VHOST}") +} -# create TrustedHosts if missing -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 +_main "${@}" diff --git a/target/scripts/helpers/error.sh b/target/scripts/helpers/error.sh index 3af736df..bac3630e 100644 --- a/target/scripts/helpers/error.sh +++ b/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" diff --git a/target/scripts/helpers/log.sh b/target/scripts/helpers/log.sh index d98f96a8..e4ba5ea6 100644 --- a/target/scripts/helpers/log.sh +++ b/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 diff --git a/target/scripts/helpers/utils.sh b/target/scripts/helpers/utils.sh index 90fd8f9b..468a4e74 100644 --- a/target/scripts/helpers/utils.sh +++ b/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' diff --git a/target/scripts/startup/setup.d/dmarc_dkim_spf.sh b/target/scripts/startup/setup.d/dmarc_dkim_spf.sh index c0d731f2..7b26f86b 100644 --- a/target/scripts/startup/setup.d/dmarc_dkim_spf.sh +++ b/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 diff --git a/test/helper/common.bash b/test/helper/common.bash index 35f41283..4e4ddf6a 100644 --- a/test/helper/common.bash +++ b/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 diff --git a/test/helper/setup.bash b/test/helper/setup.bash index ed2e4e32..95b7df28 100644 --- a/test/helper/setup.bash +++ b/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 diff --git a/test/tests/serial/open_dkim.bats b/test/tests/serial/open_dkim.bats index e51fdedb..b43b850f 100644 --- a/test/tests/serial/open_dkim.bats +++ b/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 DKIM SigningTable' - assert_output --partial 'Creating DKIM TrustedHosts' + assert_output --partial "Creating OpenDKIM config '/tmp/docker-mailserver/opendkim/KeyTable'" + assert_output --partial "Creating OpenDKIM config '/tmp/docker-mailserver/opendkim/SigningTable'" + 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'