diff --git a/.travis.yml b/.travis.yml index b27081c24..48b9b43cb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -161,7 +161,9 @@ addons: - libapache2-mod-macro install: "travis_retry pip install tox coveralls" -script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/travis-integration.sh)' +script: + - travis_retry tox + - '[ -z "${BOULDER_INTEGRATION+x}" ] || (travis_retry tests/boulder-fetch.sh && tests/tox-boulder-integration.sh)' after_success: '[ "$TOXENV" == "cover" ] && coveralls' diff --git a/acme/acme/client.py b/acme/acme/client.py index fa903f0e6..2e07d34d7 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -11,6 +11,7 @@ import six from six.moves import http_client # pylint: disable=import-error import OpenSSL +import re import requests import sys @@ -599,6 +600,7 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes return response def _send_request(self, method, url, *args, **kwargs): + # pylint: disable=too-many-locals """Send HTTP request. Makes sure that `verify_ssl` is respected. Logs request and @@ -624,7 +626,32 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes kwargs.setdefault('headers', {}) kwargs['headers'].setdefault('User-Agent', self.user_agent) kwargs.setdefault('timeout', self._default_timeout) - response = self.session.request(method, url, *args, **kwargs) + try: + response = self.session.request(method, url, *args, **kwargs) + except requests.exceptions.RequestException as e: + # pylint: disable=pointless-string-statement + """Requests response parsing + + The requests library emits exceptions with a lot of extra text. + We parse them with a regexp to raise a more readable exceptions. + + Example: + HTTPSConnectionPool(host='acme-v01.api.letsencrypt.org', + port=443): Max retries exceeded with url: /directory + (Caused by NewConnectionError(' + : Failed to establish a new connection: + [Errno 65] No route to host',))""" + + # pylint: disable=line-too-long + err_regex = r".*host='(\S*)'.*Max retries exceeded with url\: (\/\w*).*(\[Errno \d+\])([A-Za-z ]*)" + m = re.match(err_regex, str(e)) + if m is None: + raise # pragma: no cover + else: + host, path, _err_no, err_msg = m.groups() + raise ValueError("Requesting {0}{1}:{2}".format(host, path, err_msg)) + # If content is DER, log the base64 of it instead of raw bytes, to keep # binary data out of the logs. if response.headers.get("Content-Type") == DER_CONTENT_TYPE: diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 54652b46c..4bd762865 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -621,6 +621,21 @@ class ClientNetworkTest(unittest.TestCase): self.assertRaises(requests.exceptions.RequestException, self.net._send_request, 'GET', 'uri') + def test_urllib_error(self): + # Using a connection error to test a properly formatted error message + try: + # pylint: disable=protected-access + self.net._send_request('GET', "http://localhost:19123/nonexistent.txt") + + # Value Error Generated Exceptions + except ValueError as y: + self.assertEqual("Requesting localhost/nonexistent: " + "Connection refused", str(y)) + + # Requests Library Exceptions + except requests.exceptions.ConnectionError as z: #pragma: no cover + self.assertEqual("('Connection aborted.', " + "error(111, 'Connection refused'))", str(z)) class ClientNetworkWithMockedResponseTest(unittest.TestCase): """Tests for acme.client.ClientNetwork which mock out response.""" diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 2c2906f28..71a42e555 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1303,13 +1303,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): .. note:: This function saves the configuration :param ssl_vhost: Destination of traffic, an ssl enabled vhost - :type ssl_vhost: :class:`~letsencrypt_apache.obj.VirtualHost` + :type ssl_vhost: :class:`~certbot_apache.obj.VirtualHost` :param unused_options: Not currently used :type unused_options: Not Available :returns: Success, general_vhost (HTTP vhost) - :rtype: (bool, :class:`~letsencrypt_apache.obj.VirtualHost`) + :rtype: (bool, :class:`~certbot_apache.obj.VirtualHost`) """ min_apache_ver = (2, 3, 3) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index a12671576..00fc0df8e 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -374,7 +374,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_enable_site_nondebian(self): from certbot_apache import override_centos self.config.os_info = override_centos.Override(self.config) - inc_path = "/path/to/whereever" + inc_path = "/path/to/wherever" vhost = self.vh_truth[0] vhost.enabled = False vhost.filep = inc_path @@ -1367,7 +1367,7 @@ class MultipleVhostsTest(util.ApacheTest): self.assertFalse(self.config._check_aug_version()) class AugeasVhostsTest(util.ApacheTest): - """Test vhosts with illegal names dependant on augeas version.""" + """Test vhosts with illegal names dependent on augeas version.""" # pylint: disable=protected-access _multiprocess_can_split_ = True @@ -1451,7 +1451,7 @@ class AugeasVhostsTest(util.ApacheTest): broken_vhost) class MultiVhostsTest(util.ApacheTest): - """Test vhosts with illegal names dependant on augeas version.""" + """Test vhosts with illegal names dependent on augeas version.""" # pylint: disable=protected-access def setUp(self): # pylint: disable=arguments-differ diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 7e55ff0ea..4857942a1 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -30,7 +30,7 @@ from certbot_nginx import parser logger = logging.getLogger(__name__) REDIRECT_BLOCK = [[ - ['\n ', 'if', ' ', '($scheme', ' ', '!=', ' ', '"https") '], + ['\n ', 'if', ' ', '($scheme', ' ', '!=', ' ', '"https")'], [['\n ', 'return', ' ', '301', ' ', 'https://$host$request_uri'], '\n '] ], ['\n']] @@ -117,6 +117,9 @@ class NginxConfigurator(common.Installer): # Files to save self.save_notes = "" + # For creating new vhosts if no names match + self.new_vhost = None + # Add number of outstanding challenges self._chall_out = 0 @@ -191,9 +194,11 @@ class NginxConfigurator(common.Installer): "The nginx plugin currently requires --fullchain-path to " "install a cert.") - vhost = self.choose_vhost(domain) - cert_directives = [['\n', 'ssl_certificate', ' ', fullchain_path], - ['\n', 'ssl_certificate_key', ' ', key_path]] + vhost = self.choose_vhost(domain, raise_if_no_match=False) + if vhost is None: + vhost = self._vhost_from_duplicated_default(domain) + cert_directives = [['\n ', 'ssl_certificate', ' ', fullchain_path], + ['\n ', 'ssl_certificate_key', ' ', key_path]] self.parser.add_server_directives(vhost, cert_directives, replace=True) @@ -209,7 +214,7 @@ class NginxConfigurator(common.Installer): ####################### # Vhost parsing methods ####################### - def choose_vhost(self, target_name): + def choose_vhost(self, target_name, raise_if_no_match=True): """Chooses a virtual host based on the given domain name. .. note:: This makes the vhost SSL-enabled if it isn't already. Follows @@ -223,6 +228,8 @@ class NginxConfigurator(common.Installer): hostname. Currently we just ignore this. :param str target_name: domain name + :param bool raise_if_no_match: True iff not finding a match is an error; + otherwise, return None :returns: ssl vhost associated with name :rtype: :class:`~certbot_nginx.obj.VirtualHost` @@ -233,13 +240,16 @@ class NginxConfigurator(common.Installer): matches = self._get_ranked_matches(target_name) vhost = self._select_best_name_match(matches) if not vhost: - # No matches. Raise a misconfiguration error. - raise errors.MisconfigurationError( - ("Cannot find a VirtualHost matching domain %s. " - "In order for Certbot to correctly perform the challenge " - "please add a corresponding server_name directive to your " - "nginx configuration: " - "https://nginx.org/en/docs/http/server_names.html") % (target_name)) + if raise_if_no_match: + # No matches. Raise a misconfiguration error. + raise errors.MisconfigurationError( + ("Cannot find a VirtualHost matching domain %s. " + "In order for Certbot to correctly perform the challenge " + "please add a corresponding server_name directive to your " + "nginx configuration: " + "https://nginx.org/en/docs/http/server_names.html") % (target_name)) + else: + return None else: # Note: if we are enhancing with ocsp, vhost should already be ssl. if not vhost.ssl: @@ -247,6 +257,37 @@ class NginxConfigurator(common.Installer): return vhost + def _vhost_from_duplicated_default(self, domain): + if self.new_vhost is None: + default_vhost = self._get_default_vhost() + self.new_vhost = self.parser.create_new_vhost_from_default(default_vhost) + if not self.new_vhost.ssl: + self._make_server_ssl(self.new_vhost) + self.new_vhost.names = set() + + self.new_vhost.names.add(domain) + name_block = [['\n ', 'server_name', ' ', ' '.join(self.new_vhost.names)]] + self.parser.add_server_directives(self.new_vhost, name_block, replace=True) + return self.new_vhost + + def _get_default_vhost(self): + 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 len(default_vhosts) == 1: + return default_vhosts[0] + + # TODO: present a list of vhosts for user to choose from + + raise errors.MisconfigurationError("Could not automatically find a matching server" + " block. Set the `server_name` directive to use the Nginx installer.") + def _get_ranked_matches(self, target_name): """Returns a ranked list of vhosts that match target_name. The ranking gives preference to SSL vhosts. diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index 20aeeb554..14481e298 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -7,6 +7,7 @@ from pyparsing import ( Literal, White, Forward, Group, Optional, OneOrMore, QuotedString, Regex, ZeroOrMore, Combine) from pyparsing import stringEnd from pyparsing import restOfLine +import six logger = logging.getLogger(__name__) @@ -71,7 +72,7 @@ class RawNginxDumper(object): """Iterates the dumped nginx content.""" blocks = blocks or self.blocks for b0 in blocks: - if isinstance(b0, str): + if isinstance(b0, six.string_types): yield b0 continue item = copy.deepcopy(b0) @@ -88,7 +89,7 @@ class RawNginxDumper(object): yield '}' else: # not a block - list of strings semicolon = ";" - if isinstance(item[0], str) and item[0].strip() == '#': # comment + if isinstance(item[0], six.string_types) and item[0].strip() == '#': # comment semicolon = "" yield "".join(item) + semicolon @@ -145,7 +146,7 @@ def dump(blocks, _file): return _file.write(dumps(blocks)) -spacey = lambda x: (isinstance(x, str) and x.isspace()) or x == '' +spacey = lambda x: (isinstance(x, six.string_types) and x.isspace()) or x == '' class UnspacedList(list): """Wrap a list [of lists], making any whitespace entries magically invisible""" @@ -189,13 +190,15 @@ class UnspacedList(list): item, spaced_item = self._coerce(x) slicepos = self._spaced_position(i) if i < len(self) else len(self.spaced) self.spaced.insert(slicepos, spaced_item) - list.insert(self, i, item) + if not spacey(item): + list.insert(self, i, item) self.dirty = True def append(self, x): item, spaced_item = self._coerce(x) self.spaced.append(spaced_item) - list.append(self, item) + if not spacey(item): + list.append(self, item) self.dirty = True def extend(self, x): @@ -226,7 +229,8 @@ class UnspacedList(list): raise NotImplementedError("Slice operations on UnspacedLists not yet implemented") item, spaced_item = self._coerce(value) self.spaced.__setitem__(self._spaced_position(i), spaced_item) - list.__setitem__(self, i, item) + if not spacey(item): + list.__setitem__(self, i, item) self.dirty = True def __delitem__(self, i): @@ -235,8 +239,8 @@ class UnspacedList(list): self.dirty = True def __deepcopy__(self, memo): - l = UnspacedList(self[:]) - l.spaced = copy.deepcopy(self.spaced, memo=memo) + new_spaced = copy.deepcopy(self.spaced, memo=memo) + l = UnspacedList(new_spaced) l.dirty = self.dirty return l diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py index 849cefe1f..d7604bdf9 100644 --- a/certbot-nginx/certbot_nginx/obj.py +++ b/certbot-nginx/certbot_nginx/obj.py @@ -198,7 +198,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods def _find_directive(directives, directive_name): """Find a directive of type directive_name in directives """ - if not directives or isinstance(directives, str) or len(directives) == 0: + if not directives or isinstance(directives, six.string_types) or len(directives) == 0: return None if directives[0] == directive_name: diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 158cb9929..3eb6264aa 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -6,6 +6,8 @@ import os import pyparsing import re +import six + from certbot import errors from certbot_nginx import obj @@ -312,6 +314,32 @@ class NginxParser(object): except errors.MisconfigurationError as err: raise errors.MisconfigurationError("Problem in %s: %s" % (filename, str(err))) + def create_new_vhost_from_default(self, vhost_template): + """Duplicate the default vhost in the configuration files. + + :param :class:`~certbot_nginx.obj.VirtualHost` vhost_template: The vhost + whose information we copy + + :returns: A vhost object for the newly created vhost + :rtype: :class:`~certbot_nginx.obj.VirtualHost` + """ + # TODO: https://github.com/certbot/certbot/issues/5185 + # put it in the same file as the template, at the same level + enclosing_block = self.parsed[vhost_template.filep] + for index in vhost_template.path[:-1]: + enclosing_block = enclosing_block[index] + new_location = vhost_template.path[-1] + 1 + raw_in_parsed = copy.deepcopy(enclosing_block[vhost_template.path[-1]]) + enclosing_block.insert(new_location, raw_in_parsed) + new_vhost = copy.deepcopy(vhost_template) + new_vhost.path[-1] = new_location + for addr in new_vhost.addrs: + addr.default = False + for directive in enclosing_block[new_vhost.path[-1]][1]: + if len(directive) > 0 and directive[0] == 'listen' and 'default_server' in directive: + del directive[directive.index('default_server')] + return new_vhost + def _parse_ssl_options(ssl_options): if ssl_options is not None: try: @@ -444,7 +472,7 @@ def _is_include_directive(entry): """ return (isinstance(entry, list) and len(entry) == 2 and entry[0] == 'include' and - isinstance(entry[1], str)) + isinstance(entry[1], six.string_types)) def _is_ssl_on_directive(entry): """Checks if an nginx parsed entry is an 'ssl on' directive. @@ -561,7 +589,8 @@ def _add_directive(block, directive, replace): directive_name = directive[0] def can_append(loc, dir_name): """ Can we append this directive to the block? """ - return loc is None or (isinstance(dir_name, str) and dir_name in REPEATABLE_DIRECTIVES) + return loc is None or (isinstance(dir_name, six.string_types) + and dir_name in REPEATABLE_DIRECTIVES) err_fmt = 'tried to insert directive "{0}" but found conflicting "{1}".' diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 6a917204c..4f9f685c2 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -429,7 +429,7 @@ class NginxConfiguratorTest(util.NginxTest): # Test that we successfully add a redirect when there is # a listen directive expected = [ - ['if', '($scheme', '!=', '"https") '], + ['if', '($scheme', '!=', '"https")'], [['return', '301', 'https://$host$request_uri']] ] @@ -512,6 +512,23 @@ class NginxConfiguratorTest(util.NginxTest): self.assertEqual(mock_logger.info.call_args[0][0], 'No matching insecure server blocks listening on port %s found.') + def test_no_double_redirect(self): + # Test that we don't also add the commented redirect if we've just added + # a redirect to that vhost this run + example_conf = self.config.parser.abs_path('sites-enabled/example.com') + self.config.enhance("example.com", "redirect") + self.config.enhance("example.org", "redirect") + + unexpected = [ + ['#', ' Redirect non-https traffic to https'], + ['#', ' if ($scheme != "https") {'], + ['#', ' return 301 https://$host$request_uri;'], + ['#', ' } # managed by Certbot'] + ] + generated_conf = self.config.parser.parsed[example_conf] + for line in unexpected: + self.assertFalse(util.contains_at_depth(generated_conf, line, 2)) + def test_staple_ocsp_bad_version(self): self.config.version = (1, 3, 1) self.assertRaises(errors.PluginError, self.config.enhance, @@ -542,6 +559,138 @@ class NginxConfiguratorTest(util.NginxTest): self.assertTrue(util.contains_at_depth( generated_conf, ['ssl_stapling_verify', 'on'], 2)) + def test_deploy_no_match_default_set(self): + default_conf = self.config.parser.abs_path('sites-enabled/default') + foo_conf = self.config.parser.abs_path('foo.conf') + del self.config.parser.parsed[foo_conf][2][1][0][1][0] # remove default_server + self.config.version = (1, 3, 1) + + self.config.deploy_cert( + "www.nomatch.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") + self.config.save() + + self.config.parser.load() + + parsed_default_conf = util.filter_comments(self.config.parser.parsed[default_conf]) + + self.assertEqual([[['server'], + [['listen', 'myhost', 'default_server'], + ['listen', 'otherhost', 'default_server'], + ['server_name', 'www.example.org'], + [['location', '/'], + [['root', 'html'], + ['index', 'index.html', 'index.htm']]]]], + [['server'], + [['listen', 'myhost'], + ['listen', 'otherhost'], + ['server_name', 'www.nomatch.com'], + [['location', '/'], + [['root', 'html'], + ['index', 'index.html', 'index.htm']]], + ['listen', '5001', 'ssl'], + ['ssl_certificate', 'example/fullchain.pem'], + ['ssl_certificate_key', 'example/key.pem'], + ['include', self.config.mod_ssl_conf], + ['ssl_dhparam', self.config.ssl_dhparams]]]], + parsed_default_conf) + + self.config.deploy_cert( + "nomatch.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") + self.config.save() + + self.config.parser.load() + + parsed_default_conf = util.filter_comments(self.config.parser.parsed[default_conf]) + + self.assertTrue(util.contains_at_depth(parsed_default_conf, "nomatch.com", 3)) + + def test_deploy_no_match_default_set_multi_level_path(self): + default_conf = self.config.parser.abs_path('sites-enabled/default') + foo_conf = self.config.parser.abs_path('foo.conf') + del self.config.parser.parsed[default_conf][0][1][0] + del self.config.parser.parsed[default_conf][0][1][0] + self.config.version = (1, 3, 1) + + self.config.deploy_cert( + "www.nomatch.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") + self.config.save() + + self.config.parser.load() + + parsed_foo_conf = util.filter_comments(self.config.parser.parsed[foo_conf]) + + self.assertEqual([['server'], + [['listen', '*:80', 'ssl'], + ['server_name', 'www.nomatch.com'], + ['root', '/home/ubuntu/sites/foo/'], + [['location', '/status'], [[['types'], [['image/jpeg', 'jpg']]]]], + [['location', '~', 'case_sensitive\\.php$'], [['index', 'index.php'], + ['root', '/var/root']]], + [['location', '~*', 'case_insensitive\\.php$'], []], + [['location', '=', 'exact_match\\.php$'], []], + [['location', '^~', 'ignore_regex\\.php$'], []], + ['ssl_certificate', 'example/fullchain.pem'], + ['ssl_certificate_key', 'example/key.pem']]], + parsed_foo_conf[1][1][1]) + + def test_deploy_no_match_no_default_set(self): + default_conf = self.config.parser.abs_path('sites-enabled/default') + foo_conf = self.config.parser.abs_path('foo.conf') + del self.config.parser.parsed[default_conf][0][1][0] + del self.config.parser.parsed[default_conf][0][1][0] + del self.config.parser.parsed[foo_conf][2][1][0][1][0] + self.config.version = (1, 3, 1) + + self.assertRaises(errors.MisconfigurationError, self.config.deploy_cert, + "www.nomatch.com", "example/cert.pem", "example/key.pem", + "example/chain.pem", "example/fullchain.pem") + + def test_deploy_no_match_fail_multiple_defaults(self): + self.config.version = (1, 3, 1) + self.assertRaises(errors.MisconfigurationError, self.config.deploy_cert, + "www.nomatch.com", "example/cert.pem", "example/key.pem", + "example/chain.pem", "example/fullchain.pem") + + def test_deploy_no_match_add_redirect(self): + default_conf = self.config.parser.abs_path('sites-enabled/default') + foo_conf = self.config.parser.abs_path('foo.conf') + del self.config.parser.parsed[foo_conf][2][1][0][1][0] # remove default_server + self.config.version = (1, 3, 1) + + self.config.deploy_cert( + "www.nomatch.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") + + self.config.enhance("www.nomatch.com", "redirect") + + self.config.save() + + self.config.parser.load() + + expected = [ + ['if', '($scheme', '!=', '"https")'], + [['return', '301', 'https://$host$request_uri']] + ] + + generated_conf = self.config.parser.parsed[default_conf] + self.assertTrue(util.contains_at_depth(generated_conf, expected, 2)) + + class InstallSslOptionsConfTest(util.NginxTest): """Test that the options-ssl-nginx.conf file is installed and updated properly.""" diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index e655bc3e3..ec0cfd288 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -139,7 +139,8 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods False, True, set(['.example.com', 'example.*']), [], [0]) vhost4 = obj.VirtualHost(nparser.abs_path('sites-enabled/default'), - [obj.Addr('myhost', '', False, True)], + [obj.Addr('myhost', '', False, True), + obj.Addr('otherhost', '', False, True)], False, True, set(['www.example.org']), [], [0]) vhost5 = obj.VirtualHost(nparser.abs_path('foo.conf'), @@ -395,6 +396,29 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods ]) self.assertTrue(server['ssl']) + def test_create_new_vhost_from_default(self): + nparser = parser.NginxParser(self.config_path) + + vhosts = nparser.get_vhosts() + default = [x for x in vhosts if 'default' in x.filep][0] + new_vhost = nparser.create_new_vhost_from_default(default) + nparser.filedump(ext='') + + # check properties of new vhost + self.assertFalse(next(iter(new_vhost.addrs)).default) + self.assertNotEqual(new_vhost.path, default.path) + + # check that things are written to file correctly + new_nparser = parser.NginxParser(self.config_path) + new_vhosts = new_nparser.get_vhosts() + new_defaults = [x for x in new_vhosts if 'default' in x.filep] + self.assertEqual(len(new_defaults), 2) + new_vhost_parsed = new_defaults[1] + self.assertFalse(next(iter(new_vhost_parsed.addrs)).default) + self.assertEqual(next(iter(default.names)), next(iter(new_vhost_parsed.names))) + self.assertEqual(len(default.raw), len(new_vhost_parsed.raw)) + self.assertTrue(next(iter(default.addrs)).super_eq(next(iter(new_vhost_parsed.addrs)))) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default index 26f37020c..4f67fa7d1 100644 --- a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default +++ b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default @@ -1,5 +1,6 @@ server { listen myhost default_server; + listen otherhost default_server; server_name www.example.org; location / { diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py index 85db584b3..af477c460 100644 --- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py +++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py @@ -66,7 +66,7 @@ class TlsSniPerformTest(util.NginxTest): self.sni.add_chall(self.achalls[1]) mock_choose.return_value = None result = self.sni.perform() - self.assertTrue(result is None) + self.assertFalse(result is None) def test_perform0(self): responses = self.sni.perform() diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py index 48e117bba..bbfecc282 100644 --- a/certbot-nginx/certbot_nginx/tls_sni_01.py +++ b/certbot-nginx/certbot_nginx/tls_sni_01.py @@ -52,18 +52,13 @@ class NginxTlsSni01(common.TLSSNI01): self.configurator.config.tls_sni_01_port) for achall in self.achalls: - vhost = self.configurator.choose_vhost(achall.domain) - if vhost is None: - logger.error( - "No nginx vhost exists with server_name matching: %s. " - "Please specify server_names in the Nginx config.", - achall.domain) - return None + vhost = self.configurator.choose_vhost(achall.domain, raise_if_no_match=False) - if vhost.addrs: + if vhost is not None and vhost.addrs: addresses.append(list(vhost.addrs)) else: addresses.append([obj.Addr.fromstring(default_addr)]) + logger.info("Using default address %s for TLSSNI01 authentication.", default_addr) # Create challenge certs responses = [self._setup_challenge_cert(x) for x in self.achalls] @@ -115,7 +110,7 @@ class NginxTlsSni01(common.TLSSNI01): break if not included: raise errors.MisconfigurationError( - 'LetsEncrypt could not find an HTTP block to include ' + 'Certbot could not find an HTTP block to include ' 'TLS-SNI-01 challenges in %s.' % root) config = [self._make_server_block(pair[0], pair[1]) diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index 4662b2aa6..714d83cce 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -102,10 +102,14 @@ to serve all files under specified web root ({0}).""" webroot = None while webroot is None: - webroot = self._prompt_with_webroot_list(domain, known_webroots) - - if webroot is None: - webroot = self._prompt_for_new_webroot(domain) + if known_webroots: + # Only show the menu if we have options for it + webroot = self._prompt_with_webroot_list(domain, known_webroots) + if webroot is None: + webroot = self._prompt_for_new_webroot(domain) + else: + # Allow prompt to raise PluginError instead of looping forever + webroot = self._prompt_for_new_webroot(domain, True) return webroot @@ -125,13 +129,18 @@ to serve all files under specified web root ({0}).""" else: # code == display_util.OK return None if index == 0 else known_webroots[index - 1] - def _prompt_for_new_webroot(self, domain): + def _prompt_for_new_webroot(self, domain, allowraise=False): code, webroot = ops.validated_directory( _validate_webroot, "Input the webroot for {0}:".format(domain), force_interactive=True) if code == display_util.CANCEL: - return None + if not allowraise: + return None + else: + raise errors.PluginError( + "Every requested domain must have a " + "webroot when using the webroot plugin.") else: # code == display_util.OK return _validate_webroot(webroot) diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py index e202a0a6d..5a311716e 100644 --- a/certbot/plugins/webroot_test.py +++ b/certbot/plugins/webroot_test.py @@ -96,7 +96,7 @@ class AuthenticatorTest(unittest.TestCase): @test_util.patch_get_utility() def test_new_webroot(self, mock_get_utility): self.config.webroot_path = [] - self.config.webroot_map = {} + self.config.webroot_map = {"something.com": self.path} mock_display = mock_get_utility() mock_display.menu.return_value = (display_util.OK, 0,) @@ -108,6 +108,19 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.config.webroot_map[self.achall.domain], self.path) + @test_util.patch_get_utility() + def test_new_webroot_empty_map_cancel(self, mock_get_utility): + self.config.webroot_path = [] + self.config.webroot_map = {} + + mock_display = mock_get_utility() + mock_display.menu.return_value = (display_util.OK, 0,) + with mock.patch('certbot.display.ops.validated_directory') as m: + m.return_value = (display_util.CANCEL, -1) + self.assertRaises(errors.PluginError, + self.auth.perform, + [self.achall]) + def test_perform_missing_root(self): self.config.webroot_path = None self.config.webroot_map = {} @@ -132,6 +145,22 @@ class AuthenticatorTest(unittest.TestCase): mock_chown.side_effect = OSError(errno.EACCES, "msg") self.auth.perform([self.achall]) # exception caught and logged + + @test_util.patch_get_utility() + def test_perform_new_webroot_not_in_map(self, mock_get_utility): + new_webroot = tempfile.mkdtemp() + self.config.webroot_path = [] + self.config.webroot_map = {"whatever.com": self.path} + mock_display = mock_get_utility() + mock_display.menu.side_effect = ((display_util.OK, 0), + (display_util.OK, new_webroot)) + achall = achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.HTTP01_P, domain="something.com", account_key=KEY) + with mock.patch('certbot.display.ops.validated_directory') as m: + m.return_value = (display_util.OK, new_webroot,) + self.auth.perform([achall]) + self.assertEqual(self.config.webroot_map[achall.domain], new_webroot) + def test_perform_permissions(self): self.auth.prepare() diff --git a/docs/using.rst b/docs/using.rst index f2ba93a7e..4fd0b5ec8 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -503,7 +503,8 @@ run as usual after running all hooks in these directories. One minor exception to this is if a hook specified elsewhere is simply the path to an executable file in the hook directory of the same type (e.g. your pre-hook is the path to an executable in ``/etc/letsencrypt/renewal-hooks/pre``), the file is not run a -second time. +second time. You can stop Certbot from automatically running executables found +in these directories by including ``--no-directory-hooks`` on the command line. More information about hooks can be found by running ``certbot --help renew``. diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index 60538362e..08eb736c2 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -17,3 +17,9 @@ FAKE_DNS=$(ifconfig docker0 | grep "inet addr:" | cut -d: -f2 | awk '{ print $1} [ -z "$FAKE_DNS" ] && echo Unable to find the IP for docker0 && exit 1 sed -i "s/FAKE_DNS: .*/FAKE_DNS: ${FAKE_DNS}/" docker-compose.yml docker-compose up -d + +set +x # reduce verbosity while waiting for boulder +until curl http://localhost:4000/directory 2>/dev/null; do + echo waiting for boulder + sleep 1 +done diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 0cfc61dd2..7afba19df 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -326,7 +326,7 @@ CheckDirHooks 5 # test with overlapping directory hooks on the command line common renew --cert-name le2.wtf \ --pre-hook "$renewal_dir_pre_hook" \ - --renew-hook "$renewal_dir_deploy_hook" \ + --deploy-hook "$renewal_dir_deploy_hook" \ --post-hook "$renewal_dir_post_hook" CheckDirHooks 1 diff --git a/tests/tox-boulder-integration.sh b/tests/tox-boulder-integration.sh new file mode 100755 index 000000000..8c8a967fd --- /dev/null +++ b/tests/tox-boulder-integration.sh @@ -0,0 +1,12 @@ +#!/bin/bash -e +# A simple wrapper around tests/boulder-integration.sh that activates the tox +# virtual environment defined by the environment variable TOXENV before running +# integration tests. + +if [ -z "${TOXENV+x}" ]; then + echo "The environment variable TOXENV must be set to use this script!" >&2 + exit 1 +fi + +source .tox/$TOXENV/bin/activate +tests/boulder-integration.sh diff --git a/tests/travis-integration.sh b/tests/travis-integration.sh deleted file mode 100755 index b42617400..000000000 --- a/tests/travis-integration.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/bash - -set -o errexit - -./tests/boulder-fetch.sh - -source .tox/$TOXENV/bin/activate - -until curl http://boulder:4000/directory 2>/dev/null; do - echo waiting for boulder - sleep 1 -done - -./tests/boulder-integration.sh