dns-cloudflare: Implement limited-scope API Tokens (#7583)

A while ago Cloudflare added support for limited-scope API Tokens in place of using a global API key, but support for them in cloudflare/python-cloudflare took a while to get through.

In summary, this PR:
- Implements token functionality through the INI file parameter `dns_cloudflare_api_token` (in addition to the traditional `dns_cloudflare_email` and `dns_cloudflare_api_key`). This needed a more advanced parameter validator than the built in `required_variables` mechanism.
- Updates the docs to reflect the new option, needed token permissions, and version details of the `cloudflare` module

* Update python-cloudflare version

* Add Cloudflare API Token support to certbot-dns-cloudflare

* Add token-specific errors to certbot-dns-cloudflare

* Tidy up certbot-dns-cloudflare

* Implement Cloudflare API Tokens in testing for certbot-dns-cloudflare(needs work)

* Further tidying of certbot-dns-cloudflare

* Update CHANGELOG with Cloudflare API Tokens implementation

* Improve testing of certbot-dns-cloudflare

* Improve certbot-dns-cloudflare test formatting

* Further improve testing for certbot-dns-cloudflare

* Change needed permissions for token

* Add documentation regarding python-cloudflare version

* Fix changelog, references to python-cloudflare and docs

* Fix behaviour when domain does not match cloudflare root domain. Improve error handling.

* Improve testing

* Improve hints and error handling

AUTHORS.md

@@ -36,6 +36,7 @@ Authors
 * [Brad Warren](https://github.com/bmw)
 * [Brandon Kraft](https://github.com/kraftbj)
 * [Brandon Kreisel](https://github.com/kraftbj)
+* [Cameron Steel](https://github.com/Tugzrida)
 * [Ceesjan Luiten](https://github.com/quinox)
 * [Chad Whitacre](https://github.com/whit537)
 * [Chhatoi Pritam Baral](https://github.com/pritambaral)

certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py

@@ -22,17 +22,40 @@ Credentials
 
 Use of this plugin requires a configuration file containing Cloudflare API
 credentials, obtained from your Cloudflare
-`account page <https://www.cloudflare.com/a/account/my-account>`_. This plugin
+`account page <https://dash.cloudflare.com/profile/api-tokens>`_.
-does not currently support Cloudflare's "API Tokens", so please ensure you use
+
-the "Global API Key" for authentication.
+Previously, Cloudflare's "Global API Key" was used for authentication, however
+this key can access the entire Cloudflare API for all domains in your account,
+meaning it could cause a lot of damage if leaked.
+
+Cloudflare's newer API Tokens can be restricted to specific domains and
+operations, and are therefore now the recommended authentication option.
+
+However, due to some shortcomings in Cloudflare's implementation of Tokens,
+Tokens created for Certbot currently require ``Zone:Zone:Read`` and ``Zone:DNS:Edit``
+permissions for **all** zones in your account. While this is not ideal, your Token
+will still have fewer permission than the Global key, so it's still worth doing.
+Hopefully Cloudflare will improve this in the future.
+
+Using Cloudflare Tokens also requires at least version 2.3.1 of the ``cloudflare``
+python module. If the version that automatically installed with this plugin is
+older than that, and you can't upgrade it on your system, you'll have to stick to
+the Global key.
 
 .. code-block:: ini
-   :name: credentials.ini
+   :name: certbot_cloudflare_token.ini
-   :caption: Example credentials file:
+   :caption: Example credentials file using restricted API Token (recommended):
+
+   # Cloudflare API token used by Certbot
+   dns_cloudflare_api_token = 0123456789abcdef0123456789abcdef01234567
+
+.. code-block:: ini
+   :name: certbot_cloudflare_key.ini
+   :caption: Example credentials file using Global API Key (not recommended):
 
    # Cloudflare API credentials used by Certbot
    dns_cloudflare_email = cloudflare@example.com
-   dns_cloudflare_api_key = 0123456789abcdef0123456789abcdef01234567
+   dns_cloudflare_api_key = 0123456789abcdef0123456789abcdef01234
 
 The path to this file can be provided interactively or using the
 ``--dns-cloudflare-credentials`` command-line argument. Certbot records the path

certbot-dns-cloudflare/certbot_dns_cloudflare/_internal/dns_cloudflare.py

@@ -4,6 +4,10 @@ import logging
 import CloudFlare
 import zope.interface
 
+from acme.magic_typing import Any
+from acme.magic_typing import Dict
+from acme.magic_typing import List
+
 from certbot import errors
 from certbot import interfaces
 from certbot.plugins import dns_common
@@ -38,14 +42,35 @@ class Authenticator(dns_common.DNSAuthenticator):
         return 'This plugin configures a DNS TXT record to respond to a dns-01 challenge using ' + \
                'the Cloudflare API.'
 
+    def _validate_credentials(self, credentials):
+        token = credentials.conf('api-token')
+        email = credentials.conf('email')
+        key = credentials.conf('api-key')
+        if token:
+            if email or key:
+                raise errors.PluginError('{}: dns_cloudflare_email and dns_cloudflare_api_key are '
+                                         'not needed when using an API Token'
+                                         .format(credentials.confobj.filename))
+        elif email or key:
+            if not email:
+                raise errors.PluginError('{}: dns_cloudflare_email is required when using a Global '
+                                         'API Key. (should be email address associated with '
+                                         'Cloudflare account)'.format(credentials.confobj.filename))
+            if not key:
+                raise errors.PluginError('{}: dns_cloudflare_api_key is required when using a '
+                                         'Global API Key. (see {})'
+                                         .format(credentials.confobj.filename, ACCOUNT_URL))
+        else:
+            raise errors.PluginError('{}: Either dns_cloudflare_api_token (recommended), or '
+                                     'dns_cloudflare_email and dns_cloudflare_api_key are required.'
+                                     ' (see {})'.format(credentials.confobj.filename, ACCOUNT_URL))
+
     def _setup_credentials(self):
         self.credentials = self._configure_credentials(
             'credentials',
             'Cloudflare credentials INI file',
-            {
+            None,
-                'email': 'email address associated with Cloudflare account',
+            self._validate_credentials
-                'api-key': 'API key for Cloudflare account, obtained from {0}'.format(ACCOUNT_URL)
-            }
         )
 
     def _perform(self, domain, validation_name, validation):
@@ -55,6 +80,8 @@ class Authenticator(dns_common.DNSAuthenticator):
         self._get_cloudflare_client().del_txt_record(domain, validation_name, validation)
 
     def _get_cloudflare_client(self):
+        if self.credentials.conf('api-token'):
+            return _CloudflareClient(None, self.credentials.conf('api-token'))
         return _CloudflareClient(self.credentials.conf('email'), self.credentials.conf('api-key'))
 
 
@@ -88,8 +115,15 @@ class _CloudflareClient(object):
             logger.debug('Attempting to add record to zone %s: %s', zone_id, data)
             self.cf.zones.dns_records.post(zone_id, data=data)  # zones | pylint: disable=no-member
         except CloudFlare.exceptions.CloudFlareAPIError as e:
+            code = int(e)
+            hint = None
+
+            if code == 9109:
+                hint = 'Does your API token have "Zone:DNS:Edit" permissions?'
+
             logger.error('Encountered CloudFlareAPIError adding TXT record: %d %s', e, e)
-            raise errors.PluginError('Error communicating with the Cloudflare API: {0}'.format(e))
+            raise errors.PluginError('Error communicating with the Cloudflare API: {0}{1}'
+                                     .format(e, ' ({0})'.format(hint) if hint else ''))
 
         record_id = self._find_txt_record_id(zone_id, record_name, record_content)
         logger.debug('Successfully added TXT record with record_id: %s', record_id)
@@ -139,6 +173,8 @@ class _CloudflareClient(object):
         """
 
         zone_name_guesses = dns_common.base_domain_name_guesses(domain)
+        zones = []  # type: List[Dict[str, Any]]
+        code = msg = None
 
         for zone_name in zone_name_guesses:
             params = {'name': zone_name,
@@ -148,16 +184,26 @@ class _CloudflareClient(object):
                 zones = self.cf.zones.get(params=params)  # zones | pylint: disable=no-member
             except CloudFlare.exceptions.CloudFlareAPIError as e:
                 code = int(e)
+                msg = str(e)
                 hint = None
 
                 if code == 6003:
-                    hint = 'Did you copy your entire API key?'
+                    hint = ('Did you copy your entire API token/key? To use Cloudflare tokens, '
+                            'you\'ll need the python package cloudflare>=2.3.1.{}'
+                    .format(' This certbot is running cloudflare ' + str(CloudFlare.__version__)
+                    if hasattr(CloudFlare, '__version__') else ''))
                 elif code == 9103:
-                    hint = 'Did you enter the correct email address?'
+                    hint = 'Did you enter the correct email address and Global key?'
+                elif code == 9109:
+                    hint = 'Did you enter a valid Cloudflare Token?'
 
-                raise errors.PluginError('Error determining zone_id: {0} {1}. Please confirm that '
+                if hint:
-                                         'you have supplied valid Cloudflare API credentials.{2}'
+                    raise errors.PluginError('Error determining zone_id: {0} {1}. Please confirm '
-                                         .format(code, e, ' ({0})'.format(hint) if hint else ''))
+                                  'that you have supplied valid Cloudflare API credentials. ({2})'
+                                                                         .format(code, msg, hint))
+                else:
+                    logger.debug('Unrecognised CloudFlareAPIError while finding zone_id: %d %s. '
+                                 'Continuing with next zone guess...', e, e)
 
             if zones:
                 zone_id = zones[0]['id']
@@ -166,8 +212,9 @@ class _CloudflareClient(object):
 
         raise errors.PluginError('Unable to determine zone_id for {0} using zone names: {1}. '
                                 'Please confirm that the domain name has been entered correctly '
-                                 'and is already associated with the supplied Cloudflare account.'
+                                'and is already associated with the supplied Cloudflare account.{2}'
-                                 .format(domain, zone_name_guesses))
+                                .format(domain, zone_name_guesses, ' The error from Cloudflare was:'
+                                ' {0} {1}'.format(code, msg) if code is not None else ''))
 
     def _find_txt_record_id(self, zone_id, record_name, record_content):
         """

certbot-dns-cloudflare/tests/dns_cloudflare_test.py

@@ -12,6 +12,9 @@ from certbot.plugins.dns_test_common import DOMAIN
 from certbot.tests import util as test_util
 
 API_ERROR = CloudFlare.exceptions.CloudFlareAPIError(1000, '', '')
+
+API_TOKEN = 'an-api-token'
+
 API_KEY = 'an-api-key'
 EMAIL = 'example@example.com'
 
@@ -49,6 +52,50 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         expected = [mock.call.del_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY)]
         self.assertEqual(expected, self.mock_client.mock_calls)
 
+    def test_api_token(self):
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN},
+                              self.config.cloudflare_credentials)
+        self.auth.perform([self.achall])
+
+        expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)]
+        self.assertEqual(expected, self.mock_client.mock_calls)
+
+    def test_no_creds(self):
+        dns_test_common.write({}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+    def test_missing_email_or_key(self):
+        dns_test_common.write({"cloudflare_api_key": API_KEY}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+        dns_test_common.write({"cloudflare_email": EMAIL}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+    def test_email_or_key_with_token(self):
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN, "cloudflare_email": EMAIL},
+                              self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN, "cloudflare_api_key": API_KEY},
+                              self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN, "cloudflare_email": EMAIL,
+                               "cloudflare_api_key": API_KEY}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
 
 class CloudflareClientTest(unittest.TestCase):
     record_name = "foo"
@@ -83,7 +130,7 @@ class CloudflareClientTest(unittest.TestCase):
     def test_add_txt_record_error(self):
         self.cf.zones.get.return_value = [{'id': self.zone_id}]
 
-        self.cf.zones.dns_records.post.side_effect = API_ERROR
+        self.cf.zones.dns_records.post.side_effect = CloudFlare.exceptions.CloudFlareAPIError(9109, '', '')
 
         self.assertRaises(
             errors.PluginError,
@@ -106,6 +153,25 @@ class CloudflareClientTest(unittest.TestCase):
             self.cloudflare_client.add_txt_record,
             DOMAIN, self.record_name, self.record_content, self.record_ttl)
 
+    def test_add_txt_record_bad_creds(self):
+        self.cf.zones.get.side_effect = CloudFlare.exceptions.CloudFlareAPIError(6003, '', '')
+        self.assertRaises(
+            errors.PluginError,
+            self.cloudflare_client.add_txt_record,
+            DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
+        self.cf.zones.get.side_effect = CloudFlare.exceptions.CloudFlareAPIError(9103, '', '')
+        self.assertRaises(
+            errors.PluginError,
+            self.cloudflare_client.add_txt_record,
+            DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
+        self.cf.zones.get.side_effect = CloudFlare.exceptions.CloudFlareAPIError(9109, '', '')
+        self.assertRaises(
+            errors.PluginError,
+            self.cloudflare_client.add_txt_record,
+            DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
     def test_del_txt_record(self):
         self.cf.zones.get.return_value = [{'id': self.zone_id}]
         self.cf.zones.dns_records.get.return_value = [{'id': self.record_id}]

certbot/CHANGELOG.md

@@ -6,7 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-*
+* Added support for Cloudflare's limited-scope API Tokens
 
 ### Changed
 

tools/dev_constraints.txt

@@ -16,7 +16,7 @@ bcrypt==3.1.6
 boto3==1.11.7
 botocore==1.14.7
 cached-property==1.5.1
-cloudflare==1.5.1
+cloudflare==2.3.1
 codecov==2.0.15
 configparser==3.7.4
 contextlib2==0.6.0.post1