diff --git a/acme/acme/client.py b/acme/acme/client.py index 1f4ae4fad..1838fab42 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -705,6 +705,28 @@ class BackwardsCompatibleClientV2(object): regr = regr.update(terms_of_service_agreed=True) return self.client.new_account(regr) + def new_order(self, csr_pem): + """Request a new Order object from the server. + + If using ACMEv1, returns a dummy OrderResource with only + the authorizations field filled in. + + :param str csr_pem: A CSR in PEM format. + + :returns: The newly created order. + :rtype: OrderResource + """ + if self.acme_version == 1: + csr = OpenSSL.crypto.load_certificate_request(OpenSSL.crypto.FILETYPE_PEM, csr_pem) + # pylint: disable=protected-access + dnsNames = crypto_util._pyopenssl_cert_or_req_all_names(csr) + authorizations = [] + for domain in dnsNames: + authorizations.append(self.client.request_domain_challenges(domain)) + return messages.OrderResource(authorizations=authorizations) + else: + return self.client.new_order(csr_pem) + def _acme_version_from_directory(self, directory): if hasattr(directory, 'newNonce'): return 2 diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 11516c02f..1b33ca5d7 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -161,6 +161,29 @@ class BackwardsCompatibleClientV2Test(ClientTestBase): mock_client().register.assert_called_once_with(self.new_reg) mock_client().agree_to_tos.assert_not_called() + @mock.patch('OpenSSL.crypto.load_certificate_request') + @mock.patch('acme.crypto_util._pyopenssl_cert_or_req_all_names') + def test_new_order_v1(self, mock__pyopenssl_cert_or_req_all_names, + unused_mock_load_certificate_request): + self.response.json.return_value = DIRECTORY_V1.to_json() + mock__pyopenssl_cert_or_req_all_names.return_value = ['example.com', 'www.example.com'] + mock_csr_pem = mock.MagicMock() + with mock.patch('acme.client.Client') as mock_client: + mock_client().request_domain_challenges.return_value = mock.sentinel.auth + client = self._init() + orderr = client.new_order(mock_csr_pem) + self.assertEqual(orderr.authorizations, [mock.sentinel.auth, mock.sentinel.auth]) + + def test_new_order_v2(self): + self.response.json.return_value = DIRECTORY_V2.to_json() + mock_csr_pem = mock.MagicMock() + with mock.patch('acme.client.ClientV2') as mock_client: + client = self._init() + client.new_order(mock_csr_pem) + mock_client().new_order.assert_called_once_with(mock_csr_pem) + + + class ClientTest(ClientTestBase): """Tests for acme.client.Client.""" diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py index 5f520cbcb..47d806b94 100644 --- a/certbot/auth_handler.py +++ b/certbot/auth_handler.py @@ -48,12 +48,13 @@ class AuthHandler(object): # List must be used to keep responses straight. self.achalls = [] - def get_authorizations(self, domains, best_effort=False): + def handle_authorizations(self, orderr, best_effort=False): """Retrieve all authorizations for challenges. - :param list domains: Domains for authorization + :param acme.messages.OrderResource orderr: must have + authorizations filled in :param bool best_effort: Whether or not all authorizations are - required (this is useful in renewal) + required (this is useful in renewal) :returns: List of authorization resources :rtype: list @@ -62,8 +63,10 @@ class AuthHandler(object): authorizations """ - for domain in domains: - self.authzr[domain] = self.acme.request_domain_challenges(domain) + authzrs = orderr.authorizations + for authzr in authzrs: + self.authzr[authzr.body.identifier.value] = authzr + domains = self.authzr.keys() self._choose_challenges(domains) config = zope.component.getUtility(interfaces.IConfig) diff --git a/certbot/client.py b/certbot/client.py index 67ee8f7fa..404e1e0d9 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -235,18 +235,13 @@ class Client(object): else: self.auth_handler = None - def obtain_certificate_from_csr(self, domains, csr, authzr=None): + def obtain_certificate_from_csr(self, csr, orderr=None): """Obtain certificate. - Internal function with precondition that `domains` are - consistent with identifiers present in the `csr`. - - :param list domains: Domain names. :param .util.CSR csr: PEM-encoded Certificate Signing Request. The key used to generate this CSR can be different than `authkey`. - :param list authzr: List of - :class:`acme.messages.AuthorizationResource` + :param acme.messages.OrderResource orderr: contains authzrs :returns: `.CertificateResource` and certificate chain (as returned by `.fetch_chain`). @@ -261,10 +256,14 @@ class Client(object): if self.account.regr is None: raise errors.Error("Please register with the ACME server first.") - logger.debug("CSR: %s, domains: %s", csr, domains) + logger.debug("CSR: %s", csr) + + if orderr is None: + orderr = self.acme.new_order(csr.data) + authzr = self.auth_handler.handle_authorizations(orderr) + else: + authzr = orderr.authorizations - if authzr is None: - authzr = self.auth_handler.get_authorizations(domains) certr = self.acme.request_issuance( jose.ComparableX509( @@ -307,13 +306,6 @@ class Client(object): :rtype: tuple """ - authzr = self.auth_handler.get_authorizations( - domains, - self.config.allow_subset_of_names) - - auth_domains = set(a.body.identifier.value for a in authzr) - domains = [d for d in domains if d in auth_domains] - # Create CSR from names if self.config.dry_run: key = util.Key(file=None, @@ -326,10 +318,20 @@ class Client(object): self.config.rsa_key_size, self.config.key_dir) csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) - certr, chain = self.obtain_certificate_from_csr( - domains, csr, authzr=authzr) + orderr = self.acme.new_order(csr.data) + authzr = self.auth_handler.handle_authorizations(orderr, self.config.allow_subset_of_names) + auth_domains = set(a.body.identifier.value for a in authzr) + successful_domains = [d for d in domains if d in auth_domains] - return certr, chain, key, csr + if successful_domains != domains: + if not self.config.dry_run: + os.remove(key.file) + os.remove(csr.file) + return self.obtain_certificate(successful_domains) + else: + certr, chain = self.obtain_certificate_from_csr(csr, orderr) + + return certr, chain, key, csr # pylint: disable=no-member def obtain_and_enroll_certificate(self, domains, certname): diff --git a/certbot/main.py b/certbot/main.py index ff3758985..d01f68920 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -1064,7 +1064,7 @@ def _csr_get_and_save_cert(config, le_client): """ csr, _ = config.actual_csr - certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr) + certr, chain = le_client.obtain_certificate_from_csr(csr) if config.dry_run: logger.debug( "Dry run: skipping saving certificate to %s", config.cert_path) diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index 32c4c0d3b..3633b673d 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -57,8 +57,8 @@ class ChallengeFactoryTest(unittest.TestCase): errors.Error, self.handler._challenge_factory, "failure.com", [0]) -class GetAuthorizationsTest(unittest.TestCase): - """get_authorizations test. +class HandleAuthorizationsTest(unittest.TestCase): + """handle_authorizations test. This tests everything except for all functions under _poll_challenges. @@ -92,12 +92,11 @@ class GetAuthorizationsTest(unittest.TestCase): @mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") def test_name1_tls_sni_01_1(self, mock_poll): - self.mock_net.request_domain_challenges.side_effect = functools.partial( - gen_dom_authzr, challs=acme_util.CHALLENGES) - mock_poll.side_effect = self._validate_all - authzr = self.handler.get_authorizations(["0"]) + authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES) + mock_order = mock.MagicMock(authorizations=[authzr]) + authzr = self.handler.handle_authorizations(mock_order) self.assertEqual(self.mock_net.answer_challenge.call_count, 1) @@ -115,14 +114,13 @@ class GetAuthorizationsTest(unittest.TestCase): @mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") def test_name1_tls_sni_01_1_http_01_1_dns_1(self, mock_poll): - self.mock_net.request_domain_challenges.side_effect = functools.partial( - gen_dom_authzr, challs=acme_util.CHALLENGES, combos=False) - mock_poll.side_effect = self._validate_all self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01) self.mock_auth.get_chall_pref.return_value.append(challenges.DNS01) - authzr = self.handler.get_authorizations(["0"]) + authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=False) + mock_order = mock.MagicMock(authorizations=[authzr]) + authzr = self.handler.handle_authorizations(mock_order) self.assertEqual(self.mock_net.answer_challenge.call_count, 3) @@ -146,7 +144,11 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr = self.handler.get_authorizations(["0", "1", "2"]) + authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES), + gen_dom_authzr(domain="1", challs=acme_util.CHALLENGES), + gen_dom_authzr(domain="2", challs=acme_util.CHALLENGES)] + mock_order = mock.MagicMock(authorizations=authzrs) + authzr = self.handler.handle_authorizations(mock_order) self.assertEqual(self.mock_net.answer_challenge.call_count, 3) @@ -169,31 +171,33 @@ class GetAuthorizationsTest(unittest.TestCase): def test_debug_challenges(self, mock_poll): zope.component.provideUtility( mock.Mock(debug_challenges=True), interfaces.IConfig) - self.mock_net.request_domain_challenges.side_effect = functools.partial( - gen_dom_authzr, challs=acme_util.CHALLENGES) + authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)] + mock_order = mock.MagicMock(authorizations=authzrs) mock_poll.side_effect = self._validate_all - self.handler.get_authorizations(["0"]) + self.handler.handle_authorizations(mock_order) self.assertEqual(self.mock_net.answer_challenge.call_count, 1) self.assertEqual(self.mock_display.notification.call_count, 1) def test_perform_failure(self): - self.mock_net.request_domain_challenges.side_effect = functools.partial( - gen_dom_authzr, challs=acme_util.CHALLENGES) + authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)] + mock_order = mock.MagicMock(authorizations=authzrs) + self.mock_auth.perform.side_effect = errors.AuthorizationError self.assertRaises( - errors.AuthorizationError, self.handler.get_authorizations, ["0"]) + errors.AuthorizationError, self.handler.handle_authorizations, mock_order) def test_no_domains(self): - self.assertRaises(errors.AuthorizationError, self.handler.get_authorizations, []) + mock_order = mock.MagicMock(authorizations=[]) + self.assertRaises(errors.AuthorizationError, self.handler.handle_authorizations, mock_order) @mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") def test_preferred_challenge_choice(self, mock_poll): - self.mock_net.request_domain_challenges.side_effect = functools.partial( - gen_dom_authzr, challs=acme_util.CHALLENGES) + authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)] + mock_order = mock.MagicMock(authorizations=authzrs) mock_poll.side_effect = self._validate_all self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01) @@ -201,18 +205,18 @@ class GetAuthorizationsTest(unittest.TestCase): self.handler.pref_challs.extend((challenges.HTTP01.typ, challenges.DNS01.typ,)) - self.handler.get_authorizations(["0"]) + self.handler.handle_authorizations(mock_order) self.assertEqual(self.mock_auth.cleanup.call_count, 1) self.assertEqual( self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01") def test_preferred_challenges_not_supported(self): - self.mock_net.request_domain_challenges.side_effect = functools.partial( - gen_dom_authzr, challs=acme_util.CHALLENGES) + authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)] + mock_order = mock.MagicMock(authorizations=authzrs) self.handler.pref_challs.append(challenges.HTTP01.typ) self.assertRaises( - errors.AuthorizationError, self.handler.get_authorizations, ["0"]) + errors.AuthorizationError, self.handler.handle_authorizations, mock_order) def _validate_all(self, unused_1, unused_2): for dom in six.iterkeys(self.handler.authzr): diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index a9a87b80b..f4a8a5c8a 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -134,6 +134,7 @@ class ClientTest(ClientTestCommon): self.config.allow_subset_of_names = False self.config.dry_run = False self.eg_domains = ["example.com", "www.example.com"] + self.eg_order = mock.MagicMock(authorizations=[None]) def test_init_acme_verify_ssl(self): net = self.acme_client.call_args[0][0] @@ -141,16 +142,20 @@ class ClientTest(ClientTestCommon): def _mock_obtain_certificate(self): self.client.auth_handler = mock.MagicMock() - self.client.auth_handler.get_authorizations.return_value = [None] + self.client.auth_handler.handle_authorizations.return_value = [None] self.acme.request_issuance.return_value = mock.sentinel.certr self.acme.fetch_chain.return_value = mock.sentinel.chain + self.acme.new_order.return_value = self.eg_order - def _check_obtain_certificate(self): - self.client.auth_handler.get_authorizations.assert_called_once_with( - self.eg_domains, - self.config.allow_subset_of_names) + def _check_obtain_certificate(self, auth_count=1): + if auth_count == 1: + self.client.auth_handler.handle_authorizations.assert_called_once_with( + self.eg_order, + self.config.allow_subset_of_names) + else: + self.assertEqual(self.client.auth_handler.handle_authorizations.call_count, auth_count) - authzr = self.client.auth_handler.get_authorizations() + authzr = self.client.auth_handler.handle_authorizations() self.acme.request_issuance.assert_called_once_with( jose.ComparableX509(OpenSSL.crypto.load_certificate_request( @@ -167,31 +172,29 @@ class ClientTest(ClientTestCommon): test_csr = util.CSR(form="pem", file=None, data=CSR_SAN) auth_handler = self.client.auth_handler - authzr = auth_handler.get_authorizations(self.eg_domains, False) + orderr = self.acme.new_order(test_csr.data) + auth_handler.handle_authorizations(orderr, False) self.assertEqual( (mock.sentinel.certr, mock.sentinel.chain), self.client.obtain_certificate_from_csr( - self.eg_domains, test_csr, - authzr=authzr)) + orderr=orderr)) # and that the cert was obtained correctly self._check_obtain_certificate() - # Test for authzr=None + # Test for orderr=None self.assertEqual( (mock.sentinel.certr, mock.sentinel.chain), self.client.obtain_certificate_from_csr( - self.eg_domains, test_csr, - authzr=None)) - auth_handler.get_authorizations.assert_called_with(self.eg_domains) + orderr=None)) + auth_handler.handle_authorizations.assert_called_with(self.eg_order) # Test for no auth_handler self.client.auth_handler = None self.assertRaises( errors.Error, self.client.obtain_certificate_from_csr, - self.eg_domains, test_csr) mock_logger.warning.assert_called_once_with(mock.ANY) @@ -202,15 +205,13 @@ class ClientTest(ClientTestCommon): self.acme.fetch_chain.side_effect = [acme_errors.Error, mock.sentinel.chain] test_csr = util.CSR(form="der", file=None, data=CSR_SAN) - auth_handler = self.client.auth_handler - authzr = auth_handler.get_authorizations(self.eg_domains, False) + orderr = self.acme.new_order(test_csr.data) self.assertEqual( (mock.sentinel.certr, mock.sentinel.chain), self.client.obtain_certificate_from_csr( - self.eg_domains, test_csr, - authzr=authzr)) + orderr=orderr)) self.assertEqual(1, mock_get_utility().notification.call_count) @test_util.patch_get_utility() @@ -218,15 +219,13 @@ class ClientTest(ClientTestCommon): self._mock_obtain_certificate() self.acme.fetch_chain.side_effect = acme_errors.Error test_csr = util.CSR(form="der", file=None, data=CSR_SAN) - auth_handler = self.client.auth_handler - authzr = auth_handler.get_authorizations(self.eg_domains, False) + orderr = self.acme.new_order(test_csr.data) self.assertRaises( acme_errors.Error, self.client.obtain_certificate_from_csr, - self.eg_domains, test_csr, - authzr=authzr) + orderr=orderr) self.assertEqual(1, mock_get_utility().notification.call_count) @mock.patch("certbot.client.crypto_util") @@ -242,6 +241,21 @@ class ClientTest(ClientTestCommon): mock_crypto_util.init_save_csr.assert_called_once_with( mock.sentinel.key, self.eg_domains, self.config.csr_dir) + @mock.patch("certbot.client.crypto_util") + @mock.patch("os.remove") + def test_obtain_certificate_partial_success(self, mock_remove, mock_crypto_util): + csr = util.CSR(form="pem", file=mock.sentinel.csr_file, data=CSR_SAN) + key = util.CSR(form="pem", file=mock.sentinel.key_file, data=CSR_SAN) + mock_crypto_util.init_save_csr.return_value = csr + mock_crypto_util.init_save_key.return_value = key + + authzr = self._authzr_from_domains(["example.com"]) + self._test_obtain_certificate_common(key, csr, authzr_ret=authzr, auth_count=2) + + self.assertEqual(mock_crypto_util.init_save_key.call_count, 2) + self.assertEqual(mock_crypto_util.init_save_csr.call_count, 2) + self.assertEqual(mock_remove.call_count, 2) + @mock.patch("certbot.client.crypto_util") @mock.patch("certbot.client.acme_crypto_util") def test_obtain_certificate_dry_run(self, mock_acme_crypto, mock_crypto): @@ -259,24 +273,28 @@ class ClientTest(ClientTestCommon): mock_crypto.init_save_key.assert_not_called() mock_crypto.init_save_csr.assert_not_called() - def _test_obtain_certificate_common(self, key, csr): - self._mock_obtain_certificate() - - # return_value is essentially set to (None, None) in - # _mock_obtain_certificate(), which breaks this test. - # Thus fixed by the next line. - + def _authzr_from_domains(self, domains): authzr = [] # domain ordering should not be affected by authorization order - for domain in reversed(self.eg_domains): + for domain in reversed(domains): authzr.append( mock.MagicMock( body=mock.MagicMock( identifier=mock.MagicMock( value=domain)))) + return authzr - self.client.auth_handler.get_authorizations.return_value = authzr + def _test_obtain_certificate_common(self, key, csr, authzr_ret=None, auth_count=1): + self._mock_obtain_certificate() + + # return_value is essentially set to (None, None) in + # _mock_obtain_certificate(), which breaks this test. + # Thus fixed by the next line. + authzr = authzr_ret or self._authzr_from_domains(self.eg_domains) + + self.eg_order.authorizations = authzr + self.client.auth_handler.handle_authorizations.return_value = authzr with test_util.patch_get_utility(): result = self.client.obtain_certificate(self.eg_domains) @@ -284,7 +302,7 @@ class ClientTest(ClientTestCommon): self.assertEqual( result, (mock.sentinel.certr, mock.sentinel.chain, key, csr)) - self._check_obtain_certificate() + self._check_obtain_certificate(auth_count) @mock.patch('certbot.client.Client.obtain_certificate') @mock.patch('certbot.storage.RenewableCert.new_lineage')