tests: Revise dhparams tests

- `dhparam_generation` tests are no longer necessary, dropped.

Modified the remaining `dhparam` test to use multiple `nginx-proxy` images to verify correct behavior for different configs.

Tests now cover:

- Default (ffdhe4096) is used.
- Alternative via ENV (ffdhe2048) works correctly.
- Invalid group via ENV (1024-bit) fails.
- Custom DH params provided via file mount works with warning emitted.

---

- `assert_log_contains`: added a `container_name` arg with `nginxproxy` as the default value. This allows multiple nginx-proxy containers to utilize this method instead.

- Extracted out the `openssl` test (_to `negotiate_cipher()`_) and modified it to be a bit more flexible. It now takes a container with optional extra args to pass to `openssl` command called, as well as the `grep` string to match. This made the original test redundant, so I've dropped it.

- Added two methods to use `negotiate_cipher()`, one verifies a DHE cipher suite was negotiated and checks that a DH emphermal key was also mentioned in the output. The other method verifies the expectation of failing to negotiate a valid cipher if DH params have not been set, while verifying that non-DHE cipher suites can be successfully negotiated.

- Added a `get_env()` method for extracting attached environments on a container. This is useful for verifying invalid `DHPARAM_BITS` values (eg `1024`-bit).

- The original `Server Temp Key` assertion was incorrect, it was expecting a value that is unrelated to DHE cipher suite support (_`X25519` is related to ECDHE_). This is due to TLS 1.3 being negotiated where you cannot use custom DH params, nor influence the negotiated cipher due to this mechanism changing from TLS 1.3. TLS 1.3 does support DH params, but it internally negotiates RFC 7919 group between server and client instead. Thus to verify expectations, the connection via `openssl` is made explicitly with TLS 1.2 instead.

test/test_ssl/test_dhparam.py

@@ -15,7 +15,7 @@ docker_client = docker.from_env()
 ###############################################################################
 
 @backoff.on_exception(backoff.constant, AssertionError, interval=2, max_tries=15, jitter=None)
-def assert_log_contains(expected_log_line):
+def assert_log_contains(expected_log_line, container_name="nginxproxy"):
     """
     Check that the nginx-proxy container log contains a given string.
     The backoff decorator will retry the check 15 times with a 2 seconds delay.
@@ -24,7 +24,7 @@ def assert_log_contains(expected_log_line):
     :return: None
     :raises: AssertError if the expected string is not found in the log
     """
-    sut_container = docker_client.containers.get("nginxproxy")
+    sut_container = docker_client.containers.get(container_name)
     docker_logs = sut_container.logs(stdout=True, stderr=True, stream=False, follow=False)
     assert bytes(expected_log_line, encoding="utf8") in docker_logs
 
@@ -58,36 +58,117 @@ def require_openssl(required_version):
             reason=f"openssl v{openssl_version} is less than required version {required_version}")
 
 
+@require_openssl("1.0.2")
+def negotiate_cipher(sut_container, additional_params='', grep='Cipher is'):
+    host = f"{sut_container.attrs['NetworkSettings']['IPAddress']}:443"
+    
+    return subprocess.check_output(
+        f"echo '' | openssl s_client -connect {host} -tls1_2 {additional_params} | grep '{grep}'",
+        shell=True
+    )
+
+
+def can_negotiate_dhe_ciphersuite(sut_container):
+    r = negotiate_cipher(sut_container, "-cipher 'EDH'")
+    assert b"New, TLSv1.2, Cipher is DHE-RSA-AES256-GCM-SHA384\n" == r
+
+    r2 = negotiate_cipher(sut_container, "-cipher 'EDH'", "Server Temp Key")
+    assert b"DH" in r2
+
+
+def cannot_negotiate_dhe_ciphersuite(sut_container):
+    # Fail to negotiate a DHE cipher suite:
+    r = negotiate_cipher(sut_container, "-cipher 'EDH'")
+    assert b"New, (NONE), Cipher is (NONE)\n" == r
+
+    # Correctly establish a connection (TLS 1.2):
+    r2 = negotiate_cipher(sut_container)
+    assert b"New, TLSv1.2, Cipher is ECDHE-RSA-AES256-GCM-SHA384\n" == r2
+
+    r3 = negotiate_cipher(sut_container, grep="Server Temp Key")
+    assert b"X25519" in r3
+
+
+# Parse array of container ENV, splitting at the `=` and returning the value, otherwise `None`
+def get_env(sut_container, var):
+  env = sut_container.attrs['Config']['Env']
+
+  for e in env:
+    if e.startswith(var):
+      return e.split('=')[1]
+  
+  return None
+
+
 ###############################################################################
 #
 # Tests
 #
 ###############################################################################
 
-def test_dhparam_is_not_generated_if_present(docker_compose):
-    sut_container = docker_client.containers.get("nginxproxy")
+def test_default_dhparam_is_ffdhe4096(docker_compose):
+    container_name="dh-default"
+    sut_container = docker_client.containers.get(container_name)
     assert sut_container.status == "running"
 
-    assert_log_contains("Custom dhparam.pem file found, generation skipped")
+    assert_log_contains("Setting up DH Parameters..", container_name)
 
-    # Make sure the dhparam in use is not the default, pre-generated one
-    default_checksum = sut_container.exec_run("md5sum /app/dhparam.pem.default").output.split()
+    # Make sure the dhparam file used is the default ffdhe4096.pem:
+    default_checksum = sut_container.exec_run("md5sum /app/dhparam/ffdhe4096.pem").output.split()
+    current_checksum = sut_container.exec_run("md5sum /etc/nginx/dhparam/dhparam.pem").output.split()
+    assert default_checksum[0] == current_checksum[0]
+
+    can_negotiate_dhe_ciphersuite(sut_container)
+
+
+def test_can_change_dhparam_group(docker_compose):
+    container_name="dh-env"
+    sut_container = docker_client.containers.get(container_name)
+    assert sut_container.status == "running"
+
+    assert_log_contains("Setting up DH Parameters..", container_name)
+
+    # Make sure the dhparam file used is ffdhe2048.pem, not the default (ffdhe4096.pem):
+    default_checksum = sut_container.exec_run("md5sum /app/dhparam/ffdhe2048.pem").output.split()
+    current_checksum = sut_container.exec_run("md5sum /etc/nginx/dhparam/dhparam.pem").output.split()
+    assert default_checksum[0] == current_checksum[0]
+
+    can_negotiate_dhe_ciphersuite(sut_container)
+
+
+def test_fail_if_dhparam_group_not_supported(docker_compose):
+    container_name="invalid-group-1024"
+    sut_container = docker_client.containers.get(container_name)
+    assert sut_container.status == "exited"
+
+    DHPARAM_BITS = get_env(sut_container, "DHPARAM_BITS")
+    assert DHPARAM_BITS == "1024"
+
+    assert_log_contains(
+        f"ERROR: Unsupported DHPARAM_BITS size: {DHPARAM_BITS}. Use: 2048, 3072, or 4096 (default).",
+        container_name
+    )
+
+
+def test_custom_dhparam_is_supported(docker_compose):
+    container_name="dh-file"
+    sut_container = docker_client.containers.get(container_name)
+    assert sut_container.status == "running"
+
+    assert_log_contains(
+        "Warning: A custom dhparam.pem file was provided. Best practice is to use standardized RFC7919 DHE groups instead.",
+        container_name
+    )
+
+    # Make sure the dhparam file used is not the default (ffdhe4096.pem):
+    default_checksum = sut_container.exec_run("md5sum /app/dhparam/ffdhe4096.pem").output.split()
     current_checksum = sut_container.exec_run("md5sum /etc/nginx/dhparam/dhparam.pem").output.split()
     assert default_checksum[0] != current_checksum[0]
 
+    can_negotiate_dhe_ciphersuite(sut_container)
+
 
 def test_web5_https_works(docker_compose, nginxproxy):
     r = nginxproxy.get("https://web5.nginx-proxy.tld/port", allow_redirects=False)
     assert r.status_code == 200
     assert "answer from port 85\n" in r.text
-
-
-@require_openssl("1.0.2")
-def test_web5_dhparam_is_used(docker_compose):
-    sut_container = docker_client.containers.get("nginxproxy")
-    assert sut_container.status == "running"
-
-    host = f"{sut_container.attrs['NetworkSettings']['IPAddress']}:443"
-    r = subprocess.check_output(
-        f"echo '' | openssl s_client -connect {host} -cipher 'EDH' | grep 'Server Temp Key'", shell=True)
-    assert b"Server Temp Key: X25519, 253 bits\n" == r

test/test_ssl/test_dhparam.yml

@@ -6,11 +6,36 @@ web5:
     WEB_PORTS: "85"
     VIRTUAL_HOST: "web5.nginx-proxy.tld"
 
+# sut - System Under Test
+with_default_group:
+  image: nginxproxy/nginx-proxy:test
+  container_name: dh-default
+  volumes:
+    - /var/run/docker.sock:/tmp/docker.sock:ro
+    - ./certs:/etc/nginx/certs:ro
+
+with_alternative_group:
+  image: nginxproxy/nginx-proxy:test
+  container_name: dh-env
+  environment:
+    - DHPARAM_BITS=2048
+  volumes:
+    - /var/run/docker.sock:/tmp/docker.sock:ro
+    - ./certs:/etc/nginx/certs:ro
+
+with_invalid_group:
+  image: nginxproxy/nginx-proxy:test
+  container_name: invalid-group-1024
+  environment:
+    - DHPARAM_BITS=1024
+  volumes:
+    - /var/run/docker.sock:/tmp/docker.sock:ro
+    - ./certs:/etc/nginx/certs:ro
 
-sut:
+with_custom_file:
   image: nginxproxy/nginx-proxy:test
-  container_name: nginxproxy
+  container_name: dh-file
   volumes:
     - /var/run/docker.sock:/tmp/docker.sock:ro
-    - ../lib/ssl/dhparam.pem:/etc/nginx/dhparam/dhparam.pem:ro
     - ./certs:/etc/nginx/certs:ro
+    - ../lib/ssl/dhparam.pem:/etc/nginx/dhparam/dhparam.pem:ro

test/test_ssl/test_dhparam_generation.py

@@ -1,44 +0,0 @@
-import backoff
-import docker
-
-docker_client = docker.from_env()
-
-
-###############################################################################
-#
-# Tests helpers
-#
-###############################################################################
-
-@backoff.on_exception(backoff.constant, AssertionError, interval=2, max_tries=15, jitter=None)
-def assert_log_contains(expected_log_line):
-    """
-    Check that the nginx-proxy container log contains a given string.
-    The backoff decorator will retry the check 15 times with a 2 seconds delay.
-
-    :param expected_log_line: string to search for
-    :return: None
-    :raises: AssertError if the expected string is not found in the log
-    """
-    sut_container = docker_client.containers.get("nginxproxy")
-    docker_logs = sut_container.logs(stdout=True, stderr=True, stream=False, follow=False)
-    assert bytes(expected_log_line, encoding="utf8") in docker_logs
-
-
-###############################################################################
-#
-# Tests
-#
-###############################################################################
-
-def test_dhparam_is_generated_if_missing(docker_compose):
-    sut_container = docker_client.containers.get("nginxproxy")
-    assert sut_container.status == "running"
-
-    assert_log_contains("Generating DSA parameters")
-    assert_log_contains("dhparam generation complete, reloading nginx")
-
-    # Make sure the dhparam in use is not the default, pre-generated one
-    default_checksum = sut_container.exec_run("md5sum /app/dhparam.pem.default").output.split()
-    generated_checksum = sut_container.exec_run("md5sum /etc/nginx/dhparam/dhparam.pem").output.split()
-    assert default_checksum[0] != generated_checksum[0]

test/test_ssl/test_dhparam_generation.yml

@@ -1,8 +0,0 @@
-sut:
-  image: nginxproxy/nginx-proxy:test
-  container_name: nginxproxy
-  volumes:
-    - /var/run/docker.sock:/tmp/docker.sock:ro
-    - ./certs:/etc/nginx/certs:ro
-  environment:
-    - DHPARAM_BITS=256