diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 049ba9a20..444e48903 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -119,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) @@ -126,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): @@ -339,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/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