WIP External Account Binding (#6059)

* Rough draft of External Account Binding.

* Remove parameter --eab and namespace kid and hmac. Also add parameters to "register" subcommand.

* Refactor as much as possible of the EAB functionality into ExternalAccountBinding class.

* Remove debug line.

* Added external account binding to Directory.Meta.

* Rename to account_public_key, hmac_key and make some non-optional.
Rename command line argument to --eab-hmac-key.

* Error out when the server requires External Account Binding and the user
has not supplied kid and hmac key.

* Remove whitespace.

* Refactor a bit to make it possible to set the url argument.

* Move from_data method into client.

* Revert "Move from_data method into client."

This reverts commit 8963fae

* Refactored to use json field on Registration.

* Inherit from object according to Google Python Style Guide.

* Move to two separate ifs.

* Get tests to pass after External Account Binding additions.

* messages.py back to 100% test coverage with some EAB tests.

* .encode() this JSON key.

* Set eab parameter default values to None.

* * Remove unnecessary public key mock on most of the test.
* Restructure the directory mock to be able to mock both True and False for externalAccountRequired easily.
* Add EAB client tests.

* Move external_account_required check into BackwardsCompatibleClientV2 to be able to mock it.

* Update versions.

* Try 0.29.0.

* Revert "Try 0.29.0."

This reverts commit 5779509

* Try 0.29.0 again.

* Try this.

* Fix pylint failures.

* Add tests for external_account_required method.

* Test not needed, avoid:
************* Module acme.client_test
C:  1, 0: Too many lines in module (1258/1250) (too-many-lines)

* Move real external_account_required method into ClientV2 and pass through to it in BackwardsCompatibleClientV2.

* Handle missing meta key in server ACME directory.

* Add docstring for BackwardsCompatibleClientV2.external_account_required().

* Add tests for BackwardsCompatibleClientV2.external_account_required().

* Fix coverage for ACMEv1 code in BackwardsCompatibleClientV2.

* Disable pylint too-many-lines check for client_test.py.

* Fix versions.

* Remove whitespace that accidently snuck into an earlier commit.

* Remove these two stray whitespaces also.

* And the last couple of whitespaces.

* Add External Account Binding to changelog.

* Add dev0 suffix to setup.py.

Co-Authored-By: robaman <robert@kastel.se>

* Set to "-e acme[dev]" again.

Co-Authored-By: robaman <robert@kastel.se>

CHANGELOG.md

@@ -9,6 +9,8 @@ Certbot adheres to [Semantic Versioning](http://semver.org/).
 * Noninteractive renewals with `certbot renew` (those not started from a
   terminal) now randomly sleep 1-480 seconds before beginning work in
   order to spread out load spikes on the server side.
+* Added External Account Binding support in cli and acme library.
+  Command line arguments --eab-kid and --eab-hmac-key added.
 
 ### Changed
 

acme/acme/client.py

@@ -757,6 +757,13 @@ class ClientV2(ClientBase):
         """
         return self._revoke(cert, rsn, self.directory['revokeCert'])
 
+    def external_account_required(self):
+        """Checks if ACME server requires External Account Binding authentication."""
+        if hasattr(self.directory, 'meta') and self.directory.meta.external_account_required:
+            return True
+        else:
+            return False
+
     def _post_as_get(self, *args, **kwargs):
         """
         Send GET request using the POST-as-GET protocol if needed.
@@ -919,6 +926,15 @@ class BackwardsCompatibleClientV2(object):
         else:
             return 1
 
+    def external_account_required(self):
+        """Checks if the server requires an external account for ACMEv2 servers.
+
+        Always return False for ACMEv1 servers, as it doesn't use External Account Binding."""
+        if self.acme_version == 1:
+            return False
+        else:
+            return self.client.external_account_required()
+
 
 class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     """Wrapper around requests that signs POSTs for authentication.

acme/acme/client_test.py

@@ -284,6 +284,37 @@ class BackwardsCompatibleClientV2Test(ClientTestBase):
             client.update_registration(mock.sentinel.regr, None)
         mock_client().update_registration.assert_called_once_with(mock.sentinel.regr, None)
 
+    # newNonce present means it will pick acme_version 2
+    def test_external_account_required_true(self):
+        self.response.json.return_value = messages.Directory({
+            'newNonce': 'http://letsencrypt-test.com/acme/new-nonce',
+            'meta': messages.Directory.Meta(external_account_required=True),
+        }).to_json()
+
+        client = self._init()
+
+        self.assertTrue(client.external_account_required())
+
+    # newNonce present means it will pick acme_version 2
+    def test_external_account_required_false(self):
+        self.response.json.return_value = messages.Directory({
+            'newNonce': 'http://letsencrypt-test.com/acme/new-nonce',
+            'meta': messages.Directory.Meta(external_account_required=False),
+        }).to_json()
+
+        client = self._init()
+
+        self.assertFalse(client.external_account_required())
+
+    def test_external_account_required_false_v1(self):
+        self.response.json.return_value = messages.Directory({
+            'meta': messages.Directory.Meta(external_account_required=False),
+        }).to_json()
+
+        client = self._init()
+
+        self.assertFalse(client.external_account_required())
+
 
 class ClientTest(ClientTestBase):
     """Tests for acme.client.Client."""
@@ -823,6 +854,23 @@ class ClientV2Test(ClientTestBase):
         self.response.json.return_value = self.regr.body.update(
             contact=()).to_json()
 
+    def test_external_account_required_true(self):
+        self.client.directory = messages.Directory({
+            'meta': messages.Directory.Meta(external_account_required=True)
+        })
+
+        self.assertTrue(self.client.external_account_required())
+
+    def test_external_account_required_false(self):
+        self.client.directory = messages.Directory({
+            'meta': messages.Directory.Meta(external_account_required=False)
+        })
+
+        self.assertFalse(self.client.external_account_required())
+
+    def test_external_account_required_default(self):
+        self.assertFalse(self.client.external_account_required())
+
     def test_post_as_get(self):
         with mock.patch('acme.client.ClientV2._authzr_from_response') as mock_client:
             mock_client.return_value = self.authzr2

acme/acme/messages.py

@@ -1,6 +1,7 @@
 """ACME protocol messages."""
 import collections
 import six
+import json
 
 import josepy as jose
 
@@ -8,6 +9,7 @@ from acme import challenges
 from acme import errors
 from acme import fields
 from acme import util
+from acme import jws
 
 OLD_ERROR_PREFIX = "urn:acme:error:"
 ERROR_PREFIX = "urn:ietf:params:acme:error:"
@@ -27,6 +29,7 @@ ERROR_CODES = {
     'tls': 'The server experienced a TLS error during domain verification',
     'unauthorized': 'The client lacks sufficient authorization',
     'unknownHost': 'The server could not resolve a domain name',
+    'externalAccountRequired': 'The server requires external account binding',
 }
 
 ERROR_TYPE_DESCRIPTIONS = dict(
@@ -176,6 +179,7 @@ class Directory(jose.JSONDeSerializable):
         _terms_of_service_v2 = jose.Field('termsOfService', omitempty=True)
         website = jose.Field('website', omitempty=True)
         caa_identities = jose.Field('caaIdentities', omitempty=True)
+        external_account_required = jose.Field('externalAccountRequired', omitempty=True)
 
         def __init__(self, **kwargs):
             kwargs = dict((self._internal_name(k), v) for k, v in kwargs.items())
@@ -258,6 +262,24 @@ class ResourceBody(jose.JSONObjectWithFields):
     """ACME Resource Body."""
 
 
+class ExternalAccountBinding(object):
+    """ACME External Account Binding"""
+
+    @classmethod
+    def from_data(cls, account_public_key, kid, hmac_key, directory):
+        """Create External Account Binding Resource from contact details, kid and hmac."""
+
+        key_json = json.dumps(account_public_key.to_partial_json()).encode()
+        decoded_hmac_key = jose.b64.b64decode(hmac_key)
+        url = directory["newAccount"]
+
+        eab = jws.JWS.sign(key_json, jose.jwk.JWKOct(key=decoded_hmac_key),
+                           jose.jwa.HS256, None,
+                           url, kid)
+
+        return eab.to_partial_json()
+
+
 class Registration(ResourceBody):
     """Registration Resource Body.
 
@@ -275,12 +297,13 @@ class Registration(ResourceBody):
     status = jose.Field('status', omitempty=True)
     terms_of_service_agreed = jose.Field('termsOfServiceAgreed', omitempty=True)
     only_return_existing = jose.Field('onlyReturnExisting', omitempty=True)
+    external_account_binding = jose.Field('externalAccountBinding', omitempty=True)
 
     phone_prefix = 'tel:'
     email_prefix = 'mailto:'
 
     @classmethod
-    def from_data(cls, phone=None, email=None, **kwargs):
+    def from_data(cls, phone=None, email=None, external_account_binding=None, **kwargs):
         """Create registration resource from contact details."""
         details = list(kwargs.pop('contact', ()))
         if phone is not None:
@@ -288,6 +311,10 @@ class Registration(ResourceBody):
         if email is not None:
             details.extend([cls.email_prefix + mail for mail in email.split(',')])
         kwargs['contact'] = tuple(details)
+
+        if external_account_binding:
+            kwargs['external_account_binding'] = external_account_binding
+
         return cls(**kwargs)
 
     def _filter_contact(self, prefix):

acme/acme/messages_test.py

@@ -174,6 +174,24 @@ class DirectoryTest(unittest.TestCase):
         self.assertTrue(result)
 
 
+class ExternalAccountBindingTest(unittest.TestCase):
+    def setUp(self):
+        from acme.messages import Directory
+        self.key = jose.jwk.JWKRSA(key=KEY.public_key())
+        self.kid = "kid-for-testing"
+        self.hmac_key = "hmac-key-for-testing"
+        self.dir = Directory({
+            'newAccount': 'http://url/acme/new-account',
+        })
+
+    def test_from_data(self):
+        from acme.messages import ExternalAccountBinding
+        eab = ExternalAccountBinding.from_data(self.key, self.kid, self.hmac_key, self.dir)
+
+        self.assertEqual(len(eab), 3)
+        self.assertEqual(sorted(eab.keys()), sorted(['protected', 'payload', 'signature']))
+
+
 class RegistrationTest(unittest.TestCase):
     """Tests for acme.messages.Registration."""
 
@@ -205,6 +223,22 @@ class RegistrationTest(unittest.TestCase):
             'mailto:admin@foo.com',
         ))
 
+    def test_new_registration_from_data_with_eab(self):
+        from acme.messages import NewRegistration, ExternalAccountBinding, Directory
+        key = jose.jwk.JWKRSA(key=KEY.public_key())
+        kid = "kid-for-testing"
+        hmac_key = "hmac-key-for-testing"
+        directory = Directory({
+            'newAccount': 'http://url/acme/new-account',
+        })
+        eab = ExternalAccountBinding.from_data(key, kid, hmac_key, directory)
+        reg = NewRegistration.from_data(email='admin@foo.com', external_account_binding=eab)
+        self.assertEqual(reg.contact, (
+            'mailto:admin@foo.com',
+        ))
+        self.assertEqual(sorted(reg.external_account_binding.keys()),
+                         sorted(['protected', 'payload', 'signature']))
+
     def test_phones(self):
         self.assertEqual(('1234',), self.reg.phones)
 

certbot/cli.py

@@ -944,6 +944,18 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "specified or you already have a certificate with the same "
              "name. In the case of a name collision it will append a number "
              "like 0001 to the file path name. (default: Ask)")
+    helpful.add(
+        [None, "run", "certonly", "register"],
+        "--eab-kid", dest="eab_kid",
+        metavar="EAB_KID",
+        help="Key Identifier for External Account Binding"
+    )
+    helpful.add(
+        [None, "run", "certonly", "register"],
+        "--eab-hmac-key", dest="eab_hmac_key",
+        metavar="EAB_HMAC_KEY",
+        help="HMAC key for External Account Binding"
+    )
     helpful.add(
         [None, "run", "certonly", "manage", "delete", "certificates",
          "renew", "enhance"], "--cert-name", dest="certname",

certbot/client.py

@@ -203,9 +203,27 @@ def perform_registration(acme, config, tos_cb):
     :returns: Registration Resource.
     :rtype: `acme.messages.RegistrationResource`
     """
+
+    eab_credentials_supplied = config.eab_kid and config.eab_hmac_key
+    if eab_credentials_supplied:
+        account_public_key = acme.client.net.key.public_key()
+        eab = messages.ExternalAccountBinding.from_data(account_public_key=account_public_key,
+                                                        kid=config.eab_kid,
+                                                        hmac_key=config.eab_hmac_key,
+                                                        directory=acme.client.directory)
+    else:
+        eab = None
+
+    if acme.external_account_required():
+        if not eab_credentials_supplied:
+            msg = ("Server requires external account binding."
+                   " Please use --eab-kid and --eab-hmac-key.")
+            raise errors.Error(msg)
+
     try:
-        return acme.new_account_and_tos(messages.NewRegistration.from_data(email=config.email),
-            tos_cb)
+        newreg = messages.NewRegistration.from_data(email=config.email,
+                                                    external_account_binding=eab)
+        return acme.new_account_and_tos(newreg, tos_cb)
     except messages.Error as e:
         if e.code == "invalidEmail" or e.code == "invalidContact":
             if config.noninteractive_mode:

certbot/constants.py

@@ -68,6 +68,8 @@ CLI_DEFAULTS = dict(
     directory_hooks=True,
     reuse_key=False,
     disable_renew_updates=False,
+    eab_hmac_key=None,
+    eab_kid=None,
 
     # Subparsers
     num=None,

certbot/tests/client_test.py

@@ -13,6 +13,8 @@ from certbot import util
 
 import certbot.tests.util as test_util
 
+from josepy import interfaces
+
 KEY = test_util.load_vector("rsa512_key.pem")
 CSR_SAN = test_util.load_vector("csr-san_512.pem")
 
@@ -64,9 +66,28 @@ class RegisterTest(test_util.ConfigTestCase):
         tos_cb = mock.MagicMock()
         return register(self.config, self.account_storage, tos_cb)
 
+    @staticmethod
+    def _public_key_mock():
+        m = mock.Mock(__class__=interfaces.JSONDeSerializable)
+        m.to_partial_json.return_value = '{"a": 1}'
+        return m
+
+    @staticmethod
+    def _new_acct_dir_mock():
+        return "/acme/new-account"
+
+    @staticmethod
+    def _true_mock():
+        return True
+
+    @staticmethod
+    def _false_mock():
+        return False
+
     def test_no_tos(self):
         with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
             mock_client.new_account_and_tos().terms_of_service = "http://tos"
+            mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot.eff.handle_subscription") as mock_handle:
                 with mock.patch("certbot.account.report_new_account"):
                     mock_client().new_account_and_tos.side_effect = errors.Error
@@ -78,7 +99,8 @@ class RegisterTest(test_util.ConfigTestCase):
                     self.assertTrue(mock_handle.called)
 
     def test_it(self):
-        with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2"):
+        with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot.account.report_new_account"):
                 with mock.patch("certbot.eff.handle_subscription"):
                     self._call()
@@ -91,6 +113,7 @@ class RegisterTest(test_util.ConfigTestCase):
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
         mx_err = messages.Error.with_code('invalidContact', detail=msg)
         with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot.eff.handle_subscription") as mock_handle:
                 mock_client().new_account_and_tos.side_effect = [mx_err, mock.MagicMock()]
                 self._call()
@@ -104,6 +127,7 @@ class RegisterTest(test_util.ConfigTestCase):
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
         mx_err = messages.Error.with_code('invalidContact', detail=msg)
         with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot.eff.handle_subscription"):
                 mock_client().new_account_and_tos.side_effect = [mx_err, mock.MagicMock()]
                 self.assertRaises(errors.Error, self._call)
@@ -115,7 +139,8 @@ class RegisterTest(test_util.ConfigTestCase):
     @mock.patch("certbot.client.logger")
     def test_without_email(self, mock_logger):
         with mock.patch("certbot.eff.handle_subscription") as mock_handle:
-            with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2"):
+            with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_clnt:
+                mock_clnt().external_account_required.side_effect = self._false_mock
                 with mock.patch("certbot.account.report_new_account"):
                     self.config.email = None
                     self.config.register_unsafely_without_email = True
@@ -129,6 +154,7 @@ class RegisterTest(test_util.ConfigTestCase):
     def test_dry_run_no_staging_account(self, _rep, mock_get_email):
         """Tests dry-run for no staging account, expect account created with no email"""
         with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot.eff.handle_subscription"):
                 with mock.patch("certbot.account.report_new_account"):
                     self.config.dry_run = True
@@ -138,11 +164,53 @@ class RegisterTest(test_util.ConfigTestCase):
                     # check Certbot created an account with no email. Contact should return empty
                     self.assertFalse(mock_client().new_account_and_tos.call_args[0][0].contact)
 
+    def test_with_eab_arguments(self):
+        with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().client.directory.__getitem__ = mock.Mock(
+                side_effect=self._new_acct_dir_mock
+            )
+            mock_client().external_account_required.side_effect = self._false_mock
+            with mock.patch("certbot.eff.handle_subscription"):
+                target = "certbot.client.messages.ExternalAccountBinding.from_data"
+                with mock.patch(target) as mock_eab_from_data:
+                    self.config.eab_kid = "test-kid"
+                    self.config.eab_hmac_key = "J2OAqW4MHXsrHVa_PVg0Y-L_R4SYw0_aL1le6mfblbE"
+                    self._call()
+
+                    self.assertTrue(mock_eab_from_data.called)
+
+    def test_without_eab_arguments(self):
+        with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().external_account_required.side_effect = self._false_mock
+            with mock.patch("certbot.eff.handle_subscription"):
+                target = "certbot.client.messages.ExternalAccountBinding.from_data"
+                with mock.patch(target) as mock_eab_from_data:
+                    self.config.eab_kid = None
+                    self.config.eab_hmac_key = None
+                    self._call()
+
+                    self.assertFalse(mock_eab_from_data.called)
+
+    def test_external_account_required_without_eab_arguments(self):
+        with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().client.net.key.public_key = mock.Mock(side_effect=self._public_key_mock)
+            mock_client().external_account_required.side_effect = self._true_mock
+            with mock.patch("certbot.eff.handle_subscription"):
+                with mock.patch("certbot.client.messages.ExternalAccountBinding.from_data"):
+                    self.config.eab_kid = None
+                    self.config.eab_hmac_key = None
+
+                    self.assertRaises(errors.Error, self._call)
+
     def test_unsupported_error(self):
         from acme import messages
         msg = "Test"
         mx_err = messages.Error(detail=msg, typ="malformed", title="title")
         with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
+            mock_client().client.directory.__getitem__ = mock.Mock(
+                side_effect=self._new_acct_dir_mock
+            )
+            mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot.eff.handle_subscription") as mock_handle:
                 mock_client().new_account_and_tos.side_effect = [mx_err, mock.MagicMock()]
                 self.assertRaises(messages.Error, self._call)

local-oldest-requirements.txt

@@ -1 +1 @@
-acme[dev]==0.26.0
+-e acme[dev]

setup.py

@@ -31,7 +31,7 @@ version = meta['version']
 # specified here to avoid masking the more specific request requirements in
 # acme. See https://github.com/pypa/pip/issues/988 for more info.
 install_requires = [
-    'acme>=0.26.0',
+    'acme>=0.29.0.dev0',
     # We technically need ConfigArgParse 0.10.0 for Python 2.6 support, but
     # saying so here causes a runtime error against our temporary fork of 0.9.3
     # in which we added 2.6 support (see #2243), so we relax the requirement.