diff --git a/README.rst b/README.rst index 0b8e652c0..174dc7dcc 100644 --- a/README.rst +++ b/README.rst @@ -30,10 +30,12 @@ If you'd like to contribute to this project please read `Developer Guide Installation ------------ -The easiest way to install Certbot is by visiting https://certbot.eff.org, where you can +The easiest way to install Certbot is by visiting `certbot.eff.org`_, where you can find the correct installation instructions for many web server and OS combinations. For more information, see the `User Guide `_. +.. _certbot.eff.org: https://certbot.eff.org/ + How to run the client --------------------- @@ -94,6 +96,10 @@ ACME working area in github: https://github.com/ietf-wg-acme/acme Mailing list: `client-dev`_ (to subscribe without a Google account, send an email to client-dev+subscribe@letsencrypt.org) +.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt +.. _OFTC: https://webchat.oftc.net?channels=%23certbot +.. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev + |build-status| |coverage| |docs| |container| diff --git a/acme/acme/client.py b/acme/acme/client.py index 117ee6b7d..de7eef299 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -89,8 +89,6 @@ class Client(object): # pylint: disable=too-many-instance-attributes :returns: Registration Resource. :rtype: `.RegistrationResource` - :raises .UnexpectedUpdate: - """ new_reg = messages.NewRegistration() if new_reg is None else new_reg assert isinstance(new_reg, messages.NewRegistration) @@ -101,12 +99,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes # "Instance of 'Field' has no key/contact member" bug: # pylint: disable=no-member - regr = self._regr_from_response(response) - if (regr.body.key != self.key.public_key() or - regr.body.contact != new_reg.contact): - raise errors.UnexpectedUpdate(regr) - - return regr + return self._regr_from_response(response) def _send_recv_regr(self, regr, body): response = self.net.post(regr.uri, body) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index a526a0984..585576e2d 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -102,12 +102,6 @@ class ClientTest(unittest.TestCase): self.assertEqual(self.regr, self.client.register(self.new_reg)) # TODO: test POST call arguments - # TODO: split here and separate test - reg_wrong_key = self.regr.body.update(key=KEY2.public_key()) - self.response.json.return_value = reg_wrong_key.to_json() - self.assertRaises( - errors.UnexpectedUpdate, self.client.register, self.new_reg) - def test_register_missing_next(self): self.response.status_code = http_client.CREATED self.assertRaises( diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 17ff6c8db..75fbe3456 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -98,6 +98,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): help="Apache server root directory.") add("vhost-root", default=constants.os_constant("vhost_root"), help="Apache server VirtualHost configuration root") + add("logs-root", default=constants.os_constant("logs_root"), + help="Apache server logs directory") add("challenge-location", default=constants.os_constant("challenge_location"), help="Directory path for challenge configuration.") @@ -1425,13 +1427,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "RewriteEngine On\n" "RewriteRule %s\n" "\n" - "ErrorLog /var/log/apache2/redirect.error.log\n" + "ErrorLog %s/redirect.error.log\n" "LogLevel warn\n" "\n" % (" ".join(str(addr) for addr in self._get_proposed_addrs(ssl_vhost)), servername, serveralias, - " ".join(rewrite_rule_args))) + " ".join(rewrite_rule_args), + self.conf("logs-root"))) def _write_out_redirect(self, ssl_vhost, text): # This is the default name diff --git a/certbot-apache/certbot_apache/constants.py b/certbot-apache/certbot_apache/constants.py index ba545c613..dcc635c4b 100644 --- a/certbot-apache/certbot_apache/constants.py +++ b/certbot-apache/certbot_apache/constants.py @@ -6,6 +6,7 @@ CLI_DEFAULTS_DEFAULT = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", vhost_files="*", + logs_root="/var/log/apache2", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apache2ctl', 'graceful'], @@ -23,6 +24,7 @@ CLI_DEFAULTS_DEBIAN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/sites-available", vhost_files="*", + logs_root="/var/log/apache2", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apache2ctl', 'graceful'], @@ -40,6 +42,7 @@ CLI_DEFAULTS_CENTOS = dict( server_root="/etc/httpd", vhost_root="/etc/httpd/conf.d", vhost_files="*.conf", + logs_root="/var/log/httpd", version_cmd=['apachectl', '-v'], define_cmd=['apachectl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apachectl', 'graceful'], @@ -57,6 +60,7 @@ CLI_DEFAULTS_GENTOO = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", vhost_files="*.conf", + logs_root="/var/log/apache2", version_cmd=['/usr/sbin/apache2', '-v'], define_cmd=['apache2ctl', 'virtualhosts'], restart_cmd=['apache2ctl', 'graceful'], @@ -74,6 +78,7 @@ CLI_DEFAULTS_DARWIN = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/other", vhost_files="*.conf", + logs_root="/var/log/apache2", version_cmd=['/usr/sbin/httpd', '-v'], define_cmd=['/usr/sbin/httpd', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apachectl', 'graceful'], @@ -91,6 +96,7 @@ CLI_DEFAULTS_SUSE = dict( server_root="/etc/apache2", vhost_root="/etc/apache2/vhosts.d", vhost_files="*.conf", + logs_root="/var/log/apache2", version_cmd=['apache2ctl', '-v'], define_cmd=['apache2ctl', '-t', '-D', 'DUMP_RUN_CFG'], restart_cmd=['apache2ctl', 'graceful'], diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index a1c24b5c8..444e48903 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -55,7 +55,9 @@ class NginxConfigurator(common.Plugin): """ - description = "Nginx Web Server - currently doesn't work" + description = "Nginx Web Server plugin - Alpha" + + hidden = True @classmethod def add_parser_arguments(cls, add): @@ -117,6 +119,8 @@ class NginxConfigurator(common.Plugin): # Make sure configuration is valid self.config_test() + # temp_install must be run before creating the NginxParser + temp_install(self.mod_ssl_conf) self.parser = parser.NginxParser( self.conf('server-root'), self.mod_ssl_conf) @@ -124,8 +128,6 @@ class NginxConfigurator(common.Plugin): if self.version is None: self.version = self.get_version() - temp_install(self.mod_ssl_conf) - # Entry point in main.py for installing cert def deploy_cert(self, domain, cert_path, key_path, chain_path=None, fullchain_path=None): @@ -337,10 +339,16 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() - ssl_block = [['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)], - ['\n ', 'ssl_certificate', ' ', snakeoil_cert], - ['\n ', 'ssl_certificate_key', ' ', snakeoil_key], - ['\n ', 'include', ' ', self.parser.loc["ssl_options"]]] + + # the options file doesn't have a newline at the beginning, but there + # needs to be one when it's dropped into the file + ssl_block = ( + [['\n ', 'listen', ' ', '{0} ssl'.format(self.config.tls_sni_01_port)], + ['\n ', 'ssl_certificate', ' ', snakeoil_cert], + ['\n ', 'ssl_certificate_key', ' ', snakeoil_key], + ['\n']] + + self.parser.loc["ssl_options"]) + self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block, replace=False) vhost.ssl = True diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index c7a9f0c75..6f2a3ec70 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -227,7 +227,8 @@ class UnspacedList(list): def insert(self, i, x): item, spaced_item = self._coerce(x) - self.spaced.insert(self._spaced_position(i), spaced_item) + slicepos = self._spaced_position(i) if i < len(self) else len(self.spaced) + self.spaced.insert(slicepos, spaced_item) list.insert(self, i, item) self.dirty = True diff --git a/certbot-nginx/certbot_nginx/options-ssl-nginx.conf b/certbot-nginx/certbot_nginx/options-ssl-nginx.conf index f0081c1fc..89c920b3e 100644 --- a/certbot-nginx/certbot_nginx/options-ssl-nginx.conf +++ b/certbot-nginx/certbot_nginx/options-ssl-nginx.conf @@ -1,8 +1,7 @@ -ssl_session_cache shared:SSL:1m; +ssl_session_cache shared:le_nginx_SSL:1m; ssl_session_timeout 1440m; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; ssl_prefer_server_ciphers on; -# Using list of ciphers from "Bulletproof SSL and TLS" ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384 ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"; diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index aae6ce122..3919858d9 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -173,6 +173,17 @@ class NginxParser(object): logger.debug("Could not parse file: %s", item) return trees + def _parse_ssl_options(self, ssl_options): + if ssl_options is not None: + try: + with open(ssl_options) as _file: + return nginxparser.load(_file).spaced + except IOError: + logger.warn("Missing NGINX TLS options file: %s", ssl_options) + except pyparsing.ParseBaseException: + logger.debug("Could not parse file: %s", ssl_options) + return [] + def _set_locations(self, ssl_options): """Set default location for directives. @@ -192,7 +203,7 @@ class NginxParser(object): name = default return {"root": root, "default": default, "listen": listen, - "name": name, "ssl_options": ssl_options} + "name": name, "ssl_options": self._parse_ssl_options(ssl_options)} def _find_config_root(self): """Find the Nginx Configuration Root file.""" @@ -318,6 +329,9 @@ class NginxParser(object): tup = [None, None, vhost.filep] if vhost.ssl: for directive in vhost.raw: + # A directive can be an empty list to preserve whitespace + if not directive: + continue if directive[0] == 'ssl_certificate': tup[0] = directive[1] elif directive[0] == 'ssl_certificate_key': @@ -506,8 +520,30 @@ def _add_directives(block, directives, replace): """ for directive in directives: _add_directive(block, directive, replace) + if block and '\n' not in block[-1]: # could be " \n " or ["\n"] ! + block.append(nginxparser.UnspacedList('\n')) + + +REPEATABLE_DIRECTIVES = set(['server_name', 'listen', 'include']) +COMMENT = ' managed by Certbot' +COMMENT_BLOCK = [' ', '#', COMMENT] + + +def _comment_directive(block, location): + """Add a comment to the end of the line at location.""" + next_entry = block[location + 1] if location + 1 < len(block) else None + if isinstance(next_entry, list) and next_entry: + if len(next_entry) >= 2 and next_entry[-2] == "#" and COMMENT in next_entry[-1]: + return + elif isinstance(next_entry, nginxparser.UnspacedList): + next_entry = next_entry.spaced[0] + else: + next_entry = next_entry[0] + + block.insert(location + 1, COMMENT_BLOCK[:]) + if next_entry is not None and "\n" not in next_entry: + block.insert(location + 2, '\n') -repeatable_directives = set(['server_name', 'listen', 'include']) def _add_directive(block, directive, replace): """Adds or replaces a single directive in a config block. @@ -516,37 +552,34 @@ def _add_directive(block, directive, replace): """ directive = nginxparser.UnspacedList(directive) - if len(directive) == 0: - # whitespace + if len(directive) == 0 or directive[0] == '#': + # whitespace or comment block.append(directive) return - location = -1 + # Find the index of a config line where the name of the directive matches - # the name of the directive we want to add. - for index, line in enumerate(block): - if len(line) > 0 and line[0] == directive[0]: - location = index - break + # the name of the directive we want to add. If no line exists, use None. + location = next((index for index, line in enumerate(block) + if line and line[0] == directive[0]), None) if replace: - if location == -1: + if location is None: raise errors.MisconfigurationError( - 'expected directive for %s in the Nginx ' - 'config but did not find it.' % directive[0]) + 'expected directive for {0} in the Nginx ' + 'config but did not find it.'.format(directive[0])) block[location] = directive + _comment_directive(block, location) else: # Append directive. Fail if the name is not a repeatable directive name, # and there is already a copy of that directive with a different value # in the config file. directive_name = directive[0] directive_value = directive[1] - if location != -1 and directive_name.__str__() not in repeatable_directives: - if block[location][1] == directive_value: - # There's a conflict, but the existing value matches the one we - # want to insert, so it's fine. - pass - else: - raise errors.MisconfigurationError( - 'tried to insert directive "%s" but found conflicting "%s".' % ( - directive, block[location])) - else: + if location is None or (isinstance(directive_name, str) and + directive_name in REPEATABLE_DIRECTIVES): block.append(directive) + _comment_directive(block, len(block) - 1) + elif block[location][1] != directive_value: + raise errors.MisconfigurationError( + 'tried to insert directive "{0}" but found ' + 'conflicting "{1}".'.format(directive, block[location])) + diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index fede3bc08..9e0c0dda5 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -13,6 +13,7 @@ from acme import messages from certbot import achallenges from certbot import errors +from certbot_nginx import parser from certbot_nginx.tests import util @@ -39,6 +40,8 @@ class NginxConfiguratorTest(util.NginxTest): def test_prepare(self): self.assertEqual((1, 6, 2), self.config.version) self.assertEqual(5, len(self.config.parser.parsed)) + # ensure we successfully parsed a file for ssl_options + self.assertTrue(self.config.parser.loc["ssl_options"]) @mock.patch("certbot_nginx.configurator.util.exe_exists") @mock.patch("certbot_nginx.configurator.subprocess.Popen") @@ -89,13 +92,13 @@ class NginxConfiguratorTest(util.NginxTest): # pylint: disable=protected-access parsed = self.config.parser._parse_files(filep, override=True) - self.assertEqual([[['server'], [ - ['listen', '69.50.225.155:9000'], - ['listen', '127.0.0.1'], - ['server_name', '.example.com'], - ['server_name', 'example.*'], - ['listen', '5001 ssl'] - ]]], + self.assertEqual([[['server'], + [['listen', '69.50.225.155:9000'], + ['listen', '127.0.0.1'], + ['server_name', '.example.com'], + ['server_name', 'example.*'], + ['listen', '5001 ssl'], + ['#', parser.COMMENT]]]], parsed[0]) def test_choose_vhost(self): @@ -216,9 +219,9 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '5001 ssl'], ['ssl_certificate', 'example/fullchain.pem'], - ['ssl_certificate_key', 'example/key.pem'], - ['include', self.config.parser.loc["ssl_options"]] - ]]], + ['ssl_certificate_key', 'example/key.pem']] + + util.filter_comments(self.config.parser.loc["ssl_options"]) + ]], parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], parsed_server_conf) @@ -234,8 +237,9 @@ class NginxConfiguratorTest(util.NginxTest): ['index', 'index.html index.htm']]], ['listen', '5001 ssl'], ['ssl_certificate', '/etc/nginx/fullchain.pem'], - ['ssl_certificate_key', '/etc/nginx/key.pem'], - ['include', self.config.parser.loc["ssl_options"]]]], + ['ssl_certificate_key', '/etc/nginx/key.pem']] + + util.filter_comments(self.config.parser.loc["ssl_options"]) + ], 2)) def test_get_all_certs_keys(self): diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py index 5fa9a7d1e..5c8d6d215 100644 --- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py +++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py @@ -223,6 +223,26 @@ class TestUnspacedList(unittest.TestCase): self.assertRaises(IndexError, self.ul2.__getitem__, 2) self.assertRaises(IndexError, self.ul2.__getitem__, -3) + def test_insert(self): + x = UnspacedList( + [['\n ', 'listen', ' ', '69.50.225.155:9000'], + ['\n ', 'listen', ' ', '127.0.0.1'], + ['\n ', 'server_name', ' ', '.example.com'], + ['\n ', 'server_name', ' ', 'example.*'], '\n', + ['listen', ' ', '5001 ssl']]) + x.insert(5, "FROGZ") + self.assertEqual(x, + [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], + ['server_name', '.example.com'], ['server_name', 'example.*'], + ['listen', '5001 ssl'], 'FROGZ']) + self.assertEqual(x.spaced, + [['\n ', 'listen', ' ', '69.50.225.155:9000'], + ['\n ', 'listen', ' ', '127.0.0.1'], + ['\n ', 'server_name', ' ', '.example.com'], + ['\n ', 'server_name', ' ', 'example.*'], '\n', + ['listen', ' ', '5001 ssl'], + 'FROGZ']) + def test_rawlists(self): ul3 = copy.deepcopy(self.ul) ul3.insert(0, "some") diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index 77b4a877a..71807d4f4 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -141,10 +141,14 @@ class NginxParserTest(util.NginxTest): replace=False) nparser.add_server_directives(server_conf, names, [['foo', 'bar']], replace=False) + from certbot_nginx.parser import COMMENT self.assertEqual(nparser.parsed[server_conf], [['server_name', 'somename alias another.alias'], ['foo', 'bar'], - ['ssl_certificate', '/etc/ssl/cert2.pem'] + ['#', COMMENT], + ['ssl_certificate', '/etc/ssl/cert2.pem'], + ['#', COMMENT], + [], [] ]) def test_add_http_directives(self): @@ -170,12 +174,13 @@ class NginxParserTest(util.NginxTest): filep = nparser.abs_path('sites-enabled/example.com') nparser.add_server_directives( filep, target, [['server_name', 'foobar.com']], replace=True) + from certbot_nginx.parser import COMMENT self.assertEqual( nparser.parsed[filep], [[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], - ['server_name', 'foobar.com'], - ['server_name', 'example.*'], + ['server_name', 'foobar.com'], ['#', COMMENT], + ['server_name', 'example.*'], [] ]]]) self.assertRaises(errors.MisconfigurationError, nparser.add_server_directives, @@ -216,6 +221,23 @@ class NginxParserTest(util.NginxTest): self.assertEqual(winner, parser.get_best_match(target_name, names[i])) + def test_comment_directive(self): + # pylint: disable=protected-access + block = nginxparser.UnspacedList([ + ["\n", "a", " ", "b", "\n"], + ["c", " ", "d"], + ["\n", "e", " ", "f"]]) + from certbot_nginx.parser import _comment_directive, COMMENT_BLOCK + _comment_directive(block, 1) + _comment_directive(block, 0) + self.assertEqual(block.spaced, [ + ["\n", "a", " ", "b", "\n"], + COMMENT_BLOCK, + "\n", + ["c", " ", "d"], + COMMENT_BLOCK, + ["\n", "e", " ", "f"]]) + def test_get_all_certs_keys(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) filep = nparser.abs_path('sites-enabled/example.com') @@ -249,5 +271,22 @@ class NginxParserTest(util.NginxTest): ]) self.assertTrue(server['ssl']) + def test_ssl_options_should_be_parsed_ssl_directives(self): + nparser = parser.NginxParser(self.config_path, self.ssl_options) + self.assertEqual(nginxparser.UnspacedList(nparser.loc["ssl_options"]), + [['ssl_session_cache', 'shared:le_nginx_SSL:1m'], + ['ssl_session_timeout', '1440m'], + ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], + ['ssl_prefer_server_ciphers', 'on'], + ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-'+ + 'AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256'+ + '-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384'+ + ' ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384'+ + ' ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-'+ + 'AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM'+ + '-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-'+ + 'AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"'] + ]) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py index 866e5a9c7..96fdac527 100644 --- a/certbot-nginx/certbot_nginx/tests/util.py +++ b/certbot-nginx/certbot_nginx/tests/util.py @@ -17,6 +17,7 @@ from certbot.plugins import common from certbot_nginx import constants from certbot_nginx import configurator +from certbot_nginx import nginxparser class NginxTest(unittest.TestCase): # pylint: disable=too-few-public-methods @@ -84,14 +85,20 @@ def filter_comments(tree): def traverse(tree): """Generator dropping comment nodes""" for entry in tree: - key, values = entry + # key, values = entry + spaceless = [e for e in entry if not nginxparser.spacey(e)] + if spaceless: + key = spaceless[0] + values = spaceless[1] if len(spaceless) > 1 else None + else: + key = values = "" if isinstance(key, list): new = copy.deepcopy(entry) new[1] = filter_comments(values) yield new else: - if key != '#': - yield entry + if key != '#' and spaceless: + yield spaceless return list(traverse(tree)) diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py index c7ec80931..40382cab0 100644 --- a/certbot-nginx/certbot_nginx/tls_sni_01.py +++ b/certbot-nginx/certbot_nginx/tls_sni_01.py @@ -145,7 +145,6 @@ class NginxTlsSni01(common.TLSSNI01): block.extend([['server_name', ' ', achall.response(achall.account_key).z_domain], - ['include', ' ', self.configurator.parser.loc["ssl_options"]], # access and error logs necessary for # integration testing (non-root) ['access_log', ' ', os.path.join( @@ -154,6 +153,7 @@ class NginxTlsSni01(common.TLSSNI01): self.configurator.config.work_dir, 'error.log')], ['ssl_certificate', ' ', self.get_cert_path(achall)], ['ssl_certificate_key', ' ', self.get_key_path(achall)], - [['location', ' ', '/'], [['root', ' ', document_root]]]]) + [['location', ' ', '/'], [['root', ' ', document_root]]]] + + self.configurator.parser.loc["ssl_options"]) return [['server'], block] diff --git a/certbot/client.py b/certbot/client.py index d80a67bad..66e90bb1f 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -150,8 +150,14 @@ def perform_registration(acme, config): return acme.register(messages.NewRegistration.from_data(email=config.email)) except messages.Error as e: if e.typ == "urn:acme:error:invalidEmail": - config.namespace.email = display_ops.get_email(invalid=True) - return perform_registration(acme, config) + if config.noninteractive_mode: + msg = ("The ACME server believes %s is an invalid email address. " + "Please ensure it is a valid email and attempt " + "registration again." % config.email) + raise errors.Error(msg) + else: + config.namespace.email = display_ops.get_email(invalid=True) + return perform_registration(acme, config) else: raise diff --git a/certbot/display/ops.py b/certbot/display/ops.py index 4db6d71e2..e8520fe96 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -48,7 +48,7 @@ def get_email(invalid=False, optional=True): invalid_prefix + msg if invalid else msg) except errors.MissingCommandlineFlag: msg = ("You should register before running non-interactively, " - "or provide --agree-tos and --email flags") + "or provide --agree-tos and --email flags.") raise errors.MissingCommandlineFlag(msg) if code != display_util.OK: diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index e1a064fb3..27ede6533 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -4,13 +4,7 @@ import unittest import sys import mock - -try: - # Python 3.5+ - from importlib import reload as refresh # pylint: disable=no-name-in-module -except ImportError: - # Python 2-3.4 - from imp import reload as refresh +from six.moves import reload_module # pylint: disable=import-error class PathSurgeryTest(unittest.TestCase): @@ -50,14 +44,14 @@ class AlreadyListeningTestNoPsutil(unittest.TestCase): sys.modules['psutil'] = None # Reload hackery to ensure getting non-psutil version # loaded to memory - refresh(certbot.plugins.util) + reload_module(certbot.plugins.util) def tearDown(self): # Need to reload the module to ensure # getting back to normal import certbot.plugins.util sys.modules["psutil"] = self.psutil - refresh(certbot.plugins.util) + reload_module(certbot.plugins.util) @mock.patch("certbot.plugins.util.zope.component.getUtility") def test_ports_available(self, mock_getutil): @@ -81,6 +75,42 @@ class AlreadyListeningTestNoPsutil(unittest.TestCase): self.assertEqual(mock_getutil.call_count, 2) +def psutil_available(): + """Checks if psutil can be imported. + + :rtype: bool + :returns: ``True`` if psutil can be imported, otherwise, ``False`` + + """ + try: + import psutil # pylint: disable=unused-variable + except ImportError: + return False + return True + + +def skipUnless(condition, reason): + """Skip tests unless a condition holds. + + This implements the basic functionality of unittest.skipUnless + which is only available on Python 2.7+. + + :param bool condition: If ``False``, the test will be skipped + :param str reason: the reason for skipping the test + + :rtype: callable + :returns: decorator that hides tests unless condition is ``True`` + + """ + if hasattr(unittest, "skipUnless"): + return unittest.skipUnless(condition, reason) + elif condition: + return lambda cls: cls + else: + return lambda cls: None + + +@skipUnless(psutil_available(), "optional dependency psutil is not available") class AlreadyListeningTestPsutil(unittest.TestCase): """Tests for certbot.plugins.already_listening.""" def _call(self, *args, **kwargs): diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 4a8a8bdee..98d853716 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -63,6 +63,7 @@ class RegisterTest(unittest.TestCase): @mock.patch("certbot.client.display_ops.get_email") def test_email_retry(self, _rep, mock_get_email): from acme import messages + self.config.noninteractive_mode = False msg = "DNS problem: NXDOMAIN looking up MX for example.com" mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") with mock.patch("certbot.client.acme_client.Client") as mock_client: @@ -70,6 +71,15 @@ class RegisterTest(unittest.TestCase): self._call() self.assertEqual(mock_get_email.call_count, 1) + @mock.patch("certbot.account.report_new_account") + def test_email_invalid_noninteractive(self, _rep): + from acme import messages + msg = "DNS problem: NXDOMAIN looking up MX for example.com" + mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") + with mock.patch("certbot.client.acme_client.Client") as mock_client: + mock_client().register.side_effect = [mx_err, mock.MagicMock()] + self.assertRaises(errors.Error, self._call) + def test_needs_email(self): self.config.email = None self.assertRaises(errors.Error, self._call) diff --git a/docs/resources.rst b/docs/resources.rst index a284f4a3d..f10aa2920 100644 --- a/docs/resources.rst +++ b/docs/resources.rst @@ -24,6 +24,10 @@ ACME working area in github: https://github.com/ietf-wg-acme/acme Mailing list: `client-dev`_ (to subscribe without a Google account, send an email to client-dev+subscribe@letsencrypt.org) +.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt +.. _OFTC: https://webchat.oftc.net?channels=%23certbot +.. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev + |build-status| |coverage| |docs| |container| @@ -45,10 +49,6 @@ email to client-dev+subscribe@letsencrypt.org) :alt: Docker Repository on Quay.io .. _`installation instructions`: - https://letsencrypt.readthedocs.org/en/latest/using.html + https://letsencrypt.readthedocs.org/en/latest/using.html#getting-certbot .. _watch demo video: https://www.youtube.com/watch?v=Gas_sSB-5SU - -.. _Freenode: https://webchat.freenode.net?channels=%23letsencrypt -.. _OFTC: https://webchat.oftc.net?channels=%23certbot -.. _client-dev: https://groups.google.com/a/letsencrypt.org/forum/#!forum/client-dev diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 27b11c656..5979b0848 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -281,6 +281,30 @@ BootstrapRpmCommon() { exit 1 fi + if [ "$ASSUME_YES" = 1 ]; then + yes_flag="-y" + fi + + if ! $SUDO $tool list *virtualenv >/dev/null 2>&1; then + echo "To use Certbot, packages from the EPEL repository need to be installed." + if ! $SUDO $tool list epel-release >/dev/null 2>&1; then + echo "Please enable this repository and try running Certbot again." + exit 1 + fi + if [ "$ASSUME_YES" = 1 ]; then + /bin/echo -n "Enabling the EPEL repository in 3 seconds..." + sleep 1s + /bin/echo -ne "\e[0K\rEnabling the EPEL repository in 2 seconds..." + sleep 1s + /bin/echo -e "\e[0K\rEnabling the EPEL repository in 1 seconds..." + sleep 1s + fi + if ! $SUDO $tool install $yes_flag epel-release; then + echo "Could not enable EPEL. Aborting bootstrap!" + exit 1 + fi + fi + pkgs=" gcc dialog @@ -318,10 +342,6 @@ BootstrapRpmCommon() { " fi - if [ "$ASSUME_YES" = 1 ]; then - yes_flag="-y" - fi - if ! $SUDO $tool install $yes_flag $pkgs; then echo "Could not install OS dependencies. Aborting bootstrap!" exit 1 diff --git a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh index 0f98b4bbc..2fd629ff8 100755 --- a/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh +++ b/letsencrypt-auto-source/pieces/bootstrappers/rpm_common.sh @@ -17,6 +17,30 @@ BootstrapRpmCommon() { exit 1 fi + if [ "$ASSUME_YES" = 1 ]; then + yes_flag="-y" + fi + + if ! $SUDO $tool list *virtualenv >/dev/null 2>&1; then + echo "To use Certbot, packages from the EPEL repository need to be installed." + if ! $SUDO $tool list epel-release >/dev/null 2>&1; then + echo "Please enable this repository and try running Certbot again." + exit 1 + fi + if [ "$ASSUME_YES" = 1 ]; then + /bin/echo -n "Enabling the EPEL repository in 3 seconds..." + sleep 1s + /bin/echo -ne "\e[0K\rEnabling the EPEL repository in 2 seconds..." + sleep 1s + /bin/echo -e "\e[0K\rEnabling the EPEL repository in 1 seconds..." + sleep 1s + fi + if ! $SUDO $tool install $yes_flag epel-release; then + echo "Could not enable EPEL. Aborting bootstrap!" + exit 1 + fi + fi + pkgs=" gcc dialog @@ -54,10 +78,6 @@ BootstrapRpmCommon() { " fi - if [ "$ASSUME_YES" = 1 ]; then - yes_flag="-y" - fi - if ! $SUDO $tool install $yes_flag $pkgs; then echo "Could not install OS dependencies. Aborting bootstrap!" exit 1 diff --git a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt index 0c642a33e..342aa2f88 100644 --- a/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt +++ b/letsencrypt-auto-source/pieces/letsencrypt-auto-requirements.txt @@ -1,6 +1,7 @@ # This is the flattened list of packages certbot-auto installs. To generate -# this, do `pip install --no-cache-dir -e acme -e . -e certbot-apache`, and -# then use `hashin` or a more secure method to gather the hashes. +# this, do +# `pip install --no-cache-dir -e acme -e . -e certbot-apache -e certbot-nginx`, +# and then use `hashin` or a more secure method to gather the hashes. argparse==1.4.0 \ --hash=sha256:c31647edb69fd3d465a847ea3157d37bed1f95f19760b11a47aa91c04b666314 \ @@ -117,6 +118,15 @@ pyasn1==0.1.9 \ pyopenssl==16.0.0 \ --hash=sha256:5add70cf00273bf957ca31fdb0df9b0ae4639e081897d5f86a0ae1f104901230 \ --hash=sha256:363d10ee43d062285facf4e465f4f5163f9f702f9134f0a5896f134cbb92d17d +pyparsing==2.1.8 \ + --hash=sha256:2f0f5ceb14eccd5aef809d6382e87df22ca1da583c79f6db01675ce7d7f49c18 \ + --hash=sha256:03a4869b9f3493807ee1f1cb405e6d576a1a2ca4d81a982677c0c1ad6177c56b \ + --hash=sha256:ab09aee814c0241ff0c503cff30018219fe1fc14501d89f406f4664a0ec9fbcd \ + --hash=sha256:6e9a7f052f8e26bcf749e4033e3115b6dc7e3c85aafcb794b9a88c9d9ef13c97 \ + --hash=sha256:9f463a6bcc4eeb6c08f1ed84439b17818e2085937c0dee0d7674ac127c67c12b \ + --hash=sha256:3626b4d81cfb300dad57f52f2f791caaf7b06c09b368c0aa7b868e53a5775424 \ + --hash=sha256:367b90cc877b46af56d4580cd0ae278062903f02b8204ab631f5a2c0f50adfd0 \ + --hash=sha256:9f1ea360086cd68681e7f4ca8f1f38df47bf81942a0d76a9673c2d23eff35b13 pyRFC3339==1.0 \ --hash=sha256:eea31835c56e2096af4363a5745a784878a61d043e247d3a6d6a0a32a9741f56 \ --hash=sha256:8dfbc6c458b8daba1c0f3620a8c78008b323a268b27b7359e92a4ae41325f535 diff --git a/tools/release.sh b/tools/release.sh index c883e3d61..7747b0e2b 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -164,13 +164,13 @@ for module in certbot $subpkgs_modules ; do done # pin pip hashes of the things we just built -for pkg in acme certbot certbot-apache ; do +for pkg in acme certbot certbot-apache certbot-nginx ; do echo $pkg==$version \\ pip hash dist."$version/$pkg"/*.{whl,gz} | grep "^--hash" | python2 -c 'from sys import stdin; input = stdin.read(); print " ", input.replace("\n--hash", " \\\n --hash"),' done > /tmp/hashes.$$ deactivate -if ! wc -l /tmp/hashes.$$ | grep -qE "^\s*9 " ; then +if ! wc -l /tmp/hashes.$$ | grep -qE "^\s*12 " ; then echo Unexpected pip hash output exit 1 fi