From 03a5750d90c1517f0bfde601ac1c986a92d27906 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Mon, 26 Jan 2015 22:25:08 -0800 Subject: [PATCH 1/2] modify gen_dvsni_cert api --- letsencrypt/client/apache/dvsni.py | 21 +++++-- letsencrypt/client/challenge_util.py | 15 ++--- letsencrypt/client/tests/apache/dvsni_test.py | 63 +++++++++++++------ .../client/tests/challenge_util_test.py | 44 +++++-------- 4 files changed, 80 insertions(+), 63 deletions(-) diff --git a/letsencrypt/client/apache/dvsni.py b/letsencrypt/client/apache/dvsni.py index cf5c3bdb0..d514dbbdb 100644 --- a/letsencrypt/client/apache/dvsni.py +++ b/letsencrypt/client/apache/dvsni.py @@ -87,11 +87,7 @@ class ApacheDvsni(object): # Create all of the challenge certs for chall in self.dvsni_chall: - cert_path = self.get_cert_file(chall.nonce) - self.config.reverter.register_file_creation(True, cert_path) - s_b64 = challenge_util.dvsni_gen_cert( - cert_path, chall.domain, chall.r_b64, chall.nonce, chall.key) - + s_b64 = self._setup_challenge_cert(chall) responses.append({"type": "dvsni", "s": s_b64}) # Setup the configuration @@ -102,6 +98,21 @@ class ApacheDvsni(object): return responses + def _setup_challenge_cert(self, chall): + """Generate and write out challenge certificate.""" + cert_path = self.get_cert_file(chall.nonce) + # Register the path before you write out the file + self.config.reverter.register_file_creation(True, cert_path) + + cert_pem, s_b64 = challenge_util.dvsni_gen_cert( + chall.domain, chall.r_b64, chall.nonce, chall.key) + + # Write out challenge cert + with open(cert_path, 'w') as cert_chall_fd: + cert_chall_fd.write(cert_pem) + + return s_b64 + def _mod_config(self, ll_addrs): """Modifies Apache config files to include challenge vhosts. diff --git a/letsencrypt/client/challenge_util.py b/letsencrypt/client/challenge_util.py index b5198217d..b5d1cf38d 100644 --- a/letsencrypt/client/challenge_util.py +++ b/letsencrypt/client/challenge_util.py @@ -27,11 +27,9 @@ IndexedChall = collections.namedtuple("IndexedChall", "chall, index") # DVSNI Challenge functions -def dvsni_gen_cert(filepath, name, r_b64, nonce, key): +def dvsni_gen_cert(name, r_b64, nonce, key): """Generate a DVSNI cert and save it to filepath. - :param str filepath: destination to save certificate. This will overwrite - any file that is currently at the location. :param str name: domain to validate :param str r_b64: jose base64 encoded dvsni r value :param str nonce: hex value of nonce @@ -39,8 +37,10 @@ def dvsni_gen_cert(filepath, name, r_b64, nonce, key): :param key: Key to perform challenge :type key: :class:`letsencrypt.client.client.Client.Key` - :returns: dvsni s value jose base64 encoded - :rtype: str + :returns: tuple of (cert_pem, s) where + cert_pem is the certificate in pem form + s is the dvsni s value, jose base64 encoded + :rtype: tuple """ # Generate S @@ -53,10 +53,7 @@ def dvsni_gen_cert(filepath, name, r_b64, nonce, key): cert_pem = crypto_util.make_ss_cert( key.pem, [nonce + CONFIG.INVALID_EXT, name, ext]) - with open(filepath, 'w') as chall_cert_file: - chall_cert_file.write(cert_pem) - - return le_util.jose_b64encode(dvsni_s) + return cert_pem, le_util.jose_b64encode(dvsni_s) def _dvsni_gen_ext(dvsni_r, dvsni_s): diff --git a/letsencrypt/client/tests/apache/dvsni_test.py b/letsencrypt/client/tests/apache/dvsni_test.py index 68ffa283b..53e7f81e3 100644 --- a/letsencrypt/client/tests/apache/dvsni_test.py +++ b/letsencrypt/client/tests/apache/dvsni_test.py @@ -56,23 +56,49 @@ class DvsniPerformTest(util.ApacheTest): self.assertTrue(resp is None) @mock.patch("letsencrypt.client.challenge_util.dvsni_gen_cert") - def test_perform1(self, mock_dvsni_gen_cert): + def test_setup_challenge_cert(self, mock_dvsni_gen_cert): + # This is a helper function that can be used for handling + # open context managers more elegantly. It avoids dealing with + # __enter__ and __exit__ calls. + # http://www.voidspace.org.uk/python/mock/helpers.html#mock.mock_open chall = self.challs[0] - self.sni.add_chall(chall) - mock_dvsni_gen_cert.return_value = "randomS1" - responses = self.sni.perform() + m_open = mock.mock_open() + mock_dvsni_gen_cert.return_value = ("pem", "randomS1") + + with mock.patch("letsencrypt.client.apache.dvsni.open", + m_open, create=True): + # pylint: disable=protected-access + s_b64 = self.sni._setup_challenge_cert(chall) + + self.assertEqual(s_b64, "randomS1") + + self.assertTrue(m_open.called) + self.assertEqual( + m_open.call_args[0], (self.sni.get_cert_file(chall.nonce), 'w')) + self.assertEqual(m_open().write.call_args[0][0], "pem") self.assertEqual(mock_dvsni_gen_cert.call_count, 1) calls = mock_dvsni_gen_cert.call_args_list expected_call_list = [ - (self.sni.get_cert_file(chall.nonce), chall.domain, - chall.r_b64, chall.nonce, chall.key) + (chall.domain, chall.r_b64, chall.nonce, chall.key) ] for i in range(len(expected_call_list)): for j in range(len(expected_call_list[0])): self.assertEqual(calls[i][0][j], expected_call_list[i][j]) + def test_perform1(self): + chall = self.challs[0] + self.sni.add_chall(chall) + mock_setup_cert = mock.MagicMock(return_value="randomS1") + # pylint: disable=protected-access + self.sni._setup_challenge_cert = mock_setup_cert + + responses = self.sni.perform() + + mock_setup_cert.assert_called_once_with(chall) + + # Check to make sure challenge config path is included in apache config. self.assertEqual( len(self.sni.config.parser.find_dir( "Include", self.sni.challenge_conf)), @@ -80,26 +106,23 @@ class DvsniPerformTest(util.ApacheTest): self.assertEqual(len(responses), 1) self.assertEqual(responses[0]["s"], "randomS1") - @mock.patch("letsencrypt.client.challenge_util.dvsni_gen_cert") - def test_perform2(self, mock_dvsni_gen_cert): + def test_perform2(self): for chall in self.challs: self.sni.add_chall(chall) - mock_dvsni_gen_cert.side_effect = ["randomS0", "randomS1"] + mock_setup_cert = mock.MagicMock(side_effect=["randomS0", "randomS1"]) + # pylint: disable=protected-access + self.sni._setup_challenge_cert = mock_setup_cert + responses = self.sni.perform() - self.assertEqual(mock_dvsni_gen_cert.call_count, 2) - calls = mock_dvsni_gen_cert.call_args_list - expected_call_list = [] + self.assertEqual(mock_setup_cert.call_count, 2) - for chall in self.challs: - expected_call_list.append( - (self.sni.get_cert_file(chall.nonce), chall.domain, - chall.r_b64, chall.nonce, chall.key)) - - for i in range(len(expected_call_list)): - for j in range(len(expected_call_list[0])): - self.assertEqual(calls[i][0][j], expected_call_list[i][j]) + # Make sure calls made to mocked function were correct + self.assertEqual( + mock_setup_cert.call_args_list[0], mock.call(self.challs[0])) + self.assertEqual( + mock_setup_cert.call_args_list[1], mock.call(self.challs[1])) self.assertEqual( len(self.sni.config.parser.find_dir( diff --git a/letsencrypt/client/tests/challenge_util_test.py b/letsencrypt/client/tests/challenge_util_test.py index 759ee34ce..84a561d5d 100644 --- a/letsencrypt/client/tests/challenge_util_test.py +++ b/letsencrypt/client/tests/challenge_util_test.py @@ -5,7 +5,6 @@ import re import unittest import M2Crypto -import mock from letsencrypt.client import challenge_util from letsencrypt.client import client @@ -19,32 +18,19 @@ class DvsniGenCertTest(unittest.TestCase): def test_standard(self): """Basic test for straightline code.""" - # This is a helper function that can be used for handling - # open context managers more elegantly. It avoids dealing with - # __enter__ and __exit__ calls. - # http://www.voidspace.org.uk/python/mock/helpers.html#mock.mock_open - m_open = mock.mock_open() - with mock.patch("letsencrypt.client.challenge_util.open", - m_open, create=True): + domain = "example.com" + dvsni_r = "r_value" + r_b64 = le_util.jose_b64encode(dvsni_r) + pem = pkg_resources.resource_string( + __name__, os.path.join("testdata", "rsa256_key.pem")) + key = client.Client.Key("path", pem) + nonce = "12345ABCDE" + cert_pem, s_b64 = self._call(domain, r_b64, nonce, key) - domain = "example.com" - dvsni_r = "r_value" - r_b64 = le_util.jose_b64encode(dvsni_r) - pem = pkg_resources.resource_string( - __name__, os.path.join("testdata", "rsa256_key.pem")) - key = client.Client.Key("path", pem) - nonce = "12345ABCDE" - s_b64 = self._call("tmp.crt", domain, r_b64, nonce, key) - - self.assertTrue(m_open.called) - self.assertEqual(m_open.call_args[0], ("tmp.crt", 'w')) - self.assertEqual(m_open().write.call_count, 1) - - # pylint: disable=protected-access - ext = challenge_util._dvsni_gen_ext( - dvsni_r, le_util.jose_b64decode(s_b64)) - self._standard_check_cert( - m_open().write.call_args[0][0], domain, nonce, ext) + # pylint: disable=protected-access + ext = challenge_util._dvsni_gen_ext( + dvsni_r, le_util.jose_b64decode(s_b64)) + self._standard_check_cert(cert_pem, domain, nonce, ext) def _standard_check_cert(self, pem, domain, nonce, ext): """Check the certificate fields.""" @@ -60,7 +46,7 @@ class DvsniGenCertTest(unittest.TestCase): self.assertEqual(exp_sans, act_sans) - # pylint: disable= no-self-use - def _call(self, filepath, name, r_b64, nonce, key): + @classmethod + def _call(cls, name, r_b64, nonce, key): from letsencrypt.client.challenge_util import dvsni_gen_cert - return dvsni_gen_cert(filepath, name, r_b64, nonce, key) + return dvsni_gen_cert(name, r_b64, nonce, key) From d43b1dbf92852b152c7fdc1fd553ed4c431627f3 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 27 Jan 2015 14:53:28 -0800 Subject: [PATCH 2/2] range->xrange --- letsencrypt/client/tests/apache/dvsni_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/client/tests/apache/dvsni_test.py b/letsencrypt/client/tests/apache/dvsni_test.py index 53e7f81e3..a50f0a3f6 100644 --- a/letsencrypt/client/tests/apache/dvsni_test.py +++ b/letsencrypt/client/tests/apache/dvsni_test.py @@ -83,8 +83,8 @@ class DvsniPerformTest(util.ApacheTest): (chall.domain, chall.r_b64, chall.nonce, chall.key) ] - for i in range(len(expected_call_list)): - for j in range(len(expected_call_list[0])): + for i in xrange(len(expected_call_list)): + for j in xrange(len(expected_call_list[0])): self.assertEqual(calls[i][0][j], expected_call_list[i][j]) def test_perform1(self): @@ -129,7 +129,7 @@ class DvsniPerformTest(util.ApacheTest): "Include", self.sni.challenge_conf)), 1) self.assertEqual(len(responses), 2) - for i in range(2): + for i in xrange(2): self.assertEqual(responses[i]["s"], "randomS%d" % i) def test_mod_config(self):