Error out earlier in apache installer when mod_ssl is not available (#7984)

* Error out in apache installer when mod_ssl is not available

* Update to MisconfigurationError and add/fix tests

* Remove error cases we no longer hit and associated test

* mock out function to have consistent error across machines

* improve changelog message

* only check key in modules list, not value

certbot-apache/certbot_apache/_internal/configurator.py

@@ -595,6 +595,11 @@ class ApacheConfigurator(common.Installer):
         # cert_key... can all be parsed appropriately
         self.prepare_server_https("443")
 
+        # If we haven't managed to enable mod_ssl by this point, error out
+        if "ssl_module" not in self.parser.modules:
+            raise errors.MisconfigurationError("Could not find ssl_module; "
+                "not installing certificate.")
+
         # Add directives and remove duplicates
         self._add_dummy_ssl_directives(vhost.path)
         self._clean_vhost(vhost)
@@ -609,21 +614,6 @@ class ApacheConfigurator(common.Installer):
             path["chain_path"] = self.parser.find_dir(
                 "SSLCertificateChainFile", None, vhost.path)
 
-        # Handle errors when certificate/key directives cannot be found
-        if not path["cert_path"]:
-            logger.warning(
-                "Cannot find an SSLCertificateFile directive in %s. "
-                "VirtualHost was not modified", vhost.path)
-            raise errors.PluginError(
-                "Unable to find an SSLCertificateFile directive")
-        elif not path["cert_key"]:
-            logger.warning(
-                "Cannot find an SSLCertificateKeyFile directive for "
-                "certificate in %s. VirtualHost was not modified", vhost.path)
-            raise errors.PluginError(
-                "Unable to find an SSLCertificateKeyFile directive for "
-                "certificate")
-
         logger.info("Deploying Certificate to VirtualHost %s", vhost.filep)
 
         if self.version < (2, 4, 8) or (chain_path and not fullchain_path):

certbot-apache/tests/configurator_test.py

@@ -455,41 +455,6 @@ class MultipleVhostsTest(util.ApacheTest):
             "SSLCertificateChainFile", "two/cert_chain.pem",
             self.vh_truth[1].path))
 
-    def test_deploy_cert_invalid_vhost(self):
-        """For test cases where the `ApacheConfigurator` class' `_deploy_cert`
-        method is called with an invalid vhost parameter. Currently this tests
-        that a PluginError is appropriately raised when important directives
-        are missing in an SSL module."""
-        self.config.parser.modules["ssl_module"] = None
-        self.config.parser.modules["mod_ssl.c"] = None
-        self.config.parser.modules["socache_shmcb_module"] = None
-
-        def side_effect(*args):
-            """Mocks case where an SSLCertificateFile directive can be found
-            but an SSLCertificateKeyFile directive is missing."""
-            if "SSLCertificateFile" in args:
-                return ["example/cert.pem"]
-            return []
-
-        mock_find_dir = mock.MagicMock(return_value=[])
-        mock_find_dir.side_effect = side_effect
-
-        self.config.parser.find_dir = mock_find_dir
-
-        # Get the default 443 vhost
-        self.config.assoc["random.demo"] = self.vh_truth[1]
-
-        self.assertRaises(
-            errors.PluginError, self.config.deploy_cert, "random.demo",
-            "example/cert.pem", "example/key.pem", "example/cert_chain.pem")
-
-        # Remove side_effect to mock case where both SSLCertificateFile
-        # and SSLCertificateKeyFile directives are missing
-        self.config.parser.find_dir.side_effect = None
-        self.assertRaises(
-            errors.PluginError, self.config.deploy_cert, "random.demo",
-            "example/cert.pem", "example/key.pem", "example/cert_chain.pem")
-
     def test_is_name_vhost(self):
         addr = obj.Addr.fromstring("*:80")
         self.assertTrue(self.config.is_name_vhost(addr))
@@ -1349,6 +1314,16 @@ class MultipleVhostsTest(util.ApacheTest):
                 self.assertTrue(mock_add.called)
         shutil.rmtree(tmp_path)
 
+    def test_deploy_cert_no_mod_ssl(self):
+        # Create
+        ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
+        self.config.parser.modules["socache_shmcb_module"] = None
+        self.config.prepare_server_https = mock.Mock()
+
+        self.assertRaises(errors.MisconfigurationError, self.config.deploy_cert,
+            "encryption-example.demo", "example/cert.pem", "example/key.pem",
+            "example/cert_chain.pem", "example/fullchain.pem")
+
     @mock.patch("certbot_apache._internal.parser.ApacheParser.parsed_in_original")
     def test_choose_vhost_and_servername_addition_parsed(self, mock_parsed):
         ret_vh = self.vh_truth[8]

certbot/CHANGELOG.md

@@ -10,7 +10,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* Improved error message in apache installer when mod_ssl is not available.
 
 ### Fixed