diff options
author | Erica Portnoy <ebportnoy@gmail.com> | 2016-12-06 06:17:04 +0300 |
---|---|---|
committer | Peter Eckersley <pde@users.noreply.github.com> | 2016-12-06 06:17:04 +0300 |
commit | f0a7bb0e33d2d864a83050f46ec1efdff1b47949 (patch) | |
tree | 30667845312a96e9da4cfc3fc6a7acaf3d9fb45a /certbot-nginx | |
parent | 3dbf5c9fcb809c03e8098d3ee493b2aafc3fc13b (diff) |
Mark Nginx vhosts as ssl when any vhost is on ssl at that address (#3856)
* Move parse_server to be a method of NginxParser
* add super equal method to more correctly check addr equality in nginx should we support ipv6 in nginx in the future
* add addr:normalized_tuple method
* mark addresses listening sslishly due to another server block listening sslishly on that address
* test turning on ssl globally
* add docstring
* lint and remove extra file
Diffstat (limited to 'certbot-nginx')
5 files changed, 147 insertions, 60 deletions
diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py index 4a3ca865e..98bf86f5c 100644 --- a/certbot-nginx/certbot_nginx/obj.py +++ b/certbot-nginx/certbot_nginx/obj.py @@ -93,9 +93,16 @@ class Addr(common.Addr): def __repr__(self): return "Addr(" + self.__str__() + ")" + def super_eq(self, other): + """Check ip/port equality, with IPv6 support. + """ + # Nginx plugin currently doesn't support IPv6 but this will + # future-proof it + return super(Addr, self).__eq__(other) + def __eq__(self, other): if isinstance(other, self.__class__): - return (self.tup == other.tup and + return (self.super_eq(other) and self.ssl == other.ssl and self.default == other.default) return False diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 385635212..1a2c85c2c 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -82,21 +82,28 @@ class NginxParser(object): else: return path - def get_vhosts(self): - # pylint: disable=cell-var-from-loop - """Gets list of all 'virtual hosts' found in Nginx configuration. - Technically this is a misnomer because Nginx does not have virtual - hosts, it has 'server blocks'. - - :returns: List of :class:`~certbot_nginx.obj.VirtualHost` - objects found in configuration - :rtype: list + def _build_addr_to_ssl(self): + """Builds a map from address to whether it listens on ssl in any server block + """ + servers = self._get_raw_servers() + addr_to_ssl = {} + for filename in servers: + for server, _ in servers[filename]: + # Parse the server block to save addr info + parsed_server = _parse_server_raw(server) + for addr in parsed_server['addrs']: + addr_tuple = addr.normalized_tuple() + if addr_tuple not in addr_to_ssl: + addr_to_ssl[addr_tuple] = addr.ssl + addr_to_ssl[addr_tuple] = addr.ssl or addr_to_ssl[addr_tuple] + return addr_to_ssl + + def _get_raw_servers(self): + # pylint: disable=cell-var-from-loop + """Get a map of unparsed all server blocks """ - enabled = True # We only look at enabled vhosts for now - vhosts = [] servers = {} - for filename in self.parsed: tree = self.parsed[filename] servers[filename] = [] @@ -110,12 +117,28 @@ class NginxParser(object): for i, (server, path) in enumerate(servers[filename]): new_server = self._get_included_directives(server) servers[filename][i] = (new_server, path) + return servers + def get_vhosts(self): + # pylint: disable=cell-var-from-loop + """Gets list of all 'virtual hosts' found in Nginx configuration. + Technically this is a misnomer because Nginx does not have virtual + hosts, it has 'server blocks'. + + :returns: List of :class:`~certbot_nginx.obj.VirtualHost` + objects found in configuration + :rtype: list + + """ + enabled = True # We only look at enabled vhosts for now + servers = self._get_raw_servers() + + vhosts = [] for filename in servers: for server, path in servers[filename]: # Parse the server block into a VirtualHost object - parsed_server = parse_server(server) + parsed_server = _parse_server_raw(server) vhost = obj.VirtualHost(filename, parsed_server['addrs'], parsed_server['ssl'], @@ -125,8 +148,20 @@ class NginxParser(object): path) vhosts.append(vhost) + self._update_vhosts_addrs_ssl(vhosts) + return vhosts + def _update_vhosts_addrs_ssl(self, vhosts): + """Update a list of raw parsed vhosts to include global address sslishness + """ + addr_to_ssl = self._build_addr_to_ssl() + for vhost in vhosts: + for addr in vhost.addrs: + addr.ssl = addr_to_ssl[addr.normalized_tuple()] + if addr.ssl: + vhost.ssl = True + def _get_included_directives(self, block): """Returns array with the "include" directives expanded out by concatenating the contents of the included file to the block. @@ -241,6 +276,17 @@ class NginxParser(object): except IOError: logger.error("Could not open file for writing: %s", filename) + def parse_server(self, server): + """Parses a list of server directives, accounting for global address sslishness. + + :param list server: list of directives in a server block + :rtype: dict + """ + addr_to_ssl = self._build_addr_to_ssl() + parsed_server = _parse_server_raw(server) + _apply_global_addr_ssl(addr_to_ssl, parsed_server) + return parsed_server + def has_ssl_on_directive(self, vhost): """Does vhost have ssl on for all ports? @@ -290,7 +336,7 @@ class NginxParser(object): # update vhost based on new directives new_server = self._get_included_directives(result) - parsed_server = parse_server(new_server) + parsed_server = self.parse_server(new_server) vhost.addrs = parsed_server['addrs'] vhost.ssl = parsed_server['ssl'] vhost.names = parsed_server['names'] @@ -434,41 +480,6 @@ def _get_servernames(names): names = re.sub(whitespace_re, ' ', names) return names.split(' ') - -def parse_server(server): - """Parses a list of server directives. - - :param list server: list of directives in a server block - :rtype: dict - - """ - parsed_server = {'addrs': set(), - 'ssl': False, - 'names': set()} - - apply_ssl_to_all_addrs = False - - for directive in server: - if not directive: - continue - if directive[0] == 'listen': - addr = obj.Addr.fromstring(directive[1]) - parsed_server['addrs'].add(addr) - if not parsed_server['ssl'] and addr.ssl: - parsed_server['ssl'] = True - elif directive[0] == 'server_name': - parsed_server['names'].update( - _get_servernames(directive[1])) - elif directive[0] == 'ssl' and directive[1] == 'on': - parsed_server['ssl'] = True - apply_ssl_to_all_addrs = True - - if apply_ssl_to_all_addrs: - for addr in parsed_server['addrs']: - addr.ssl = True - - return parsed_server - def _add_directives(block, directives, replace): """Adds or replaces directives in a config block. @@ -549,3 +560,44 @@ def _add_directive(block, directive, replace): 'tried to insert directive "{0}" but found ' 'conflicting "{1}".'.format(directive, block[location])) +def _apply_global_addr_ssl(addr_to_ssl, parsed_server): + """Apply global sslishness information to the parsed server block + """ + for addr in parsed_server['addrs']: + addr.ssl = addr_to_ssl[addr.normalized_tuple()] + if addr.ssl: + parsed_server['ssl'] = True + +def _parse_server_raw(server): + """Parses a list of server directives. + + :param list server: list of directives in a server block + :rtype: dict + + """ + parsed_server = {'addrs': set(), + 'ssl': False, + 'names': set()} + + apply_ssl_to_all_addrs = False + + for directive in server: + if not directive: + continue + if directive[0] == 'listen': + addr = obj.Addr.fromstring(directive[1]) + parsed_server['addrs'].add(addr) + if addr.ssl: + parsed_server['ssl'] = True + elif directive[0] == 'server_name': + parsed_server['names'].update( + _get_servernames(directive[1])) + elif directive[0] == 'ssl' and directive[1] == 'on': + parsed_server['ssl'] = True + apply_ssl_to_all_addrs = True + + if apply_ssl_to_all_addrs: + for addr in parsed_server['addrs']: + addr.ssl = True + + return parsed_server diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index f165ea23a..08a66fc98 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -40,7 +40,7 @@ class NginxConfiguratorTest(util.NginxTest): def test_prepare(self): self.assertEqual((1, 6, 2), self.config.version) - self.assertEqual(7, len(self.config.parser.parsed)) + self.assertEqual(8, len(self.config.parser.parsed)) # ensure we successfully parsed a file for ssl_options self.assertTrue(self.config.parser.loc["ssl_options"]) @@ -68,7 +68,8 @@ class NginxConfiguratorTest(util.NginxTest): names = self.config.get_all_names() self.assertEqual(names, set( ["155.225.50.69.nephoscale.net", "www.example.org", "another.alias", - "migration.com", "summer.com", "geese.com", "sslon.com"])) + "migration.com", "summer.com", "geese.com", "sslon.com", + "globalssl.com", "globalsslsetssl.com"])) def test_supported_enhancements(self): self.assertEqual(['redirect', 'staple-ocsp'], diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index 54deffd7a..921cc3c5a 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -49,7 +49,8 @@ class NginxParserTest(util.NginxTest): 'sites-enabled/default', 'sites-enabled/example.com', 'sites-enabled/migration.com', - 'sites-enabled/sslon.com']]), + 'sites-enabled/sslon.com', + 'sites-enabled/globalssl.com']]), set(nparser.parsed.keys())) self.assertEqual([['server_name', 'somename alias another.alias']], nparser.parsed[nparser.abs_path('server.conf')]) @@ -73,7 +74,7 @@ class NginxParserTest(util.NginxTest): parsed = nparser._parse_files(nparser.abs_path( 'sites-enabled/example.com.test')) self.assertEqual(3, len(glob.glob(nparser.abs_path('*.test')))) - self.assertEqual(4, len( + self.assertEqual(5, len( glob.glob(nparser.abs_path('sites-enabled/*.test')))) self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], @@ -104,6 +105,16 @@ class NginxParserTest(util.NginxTest): lambda x, y, pts=paths: pts.append(y)) self.assertEqual(paths, result) + def test_get_vhosts_global_ssl(self): + nparser = parser.NginxParser(self.config_path, self.ssl_options) + vhosts = nparser.get_vhosts() + + vhost = obj.VirtualHost(nparser.abs_path('sites-enabled/globalssl.com'), + [obj.Addr('4.8.2.6', '57', True, False)], + True, True, set(['globalssl.com']), [], [0]) + + globalssl_com = [x for x in vhosts if 'globalssl.com' in x.filep][0] + self.assertEqual(vhost, globalssl_com) def test_get_vhosts(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) @@ -137,7 +148,7 @@ class NginxParserTest(util.NginxTest): '*.www.example.com']), [], [2, 1, 0]) - self.assertEqual(8, len(vhosts)) + self.assertEqual(10, len(vhosts)) example_com = [x for x in vhosts if 'example.com' in x.filep][0] self.assertEqual(vhost3, example_com) default = [x for x in vhosts if 'default' in x.filep][0] @@ -291,27 +302,34 @@ class NginxParserTest(util.NginxTest): COMMENT_BLOCK, ["\n", "e", " ", "f"]]) - def test_parse_server_ssl(self): - server = parser.parse_server([ + def test_parse_server_raw_ssl(self): + server = parser._parse_server_raw([ #pylint: disable=protected-access ['listen', '443'] ]) self.assertFalse(server['ssl']) - server = parser.parse_server([ + server = parser._parse_server_raw([ #pylint: disable=protected-access ['listen', '443 ssl'] ]) self.assertTrue(server['ssl']) - server = parser.parse_server([ + server = parser._parse_server_raw([ #pylint: disable=protected-access ['listen', '443'], ['ssl', 'off'] ]) self.assertFalse(server['ssl']) - server = parser.parse_server([ + server = parser._parse_server_raw([ #pylint: disable=protected-access ['listen', '443'], ['ssl', 'on'] ]) self.assertTrue(server['ssl']) + def test_parse_server_global_ssl_applied(self): + nparser = parser.NginxParser(self.config_path, self.ssl_options) + server = nparser.parse_server([ + ['listen', '443'] + ]) + 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"]), diff --git a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/globalssl.com b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/globalssl.com new file mode 100644 index 000000000..969447d6e --- /dev/null +++ b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/globalssl.com @@ -0,0 +1,9 @@ +server { + server_name globalssl.com; + listen 4.8.2.6:57; +} + +server { + server_name globalsslsetssl.com; + listen 4.8.2.6:57 ssl; +} |