From e9ffaf5793563243fe6282e15f058b1a68671d18 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Mon, 13 Apr 2015 17:33:11 -0700 Subject: [PATCH] Better authorization handling --- letsencrypt/acme/messages2.py | 13 +- letsencrypt/client/auth_handler.py | 213 ++++++++++++++++++----------- letsencrypt/client/client.py | 60 +++----- letsencrypt/client/errors.py | 10 +- letsencrypt/client/network2.py | 14 +- 5 files changed, 165 insertions(+), 145 deletions(-) diff --git a/letsencrypt/acme/messages2.py b/letsencrypt/acme/messages2.py index 38b2351b4..4d4919255 100644 --- a/letsencrypt/acme/messages2.py +++ b/letsencrypt/acme/messages2.py @@ -1,5 +1,4 @@ """ACME protocol v02 messages.""" -from letsencrypt.acme import challenges from letsencrypt.acme import fields from letsencrypt.acme import jose @@ -111,10 +110,6 @@ class Resource(jose.ImmutableMap): __slots__ = ('body', 'uri') -class ResourceBody(jose.JSONObjectWithFields): - """ACME Resource Body.""" - - class TypedResourceBody(jose.TypedJSONObjectWithFields): """ACME Resource Body with type.""" @@ -153,7 +148,7 @@ class Registration(ResourceBody): class ChallengeResource(Resource, jose.JSONObjectWithFields): """Challenge Resource. - :ivar letsencrypt.acme.messages2.ChallengeBody body: + :ivar letsencrypt.acme.messages2.Challenge body: :ivar str authzr_uri: URI found in the 'up' ``Link`` header. """ @@ -174,7 +169,6 @@ class Challenge(TypedResourceBody): """ TYPES = {} - # __slots__ = ('chall',) uri = jose.Field('uri') status = jose.Field('status', decoder=Status.from_json) validated = fields.RFC3339Field('validated', omitempty=True) @@ -184,6 +178,7 @@ class Challenge(TypedResourceBody): return jobj + class AuthorizationResource(Resource): """Authorization Resource. @@ -198,7 +193,7 @@ class Authorization(ResourceBody): """Authorization Resource Body. :ivar letsencrypt.acme.messages2.Identifier identifier: - :ivar list challenges: `list` of `ChallengeBody` + :ivar list challenges: `list` of `.Challenge` :ivar tuple combinations: Challenge combinations (`tuple` of `tuple` of `int`, as opposed to `list` of `list` from the spec). :ivar letsencrypt.acme.jose.jwk.JWK key: Public key. @@ -225,7 +220,7 @@ class Authorization(ResourceBody): @challenges.decoder def challenges(value): # pylint: disable=missing-docstring,no-self-argument - return tuple(challenges.Challenge.from_json(chall) for chall in value) + return tuple(Challenge.from_json(chall) for chall in value) @property def resolved_combinations(self): diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index 4af99761f..e72b1ce40 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -1,13 +1,9 @@ """ACME AuthHandler.""" import itertools import logging -import sys import time -import Crypto.PublicKey.RSA - from letsencrypt.acme import challenges -from letsencrypt.acme import jose from letsencrypt.acme import messages2 from letsencrypt.client import achallenges @@ -30,14 +26,14 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes messages :type network: :class:`letsencrypt.client.network2.Network` - :ivar list domains: list of str domains to get authorization - :ivar dict authkey: Authorized Keys for each domain. - values are of type :class:`letsencrypt.client.le_util.Key` + :ivar authkey: Authorized Keys for domains. + :type authkey: :class:`letsencrypt.client.le_util.Key` + :ivar dict authzr: ACME Authorization Resource dict where keys are domains. - :ivar list dv_c: Keys - DV challenges in the form of - :class:`letsencrypt.client.achallenges.Indexed` - :ivar list cont_c: Keys - Continuity challenges in the - form of :class:`letsencrypt.client.achallenges.Indexed` + :ivar list dv_c: DV challenges in the form of + :class:`letsencrypt.client.achallenges.AnnotatedChallenge` + :ivar list cont_c: Continuity challenges in the + form of :class:`letsencrypt.client.achallenges.AnnotatedChallenge` """ def __init__(self, dv_auth, cont_auth, network, authkey): @@ -45,23 +41,26 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes self.cont_auth = cont_auth self.network = network - self.domains = [] self.authkey = authkey self.authzr = dict() + # List must be used to keep responses straight. self.dv_c = [] self.cont_c = [] - def get_authorizations(self, domains, new_authz_uri): + def get_authorizations(self, domains, new_authz_uri, best_effort=False): """Retrieve all authorizations for challenges. :param set domains: Domains for authorization + :param str new_authz_uri: Location to get new authorization resources + :param bool best_effort: Whether or not all authorizations are required + (this is useful in renewal) :returns: tuple of lists of authorization resources. Takes the form of (`completed`, `failed`) rtype: tuple - :raises AuthHandlerError: If unable to retrieve all + :raises AuthorizationError: If unable to retrieve all authorizations """ @@ -69,13 +68,16 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes self.authzr[domain] = self.network.request_domain_challenges( domain, new_authz_uri) self._choose_challenges(domains) - cont_resp, dv_resp = self._get_responses() - logging.info("Ready for verification...") - # Send all Responses - self._respond(cont_resp, dv_resp) + # While there are still challenges remaining... + while self.dv_c or self.cont_c: + cont_resp, dv_resp = self._solve_challenges() + logging.info("Waiting for verification...") - return self._verify_auths() + # Send all Responses - this modifies dv_c and cont_c + self._respond(cont_resp, dv_resp, best_effort) + + return self.authzr.values() def _choose_challenges(self, domains): logging.info("Performing the following challenges:") @@ -90,7 +92,7 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes self.dv_c.extend(dom_dv_c) self.cont_c.extend(dom_cont_c) - def _get_responses(self): + def _solve_challenges(self): """Get Responses for challenges from authenticators.""" cont_resp = [] dv_resp = [] @@ -100,11 +102,11 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes if self.dv_c: dv_resp = self.dv_auth.perform(self.dv_c) # This will catch both specific types of errors. - except errors.AuthHandlerError as err: + except errors.AuthorizationError as err: logging.critical("Failure in setting up challenges.") logging.info("Attempting to clean up outstanding challenges...") self._cleanup_challenges() - raise errors.AuthHandlerError( + raise errors.AuthorizationError( "Unable to perform challenges") assert len(cont_resp) == len(self.cont_c) @@ -112,65 +114,101 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes return cont_resp, dv_resp - def _verify_auths(self): - time.sleep(6) - for domain in self.authzr: - self.authzr[domain], resp = self.network.poll(self.authzr[domain]) - if self.authzr[domain].body.status == messages2.STATUS_INVALID: - raise errors.AuthHandlerError( - "Unable to retrieve authorization for %s" % domain) - - self._cleanup_challenges() - return [self.authzr[domain] for domain in self.authzr] - - def _respond(self, cont_resp, dv_resp): + def _respond(self, cont_resp, dv_resp, best_effort): """Send/Receive confirmation of all challenges. .. note:: This method also cleans up the auth_handler state. """ + # TODO: chall_update is a dirty hack to get around acme-spec #105 chall_update = dict() self._send_responses(self.dv_c, dv_resp, chall_update) self._send_responses(self.cont_c, cont_resp, chall_update) - # self._poll_challenges(chall_update) + # Check for updated status... + self._poll_challenges(chall_update, best_effort) def _send_responses(self, achalls, resps, chall_update): - """Send responses and make sure errors are handled.""" + """Send responses and make sure errors are handled. + + :param dict chall_update: parameter that is updated to hold + authzr -> list of outstanding solved annotated challenges + + """ for achall, resp in itertools.izip(achalls, resps): + # Don't send challenges for None and False authenticator responses if resp: challr = self.network.answer_challenge(achall.chall, resp) - chall_update[achall.domain] = chall_update.get( - achall.domain, []).append(challr) + if achall.domain in chall_update: + chall_update[achall.domain].append(achall) + else: + chall_update[achall.domain] = [achall] - # def _poll_challenges(self, chall_update): - # to_check = chall_update.keys() - # completed = [] - # while to_check: - # - # def _handle_to_check(self): - # for domain in to_check: - # self.authzr[domain] = self.network.poll(self.authzr[domain]) - # if self.authzr[domain].status == messages2.STATUS_VALID: - # completed.append(domain) - # if self.authzr[domain].status == messages2.STATUS_INVALID: - # logging.error("Failed authorization for %s", domain) - # raise errors.AuthHandlerError( - # "Failed Authorization for %s" % domain) - # for challr in chall_update[domain]: - # status = self._get_status_of_chall(self.authzr[domain], challr) - # if status == messages2.STATUS_VALID: - # chall_update[domain].remove(challr) - # elif status == messages2.STATUS_INVALID: - # raise errors.AuthHandlerError( - # "Failed %s challenge for domain %s" % ( - # challr.body.chall.typ, domain)) - # - # def _get_status_of_chall(self, authzr, challr): - # for challb in authzr.challenges: - # # TODO: Use better identifiers... instead of type - # if isinstance(challb.chall, challr.body.chall): - # return challb.status + def _poll_challenges(self, chall_update, best_effort, min_sleep=3): + """Wait for all challenge results to be determined.""" + dom_to_check = set(chall_update.keys()) + comp_domains = set() + + while dom_to_check: + # TODO: Use retry-after... + time.sleep(min_sleep) + for domain in dom_to_check: + comp_challs, failed_challs = self._handle_check( + domain, chall_update[domain]) + + if len(comp_challs) == len(chall_update[domain]): + comp_domains.add(domain) + elif not failed_challs: + for chall in comp_challs: + chall_update[domain].remove(chall) + # We failed some challenges... damage control + else: + # Right now... just assume a loss and carry on... + if best_effort: + # Add to completed list... but remove authzr + del self.authzr[domain] + comp_domains.add(domain) + else: + raise errors.AuthorizationError( + "Failed Authorization procedure for %s" % domain) + + self._cleanup_challenges(comp_challs) + self._cleanup_challenges(failed_challs) + + dom_to_check -= comp_domains + comp_domains.clear() + + def _handle_check(self, domain, achalls): + """Returns tuple of ('completed', 'failed').""" + completed = [] + failed = [] + + self.authzr[domain], _ = self.network.poll(self.authzr[domain]) + if self.authzr[domain].body.status == messages2.STATUS_VALID: + return achalls, [] + + # Note: if the whole authorization is invalid, the individual failed + # challenges will be determined here... + for achall in achalls: + status = self._get_chall_status(self.authzr[domain]) + # This does nothing for challenges that have yet to be decided yet. + if status == messages2.STATUS_VALID: + completed.append(achall) + elif status == messages2.STATUS_INVALID: + failed.append(achall) + + return completed, failed + + def _get_chall_status(self, authzr, chall): + """Get the status of the challenge. + + .. warning:: This assumes only one instance of type of challenge in + each challenge resource. + + """ + for authzr_chall in authzr: + if type(authzr_chall) is type(chall): + return chall.status def _get_chall_pref(self, domain): """Return list of challenge preferences. @@ -182,14 +220,31 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes chall_prefs.extend(self.dv_auth.get_chall_pref(domain)) return chall_prefs - def _cleanup_challenges(self): - """Cleanup all configuration challenges.""" - logging.info("Cleaning up all challenges") + def _cleanup_challenges(self, achall_list=None): + """Cleanup challenges. - if self.dv_c: - self.dv_auth.cleanup(self.dv_c) - if self.cont_c: - self.cont_auth.cleanup(self.cont_c) + If achall_list is not provided, cleanup all achallenges. + + """ + logging.info("Cleaning up challenges") + + if achall_list is None: + dv_c = self.dv_c + cont_c = self.cont_c + else: + dv_c = [achall for achall in achall_list + if isinstance(achall.chall, challenges.DVChallenge)] + cont_c = [achall for achall in achall_list if isinstance( + achall.chall, challenges.ContinuityChallenge)] + + if dv_c: + self.dv_auth.cleanup(dv_c) + for achall in dv_c: + self.dv_c.remove(achall) + if cont_c: + self.cont_auth.cleanup(cont_c) + for achall in cont_c: + self.cont_c.remove(achall) def _challenge_factory(self, domain, path): """Construct Namedtuple Challenges @@ -208,8 +263,8 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes recognized """ - dv_chall = [] - cont_chall = [] + dv_chall = set() + cont_chall = set() for index in path: chall = self.authzr[domain].body.challenges[index] @@ -244,9 +299,9 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes chall.typ) if isinstance(chall, challenges.ContinuityChallenge): - cont_chall.append(achall) + cont_chall.add(achall) elif isinstance(chall, challenges.DVChallenge): - dv_chall.append(achall) + dv_chall.add(achall) return dv_chall, cont_chall @@ -272,7 +327,7 @@ def gen_challenge_path(challs, preferences, combinations): :returns: tuple of indices from ``challenges``. :rtype: tuple - :raises letsencrypt.client.errors.AuthHandlerError: If a + :raises letsencrypt.client.errors.AuthorizationError: If a path cannot be created that satisfies the CA given the preferences and combinations. @@ -318,7 +373,7 @@ def _find_smart_path(challs, preferences, combinations): msg = ("Client does not support any combination of challenges that " "will satisfy the CA.") logging.fatal(msg) - raise errors.AuthHandlerError(msg) + raise errors.AuthorizationError(msg) return best_combo diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 7d84feb10..48fc8f8e6 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -3,12 +3,10 @@ import logging import os import sys -import Crypto.PublicKey.RSA import M2Crypto from letsencrypt.acme import jose from letsencrypt.acme.jose import jwk -from letsencrypt.acme import messages from letsencrypt.client import auth_handler from letsencrypt.client import continuity_auth @@ -134,6 +132,8 @@ class Client(object): if self.regr.new_authzr_uri: authzr = self.auth_handler.get_authorizations( domains, self.regr.new_authzr_uri) + # This isn't required to be in the registration resource... + # and it isn't standardized... ugh - acme-spec #93 else: authzr = self.auth_handler.get_authorizations( domains, @@ -158,35 +158,6 @@ class Client(object): return cert_file, chain_file - def acme_challenge(self, domain): - """Handle ACME "challenge" phase. - - :returns: ACME "challenge" message. - :rtype: :class:`letsencrypt.acme.messages.Challenge` - - """ - return self.network.send_and_receive_expected( - messages.ChallengeRequest(identifier=domain), - messages.Challenge) - - def acme_certificate(self, csr_der): - """Handle ACME "certificate" phase. - - :param str csr_der: CSR in DER format. - - :returns: ACME "certificate" message. - :rtype: :class:`letsencrypt.acme.message.Certificate` - - """ - logging.info("Preparing and sending CSR...") - return self.network.send_and_receive_expected( - messages.CertificateRequest.create( - csr=jose.ComparableX509( - M2Crypto.X509.load_request_der_string(csr_der)), - key=jose.HashableRSAKey(Crypto.PublicKey.RSA.importKey( - self.authkey.pem))), - messages.Certificate) - def save_certificate(self, certr, cert_path, chain_path): # pylint: disable=no-self-use """Saves the certificate received from the ACME server. @@ -205,25 +176,30 @@ class Client(object): """ # try finally close cert_chain_abspath = None - cert_fd, cert_file = le_util.unique_file(cert_path, 0o644) - cert_fd.write(certr.body.as_pem()) - cert_fd.close() + cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) + # TODO: Except + try: + cert_file.write(certr.body.as_pem()) + finally: + cert_file.close() logging.info( - "Server issued certificate; certificate written to %s", cert_file) + "Server issued certificate; certificate written to %s", cert_path) if certr.cert_chain_uri: - # try finally close + # TODO: Except chain_cert = self.network.fetch_chain(certr.cert_chain_uri) - chain_fd, chain_fn = le_util.unique_file(chain_path, 0o644) - chain_fd.write(chain_cert.to_pem()) - chain_fd.close() + chain_file, act_chain_path = le_util.unique_file(chain_path, 0o644) + try: + chain_file.write(chain_cert.to_pem()) + finally: + chain_file.close() - logging.info("Cert chain written to %s", chain_fn) + logging.info("Cert chain written to %s", act_chain_path) # This expects a valid chain file - cert_chain_abspath = os.path.abspath(chain_fn) + cert_chain_abspath = os.path.abspath(act_chain_path) - return os.path.abspath(cert_file), cert_chain_abspath + return os.path.abspath(act_cert_path), cert_chain_abspath def deploy_certificate(self, domains, privkey, cert_file, chain_file=None): """Install certificate diff --git a/letsencrypt/client/errors.py b/letsencrypt/client/errors.py index 08201f35e..f5d9f5f44 100644 --- a/letsencrypt/client/errors.py +++ b/letsencrypt/client/errors.py @@ -18,15 +18,15 @@ class LetsEncryptReverterError(LetsEncryptClientError): # Auth Handler Errors -class AuthHandlerError(LetsEncryptClientError): - """Auth Handler error.""" +class AuthorizationError(LetsEncryptClientError): + """Authorization error.""" -class LetsEncryptContAuthError(AuthHandlerError): - """Let's Encrypt Client Authenticator error.""" +class LetsEncryptContAuthError(AuthorizationError): + """Let's Encrypt Continuity Authenticator error.""" -class LetsEncryptDvAuthError(AuthHandlerError): +class LetsEncryptDvAuthError(AuthorizationError): """Let's Encrypt DV Authenticator error.""" diff --git a/letsencrypt/client/network2.py b/letsencrypt/client/network2.py index aec6f8ddd..534cac14b 100644 --- a/letsencrypt/client/network2.py +++ b/letsencrypt/client/network2.py @@ -2,7 +2,6 @@ import datetime import heapq import httplib -import itertools import logging import time @@ -50,7 +49,6 @@ class Network(object): """ dumps = obj.json_dumps() logging.debug('Serialized JSON: %s', dumps) - print "json_dumps:", dumps return jose.JWS.sign( payload=dumps, key=self.key, alg=self.alg).json_dumps() @@ -83,9 +81,6 @@ class Network(object): jobj = None if not response.ok: - print response - print response.headers - print response.content if jobj is not None: if response_ct != cls.JSON_ERROR_CONTENT_TYPE: logging.debug( @@ -93,6 +88,7 @@ class Network(object): response_ct) try: + # TODO: This is insufficient or doesn't work as intended. raise messages2.Error.from_json(jobj) except jose.DeserializationError as error: # Couldn't deserialize JSON object @@ -297,7 +293,6 @@ class Network(object): :raises errors.UnexpectedUpdate: """ - print "sendinging challenge to:", challb.uri response = self._post(challb.uri, self._wrap_in_jws(response)) try: authzr_uri = response.links['up']['url'] @@ -307,13 +302,13 @@ class Network(object): # once the error is fixed. return challb # raise errors.NetworkError('"up" Link header missing') - challr2 = messages2.ChallengeResource( + challr = messages2.ChallengeResource( authzr_uri=authzr_uri, body=messages2.ChallengeBody.from_json(response.json())) # TODO: check that challr.uri == response.headers['Location']? - if challr2.uri != challb.uri: + if challr.uri != challb.uri: raise errors.UnexpectedUpdate(challb.uri) - return challr2 + return challr @classmethod def retry_after(cls, response, default): @@ -374,7 +369,6 @@ class Network(object): """ assert authzrs, "Authorizations list is empty" logging.debug("Requesting issuance...") - print "Requesting issuance: ", authzrs[0] # TODO: assert len(authzrs) == number of SANs req = messages2.CertificateRequest(