Improve "cannot find cert of key directive" error (#5525) (#5679)

- Fix code to log separate error messages when either SSLCertificateFile or SSLCertificateKeyFile -
 directives are not found.
- Update the section in install.rst where the relevant error is referenced.
- Edit a docstring where 'cert' previously referred to certificate.
- Edit test_deploy_cert_invalid_vhost in the test suite to cover changes.

Fixes #5525.

certbot-apache/certbot_apache/configurator.py

@@ -285,8 +285,8 @@ def deploy_cert(self, domain, cert_path, key_path,
                     chain_path=None, fullchain_path=None):
         """Deploys certificate to specified virtual host.
 
-        Currently tries to find the last directives to deploy the cert in
-        the VHost associated with the given domain. If it can't find the
+        Currently tries to find the last directives to deploy the certificate
+        in the VHost associated with the given domain. If it can't find the
         directives, it searches the "included" confs. The function verifies
         that it has located the three directives and finally modifies them
         to point to the correct destination. After the certificate is
@@ -424,14 +424,20 @@ def _deploy_cert(self, vhost, cert_path, key_path, chain_path, fullchain_path):
             path["chain_path"] = self.parser.find_dir(
                 "SSLCertificateChainFile", None, vhost.path)
 
-        if not path["cert_path"] or not path["cert_key"]:
-            # Throw some can't find all of the directives error"
+        # Handle errors when certificate/key directives cannot be found
+        if not path["cert_path"]:
             logger.warning(
-                "Cannot find a cert or key directive in %s. "
+                "Cannot find an SSLCertificateFile directive in %s. "
                 "VirtualHost was not modified", vhost.path)
-            # Presumably break here so that the virtualhost is not modified
             raise errors.PluginError(
-                "Unable to find cert and/or key directives")
+                "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)
 
@@ -2117,5 +2123,3 @@ def install_ssl_options_conf(self, options_ssl, options_ssl_digest):
         # to be modified.
         return common.install_version_controlled_file(options_ssl, options_ssl_digest,
             self.constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
-
-

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -441,13 +441,37 @@ def find_args(path, directive):
             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.add("ssl_module")
-        mock_find = mock.MagicMock()
-        mock_find.return_value = []
-        self.config.parser.find_dir = mock_find
+        self.config.parser.modules.add("mod_ssl.c")
+        self.config.parser.modules.add("socache_shmcb_module")
+
+        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"]
+            else:
+                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")

docs/install.rst

@@ -204,10 +204,11 @@ want to use the Apache plugin, it has to be installed separately:
    emerge -av app-crypt/certbot
    emerge -av app-crypt/certbot-apache
 
-When using the Apache plugin, you will run into a "cannot find a cert or key
-directive" error if you're sporting the default Gentoo ``httpd.conf``.
-You can fix this by commenting out two lines in ``/etc/apache2/httpd.conf``
-as follows:
+When using the Apache plugin, you will run into a "cannot find an
+SSLCertificateFile directive" or "cannot find an SSLCertificateKeyFile
+directive for certificate" error if you're sporting the default Gentoo
+``httpd.conf``. You can fix this by commenting out two lines in
+``/etc/apache2/httpd.conf`` as follows:
 
 Change
 
@@ -257,4 +258,3 @@ whole process is described in the :doc:`contributing`.
    e.g. ``sudo python setup.py install``, ``sudo pip install``, ``sudo
    ./venv/bin/...``. These modes of operation might corrupt your operating
    system and are **not supported** by the Certbot team!
-