upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-06-03 13:59:02 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Fix ACMEv2 issues (#5612)

Browse source 
* Add post wrapper to automatically add acme_version

* Add uri to authzr.

* Only add kid when account is set.

* Add content_type when downloading certificate.

* Only save new_authz URL when it exists.

* Handle combinations in ACMEv1 and ACMEv2.

* Add tests for ACMEv2 "combinations".
This commit is contained in:
Brad Warren 2018-02-22 16:28:50 -08:00 committed by ohemorange
parent 990b211a76
commit 1e46d26ac3
5 changed files with 134 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
acme/acme/client.py
View file

@ -70,7 +70,7 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes
terms_of_service=terms_of_service)
def _send_recv_regr(self, regr, body):
response = self.net.post(regr.uri, body, acme_version=self.acme_version)
response = self._post(regr.uri, body)
# TODO: Boulder returns httplib.ACCEPTED
#assert response.status_code == httplib.OK
@ -82,6 +82,13 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes
response, uri=regr.uri,
terms_of_service=regr.terms_of_service)
def _post(self, *args, **kwargs):
"""Wrapper around self.net.post that adds the acme_version.
"""
kwargs.setdefault('acme_version', self.acme_version)
return self.net.post(*args, **kwargs)
def update_registration(self, regr, update=None):
"""Update registration.
@ -143,8 +150,7 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes
:raises .UnexpectedUpdate:
"""
response = self.net.post(challb.uri, response,
acme_version=self.acme_version)
response = self._post(challb.uri, response)
try:
authzr_uri = response.links['up']['url']
except KeyError:
@ -216,12 +222,11 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes
:raises .ClientError: If revocation is unsuccessful.
"""
response = self.net.post(self.directory[messages.Revocation],
response = self._post(self.directory[messages.Revocation],
messages.Revocation(
certificate=cert,
reason=rsn),
content_type=None,
acme_version=self.acme_version)
content_type=None)
if response.status_code != http_client.OK:
raise errors.ClientError(
'Successful revocation must return HTTP OK status')
@ -271,8 +276,7 @@ class Client(ClientBase):
"""
new_reg = messages.NewRegistration() if new_reg is None else new_reg
response = self.net.post(self.directory[new_reg], new_reg,
acme_version=1)
response = self._post(self.directory[new_reg], new_reg)
# TODO: handle errors
assert response.status_code == http_client.CREATED
@ -308,8 +312,7 @@ class Client(ClientBase):
if new_authzr_uri is not None:
logger.debug("request_challenges with new_authzr_uri deprecated.")
new_authz = messages.NewAuthorization(identifier=identifier)
response = self.net.post(self.directory.new_authz, new_authz,
acme_version=1)
response = self._post(self.directory.new_authz, new_authz)
# TODO: handle errors
assert response.status_code == http_client.CREATED
return self._authzr_from_response(response, identifier)
@ -351,12 +354,11 @@ class Client(ClientBase):
req = messages.CertificateRequest(csr=csr)
content_type = DER_CONTENT_TYPE # TODO: add 'cert_type 'argument
response = self.net.post(
response = self._post(
self.directory.new_cert,
req,
content_type=content_type,
headers={'Accept': content_type},
acme_version=1)
headers={'Accept': content_type})
cert_chain_uri = response.links.get('up', {}).get('url')
@ -552,8 +554,7 @@ class ClientV2(ClientBase):
:returns: Registration Resource.
:rtype: `.RegistrationResource`
"""
response = self.net.post(self.directory['newAccount'], new_account,
acme_version=2)
response = self._post(self.directory['newAccount'], new_account)
# "Instance of 'Field' has no key/contact member" bug:
# pylint: disable=no-member
regr = self._regr_from_response(response)
@ -577,11 +578,11 @@ class ClientV2(ClientBase):
identifiers.append(messages.Identifier(typ=messages.IDENTIFIER_FQDN,
value=name))
order = messages.NewOrder(identifiers=identifiers)
response = self.net.post(self.directory['newOrder'], order)
response = self._post(self.directory['newOrder'], order)
body = messages.Order.from_json(response.json())
authorizations = []
for url in body.authorizations:
authorizations.append(self._authzr_from_response(self.net.get(url)))
authorizations.append(self._authzr_from_response(self.net.get(url), uri=url))
return messages.OrderResource(
body=body,
uri=response.headers.get('Location'),
@ -643,7 +644,7 @@ class ClientV2(ClientBase):
csr = OpenSSL.crypto.load_certificate_request(
OpenSSL.crypto.FILETYPE_PEM, orderr.csr_pem)
wrapped_csr = messages.CertificateRequest(csr=jose.ComparableX509(csr))
self.net.post(orderr.body.finalize, wrapped_csr)
self._post(orderr.body.finalize, wrapped_csr)
while datetime.datetime.now() < deadline:
time.sleep(1)
response = self.net.get(orderr.uri)
@ -651,17 +652,29 @@ class ClientV2(ClientBase):
if body.error is not None:
raise errors.IssuanceError(body.error)
if body.certificate is not None:
certificate_response = self.net.get(body.certificate).text
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()
class BackwardsCompatibleClientV2(object):
"""ACME client wrapper that tends towards V2-style calls, but
supports V1 servers.
supports V1 servers.
:ivar int acme_version: 1 or 2, corresponding to the Let's Encrypt endpoint
:ivar .ClientBase client: either Client or ClientV2
.. note:: While this class handles the majority of the differences
between versions of the ACME protocol, if you need to support an
ACME server based on version 3 or older of the IETF ACME draft
that uses combinations in authorizations (or lack thereof) to
signal that the client needs to complete something other than
any single challenge in the authorization to make it valid, the
user of this class needs to understand and handle these
differences themselves. This does not apply to either of Let's
Encrypt's endpoints where successfully completing any challenge
in an authorization will make it valid.
:ivar int acme_version: 1 or 2, corresponding to the Let's Encrypt endpoint
:ivar .ClientBase client: either Client or ClientV2
"""
def __init__(self, net, key, server):
@ -829,7 +842,9 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes
}
if acme_version == 2:
kwargs["url"] = url
kwargs["kid"] = self.account["uri"]
# newAccount and revokeCert work without the kid
if self.account is not None:
kwargs["kid"] = self.account["uri"]
kwargs["key"] = self.key
# pylint: disable=star-args
return jws.JWS.sign(jobj, **kwargs).json_dumps(indent=2)

17
certbot/account.py
View file

@ -223,12 +223,17 @@ class AccountFileStorage(interfaces.AccountStorage):
try:
with open(self._regr_path(account_dir_path), "w") as regr_file:
regr = account.regr
with_uri = RegistrationResourceWithNewAuthzrURI(
new_authzr_uri=acme.directory.new_authz,
body=regr.body,
uri=regr.uri,
terms_of_service=regr.terms_of_service)
regr_file.write(with_uri.json_dumps())
# If we have a value for new-authz, save it for forwards
# compatibility with older versions of Certbot. If we don't
# have a value for new-authz, this is an ACMEv2 directory where
# an older version of Certbot won't work anyway.
if hasattr(acme.directory, "new-authz"):
regr = RegistrationResourceWithNewAuthzrURI(
new_authzr_uri=acme.directory.new_authz,
body=regr.body,
uri=regr.uri,
terms_of_service=regr.terms_of_service)
regr_file.write(regr.json_dumps())
if not regr_only:
with util.safe_open(self._key_path(account_dir_path),
"w", chmod=0o400) as key_file:

12
certbot/auth_handler.py
View file

@ -24,7 +24,7 @@ class AuthHandler(object):
:class:`~acme.challenges.Challenge` types
:type auth: :class:`certbot.interfaces.IAuthenticator`
:ivar acme.client.Client acme: ACME client API.
:ivar acme.client.BackwardsCompatibleClientV2 acme: ACME client API.
:ivar account: Client's Account
:type account: :class:`certbot.account.Account`
@ -100,10 +100,16 @@ class AuthHandler(object):
"""Retrieve necessary challenges to satisfy server."""
logger.info("Performing the following challenges:")
for dom in domains:
dom_challenges = self.authzr[dom].body.challenges
if self.acme.acme_version == 1:
combinations = self.authzr[dom].body.combinations
else:
combinations = tuple((i,) for i in range(len(dom_challenges)))
path = gen_challenge_path(
self.authzr[dom].body.challenges,
dom_challenges,
self._get_chall_pref(dom),
self.authzr[dom].body.combinations)
combinations)
dom_achalls = self._challenge_factory(
dom, path)

4
certbot/client.py
View file

@ -212,8 +212,8 @@ class Client(object):
:ivar .IAuthenticator auth: Prepared (`.IAuthenticator.prepare`)
authenticator that can solve ACME challenges.
:ivar .IInstaller installer: Installer.
:ivar acme.client.Client acme: Optional ACME client API handle.
You might already have one from `register`.
:ivar acme.client.BackwardsCompatibleClientV2 acme: Optional ACME
client API handle. You might already have one from `register`.
"""

93
certbot/tests/auth_handler_test.py
View file

@ -81,6 +81,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
self.mock_account = mock.Mock(key=util.Key("file_path", "PEM"))
self.mock_net = mock.MagicMock(spec=acme_client.Client)
self.mock_net.acme_version = 1
self.handler = AuthHandler(
self.mock_auth, self.mock_net, self.mock_account, [])
@ -90,13 +91,13 @@ class HandleAuthorizationsTest(unittest.TestCase):
def tearDown(self):
logging.disable(logging.NOTSET)
@mock.patch("certbot.auth_handler.AuthHandler._poll_challenges")
def test_name1_tls_sni_01_1(self, mock_poll):
mock_poll.side_effect = self._validate_all
authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)
def _test_name1_tls_sni_01_1_common(self, combos):
authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=combos)
mock_order = mock.MagicMock(authorizations=[authzr])
authzr = self.handler.handle_authorizations(mock_order)
with mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") as mock_poll:
mock_poll.side_effect = self._validate_all
authzr = self.handler.handle_authorizations(mock_order)
self.assertEqual(self.mock_net.answer_challenge.call_count, 1)
@ -112,8 +113,15 @@ class HandleAuthorizationsTest(unittest.TestCase):
self.assertEqual(len(authzr), 1)
def test_name1_tls_sni_01_1_acme_1(self):
self._test_name1_tls_sni_01_1_common(combos=True)
def test_name1_tls_sni_01_1_acme_2(self):
self.mock_net.acme_version = 2
self._test_name1_tls_sni_01_1_common(combos=False)
@mock.patch("certbot.auth_handler.AuthHandler._poll_challenges")
def test_name1_tls_sni_01_1_http_01_1_dns_1(self, mock_poll):
def test_name1_tls_sni_01_1_http_01_1_dns_1_acme_1(self, mock_poll):
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)
@ -138,17 +146,43 @@ class HandleAuthorizationsTest(unittest.TestCase):
self.assertEqual(len(authzr), 1)
@mock.patch("certbot.auth_handler.AuthHandler._poll_challenges")
def test_name3_tls_sni_01_3(self, mock_poll):
self.mock_net.request_domain_challenges.side_effect = functools.partial(
gen_dom_authzr, challs=acme_util.CHALLENGES)
def test_name1_tls_sni_01_1_http_01_1_dns_1_acme_2(self, mock_poll):
self.mock_net.acme_version = 2
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 = 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, 1)
self.assertEqual(mock_poll.call_count, 1)
chall_update = mock_poll.call_args[0][0]
self.assertEqual(list(six.iterkeys(chall_update)), ["0"])
self.assertEqual(len(chall_update.values()), 1)
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
cleaned_up_achalls = self.mock_auth.cleanup.call_args[0][0]
self.assertEqual(len(cleaned_up_achalls), 1)
self.assertEqual(cleaned_up_achalls[0].typ, "tls-sni-01")
# Length of authorizations list
self.assertEqual(len(authzr), 1)
def _test_name3_tls_sni_01_3_common(self, combos):
self.mock_net.request_domain_challenges.side_effect = functools.partial(
gen_dom_authzr, challs=acme_util.CHALLENGES, combos=combos)
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)
with mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") as mock_poll:
mock_poll.side_effect = self._validate_all
authzr = self.handler.handle_authorizations(mock_order)
self.assertEqual(self.mock_net.answer_challenge.call_count, 3)
@ -167,6 +201,13 @@ class HandleAuthorizationsTest(unittest.TestCase):
self.assertEqual(len(authzr), 3)
def test_name3_tls_sni_01_3_common_acme_1(self):
self._test_name3_tls_sni_01_3_common(combos=True)
def test_name3_tls_sni_01_3_common_acme_2(self):
self.mock_net.acme_version = 2
self._test_name3_tls_sni_01_3_common(combos=False)
@mock.patch("certbot.auth_handler.AuthHandler._poll_challenges")
def test_debug_challenges(self, mock_poll):
zope.component.provideUtility(
@ -194,30 +235,44 @@ class HandleAuthorizationsTest(unittest.TestCase):
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):
authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)]
def _test_preferred_challenge_choice_common(self, combos):
authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=combos)]
mock_order = mock.MagicMock(authorizations=authzrs)
mock_poll.side_effect = self._validate_all
self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01)
self.handler.pref_challs.extend((challenges.HTTP01.typ,
challenges.DNS01.typ,))
self.handler.handle_authorizations(mock_order)
with mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") as mock_poll:
mock_poll.side_effect = self._validate_all
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):
authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)]
def test_preferred_challenge_choice_common_acme_1(self):
self._test_preferred_challenge_choice_common(combos=True)
def test_preferred_challenge_choice_common_acme_2(self):
self.mock_net.acme_version = 2
self._test_preferred_challenge_choice_common(combos=False)
def _test_preferred_challenges_not_supported_common(self, combos):
authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=combos)]
mock_order = mock.MagicMock(authorizations=authzrs)
self.handler.pref_challs.append(challenges.HTTP01.typ)
self.assertRaises(
errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
def test_preferred_challenges_not_supported_acme_1(self):
self._test_preferred_challenges_not_supported_common(combos=True)
def test_preferred_challenges_not_supported_acme_2(self):
self.mock_net.acme_version = 2
self._test_preferred_challenges_not_supported_common(combos=False)
def _validate_all(self, unused_1, unused_2):
for dom in six.iterkeys(self.handler.authzr):
azr = self.handler.authzr[dom]