diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 2a7f9ce52..50a7a9dc0 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -21,6 +21,7 @@ from typing import Set from typing import Tuple from typing import Type from typing import Union +from typing import cast from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import rsa @@ -371,13 +372,14 @@ class NginxConfigurator(common.Configurator): return vhosts - def ipv6_info(self, port: str) -> Tuple[bool, bool]: + def ipv6_info(self, host: str, port: str) -> Tuple[bool, bool]: """Returns tuple of booleans (ipv6_active, ipv6only_present) ipv6_active is true if any server block listens ipv6 address in any port ipv6only_present is true if ipv6only=on option exists in any server block ipv6 listen directive for the specified port. + :param str host: Host to check ipv6only=on directive for :param str port: Port to check ipv6only=on directive for :returns: Tuple containing information if IPv6 is enabled in the global @@ -391,7 +393,7 @@ class NginxConfigurator(common.Configurator): for addr in vh.addrs: if addr.ipv6: ipv6_active = True - if addr.ipv6only and addr.get_port() == port: + if addr.ipv6only and addr.get_port() == port and addr.get_addr() == host: ipv6only_present = True return ipv6_active, ipv6only_present @@ -723,9 +725,16 @@ class NginxConfigurator(common.Configurator): """ https_port = self.config.https_port - ipv6info = self.ipv6_info(str(https_port)) - ipv6_block = [''] - ipv4_block = [''] + http_port = self.config.http01_port + + # no addresses should have ssl turned on here + assert not vhost.ssl + + addrs_to_insert: List[obj.Addr] = [ + cast(obj.Addr, obj.Addr.fromstring(f'{addr.get_addr()}:{https_port} ssl')) + for addr in vhost.addrs + if addr.get_port() == str(http_port) + ] # If the vhost was implicitly listening on the default Nginx port, # have it continue to do so. @@ -733,31 +742,46 @@ class NginxConfigurator(common.Configurator): listen_block = [['\n ', 'listen', ' ', self.DEFAULT_LISTEN_PORT]] self.parser.add_server_directives(vhost, listen_block) - if vhost.ipv6_enabled(): - ipv6_block = ['\n ', - 'listen', - ' ', - '[::]:{0}'.format(https_port), - ' ', - 'ssl'] - if not ipv6info[1]: - # ipv6only=on is absent in global config - ipv6_block.append(' ') - ipv6_block.append('ipv6only=on') + if not addrs_to_insert: + # there are no existing addresses listening on 80 + if vhost.ipv6_enabled(): + addrs_to_insert += [cast(obj.Addr, obj.Addr.fromstring(f'[::]:{https_port} ssl'))] + if vhost.ipv4_enabled(): + addrs_to_insert += [cast(obj.Addr, obj.Addr.fromstring(f'{https_port} ssl'))] - if vhost.ipv4_enabled(): - ipv4_block = ['\n ', - 'listen', - ' ', - '{0}'.format(https_port), - ' ', - 'ssl'] + addr_blocks: List[List[str]] = [] + ipv6only_set_here: Set[Tuple[str, str]] = set() + for addr in addrs_to_insert: + host = addr.get_addr() + port = addr.get_port() + if addr.ipv6: + addr_block = ['\n ', + 'listen', + ' ', + f'{host}:{port}', + ' ', + 'ssl'] + ipv6only_exists = self.ipv6_info(host, port)[1] + if not ipv6only_exists and (host, port) not in ipv6only_set_here: + addr.ipv6only = True # bookkeeping in case we switch output implementation + ipv6only_set_here.add((host, port)) + addr_block.append(' ') + addr_block.append('ipv6only=on') + addr_blocks.append(addr_block) + else: + tuple_string = f'{host}:{port}' if host else f'{port}' + addr_block = ['\n ', + 'listen', + ' ', + tuple_string, + ' ', + 'ssl'] + addr_blocks.append(addr_block) snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() ssl_block = ([ - ipv6_block, - ipv4_block, + *addr_blocks, ['\n ', 'ssl_certificate', ' ', snakeoil_cert], ['\n ', 'ssl_certificate_key', ' ', snakeoil_key], ['\n ', 'include', ' ', self.mod_ssl_conf], diff --git a/certbot-nginx/certbot_nginx/_internal/http_01.py b/certbot-nginx/certbot_nginx/_internal/http_01.py index 2dc874b4b..9ca02dc57 100644 --- a/certbot-nginx/certbot_nginx/_internal/http_01.py +++ b/certbot-nginx/certbot_nginx/_internal/http_01.py @@ -152,7 +152,7 @@ class NginxHttp01(common.ChallengePerformer): self.configurator.config.http01_port) port = self.configurator.config.http01_port - ipv6, ipv6only = self.configurator.ipv6_info(str(port)) + ipv6, ipv6only = self.configurator.ipv6_info("[::]", str(port)) if ipv6: # If IPv6 is active in Nginx configuration diff --git a/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py b/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py index 8a284ee55..f10649920 100644 --- a/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/_internal/tests/configurator_test.py @@ -42,7 +42,7 @@ class NginxConfiguratorTest(util.NginxTest): def test_prepare(self): assert (1, 6, 2) == self.config.version - assert 14 == len(self.config.parser.parsed) + assert 15 == len(self.config.parser.parsed) @mock.patch("certbot_nginx._internal.configurator.util.exe_exists") @mock.patch("certbot_nginx._internal.configurator.subprocess.run") @@ -100,7 +100,7 @@ class NginxConfiguratorTest(util.NginxTest): "155.225.50.69.nephoscale.net", "www.example.org", "another.alias", "migration.com", "summer.com", "geese.com", "sslon.com", "globalssl.com", "globalsslsetssl.com", "ipv6.com", "ipv6ssl.com", - "headers.com", "example.net", "ssl.both.com"} + "headers.com", "example.net", "ssl.both.com", 'addr-80.com'} def test_supported_enhancements(self): assert ['redirect', 'ensure-http-header', 'staple-ocsp'] == \ @@ -201,11 +201,38 @@ class NginxConfiguratorTest(util.NginxTest): with pytest.raises(errors.MisconfigurationError): self.config.choose_vhosts(name) + def test_choose_vhosts_keep_ip_address(self): + # no listen on port 80 + # listen 69.50.225.155:9000; + # listen 127.0.0.1; + vhost = self.config.choose_vhosts('example.com')[0] + assert obj.Addr.fromstring("5001 ssl") in vhost.addrs + + # no listens at all + vhost = self.config.choose_vhosts('headers.com')[0] + assert obj.Addr.fromstring("5001 ssl") in vhost.addrs + assert obj.Addr.fromstring("80") in vhost.addrs + + # blank addr listen on 80 should result in blank addr ssl + # listen 80; + # listen [::]:80; + vhost = self.config.choose_vhosts('ipv6.com')[0] + assert obj.Addr.fromstring("5001 ssl") in vhost.addrs + assert obj.Addr.fromstring("[::]:5001 ssl") in vhost.addrs + + # listen on 80 with ip address should result in copied addr + # listen 1.2.3.4:80; + # listen [1:20::300]:80; + vhost = self.config.choose_vhosts('addr-80.com')[0] + assert obj.Addr.fromstring("1.2.3.4:5001 ssl") in vhost.addrs + assert obj.Addr.fromstring("[1:20::300]:5001 ssl ipv6only=on") in vhost.addrs + + def test_ipv6only(self): # ipv6_info: (ipv6_active, ipv6only_present) - assert (True, False) == self.config.ipv6_info("80") + assert (True, False) == self.config.ipv6_info("[::]", "80") # Port 443 has ipv6only=on because of ipv6ssl.com vhost - assert (True, True) == self.config.ipv6_info("443") + assert (True, True) == self.config.ipv6_info("[::]", "443") def test_ipv6only_detection(self): self.config.version = (1, 3, 1) @@ -585,7 +612,7 @@ class NginxConfiguratorTest(util.NginxTest): generated_conf = self.config.parser.parsed[example_conf] assert [[['server'], [ ['server_name', '.example.com'], - ['server_name', 'example.*'], [], + ['server_name', 'example.*'], ['listen', '5001', 'ssl'], ['#', ' managed by Certbot'], ['ssl_certificate', 'example/fullchain.pem'], ['#', ' managed by Certbot'], ['ssl_certificate_key', 'example/key.pem'], ['#', ' managed by Certbot'], @@ -615,7 +642,7 @@ class NginxConfiguratorTest(util.NginxTest): generated_conf = self.config.parser.parsed[example_conf] assert [[['server'], [ ['server_name', '.example.com'], - ['server_name', 'example.*'], [], + ['server_name', 'example.*'], ['listen', '5001', 'ssl'], ['#', ' managed by Certbot'], ['ssl_certificate', 'example/fullchain.pem'], ['#', ' managed by Certbot'], ['ssl_certificate_key', 'example/key.pem'], ['#', ' managed by Certbot'], @@ -630,7 +657,7 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], - [], [], []]]] == \ + [], []]]] == \ generated_conf def test_http_header_hsts(self): @@ -957,7 +984,7 @@ class NginxConfiguratorTest(util.NginxTest): prefer_ssl=False, no_ssl_filter_port='80') # Check that the dialog was called with only port 80 vhosts - assert len(mock_select_vhs.call_args[0][0]) == 8 + assert len(mock_select_vhs.call_args[0][0]) == 9 def test_choose_auth_vhosts(self): """choose_auth_vhosts correctly selects duplicative and HTTP/HTTPS vhosts""" diff --git a/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py b/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py index f45c2e45d..7cb1d2730 100644 --- a/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/_internal/tests/parser_test.py @@ -62,7 +62,8 @@ class NginxParserTest(util.NginxTest): 'sites-enabled/globalssl.com', 'sites-enabled/ipv6.com', 'sites-enabled/ipv6ssl.com', - 'sites-enabled/example.net']} == \ + 'sites-enabled/example.net', + 'sites-enabled/addr-80.com']} == \ set(nparser.parsed.keys()) assert [['server_name', 'somename', 'alias', 'another.alias']] == \ nparser.parsed[nparser.abs_path('server.conf')] @@ -92,7 +93,7 @@ class NginxParserTest(util.NginxTest): parsed = nparser._parse_files(nparser.abs_path( 'sites-enabled/example.com.test')) assert 4 == len(glob.glob(nparser.abs_path('*.test'))) - assert 10 == len( + assert 11 == len( glob.glob(nparser.abs_path('sites-enabled/*.test'))) assert [[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], @@ -175,7 +176,7 @@ class NginxParserTest(util.NginxTest): '*.www.example.com'}, [], [2, 1, 0]) - assert 19 == len(vhosts) + assert 20 == len(vhosts) example_com = [x for x in vhosts if 'example.com' in x.filep][0] assert vhost3 == example_com default = [x for x in vhosts if 'default' in x.filep][0] diff --git a/certbot-nginx/certbot_nginx/_internal/tests/testdata/etc_nginx/sites-enabled/addr-80.com b/certbot-nginx/certbot_nginx/_internal/tests/testdata/etc_nginx/sites-enabled/addr-80.com new file mode 100644 index 000000000..aeef7abdf --- /dev/null +++ b/certbot-nginx/certbot_nginx/_internal/tests/testdata/etc_nginx/sites-enabled/addr-80.com @@ -0,0 +1,5 @@ +server { + listen 1.2.3.4:80; + listen [1:20::300]:80; + server_name addr-80.com; +} diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 3dfaf1db8..b13dad286 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -26,6 +26,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * Honor --reuse-key when --allow-subset-of-names is set * Fixed regression in symlink parsing on Windows that was introduced in Certbot 3.1.0. +* When adding ssl listen directives in nginx server blocks, IP addresses are now + preserved. More details about these changes can be found on our GitHub repo.