From 0ecb647ae2b8e333ee754b2a27f104c222f2ef88 Mon Sep 17 00:00:00 2001
From: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Date: Thu, 12 Jan 2023 10:04:50 +1300
Subject: [PATCH] tests(refactor): Adjust `mail_tls_dhparams.bats` (#2994)

* tests(chore): `tls-dh-params.bats` - Drop `ONE_DIR` ENV variants

There is no longer special handling for this ENV with this feature, these variant test cases serve no value.

* tests(refactor): `tls-dh-params.bats`

Converted to new common setup helper methods and testing structure.

No `setup_file` needed. Only two test cases used now, the Mozilla check is bundled into the default params test case where it's relevant.

Refactored some logic into common functions. Should be easier to grok intention.

* chore: Apply review feedback

Co-authored-by: Casper <casperklein@users.noreply.github.com>

* chore: Inline functions into test cases

As per review feedback
---
 test/tests/serial/mail_tls_dhparams.bats | 155 +++++++----------------
 1 file changed, 48 insertions(+), 107 deletions(-)

diff --git a/test/tests/serial/mail_tls_dhparams.bats b/test/tests/serial/mail_tls_dhparams.bats
index 274b1dd1..2379e50e 100644
--- a/test/tests/serial/mail_tls_dhparams.bats
+++ b/test/tests/serial/mail_tls_dhparams.bats
@@ -1,127 +1,68 @@
-load "${REPOSITORY_ROOT}/test/test_helper/common"
+load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/setup"
 
 # Test case
 # ---------
 # By default, this image is using audited FFDHE groups (https://github.com/docker-mailserver/docker-mailserver/pull/1463)
-#
-# This test case covers the described case against both boolean states for `ONE_DIR`.
-#
-# Description:
-# 1. Verify that the file `ffdhe4096.pem` has not been modified (checksum verification).
-# 2. Verify Postfix and Dovecot are using the default `ffdhe4096.pem` from Dockerfile build.
-# 3. When custom DHE parameters are supplied by the user as `/tmp/docker-mailserver/dhparams.pem`:
-#    - Verify Postfix and Dovecot use the custom `custom-dhe-params.pem` (contents is actually `ffdhe2048.pem`).
-#    - A warning is raised about usage of potentially insecure parameters.
+# Reference used (22/04/2020) - Page 27 (ffdhe4096 RFC 7919, regarded as sufficient):
+# https://english.ncsc.nl/publications/publications/2019/juni/01/it-security-guidelines-for-transport-layer-security-tls
 
-function teardown() {
-  docker rm -f mail_dhparams
-}
+BATS_TEST_NAME_PREFIX='[Security] TLS (DH Parameters) '
 
-function setup_file() {
-  # Delegated container setup to common_container_setup
-  # DRY - Explicit config changes between tests are more apparent this way.
+CONTAINER1_NAME='dms-test_tls-dh-params_default'
+CONTAINER2_NAME='dms-test_tls-dh-params_custom'
 
-  # Global scope
-  # Copies all of `./test/config/` to specific directory for testing
-  # `${PRIVATE_CONFIG}` becomes `$(pwd)/test/duplicate_configs/<bats test filename>`
-  export PRIVATE_CONFIG
+function teardown() { _default_teardown ; }
 
-  export DMS_ONE_DIR=1 # default
-
-  local DH_DEFAULT_PARAMS
-  export DH_DEFAULT_CHECKSUM
-  export DH_CUSTOM_PARAMS
-  export DH_CUSTOM_CHECKSUM
-
-  DH_DEFAULT_PARAMS="$(pwd)/target/shared/ffdhe4096.pem"
-  DH_DEFAULT_CHECKSUM=$(sha512sum "${DH_DEFAULT_PARAMS}" | awk '{print $1}')
-
-  DH_CUSTOM_PARAMS="$(pwd)/test/test-files/ssl/custom-dhe-params.pem"
-  DH_CUSTOM_CHECKSUM=$(sha512sum "${DH_CUSTOM_PARAMS}" | awk '{print $1}')
-}
-
-# Not used
-# function teardown_file() {
-# }
-
-@test "testing tls: DH Parameters - Verify integrity of Default (ffdhe4096)" {
-  # Reference used (22/04/2020):
-  # https://english.ncsc.nl/publications/publications/2019/juni/01/it-security-guidelines-for-transport-layer-security-tls
-
-  run echo "${DH_DEFAULT_CHECKSUM}"
-  refute_output '' # checksum must not be empty
-
-  # Verify the FFDHE params file has not been modified (equivalent to `target/shared/ffdhe4096.pem.sha512sum`):
-  local DH_MOZILLA_CHECKSUM
-  DH_MOZILLA_CHECKSUM=$(curl https://ssl-config.mozilla.org/ffdhe4096.txt -s | sha512sum | awk '{print $1}')
-  assert_equal "${DH_DEFAULT_CHECKSUM}" "${DH_MOZILLA_CHECKSUM}"
-}
-
-@test "testing tls: DH Parameters - Default [ONE_DIR=0]" {
-  PRIVATE_CONFIG=$(duplicate_config_for_container . mail_dhparams_default_0)
-  DMS_ONE_DIR=0
+# Verify Postfix and Dovecot are using the default `ffdhe4096.pem` from Dockerfile build.
+# Verify that the file `ffdhe4096.pem` has not been modified (checksum verification against trusted third-party copy).
+@test "Default" {
+  export CONTAINER_NAME=${CONTAINER1_NAME}
+  local DH_PARAMS_DEFAULT='target/shared/ffdhe4096.pem'
+  local DH_CHECKSUM_DEFAULT=$(sha512sum "${DH_PARAMS_DEFAULT}" | awk '{print $1}')
 
+  init_with_defaults
   common_container_setup
-  should_have_valid_checksum "${DH_DEFAULT_CHECKSUM}"
+
+  _should_match_service_copies "${DH_CHECKSUM_DEFAULT}"
+  
+  # Verify integrity of the default supplied DH Params (ffdhe4096, should be equivalent to `target/shared/ffdhe4096.pem.sha512sum`):
+  # 716a462baecb43520fb1ba6f15d288ba8df4d612bf9d450474b4a1c745b64be01806e5ca4fb2151395fd4412a98831b77ea8dfd389fe54a9c768d170b9565a25
+  local DH_CHECKSUM_MOZILLA
+  DH_CHECKSUM_MOZILLA=$(curl https://ssl-config.mozilla.org/ffdhe4096.txt -s | sha512sum | awk '{print $1}')
+  assert_equal "${DH_CHECKSUM_DEFAULT}" "${DH_CHECKSUM_MOZILLA}"
 }
 
-@test "testing tls: DH Parameters - Default [ONE_DIR=1]" {
-  PRIVATE_CONFIG=$(duplicate_config_for_container . mail_dhparams_default_1)
+# When custom DHE parameters are supplied by the user to `/tmp/docker-mailserver/dhparams.pem`:
+# - Verify Postfix and Dovecot use the custom `custom-dhe-params.pem` (contents tested is actually `ffdhe2048.pem`).
+# - A warning is raised about usage of potentially insecure parameters.
+@test "Custom" {
+  export CONTAINER_NAME=${CONTAINER2_NAME}
+  local DH_PARAMS_CUSTOM='test/test-files/ssl/custom-dhe-params.pem'
+  local DH_CHECKSUM_CUSTOM=$(sha512sum "${DH_PARAMS_CUSTOM}" | awk '{print $1}')
 
+  init_with_defaults
+  cp "${DH_PARAMS_CUSTOM}" "${TEST_TMP_CONFIG}/dhparams.pem"
   common_container_setup
-  should_have_valid_checksum "${DH_DEFAULT_CHECKSUM}"
+
+  _should_match_service_copies "${DH_CHECKSUM_CUSTOM}"
+
+  # Should emit a warning:
+  run docker logs "${CONTAINER_NAME}"
+  assert_success
+  assert_output --partial '[ WARNING ]  Using self-generated dhparams is considered insecure - unless you know what you are doing, please remove'
 }
 
-@test "testing tls: DH Parameters - Custom [ONE_DIR=0]" {
-  PRIVATE_CONFIG=$(duplicate_config_for_container . mail_dhparams_custom_0)
-  # shellcheck disable=SC2030
-  DMS_ONE_DIR=0
-
-  cp "${DH_CUSTOM_PARAMS}" "${PRIVATE_CONFIG}/dhparams.pem"
-
-  common_container_setup
-  should_have_valid_checksum "${DH_CUSTOM_CHECKSUM}"
-  should_emit_warning
-}
-
-@test "testing tls: DH Parameters - Custom [ONE_DIR=1]" {
-  # shellcheck disable=SC2030
-  PRIVATE_CONFIG=$(duplicate_config_for_container . mail_dhparams_custom_1)
-
-  cp "${DH_CUSTOM_PARAMS}" "${PRIVATE_CONFIG}/dhparams.pem"
-
-  common_container_setup
-  should_have_valid_checksum "${DH_CUSTOM_CHECKSUM}"
-  should_emit_warning
-}
-
-function common_container_setup() {
-  # shellcheck disable=SC2031
-  docker run -d --name mail_dhparams \
-    -v "${PRIVATE_CONFIG}:/tmp/docker-mailserver" \
-    -v "$(pwd)/test/test-files:/tmp/docker-mailserver-test:ro" \
-    -e ONE_DIR="${DMS_ONE_DIR}" \
-    -h mail.my-domain.com \
-    --tty \
-    "${NAME}"
-
-  wait_for_finished_setup_in_container mail_dhparams
-}
-
-# Ensures the docker image services (Postfix and Dovecot) have the intended DH files
-function should_have_valid_checksum() {
+# Ensures the docker image services (Postfix and Dovecot) have the expected DH files:
+function _should_match_service_copies() {
   local DH_CHECKSUM=$1
 
-  local DH_CHECKSUM_DOVECOT
-  DH_CHECKSUM_DOVECOT=$(docker exec mail_dhparams sha512sum /etc/dovecot/dh.pem | awk '{print $1}')
-  assert_equal "${DH_CHECKSUM_DOVECOT}" "${DH_CHECKSUM}"
+  function __should_have_expected_checksum() {
+    _run_in_container bash -c "sha512sum ${1} | awk '{print \$1}'"
+    assert_success
+    assert_output "${DH_CHECKSUM}"
+  }
 
-  local DH_CHECKSUM_POSTFIX
-  DH_CHECKSUM_POSTFIX=$(docker exec mail_dhparams sha512sum /etc/postfix/dhparams.pem | awk '{print $1}')
-  assert_equal "${DH_CHECKSUM_POSTFIX}" "${DH_CHECKSUM}"
-}
-
-function should_emit_warning() {
-  run sh -c "docker logs mail_dhparams | grep 'Using self-generated dhparams is considered insecure.'"
-  assert_success
+  __should_have_expected_checksum '/etc/dovecot/dh.pem'
+  __should_have_expected_checksum '/etc/postfix/dhparams.pem'
 }