Remove tls-sni related flags in cli. Add a deprecation warning instead. (#6853)

This PR is a part of the tls-sni-01 removal plan described in #6849.

This PR removes --tls-sni-01-port, --tls-sni-01-address and tls-sni-01/tls-sni options from --preferred-challenges. They are replace by deprecation warning, indicating that these options will be removed soon.

This deprecation, instead of complete removal, is done to avoid certbot instances to hard fail if some automated scripts still use these flags for some users.

Once this PR lands, we can remove completely theses flags in one or two release.

* Remove tls-sni related flags in cli. Add a deprecation warning instead.

* Adapt tests to cli and renewal towards tls-sni flags deprecation

* Add https_port option. Make tls_sni_01_port show a deprecation warning, but silently modify https_port if set

* Migrate last items

* Fix lint

* Update certbot/cli.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Ensure to remove all occurences of tls-sni-01

* Remove unused parameter

* Revert modifications on cli-help.txt

* Use logger.warning instead of sys.stderr

* Update the logger warning message

* Remove standalone_supported_challenges option.

* Fix order of preferred-challenges

* Remove supported_challenges property

* Fix some tests

* Fix lint

* Fix tests

* Add a changelog

* Clean code, fix test

* Update CI

* Reload

* No hard date for tls-sni removal

* Remove useless cast to list

* Update certbot/tests/renewal_test.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Add entry to the changelog

* Add entry to the changelog
This commit is contained in:
Adrien Ferrand 2019-03-27 01:46:32 +01:00 committed by Brad Warren
parent a27bd28b39
commit 821bec6997
21 changed files with 90 additions and 173 deletions

View file

@ -8,10 +8,18 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
* Fedora 29+ is now supported by certbot-auto. Since Python 2.x is on a deprecation
path in Fedora, certbot-auto will install and use Python 3.x on Fedora 29+.
* CLI flag `--http-port` has been added for Nginx plugin exclusively, and replaces
`--tls-sni-01-port`. It defines the HTTPS port the Nginx plugin will use while
setting up a new SSL vhost. By default the HTTPS port is 443.
### Changed
* Support for TLS-SNI-01 has been removed from all official Certbot plugins.
* CLI flags `--tls-sni-01-port` and `--tls-sni-01-address` are now no-op, will
generate a deprecation warning if used, and will be removed soon.
* Options `tls-sni` and `tls-sni-01` in `--preferred-challenges` flag are now no-op,
will generate a deprecation warning if used, and will be removed soon.
* CLI flag `--standalone-supported-challenges` has been removed.
### Fixed

View file

@ -295,7 +295,7 @@ class NginxConfigurator(common.Installer):
if create_if_no_match:
# result will not be [None] because it errors on failure
vhosts = [self._vhost_from_duplicated_default(target_name, True,
str(self.config.tls_sni_01_port))]
str(self.config.https_port))]
else:
# No matches. Raise a misconfiguration error.
raise errors.MisconfigurationError(
@ -609,7 +609,7 @@ class NginxConfigurator(common.Installer):
:type vhost: :class:`~certbot_nginx.obj.VirtualHost`
"""
https_port = self.config.tls_sni_01_port
https_port = self.config.https_port
ipv6info = self.ipv6_info(https_port)
ipv6_block = ['']
ipv4_block = ['']

View file

@ -82,7 +82,7 @@ def get_nginx_configurator(
in_progress_dir=os.path.join(backups, "IN_PROGRESS"),
server="https://acme-server.org:443/new",
http01_port=80,
tls_sni_01_port=5001,
https_port=5001,
),
name="nginx",
version=version)

View file

@ -1117,14 +1117,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis
"testing", "--no-verify-ssl", action="store_true",
help=config_help("no_verify_ssl"),
default=flag_default("no_verify_ssl"))
helpful.add(
["testing", "standalone", "apache", "nginx"], "--tls-sni-01-port", type=int,
default=flag_default("tls_sni_01_port"),
help=config_help("tls_sni_01_port"))
helpful.add(
["testing", "standalone"], "--tls-sni-01-address",
default=flag_default("tls_sni_01_address"),
help=config_help("tls_sni_01_address"))
helpful.add(
["testing", "standalone", "manual"], "--http-01-port", type=int,
dest="http01_port",
@ -1133,6 +1125,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis
["testing", "standalone"], "--http-01-address",
dest="http01_address",
default=flag_default("http01_address"), help=config_help("http01_address"))
helpful.add(
["testing", "nginx"], "--https-port", type=int,
default=flag_default("https_port"),
help=config_help("https_port"))
helpful.add(
"testing", "--break-my-certs", action="store_true",
default=flag_default("break_my_certs"),
@ -1193,7 +1189,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis
action=_PrefChallAction, default=flag_default("pref_challs"),
help='A sorted, comma delimited list of the preferred challenge to '
'use during authorization with the most preferred challenge '
'listed first (Eg, "dns" or "tls-sni-01,http,dns"). '
'listed first (Eg, "dns" or "http,dns"). '
'Not all plugins support all challenges. See '
'https://certbot.eff.org/docs/using.html#plugins for details. '
'ACME Challenges are versioned, but if you pick "http" rather '
@ -1264,6 +1260,17 @@ 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)
@ -1556,6 +1563,15 @@ def parse_preferred_challenges(pref_challs):
aliases = {"dns": "dns-01", "http": "http-01", "tls-sni": "tls-sni-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:
@ -1563,11 +1579,13 @@ def parse_preferred_challenges(pref_challs):
"Unrecognized challenges: {0}".format(unrecognized))
return challs
def _user_agent_comment_type(value):
if "(" in value or ")" in value:
raise argparse.ArgumentTypeError("may not contain parentheses")
return value
class _DeployHookAction(argparse.Action):
"""Action class for parsing deploy hooks."""

View file

@ -148,10 +148,10 @@ def check_config_sanity(config):
"""
# Port check
if config.http01_port == config.tls_sni_01_port:
if config.http01_port == config.https_port:
raise errors.ConfigurationError(
"Trying to run http-01 and tls-sni-01 "
"on the same port ({0})".format(config.tls_sni_01_port))
"Trying to run http-01 and https-port "
"on the same port ({0})".format(config.https_port))
# Domain checks
if config.namespace.domains is not None:

View file

@ -50,10 +50,9 @@ CLI_DEFAULTS = dict(
debug=False,
debug_challenges=False,
no_verify_ssl=False,
tls_sni_01_port=challenges.TLSSNI01Response.PORT,
tls_sni_01_address="",
http01_port=challenges.HTTP01Response.PORT,
http01_address="",
https_port=443,
break_my_certs=False,
rsa_key_size=2048,
must_staple=False,

View file

@ -220,12 +220,6 @@ class IConfig(zope.interface.Interface):
no_verify_ssl = zope.interface.Attribute(
"Disable verification of the ACME server's certificate.")
tls_sni_01_port = zope.interface.Attribute(
"Port used during tls-sni-01 challenge. "
"This only affects the port Certbot listens on. "
"A conforming ACME server will still attempt to connect on port 443.")
tls_sni_01_address = zope.interface.Attribute(
"The address the server listens to during tls-sni-01 challenge.")
http01_port = zope.interface.Attribute(
"Port used in the http-01 challenge. "
@ -235,6 +229,11 @@ class IConfig(zope.interface.Interface):
http01_address = zope.interface.Attribute(
"The address the server listens to during http-01 challenge.")
https_port = zope.interface.Attribute(
"Port used to serve HTTPS. "
"This affects which port Nginx will listen on after a LE certificate "
"is installed.")
pref_challs = zope.interface.Attribute(
"Sorted user specified preferred challenges"
"type strings with the most preferred challenge listed first")

View file

@ -1,5 +1,4 @@
"""Standalone Authenticator."""
import argparse
import collections
import logging
import socket
@ -108,52 +107,6 @@ class ServerManager(object):
return self._instances.copy()
SUPPORTED_CHALLENGES = [challenges.HTTP01] \
# type: List[Type[challenges.KeyAuthorizationChallenge]]
class SupportedChallengesAction(argparse.Action):
"""Action class for parsing standalone_supported_challenges."""
def __call__(self, parser, namespace, values, option_string=None):
logger.warning(
"The standalone specific supported challenges flag is "
"deprecated. Please use the --preferred-challenges flag "
"instead.")
converted_values = self._convert_and_validate(values)
namespace.standalone_supported_challenges = converted_values
def _convert_and_validate(self, data):
"""Validate the value of supported challenges provided by the user.
:param str data: comma delimited list of challenge types
:returns: validated and converted list of challenge types
:rtype: str
"""
challs = data.split(",")
unrecognized = [name for name in challs
if name not in challenges.Challenge.TYPES]
# argparse.ArgumentErrors raised out of argparse.Action objects
# are caught by argparse which prints usage information and the
# error that occurred before calling sys.exit.
if unrecognized:
raise argparse.ArgumentError(
self,
"Unrecognized challenges: {0}".format(", ".join(unrecognized)))
choices = set(chall.typ for chall in SUPPORTED_CHALLENGES)
if not set(challs).issubset(choices):
raise argparse.ArgumentError(
self,
"Plugin does not support the following (valid) "
"challenges: {0}".format(", ".join(set(challs) - choices)))
return data
@zope.interface.implementer(interfaces.IAuthenticator)
@zope.interface.provider(interfaces.IPluginFactory)
class Authenticator(common.Plugin):
@ -184,16 +137,7 @@ class Authenticator(common.Plugin):
@classmethod
def add_parser_arguments(cls, add):
add("supported-challenges",
help=argparse.SUPPRESS,
action=SupportedChallengesAction,
default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES))
@property
def supported_challenges(self):
"""Challenges supported by this plugin."""
return [challenges.Challenge.TYPES[name] for name in
self.conf("supported-challenges").split(",")]
pass # No additional argument for the standalone plugin parser
def more_info(self): # pylint: disable=missing-docstring
return("This authenticator creates its own ephemeral TCP listener "
@ -206,7 +150,7 @@ class Authenticator(common.Plugin):
def get_chall_pref(self, domain):
# pylint: disable=unused-argument,missing-docstring
return self.supported_challenges
return [challenges.HTTP01]
def perform(self, achalls): # pylint: disable=missing-docstring
return [self._try_perform_single(achall) for achall in achalls]

View file

@ -1,5 +1,4 @@
"""Tests for certbot.plugins.standalone."""
import argparse
import socket
import unittest
# https://github.com/python/typeshed/blob/master/stdlib/2and3/socket.pyi
@ -73,38 +72,6 @@ class ServerManagerTest(unittest.TestCase):
maybe_another_server.close()
class SupportedChallengesActionTest(unittest.TestCase):
"""Tests for plugins.standalone.SupportedChallengesAction."""
def _call(self, value):
with mock.patch("certbot.plugins.standalone.logger") as mock_logger:
# stderr is mocked to prevent potential argparse error
# output from cluttering test output
with mock.patch("sys.stderr"):
config = self.parser.parse_args([self.flag, value])
self.assertTrue(mock_logger.warning.called)
return getattr(config, self.dest)
def setUp(self):
self.flag = "--standalone-supported-challenges"
self.dest = self.flag[2:].replace("-", "_")
self.parser = argparse.ArgumentParser()
from certbot.plugins.standalone import SupportedChallengesAction
self.parser.add_argument(self.flag, action=SupportedChallengesAction)
def test_correct(self):
self.assertEqual("http-01", self._call("http-01"))
def test_unrecognized(self):
assert "foo" not in challenges.Challenge.TYPES
self.assertRaises(SystemExit, self._call, "foo")
def test_not_subset(self):
self.assertRaises(SystemExit, self._call, "dns")
def get_open_port():
"""Gets an open port number from the OS."""
open_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0)
@ -120,21 +87,10 @@ class AuthenticatorTest(unittest.TestCase):
def setUp(self):
from certbot.plugins.standalone import Authenticator
self.config = mock.MagicMock(
http01_port=get_open_port(),
standalone_supported_challenges="http-01")
self.config = mock.MagicMock(http01_port=get_open_port())
self.auth = Authenticator(self.config, name="standalone")
self.auth.servers = mock.MagicMock()
def test_supported_challenges(self):
self.assertEqual(self.auth.supported_challenges,
[challenges.HTTP01])
def test_supported_challenges_configured(self):
self.config.standalone_supported_challenges = "http-01"
self.assertEqual(self.auth.supported_challenges,
[challenges.HTTP01])
def test_more_info(self):
self.assertTrue(isinstance(self.auth.more_info(), six.string_types))
@ -142,11 +98,6 @@ class AuthenticatorTest(unittest.TestCase):
self.assertEqual(self.auth.get_chall_pref(domain=None),
[challenges.HTTP01])
def test_get_chall_pref_configured(self):
self.config.standalone_supported_challenges = "http-01"
self.assertEqual(self.auth.get_chall_pref(domain=None),
[challenges.HTTP01])
def test_perform(self):
achalls = self._get_achalls()
response = self.auth.perform(achalls)

View file

@ -35,10 +35,8 @@ logger = logging.getLogger(__name__)
# the renewal configuration process loses this information.
STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
"server", "account", "authenticator", "installer",
"standalone_supported_challenges", "renew_hook",
"pre_hook", "post_hook", "tls_sni_01_address",
"http01_address"]
INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
"renew_hook", "pre_hook", "post_hook", "http01_address"]
INT_CONFIG_ITEMS = ["rsa_key_size", "http01_port"]
BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names", "reuse_key",
"autorenew"]

View file

@ -235,13 +235,19 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods
self.assertEqual(namespace.domains, ['example.com', 'another.net'])
def test_preferred_challenges(self):
short_args = ['--preferred-challenges', 'http, tls-sni-01, dns']
short_args = ['--preferred-challenges', 'http, dns']
namespace = self.parse(short_args)
expected = [challenges.HTTP01.typ,
challenges.TLSSNI01.typ, challenges.DNS01.typ]
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()
@ -260,12 +266,13 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods
def test_no_gui(self):
args = ['renew', '--dialog']
stderr = six.StringIO()
with mock.patch('certbot.main.sys.stderr', new=stderr):
with mock.patch("certbot.util.logger.warning") as mock_warn:
namespace = self.parse(args)
self.assertTrue(namespace.noninteractive_mode)
self.assertTrue("--dialog is deprecated" in stderr.getvalue())
self.assertEqual(mock_warn.call_count, 1)
self.assertTrue("is deprecated" in mock_warn.call_args[0][0])
self.assertEqual("--dialog", mock_warn.call_args[0][1])
def _check_server_conflict_message(self, parser_args, conflicting_args):
try:

View file

@ -17,11 +17,11 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
super(NamespaceConfigTest, self).setUp()
self.config.foo = 'bar'
self.config.server = 'https://acme-server.org:443/new'
self.config.tls_sni_01_port = 1234
self.config.https_port = 1234
self.config.http01_port = 4321
def test_init_same_ports(self):
self.config.namespace.tls_sni_01_port = 4321
self.config.namespace.https_port = 4321
from certbot.configuration import NamespaceConfig
self.assertRaises(errors.Error, NamespaceConfig, self.config.namespace)
@ -79,7 +79,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir',
'logs_dir', 'http01_port',
'tls_sni_01_port',
'https_port',
'domains', 'server'])
mock_namespace.config_dir = config_base
mock_namespace.work_dir = work_base
@ -126,7 +126,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir',
'logs_dir', 'http01_port',
'tls_sni_01_port',
'https_port',
'domains', 'server'])
mock_namespace.config_dir = config_base
mock_namespace.work_dir = work_base

View file

@ -58,11 +58,15 @@ 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(',')}
self._call(self.config, renewalparams)
expected = [challenges.TLSSNI01.typ,
challenges.HTTP01.typ, challenges.DNS01.typ]
with mock.patch('certbot.renewal.cli.logger.warning') as mock_warn:
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

@ -62,14 +62,12 @@ break_my_certs = False
standalone = True
manual = False
server = https://acme-staging.api.letsencrypt.org/directory
standalone_supported_challenges = "tls-sni-01,http-01"
webroot = True
os_packages_only = False
apache_init_script = None
user_agent = None
apache_le_vhost_ext = -le-ssl.conf
debug = False
tls_sni_01_port = 443
logs_dir = /var/log/letsencrypt
apache_vhost_root = /etc/apache2/sites-available
configurator = None

View file

@ -62,14 +62,12 @@ break_my_certs = False
standalone = True
manual = False
server = https://acme-staging-v02.api.letsencrypt.org/directory
standalone_supported_challenges = "tls-sni-01,http-01"
webroot = False
os_packages_only = False
apache_init_script = None
user_agent = None
apache_le_vhost_ext = -le-ssl.conf
debug = False
tls_sni_01_port = 443
logs_dir = /var/log/letsencrypt
apache_vhost_root = /etc/apache2/sites-available
configurator = None

View file

@ -351,29 +351,28 @@ class AddDeprecatedArgumentTest(unittest.TestCase):
def _call(self, argument_name, nargs):
from certbot.util import add_deprecated_argument
add_deprecated_argument(self.parser.add_argument, argument_name, nargs)
def test_warning_no_arg(self):
self._call("--old-option", 0)
stderr = self._get_argparse_warnings(["--old-option"])
self.assertTrue("--old-option is deprecated" in stderr)
with mock.patch("certbot.util.logger.warning") as mock_warn:
self.parser.parse_args(["--old-option"])
self.assertEqual(mock_warn.call_count, 1)
self.assertTrue("is deprecated" in mock_warn.call_args[0][0])
self.assertEqual("--old-option", mock_warn.call_args[0][1])
def test_warning_with_arg(self):
self._call("--old-option", 1)
stderr = self._get_argparse_warnings(["--old-option", "42"])
self.assertTrue("--old-option is deprecated" in stderr)
def _get_argparse_warnings(self, args):
stderr = six.StringIO()
with mock.patch("certbot.util.sys.stderr", new=stderr):
self.parser.parse_args(args)
return stderr.getvalue()
with mock.patch("certbot.util.logger.warning") as mock_warn:
self.parser.parse_args(["--old-option", "42"])
self.assertEqual(mock_warn.call_count, 1)
self.assertTrue("is deprecated" in mock_warn.call_args[0][0])
self.assertEqual("--old-option", mock_warn.call_args[0][1])
def test_help(self):
self._call("--old-option", 2)
stdout = six.StringIO()
with mock.patch("certbot.util.sys.stdout", new=stdout):
with mock.patch("sys.stdout", new=stdout):
try:
self.parser.parse_args(["-h"])
except SystemExit:

View file

@ -13,7 +13,6 @@ import re
import six
import socket
import subprocess
import sys
from collections import OrderedDict
@ -477,8 +476,7 @@ def safe_email(email):
class _ShowWarning(argparse.Action):
"""Action to log a warning when an argument is used."""
def __call__(self, unused1, unused2, unused3, option_string=None):
sys.stderr.write(
"Use of {0} is deprecated.\n".format(option_string))
logger.warning("Use of %s is deprecated.", option_string)
def add_deprecated_argument(add_argument, argument_name, nargs):

View file

@ -1,4 +1,4 @@
usage:
usage:
certbot [SUBCOMMAND] [options] [-d DOMAIN] [-d DOMAIN] ...
Certbot can obtain and install HTTPS/TLS/SSL certificates. By default,

View file

@ -169,9 +169,6 @@ the bound IPv6 port and the failure during the second bind is expected.
Use ``--<challenge-type>-address`` to explicitly tell Certbot which interface
(and protocol) to bind.
.. note:: The ``--standalone-supported-challenges`` option has been
deprecated since ``certbot`` version 0.9.0.
.. _dns_plugins:
DNS Plugins

View file

@ -15,7 +15,6 @@ rsa-key-size = 4096
# Uncomment to use the standalone authenticator on port 443
# authenticator = standalone
# standalone-supported-challenges = tls-sni-01
# Uncomment to use the webroot authenticator. Replace webroot-path with the
# path to the public_html / webroot folder being served by your web server.

View file

@ -61,7 +61,7 @@ certbot_test_no_force_renew () {
--server "${SERVER:-http://localhost:4000/directory}" \
--no-verify-ssl \
--http-01-port $http_01_port \
--tls-sni-01-port $https_port \
--https-port $https_port \
--manual-public-ip-logging-ok \
$other_flags \
--non-interactive \