Merge pull request #4915 from Baeumla/master

Check keys if revoke certificate by private key
This commit is contained in:
yomna 2017-07-17 15:54:47 -07:00 committed by GitHub
commit 7d0e4d7bad
4 changed files with 23 additions and 15 deletions

View file

@ -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)

View file

@ -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])

View file

@ -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):

View file

@ -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)