From fbab449694dad2dc5b546bf7bc867a79ea87a9a6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 8 May 2015 15:17:29 -0400 Subject: [PATCH 1/5] Finished basic POP challenge --- letsencrypt/client/continuity_auth.py | 2 +- letsencrypt/client/proof_of_possession.py | 88 +++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 letsencrypt/client/proof_of_possession.py diff --git a/letsencrypt/client/continuity_auth.py b/letsencrypt/client/continuity_auth.py index 063d3d408..2f01d901e 100644 --- a/letsencrypt/client/continuity_auth.py +++ b/letsencrypt/client/continuity_auth.py @@ -32,7 +32,7 @@ class ContinuityAuthenticator(object): def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use """Return list of challenge preferences.""" - return [challenges.RecoveryToken] + return [challenges.ProofOfPossession, challenges.RecoveryToken] def perform(self, achalls): """Perform client specific challenges for IAuthenticator""" diff --git a/letsencrypt/client/proof_of_possession.py b/letsencrypt/client/proof_of_possession.py new file mode 100644 index 000000000..2abed067d --- /dev/null +++ b/letsencrypt/client/proof_of_possession.py @@ -0,0 +1,88 @@ +"""Proof of Possession Identifier Validation Challenge. + +Based on draft-barnes-acme, section 6.5. + +""" +import M2Crypto +import os +import zope.component + +from letsencrypt.acme import challenges +from letsencrypt.acme import jose +from letsencrypt.acme import other +from letsencrypt.client import interfaces +from letsencrypt.client.display import util as display_util + + +class ProofOfPossession(object): + """Proof of Possession Identifier Validation Challenge. + + Based on draft-barnes-acme, section 6.5. + + """ + def __init__(self, certs_keys): + """Initializes the object with known certificates and keys. + + :param list certs_keys: tuples with form `[(cert, key, path)]`, where: + - `cert` - str path to certificate file + - `key` - str path to associated key file + - `path` - file path to configuration file + + """ + self.certs_keys = certs_keys + + def perform(self, achall): + """Perform the Proof of Possession Challenge. + + :param achall: Proof of Possession Challenge + :type achall: :class:`letsencrypt.client.achallenges.ProofOfPossession` + + :returns: Response or None/False if the challenge cannot be completed + :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse' + or False + + """ + if (not isinstance(achall.challb.hints.jwk, achall.challb.alg.kty) or + achall.challb.alg in [jose.HS256, jose.HS384, jose.HS512]): + return None + + for cert, prv_key, _ in self.certs_keys: + der_key = M2Crypto.X509.load_cert(cert).get_pubkey().as_der() + cert_key = challb.alg.kty.load(der_key) + if cert_key == challb.hints.jwk: + return _gen_response(achall, key) + + # Is there are different prompt we should give the user? + code, prv_key = zope.component.getUtility( + interfaces.IDsiplay).input( + "Path to private key for identifier: %s " % achall.domain) + if code != display_util.CANCEL: + return _gen_response(achall, prv_key) + + # If we get here, the key wasn't found + return False + + def _gen_response(self, challb, key_path): # pylint: disable=no-self-use + """Create the response to the Proof of Possession Challenge. + + :param challb: Proof of Possession Challenge + :type challb: :class:`letsencrypt.acme.challenges.ProofOfPossession` + + :param str key_path: Path to the private key corresponding to the + hinted to public key + + :returns: Response or None/False if the challenge cannot be completed + :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse' + or False + + """ + + if os.path.isfile(key_path): + with key as open(key_path, 'rb'): + try: + jwk = challb.alg.kty.load(key.read()) + except (IndexError, ValueError, TypeError): + return False + sig = other.Signature.from_msg(challb.nonce, jwk, alg=challb.alg) + return challenges.ProofOfPossessionResponse(nonce=challb.nonce, + signature=sig) From ae6b13cd5b45d09f053c0666a8a2737894eea170 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 8 May 2015 23:14:04 -0400 Subject: [PATCH 2/5] Finished basic POP challenge with tests --- letsencrypt/client/client.py | 3 +- letsencrypt/client/continuity_auth.py | 19 ++++- letsencrypt/client/proof_of_possession.py | 52 ++++++------- .../client/tests/continuity_auth_test.py | 27 ++++++- .../client/tests/proof_of_possession_test.py | 78 +++++++++++++++++++ .../client/tests/testdata/matching_cert.pem | 14 ++++ 6 files changed, 156 insertions(+), 37 deletions(-) create mode 100644 letsencrypt/client/tests/proof_of_possession_test.py create mode 100644 letsencrypt/client/tests/testdata/matching_cert.pem diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 8518c56b9..6a54de26d 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -68,7 +68,8 @@ class Client(object): self.config = config if dv_auth is not None: - cont_auth = continuity_auth.ContinuityAuthenticator(config) + cont_auth = continuity_auth.ContinuityAuthenticator(config, + installer) self.auth_handler = auth_handler.AuthHandler( dv_auth, cont_auth, self.network, self.account) else: diff --git a/letsencrypt/client/continuity_auth.py b/letsencrypt/client/continuity_auth.py index 2f01d901e..c6926d952 100644 --- a/letsencrypt/client/continuity_auth.py +++ b/letsencrypt/client/continuity_auth.py @@ -6,6 +6,7 @@ from letsencrypt.acme import challenges from letsencrypt.client import achallenges from letsencrypt.client import errors from letsencrypt.client import interfaces +from letsencrypt.client import proof_of_possession from letsencrypt.client import recovery_token @@ -13,22 +14,30 @@ class ContinuityAuthenticator(object): """IAuthenticator for :const:`~letsencrypt.acme.challenges.ContinuityChallenge` class challenges. - :ivar rec_token: Performs "recoveryToken" challenges + :ivar rec_token: Performs "recoveryToken" challenges. :type rec_token: :class:`letsencrypt.client.recovery_token.RecoveryToken` + :ivar proof_of_pos: Performs "proofOfPossession" challenges. + :type proof_of_pos: + :class:`letsencrypt.client.proof_of_possession.Proof_of_Possession` + """ zope.interface.implements(interfaces.IAuthenticator) # This will have an installer soon for get_key/cert purposes - def __init__(self, config): + def __init__(self, config, installer): """Initialize Client Authenticator. :param config: Configuration. :type config: :class:`letsencrypt.client.interfaces.IConfig` + :param installer: Let's Encrypt Installer. + :type installer: :class:`letsencrypt.client.interfaces.IInstaller` + """ self.rec_token = recovery_token.RecoveryToken( config.server, config.rec_token_dir) + self.proof_of_pos = proof_of_possession.ProofOfPossession(installer) def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use """Return list of challenge preferences.""" @@ -38,7 +47,9 @@ class ContinuityAuthenticator(object): """Perform client specific challenges for IAuthenticator""" responses = [] for achall in achalls: - if isinstance(achall, achallenges.RecoveryToken): + if isinstance(achall, achallenges.ProofOfPossession): + responses.append(self.proof_of_pos.perform(achall)) + elif isinstance(achall, achallenges.RecoveryToken): responses.append(self.rec_token.perform(achall)) else: raise errors.LetsEncryptContAuthError("Unexpected Challenge") @@ -49,5 +60,5 @@ class ContinuityAuthenticator(object): for achall in achalls: if isinstance(achall, achallenges.RecoveryToken): self.rec_token.cleanup(achall) - else: + elif not isinstance(achall, achallenges.ProofOfPossession): raise errors.LetsEncryptContAuthError("Unexpected Challenge") diff --git a/letsencrypt/client/proof_of_possession.py b/letsencrypt/client/proof_of_possession.py index 2abed067d..82e3524b4 100644 --- a/letsencrypt/client/proof_of_possession.py +++ b/letsencrypt/client/proof_of_possession.py @@ -1,8 +1,4 @@ -"""Proof of Possession Identifier Validation Challenge. - -Based on draft-barnes-acme, section 6.5. - -""" +"""Proof of Possession Identifier Validation Challenge.""" import M2Crypto import os import zope.component @@ -14,22 +10,17 @@ from letsencrypt.client import interfaces from letsencrypt.client.display import util as display_util -class ProofOfPossession(object): +class ProofOfPossession(object): # pylint: disable=too-few-public-methods """Proof of Possession Identifier Validation Challenge. Based on draft-barnes-acme, section 6.5. + :ivar installer: Installer object + :type installer: :class:`~letsencrypt.client.interfaces.IInstaller` + """ - def __init__(self, certs_keys): - """Initializes the object with known certificates and keys. - - :param list certs_keys: tuples with form `[(cert, key, path)]`, where: - - `cert` - str path to certificate file - - `key` - str path to associated key file - - `path` - file path to configuration file - - """ - self.certs_keys = certs_keys + def __init__(self, installer): + self.installer = installer def perform(self, achall): """Perform the Proof of Possession Challenge. @@ -46,18 +37,19 @@ class ProofOfPossession(object): achall.challb.alg in [jose.HS256, jose.HS384, jose.HS512]): return None - for cert, prv_key, _ in self.certs_keys: - der_key = M2Crypto.X509.load_cert(cert).get_pubkey().as_der() - cert_key = challb.alg.kty.load(der_key) - if cert_key == challb.hints.jwk: - return _gen_response(achall, key) + # This will work regardless of how JWKES is implemented + for cert, key, _ in self.installer.get_all_certs_keys(): + der_cert_key = M2Crypto.X509.load_cert(cert).get_pubkey().as_der() + cert_key = achall.challb.alg.kty.load(der_cert_key) + if cert_key == achall.challb.hints.jwk: + return self._gen_response(achall, key) # Is there are different prompt we should give the user? - code, prv_key = zope.component.getUtility( - interfaces.IDsiplay).input( + code, key = zope.component.getUtility( + interfaces.IDisplay).input( "Path to private key for identifier: %s " % achall.domain) if code != display_util.CANCEL: - return _gen_response(achall, prv_key) + return self._gen_response(achall, key) # If we get here, the key wasn't found return False @@ -68,8 +60,8 @@ class ProofOfPossession(object): :param challb: Proof of Possession Challenge :type challb: :class:`letsencrypt.acme.challenges.ProofOfPossession` - :param str key_path: Path to the private key corresponding to the - hinted to public key + :param str key_path: Path to the key corresponding to the hinted to + public key. :returns: Response or None/False if the challenge cannot be completed :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse' @@ -78,11 +70,13 @@ class ProofOfPossession(object): """ if os.path.isfile(key_path): - with key as open(key_path, 'rb'): + with open(key_path, 'rb') as key: try: jwk = challb.alg.kty.load(key.read()) except (IndexError, ValueError, TypeError): - return False - sig = other.Signature.from_msg(challb.nonce, jwk, alg=challb.alg) + return False + # If JWKES doesn't have a key attribute, this needs to be modified + sig = other.Signature.from_msg(challb.nonce, jwk.key, + alg=challb.alg) return challenges.ProofOfPossessionResponse(nonce=challb.nonce, signature=sig) diff --git a/letsencrypt/client/tests/continuity_auth_test.py b/letsencrypt/client/tests/continuity_auth_test.py index 7a2279bcd..567a4e93e 100644 --- a/letsencrypt/client/tests/continuity_auth_test.py +++ b/letsencrypt/client/tests/continuity_auth_test.py @@ -16,9 +16,11 @@ class PerformTest(unittest.TestCase): from letsencrypt.client.continuity_auth import ContinuityAuthenticator self.auth = ContinuityAuthenticator( - mock.MagicMock(server="demo_server.org")) + mock.MagicMock(server="demo_server.org"), None) self.auth.rec_token.perform = mock.MagicMock( name="rec_token_perform", side_effect=gen_client_resp) + self.auth.proof_of_pos.perform = mock.MagicMock( + name="proof_of_pos_perform", side_effect=gen_client_resp) def test_rec_token1(self): token = achallenges.RecoveryToken(challb=None, domain="0") @@ -36,6 +38,24 @@ class PerformTest(unittest.TestCase): for i in xrange(5): self.assertEqual(responses[i], "RecoveryToken%d" % i) + def test_pop_and_rec_token(self): + achalls = [] + for i in xrange(4): + if i % 2 == 0: + achalls.append(achallenges.RecoveryToken(challb=None, + domain=str(i))) + else: + achalls.append(achallenges.ProofOfPossession(challb=None, + domain=str(i))) + responses = self.auth.perform(achalls) + + self.assertEqual(len(responses), 4) + for i in xrange(4): + if i % 2 == 0: + self.assertEqual(responses[i], "RecoveryToken%d" % i) + else: + self.assertEqual(responses[i], "ProofOfPossession%d" % i) + def test_unexpected(self): self.assertRaises( errors.LetsEncryptContAuthError, self.auth.perform, [ @@ -43,7 +63,8 @@ class PerformTest(unittest.TestCase): def test_chall_pref(self): self.assertEqual( - self.auth.get_chall_pref("example.com"), [challenges.RecoveryToken]) + self.auth.get_chall_pref("example.com"), + [challenges.ProofOfPossession, challenges.RecoveryToken]) class CleanupTest(unittest.TestCase): @@ -53,7 +74,7 @@ class CleanupTest(unittest.TestCase): from letsencrypt.client.continuity_auth import ContinuityAuthenticator self.auth = ContinuityAuthenticator( - mock.MagicMock(server="demo_server.org")) + mock.MagicMock(server="demo_server.org"), None) self.mock_cleanup = mock.MagicMock(name="rec_token_cleanup") self.auth.rec_token.cleanup = self.mock_cleanup diff --git a/letsencrypt/client/tests/proof_of_possession_test.py b/letsencrypt/client/tests/proof_of_possession_test.py new file mode 100644 index 000000000..0ba801ff1 --- /dev/null +++ b/letsencrypt/client/tests/proof_of_possession_test.py @@ -0,0 +1,78 @@ +"""Tests for proof_of_possession.py""" +import Crypto.PublicKey.RSA +import os +import pkg_resources +import unittest + +import mock + +from letsencrypt.acme import challenges +from letsencrypt.acme import jose +from letsencrypt.client import achallenges +from letsencrypt.client import proof_of_possession +from letsencrypt.client.display import util as display_util + + +BASE_PACKAGE = "letsencrypt.client.tests" +CERT0_PATH = pkg_resources.resource_filename( + BASE_PACKAGE, os.path.join("testdata", "cert.pem")) +CERT1_PATH = pkg_resources.resource_filename( + BASE_PACKAGE, os.path.join("testdata", "cert-san.pem")) +CERT2_PATH = pkg_resources.resource_filename( + BASE_PACKAGE, os.path.join("testdata", "matching_cert.pem")) +KEY_PATH = pkg_resources.resource_filename( + BASE_PACKAGE, os.path.join("testdata", "rsa512_key.pem")) +KEY = Crypto.PublicKey.RSA.importKey(pkg_resources.resource_string( + BASE_PACKAGE, os.path.join('testdata', 'rsa512_key.pem'))).publickey() + + +class ProofOfPossessionTest(unittest.TestCase): + def setUp(self): + self.installer = mock.MagicMock() + self.installer.get_all_certs_keys.return_value = zip( + [CERT0_PATH, CERT1_PATH, CERT2_PATH], 3 * [KEY_PATH], 3 * [None]) + self.proof_of_pos = proof_of_possession.ProofOfPossession( + self.installer) + + hints = challenges.ProofOfPossession.Hints( + jwk=jose.JWKRSA(key=KEY), cert_fingerprints=(), + certs=(), serial_numbers=(), subject_key_identifiers=(), + issuers=(), authorized_for=()) + challenge = challenges.ProofOfPossession( + alg=jose.RS256, nonce='zczv4HMLVe_0kimJ25Juig', hints=hints) + self.achall = achallenges.ProofOfPossession( + challb=challenge, domain="example.com") + + def test_perform_no_input(self): + response = self.proof_of_pos.perform(self.achall) + self.assertTrue(response.verify()) + + @mock.patch("letsencrypt.client.recovery_token.zope.component.getUtility") + def test_perform_with_input(self, mock_input): + # Remove the matching certificate + self.installer.get_all_certs_keys.return_value.pop() + mock_input().input.side_effect = [(display_util.CANCEL, ""), + (display_util.OK, CERT0_PATH), + (display_util.OK, KEY_PATH)] + + response = self.proof_of_pos.perform(self.achall) + self.assertFalse(response) + + response = self.proof_of_pos.perform(self.achall) + self.assertFalse(response) + + response = self.proof_of_pos.perform(self.achall) + self.assertTrue(response.verify()) + + def test_perform_bad_challenge(self): + hints = challenges.ProofOfPossession.Hints( + jwk=jose.jwk.JWKOct(key=KEY), cert_fingerprints=(), + certs=(), serial_numbers=(), subject_key_identifiers=(), + issuers=(), authorized_for=()) + challenge = challenges.ProofOfPossession( + alg=jose.HS512, nonce='zczv4HMLVe_0kimJ25Juig', hints=hints) + self.achall = achallenges.ProofOfPossession( + challb=challenge, domain="example.com") + + response = self.proof_of_pos.perform(self.achall) + self.assertEqual(response, None) diff --git a/letsencrypt/client/tests/testdata/matching_cert.pem b/letsencrypt/client/tests/testdata/matching_cert.pem new file mode 100644 index 000000000..fda9cb1f4 --- /dev/null +++ b/letsencrypt/client/tests/testdata/matching_cert.pem @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICNzCCAeGgAwIBAgIJALizm9Y3q620MA0GCSqGSIb3DQEBCwUAMHcxCzAJBgNV +BAYTAlVTMREwDwYDVQQIDAhNaWNoaWdhbjESMBAGA1UEBwwJQW5uIEFyYm9yMSsw +KQYDVQQKDCJVbml2ZXJzaXR5IG9mIE1pY2hpZ2FuIGFuZCB0aGUgRUZGMRQwEgYD +VQQDDAtleGFtcGxlLmNvbTAeFw0xNTA1MDkwMDI0NTJaFw0xNjA1MDgwMDI0NTJa +MHcxCzAJBgNVBAYTAlVTMREwDwYDVQQIDAhNaWNoaWdhbjESMBAGA1UEBwwJQW5u +IEFyYm9yMSswKQYDVQQKDCJVbml2ZXJzaXR5IG9mIE1pY2hpZ2FuIGFuZCB0aGUg +RUZGMRQwEgYDVQQDDAtleGFtcGxlLmNvbTBcMA0GCSqGSIb3DQEBAQUAA0sAMEgC +QQD0thFxUTc2v6qV55wRxfwnBUOeN4bVfu5ywJqy65kzR7T1yZi5TPEiQyM7/3Hg +BVy9ddFc8RX4vNZaR+ROXNEzAgMBAAGjUDBOMB0GA1UdDgQWBBRJieHEVSHKmBk0 +mTExx1erzlylCjAfBgNVHSMEGDAWgBRJieHEVSHKmBk0mTExx1erzlylCjAMBgNV +HRMEBTADAQH/MA0GCSqGSIb3DQEBCwUAA0EABT/nlpqOaanFSLZmWIrKv0zt63k4 +bmWNMA8fYT45KYpLomsW8qXdpC82IlVKfNk7fW0UYT3HOeDSJRcycxNCTQ== +-----END CERTIFICATE----- From e3d95c5a6887894be2a82784c7436f17205a8023 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 8 May 2015 23:43:06 -0400 Subject: [PATCH 3/5] Final changes --- docs/api/client/proof_of_possession.rst | 5 ++++ letsencrypt/client/proof_of_possession.py | 4 +-- .../client/tests/proof_of_possession_test.py | 26 +++++++++++-------- 3 files changed, 22 insertions(+), 13 deletions(-) create mode 100644 docs/api/client/proof_of_possession.rst diff --git a/docs/api/client/proof_of_possession.rst b/docs/api/client/proof_of_possession.rst new file mode 100644 index 000000000..9f1ea0793 --- /dev/null +++ b/docs/api/client/proof_of_possession.rst @@ -0,0 +1,5 @@ +:mod:`letsencrypt.client.proof_of_possession` +-------------------------------------------------- + +.. automodule:: letsencrypt.client.proof_of_possession + :members: diff --git a/letsencrypt/client/proof_of_possession.py b/letsencrypt/client/proof_of_possession.py index 82e3524b4..a2f4956b9 100644 --- a/letsencrypt/client/proof_of_possession.py +++ b/letsencrypt/client/proof_of_possession.py @@ -29,7 +29,7 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods :type achall: :class:`letsencrypt.client.achallenges.ProofOfPossession` :returns: Response or None/False if the challenge cannot be completed - :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse' + :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse` or False """ @@ -64,7 +64,7 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods public key. :returns: Response or None/False if the challenge cannot be completed - :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse' + :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse` or False """ diff --git a/letsencrypt/client/tests/proof_of_possession_test.py b/letsencrypt/client/tests/proof_of_possession_test.py index 0ba801ff1..f068bc1b2 100644 --- a/letsencrypt/client/tests/proof_of_possession_test.py +++ b/letsencrypt/client/tests/proof_of_possession_test.py @@ -43,6 +43,19 @@ class ProofOfPossessionTest(unittest.TestCase): self.achall = achallenges.ProofOfPossession( challb=challenge, domain="example.com") + def test_perform_bad_challenge(self): + hints = challenges.ProofOfPossession.Hints( + jwk=jose.jwk.JWKOct(key=KEY), cert_fingerprints=(), + certs=(), serial_numbers=(), subject_key_identifiers=(), + issuers=(), authorized_for=()) + challenge = challenges.ProofOfPossession( + alg=jose.HS512, nonce='zczv4HMLVe_0kimJ25Juig', hints=hints) + self.achall = achallenges.ProofOfPossession( + challb=challenge, domain="example.com") + + response = self.proof_of_pos.perform(self.achall) + self.assertEqual(response, None) + def test_perform_no_input(self): response = self.proof_of_pos.perform(self.achall) self.assertTrue(response.verify()) @@ -64,15 +77,6 @@ class ProofOfPossessionTest(unittest.TestCase): response = self.proof_of_pos.perform(self.achall) self.assertTrue(response.verify()) - def test_perform_bad_challenge(self): - hints = challenges.ProofOfPossession.Hints( - jwk=jose.jwk.JWKOct(key=KEY), cert_fingerprints=(), - certs=(), serial_numbers=(), subject_key_identifiers=(), - issuers=(), authorized_for=()) - challenge = challenges.ProofOfPossession( - alg=jose.HS512, nonce='zczv4HMLVe_0kimJ25Juig', hints=hints) - self.achall = achallenges.ProofOfPossession( - challb=challenge, domain="example.com") - response = self.proof_of_pos.perform(self.achall) - self.assertEqual(response, None) +if __name__ == "__main__": + unittest.main() From a496179f7436d02fb8ff73da59f18b89e46319ea Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 12 May 2015 11:56:26 -0400 Subject: [PATCH 4/5] Incorporated feedback and added extra error checking to POP --- letsencrypt/client/proof_of_possession.py | 35 +++++++------ .../client/tests/proof_of_possession_test.py | 52 ++++++++++--------- .../client/tests/testdata/dsa512_key.pem | 14 +++++ .../client/tests/testdata/dsa_cert.pem | 17 ++++++ 4 files changed, 78 insertions(+), 40 deletions(-) create mode 100644 letsencrypt/client/tests/testdata/dsa512_key.pem create mode 100644 letsencrypt/client/tests/testdata/dsa_cert.pem diff --git a/letsencrypt/client/proof_of_possession.py b/letsencrypt/client/proof_of_possession.py index a2f4956b9..8f11f56c8 100644 --- a/letsencrypt/client/proof_of_possession.py +++ b/letsencrypt/client/proof_of_possession.py @@ -33,15 +33,18 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods or False """ - if (not isinstance(achall.challb.hints.jwk, achall.challb.alg.kty) or - achall.challb.alg in [jose.HS256, jose.HS384, jose.HS512]): + if (achall.alg in [jose.HS256, jose.HS384, jose.HS512] or + not isinstance(achall.hints.jwk, achall.alg.kty)): return None - # This will work regardless of how JWKES is implemented for cert, key, _ in self.installer.get_all_certs_keys(): der_cert_key = M2Crypto.X509.load_cert(cert).get_pubkey().as_der() - cert_key = achall.challb.alg.kty.load(der_cert_key) - if cert_key == achall.challb.hints.jwk: + try: + cert_key = achall.alg.kty.load(der_cert_key) + # If JWKES.load raises other exceptions, they should be caught here + except (IndexError, ValueError, TypeError): + continue + if cert_key == achall.hints.jwk: return self._gen_response(achall, key) # Is there are different prompt we should give the user? @@ -54,29 +57,29 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods # If we get here, the key wasn't found return False - def _gen_response(self, challb, key_path): # pylint: disable=no-self-use + def _gen_response(self, achall, key_path): # pylint: disable=no-self-use """Create the response to the Proof of Possession Challenge. - :param challb: Proof of Possession Challenge - :type challb: :class:`letsencrypt.acme.challenges.ProofOfPossession` + :param achall: Proof of Possession Challenge + :type achall: :class:`letsencrypt.client.achallenges.ProofOfPossession` :param str key_path: Path to the key corresponding to the hinted to public key. - :returns: Response or None/False if the challenge cannot be completed + :returns: Response or False if the challenge cannot be completed :rtype: :class:`letsencrypt.acme.challenges.ProofOfPossessionResponse` or False """ - if os.path.isfile(key_path): with open(key_path, 'rb') as key: try: - jwk = challb.alg.kty.load(key.read()) - except (IndexError, ValueError, TypeError): + # Needs to be changed if JWKES doesn't have a key attribute + jwk = achall.alg.kty.load(key.read()) + sig = other.Signature.from_msg(achall.nonce, jwk.key, + alg=achall.alg) + except (IndexError, ValueError, TypeError, jose.errors.Error): return False - # If JWKES doesn't have a key attribute, this needs to be modified - sig = other.Signature.from_msg(challb.nonce, jwk.key, - alg=challb.alg) - return challenges.ProofOfPossessionResponse(nonce=challb.nonce, + return challenges.ProofOfPossessionResponse(nonce=achall.nonce, signature=sig) + return False diff --git a/letsencrypt/client/tests/proof_of_possession_test.py b/letsencrypt/client/tests/proof_of_possession_test.py index f068bc1b2..80e492887 100644 --- a/letsencrypt/client/tests/proof_of_possession_test.py +++ b/letsencrypt/client/tests/proof_of_possession_test.py @@ -8,6 +8,7 @@ import mock from letsencrypt.acme import challenges from letsencrypt.acme import jose +from letsencrypt.acme import messages2 from letsencrypt.client import achallenges from letsencrypt.client import proof_of_possession from letsencrypt.client.display import util as display_util @@ -19,46 +20,53 @@ CERT0_PATH = pkg_resources.resource_filename( CERT1_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "cert-san.pem")) CERT2_PATH = pkg_resources.resource_filename( + BASE_PACKAGE, os.path.join("testdata", "dsa_cert.pem")) +CERT2_KEY_PATH = pkg_resources.resource_filename( + BASE_PACKAGE, os.path.join("testdata", "dsa512_key.pem")) +CERT3_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "matching_cert.pem")) -KEY_PATH = pkg_resources.resource_filename( +CERT3_KEY_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "rsa512_key.pem")) -KEY = Crypto.PublicKey.RSA.importKey(pkg_resources.resource_string( +CERT3_KEY = Crypto.PublicKey.RSA.importKey(pkg_resources.resource_string( BASE_PACKAGE, os.path.join('testdata', 'rsa512_key.pem'))).publickey() class ProofOfPossessionTest(unittest.TestCase): def setUp(self): self.installer = mock.MagicMock() + certs = [CERT0_PATH, CERT1_PATH, CERT2_PATH, CERT3_PATH] + keys = [None, None, CERT2_KEY_PATH, CERT3_KEY_PATH] self.installer.get_all_certs_keys.return_value = zip( - [CERT0_PATH, CERT1_PATH, CERT2_PATH], 3 * [KEY_PATH], 3 * [None]) + certs, keys, 4 * [None]) self.proof_of_pos = proof_of_possession.ProofOfPossession( self.installer) hints = challenges.ProofOfPossession.Hints( - jwk=jose.JWKRSA(key=KEY), cert_fingerprints=(), + jwk=jose.JWKRSA(key=CERT3_KEY), cert_fingerprints=(), certs=(), serial_numbers=(), subject_key_identifiers=(), issuers=(), authorized_for=()) - challenge = challenges.ProofOfPossession( + chall = challenges.ProofOfPossession( alg=jose.RS256, nonce='zczv4HMLVe_0kimJ25Juig', hints=hints) + challb = messages2.ChallengeBody( + chall=chall, uri="http://example", status=messages2.STATUS_PENDING) self.achall = achallenges.ProofOfPossession( - challb=challenge, domain="example.com") + challb=challb, domain="example.com") def test_perform_bad_challenge(self): hints = challenges.ProofOfPossession.Hints( - jwk=jose.jwk.JWKOct(key=KEY), cert_fingerprints=(), + jwk=jose.jwk.JWKOct(key=CERT3_KEY), cert_fingerprints=(), certs=(), serial_numbers=(), subject_key_identifiers=(), issuers=(), authorized_for=()) - challenge = challenges.ProofOfPossession( + chall = challenges.ProofOfPossession( alg=jose.HS512, nonce='zczv4HMLVe_0kimJ25Juig', hints=hints) + challb = messages2.ChallengeBody( + chall=chall, uri="http://example", status=messages2.STATUS_PENDING) self.achall = achallenges.ProofOfPossession( - challb=challenge, domain="example.com") - - response = self.proof_of_pos.perform(self.achall) - self.assertEqual(response, None) + challb=challb, domain="example.com") + self.assertEqual(self.proof_of_pos.perform(self.achall), None) def test_perform_no_input(self): - response = self.proof_of_pos.perform(self.achall) - self.assertTrue(response.verify()) + self.assertTrue(self.proof_of_pos.perform(self.achall).verify()) @mock.patch("letsencrypt.client.recovery_token.zope.component.getUtility") def test_perform_with_input(self, mock_input): @@ -66,16 +74,12 @@ class ProofOfPossessionTest(unittest.TestCase): self.installer.get_all_certs_keys.return_value.pop() mock_input().input.side_effect = [(display_util.CANCEL, ""), (display_util.OK, CERT0_PATH), - (display_util.OK, KEY_PATH)] - - response = self.proof_of_pos.perform(self.achall) - self.assertFalse(response) - - response = self.proof_of_pos.perform(self.achall) - self.assertFalse(response) - - response = self.proof_of_pos.perform(self.achall) - self.assertTrue(response.verify()) + (display_util.OK, "imaginary_file"), + (display_util.OK, CERT3_KEY_PATH)] + self.assertFalse(self.proof_of_pos.perform(self.achall)) + self.assertFalse(self.proof_of_pos.perform(self.achall)) + self.assertFalse(self.proof_of_pos.perform(self.achall)) + self.assertTrue(self.proof_of_pos.perform(self.achall).verify()) if __name__ == "__main__": diff --git a/letsencrypt/client/tests/testdata/dsa512_key.pem b/letsencrypt/client/tests/testdata/dsa512_key.pem new file mode 100644 index 000000000..78e164712 --- /dev/null +++ b/letsencrypt/client/tests/testdata/dsa512_key.pem @@ -0,0 +1,14 @@ +-----BEGIN DSA PARAMETERS----- +MIGdAkEAwebEoGBfokKQeALHHnAZMQwYU35ILEBdV8oUmzv7qpSVUoHihyqfn6GC +OixAKSP8EJYcTilIqPbFbfFyOPlbLwIVANoFHEDiQgknAvKrG78pHzAJdQSPAkEA +qfka5Bnl+CeEMpzVZGrOVqZE/LFdZK9eT6YtWjzqtIkf3hwXUVxJsTnBG4xmrfvl +41pgNJpgu99YOYqPpS0g7A== +-----END DSA PARAMETERS----- +-----BEGIN DSA PRIVATE KEY----- +MIH5AgEAAkEAwebEoGBfokKQeALHHnAZMQwYU35ILEBdV8oUmzv7qpSVUoHihyqf +n6GCOixAKSP8EJYcTilIqPbFbfFyOPlbLwIVANoFHEDiQgknAvKrG78pHzAJdQSP +AkEAqfka5Bnl+CeEMpzVZGrOVqZE/LFdZK9eT6YtWjzqtIkf3hwXUVxJsTnBG4xm +rfvl41pgNJpgu99YOYqPpS0g7AJATQ2LUzjGQSM6UljcPY5I2OD9THkUR9kH2tth +zZd70UoI9btrVaTizgqYShuok94glSQNK0H92JgUk3scJPaAkAIVAMDn61h6vrCE +mNv063So6E+eYaIN +-----END DSA PRIVATE KEY----- diff --git a/letsencrypt/client/tests/testdata/dsa_cert.pem b/letsencrypt/client/tests/testdata/dsa_cert.pem new file mode 100644 index 000000000..ef94536e7 --- /dev/null +++ b/letsencrypt/client/tests/testdata/dsa_cert.pem @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICuDCCAnWgAwIBAgIJAPjmErVMzwVLMAsGCWCGSAFlAwQDAjB3MQswCQYDVQQG +EwJVUzERMA8GA1UECAwITWljaGlnYW4xEjAQBgNVBAcMCUFubiBBcmJvcjErMCkG +A1UECgwiVW5pdmVyc2l0eSBvZiBNaWNoaWdhbiBhbmQgdGhlIEVGRjEUMBIGA1UE +AwwLZXhhbXBsZS5jb20wHhcNMTUwNTEyMTUzOTQzWhcNMTUwNjExMTUzOTQzWjB3 +MQswCQYDVQQGEwJVUzERMA8GA1UECAwITWljaGlnYW4xEjAQBgNVBAcMCUFubiBB +cmJvcjErMCkGA1UECgwiVW5pdmVyc2l0eSBvZiBNaWNoaWdhbiBhbmQgdGhlIEVG +RjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wgfEwgakGByqGSM44BAEwgZ0CQQDB5sSg +YF+iQpB4AscecBkxDBhTfkgsQF1XyhSbO/uqlJVSgeKHKp+foYI6LEApI/wQlhxO +KUio9sVt8XI4+VsvAhUA2gUcQOJCCScC8qsbvykfMAl1BI8CQQCp+RrkGeX4J4Qy +nNVkas5WpkT8sV1kr15Ppi1aPOq0iR/eHBdRXEmxOcEbjGat++XjWmA0mmC731g5 +io+lLSDsA0MAAkBNDYtTOMZBIzpSWNw9jkjY4P1MeRRH2Qfa22HNl3vRSgj1u2tV +pOLOCphKG6iT3iCVJA0rQf3YmBSTexwk9oCQo1AwTjAdBgNVHQ4EFgQUZ2DlTDGU +PMwTUt0KztM6IyX61BcwHwYDVR0jBBgwFoAUZ2DlTDGUPMwTUt0KztM6IyX61Bcw +DAYDVR0TBAUwAwEB/zALBglghkgBZQMEAwIDMAAwLQIVAIbMgGx+KwBr4rgqZ2Lh +AAO8TegHAhQsuxpIIIphiReoWEtEJk4TqEIz/A== +-----END CERTIFICATE----- From 27511d48225ef298383ea363472676685ec94270 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 12 May 2015 12:42:59 -0400 Subject: [PATCH 5/5] Added no cover line to unittest.main() --- letsencrypt/client/tests/proof_of_possession_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/client/tests/proof_of_possession_test.py b/letsencrypt/client/tests/proof_of_possession_test.py index 80e492887..08f3f3797 100644 --- a/letsencrypt/client/tests/proof_of_possession_test.py +++ b/letsencrypt/client/tests/proof_of_possession_test.py @@ -83,4 +83,4 @@ class ProofOfPossessionTest(unittest.TestCase): if __name__ == "__main__": - unittest.main() + unittest.main() # pragma: no cover