Remove references to TLS-SNI-01 outside of ACME (#7479)

This is a big part of #7214. It removes all references to TLS-SNI-01 outside of acme (and pytest.ini). Those changes will come in a subsequent PR. I thought this one was getting big enough.

* Remove references to TLS-SNI-01 in Apache plugin

* Remove references to TLS-SNI-01 from certbot-nginx

* Remove references to TLS-SNI from Certbot.

* Remove TLS-SNI reference from docs

* add certbot changelog

* Clarify test behavior
This commit is contained in:
Brad Warren 2019-10-31 10:17:29 -07:00 committed by GitHub
parent 9796128fee
commit 63d673a3e0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 64 additions and 113 deletions

View file

@ -22,6 +22,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
google-api-python-client and oauth2client.
* certbot.plugins.common.TLSSNI01 has been deprecated and will be removed in a
future release.
* CLI flags --tls-sni-01-port and --tls-sni-01-address have been removed.
* The values tls-sni and tls-sni-01 for the --preferred-challenges flag are no
longer accepted.
### Fixed

View file

@ -1296,13 +1296,13 @@ class MultipleVhostsTest(util.ApacheTest):
account_key = self.rsa512jwk
achall1 = achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.TLSSNI01(
challenges.HTTP01(
token=b"jIq_Xy1mXGN37tb4L6Xj_es58fW571ZNyXekdZzhh7Q"),
"pending"),
domain="encryption-example.demo", account_key=account_key)
achall2 = achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.TLSSNI01(
challenges.HTTP01(
token=b"uqnaPzxtrndteOqtrXb0Asl5gOJfWAnnx6QJyvcmlDU"),
"pending"),
domain="certbot.demo", account_key=account_key)

View file

@ -1,5 +0,0 @@
:mod:`certbot_apache.tls_sni_01`
------------------------------------
.. automodule:: certbot_apache.tls_sni_01
:members:

View file

@ -1107,7 +1107,7 @@ class NginxConfigurator(common.Installer):
###########################################################################
def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use
"""Return list of challenge preferences."""
return [challenges.HTTP01, challenges.TLSSNI01]
return [challenges.HTTP01]
# Entry point in main.py for performing challenges
def perform(self, achalls):

View file

@ -29,10 +29,10 @@ class NginxHttp01(common.ChallengePerformer):
:param list indices: Meant to hold indices of challenges in a
larger array. NginxHttp01 is capable of solving many challenges
at once which causes an indexing issue within NginxConfigurator
who must return all responses in order. Imagine NginxConfigurator
maintaining state about where all of the http-01 Challenges,
TLS-SNI-01 Challenges belong in the response array. This is an
optional utility.
who must return all responses in order. Imagine
NginxConfigurator maintaining state about where all of the
challenges, possibly of different types, belong in the response
array. This is an optional utility.
"""

View file

@ -97,7 +97,7 @@ class NginxConfiguratorTest(util.NginxTest):
errors.PluginError, self.config.enhance, 'myhost', 'unknown_enhancement')
def test_get_chall_pref(self):
self.assertEqual([challenges.HTTP01, challenges.TLSSNI01],
self.assertEqual([challenges.HTTP01],
self.config.get_chall_pref('myhost'))
def test_save(self):

View file

@ -73,11 +73,11 @@ class HttpPerformTest(util.NginxTest):
self.http01.add_chall(achall)
acme_responses.append(achall.response(self.account_key))
sni_responses = self.http01.perform()
http_responses = self.http01.perform()
self.assertEqual(len(sni_responses), 4)
self.assertEqual(len(http_responses), 4)
for i in six.moves.range(4):
self.assertEqual(sni_responses[i], acme_responses[i])
self.assertEqual(http_responses[i], acme_responses[i])
def test_mod_config(self):
self.http01.add_chall(self.achalls[0])

View file

@ -1,5 +0,0 @@
:mod:`certbot_nginx.tls_sni_01`
-----------------------------------
.. automodule:: certbot_nginx.tls_sni_01
:members:

View file

@ -182,9 +182,6 @@ class AuthHandler(object):
achalls.extend(self._challenge_factory(authzr, path))
if any(isinstance(achall.chall, challenges.TLSSNI01) for achall in achalls):
logger.warning("TLS-SNI-01 is deprecated, and will stop working soon.")
return achalls
def _get_chall_pref(self, domain):

View file

@ -1249,17 +1249,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis
helpful.add_deprecated_argument("--agree-dev-preview", 0)
helpful.add_deprecated_argument("--dialog", 0)
# Deprecation of tls-sni-01 related cli flags
# TODO: remove theses flags completely in few releases
class _DeprecatedTLSSNIAction(util._ShowWarning): # pylint: disable=protected-access
def __call__(self, parser, namespace, values, option_string=None):
super(_DeprecatedTLSSNIAction, self).__call__(parser, namespace, values, option_string)
namespace.https_port = values
helpful.add(
["testing", "standalone", "apache", "nginx"], "--tls-sni-01-port",
type=int, action=_DeprecatedTLSSNIAction, help=argparse.SUPPRESS)
helpful.add_deprecated_argument("--tls-sni-01-address", 1)
# Populate the command line parameters for new style enhancements
enhancements.populate_cli(helpful.add)
@ -1546,18 +1535,10 @@ def parse_preferred_challenges(pref_challs):
:raises errors.Error: if pref_challs is invalid
"""
aliases = {"dns": "dns-01", "http": "http-01", "tls-sni": "tls-sni-01"}
aliases = {"dns": "dns-01", "http": "http-01"}
challs = [c.strip() for c in pref_challs]
challs = [aliases.get(c, c) for c in challs]
# Ignore tls-sni-01 from the list, and generates a deprecation warning
# TODO: remove this option completely in few releases
if "tls-sni-01" in challs:
logger.warning('TLS-SNI-01 support is deprecated. This value is being dropped from the '
'setting of --preferred-challenges and future versions of Certbot will '
'error if it is included.')
challs = [chall for chall in challs if chall != "tls-sni-01"]
unrecognized = ", ".join(name for name in challs
if name not in challenges.Challenge.TYPES)
if unrecognized:

View file

@ -18,12 +18,10 @@ KEY = util.load_rsa_private_key('rsa512_key.pem')
# Challenges
HTTP01 = challenges.HTTP01(
token=b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+oA")
TLSSNI01 = challenges.TLSSNI01(
token=jose.b64decode(b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJyPCt92wrDoA"))
DNS01 = challenges.DNS01(token=b"17817c66b60ce2e4012dfad92657527a")
DNS01_2 = challenges.DNS01(token=b"cafecafecafecafecafecafe0feedbac")
CHALLENGES = [HTTP01, TLSSNI01, DNS01]
CHALLENGES = [HTTP01, DNS01]
def gen_combos(challbs):
@ -47,21 +45,19 @@ def chall_to_challb(chall, status): # pylint: disable=redefined-outer-name
# Pending ChallengeBody objects
TLSSNI01_P = chall_to_challb(TLSSNI01, messages.STATUS_PENDING)
HTTP01_P = chall_to_challb(HTTP01, messages.STATUS_PENDING)
DNS01_P = chall_to_challb(DNS01, messages.STATUS_PENDING)
DNS01_P_2 = chall_to_challb(DNS01_2, messages.STATUS_PENDING)
CHALLENGES_P = [HTTP01_P, TLSSNI01_P, DNS01_P]
CHALLENGES_P = [HTTP01_P, DNS01_P]
# AnnotatedChallenge objects
HTTP01_A = auth_handler.challb_to_achall(HTTP01_P, JWK, "example.com")
TLSSNI01_A = auth_handler.challb_to_achall(TLSSNI01_P, JWK, "example.net")
DNS01_A = auth_handler.challb_to_achall(DNS01_P, JWK, "example.org")
DNS01_A_2 = auth_handler.challb_to_achall(DNS01_P_2, JWK, "esimerkki.example.org")
ACHALLENGES = [HTTP01_A, TLSSNI01_A, DNS01_A]
ACHALLENGES = [HTTP01_A, DNS01_A]
def gen_authzr(authz_status, domain, challs, statuses, combos=True):

View file

@ -39,11 +39,11 @@ class ChallengeFactoryTest(unittest.TestCase):
self.assertEqual(
[achall.chall for achall in achalls], acme_util.CHALLENGES)
def test_one_tls_sni(self):
achalls = self.handler._challenge_factory(self.authzr, [1])
def test_one_http(self):
achalls = self.handler._challenge_factory(self.authzr, [0])
self.assertEqual(
[achall.chall for achall in achalls], [acme_util.TLSSNI01])
[achall.chall for achall in achalls], [acme_util.HTTP01])
def test_unrecognized(self):
authzr = acme_util.gen_authzr(
@ -73,7 +73,7 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.mock_auth = mock.MagicMock(name="ApacheConfigurator")
self.mock_auth.get_chall_pref.return_value = [challenges.TLSSNI01]
self.mock_auth.get_chall_pref.return_value = [challenges.HTTP01]
self.mock_auth.perform.side_effect = gen_auth_resp
@ -90,7 +90,7 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
def tearDown(self):
logging.disable(logging.NOTSET)
def _test_name1_tls_sni_01_1_common(self, combos):
def _test_name1_http_01_1_common(self, combos):
authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=combos)
mock_order = mock.MagicMock(authorizations=[authzr])
@ -109,44 +109,42 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.assertTrue(mock_time.sleep.call_args_list[1][0][0] > 3)
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
# Test if list first element is TLSSNI01, use typ because it is an achall
# Test if list first element is http-01, use typ because it is an achall
self.assertEqual(
self.mock_auth.cleanup.call_args[0][0][0].typ, "tls-sni-01")
self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01")
self.assertEqual(len(authzr), 1)
def test_name1_tls_sni_01_1_acme_1(self):
self._test_name1_tls_sni_01_1_common(combos=True)
def test_name1_http_01_1_acme_1(self):
self._test_name1_http_01_1_common(combos=True)
def test_name1_tls_sni_01_1_acme_2(self):
def test_name1_http_01_1_acme_2(self):
self.mock_net.acme_version = 2
self._test_name1_tls_sni_01_1_common(combos=False)
self._test_name1_http_01_1_common(combos=False)
def test_name1_tls_sni_01_1_http_01_1_dns_1_acme_1(self):
def test_name1_http_01_1_dns_1_acme_1(self):
self.mock_net.poll.side_effect = _gen_mock_on_poll()
self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01)
self.mock_auth.get_chall_pref.return_value.append(challenges.DNS01)
authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=False)
mock_order = mock.MagicMock(authorizations=[authzr])
authzr = self.handler.handle_authorizations(mock_order)
self.assertEqual(self.mock_net.answer_challenge.call_count, 3)
self.assertEqual(self.mock_net.answer_challenge.call_count, 2)
self.assertEqual(self.mock_net.poll.call_count, 1)
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
# Test if list first element is TLSSNI01, use typ because it is an achall
# Test if list first element is http-01, use typ because it is an achall
for achall in self.mock_auth.cleanup.call_args[0][0]:
self.assertTrue(achall.typ in ["tls-sni-01", "http-01", "dns-01"])
self.assertTrue(achall.typ in ["http-01", "dns-01"])
# Length of authorizations list
self.assertEqual(len(authzr), 1)
def test_name1_tls_sni_01_1_http_01_1_dns_1_acme_2(self):
def test_name1_http_01_1_dns_1_acme_2(self):
self.mock_net.acme_version = 2
self.mock_net.poll.side_effect = _gen_mock_on_poll()
self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01)
self.mock_auth.get_chall_pref.return_value.append(challenges.DNS01)
authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=False)
@ -160,12 +158,12 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
cleaned_up_achalls = self.mock_auth.cleanup.call_args[0][0]
self.assertEqual(len(cleaned_up_achalls), 1)
self.assertEqual(cleaned_up_achalls[0].typ, "tls-sni-01")
self.assertEqual(cleaned_up_achalls[0].typ, "http-01")
# Length of authorizations list
self.assertEqual(len(authzr), 1)
def _test_name3_tls_sni_01_3_common(self, combos):
def _test_name3_http_01_3_common(self, combos):
self.mock_net.request_domain_challenges.side_effect = functools.partial(
gen_dom_authzr, challs=acme_util.CHALLENGES, combos=combos)
@ -186,12 +184,12 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.assertEqual(len(authzr), 3)
def test_name3_tls_sni_01_3_common_acme_1(self):
self._test_name3_tls_sni_01_3_common(combos=True)
def test_name3_http_01_3_common_acme_1(self):
self._test_name3_http_01_3_common(combos=True)
def test_name3_tls_sni_01_3_common_acme_2(self):
def test_name3_http_01_3_common_acme_2(self):
self.mock_net.acme_version = 2
self._test_name3_tls_sni_01_3_common(combos=False)
self._test_name3_http_01_3_common(combos=False)
def test_debug_challenges(self):
zope.component.provideUtility(
@ -257,7 +255,7 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
def _test_preferred_challenges_not_supported_common(self, combos):
authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=combos)]
mock_order = mock.MagicMock(authorizations=authzrs)
self.handler.pref_challs.append(challenges.HTTP01.typ)
self.handler.pref_challs.append(challenges.DNS01.typ)
self.assertRaises(
errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
@ -283,7 +281,7 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
self.assertEqual(
self.mock_auth.cleanup.call_args[0][0][0].typ, "tls-sni-01")
self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01")
def test_answer_error(self):
self.mock_net.answer_challenge.side_effect = errors.AuthorizationError
@ -295,7 +293,7 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
self.assertEqual(
self.mock_auth.cleanup.call_args[0][0][0].typ, "tls-sni-01")
self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01")
def test_incomplete_authzr_error(self):
authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)]
@ -308,7 +306,7 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.assertTrue('Some challenges have failed.' in str(error.exception))
self.assertEqual(self.mock_auth.cleanup.call_count, 1)
self.assertEqual(
self.mock_auth.cleanup.call_args[0][0][0].typ, "tls-sni-01")
self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01")
def test_best_effort(self):
def _conditional_mock_on_poll(authzr):
@ -344,28 +342,26 @@ class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-p
self.assertTrue('All challenges have failed.' in str(error.exception))
def test_validated_challenge_not_rerun(self):
# With pending challenge, we expect the challenge to be tried, and fail.
# With a pending challenge that is not supported by the plugin, we
# expect an exception to be raised.
authzr = acme_util.gen_authzr(
messages.STATUS_PENDING, "0",
[acme_util.HTTP01],
[acme_util.DNS01],
[messages.STATUS_PENDING], False)
mock_order = mock.MagicMock(authorizations=[authzr])
self.assertRaises(
errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
# With validated challenge; we expect the challenge not be tried again, and succeed.
# With a validated challenge that is not supported by the plugin, we
# expect the challenge to not be solved again and
# handle_authorizations() to succeed.
authzr = acme_util.gen_authzr(
messages.STATUS_VALID, "0",
[acme_util.HTTP01],
[acme_util.DNS01],
[messages.STATUS_VALID], False)
mock_order = mock.MagicMock(authorizations=[authzr])
self.handler.handle_authorizations(mock_order)
@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])
def _gen_mock_on_poll(status=messages.STATUS_VALID, retry=0, wait_value=1):
state = {'count': retry}
@ -417,9 +413,9 @@ class GenChallengePathTest(unittest.TestCase):
return gen_challenge_path(challbs, preferences, combinations)
def test_common_case(self):
"""Given TLSSNI01 and HTTP01 with appropriate combos."""
challbs = (acme_util.TLSSNI01_P, acme_util.HTTP01_P)
prefs = [challenges.TLSSNI01, challenges.HTTP01]
"""Given DNS01 and HTTP01 with appropriate combos."""
challbs = (acme_util.DNS01_P, acme_util.HTTP01_P)
prefs = [challenges.DNS01, challenges.HTTP01]
combos = ((0,), (1,))
# Smart then trivial dumb path test
@ -430,8 +426,8 @@ class GenChallengePathTest(unittest.TestCase):
self.assertTrue(self._call(challbs[::-1], prefs, None))
def test_not_supported(self):
challbs = (acme_util.DNS01_P, acme_util.TLSSNI01_P)
prefs = [challenges.TLSSNI01]
challbs = (acme_util.DNS01_P, acme_util.HTTP01_P)
prefs = [challenges.HTTP01]
combos = ((0, 1),)
# smart path fails because no challs in perfs satisfies combos
@ -459,19 +455,19 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
http_01 = messages.ChallengeBody(**kwargs)
kwargs["chall"] = acme_util.TLSSNI01
tls_sni_01 = messages.ChallengeBody(**kwargs)
kwargs["chall"] = acme_util.HTTP01
http_01 = messages.ChallengeBody(**kwargs)
self.authzr1 = mock.MagicMock()
self.authzr1.body.identifier.value = 'example.com'
self.authzr1.body.challenges = [http_01, tls_sni_01]
self.authzr1.body.challenges = [http_01, http_01]
kwargs["error"] = messages.Error(typ="dnssec", detail="detail")
tls_sni_01_diff = messages.ChallengeBody(**kwargs)
http_01_diff = messages.ChallengeBody(**kwargs)
self.authzr2 = mock.MagicMock()
self.authzr2.body.identifier.value = 'foo.bar'
self.authzr2.body.challenges = [tls_sni_01_diff]
self.authzr2.body.challenges = [http_01_diff]
@test_util.patch_get_utility()
def test_same_error_and_domain(self, mock_zope):

View file

@ -249,13 +249,6 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods
expected = [challenges.HTTP01.typ, challenges.DNS01.typ]
self.assertEqual(namespace.pref_challs, expected)
# TODO: to be removed once tls-sni deprecation logic is removed
with mock.patch('certbot.cli.logger.warning') as mock_warn:
self.assertEqual(self.parse(['--preferred-challenges', 'http, tls-sni']).pref_challs,
[challenges.HTTP01.typ])
self.assertEqual(mock_warn.call_count, 1)
self.assertTrue('deprecated' in mock_warn.call_args[0][0])
short_args = ['--preferred-challenges', 'jumping-over-the-moon']
# argparse.ArgumentError makes argparse print more information
# to stderr and call sys.exit()

View file

@ -75,15 +75,10 @@ class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase):
@mock.patch('certbot.renewal.cli.set_by_cli')
def test_pref_challs_list(self, mock_set_by_cli):
mock_set_by_cli.return_value = False
# TODO: remove tls-sni and related assertions to logger.warning call once
# the deprecation logic has been removed
renewalparams = {'pref_challs': 'tls-sni, http-01, dns'.split(',')}
with mock.patch('certbot.renewal.cli.logger.warning') as mock_warn:
self._call(self.config, renewalparams)
renewalparams = {'pref_challs': 'http-01, dns'.split(',')}
self._call(self.config, renewalparams)
expected = [challenges.HTTP01.typ, challenges.DNS01.typ]
self.assertEqual(self.config.pref_challs, expected)
self.assertEqual(mock_warn.call_count, 1)
self.assertTrue('deprecated' in mock_warn.call_args[0][0])
@mock.patch('certbot.renewal.cli.set_by_cli')
def test_pref_challs_str(self, mock_set_by_cli):

View file

@ -234,7 +234,7 @@ Authenticators
Authenticators are plugins that prove control of a domain name by solving a
challenge provided by the ACME server. ACME currently defines several types of
challenges: HTTP, TLS-SNI (deprecated), TLS-ALPR, and DNS, represented by classes in `acme.challenges`.
challenges: HTTP, TLS-ALPN, and DNS, represented by classes in `acme.challenges`.
An authenticator plugin should implement support for at least one challenge type.
An Authenticator indicates which challenges it supports by implementing