diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index e22effeb7..112ef7c85 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -214,7 +214,7 @@ def verify_renewable_cert(renewable_cert): """ verify_renewable_cert_sig(renewable_cert) verify_fullchain(renewable_cert) - verify_cert_matches_priv_key(renewable_cert) + verify_cert_matches_priv_key(renewable_cert.cert, renewable_cert.privkey) def verify_renewable_cert_sig(renewable_cert): @@ -238,27 +238,24 @@ def verify_renewable_cert_sig(renewable_cert): raise errors.Error(error_str) -def verify_cert_matches_priv_key(renewable_cert): +def verify_cert_matches_priv_key(cert_path, key_path): """ Verifies that the private key and cert match. - :param `.storage.RenewableCert` renewable_cert: cert to verify + :param str cert_path: path to a cert in PEM format + :param str key_path: path to a private key file :raises errors.Error: If they don't match. """ try: - with open(renewable_cert.cert) as cert: - cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, cert.read()) - with open(renewable_cert.privkey) as privkey: - privkey = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, privkey.read()) context = OpenSSL.SSL.Context(OpenSSL.SSL.SSLv23_METHOD) - context.use_privatekey(privkey) - context.use_certificate(cert) + context.use_certificate_file(cert_path) + context.use_privatekey_file(key_path) context.check_privatekey() except (IOError, OpenSSL.SSL.Error) as e: error_str = "verifying the cert located at {0} matches the \ private key located at {1} has failed. \ - Details: {2}".format(renewable_cert.cert, - renewable_cert.privkey, e) + Details: {2}".format(cert_path, + key_path, e) logger.exception(error_str) raise errors.Error(error_str) diff --git a/certbot/main.py b/certbot/main.py index 6964e9352..a6e630540 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -567,6 +567,7 @@ def revoke(config, unused_plugins): # TODO: coop with renewal config if config.key_path is not None: # revocation by cert key logger.debug("Revoking %s using cert key %s", config.cert_path[0], config.key_path[0]) + crypto_util.verify_cert_matches_priv_key(config.cert_path[0], config.key_path[0]) key = jose.JWK.load(config.key_path[1]) else: # revocation by account key logger.debug("Revoking %s using Account Key", config.cert_path[0]) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index 729b09dc1..bc41ca950 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -252,9 +252,11 @@ class VerifyCertMatchesPrivKeyTest(VerifyCertSetup): def _call(self, renewable_cert): from certbot.crypto_util import verify_cert_matches_priv_key - return verify_cert_matches_priv_key(renewable_cert) + return verify_cert_matches_priv_key(renewable_cert.cert, renewable_cert.privkey) def test_cert_priv_key_match(self): + self.renewable_cert.cert = SS_CERT_PATH + self.renewable_cert.privkey = RSA2048_KEY_PATH self.assertEqual(None, self._call(self.renewable_cert)) def test_cert_priv_key_mismatch(self): diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 99e5ba0ee..48e19b7b3 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -37,6 +37,8 @@ CERT = test_util.vector_path('cert.pem') CSR = test_util.vector_path('csr.der') KEY = test_util.vector_path('rsa256_key.pem') JWK = jose.JWKRSA.load(test_util.load_vector("rsa512_key_2.pem")) +RSA2048_KEY_PATH = test_util.vector_path('rsa2048_key.pem') +SS_CERT_PATH = test_util.vector_path('self_signed_cert.pem') class TestHandleIdenticalCerts(unittest.TestCase): @@ -1015,18 +1017,24 @@ class MainTest(test_util.TempDirTestCase): # pylint: disable=too-many-public-me @mock.patch('certbot.main.client.acme_client') def test_revoke_with_key(self, mock_acme_client): server = 'foo.bar' - self._call_no_clientmock(['--cert-path', CERT, '--key-path', KEY, + self._call_no_clientmock(['--cert-path', SS_CERT_PATH, '--key-path', RSA2048_KEY_PATH, '--server', server, 'revoke']) - with open(KEY, 'rb') as f: + with open(RSA2048_KEY_PATH, 'rb') as f: mock_acme_client.Client.assert_called_once_with( server, key=jose.JWK.load(f.read()), net=mock.ANY) - with open(CERT, 'rb') as f: + with open(SS_CERT_PATH, 'rb') as f: cert = crypto_util.pyopenssl_load_certificate(f.read())[0] mock_revoke = mock_acme_client.Client().revoke mock_revoke.assert_called_once_with( jose.ComparableX509(cert), mock.ANY) + def test_revoke_with_key_mismatch(self): + server = 'foo.bar' + self.assertRaises(errors.Error, self._call_no_clientmock, + ['--cert-path', CERT, '--key-path', KEY, + '--server', server, 'revoke']) + @mock.patch('certbot.main._determine_account') def test_revoke_without_key(self, mock_determine_account): mock_determine_account.return_value = (mock.MagicMock(), None)