diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index dbfc15468..10b3c52d9 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/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): diff --git a/certbot-apache/tests/configurator_test.py b/certbot-apache/tests/configurator_test.py index 8fd3cb750..b5dcc464d 100644 --- a/certbot-apache/tests/configurator_test.py +++ b/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] diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index ad8a04e07..9f5ce6226 100644 --- a/certbot/CHANGELOG.md +++ b/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