diff --git a/CHANGELOG.md b/CHANGELOG.md index fa0fcd334..a5c3afdb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,15 @@ Certbot adheres to [Semantic Versioning](http://semver.org/). ### Added * `revoke` accepts `--cert-name`, and doesn't accept both `--cert-name` and `--cert-path`. +* Use the ACMEv2 newNonce endpoint when a new nonce is needed, and newNonce is available in the directory. ### Changed * Removed documentation mentions of `#letsencrypt` IRC on Freenode. * Write README to the base of (config-dir)/live directory * `--manual` will explicitly warn users that earlier challenges should remain in place when setting up subsequent challenges. +* Warn when using deprecated acme.challenges.TLSSNI01 +* Stop preferring TLS-SNI in the Apache, Nginx, and standalone plugins ### Fixed diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 2f0bf004d..a65768228 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -4,6 +4,7 @@ import functools import hashlib import logging import socket +import warnings from cryptography.hazmat.primitives import hashes # type: ignore import josepy as jose @@ -493,6 +494,11 @@ class TLSSNI01(KeyAuthorizationChallenge): # boulder#962, ietf-wg-acme#22 #n = jose.Field("n", encoder=int, decoder=int) + def __init__(self, *args, **kwargs): + warnings.warn("TLS-SNI-01 is deprecated, and will stop working soon.", + DeprecationWarning, stacklevel=2) + super(TLSSNI01, self).__init__(*args, **kwargs) + def validation(self, account_key, **kwargs): """Generate validation. diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 661d25a35..9307eb95b 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -1,5 +1,6 @@ """Tests for acme.challenges.""" import unittest +import warnings import josepy as jose import mock @@ -360,20 +361,29 @@ class TLSSNI01ResponseTest(unittest.TestCase): class TLSSNI01Test(unittest.TestCase): def setUp(self): - from acme.challenges import TLSSNI01 - self.msg = TLSSNI01( - token=jose.b64decode('a82d5ff8ef740d12881f6d3c2277ab2e')) self.jmsg = { 'type': 'tls-sni-01', 'token': 'a82d5ff8ef740d12881f6d3c2277ab2e', } + def _msg(self): + from acme.challenges import TLSSNI01 + with warnings.catch_warnings(record=True) as warn: + warnings.simplefilter("always") + msg = TLSSNI01( + token=jose.b64decode('a82d5ff8ef740d12881f6d3c2277ab2e')) + assert warn is not None # using a raw assert for mypy + self.assertTrue(len(warn) == 1) + self.assertTrue(issubclass(warn[-1].category, DeprecationWarning)) + self.assertTrue('deprecated' in str(warn[-1].message)) + return msg + def test_to_partial_json(self): - self.assertEqual(self.jmsg, self.msg.to_partial_json()) + self.assertEqual(self.jmsg, self._msg().to_partial_json()) def test_from_json(self): from acme.challenges import TLSSNI01 - self.assertEqual(self.msg, TLSSNI01.from_json(self.jmsg)) + self.assertEqual(self._msg(), TLSSNI01.from_json(self.jmsg)) def test_from_json_hashable(self): from acme.challenges import TLSSNI01 @@ -388,7 +398,7 @@ class TLSSNI01Test(unittest.TestCase): @mock.patch('acme.challenges.TLSSNI01Response.gen_cert') def test_validation(self, mock_gen_cert): mock_gen_cert.return_value = ('cert', 'key') - self.assertEqual(('cert', 'key'), self.msg.validation( + self.assertEqual(('cert', 'key'), self._msg().validation( KEY, cert_key=mock.sentinel.cert_key)) mock_gen_cert.assert_called_once_with(key=mock.sentinel.cert_key) diff --git a/acme/acme/client.py b/acme/acme/client.py index 7a8a15219..54ae58764 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -86,6 +86,8 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes """ kwargs.setdefault('acme_version', self.acme_version) + if hasattr(self.directory, 'newNonce'): + kwargs.setdefault('new_nonce_url', getattr(self.directory, 'newNonce')) return self.net.post(*args, **kwargs) def update_registration(self, regr, update=None): @@ -1103,10 +1105,15 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes else: raise errors.MissingNonce(response) - def _get_nonce(self, url): + def _get_nonce(self, url, new_nonce_url): if not self._nonces: logger.debug('Requesting fresh nonce') - self._add_nonce(self.head(url)) + if new_nonce_url is None: + response = self.head(url) + else: + # request a new nonce from the acme newNonce endpoint + response = self._check_response(self.head(new_nonce_url), content_type=None) + self._add_nonce(response) return self._nonces.pop() def post(self, *args, **kwargs): @@ -1127,8 +1134,13 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes def _post_once(self, url, obj, content_type=JOSE_CONTENT_TYPE, acme_version=1, **kwargs): - data = self._wrap_in_jws(obj, self._get_nonce(url), url, acme_version) + try: + new_nonce_url = kwargs.pop('new_nonce_url') + except KeyError: + new_nonce_url = None + data = self._wrap_in_jws(obj, self._get_nonce(url, new_nonce_url), url, acme_version) kwargs.setdefault('headers', {'Content-Type': content_type}) response = self._send_request('POST', url, data=data, **kwargs) + response = self._check_response(response, content_type=content_type) self._add_nonce(response) - return self._check_response(response, content_type=content_type) + return response diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 42d58d1ab..18b61d537 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -805,7 +805,8 @@ class ClientV2Test(ClientTestBase): def test_revoke(self): self.client.revoke(messages_test.CERT, self.rsn) self.net.post.assert_called_once_with( - self.directory["revokeCert"], mock.ANY, acme_version=2) + self.directory["revokeCert"], mock.ANY, acme_version=2, + new_nonce_url=DIRECTORY_V2['newNonce']) def test_update_registration(self): # "Instance of 'Field' has no to_json/update member" bug: @@ -1052,7 +1053,10 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.response = mock.MagicMock(ok=True, status_code=http_client.OK) self.response.headers = {} self.response.links = {} - self.checked_response = mock.MagicMock() + self.response.checked = False + self.acmev1_nonce_response = mock.MagicMock(ok=False, + status_code=http_client.METHOD_NOT_ALLOWED) + self.acmev1_nonce_response.headers = {} self.obj = mock.MagicMock() self.wrapped_obj = mock.MagicMock() self.content_type = mock.sentinel.content_type @@ -1064,13 +1068,21 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): def send_request(*args, **kwargs): # pylint: disable=unused-argument,missing-docstring + self.assertFalse("new_nonce_url" in kwargs) + method = args[0] + uri = args[1] + if method == 'HEAD' and uri != "new_nonce_uri": + response = self.acmev1_nonce_response + else: + response = self.response + if self.available_nonces: - self.response.headers = { + response.headers = { self.net.REPLAY_NONCE_HEADER: self.available_nonces.pop().decode()} else: - self.response.headers = {} - return self.response + response.headers = {} + return response # pylint: disable=protected-access self.net._send_request = self.send_request = mock.MagicMock( @@ -1082,28 +1094,39 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): # pylint: disable=missing-docstring self.assertEqual(self.response, response) self.assertEqual(self.content_type, content_type) - return self.checked_response + self.assertTrue(self.response.ok) + self.response.checked = True + return self.response def test_head(self): - self.assertEqual(self.response, self.net.head( + self.assertEqual(self.acmev1_nonce_response, self.net.head( 'http://example.com/', 'foo', bar='baz')) self.send_request.assert_called_once_with( 'HEAD', 'http://example.com/', 'foo', bar='baz') + def test_head_v2(self): + self.assertEqual(self.response, self.net.head( + 'new_nonce_uri', 'foo', bar='baz')) + self.send_request.assert_called_once_with( + 'HEAD', 'new_nonce_uri', 'foo', bar='baz') + def test_get(self): - self.assertEqual(self.checked_response, self.net.get( + self.assertEqual(self.response, self.net.get( 'http://example.com/', content_type=self.content_type, bar='baz')) + self.assertTrue(self.response.checked) self.send_request.assert_called_once_with( 'GET', 'http://example.com/', bar='baz') def test_post_no_content_type(self): self.content_type = self.net.JOSE_CONTENT_TYPE - self.assertEqual(self.checked_response, self.net.post('uri', self.obj)) + self.assertEqual(self.response, self.net.post('uri', self.obj)) + self.assertTrue(self.response.checked) def test_post(self): # pylint: disable=protected-access - self.assertEqual(self.checked_response, self.net.post( + self.assertEqual(self.response, self.net.post( 'uri', self.obj, content_type=self.content_type)) + self.assertTrue(self.response.checked) self.net._wrap_in_jws.assert_called_once_with( self.obj, jose.b64decode(self.all_nonces.pop()), "uri", 1) @@ -1135,7 +1158,7 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): def test_post_not_retried(self): check_response = mock.MagicMock() check_response.side_effect = [messages.Error.with_code('malformed'), - self.checked_response] + self.response] # pylint: disable=protected-access self.net._check_response = check_response @@ -1143,13 +1166,12 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.obj, content_type=self.content_type) def test_post_successful_retry(self): - check_response = mock.MagicMock() - check_response.side_effect = [messages.Error.with_code('badNonce'), - self.checked_response] + post_once = mock.MagicMock() + post_once.side_effect = [messages.Error.with_code('badNonce'), + self.response] # pylint: disable=protected-access - self.net._check_response = check_response - self.assertEqual(self.checked_response, self.net.post( + self.assertEqual(self.response, self.net.post( 'uri', self.obj, content_type=self.content_type)) def test_head_get_post_error_passthrough(self): @@ -1160,6 +1182,26 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.assertRaises(requests.exceptions.RequestException, self.net.post, 'uri', obj=self.obj) + def test_post_bad_nonce_head(self): + # pylint: disable=protected-access + # regression test for https://github.com/certbot/certbot/issues/6092 + bad_response = mock.MagicMock(ok=False, status_code=http_client.SERVICE_UNAVAILABLE) + self.net._send_request = mock.MagicMock() + self.net._send_request.return_value = bad_response + self.content_type = None + check_response = mock.MagicMock() + self.net._check_response = check_response + self.assertRaises(errors.ClientError, self.net.post, 'uri', + self.obj, content_type=self.content_type, acme_version=2, + new_nonce_url='new_nonce_uri') + self.assertEqual(check_response.call_count, 1) + + def test_new_nonce_uri_removed(self): + self.content_type = None + self.net.post('uri', self.obj, content_type=None, + acme_version=2, new_nonce_url='new_nonce_uri') + + class ClientNetworkSourceAddressBindingTest(unittest.TestCase): """Tests that if ClientNetwork has a source IP set manually, the underlying library has used the provided source address.""" diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index da632dc81..f431b9dab 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -2253,7 +2253,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ########################################################################### def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use """Return list of challenge preferences.""" - return [challenges.TLSSNI01, challenges.HTTP01] + return [challenges.HTTP01, challenges.TLSSNI01] def perform(self, achalls): """Perform the configuration related challenge. diff --git a/certbot-dns-cloudxns/certbot_dns_cloudxns/dns_cloudxns.py b/certbot-dns-cloudxns/certbot_dns_cloudxns/dns_cloudxns.py index 674194fee..658db6072 100644 --- a/certbot-dns-cloudxns/certbot_dns_cloudxns/dns_cloudxns.py +++ b/certbot-dns-cloudxns/certbot_dns_cloudxns/dns_cloudxns.py @@ -70,6 +70,7 @@ class _CloudXNSLexiconClient(dns_common_lexicon.LexiconClient): super(_CloudXNSLexiconClient, self).__init__() self.provider = cloudxns.Provider({ + 'provider_name': 'cloudxns', 'auth_username': api_key, 'auth_token': secret_key, 'ttl': ttl, diff --git a/certbot-dns-dnsimple/certbot_dns_dnsimple/dns_dnsimple.py b/certbot-dns-dnsimple/certbot_dns_dnsimple/dns_dnsimple.py index f3a98567e..3eb56e37c 100644 --- a/certbot-dns-dnsimple/certbot_dns_dnsimple/dns_dnsimple.py +++ b/certbot-dns-dnsimple/certbot_dns_dnsimple/dns_dnsimple.py @@ -66,6 +66,7 @@ class _DNSimpleLexiconClient(dns_common_lexicon.LexiconClient): super(_DNSimpleLexiconClient, self).__init__() self.provider = dnsimple.Provider({ + 'provider_name': 'dnssimple', 'auth_token': token, 'ttl': ttl, }) diff --git a/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/dns_dnsmadeeasy.py b/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/dns_dnsmadeeasy.py index 982edfdd3..4236ce37a 100644 --- a/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/dns_dnsmadeeasy.py +++ b/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/dns_dnsmadeeasy.py @@ -72,6 +72,7 @@ class _DNSMadeEasyLexiconClient(dns_common_lexicon.LexiconClient): super(_DNSMadeEasyLexiconClient, self).__init__() self.provider = dnsmadeeasy.Provider({ + 'provider_name': 'dnsmadeeasy', 'auth_username': api_key, 'auth_token': secret_key, 'ttl': ttl, diff --git a/certbot-dns-gehirn/certbot_dns_gehirn/dns_gehirn.py b/certbot-dns-gehirn/certbot_dns_gehirn/dns_gehirn.py index 50bfce1ae..9c35e72ab 100644 --- a/certbot-dns-gehirn/certbot_dns_gehirn/dns_gehirn.py +++ b/certbot-dns-gehirn/certbot_dns_gehirn/dns_gehirn.py @@ -73,6 +73,7 @@ class _GehirnLexiconClient(dns_common_lexicon.LexiconClient): super(_GehirnLexiconClient, self).__init__() self.provider = gehirn.Provider({ + 'provider_name': 'gehirn', 'auth_token': api_token, 'auth_secret': api_secret, 'ttl': ttl, diff --git a/certbot-dns-linode/certbot_dns_linode/dns_linode.py b/certbot-dns-linode/certbot_dns_linode/dns_linode.py index cc29ce842..01da2cf60 100644 --- a/certbot-dns-linode/certbot_dns_linode/dns_linode.py +++ b/certbot-dns-linode/certbot_dns_linode/dns_linode.py @@ -62,6 +62,7 @@ class _LinodeLexiconClient(dns_common_lexicon.LexiconClient): def __init__(self, api_key): super(_LinodeLexiconClient, self).__init__() self.provider = linode.Provider({ + 'provider_name': 'linode', 'auth_token': api_key }) diff --git a/certbot-dns-luadns/certbot_dns_luadns/dns_luadns.py b/certbot-dns-luadns/certbot_dns_luadns/dns_luadns.py index 00b62e6e1..bd6a16f69 100644 --- a/certbot-dns-luadns/certbot_dns_luadns/dns_luadns.py +++ b/certbot-dns-luadns/certbot_dns_luadns/dns_luadns.py @@ -69,6 +69,7 @@ class _LuaDNSLexiconClient(dns_common_lexicon.LexiconClient): super(_LuaDNSLexiconClient, self).__init__() self.provider = luadns.Provider({ + 'provider_name': 'luadns', 'auth_username': email, 'auth_token': token, 'ttl': ttl, diff --git a/certbot-dns-nsone/certbot_dns_nsone/dns_nsone.py b/certbot-dns-nsone/certbot_dns_nsone/dns_nsone.py index 28db126c1..5f33efbba 100644 --- a/certbot-dns-nsone/certbot_dns_nsone/dns_nsone.py +++ b/certbot-dns-nsone/certbot_dns_nsone/dns_nsone.py @@ -66,6 +66,7 @@ class _NS1LexiconClient(dns_common_lexicon.LexiconClient): super(_NS1LexiconClient, self).__init__() self.provider = nsone.Provider({ + 'provider_name': 'nsone', 'auth_token': api_key, 'ttl': ttl, }) diff --git a/certbot-dns-ovh/certbot_dns_ovh/dns_ovh.py b/certbot-dns-ovh/certbot_dns_ovh/dns_ovh.py index c4ded7748..578ee8e89 100644 --- a/certbot-dns-ovh/certbot_dns_ovh/dns_ovh.py +++ b/certbot-dns-ovh/certbot_dns_ovh/dns_ovh.py @@ -78,6 +78,7 @@ class _OVHLexiconClient(dns_common_lexicon.LexiconClient): super(_OVHLexiconClient, self).__init__() self.provider = ovh.Provider({ + 'provider_name': 'ovh', 'auth_entrypoint': endpoint, 'auth_application_key': application_key, 'auth_application_secret': application_secret, diff --git a/certbot-dns-sakuracloud/certbot_dns_sakuracloud/dns_sakuracloud.py b/certbot-dns-sakuracloud/certbot_dns_sakuracloud/dns_sakuracloud.py index 6f1c74b68..b892330f5 100644 --- a/certbot-dns-sakuracloud/certbot_dns_sakuracloud/dns_sakuracloud.py +++ b/certbot-dns-sakuracloud/certbot_dns_sakuracloud/dns_sakuracloud.py @@ -76,6 +76,7 @@ class _SakuraCloudLexiconClient(dns_common_lexicon.LexiconClient): super(_SakuraCloudLexiconClient, self).__init__() self.provider = sakuracloud.Provider({ + 'provider_name': 'sakuracloud', 'auth_token': api_token, 'auth_secret': api_secret, 'ttl': ttl, diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 0f9c43e2d..dd0bf9e8b 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -1039,7 +1039,7 @@ class NginxConfigurator(common.Installer): ########################################################################### def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use """Return list of challenge preferences.""" - return [challenges.TLSSNI01, challenges.HTTP01] + return [challenges.HTTP01, challenges.TLSSNI01] # Entry point in main.py for performing challenges def perform(self, achalls): diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 08209ad57..957588e2a 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -103,7 +103,7 @@ class NginxConfiguratorTest(util.NginxTest): errors.PluginError, self.config.enhance, 'myhost', 'unknown_enhancement') def test_get_chall_pref(self): - self.assertEqual([challenges.TLSSNI01, challenges.HTTP01], + self.assertEqual([challenges.HTTP01, challenges.TLSSNI01], self.config.get_chall_pref('myhost')) def test_save(self): diff --git a/certbot-nginx/tests/boulder-integration.sh b/certbot-nginx/tests/boulder-integration.sh index 194413f1d..2a24e645f 100755 --- a/certbot-nginx/tests/boulder-integration.sh +++ b/certbot-nginx/tests/boulder-integration.sh @@ -39,6 +39,8 @@ nginx -v reload_nginx certbot_test_nginx --domains nginx.wtf run test_deployment_and_rollback nginx.wtf +certbot_test_nginx --domains nginx.wtf run --preferred-challenges tls-sni +test_deployment_and_rollback nginx.wtf certbot_test_nginx --domains nginx2.wtf --preferred-challenges http test_deployment_and_rollback nginx2.wtf # Overlapping location block and server-block-level return 301 diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py index e7d658b25..efee49143 100644 --- a/certbot/auth_handler.py +++ b/certbot/auth_handler.py @@ -113,6 +113,12 @@ class AuthHandler(object): aauthzr.authzr, path) aauthzr.achalls.extend(aauthzr_achalls) + for aauthzr in aauthzrs: + for achall in aauthzr.achalls: + if isinstance(achall.chall, challenges.TLSSNI01): + logger.warning("TLS-SNI-01 is deprecated, and will stop working soon.") + return + def _has_challenges(self, aauthzrs): """Do we have any challenges to perform?""" return any(aauthzr.achalls for aauthzr in aauthzrs) diff --git a/certbot/plugins/dns_common_lexicon.py b/certbot/plugins/dns_common_lexicon.py index 7a97fc950..f9610b816 100644 --- a/certbot/plugins/dns_common_lexicon.py +++ b/certbot/plugins/dns_common_lexicon.py @@ -68,7 +68,12 @@ class LexiconClient(object): for domain_name in domain_name_guesses: try: - self.provider.options['domain'] = domain_name + if hasattr(self.provider, 'options'): + # For Lexicon 2.x + self.provider.options['domain'] = domain_name + else: + # For Lexicon 3.x + self.provider.domain = domain_name self.provider.authenticate() diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index cb2e69511..16f872a3f 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -114,7 +114,7 @@ class ServerManager(object): return self._instances.copy() -SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] \ +SUPPORTED_CHALLENGES = [challenges.HTTP01, challenges.TLSSNI01] \ # type: List[Type[challenges.KeyAuthorizationChallenge]] diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index 76d1df90f..e1319b614 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -327,6 +327,11 @@ class HandleAuthorizationsTest(unittest.TestCase): azr.body.combinations) aauthzrs[i] = type(aauthzr)(updated_azr, aauthzr.achalls) + @mock.patch("certbot.auth_handler.logger") + def test_tls_sni_logs(self, logger): + self._test_name1_tls_sni_01_1_common(combos=True) + self.assertTrue("deprecated" in logger.warning.call_args[0][0]) + class PollChallengesTest(unittest.TestCase): # pylint: disable=protected-access