From 444e3251bff0cb54549184afdfdcaa9ad1acf8d6 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Mon, 25 Aug 2025 10:19:18 -0700
Subject: [PATCH] make apache tls warning conditional (#10444)

this is in response to the thread at
https://opensource.eff.org/eff-open-source/pl/49gnrcner7gptrnsi339bqu1yo
---
 .../certbot_apache/_internal/configurator.py  | 12 ++--
 .../_internal/tests/configurator_test.py      | 56 +++++++++++++++----
 newsfragments/10444.fixed                     |  1 +
 3 files changed, 53 insertions(+), 16 deletions(-)
 create mode 100644 newsfragments/10444.fixed

diff --git a/certbot-apache/src/certbot_apache/_internal/configurator.py b/certbot-apache/src/certbot_apache/_internal/configurator.py
index 7e33517f9..6ec52c2fc 100644
--- a/certbot-apache/src/certbot_apache/_internal/configurator.py
+++ b/certbot-apache/src/certbot_apache/_internal/configurator.py
@@ -160,10 +160,14 @@ class ApacheConfigurator(common.Configurator):
         openssl_version = self.openssl_version(warn_on_no_mod_ssl)
         if self.version < (2, 4, 11) or not openssl_version or \
             util.parse_loose_version(openssl_version) < min_openssl_version:
-            logger.warning('Certbot has detected that apache version < 2.4.11 or compiled against '
-                'openssl < 1.0.2l. Since these are deprecated, the configuration file being '
-                'installed at %s will not receive future updates. To get the latest configuration '
-                'version, update apache.', self.mod_ssl_conf)
+            # If we're supposed to warn about failing to find mod_ssl, we already did it in the
+            # openssl_version function and don't need to do it again here.
+            if openssl_version is not None:
+                logger.warning('Certbot has detected that apache version < 2.4.11 or compiled '
+                    'against openssl < 1.0.2l. Since these are deprecated, the configuration file '
+                    'being installed at %s will not receive future updates. To get the latest '
+                    'configuration version, update apache.',
+                    self.mod_ssl_conf)
             return apache_util.find_ssl_apache_conf("old")
         return apache_util.find_ssl_apache_conf("current")
 
diff --git a/certbot-apache/src/certbot_apache/_internal/tests/configurator_test.py b/certbot-apache/src/certbot_apache/_internal/tests/configurator_test.py
index 1ec50e4da..912a1f3df 100644
--- a/certbot-apache/src/certbot_apache/_internal/tests/configurator_test.py
+++ b/certbot-apache/src/certbot_apache/_internal/tests/configurator_test.py
@@ -1646,6 +1646,50 @@ class InstallSslOptionsConfTest(util.ApacheTest):
             self._call()
             assert mock_logger.warning.called is False
 
+    @mock.patch('certbot_apache._internal.configurator.logger.warning')
+    @mock.patch('certbot_apache._internal.configurator.ApacheConfigurator.openssl_version')
+    def test_pick_apache_config_versions_and_warnings(self, mock_openssl_version, mock_warning):
+        def has_logged_warning():
+            """Returns True if a warning was logged about updating Apache."""
+            return any('update apache' in call.args[0] for call in mock_warning.call_args_list)
+
+        # old apache, no openssl
+        self.config.version = (2, 4, 10)
+        mock_openssl_version.return_value = None
+        assert 'old' in self.config.pick_apache_config()
+        assert not has_logged_warning()
+
+        # old apache, old openssl
+        mock_warning.reset_mock()
+        mock_openssl_version.return_value = '1.0.2a'
+        assert 'old' in self.config.pick_apache_config()
+        assert has_logged_warning()
+
+        # old apache, new openssl
+        mock_warning.reset_mock()
+        mock_openssl_version.return_value = '1.0.2m'
+        assert 'old' in self.config.pick_apache_config()
+        assert has_logged_warning()
+
+        # new apache, no openssl
+        mock_warning.reset_mock()
+        self.config.version = (2, 4, 11)
+        mock_openssl_version.return_value = None
+        assert 'old' in self.config.pick_apache_config()
+        assert not has_logged_warning()
+
+        # new apache, old openssl
+        mock_warning.reset_mock()
+        mock_openssl_version.return_value = '1.0.2a'
+        assert 'old' in self.config.pick_apache_config()
+        assert has_logged_warning()
+
+        # new apache, new openssl
+        mock_warning.reset_mock()
+        mock_openssl_version.return_value = '1.0.2m'
+        assert 'current' in self.config.pick_apache_config()
+        assert not has_logged_warning()
+
     def test_ssl_config_files_hash_in_all_hashes(self):
         """
         It is really critical that all TLS Apache config files have their SHA256 hash registered in
@@ -1697,18 +1741,6 @@ class InstallSslOptionsConfTest(util.ApacheTest):
             mock_omf.return_value = some_string_contents
             assert self.config.openssl_version() == "1.0.2g"
 
-    def test_current_version(self):
-        self.config.version = (2, 4, 10)
-        self.config._openssl_version = '1.0.2m'
-        assert 'old' in self.config.pick_apache_config()
-
-        self.config.version = (2, 4, 11)
-        self.config._openssl_version = '1.0.2m'
-        assert 'current' in self.config.pick_apache_config()
-
-        self.config._openssl_version = '1.0.2a'
-        assert 'old' in self.config.pick_apache_config()
-
     def test_openssl_version_warns(self):
         self.config._openssl_version = '1.0.2a'
         assert self.config.openssl_version() == '1.0.2a'
diff --git a/newsfragments/10444.fixed b/newsfragments/10444.fixed
new file mode 100644
index 000000000..ccd999e85
--- /dev/null
+++ b/newsfragments/10444.fixed
@@ -0,0 +1 @@
+certbot-apache no longer prints a warning claiming the version of OpenSSL used by Apache is too old when we were unable determine the OpenSSL version.