From fcc470d0a2ad1123ad00bd2653edffc433dc2ae0 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 17 Jul 2015 14:58:06 +0000 Subject: [PATCH] Fix "reg vs new-reg" encoding problem. --- acme/acme/challenges.py | 6 +++--- acme/acme/challenges_test.py | 22 ++++++++++++++++++--- acme/acme/client.py | 22 ++++++++++----------- acme/acme/client_test.py | 16 ++++++++-------- acme/acme/fields.py | 18 ++++++++++++++++++ acme/acme/fields_test.py | 14 ++++++++++++++ acme/acme/interfaces.py | 13 ------------- acme/acme/jose/json_util.py | 2 +- acme/acme/messages.py | 37 ++++++++++++++++++++---------------- letsencrypt/client.py | 2 +- 10 files changed, 95 insertions(+), 57 deletions(-) delete mode 100644 acme/acme/interfaces.py diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index c5aec70cb..cf2577696 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -7,7 +7,7 @@ import os import requests -from acme import interfaces +from acme import fields from acme import jose from acme import other @@ -32,12 +32,12 @@ class DVChallenge(Challenge): # pylint: disable=abstract-method """Domain validation challenges.""" -class ChallengeResponse(interfaces.ClientRequestableResource, - jose.TypedJSONObjectWithFields): +class ChallengeResponse(jose.TypedJSONObjectWithFields): # _fields_to_partial_json | pylint: disable=abstract-method """ACME challenge response.""" TYPES = {} resource_type = 'challenge' + resource = fields.Resource(resource_type) @Challenge.register diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index dbb9337c2..650075257 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -48,11 +48,13 @@ class SimpleHTTPResponseTest(unittest.TestCase): path='6tbIMBC5Anhl5bOlWT5ZFA', tls=False) self.msg_https = SimpleHTTPResponse(path='6tbIMBC5Anhl5bOlWT5ZFA') self.jmsg_http = { + 'resource': 'challenge', 'type': 'simpleHttp', 'path': '6tbIMBC5Anhl5bOlWT5ZFA', 'tls': False, } self.jmsg_https = { + 'resource': 'challenge', 'type': 'simpleHttp', 'path': '6tbIMBC5Anhl5bOlWT5ZFA', 'tls': True, @@ -184,6 +186,7 @@ class DVSNIResponseTest(unittest.TestCase): s=b'\xf5\xd6\xe3\xb2]\xe0L\x0bN\x9cKJ\x14I\xa1K\xa3#\xf9\xa8' b'\xcd\x8c7\x0e\x99\x19)\xdc\xb7\xf3\x9bw') self.jmsg = { + 'resource': 'challenge', 'type': 'dvsni', 's': '9dbjsl3gTAtOnEtKFEmhS6Mj-ajNjDcOmRkp3Lfzm3c', } @@ -257,7 +260,11 @@ class RecoveryContactResponseTest(unittest.TestCase): def setUp(self): from acme.challenges import RecoveryContactResponse self.msg = RecoveryContactResponse(token='23029d88d9e123e') - self.jmsg = {'type': 'recoveryContact', 'token': '23029d88d9e123e'} + self.jmsg = { + 'resource': 'challenge', + 'type': 'recoveryContact', + 'token': '23029d88d9e123e', + } def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) @@ -305,7 +312,11 @@ class RecoveryTokenResponseTest(unittest.TestCase): def setUp(self): from acme.challenges import RecoveryTokenResponse self.msg = RecoveryTokenResponse(token='23029d88d9e123e') - self.jmsg = {'type': 'recoveryToken', 'token': '23029d88d9e123e'} + self.jmsg = { + 'resource': 'challenge', + 'type': 'recoveryToken', + 'token': '23029d88d9e123e' + } def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) @@ -455,11 +466,13 @@ class ProofOfPossessionResponseTest(unittest.TestCase): signature=signature) self.jmsg_to = { + 'resource': 'challenge', 'type': 'proofOfPossession', 'nonce': 'eET5udtV7aoX8Xl8gYiZIA', 'signature': signature, } self.jmsg_from = { + 'resource': 'challenge', 'type': 'proofOfPossession', 'nonce': 'eET5udtV7aoX8Xl8gYiZIA', 'signature': signature.to_json(), @@ -505,7 +518,10 @@ class DNSResponseTest(unittest.TestCase): def setUp(self): from acme.challenges import DNSResponse self.msg = DNSResponse() - self.jmsg = {'type': 'dns'} + self.jmsg = { + 'resource': 'challenge', + 'type': 'dns', + } def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) diff --git a/acme/acme/client.py b/acme/acme/client.py index 113042907..8e2426b96 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -1,7 +1,6 @@ """ACME client API.""" import datetime import heapq -import json import logging import time @@ -71,8 +70,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes def register(self, new_reg=None): """Register. - :param contact: Contact list, as accepted by `.Registration` - :type contact: `tuple` + :param .NewRegistration new_reg: :returns: Registration Resource. :rtype: `.RegistrationResource` @@ -80,7 +78,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes :raises .UnexpectedUpdate: """ - new_reg = messages.Registration() if new_reg is None else new_reg + new_reg = messages.NewRegistration() if new_reg is None else new_reg + assert isinstance(new_reg, messages.NewRegistration) response = self.net.post(self.new_reg_uri, new_reg) # TODO: handle errors @@ -105,7 +104,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes :rtype: `.RegistrationResource` """ - response = self.net.post(regr.uri, regr.body) + response = self.net.post( + regr.uri, messages.UpdateRegistration(**dict(regr.body))) # TODO: Boulder returns httplib.ACCEPTED #assert response.status_code == httplib.OK @@ -164,7 +164,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes :rtype: `.AuthorizationResource` """ - new_authz = messages.Authorization(identifier=identifier) + new_authz = messages.NewAuthorization(identifier=identifier) response = self.net.post(new_authzr_uri, new_authz) # TODO: handle errors assert response.status_code == http_client.CREATED @@ -451,17 +451,15 @@ class ClientNetwork(object): .. todo:: Implement ``acmePath``. - :param .ClientRequestableResource obj: + :param .JSONDeSerializable obj: :param bytes nonce: :rtype: `.JWS` """ - jobj = obj.to_json() - jobj['resource'] = obj.resource_type - dumps = json.dumps(jobj).encode() - logger.debug('Serialized JSON: %s', dumps) + jobj = obj.json_dumps().encode() + logger.debug('Serialized JSON: %s', jobj) return jws.JWS.sign( - payload=dumps, key=self.key, alg=self.alg, nonce=nonce).json_dumps() + payload=jobj, key=self.key, alg=self.alg, nonce=nonce).json_dumps() @classmethod def _check_response(cls, response, content_type=None): diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 8e731febc..97e0b9662 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -45,6 +45,7 @@ class ClientTest(unittest.TestCase): self.contact = ('mailto:cert-admin@example.com', 'tel:+12025551212') reg = messages.Registration( contact=self.contact, key=KEY.public_key(), recovery_token='t') + self.new_reg = messages.NewRegistration(**dict(reg)) self.regr = messages.RegistrationResource( body=reg, uri='https://www.letsencrypt-demo.org/acme/reg/1', new_authzr_uri='https://www.letsencrypt-demo.org/acme/new-reg', @@ -82,19 +83,19 @@ class ClientTest(unittest.TestCase): 'terms-of-service': {'url': self.regr.terms_of_service}, }) - self.assertEqual(self.regr, self.client.register(self.regr.body)) + self.assertEqual(self.regr, self.client.register(self.new_reg)) # TODO: test POST call arguments # TODO: split here and separate test reg_wrong_key = self.regr.body.update(key=KEY2.public_key()) self.response.json.return_value = reg_wrong_key.to_json() self.assertRaises( - errors.UnexpectedUpdate, self.client.register, self.regr.body) + errors.UnexpectedUpdate, self.client.register, self.new_reg) def test_register_missing_next(self): self.response.status_code = http_client.CREATED self.assertRaises( - errors.ClientError, self.client.register, self.regr.body) + errors.ClientError, self.client.register, self.new_reg) def test_update_registration(self): # "Instance of 'Field' has no to_json/update member" bug: @@ -102,6 +103,7 @@ class ClientTest(unittest.TestCase): self.response.headers['Location'] = self.regr.uri self.response.json.return_value = self.regr.body.to_json() self.assertEqual(self.regr, self.client.update_registration(self.regr)) + # TODO: test POST call arguments # TODO: split here and separate test self.response.json.return_value = self.regr.body.update( @@ -369,9 +371,8 @@ class ClientNetworkTest(unittest.TestCase): self.assertTrue(self.net.verify_ssl is self.verify_ssl) def test_wrap_in_jws(self): - class MockClientRequestableResource(jose.JSONDeSerializable): + class MockJSONDeSerializable(jose.JSONDeSerializable): # pylint: disable=missing-docstring - resource_type = 'mock' def __init__(self, value): self.value = value def to_partial_json(self): @@ -381,10 +382,9 @@ class ClientNetworkTest(unittest.TestCase): pass # pragma: no cover # pylint: disable=protected-access jws_dump = self.net._wrap_in_jws( - MockClientRequestableResource('foo'), nonce=b'Tg') + MockJSONDeSerializable('foo'), nonce=b'Tg') jws = acme_jws.JWS.json_loads(jws_dump) - self.assertEqual(json.loads(jws.payload.decode()), - {'foo': 'foo', 'resource': 'mock'}) + self.assertEqual(json.loads(jws.payload.decode()), {'foo': 'foo'}) self.assertEqual(jws.signature.combined.nonce, b'Tg') def test_check_response_not_ok_jobj_no_error(self): diff --git a/acme/acme/fields.py b/acme/acme/fields.py index 3af336fe5..6943563d4 100644 --- a/acme/acme/fields.py +++ b/acme/acme/fields.py @@ -23,3 +23,21 @@ class RFC3339Field(jose.Field): return pyrfc3339.parse(value) except ValueError as error: raise jose.DeserializationError(error) + + +class Resource(jose.Field): + """Resource MITM field.""" + + def __init__(self, resource_type, *args, **kwargs): + self.resource_type = resource_type + super(Resource, self).__init__( + # TODO: omitempty used only to trick + # JSONObjectWithFieldsMeta._defaults..., server implementation + 'resource', default=resource_type, *args, **kwargs) + + def decode(self, value): + if value != self.resource_type: + raise jose.DeserializationError( + 'Wrong resource type: {0} instead of {1}'.format( + value, self.resource_type)) + return value diff --git a/acme/acme/fields_test.py b/acme/acme/fields_test.py index 40f0ad59b..e95c450ce 100644 --- a/acme/acme/fields_test.py +++ b/acme/acme/fields_test.py @@ -35,5 +35,19 @@ class RFC3339FieldTest(unittest.TestCase): jose.DeserializationError, RFC3339Field.default_decoder, '') +class ResourceTest(unittest.TestCase): + """Tests for acme.fields.Resource.""" + + def setUp(self): + from acme.fields import Resource + self.field = Resource('x') + + def test_decode_good(self): + self.assertEqual('x', self.field.decode('x')) + + def test_decode_wrong(self): + self.assertRaises(jose.DeserializationError, self.field.decode, 'y') + + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/acme/acme/interfaces.py b/acme/acme/interfaces.py deleted file mode 100644 index 39078f463..000000000 --- a/acme/acme/interfaces.py +++ /dev/null @@ -1,13 +0,0 @@ -"""ACME interfaces.""" -from acme import jose - - -class ClientRequestableResource(jose.JSONDeSerializable): - """Resource that can be requested by client. - - :ivar unicode resource_type: ACME resource identifier used in client - HTTPS requests in order to protect against MITM. - - """ - # pylint: disable=abstract-method - resource_type = NotImplemented diff --git a/acme/acme/jose/json_util.py b/acme/acme/jose/json_util.py index c531efd9d..fec13aa26 100644 --- a/acme/acme/jose/json_util.py +++ b/acme/acme/jose/json_util.py @@ -214,7 +214,7 @@ class JSONObjectWithFields(util.ImmutableMap, interfaces.JSONDeSerializable): def _defaults(cls): """Get default fields values.""" return dict([(slot, field.default) for slot, field - in six.iteritems(cls._fields) if field.omitempty]) + in six.iteritems(cls._fields)]) def __init__(self, **kwargs): # pylint: disable=star-args diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 1ffdc48cc..39594cbda 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -5,7 +5,6 @@ from six.moves.urllib import parse as urllib_parse # pylint: disable=import-err from acme import challenges from acme import fields -from acme import interfaces from acme import jose @@ -151,7 +150,7 @@ class ResourceBody(jose.JSONObjectWithFields): """ACME Resource Body.""" -class Registration(interfaces.ClientRequestableResource, ResourceBody): +class Registration(ResourceBody): """Registration Resource Body. :ivar acme.jose.jwk.JWK key: Public key. @@ -161,8 +160,6 @@ class Registration(interfaces.ClientRequestableResource, ResourceBody): :ivar unicode agreement: """ - resource_type = 'new-reg' - # on new-reg key server ignores 'key' and populates it based on # JWS.signature.combined.jwk key = jose.Field('key', omitempty=True, decoder=jose.JWK.from_json) @@ -199,9 +196,17 @@ class Registration(interfaces.ClientRequestableResource, ResourceBody): """All emails found in the ``contact`` field.""" return self._filter_contact(self.email_prefix) +class NewRegistration(Registration): + """New registration.""" + resource_type = 'new-reg' + resource = fields.Resource(resource_type) -class RegistrationResource(interfaces.ClientRequestableResource, - ResourceWithURI): +class UpdateRegistration(Registration): + """Update registration.""" + resource_type = 'reg' + resource = fields.Resource(resource_type) + +class RegistrationResource(ResourceWithURI): """Registration Resource. :ivar acme.messages.Registration body: @@ -209,7 +214,6 @@ class RegistrationResource(interfaces.ClientRequestableResource, :ivar unicode terms_of_service: URL for the CA TOS. """ - resource_type = 'reg' body = jose.Field('body', decoder=Registration.from_json) new_authzr_uri = jose.Field('new_authzr_uri') terms_of_service = jose.Field('terms_of_service', omitempty=True) @@ -272,7 +276,7 @@ class ChallengeResource(Resource): return self.body.uri # pylint: disable=no-member -class Authorization(interfaces.ClientRequestableResource, ResourceBody): +class Authorization(ResourceBody): """Authorization Resource Body. :ivar acme.messages.Identifier identifier: @@ -283,7 +287,6 @@ class Authorization(interfaces.ClientRequestableResource, ResourceBody): :ivar datetime.datetime expires: """ - resource_type = 'new-authz' identifier = jose.Field('identifier', decoder=Identifier.from_json) challenges = jose.Field('challenges', omitempty=True) combinations = jose.Field('combinations', omitempty=True) @@ -305,6 +308,10 @@ class Authorization(interfaces.ClientRequestableResource, ResourceBody): return tuple(tuple(self.challenges[idx] for idx in combo) for combo in self.combinations) +class NewAuthorization(Authorization): + """New authorization.""" + resource_type = 'new-authz' + resource = fields.Resource(resource_type) class AuthorizationResource(ResourceWithURI): """Authorization Resource. @@ -317,8 +324,7 @@ class AuthorizationResource(ResourceWithURI): new_cert_uri = jose.Field('new_cert_uri') -class CertificateRequest(interfaces.ClientRequestableResource, - jose.JSONObjectWithFields): +class CertificateRequest(jose.JSONObjectWithFields): """ACME new-cert request. :ivar acme.jose.util.ComparableX509 csr: @@ -327,12 +333,12 @@ class CertificateRequest(interfaces.ClientRequestableResource, """ resource_type = 'new-cert' + resource = fields.Resource(resource_type) csr = jose.Field('csr', decoder=jose.decode_csr, encoder=jose.encode_csr) authorizations = jose.Field('authorizations', decoder=tuple) -class CertificateResource(interfaces.ClientRequestableResource, - ResourceWithURI): +class CertificateResource(ResourceWithURI): """Certificate Resource. :ivar acme.jose.util.ComparableX509 body: @@ -341,13 +347,11 @@ class CertificateResource(interfaces.ClientRequestableResource, :ivar tuple authzrs: `tuple` of `AuthorizationResource`. """ - resource_type = 'cert' cert_chain_uri = jose.Field('cert_chain_uri') authzrs = jose.Field('authzrs') -class Revocation(interfaces.ClientRequestableResource, - jose.JSONObjectWithFields): +class Revocation(jose.JSONObjectWithFields): """Revocation message. :ivar .ComparableX509 certificate: `OpenSSL.crypto.X509` wrapped in @@ -355,6 +359,7 @@ class Revocation(interfaces.ClientRequestableResource, """ resource_type = 'revoke-cert' + resource = fields.Resource(resource_type) certificate = jose.Field( 'certificate', decoder=jose.decode_cert, encoder=jose.encode_cert) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index a513ff797..e8dd08c8e 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -88,7 +88,7 @@ def register(config, account_storage, tos_cb=None): backend=default_backend()))) acme = _acme_from_config_key(config, key) # TODO: add phone? - regr = acme.register(messages.Registration.from_data(email=config.email)) + regr = acme.register(messages.NewRegistration.from_data(email=config.email)) if regr.terms_of_service is not None: if tos_cb is not None and not tos_cb(regr):