From 327804608f7cc44ab6052d3944254611b4eaa7bc Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Fri, 30 Nov 2018 20:06:45 +0000 Subject: [PATCH] Revert "Implement POST-as-GET requests (#6522)" This reverts commit 0b5468e992ab57fa028ddf33ca2351cb37c8e1ee. --- acme/acme/client.py | 92 +++++++++-------------------- acme/acme/client_test.py | 30 +--------- tests/certbot-pebble-integration.sh | 16 ----- tests/pebble-fetch.sh | 41 ------------- 4 files changed, 29 insertions(+), 150 deletions(-) delete mode 100755 tests/certbot-pebble-integration.sh delete mode 100755 tests/pebble-fetch.sh diff --git a/acme/acme/client.py b/acme/acme/client.py index 88938e999..af52b44ed 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -200,6 +200,22 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes return datetime.datetime.now() + datetime.timedelta(seconds=seconds) + def poll(self, authzr): + """Poll Authorization Resource for status. + + :param authzr: Authorization Resource + :type authzr: `.AuthorizationResource` + + :returns: Updated Authorization Resource and HTTP response. + + :rtype: (`.AuthorizationResource`, `requests.Response`) + + """ + response = self.net.get(authzr.uri) + updated_authzr = self._authzr_from_response( + response, authzr.body.identifier, authzr.uri) + return updated_authzr, response + def _revoke(self, cert, rsn, url): """Revoke certificate. @@ -221,7 +237,6 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes raise errors.ClientError( 'Successful revocation must return HTTP OK status') - class Client(ClientBase): """ACME client for a v1 API. @@ -374,22 +389,6 @@ class Client(ClientBase): body=jose.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_ASN1, response.content))) - def poll(self, authzr): - """Poll Authorization Resource for status. - - :param authzr: Authorization Resource - :type authzr: `.AuthorizationResource` - - :returns: Updated Authorization Resource and HTTP response. - - :rtype: (`.AuthorizationResource`, `requests.Response`) - - """ - response = self.net.get(authzr.uri) - updated_authzr = self._authzr_from_response( - response, authzr.body.identifier, authzr.uri) - return updated_authzr, response - def poll_and_request_issuance( self, csr, authzrs, mintime=5, max_attempts=10): """Poll and request issuance. @@ -653,29 +652,13 @@ class ClientV2(ClientBase): body = messages.Order.from_json(response.json()) authorizations = [] for url in body.authorizations: - authorizations.append(self._authzr_from_response(self._post_as_get(url), uri=url)) + authorizations.append(self._authzr_from_response(self.net.get(url), uri=url)) return messages.OrderResource( body=body, uri=response.headers.get('Location'), authorizations=authorizations, csr_pem=csr_pem) - def poll(self, authzr): - """Poll Authorization Resource for status. - - :param authzr: Authorization Resource - :type authzr: `.AuthorizationResource` - - :returns: Updated Authorization Resource and HTTP response. - - :rtype: (`.AuthorizationResource`, `requests.Response`) - - """ - response = self._post_as_get(authzr.uri) - updated_authzr = self._authzr_from_response( - response, authzr.body.identifier, authzr.uri) - return updated_authzr, response - def poll_and_finalize(self, orderr, deadline=None): """Poll authorizations and finalize the order. @@ -699,7 +682,7 @@ class ClientV2(ClientBase): responses = [] for url in orderr.body.authorizations: while datetime.datetime.now() < deadline: - authzr = self._authzr_from_response(self._post_as_get(url), uri=url) + authzr = self._authzr_from_response(self.net.get(url), uri=url) if authzr.body.status != messages.STATUS_PENDING: responses.append(authzr) break @@ -734,12 +717,12 @@ class ClientV2(ClientBase): self._post(orderr.body.finalize, wrapped_csr) while datetime.datetime.now() < deadline: time.sleep(1) - response = self._post_as_get(orderr.uri) + response = self.net.get(orderr.uri) body = messages.Order.from_json(response.json()) if body.error is not None: raise errors.IssuanceError(body.error) if body.certificate is not None: - certificate_response = self._post_as_get(body.certificate, + certificate_response = self.net.get(body.certificate, content_type=DER_CONTENT_TYPE).text return orderr.update(body=body, fullchain_pem=certificate_response) raise errors.TimeoutError() @@ -757,32 +740,6 @@ class ClientV2(ClientBase): """ return self._revoke(cert, rsn, self.directory['revokeCert']) - def _post_as_get(self, *args, **kwargs): - """ - Send GET request using the POST-as-GET protocol if needed. - The request will be first issued using POST-as-GET for ACME v2. If the ACME CA servers do - not support this yet and return an error, request will be retried using GET. - For ACME v1, only GET request will be tried, as POST-as-GET is not supported. - :param args: - :param kwargs: - :return: - """ - if self.acme_version >= 2: - # We add an empty payload for POST-as-GET requests - new_args = args[:1] + (None,) + args[1:] - try: - return self._post(*new_args, **kwargs) # pylint: disable=star-args - except messages.Error as error: - if error.code == 'malformed': - logger.debug('Error during a POST-as-GET request, ' - 'your ACME CA may not support it:\n%s', error) - logger.debug('Retrying request with GET.') - else: # pragma: no cover - raise - - # If POST-as-GET is not supported yet, we use a GET instead. - return self.net.get(*args, **kwargs) - class BackwardsCompatibleClientV2(object): """ACME client wrapper that tends towards V2-style calls, but @@ -812,7 +769,12 @@ class BackwardsCompatibleClientV2(object): self.client = ClientV2(directory, net=net) def __getattr__(self, name): - return getattr(self.client, name) + if name in vars(self.client): + return getattr(self.client, name) + elif name in dir(ClientBase): + return getattr(self.client, name) + else: + raise AttributeError() def new_account_and_tos(self, regr, check_tos_cb=None): """Combined register and agree_tos for V1, new_account for V2 @@ -982,7 +944,7 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes :rtype: `josepy.JWS` """ - jobj = obj.json_dumps(indent=2).encode() if obj else b'' + jobj = obj.json_dumps(indent=2).encode() logger.debug('JWS payload:\n%s', jobj) kwargs = { "alg": self.alg, diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 8b6cda47e..18b61d537 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -1,5 +1,4 @@ """Tests for acme.client.""" -# pylint: disable=too-many-lines import copy import datetime import json @@ -731,10 +730,9 @@ class ClientV2Test(ClientTestBase): authz_response2 = self.response authz_response2.json.return_value = self.authz2.to_json() authz_response2.headers['Location'] = self.authzr2.uri + self.net.get.side_effect = (authz_response, authz_response2) - with mock.patch('acme.client.ClientV2._post_as_get') as mock_post_as_get: - mock_post_as_get.side_effect = (authz_response, authz_response2) - self.assertEqual(self.client.new_order(CSR_SAN_PEM), self.orderr) + self.assertEqual(self.client.new_order(CSR_SAN_PEM), self.orderr) @mock.patch('acme.client.datetime') def test_poll_and_finalize(self, mock_datetime): @@ -823,30 +821,6 @@ class ClientV2Test(ClientTestBase): self.response.json.return_value = self.regr.body.update( contact=()).to_json() - def test_post_as_get(self): - with mock.patch('acme.client.ClientV2._authzr_from_response') as mock_client: - mock_client.return_value = self.authzr2 - - self.client.poll(self.authzr2) # pylint: disable=protected-access - - self.client.net.post.assert_called_once_with( - self.authzr2.uri, None, acme_version=2, - new_nonce_url='https://www.letsencrypt-demo.org/acme/new-nonce') - self.client.net.get.assert_not_called() - - class FakeError(messages.Error): # pylint: disable=too-many-ancestors - """Fake error to reproduce a malformed request ACME error""" - def __init__(self): # pylint: disable=super-init-not-called - pass - @property - def code(self): - return 'malformed' - self.client.net.post.side_effect = FakeError() - - self.client.poll(self.authzr2) # pylint: disable=protected-access - - self.client.net.get.assert_called_once_with(self.authzr2.uri) - class MockJSONDeSerializable(jose.JSONDeSerializable): # pylint: disable=missing-docstring diff --git a/tests/certbot-pebble-integration.sh b/tests/certbot-pebble-integration.sh deleted file mode 100755 index 8711f72c1..000000000 --- a/tests/certbot-pebble-integration.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/bash -# Simple integration test. Make sure to activate virtualenv beforehand -# (source venv/bin/activate) and that you are running Pebble test -# instance (see ./pebble-fetch.sh). - -cleanup_and_exit() { - EXIT_STATUS=$? - unset SERVER - exit $EXIT_STATUS -} - -trap cleanup_and_exit EXIT - -export SERVER=https://localhost:14000/dir - -./tests/certbot-boulder-integration.sh diff --git a/tests/pebble-fetch.sh b/tests/pebble-fetch.sh deleted file mode 100755 index b0ba08961..000000000 --- a/tests/pebble-fetch.sh +++ /dev/null @@ -1,41 +0,0 @@ -#!/bin/bash -# Download and run Pebble instance for integration testing -set -xe - -PEBBLE_VERSION=2018-11-02 - -# We reuse the same GOPATH-style directory than for Boulder. -# Pebble does not need it, but it will make the installation consistent with Boulder's one. -export GOPATH=${GOPATH:-$HOME/gopath} -PEBBLEPATH=${PEBBLEPATH:-$GOPATH/src/github.com/letsencrypt/pebble} - -mkdir -p ${PEBBLEPATH} - -cat << UNLIKELY_EOF > "$PEBBLEPATH/docker-compose.yml" -version: '3' - -services: - pebble: - image: letsencrypt/pebble:${PEBBLE_VERSION} - command: pebble -strict ${PEBBLE_STRICT:-false} -dnsserver 10.77.77.1 - ports: - - 14000:14000 - environment: - - PEBBLE_VA_NOSLEEP=1 -UNLIKELY_EOF - -docker-compose -f "$PEBBLEPATH/docker-compose.yml" up -d pebble - -set +x # reduce verbosity while waiting for boulder -for n in `seq 1 150` ; do - if curl -k https://localhost:14000/dir 2>/dev/null; then - break - else - sleep 1 - fi -done - -if ! curl -k https://localhost:14000/dir 2>/dev/null; then - echo "timed out waiting for pebble to start" - exit 1 -fi