From 7e463bccad469c1fa7cda6aba09b1e459a514e2d Mon Sep 17 00:00:00 2001 From: ohemorange Date: Tue, 16 Jan 2018 14:58:45 -0800 Subject: [PATCH] Handle more edge cases for HTTP-01 support in Nginx (#5421) * only when using http01, only match default_server by port * import errors * put back in the code that creates a dummy block, but only when we can't find anything else --- certbot-nginx/certbot_nginx/configurator.py | 30 +++--- certbot-nginx/certbot_nginx/http_01.py | 107 ++++++++++++++++++-- 2 files changed, 113 insertions(+), 24 deletions(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index a77cf2bc3..bb2933a39 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -261,9 +261,9 @@ class NginxConfigurator(common.Installer): ipv6only_present = True return (ipv6_active, ipv6only_present) - def _vhost_from_duplicated_default(self, domain): + def _vhost_from_duplicated_default(self, domain, port=None): if self.new_vhost is None: - default_vhost = self._get_default_vhost() + default_vhost = self._get_default_vhost(port) self.new_vhost = self.parser.duplicate_vhost(default_vhost, delete_default=True) self.new_vhost.names = set() @@ -278,15 +278,16 @@ class NginxConfigurator(common.Installer): name_block[0].append(name) self.parser.add_server_directives(vhost, name_block, replace=True) - def _get_default_vhost(self): + def _get_default_vhost(self, port): vhost_list = self.parser.get_vhosts() # if one has default_server set, return that one default_vhosts = [] for vhost in vhost_list: for addr in vhost.addrs: if addr.default: - default_vhosts.append(vhost) - break + if port is None or self._port_matches(port, addr.get_port()): + default_vhosts.append(vhost) + break if len(default_vhosts) == 1: return default_vhosts[0] @@ -393,9 +394,17 @@ class NginxConfigurator(common.Installer): matches = self._get_redirect_ranked_matches(target_name, port) vhost = self._select_best_name_match(matches) if not vhost and create_if_no_match: - vhost = self._vhost_from_duplicated_default(target_name) + vhost = self._vhost_from_duplicated_default(target_name, port=port) return vhost + def _port_matches(self, test_port, matching_port): + # test_port is a number, matching is a number or "" or None + if matching_port == "" or matching_port is None: + # if no port is specified, Nginx defaults to listening on port 80. + return test_port == self.DEFAULT_LISTEN_PORT + else: + return test_port == matching_port + def _get_redirect_ranked_matches(self, target_name, port): """Gets a ranked list of plaintextish port-listening vhosts matching target_name @@ -410,13 +419,6 @@ class NginxConfigurator(common.Installer): """ all_vhosts = self.parser.get_vhosts() - def _port_matches(test_port, matching_port): - # test_port is a number, matching is a number or "" or None - if matching_port == "" or matching_port is None: - # if no port is specified, Nginx defaults to listening on port 80. - return test_port == self.DEFAULT_LISTEN_PORT - else: - return test_port == matching_port def _vhost_matches(vhost, port): found_matching_port = False @@ -426,7 +428,7 @@ class NginxConfigurator(common.Installer): found_matching_port = (port == self.DEFAULT_LISTEN_PORT) else: for addr in vhost.addrs: - if _port_matches(port, addr.get_port()) and addr.ssl == False: + if self._port_matches(port, addr.get_port()) and addr.ssl == False: found_matching_port = True if found_matching_port: diff --git a/certbot-nginx/certbot_nginx/http_01.py b/certbot-nginx/certbot_nginx/http_01.py index 1f1e37891..c25081ae0 100644 --- a/certbot-nginx/certbot_nginx/http_01.py +++ b/certbot-nginx/certbot_nginx/http_01.py @@ -5,8 +5,12 @@ import os from acme import challenges +from certbot import errors from certbot.plugins import common +from certbot_nginx import obj +from certbot_nginx import nginxparser + logger = logging.getLogger(__name__) @@ -31,6 +35,13 @@ class NginxHttp01(common.ChallengePerformer): """ + def __init__(self, configurator): + super(NginxHttp01, self).__init__(configurator) + self.challenge_conf = os.path.join( + configurator.config.config_dir, "le_http_01_cert_challenge.conf") + self._ipv6 = None + self._ipv6only = None + def perform(self): """Perform a challenge on Nginx. @@ -51,8 +62,16 @@ class NginxHttp01(common.ChallengePerformer): return responses - def _add_bucket_directive(self): - """Modifies Nginx config to include server_names_hash_bucket_size directive.""" + def _mod_config(self): + """Modifies Nginx config to include server_names_hash_bucket_size directive + and server challenge blocks. + + :raises .MisconfigurationError: + Unable to find a suitable HTTP block in which to include + authenticator hosts. + """ + included = False + include_directive = ['\n', 'include', ' ', self.challenge_conf] root = self.configurator.parser.config_root bucket_directive = ['\n', 'server_names_hash_bucket_size', ' ', '128'] @@ -71,21 +90,82 @@ class NginxHttp01(common.ChallengePerformer): posn += 1 if not found_bucket: body.insert(0, bucket_directive) + if include_directive not in body: + body.insert(0, include_directive) + included = True break + if not included: + raise errors.MisconfigurationError( + 'Certbot could not find a block to include ' + 'challenges in %s.' % root) + config = [self._make_or_mod_server_block(achall) for achall in self.achalls] + config = [x for x in config if x is not None] + config = nginxparser.UnspacedList(config) - def _mod_config(self): - """Modifies Nginx config to handle challenges. + self.configurator.reverter.register_file_creation( + True, self.challenge_conf) + with open(self.challenge_conf, "w") as new_conf: + nginxparser.dump(config, new_conf) + + def _default_listen_addresses(self): + """Finds addresses for a challenge block to listen on. + :returns: list of :class:`certbot_nginx.obj.Addr` to apply + :rtype: list """ - self._add_bucket_directive() + addresses = [] + default_addr = "%s" % self.configurator.config.http01_port + ipv6_addr = "[::]:{0}".format( + self.configurator.config.http01_port) + port = self.configurator.config.http01_port - for achall in self.achalls: - self._mod_server_block(achall) + if self._ipv6 is None or self._ipv6only is None: + self._ipv6, self._ipv6only = self.configurator.ipv6_info(port) + ipv6, ipv6only = self._ipv6, self._ipv6only + + if ipv6: + # If IPv6 is active in Nginx configuration + if not ipv6only: + # If ipv6only=on is not already present in the config + ipv6_addr = ipv6_addr + " ipv6only=on" + addresses = [obj.Addr.fromstring(default_addr), + obj.Addr.fromstring(ipv6_addr)] + logger.info(("Using default addresses %s and %s for authentication."), + default_addr, + ipv6_addr) + else: + addresses = [obj.Addr.fromstring(default_addr)] + logger.info("Using default address %s for authentication.", + default_addr) + return addresses def _get_validation_path(self, achall): return os.sep + os.path.join(challenges.HTTP01.URI_ROOT_PATH, achall.chall.encode("token")) - def _mod_server_block(self, achall): + def _make_server_block(self, achall): + """Creates a server block for a challenge. + :param achall: Annotated HTTP-01 challenge + :type achall: + :class:`certbot.achallenges.KeyAuthorizationAnnotatedChallenge` + :param list addrs: addresses of challenged domain + :class:`list` of type :class:`~nginx.obj.Addr` + :returns: server block for the challenge host + :rtype: list + """ + addrs = self._default_listen_addresses() + block = [['listen', ' ', addr.to_string(include_default=False)] for addr in addrs] + + validation = achall.validation(achall.account_key) + validation_path = self._get_validation_path(achall) + + block.extend([['server_name', ' ', achall.domain], + [['location', ' ', '=', ' ', validation_path], + [['default_type', ' ', 'text/plain'], + ['return', ' ', '200', ' ', validation]]]]) + # TODO: do we want to return something else if they otherwise access this block? + return [['server'], block] + + def _make_or_mod_server_block(self, achall): """Modifies a server block to respond to a challenge. :param achall: Annotated HTTP-01 challenge @@ -93,8 +173,15 @@ class NginxHttp01(common.ChallengePerformer): :class:`certbot.achallenges.KeyAuthorizationAnnotatedChallenge` """ - vhost = self.configurator.choose_redirect_vhost(achall.domain, - '%i' % self.configurator.config.http01_port, create_if_no_match=True) + try: + vhost = self.configurator.choose_redirect_vhost(achall.domain, + '%i' % self.configurator.config.http01_port, create_if_no_match=True) + except errors.MisconfigurationError: + # Couldn't find either a matching name+port server block + # or a port+default_server block, so create a dummy block + return self._make_server_block(achall) + + # Modify existing server block validation = achall.validation(achall.account_key) validation_path = self._get_validation_path(achall)