From 62361dac442c065005455a0c78ea066fedc6c5f8 Mon Sep 17 00:00:00 2001 From: ohemorange Date: Fri, 2 May 2025 12:08:46 -0700 Subject: [PATCH] Catch and ignore orderNotReady response when posting a request to begin finalization and poll until `ready` (#10239) Fixes #9766. --- acme/src/acme/_internal/tests/client_test.py | 63 ++++++++++++++++++-- acme/src/acme/client.py | 25 +++++++- certbot/CHANGELOG.md | 6 +- 3 files changed, 87 insertions(+), 7 deletions(-) diff --git a/acme/src/acme/_internal/tests/client_test.py b/acme/src/acme/_internal/tests/client_test.py index f863fffd0..d86bda7aa 100644 --- a/acme/src/acme/_internal/tests/client_test.py +++ b/acme/src/acme/_internal/tests/client_test.py @@ -254,6 +254,26 @@ class ClientV2Test(unittest.TestCase): with pytest.raises(errors.IssuanceError): self.client.finalize_order(self.orderr, deadline) + @mock.patch('acme.client.ClientV2.begin_finalization') + def test_finalize_order_ready(self, mock_begin): + # https://github.com/certbot/certbot/issues/9766 + updated_order_ready = self.order.update(status=messages.STATUS_READY) + + updated_order_valid = self.order.update( + certificate='https://www.letsencrypt-demo.org/acme/cert/', + status=messages.STATUS_VALID) + updated_orderr = self.orderr.update(body=updated_order_valid, fullchain_pem=CERT_SAN_PEM) + + self.response.text = CERT_SAN_PEM + + self.response.json.side_effect = [updated_order_ready.to_json(), + updated_order_valid.to_json()] + + deadline = datetime.datetime(9999, 9, 9) + assert self.client.finalize_order(self.orderr, deadline) == updated_orderr + assert self.response.json.call_count == 2 + assert mock_begin.call_count == 2 + def test_finalize_order_invalid_status(self): # https://github.com/certbot/certbot/issues/9296 order = self.order.update(error=None, status=messages.STATUS_INVALID) @@ -261,6 +281,43 @@ class ClientV2Test(unittest.TestCase): with pytest.raises(errors.Error, match="The certificate order failed"): self.client.finalize_order(self.orderr, datetime.datetime(9999, 9, 9)) + def test_finalize_order_orderNotReady(self): + # https://github.com/certbot/certbot/issues/9766 + updated_order_processing = self.order.update(status=messages.STATUS_PROCESSING) + updated_order_ready = self.order.update(status=messages.STATUS_READY) + + updated_order_valid = self.order.update( + certificate='https://www.letsencrypt-demo.org/acme/cert/', + status=messages.STATUS_VALID) + updated_orderr = self.orderr.update(body=updated_order_valid, fullchain_pem=CERT_SAN_PEM) + + self.response.text = CERT_SAN_PEM + + self.response.json.side_effect = [updated_order_processing.to_json(), + updated_order_ready.to_json(), + updated_order_valid.to_json()] + + post = mock.MagicMock() + post.side_effect = [messages.Error.with_code('orderNotReady'), # first begin_finalization + self.response, # first poll_finalization poll --> still returns pending + self.response, # second poll_finalization poll --> returns ready + mock.MagicMock(), # second begin_finalization + self.response, # third poll_finalization poll --> returns valid + self.response # fetch cert + ] + self.net.post = post + + self.client.finalize_order(self.orderr, datetime.datetime(9999, 9, 9)) + assert self.net.post.call_count == 6 + + def test_finalize_order_otherErrorCode(self): + post = mock.MagicMock() + post.side_effect = [messages.Error.with_code('serverInternal')] + self.net.post = post + + with pytest.raises(messages.Error): + self.client.finalize_order(self.orderr, datetime.datetime(9999, 9, 9)) + def test_finalize_order_timeout(self): deadline = datetime.datetime.now() - datetime.timedelta(seconds=60) with pytest.raises(errors.TimeoutError): @@ -342,7 +399,7 @@ class ClientV2Test(unittest.TestCase): 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.poll(self.authzr2) self.client.net.post.assert_called_once_with( self.authzr2.uri, None, @@ -716,7 +773,6 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): assert self.response.checked def test_post(self): - # pylint: disable=protected-access assert self.response == self.net.post( 'uri', self.obj, content_type=self.content_type) assert self.response.checked @@ -745,7 +801,6 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): check_response = mock.MagicMock() check_response.side_effect = messages.Error.with_code('badNonce') - # pylint: disable=protected-access self.net._check_response = check_response with pytest.raises(messages.Error): self.net.post('uri', @@ -756,7 +811,6 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): check_response.side_effect = [messages.Error.with_code('malformed'), self.response] - # pylint: disable=protected-access self.net._check_response = check_response with pytest.raises(messages.Error): self.net.post('uri', @@ -767,7 +821,6 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): post_once.side_effect = [messages.Error.with_code('badNonce'), self.response] - # pylint: disable=protected-access assert self.response == self.net.post( 'uri', self.obj, content_type=self.content_type) diff --git a/acme/src/acme/client.py b/acme/src/acme/client.py index ede6ccac1..84ff64cbc 100644 --- a/acme/src/acme/client.py +++ b/acme/src/acme/client.py @@ -224,6 +224,11 @@ class ClientV2: :returns: updated order :rtype: messages.OrderResource + + :raises .messages.Error: If server indicates order is not yet in ready state, + it will return a 403 (Forbidden) error with a problem document/error code of type + "orderNotReady" + """ csr = x509.load_pem_x509_csr(orderr.csr_pem) wrapped_csr = messages.CertificateRequest(csr=csr) @@ -239,6 +244,10 @@ class ClientV2: Poll an order that has been finalized for its status. If it becomes valid, obtain the certificate. + If a finalization request previously returned `orderNotReady`, + poll until ready, send a new finalization request, and continue + polling until valid as above. + :returns: finalized order (with certificate) :rtype: messages.OrderResource """ @@ -248,12 +257,22 @@ class ClientV2: response = self._post_as_get(orderr.uri) body = messages.Order.from_json(response.json()) if body.status == messages.STATUS_INVALID: + # "invalid": The certificate will not be issued. Consider this + # order process abandoned. if body.error is not None: raise errors.IssuanceError(body.error) raise errors.Error( "The certificate order failed. No further information was provided " "by the server.") + elif body.status == messages.STATUS_READY: + # "ready": The server agrees that the requirements have been + # fulfilled, and is awaiting finalization. Submit a finalization + # request. + self.begin_finalization(orderr) elif body.status == messages.STATUS_VALID and body.certificate is not None: + # "valid": The server has issued the certificate and provisioned its + # URL to the "certificate" field of the order. Download the + # certificate. certificate_response = self._post_as_get(body.certificate) orderr = orderr.update(body=body, fullchain_pem=certificate_response.text) if fetch_alternative_chains: @@ -276,7 +295,11 @@ class ClientV2: :rtype: messages.OrderResource """ - self.begin_finalization(orderr) + try: + self.begin_finalization(orderr) + except messages.Error as e: + if e.code != 'orderNotReady': + raise e return self.poll_finalization(orderr, deadline, fetch_alternative_chains) def revoke(self, cert: x509.Certificate, rsn: int) -> None: diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index f0b89129c..92622e8c2 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -19,7 +19,11 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* Order finalization now catches `orderNotReady` response, polls until order status is + `ready`, and resubmits finalization request before polling for `valid` to download + certificate. This conforms to RFC 8555 more accurately and avoids race conditions where + all authorizations are fulfilled but order has not yet transitioned to ready state on + the server when the finalization request is sent. More details about these changes can be found on our GitHub repo.