From 22da2447d5d89c11b9353ebbd7777690fee999df Mon Sep 17 00:00:00 2001 From: ohemorange Date: Wed, 17 Oct 2018 10:54:43 -0700 Subject: [PATCH 1/3] Stop caching the results of ipv6_info in http01.py (#6411) Stop caching the results of ipv6_info in http01.py. A call to choose_vhosts might change the ipv6 results of later calls. Add tests for this and default_listen_addresses more broadly. --- CHANGELOG.md | 1 + certbot-nginx/certbot_nginx/http_01.py | 6 +--- .../certbot_nginx/tests/http_01_test.py | 36 +++++++++++++++++++ certbot-nginx/certbot_nginx/tls_sni_01.py | 6 ++-- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index def17accc..81cc12b15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Certbot adheres to [Semantic Versioning](http://semver.org/). * Match Nginx parser update in allowing variable names to start with `${`. * Correct OVH integration tests on machines without internet access. +* Stop caching the results of ipv6_info in http01.py ## 0.27.1 - 2018-09-06 diff --git a/certbot-nginx/certbot_nginx/http_01.py b/certbot-nginx/certbot_nginx/http_01.py index 9b385bc3b..e46d7b9b9 100644 --- a/certbot-nginx/certbot_nginx/http_01.py +++ b/certbot-nginx/certbot_nginx/http_01.py @@ -40,8 +40,6 @@ class NginxHttp01(common.ChallengePerformer): super(NginxHttp01, self).__init__(configurator) self.challenge_conf = os.path.join( configurator.config.config_dir, "le_http_01_cert_challenge.conf") - self._ipv6 = None - self._ipv6only = None def perform(self): """Perform a challenge on Nginx. @@ -121,9 +119,7 @@ class NginxHttp01(common.ChallengePerformer): self.configurator.config.http01_port) port = self.configurator.config.http01_port - if self._ipv6 is None or self._ipv6only is None: - self._ipv6, self._ipv6only = self.configurator.ipv6_info(port) - ipv6, ipv6only = self._ipv6, self._ipv6only + ipv6, ipv6only = self.configurator.ipv6_info(port) if ipv6: # If IPv6 is active in Nginx configuration diff --git a/certbot-nginx/certbot_nginx/tests/http_01_test.py b/certbot-nginx/certbot_nginx/tests/http_01_test.py index 0f764e92e..ed3c257ee 100644 --- a/certbot-nginx/certbot_nginx/tests/http_01_test.py +++ b/certbot-nginx/certbot_nginx/tests/http_01_test.py @@ -12,6 +12,7 @@ from certbot import achallenges from certbot.plugins import common_test from certbot.tests import acme_util +from certbot_nginx.obj import Addr from certbot_nginx.tests import util @@ -108,6 +109,41 @@ class HttpPerformTest(util.NginxTest): # self.assertEqual(vhost.addrs, set(v_addr2_print)) # self.assertEqual(vhost.names, set([response.z_domain.decode('ascii')])) + @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info") + def test_default_listen_addresses_no_memoization(self, ipv6_info): + # pylint: disable=protected-access + ipv6_info.return_value = (True, True) + self.http01._default_listen_addresses() + self.assertEqual(ipv6_info.call_count, 1) + ipv6_info.return_value = (False, False) + self.http01._default_listen_addresses() + self.assertEqual(ipv6_info.call_count, 2) + + @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info") + def test_default_listen_addresses_t_t(self, ipv6_info): + # pylint: disable=protected-access + ipv6_info.return_value = (True, True) + addrs = self.http01._default_listen_addresses() + http_addr = Addr.fromstring("80") + http_ipv6_addr = Addr.fromstring("[::]:80") + self.assertEqual(addrs, [http_addr, http_ipv6_addr]) + + @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info") + def test_default_listen_addresses_t_f(self, ipv6_info): + # pylint: disable=protected-access + ipv6_info.return_value = (True, False) + addrs = self.http01._default_listen_addresses() + http_addr = Addr.fromstring("80") + http_ipv6_addr = Addr.fromstring("[::]:80 ipv6only=on") + self.assertEqual(addrs, [http_addr, http_ipv6_addr]) + + @mock.patch("certbot_nginx.configurator.NginxConfigurator.ipv6_info") + def test_default_listen_addresses_f_f(self, ipv6_info): + # pylint: disable=protected-access + ipv6_info.return_value = (False, False) + addrs = self.http01._default_listen_addresses() + http_addr = Addr.fromstring("80") + self.assertEqual(addrs, [http_addr]) if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py index d49ec8643..60ec1ed1a 100644 --- a/certbot-nginx/certbot_nginx/tls_sni_01.py +++ b/certbot-nginx/certbot_nginx/tls_sni_01.py @@ -51,9 +51,6 @@ class NginxTlsSni01(common.TLSSNI01): default_addr = "{0} ssl".format( self.configurator.config.tls_sni_01_port) - ipv6, ipv6only = self.configurator.ipv6_info( - self.configurator.config.tls_sni_01_port) - for achall in self.achalls: vhosts = self.configurator.choose_vhosts(achall.domain, create_if_no_match=True) @@ -61,6 +58,9 @@ class NginxTlsSni01(common.TLSSNI01): if vhosts and vhosts[0].addrs: addresses.append(list(vhosts[0].addrs)) else: + # choose_vhosts might have modified vhosts, so put this after + ipv6, ipv6only = self.configurator.ipv6_info( + self.configurator.config.tls_sni_01_port) if ipv6: # If IPv6 is active in Nginx configuration ipv6_addr = "[::]:{0} ssl".format( From 819f95c37d6d8471a5413137e033d30cea6822d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C8=98tefan=20Talpalaru?= Date: Wed, 17 Oct 2018 22:48:49 +0200 Subject: [PATCH 2/3] certbot_dns_linode: increase the default propagation interval (#6320) Using the default value of 16 minutes (960 seconds) for --dns-linode-propagation-seconds leads to DNS failures when the randomly selected Linode DNS is not the first one out of six, due to an additional delay before the other five are updated. The problem can be easily solved by increasing the wait interval, so this commit increases the default value to 20 minutes. More details: https://community.letsencrypt.org/t/dns-servers-used-by-letsencrypt-for-challenges/32127/16 --- certbot-dns-linode/certbot_dns_linode/__init__.py | 14 ++++++++++---- .../certbot_dns_linode/dns_linode.py | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/certbot-dns-linode/certbot_dns_linode/__init__.py b/certbot-dns-linode/certbot_dns_linode/__init__.py index 0c445f45d..0a6ccec61 100644 --- a/certbot-dns-linode/certbot_dns_linode/__init__.py +++ b/certbot-dns-linode/certbot_dns_linode/__init__.py @@ -14,7 +14,11 @@ Named Arguments DNS to propagate before asking the ACME server to verify the DNS record. - (Default: 960) + (Default: 1200 because Linode + updates its first DNS every 15 + minutes and we allow 5 more minutes + for the update to reach the other 5 + servers) ========================================== =================================== @@ -74,13 +78,15 @@ Examples -d www.example.com .. code-block:: bash - :caption: To acquire a certificate for ``example.com``, waiting 60 seconds - for DNS propagation + :caption: To acquire a certificate for ``example.com``, waiting 1000 seconds + for DNS propagation (Linode updates its first DNS every 15 minutes + and we allow some extra time for the update to reach the other 5 + servers) certbot certonly \\ --dns-linode \\ --dns-linode-credentials ~/.secrets/certbot/linode.ini \\ - --dns-linode-propagation-seconds 60 \\ + --dns-linode-propagation-seconds 1000 \\ -d example.com """ diff --git a/certbot-dns-linode/certbot_dns_linode/dns_linode.py b/certbot-dns-linode/certbot_dns_linode/dns_linode.py index 323c0810a..cc29ce842 100644 --- a/certbot-dns-linode/certbot_dns_linode/dns_linode.py +++ b/certbot-dns-linode/certbot_dns_linode/dns_linode.py @@ -29,7 +29,7 @@ class Authenticator(dns_common.DNSAuthenticator): @classmethod def add_parser_arguments(cls, add): # pylint: disable=arguments-differ - super(Authenticator, cls).add_parser_arguments(add, default_propagation_seconds=960) + super(Authenticator, cls).add_parser_arguments(add, default_propagation_seconds=1200) add('credentials', help='Linode credentials INI file.') def more_info(self): # pylint: disable=missing-docstring,no-self-use From 92501eaf8fe6afc514999b207037e5003f38c5db Mon Sep 17 00:00:00 2001 From: schoen Date: Wed, 17 Oct 2018 14:08:59 -0700 Subject: [PATCH 3/3] Note about running on web server, not PC (#6422) --- README.rst | 2 +- docs/install.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 0dbe1cdef..d75b44a65 100644 --- a/README.rst +++ b/README.rst @@ -6,7 +6,7 @@ Anyone who has gone through the trouble of setting up a secure website knows wha How you use Certbot depends on the configuration of your web server. The best way to get started is to use our `interactive guide `_. It generates instructions based on your configuration settings. In most cases, you’ll need `root or administrator access `_ to your web server to run Certbot. -If you’re using a hosted service and don’t have direct access to your web server, you might not be able to use Certbot. Check with your hosting provider for documentation about uploading certificates or using certificates issued by Let’s Encrypt. +Certbot is meant to be run directly on your web server, not on your personal computer. If you’re using a hosted service and don’t have direct access to your web server, you might not be able to use Certbot. Check with your hosting provider for documentation about uploading certificates or using certificates issued by Let’s Encrypt. Certbot is a fully-featured, extensible client for the Let's Encrypt CA (or any other CA that speaks the `ACME diff --git a/docs/install.rst b/docs/install.rst index f7504baa5..fc6abad7a 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -9,6 +9,8 @@ Get Certbot About Certbot ============= +*Certbot is meant to be run directly on a web server*, normally by a system administrator. In most cases, running Certbot on your personal computer is not a useful option. The instructions below relate to installing and running Certbot on a server. + Certbot is packaged for many common operating systems and web servers. Check whether ``certbot`` (or ``letsencrypt``) is packaged for your web server's OS by visiting certbot.eff.org_, where you will also find the correct installation instructions for