From d0a9695b0963eb06e6405115657471b3a37103ed Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Jul 2019 18:14:01 -0700 Subject: [PATCH 01/26] Make PR template a checklist and suggest mypy. (#7223) --- pull_request_template.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pull_request_template.md b/pull_request_template.md index 9ab07e3aa..77d65499f 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,4 +1,8 @@ -Be sure to edit the `master` section of `CHANGELOG.md` to include a description -of the change being made in this PR. +## Pull Request Checklist -You are also welcome to add your name to `AUTHORS.md` if you like. +- [ ] Edit the `master` section of `CHANGELOG.md` to include a description of + the change being made. +- [ ] Add [mypy type + annotations](https://certbot.eff.org/docs/contributing.html#mypy-type-annotations) + for any functions that were added or modified. +- [ ] Include your name in `AUTHORS.md` if you like. From 89f52ca9f921133918c26581f6fa91ed8204ad4a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Jul 2019 18:14:12 -0700 Subject: [PATCH 02/26] Add mypy to contributing checklist. (#7224) --- docs/contributing.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 14256b15a..715788332 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -375,6 +375,8 @@ As a developer, when working on Certbot or its plugins, you must use ``certbot.c in every place you would need ``os`` (eg. ``from certbot.compat import os`` instead of ``import os``). Otherwise the tests will fail when your PR is submitted. +.. _type annotations: + Mypy type annotations ===================== @@ -414,7 +416,10 @@ Submitting a pull request Steps: -1. Write your code! +1. Write your code! When doing this, you should add :ref:`mypy type annotations + ` for any functions you add or modify. You can check that + you've done this correctly by running ``tox -e mypy`` on a machine that has + Python 3 installed. 2. Make sure your environment is set up properly and that you're in your virtualenv. You can do this by following the instructions in the :ref:`Getting Started ` section. From 13c44a059580d5dfa7d79570bdb55e3a98945d01 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 11 Jul 2019 12:12:24 -0700 Subject: [PATCH 03/26] Update changelog for 0.36.0 release --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc3acb665..50855c942 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). -## 0.36.0 - master +## 0.36.0 - 2019-07-11 ### Added From cbd0a37c7a5b277b2d69edb03b56f24f244fdf61 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 11 Jul 2019 12:31:51 -0700 Subject: [PATCH 04/26] Release 0.36.0 --- acme/setup.py | 2 +- certbot-apache/local-oldest-requirements.txt | 2 +- certbot-apache/setup.py | 4 +-- certbot-auto | 26 +++++++++---------- certbot-compatibility-test/setup.py | 2 +- certbot-dns-cloudflare/setup.py | 2 +- certbot-dns-cloudxns/setup.py | 2 +- certbot-dns-digitalocean/setup.py | 2 +- certbot-dns-dnsimple/setup.py | 2 +- certbot-dns-dnsmadeeasy/setup.py | 2 +- certbot-dns-gehirn/setup.py | 2 +- certbot-dns-google/setup.py | 2 +- certbot-dns-linode/setup.py | 2 +- certbot-dns-luadns/setup.py | 2 +- certbot-dns-nsone/setup.py | 2 +- certbot-dns-ovh/setup.py | 2 +- certbot-dns-rfc2136/setup.py | 2 +- certbot-dns-route53/setup.py | 2 +- certbot-dns-sakuracloud/setup.py | 2 +- certbot-nginx/local-oldest-requirements.txt | 2 +- certbot-nginx/setup.py | 4 +-- certbot/__init__.py | 2 +- docs/cli-help.txt | 17 +++++------- letsencrypt-auto | 26 +++++++++---------- letsencrypt-auto-source/certbot-auto.asc | 16 ++++++------ letsencrypt-auto-source/letsencrypt-auto | 26 +++++++++---------- letsencrypt-auto-source/letsencrypt-auto.sig | 3 +-- .../pieces/certbot-requirements.txt | 24 ++++++++--------- 28 files changed, 90 insertions(+), 94 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 9fca57e01..6ab8b4930 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -3,7 +3,7 @@ from setuptools import find_packages from setuptools.command.test import test as TestCommand import sys -version = '0.36.0.dev0' +version = '0.36.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-apache/local-oldest-requirements.txt b/certbot-apache/local-oldest-requirements.txt index da509406e..a959ad44f 100644 --- a/certbot-apache/local-oldest-requirements.txt +++ b/certbot-apache/local-oldest-requirements.txt @@ -1,3 +1,3 @@ # Remember to update setup.py to match the package versions below. acme[dev]==0.29.0 --e .[dev] +certbot[dev]==0.36.0 diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index 6e6f0277f..b1cd344bf 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -4,13 +4,13 @@ from setuptools.command.test import test as TestCommand import sys -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. install_requires = [ 'acme>=0.29.0', - 'certbot>=0.36.0.dev0', + 'certbot>=0.36.0', 'mock', 'python-augeas', 'setuptools', diff --git a/certbot-auto b/certbot-auto index 756b8e247..fd2cfb29b 100755 --- a/certbot-auto +++ b/certbot-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="0.35.1" +LE_AUTO_VERSION="0.36.0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates @@ -1314,18 +1314,18 @@ letsencrypt==0.7.0 \ --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \ --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9 -certbot==0.35.1 \ - --hash=sha256:24821e10b05084a45c5bf29da704115f2637af613866589737cff502294dad2a \ - --hash=sha256:d7e8ecc14e06ed1dc691c6069bc9ce42dce04e8db1684ddfab446fbd71290860 -acme==0.35.1 \ - --hash=sha256:3ec62f638f2b3684bcb3d8476345c7ae37c8f3b28f2999622ff836aec6e73d64 \ - --hash=sha256:a988b8b418cc74075e68b4acf3ff64c026bf52c377b0d01223233660a755c423 -certbot-apache==0.35.1 \ - --hash=sha256:ee4fe10cbd18e0aa7fe36d43ad7792187f41a7298f383610b87049c3a6493bbb \ - --hash=sha256:69962eafe0ec9be8eb2845e3622da6f37ecaeee7e517ea172d71d7b31f01be71 -certbot-nginx==0.35.1 \ - --hash=sha256:22150f13b3c0bd1c3c58b11a64886dad9695796aac42f5809da7ec66de187760 \ - --hash=sha256:85e9a48b4b549f6989304f66cb2fad822c3f8717d361bde0d6a43aabb792d461 +certbot==0.36.0 \ + --hash=sha256:486cee6c4861762fe4a94b4f44f7d227034d026d1a8d7ba2911ef4e86a737613 \ + --hash=sha256:bf6745b823644cdca8461150455aeb67d417f87f80b9ec774c716e9587ef20a2 +acme==0.36.0 \ + --hash=sha256:5570c8e87383fbc733224fd0f7d164313b67dd9c21deafe9ddc8e769441f0c86 \ + --hash=sha256:0461ee3c882d865e98e624561843dc135fa1a1412b15603d7ebfbb392de6a668 +certbot-apache==0.36.0 \ + --hash=sha256:2537f7fb67a38b6d1ed5ee79f6a799090ca609695ac3799bb840b2fb677ac98d \ + --hash=sha256:458d20a3e9e8a88563d3deb0bbe38752bd2b80100f0e5854e4069390c1b4e5cd +certbot-nginx==0.36.0 \ + --hash=sha256:4303b54adf2030671c54bb3964c1f43aec0f677045e0cdb6d4fb931268d08310 \ + --hash=sha256:4c34e6114dd8204b6667f101579dd9ab2b38fef0dd5a15702585edcb2aefb322 UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/certbot-compatibility-test/setup.py b/certbot-compatibility-test/setup.py index 362043531..f3dfac291 100644 --- a/certbot-compatibility-test/setup.py +++ b/certbot-compatibility-test/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' install_requires = [ 'certbot', diff --git a/certbot-dns-cloudflare/setup.py b/certbot-dns-cloudflare/setup.py index bd201aca2..8c6e8e434 100644 --- a/certbot-dns-cloudflare/setup.py +++ b/certbot-dns-cloudflare/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-cloudxns/setup.py b/certbot-dns-cloudxns/setup.py index d8d7aa9b8..13c9cf5e0 100644 --- a/certbot-dns-cloudxns/setup.py +++ b/certbot-dns-cloudxns/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-digitalocean/setup.py b/certbot-dns-digitalocean/setup.py index 92a9b2b14..3b6d8ce2e 100644 --- a/certbot-dns-digitalocean/setup.py +++ b/certbot-dns-digitalocean/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsimple/setup.py b/certbot-dns-dnsimple/setup.py index 709ca8330..cc106a7ac 100644 --- a/certbot-dns-dnsimple/setup.py +++ b/certbot-dns-dnsimple/setup.py @@ -3,7 +3,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsmadeeasy/setup.py b/certbot-dns-dnsmadeeasy/setup.py index 1d55b0fe4..04c4c33a4 100644 --- a/certbot-dns-dnsmadeeasy/setup.py +++ b/certbot-dns-dnsmadeeasy/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-gehirn/setup.py b/certbot-dns-gehirn/setup.py index 6dac126d0..64999b851 100644 --- a/certbot-dns-gehirn/setup.py +++ b/certbot-dns-gehirn/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-google/setup.py b/certbot-dns-google/setup.py index 5c31a81f8..f8989c340 100644 --- a/certbot-dns-google/setup.py +++ b/certbot-dns-google/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-linode/setup.py b/certbot-dns-linode/setup.py index b70a6a39c..94675e939 100644 --- a/certbot-dns-linode/setup.py +++ b/certbot-dns-linode/setup.py @@ -1,7 +1,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-luadns/setup.py b/certbot-dns-luadns/setup.py index 5f5322319..7d1bd28b2 100644 --- a/certbot-dns-luadns/setup.py +++ b/certbot-dns-luadns/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-nsone/setup.py b/certbot-dns-nsone/setup.py index 00ed64032..e024f4676 100644 --- a/certbot-dns-nsone/setup.py +++ b/certbot-dns-nsone/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-ovh/setup.py b/certbot-dns-ovh/setup.py index bff394bf9..ef272f190 100644 --- a/certbot-dns-ovh/setup.py +++ b/certbot-dns-ovh/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-rfc2136/setup.py b/certbot-dns-rfc2136/setup.py index 25c6ae1d1..2bd6df51e 100644 --- a/certbot-dns-rfc2136/setup.py +++ b/certbot-dns-rfc2136/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-route53/setup.py b/certbot-dns-route53/setup.py index b8af58b30..dede8e7c4 100644 --- a/certbot-dns-route53/setup.py +++ b/certbot-dns-route53/setup.py @@ -1,7 +1,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-sakuracloud/setup.py b/certbot-dns-sakuracloud/setup.py index b674b7199..9d79fab9c 100644 --- a/certbot-dns-sakuracloud/setup.py +++ b/certbot-dns-sakuracloud/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0.dev0' +version = '0.36.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-nginx/local-oldest-requirements.txt b/certbot-nginx/local-oldest-requirements.txt index da509406e..a959ad44f 100644 --- a/certbot-nginx/local-oldest-requirements.txt +++ b/certbot-nginx/local-oldest-requirements.txt @@ -1,3 +1,3 @@ # Remember to update setup.py to match the package versions below. acme[dev]==0.29.0 --e .[dev] +certbot[dev]==0.36.0 diff --git a/certbot-nginx/setup.py b/certbot-nginx/setup.py index 64f343730..d3b5c2132 100644 --- a/certbot-nginx/setup.py +++ b/certbot-nginx/setup.py @@ -4,13 +4,13 @@ from setuptools.command.test import test as TestCommand import sys -version = '0.36.0.dev0' +version = '0.36.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. install_requires = [ 'acme>=0.29.0', - 'certbot>=0.35.0.dev0', + 'certbot>=0.35.0', 'mock', 'PyOpenSSL', 'pyparsing>=1.5.5', # Python3 support; perhaps unnecessary? diff --git a/certbot/__init__.py b/certbot/__init__.py index 32ab75aaa..0d23d2084 100644 --- a/certbot/__init__.py +++ b/certbot/__init__.py @@ -1,4 +1,4 @@ """Certbot client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.36.0.dev0' +__version__ = '0.36.0' diff --git a/docs/cli-help.txt b/docs/cli-help.txt index 3499acf19..9532b7b22 100644 --- a/docs/cli-help.txt +++ b/docs/cli-help.txt @@ -27,10 +27,10 @@ manage certificates: revoke Revoke a certificate (supply --cert-path or --cert-name) delete Delete a certificate -manage your account with Let's Encrypt: - register Create a Let's Encrypt ACME account - unregister Deactivate a Let's Encrypt ACME account - update_account Update a Let's Encrypt ACME account +manage your account: + register Create an ACME account + unregister Deactivate an ACME account + update_account Update an ACME account --agree-tos Agree to the ACME server's Subscriber Agreement -m EMAIL Email address for important account notifications @@ -113,7 +113,7 @@ optional arguments: case, and to know when to deprecate support for past Python versions and flags. If you wish to hide this information from the Let's Encrypt server, set this to - "". (default: CertbotACMEClient/0.35.1 + "". (default: CertbotACMEClient/0.36.0 (certbot(-auto); OS_NAME OS_VERSION) Authenticator/XXX Installer/YYY (SUBCOMMAND; flags: FLAGS) Py/major.minor.patchlevel). The flags encoded in the @@ -393,10 +393,7 @@ install: Options for modifying how a certificate is deployed config_changes: - Options for controlling which changes are displayed - - --num NUM How many past revisions you want to be displayed - (default: None) + Options for viewing configuration changes rollback: Options for rolling back server configuration changes @@ -405,7 +402,7 @@ rollback: (default: 1) plugins: - Options for for the "plugins" subcommand + Options for the "plugins" subcommand --init Initialize plugins. (default: False) --prepare Initialize and prepare plugins. (default: False) diff --git a/letsencrypt-auto b/letsencrypt-auto index 756b8e247..fd2cfb29b 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="0.35.1" +LE_AUTO_VERSION="0.36.0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates @@ -1314,18 +1314,18 @@ letsencrypt==0.7.0 \ --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \ --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9 -certbot==0.35.1 \ - --hash=sha256:24821e10b05084a45c5bf29da704115f2637af613866589737cff502294dad2a \ - --hash=sha256:d7e8ecc14e06ed1dc691c6069bc9ce42dce04e8db1684ddfab446fbd71290860 -acme==0.35.1 \ - --hash=sha256:3ec62f638f2b3684bcb3d8476345c7ae37c8f3b28f2999622ff836aec6e73d64 \ - --hash=sha256:a988b8b418cc74075e68b4acf3ff64c026bf52c377b0d01223233660a755c423 -certbot-apache==0.35.1 \ - --hash=sha256:ee4fe10cbd18e0aa7fe36d43ad7792187f41a7298f383610b87049c3a6493bbb \ - --hash=sha256:69962eafe0ec9be8eb2845e3622da6f37ecaeee7e517ea172d71d7b31f01be71 -certbot-nginx==0.35.1 \ - --hash=sha256:22150f13b3c0bd1c3c58b11a64886dad9695796aac42f5809da7ec66de187760 \ - --hash=sha256:85e9a48b4b549f6989304f66cb2fad822c3f8717d361bde0d6a43aabb792d461 +certbot==0.36.0 \ + --hash=sha256:486cee6c4861762fe4a94b4f44f7d227034d026d1a8d7ba2911ef4e86a737613 \ + --hash=sha256:bf6745b823644cdca8461150455aeb67d417f87f80b9ec774c716e9587ef20a2 +acme==0.36.0 \ + --hash=sha256:5570c8e87383fbc733224fd0f7d164313b67dd9c21deafe9ddc8e769441f0c86 \ + --hash=sha256:0461ee3c882d865e98e624561843dc135fa1a1412b15603d7ebfbb392de6a668 +certbot-apache==0.36.0 \ + --hash=sha256:2537f7fb67a38b6d1ed5ee79f6a799090ca609695ac3799bb840b2fb677ac98d \ + --hash=sha256:458d20a3e9e8a88563d3deb0bbe38752bd2b80100f0e5854e4069390c1b4e5cd +certbot-nginx==0.36.0 \ + --hash=sha256:4303b54adf2030671c54bb3964c1f43aec0f677045e0cdb6d4fb931268d08310 \ + --hash=sha256:4c34e6114dd8204b6667f101579dd9ab2b38fef0dd5a15702585edcb2aefb322 UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/letsencrypt-auto-source/certbot-auto.asc b/letsencrypt-auto-source/certbot-auto.asc index 0abdfdfdb..e102e3720 100644 --- a/letsencrypt-auto-source/certbot-auto.asc +++ b/letsencrypt-auto-source/certbot-auto.asc @@ -1,11 +1,11 @@ -----BEGIN PGP SIGNATURE----- -iQEzBAABCAAdFiEEos+1H6J1pyhiNOeyTRfJlc2XdfIFAlz+2KgACgkQTRfJlc2X -dfKLvwf/XUnWRyKldk/d8Egx514mpjzV38grCcZTZrY0O/Rd3YMv5KtrxnTnmvxJ -zAkTfNIo7Y1894mZ6XUIF3D7BiPoRqLj6F8tYLV6jbdsPJKC75dQY/6rcttTSJab -Zbqcw+WTXYNZ72AlHw0uTaxNT+S31KvrJ0pNmuj2ezKzZcDfgcxeeqmI1pJYozbQ -+AfqJMFgP9qHtfZVnmRO5UFBW2qPM522E02wWCtkaQSybI9ikRTvJOtilQgsLFJK -vBdD/MgGHm/xQC6lxG6l/SD4pebvNBaIPCiPAo2XC8ML+4tpjuJ5lPoK/aFCvfYQ -wSMHbDbse/Ndw+ssjmriiBreKB37Mg== -=flRo +iQEzBAABCAAdFiEEos+1H6J1pyhiNOeyTRfJlc2XdfIFAl0njnkACgkQTRfJlc2X +dfIqjwf/XBEzOEtwi84v97jZjHi9bqeFBzAt+n6YXleKySk8anxFMFmFIvrc2w/U +eyskpn0mmJDX2LjaXcsJji+l5yAWbm3p8M2J2toaPI2TLznhM6+uEWP62BHJiQYi +1ORBJYATSfLxA541CwXXW3VTYDu+CLq0w1nr5mHg1Y20ZFBrPIlt04mkh9o70fD4 +qv6MsMXKZxglhH1ORyLMVn5Jze22awmJ944pP8aI54ZEkTl2XT9DsZt3QpZ1muOy +IRg6sU86ukgWK66zWjTyd1AOddDL2OY3+U7JachFd5eb7dnnaCGeZhCjfVide7a3 +Fk8NrXwlrpKKJYkbqDfRkT4Pba47VQ== +=gf1k -----END PGP SIGNATURE----- diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 24e9b295e..fd2cfb29b 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="0.36.0.dev0" +LE_AUTO_VERSION="0.36.0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates @@ -1314,18 +1314,18 @@ letsencrypt==0.7.0 \ --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \ --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9 -certbot==0.35.1 \ - --hash=sha256:24821e10b05084a45c5bf29da704115f2637af613866589737cff502294dad2a \ - --hash=sha256:d7e8ecc14e06ed1dc691c6069bc9ce42dce04e8db1684ddfab446fbd71290860 -acme==0.35.1 \ - --hash=sha256:3ec62f638f2b3684bcb3d8476345c7ae37c8f3b28f2999622ff836aec6e73d64 \ - --hash=sha256:a988b8b418cc74075e68b4acf3ff64c026bf52c377b0d01223233660a755c423 -certbot-apache==0.35.1 \ - --hash=sha256:ee4fe10cbd18e0aa7fe36d43ad7792187f41a7298f383610b87049c3a6493bbb \ - --hash=sha256:69962eafe0ec9be8eb2845e3622da6f37ecaeee7e517ea172d71d7b31f01be71 -certbot-nginx==0.35.1 \ - --hash=sha256:22150f13b3c0bd1c3c58b11a64886dad9695796aac42f5809da7ec66de187760 \ - --hash=sha256:85e9a48b4b549f6989304f66cb2fad822c3f8717d361bde0d6a43aabb792d461 +certbot==0.36.0 \ + --hash=sha256:486cee6c4861762fe4a94b4f44f7d227034d026d1a8d7ba2911ef4e86a737613 \ + --hash=sha256:bf6745b823644cdca8461150455aeb67d417f87f80b9ec774c716e9587ef20a2 +acme==0.36.0 \ + --hash=sha256:5570c8e87383fbc733224fd0f7d164313b67dd9c21deafe9ddc8e769441f0c86 \ + --hash=sha256:0461ee3c882d865e98e624561843dc135fa1a1412b15603d7ebfbb392de6a668 +certbot-apache==0.36.0 \ + --hash=sha256:2537f7fb67a38b6d1ed5ee79f6a799090ca609695ac3799bb840b2fb677ac98d \ + --hash=sha256:458d20a3e9e8a88563d3deb0bbe38752bd2b80100f0e5854e4069390c1b4e5cd +certbot-nginx==0.36.0 \ + --hash=sha256:4303b54adf2030671c54bb3964c1f43aec0f677045e0cdb6d4fb931268d08310 \ + --hash=sha256:4c34e6114dd8204b6667f101579dd9ab2b38fef0dd5a15702585edcb2aefb322 UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/letsencrypt-auto-source/letsencrypt-auto.sig b/letsencrypt-auto-source/letsencrypt-auto.sig index d3824cf47..9430fd293 100644 --- a/letsencrypt-auto-source/letsencrypt-auto.sig +++ b/letsencrypt-auto-source/letsencrypt-auto.sig @@ -1,2 +1 @@ -{¡Ü–=Çà’¯Q\0Åÿ<*ÝOÛÍ#ýå›pýÚ£í²5wtl]ýÓ  pT—éM!ͺC¿˜püÂ!”Õ³ÿÍ ÍÍ ³ìÚ_ÁÒF…–õë@íìrÀ솑=Î^%†Ò~x~û:0ŸÏ×7¢”Éo'7œª “™ª#šnnRèg:SC·Y®—b¡ÆŒÇÛéx™pajoýûó­Oj¶IÛqQ¾;6ôÙèò,ƒ=^ƒ/ƒ¤©{³ÚTJΩ^õòþÓ#L”TÉŠ§±+Ü1XcYw„ß _mÒrñìã :D–ŽOßäÎûá*¤ß@»’té‡+¥`jû¾F˜ -;Ii» \ No newline at end of file +Å„–Üp|?Ê¢5ªÇgÒl,ÂzCHvöW3–tœÌºZê| Þ3µ‚­^¹Ç 7Iª‹Âœ2¦¾ä^m¡(Þ`…X™í²ËÅq. U“±DDR»À]À£ÁGý{ìÁ¨"øËâòÐæÆ°”9]æ{×}ºMž g/åAüA"ºš³ø×£×ûÆáêñ³¬¼B c˜R{÷¶²Gâ}dÎ$H&”[éðµÉâiÀ­C^Y­˜¡îl ÒS·nüE?7¼TÙ¡†BÓò{ÊÅv—!n/…q~‘Hh•#R£9Èb€×}j5:}šO®3_»ìaª}¬Í­þ \ No newline at end of file diff --git a/letsencrypt-auto-source/pieces/certbot-requirements.txt b/letsencrypt-auto-source/pieces/certbot-requirements.txt index 71c041934..ee5705766 100644 --- a/letsencrypt-auto-source/pieces/certbot-requirements.txt +++ b/letsencrypt-auto-source/pieces/certbot-requirements.txt @@ -1,12 +1,12 @@ -certbot==0.35.1 \ - --hash=sha256:24821e10b05084a45c5bf29da704115f2637af613866589737cff502294dad2a \ - --hash=sha256:d7e8ecc14e06ed1dc691c6069bc9ce42dce04e8db1684ddfab446fbd71290860 -acme==0.35.1 \ - --hash=sha256:3ec62f638f2b3684bcb3d8476345c7ae37c8f3b28f2999622ff836aec6e73d64 \ - --hash=sha256:a988b8b418cc74075e68b4acf3ff64c026bf52c377b0d01223233660a755c423 -certbot-apache==0.35.1 \ - --hash=sha256:ee4fe10cbd18e0aa7fe36d43ad7792187f41a7298f383610b87049c3a6493bbb \ - --hash=sha256:69962eafe0ec9be8eb2845e3622da6f37ecaeee7e517ea172d71d7b31f01be71 -certbot-nginx==0.35.1 \ - --hash=sha256:22150f13b3c0bd1c3c58b11a64886dad9695796aac42f5809da7ec66de187760 \ - --hash=sha256:85e9a48b4b549f6989304f66cb2fad822c3f8717d361bde0d6a43aabb792d461 +certbot==0.36.0 \ + --hash=sha256:486cee6c4861762fe4a94b4f44f7d227034d026d1a8d7ba2911ef4e86a737613 \ + --hash=sha256:bf6745b823644cdca8461150455aeb67d417f87f80b9ec774c716e9587ef20a2 +acme==0.36.0 \ + --hash=sha256:5570c8e87383fbc733224fd0f7d164313b67dd9c21deafe9ddc8e769441f0c86 \ + --hash=sha256:0461ee3c882d865e98e624561843dc135fa1a1412b15603d7ebfbb392de6a668 +certbot-apache==0.36.0 \ + --hash=sha256:2537f7fb67a38b6d1ed5ee79f6a799090ca609695ac3799bb840b2fb677ac98d \ + --hash=sha256:458d20a3e9e8a88563d3deb0bbe38752bd2b80100f0e5854e4069390c1b4e5cd +certbot-nginx==0.36.0 \ + --hash=sha256:4303b54adf2030671c54bb3964c1f43aec0f677045e0cdb6d4fb931268d08310 \ + --hash=sha256:4c34e6114dd8204b6667f101579dd9ab2b38fef0dd5a15702585edcb2aefb322 From 15b1d8e5a7a4035199bc88eb3b90def75b46763a Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 11 Jul 2019 12:31:53 -0700 Subject: [PATCH 05/26] Add contents to CHANGELOG.md for next version --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50855c942..091ec0493 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). +## 0.37.0 - master + +### Added + +* + +### Changed + +* + +### Fixed + +* + +More details about these changes can be found on our GitHub repo. + ## 0.36.0 - 2019-07-11 ### Added From d1934e36fe9e919b67857dd4af0d2523d15c25d4 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Thu, 11 Jul 2019 12:31:53 -0700 Subject: [PATCH 06/26] Bump version to 0.37.0 --- acme/setup.py | 2 +- certbot-apache/setup.py | 2 +- certbot-compatibility-test/setup.py | 2 +- certbot-dns-cloudflare/setup.py | 2 +- certbot-dns-cloudxns/setup.py | 2 +- certbot-dns-digitalocean/setup.py | 2 +- certbot-dns-dnsimple/setup.py | 2 +- certbot-dns-dnsmadeeasy/setup.py | 2 +- certbot-dns-gehirn/setup.py | 2 +- certbot-dns-google/setup.py | 2 +- certbot-dns-linode/setup.py | 2 +- certbot-dns-luadns/setup.py | 2 +- certbot-dns-nsone/setup.py | 2 +- certbot-dns-ovh/setup.py | 2 +- certbot-dns-rfc2136/setup.py | 2 +- certbot-dns-route53/setup.py | 2 +- certbot-dns-sakuracloud/setup.py | 2 +- certbot-nginx/setup.py | 2 +- certbot/__init__.py | 2 +- letsencrypt-auto-source/letsencrypt-auto | 2 +- 20 files changed, 20 insertions(+), 20 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 6ab8b4930..5493df0cc 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -3,7 +3,7 @@ from setuptools import find_packages from setuptools.command.test import test as TestCommand import sys -version = '0.36.0' +version = '0.37.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index b1cd344bf..1003ad678 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -4,7 +4,7 @@ from setuptools.command.test import test as TestCommand import sys -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-compatibility-test/setup.py b/certbot-compatibility-test/setup.py index f3dfac291..8c04a96ae 100644 --- a/certbot-compatibility-test/setup.py +++ b/certbot-compatibility-test/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' install_requires = [ 'certbot', diff --git a/certbot-dns-cloudflare/setup.py b/certbot-dns-cloudflare/setup.py index 8c6e8e434..8a7a36c2e 100644 --- a/certbot-dns-cloudflare/setup.py +++ b/certbot-dns-cloudflare/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-cloudxns/setup.py b/certbot-dns-cloudxns/setup.py index 13c9cf5e0..196a3641f 100644 --- a/certbot-dns-cloudxns/setup.py +++ b/certbot-dns-cloudxns/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-digitalocean/setup.py b/certbot-dns-digitalocean/setup.py index 3b6d8ce2e..466ab60ec 100644 --- a/certbot-dns-digitalocean/setup.py +++ b/certbot-dns-digitalocean/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsimple/setup.py b/certbot-dns-dnsimple/setup.py index cc106a7ac..26eaaf7c2 100644 --- a/certbot-dns-dnsimple/setup.py +++ b/certbot-dns-dnsimple/setup.py @@ -3,7 +3,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsmadeeasy/setup.py b/certbot-dns-dnsmadeeasy/setup.py index 04c4c33a4..db36f256f 100644 --- a/certbot-dns-dnsmadeeasy/setup.py +++ b/certbot-dns-dnsmadeeasy/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-gehirn/setup.py b/certbot-dns-gehirn/setup.py index 64999b851..975d79231 100644 --- a/certbot-dns-gehirn/setup.py +++ b/certbot-dns-gehirn/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-google/setup.py b/certbot-dns-google/setup.py index f8989c340..a7cae3bbe 100644 --- a/certbot-dns-google/setup.py +++ b/certbot-dns-google/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-linode/setup.py b/certbot-dns-linode/setup.py index 94675e939..81329ee8d 100644 --- a/certbot-dns-linode/setup.py +++ b/certbot-dns-linode/setup.py @@ -1,7 +1,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-luadns/setup.py b/certbot-dns-luadns/setup.py index 7d1bd28b2..4ee89f076 100644 --- a/certbot-dns-luadns/setup.py +++ b/certbot-dns-luadns/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-nsone/setup.py b/certbot-dns-nsone/setup.py index e024f4676..73fbf768b 100644 --- a/certbot-dns-nsone/setup.py +++ b/certbot-dns-nsone/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-ovh/setup.py b/certbot-dns-ovh/setup.py index ef272f190..b18dc5057 100644 --- a/certbot-dns-ovh/setup.py +++ b/certbot-dns-ovh/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-rfc2136/setup.py b/certbot-dns-rfc2136/setup.py index 2bd6df51e..f78f7a2c7 100644 --- a/certbot-dns-rfc2136/setup.py +++ b/certbot-dns-rfc2136/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-route53/setup.py b/certbot-dns-route53/setup.py index dede8e7c4..43f8081fe 100644 --- a/certbot-dns-route53/setup.py +++ b/certbot-dns-route53/setup.py @@ -1,7 +1,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-sakuracloud/setup.py b/certbot-dns-sakuracloud/setup.py index 9d79fab9c..17e419ae0 100644 --- a/certbot-dns-sakuracloud/setup.py +++ b/certbot-dns-sakuracloud/setup.py @@ -2,7 +2,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.36.0' +version = '0.37.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-nginx/setup.py b/certbot-nginx/setup.py index d3b5c2132..c30202272 100644 --- a/certbot-nginx/setup.py +++ b/certbot-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools.command.test import test as TestCommand import sys -version = '0.36.0' +version = '0.37.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot/__init__.py b/certbot/__init__.py index 0d23d2084..f5bd37e3c 100644 --- a/certbot/__init__.py +++ b/certbot/__init__.py @@ -1,4 +1,4 @@ """Certbot client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.36.0' +__version__ = '0.37.0.dev0' diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index fd2cfb29b..b5f8500ad 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="0.36.0" +LE_AUTO_VERSION="0.37.0.dev0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates From 82ad73612097146b019b1a803eb1f53bd16ac1c9 Mon Sep 17 00:00:00 2001 From: Lucid One Date: Thu, 11 Jul 2019 17:40:24 -0400 Subject: [PATCH 07/26] Fixes #7220 to allow config to be loaded from <(envsubst < template) (#7221) * Fixes #7220 to allow config to be loaded from <(envsubst < template) --- certbot/plugins/dns_common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot/plugins/dns_common.py b/certbot/plugins/dns_common.py index 9be2868b0..e7fbd3889 100644 --- a/certbot/plugins/dns_common.py +++ b/certbot/plugins/dns_common.py @@ -303,8 +303,8 @@ def validate_file(filename): if not os.path.exists(filename): raise errors.PluginError('File not found: {0}'.format(filename)) - if not os.path.isfile(filename): - raise errors.PluginError('Path is not a file: {0}'.format(filename)) + if os.path.isdir(filename): + raise errors.PluginError('Path is a directory: {0}'.format(filename)) def validate_file_permissions(filename): From c4684f187a4ce4ef13425cfba607dec9d8bfa963 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Fri, 12 Jul 2019 02:49:52 +0200 Subject: [PATCH 08/26] Add a test for the default directories on Windows (#7238) There is a unit test to check that the default directories for Certbot are not diverging, in certbot.tests.cli_test:FlagDefaultTests:test_linux_directories. But this test is not done on Windows. This PR fixes that. --- certbot/tests/cli_test.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 9dd16db2d..c1a489267 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -1,7 +1,6 @@ """Tests for certbot.cli.""" import argparse import copy -import sys import tempfile import unittest @@ -44,11 +43,15 @@ class TestReadFile(TempDirTestCase): class FlagDefaultTest(unittest.TestCase): """Tests cli.flag_default""" - def test_linux_directories(self): - if 'fcntl' in sys.modules: + def test_default_directories(self): + if os.name != 'nt': self.assertEqual(cli.flag_default('config_dir'), '/etc/letsencrypt') self.assertEqual(cli.flag_default('work_dir'), '/var/lib/letsencrypt') self.assertEqual(cli.flag_default('logs_dir'), '/var/log/letsencrypt') + else: + self.assertEqual(cli.flag_default('config_dir'), 'C:\\Certbot') + self.assertEqual(cli.flag_default('work_dir'), 'C:\\Certbot\\lib') + self.assertEqual(cli.flag_default('logs_dir'), 'C:\\Certbot\\log') class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods From 750d6a9686a98204c3ea46208999f6ae0ee15479 Mon Sep 17 00:00:00 2001 From: Po-Chuan Hsieh Date: Sat, 13 Jul 2019 03:53:43 +0800 Subject: [PATCH 09/26] Unify license filename (LICENSE.txt) (#7239) * Unify license filename (LICENSE.txt) --- certbot-dns-route53/{LICENSE => LICENSE.txt} | 0 certbot-dns-route53/MANIFEST.in | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename certbot-dns-route53/{LICENSE => LICENSE.txt} (100%) diff --git a/certbot-dns-route53/LICENSE b/certbot-dns-route53/LICENSE.txt similarity index 100% rename from certbot-dns-route53/LICENSE rename to certbot-dns-route53/LICENSE.txt diff --git a/certbot-dns-route53/MANIFEST.in b/certbot-dns-route53/MANIFEST.in index c48c07e59..ca37a7baf 100644 --- a/certbot-dns-route53/MANIFEST.in +++ b/certbot-dns-route53/MANIFEST.in @@ -1,3 +1,3 @@ -include LICENSE +include LICENSE.txt include README recursive-include docs * From 41a17f913e7d710c584105a4f16cc13d0640b88e Mon Sep 17 00:00:00 2001 From: J0WI Date: Wed, 17 Jul 2019 22:05:02 +0200 Subject: [PATCH 10/26] Use Buster as base image (#7251) --- Dockerfile-dev | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile-dev b/Dockerfile-dev index 1ab56e081..86f5f04e7 100644 --- a/Dockerfile-dev +++ b/Dockerfile-dev @@ -1,5 +1,5 @@ # This Dockerfile builds an image for development. -FROM ubuntu:xenial +FROM debian:buster # Note: this only exposes the port to other docker containers. EXPOSE 80 443 From 71ff47daad97dab3c2659a705d036428275c7bd2 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 18 Jul 2019 23:31:39 +0200 Subject: [PATCH 11/26] Implement a consistent realpath function in certbot.compat.filesystem (#7242) Fixes #7115 This PR creates a `realpath` method in `filesystem`, whose goal is to replace any call to `os.path.realpath` in Certbot. The reason is that `os.path.realpath` is broken on some versions of Python for Windows. See https://bugs.python.org/issue9949. The function created here works consistently across Linux and Windows. As for the other forbidden functions in `os` module, our `certbot.compat.os` will raise an exception if its `path.realpath` function is invoked, and using the `os` module from Python is forbidden from the pylint check implemented in our CI. Every call to `os.path.realpath` is corrected in `certbot` and `certbot-apache` modules. * Forbid os.path.realpath * Finish implementation * Use filesystem.realpath * Control symlink loops also for Linux * Add a test for forbidden method * Import a new object from os.path module * Use same approach of wrapping than certbot.compat.os * Correct errors * Fix dependencies * Make path module internal --- certbot-apache/certbot_apache/configurator.py | 7 +-- .../certbot_apache/override_debian.py | 3 +- .../certbot_apache/tests/centos_test.py | 3 +- .../certbot_apache/tests/configurator_test.py | 5 +- .../certbot_apache/tests/debian_test.py | 4 +- .../certbot_apache/tests/fedora_test.py | 3 +- .../certbot_apache/tests/gentoo_test.py | 3 +- certbot-apache/local-oldest-requirements.txt | 2 +- certbot-apache/setup.py | 2 +- certbot/compat/_path.py | 31 +++++++++++ certbot/compat/filesystem.py | 28 ++++++++-- certbot/compat/os.py | 4 +- certbot/main.py | 9 ++-- certbot/plugins/common.py | 2 +- certbot/plugins/storage_test.py | 2 +- certbot/tests/cert_manager_test.py | 4 +- certbot/tests/compat/filesystem_test.py | 53 ++++++++++++++----- certbot/tests/compat/os_test.py | 4 ++ certbot/tests/main_test.py | 2 +- certbot/tests/util_test.py | 8 +-- 20 files changed, 134 insertions(+), 45 deletions(-) create mode 100644 certbot/compat/_path.py diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index c433c5159..f7c27bf76 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -23,6 +23,7 @@ from certbot import interfaces from certbot import util from certbot.achallenges import KeyAuthorizationAnnotatedChallenge # pylint: disable=unused-import +from certbot.compat import filesystem from certbot.compat import os from certbot.plugins import common from certbot.plugins.util import path_surgery @@ -895,7 +896,7 @@ class ApacheConfigurator(common.Installer): if not new_vhost: continue internal_path = apache_util.get_internal_aug_path(new_vhost.path) - realpath = os.path.realpath(new_vhost.filep) + realpath = filesystem.realpath(new_vhost.filep) if realpath not in file_paths: file_paths[realpath] = new_vhost.filep internal_paths[realpath].add(internal_path) @@ -1221,11 +1222,11 @@ class ApacheConfigurator(common.Installer): """ if self.conf("vhost-root") and os.path.exists(self.conf("vhost-root")): - fp = os.path.join(os.path.realpath(self.option("vhost_root")), + fp = os.path.join(filesystem.realpath(self.option("vhost_root")), os.path.basename(non_ssl_vh_fp)) else: # Use non-ssl filepath - fp = os.path.realpath(non_ssl_vh_fp) + fp = filesystem.realpath(non_ssl_vh_fp) if fp.endswith(".conf"): return fp[:-(len(".conf"))] + self.option("le_vhost_ext") diff --git a/certbot-apache/certbot_apache/override_debian.py b/certbot-apache/certbot_apache/override_debian.py index 5706ce760..58492bd01 100644 --- a/certbot-apache/certbot_apache/override_debian.py +++ b/certbot-apache/certbot_apache/override_debian.py @@ -7,6 +7,7 @@ import zope.interface from certbot import errors from certbot import interfaces from certbot import util +from certbot.compat import filesystem from certbot.compat import os from certbot_apache import apache_util @@ -65,7 +66,7 @@ class DebianConfigurator(configurator.ApacheConfigurator): try: os.symlink(vhost.filep, enabled_path) except OSError as err: - if os.path.islink(enabled_path) and os.path.realpath( + if os.path.islink(enabled_path) and filesystem.realpath( enabled_path) == vhost.filep: # Already in shape vhost.enabled = True diff --git a/certbot-apache/certbot_apache/tests/centos_test.py b/certbot-apache/certbot_apache/tests/centos_test.py index 5d16c2b55..dddbf489e 100644 --- a/certbot-apache/certbot_apache/tests/centos_test.py +++ b/certbot-apache/certbot_apache/tests/centos_test.py @@ -4,6 +4,7 @@ import unittest import mock from certbot import errors +from certbot.compat import filesystem from certbot.compat import os from certbot_apache import obj @@ -160,7 +161,7 @@ class MultipleVhostsTestCentOS(util.ApacheTest): """Make sure we read the sysconfig OPTIONS variable correctly""" # Return nothing for the process calls mock_cfg.return_value = "" - self.config.parser.sysconfig_filep = os.path.realpath( + self.config.parser.sysconfig_filep = filesystem.realpath( os.path.join(self.config.parser.root, "../sysconfig/httpd")) self.config.parser.variables = {} diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index f9b6fb186..1eafae982 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -675,8 +675,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_make_vhost_ssl_nonexistent_vhost_path(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[1]) self.assertEqual(os.path.dirname(ssl_vhost.filep), - os.path.dirname(os.path.realpath( - self.vh_truth[1].filep))) + os.path.dirname(filesystem.realpath(self.vh_truth[1].filep))) def test_make_vhost_ssl(self): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) @@ -1336,7 +1335,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") self.config.parser.modules.add("socache_shmcb_module") - tmp_path = os.path.realpath(tempfile.mkdtemp("vhostroot")) + tmp_path = filesystem.realpath(tempfile.mkdtemp("vhostroot")) filesystem.chmod(tmp_path, 0o755) mock_p = "certbot_apache.configurator.ApacheConfigurator._get_ssl_vhost_path" mock_a = "certbot_apache.parser.ApacheParser.add_include" diff --git a/certbot-apache/certbot_apache/tests/debian_test.py b/certbot-apache/certbot_apache/tests/debian_test.py index f1d5843a5..54ced2d0b 100644 --- a/certbot-apache/certbot_apache/tests/debian_test.py +++ b/certbot-apache/certbot_apache/tests/debian_test.py @@ -79,9 +79,9 @@ class MultipleVhostsTestDebian(util.ApacheTest): def test_enable_site_failure(self): self.config.parser.root = "/tmp/nonexistent" - with mock.patch("os.path.isdir") as mock_dir: + with mock.patch("certbot.compat.os.path.isdir") as mock_dir: mock_dir.return_value = True - with mock.patch("os.path.islink") as mock_link: + with mock.patch("certbot.compat.os.path.islink") as mock_link: mock_link.return_value = False self.assertRaises( errors.NotSupportedError, diff --git a/certbot-apache/certbot_apache/tests/fedora_test.py b/certbot-apache/certbot_apache/tests/fedora_test.py index 67533fe1d..4d3f3a313 100644 --- a/certbot-apache/certbot_apache/tests/fedora_test.py +++ b/certbot-apache/certbot_apache/tests/fedora_test.py @@ -4,6 +4,7 @@ import unittest import mock from certbot import errors +from certbot.compat import filesystem from certbot.compat import os from certbot_apache import obj @@ -160,7 +161,7 @@ class MultipleVhostsTestFedora(util.ApacheTest): """Make sure we read the sysconfig OPTIONS variable correctly""" # Return nothing for the process calls mock_cfg.return_value = "" - self.config.parser.sysconfig_filep = os.path.realpath( + self.config.parser.sysconfig_filep = filesystem.realpath( os.path.join(self.config.parser.root, "../sysconfig/httpd")) self.config.parser.variables = {} diff --git a/certbot-apache/certbot_apache/tests/gentoo_test.py b/certbot-apache/certbot_apache/tests/gentoo_test.py index dd01e9170..d0d3ba0dd 100644 --- a/certbot-apache/certbot_apache/tests/gentoo_test.py +++ b/certbot-apache/certbot_apache/tests/gentoo_test.py @@ -4,6 +4,7 @@ import unittest import mock from certbot import errors +from certbot.compat import filesystem from certbot.compat import os from certbot_apache import obj @@ -81,7 +82,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest): """Make sure we read the Gentoo APACHE2_OPTS variable correctly""" defines = ['DEFAULT_VHOST', 'INFO', 'SSL', 'SSL_DEFAULT_VHOST', 'LANGUAGE'] - self.config.parser.apacheconfig_filep = os.path.realpath( + self.config.parser.apacheconfig_filep = filesystem.realpath( os.path.join(self.config.parser.root, "../conf.d/apache2")) self.config.parser.variables = {} with mock.patch("certbot_apache.override_gentoo.GentooParser.update_modules"): diff --git a/certbot-apache/local-oldest-requirements.txt b/certbot-apache/local-oldest-requirements.txt index a959ad44f..da509406e 100644 --- a/certbot-apache/local-oldest-requirements.txt +++ b/certbot-apache/local-oldest-requirements.txt @@ -1,3 +1,3 @@ # Remember to update setup.py to match the package versions below. acme[dev]==0.29.0 -certbot[dev]==0.36.0 +-e .[dev] diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index 1003ad678..98b7382db 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -10,7 +10,7 @@ version = '0.37.0.dev0' # acme/certbot version. install_requires = [ 'acme>=0.29.0', - 'certbot>=0.36.0', + 'certbot>=0.37.0.dev0', 'mock', 'python-augeas', 'setuptools', diff --git a/certbot/compat/_path.py b/certbot/compat/_path.py new file mode 100644 index 000000000..fe2d2d1d2 --- /dev/null +++ b/certbot/compat/_path.py @@ -0,0 +1,31 @@ +"""This compat module wraps os.path to forbid some functions.""" +# pylint: disable=function-redefined +from __future__ import absolute_import + +# First round of wrapping: we import statically all public attributes exposed by the os.path +# module. This allows in particular to have pylint, mypy, IDEs be aware that most of os.path +# members are available in certbot.compat.path. +from os.path import * # type: ignore # pylint: disable=wildcard-import,unused-wildcard-import,redefined-builtin,os-module-forbidden + +# Second round of wrapping: we import dynamically all attributes from the os.path module that have +# not yet been imported by the first round (static star import). +import os.path as std_os_path # pylint: disable=os-module-forbidden +import sys as std_sys + +ourselves = std_sys.modules[__name__] +for attribute in dir(std_os_path): + # Check if the attribute does not already exist in our module. It could be internal attributes + # of the module (__name__, __doc__), or attributes from standard os.path already imported with + # `from os.path import *`. + if not hasattr(ourselves, attribute): + setattr(ourselves, attribute, getattr(std_os_path, attribute)) + +# Clean all remaining importables that are not from the core os.path module. +del ourselves, std_os_path, std_sys + + +# Function os.path.realpath is broken on some versions of Python for Windows. +def realpath(*unused_args, **unused_kwargs): + """Method os.path.realpath() is forbidden""" + raise RuntimeError('Usage of os.path.realpath() is forbidden. ' + 'Use certbot.compat.filesystem.realpath() instead.') diff --git a/certbot/compat/filesystem.py b/certbot/compat/filesystem.py index d0d1c262d..e2757eb2f 100644 --- a/certbot/compat/filesystem.py +++ b/certbot/compat/filesystem.py @@ -207,13 +207,22 @@ def replace(src, dst): os.rename(src, dst) -def _apply_win_mode(file_path, mode): +def realpath(file_path): """ - This function converts the given POSIX mode into a Windows ACL list, and applies it to the - file given its path. If the given path is a symbolic link, it will resolved to apply the - mode on the targeted file. + Find the real path for the given path. This method resolves symlinks, including + recursive symlinks, and is protected against symlinks that creates an infinite loop. """ original_path = file_path + + if POSIX_MODE: + path = os.path.realpath(file_path) + if os.path.islink(path): + # If path returned by realpath is still a link, it means that it failed to + # resolve the symlink because of a loop. + # See realpath code: https://github.com/python/cpython/blob/master/Lib/posixpath.py + raise RuntimeError('Error, link {0} is a loop!'.format(original_path)) + return path + inspected_paths = [] # type: List[str] while os.path.islink(file_path): link_path = file_path @@ -223,6 +232,17 @@ def _apply_win_mode(file_path, mode): if file_path in inspected_paths: raise RuntimeError('Error, link {0} is a loop!'.format(original_path)) inspected_paths.append(file_path) + + return os.path.abspath(file_path) + + +def _apply_win_mode(file_path, mode): + """ + This function converts the given POSIX mode into a Windows ACL list, and applies it to the + file given its path. If the given path is a symbolic link, it will resolved to apply the + mode on the targeted file. + """ + file_path = realpath(file_path) # Get owner sid of the file security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION) user = security.GetSecurityDescriptorOwner() diff --git a/certbot/compat/os.py b/certbot/compat/os.py index 6d3c7bb02..bb0e02eb3 100644 --- a/certbot/compat/os.py +++ b/certbot/compat/os.py @@ -26,7 +26,9 @@ for attribute in dir(std_os): if not hasattr(ourselves, attribute): setattr(ourselves, attribute, getattr(std_os, attribute)) -# Similar to os.path, allow certbot.compat.os.path to behave as a module +# Import our internal path module, then allow certbot.compat.os.path +# to behave as a module (similarly to os.path). +from certbot.compat import _path as path # type: ignore # pylint: disable=wrong-import-position std_sys.modules[__name__ + '.path'] = path # Clean all remaining importables that are not from the core os module. diff --git a/certbot/main.py b/certbot/main.py index 6bd47cee3..50470438f 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -31,6 +31,7 @@ from certbot import reporter from certbot import storage from certbot import updater from certbot import util +from certbot.compat import filesystem from certbot.compat import misc from certbot.compat import os from certbot.display import util as display_util, ops as display_ops @@ -841,12 +842,12 @@ def _populate_from_certname(config): return config def _check_certificate_and_key(config): - if not os.path.isfile(os.path.realpath(config.cert_path)): + if not os.path.isfile(filesystem.realpath(config.cert_path)): raise errors.ConfigurationError("Error while reading certificate from path " - "{0}".format(config.cert_path)) - if not os.path.isfile(os.path.realpath(config.key_path)): + "{0}".format(config.cert_path)) + if not os.path.isfile(filesystem.realpath(config.key_path)): raise errors.ConfigurationError("Error while reading private key from path " - "{0}".format(config.key_path)) + "{0}".format(config.key_path)) def plugins_cmd(config, plugins): """List server software plugins. diff --git a/certbot/plugins/common.py b/certbot/plugins/common.py index 2e2e0f64c..5c292bfad 100644 --- a/certbot/plugins/common.py +++ b/certbot/plugins/common.py @@ -486,7 +486,7 @@ def dir_setup(test_dir, pkg): # pragma: no cover link, (ex: OS X) such plugins will be confused. This function prevents such a case. """ - return os.path.realpath(tempfile.mkdtemp(prefix)) + return filesystem.realpath(tempfile.mkdtemp(prefix)) temp_dir = expanded_tempdir("temp") config_dir = expanded_tempdir("config") diff --git a/certbot/plugins/storage_test.py b/certbot/plugins/storage_test.py index f72e69d4b..9d08cc7ef 100644 --- a/certbot/plugins/storage_test.py +++ b/certbot/plugins/storage_test.py @@ -31,7 +31,7 @@ class PluginStorageTest(test_util.ConfigTestCase): self.plugin.storage.storagepath = os.path.join(self.config.config_dir, ".pluginstorage.json") with mock.patch("six.moves.builtins.open", mock_open): - with mock.patch('os.path.isfile', return_value=True): + with mock.patch('certbot.compat.os.path.isfile', return_value=True): with mock.patch("certbot.reverter.util"): self.assertRaises(errors.PluginStorageError, self.plugin.storage._load) # pylint: disable=protected-access diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index 30ca6f099..b4cd946ee 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -97,8 +97,8 @@ class UpdateLiveSymlinksTest(BaseCertManagerTest): for kind in ALL_FOUR: os.chdir(os.path.dirname(self.config_files[domain][kind])) self.assertEqual( - os.path.realpath(os.readlink(self.config_files[domain][kind])), - os.path.realpath(archive_paths[domain][kind])) + filesystem.realpath(os.readlink(self.config_files[domain][kind])), + filesystem.realpath(archive_paths[domain][kind])) finally: os.chdir(prev_dir) diff --git a/certbot/tests/compat/filesystem_test.py b/certbot/tests/compat/filesystem_test.py index 27c48c19a..5d112b652 100644 --- a/certbot/tests/compat/filesystem_test.py +++ b/certbot/tests/compat/filesystem_test.py @@ -48,18 +48,6 @@ class WindowsChmodTests(TempDirTestCase): self.assertFalse(filesystem._compare_dacls(ref_dacl_probe, cur_dacl_probe)) # pylint: disable=protected-access self.assertTrue(filesystem._compare_dacls(ref_dacl_link, cur_dacl_link)) # pylint: disable=protected-access - def test_symlink_loop_mitigation(self): - link1_path = os.path.join(self.tempdir, 'link1') - link2_path = os.path.join(self.tempdir, 'link2') - link3_path = os.path.join(self.tempdir, 'link3') - os.symlink(link1_path, link2_path) - os.symlink(link2_path, link3_path) - os.symlink(link3_path, link1_path) - - with self.assertRaises(RuntimeError) as error: - filesystem.chmod(link1_path, 0o755) - self.assertTrue('link1 is a loop!' in str(error.exception)) - def test_world_permission(self): everybody = win32security.ConvertStringSidToSid(EVERYBODY_SID) @@ -320,7 +308,6 @@ class CopyOwnershipTest(test_util.TempDirTestCase): class OsReplaceTest(test_util.TempDirTestCase): """Test to ensure consistent behavior of rename method""" - def test_os_replace_to_existing_file(self): """Ensure that replace will effectively rename src into dst for all platforms.""" src = os.path.join(self.tempdir, 'src') @@ -335,6 +322,46 @@ class OsReplaceTest(test_util.TempDirTestCase): self.assertTrue(os.path.exists(dst)) +class RealpathTest(test_util.TempDirTestCase): + """Tests for realpath method""" + def setUp(self): + super(RealpathTest, self).setUp() + self.probe_path = _create_probe(self.tempdir) + + def test_symlink_resolution(self): + # Absolute resolution + link_path = os.path.join(self.tempdir, 'link_abs') + os.symlink(self.probe_path, link_path) + + self.assertEqual(self.probe_path, filesystem.realpath(self.probe_path)) + self.assertEqual(self.probe_path, filesystem.realpath(link_path)) + + # Relative resolution + curdir = os.getcwd() + link_path = os.path.join(self.tempdir, 'link_rel') + probe_name = os.path.basename(self.probe_path) + try: + os.chdir(os.path.dirname(self.probe_path)) + os.symlink(probe_name, link_path) + + self.assertEqual(self.probe_path, filesystem.realpath(probe_name)) + self.assertEqual(self.probe_path, filesystem.realpath(link_path)) + finally: + os.chdir(curdir) + + def test_symlink_loop_mitigation(self): + link1_path = os.path.join(self.tempdir, 'link1') + link2_path = os.path.join(self.tempdir, 'link2') + link3_path = os.path.join(self.tempdir, 'link3') + os.symlink(link1_path, link2_path) + os.symlink(link2_path, link3_path) + os.symlink(link3_path, link1_path) + + with self.assertRaises(RuntimeError) as error: + filesystem.realpath(link1_path) + self.assertTrue('link1 is a loop!' in str(error.exception)) + + def _get_security_dacl(target): return win32security.GetFileSecurity(target, win32security.DACL_SECURITY_INFORMATION) diff --git a/certbot/tests/compat/os_test.py b/certbot/tests/compat/os_test.py index 9e25548f0..754e97a10 100644 --- a/certbot/tests/compat/os_test.py +++ b/certbot/tests/compat/os_test.py @@ -7,8 +7,12 @@ from certbot.compat import os class OsTest(unittest.TestCase): """Unit tests for os module.""" def test_forbidden_methods(self): + # Checks for os module for method in ['chmod', 'chown', 'open', 'mkdir', 'makedirs', 'rename', 'replace']: self.assertRaises(RuntimeError, getattr(os, method)) + # Checks for os.path module + for method in ['realpath']: + self.assertRaises(RuntimeError, getattr(os.path, method)) if __name__ == "__main__": diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index ebd9a28e1..ff8a800ab 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -542,7 +542,7 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met return True return orig_open(fn) - with mock.patch("os.path.isfile") as mock_if: + with mock.patch("certbot.compat.os.path.isfile") as mock_if: mock_if.side_effect = mock_isfile with mock.patch('certbot.main.client') as client: ret, stdout, stderr = self._call_no_clientmock(args, stdout) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 41b5cf71d..b96f8afd2 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -528,7 +528,7 @@ class OsInfoTest(unittest.TestCase): from certbot.util import (get_os_info, get_systemd_os_info, get_os_info_ua) - with mock.patch('os.path.isfile', return_value=True): + with mock.patch('certbot.compat.os.path.isfile', return_value=True): self.assertEqual(get_os_info( test_util.vector_path("os-release"))[0], 'systemdos') self.assertEqual(get_os_info( @@ -536,13 +536,13 @@ class OsInfoTest(unittest.TestCase): self.assertEqual(get_systemd_os_info(os.devnull), ("", "")) self.assertEqual(get_os_info_ua( test_util.vector_path("os-release")), "SystemdOS") - with mock.patch('os.path.isfile', return_value=False): + with mock.patch('certbot.compat.os.path.isfile', return_value=False): self.assertEqual(get_systemd_os_info(), ("", "")) def test_systemd_os_release_like(self): from certbot.util import get_systemd_os_like - with mock.patch('os.path.isfile', return_value=True): + with mock.patch('certbot.compat.os.path.isfile', return_value=True): id_likes = get_systemd_os_like(test_util.vector_path( "os-release")) self.assertEqual(len(id_likes), 3) @@ -552,7 +552,7 @@ class OsInfoTest(unittest.TestCase): def test_non_systemd_os_info(self, popen_mock): from certbot.util import (get_os_info, get_python_os_info, get_os_info_ua) - with mock.patch('os.path.isfile', return_value=False): + with mock.patch('certbot.compat.os.path.isfile', return_value=False): with mock.patch('platform.system_alias', return_value=('NonSystemD', '42', '42')): self.assertEqual(get_os_info()[0], 'nonsystemd') From f7c736da6fa04b3db6520a11152e0a4c6a6400cf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 18 Jul 2019 15:44:01 -0700 Subject: [PATCH 12/26] Update pexpect to fix Python 3.7 dev venvs. (#7259) --- tools/dev_constraints.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/dev_constraints.txt b/tools/dev_constraints.txt index 90b4775c1..1aa8414f9 100644 --- a/tools/dev_constraints.txt +++ b/tools/dev_constraints.txt @@ -42,12 +42,12 @@ mypy==0.600 ndg-httpsclient==0.3.2 oauth2client==2.0.0 pathlib2==2.3.0 -pexpect==4.2.1 +pexpect==4.7.0 pickleshare==0.7.4 pkginfo==1.4.2 pluggy==0.5.2 prompt-toolkit==1.0.15 -ptyprocess==0.5.2 +ptyprocess==0.6.0 py==1.8.0 pyasn1==0.1.9 pyasn1-modules==0.0.10 From 47f64c7280ec547561e8f92c022e57d82061c29e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 19 Jul 2019 10:44:17 -0700 Subject: [PATCH 13/26] Remove list of packaging efforts. (#7258) I think this list maybe had value when distros were first starting to package Certbot, but now I don't think it does. What function does this list serve? The instruction generator at https://certbot.eff.org/instructions does a much better job telling users how to use these packages. On the packaging side, I think anyone capable of packaging Certbot at the various distros would be able to search their repositories to see if a Certbot package is available. Since this list is hard to maintain as links semi-regularly break and keeping it up to date with all distros and all Certbot components is a fair bit of work, let's just remove it. This PR was motivated by the Travis failures at https://travis-ci.com/certbot/website/builds/119588518 due to GNU Guix changing the layout of their site. --- docs/packaging.rst | 79 ---------------------------------------------- 1 file changed, 79 deletions(-) diff --git a/docs/packaging.rst b/docs/packaging.rst index bb26c7ef6..7b0b1d41a 100644 --- a/docs/packaging.rst +++ b/docs/packaging.rst @@ -47,82 +47,3 @@ Notes for package maintainers 4. ``jws`` is an internal script for ``acme`` module and it doesn't have to be packaged - it's mostly for debugging: you can use it as ``echo foo | jws sign | jws verify``. 5. Do get in touch with us. We are happy to make any changes that will make packaging easier. If you need to apply some patches don't do it downstream - make a PR here. - -Already ongoing efforts -======================= - - -Arch ----- - -From our official releases: - -- https://www.archlinux.org/packages/community/any/python-acme -- https://www.archlinux.org/packages/community/any/certbot -- https://www.archlinux.org/packages/community/any/certbot-apache -- https://www.archlinux.org/packages/community/any/certbot-nginx -- https://www.archlinux.org/packages/community/any/certbot-dns-cloudflare -- https://www.archlinux.org/packages/community/any/certbot-dns-cloudxns -- https://www.archlinux.org/packages/community/any/certbot-dns-digitalocean -- https://www.archlinux.org/packages/community/any/certbot-dns-dnsimple -- https://www.archlinux.org/packages/community/any/certbot-dns-dnsmadeeasy -- https://www.archlinux.org/packages/community/any/certbot-dns-google -- https://www.archlinux.org/packages/community/any/certbot-dns-luadns -- https://www.archlinux.org/packages/community/any/certbot-dns-nsone -- https://www.archlinux.org/packages/community/any/certbot-dns-rfc2136 -- https://www.archlinux.org/packages/community/any/certbot-dns-route53 - -From ``master``: https://aur.archlinux.org/packages/certbot-git - -Debian (and its derivatives, including Ubuntu) ----------------------------------------------- - -- https://packages.debian.org/sid/certbot -- https://packages.debian.org/sid/python-certbot -- https://packages.debian.org/sid/python-certbot-apache - -Fedora ------- - -In Fedora 23+. - -- https://apps.fedoraproject.org/packages/python-acme -- https://apps.fedoraproject.org/packages/certbot -- https://apps.fedoraproject.org/packages/python-certbot-apache -- https://apps.fedoraproject.org/packages/python-certbot-dns-cloudflare -- https://apps.fedoraproject.org/packages/python-certbot-dns-cloudxns -- https://apps.fedoraproject.org/packages/python-certbot-dns-digitalocean -- https://apps.fedoraproject.org/packages/python-certbot-dns-dnsimple -- https://apps.fedoraproject.org/packages/python-certbot-dns-dnsmadeeasy -- https://apps.fedoraproject.org/packages/python-certbot-dns-google -- https://apps.fedoraproject.org/packages/python-certbot-dns-luadns -- https://apps.fedoraproject.org/packages/python-certbot-dns-nsone -- https://apps.fedoraproject.org/packages/python-certbot-dns-rfc2136 -- https://apps.fedoraproject.org/packages/python-certbot-dns-route53 -- https://apps.fedoraproject.org/packages/python-certbot-nginx - -FreeBSD -------- - -- https://www.freshports.org/security/py-acme/ -- https://www.freshports.org/security/py-certbot/ - -Gentoo ------- - -Currently, all ``certbot`` related packages are in the testing branch: - -- https://packages.gentoo.org/packages/app-crypt/certbot -- https://packages.gentoo.org/packages/app-crypt/certbot-apache -- https://packages.gentoo.org/packages/app-crypt/certbot-nginx -- https://packages.gentoo.org/packages/app-crypt/acme - -GNU Guix --------- - -- https://www.gnu.org/software/guix/package-list.html#certbot - -OpenBSD -------- - -- http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/security/letsencrypt/client/ From a35470292e272fc6b0a99a0d1c47dda51e797f1f Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Mon, 22 Jul 2019 12:43:58 +0200 Subject: [PATCH 14/26] Remove Dockerfiles (#7257) --- Dockerfile | 35 ----------------------------- certbot-dns-cloudflare/Dockerfile | 5 ----- certbot-dns-cloudxns/Dockerfile | 5 ----- certbot-dns-digitalocean/Dockerfile | 5 ----- certbot-dns-dnsimple/Dockerfile | 5 ----- certbot-dns-dnsmadeeasy/Dockerfile | 5 ----- certbot-dns-gehirn/Dockerfile | 5 ----- certbot-dns-google/Dockerfile | 5 ----- certbot-dns-linode/Dockerfile | 5 ----- certbot-dns-luadns/Dockerfile | 5 ----- certbot-dns-nsone/Dockerfile | 5 ----- certbot-dns-ovh/Dockerfile | 5 ----- certbot-dns-rfc2136/Dockerfile | 5 ----- certbot-dns-route53/Dockerfile | 5 ----- certbot-dns-sakuracloud/Dockerfile | 5 ----- 15 files changed, 105 deletions(-) delete mode 100644 Dockerfile delete mode 100644 certbot-dns-cloudflare/Dockerfile delete mode 100644 certbot-dns-cloudxns/Dockerfile delete mode 100644 certbot-dns-digitalocean/Dockerfile delete mode 100644 certbot-dns-dnsimple/Dockerfile delete mode 100644 certbot-dns-dnsmadeeasy/Dockerfile delete mode 100644 certbot-dns-gehirn/Dockerfile delete mode 100644 certbot-dns-google/Dockerfile delete mode 100644 certbot-dns-linode/Dockerfile delete mode 100644 certbot-dns-luadns/Dockerfile delete mode 100644 certbot-dns-nsone/Dockerfile delete mode 100644 certbot-dns-ovh/Dockerfile delete mode 100644 certbot-dns-rfc2136/Dockerfile delete mode 100644 certbot-dns-route53/Dockerfile delete mode 100644 certbot-dns-sakuracloud/Dockerfile diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index 828f5ec94..000000000 --- a/Dockerfile +++ /dev/null @@ -1,35 +0,0 @@ -FROM python:2-alpine3.9 - -ENTRYPOINT [ "certbot" ] -EXPOSE 80 443 -VOLUME /etc/letsencrypt /var/lib/letsencrypt -WORKDIR /opt/certbot - -COPY CHANGELOG.md README.rst setup.py src/ - -# Generate constraints file to pin dependency versions -COPY letsencrypt-auto-source/pieces/dependency-requirements.txt . -COPY tools /opt/certbot/tools -RUN sh -c 'cat dependency-requirements.txt | /opt/certbot/tools/strip_hashes.py > unhashed_requirements.txt' -RUN sh -c 'cat tools/dev_constraints.txt unhashed_requirements.txt | /opt/certbot/tools/merge_requirements.py > docker_constraints.txt' - -COPY acme src/acme -COPY certbot src/certbot - -RUN apk add --no-cache --virtual .certbot-deps \ - libffi \ - libssl1.1 \ - openssl \ - ca-certificates \ - binutils -RUN apk add --no-cache --virtual .build-deps \ - gcc \ - linux-headers \ - openssl-dev \ - musl-dev \ - libffi-dev \ - && pip install -r /opt/certbot/dependency-requirements.txt \ - && pip install --no-cache-dir --no-deps \ - --editable /opt/certbot/src/acme \ - --editable /opt/certbot/src \ - && apk del .build-deps diff --git a/certbot-dns-cloudflare/Dockerfile b/certbot-dns-cloudflare/Dockerfile deleted file mode 100644 index adbf715fa..000000000 --- a/certbot-dns-cloudflare/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-cloudflare - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-cloudflare diff --git a/certbot-dns-cloudxns/Dockerfile b/certbot-dns-cloudxns/Dockerfile deleted file mode 100644 index 48c88c35c..000000000 --- a/certbot-dns-cloudxns/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-cloudxns - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-cloudxns diff --git a/certbot-dns-digitalocean/Dockerfile b/certbot-dns-digitalocean/Dockerfile deleted file mode 100644 index 342e0e876..000000000 --- a/certbot-dns-digitalocean/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-digitalocean - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-digitalocean diff --git a/certbot-dns-dnsimple/Dockerfile b/certbot-dns-dnsimple/Dockerfile deleted file mode 100644 index 724675339..000000000 --- a/certbot-dns-dnsimple/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-dnsimple - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-dnsimple diff --git a/certbot-dns-dnsmadeeasy/Dockerfile b/certbot-dns-dnsmadeeasy/Dockerfile deleted file mode 100644 index 1480baf4f..000000000 --- a/certbot-dns-dnsmadeeasy/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-dnsmadeeasy - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-dnsmadeeasy diff --git a/certbot-dns-gehirn/Dockerfile b/certbot-dns-gehirn/Dockerfile deleted file mode 100644 index 7dce0e521..000000000 --- a/certbot-dns-gehirn/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-gehirn - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-gehirn diff --git a/certbot-dns-google/Dockerfile b/certbot-dns-google/Dockerfile deleted file mode 100644 index 5750b31d9..000000000 --- a/certbot-dns-google/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-google - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-google diff --git a/certbot-dns-linode/Dockerfile b/certbot-dns-linode/Dockerfile deleted file mode 100644 index 6db8b59fb..000000000 --- a/certbot-dns-linode/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-linode - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-linode diff --git a/certbot-dns-luadns/Dockerfile b/certbot-dns-luadns/Dockerfile deleted file mode 100644 index efc9f36d6..000000000 --- a/certbot-dns-luadns/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-luadns - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-luadns diff --git a/certbot-dns-nsone/Dockerfile b/certbot-dns-nsone/Dockerfile deleted file mode 100644 index de541e850..000000000 --- a/certbot-dns-nsone/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-nsone - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-nsone diff --git a/certbot-dns-ovh/Dockerfile b/certbot-dns-ovh/Dockerfile deleted file mode 100644 index 37e488dc4..000000000 --- a/certbot-dns-ovh/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-ovh - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-ovh diff --git a/certbot-dns-rfc2136/Dockerfile b/certbot-dns-rfc2136/Dockerfile deleted file mode 100644 index 3ebb6a72e..000000000 --- a/certbot-dns-rfc2136/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-rfc2136 - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-rfc2136 diff --git a/certbot-dns-route53/Dockerfile b/certbot-dns-route53/Dockerfile deleted file mode 100644 index e1825c11d..000000000 --- a/certbot-dns-route53/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-route53 - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-route53 diff --git a/certbot-dns-sakuracloud/Dockerfile b/certbot-dns-sakuracloud/Dockerfile deleted file mode 100644 index 9fa9b3c22..000000000 --- a/certbot-dns-sakuracloud/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM certbot/certbot - -COPY . src/certbot-dns-sakuracloud - -RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-sakuracloud From 06a0dae67f7497075e6da1b0e51d4fcab2c679c6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 23 Jul 2019 11:01:29 -0700 Subject: [PATCH 15/26] Fix test_symlink_resolution on macOS. (#7263) This fixes the test failures which can be seen at https://travis-ci.com/certbot/certbot/builds/120123338. The problem here is the path returned by tempfile.mkdtemp() contains a symlink. For instance, one run of the function produced '/var/folders/3b/zg8fdh5j71x92yyzc1tyllfw0000gp/T/tmp3k9ytfj1' which is a symlink to '/private/var/folders/3b/zg8fdh5j71x92yyzc1tyllfw0000gp/T/tmp3k9ytfj1'. Removing this symlink before testing filesystem.realpath solves the problem. You can see the macOS tests passing with this change at https://travis-ci.com/certbot/certbot/builds/120250667. --- certbot/tests/compat/filesystem_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot/tests/compat/filesystem_test.py b/certbot/tests/compat/filesystem_test.py index 5d112b652..48ac04670 100644 --- a/certbot/tests/compat/filesystem_test.py +++ b/certbot/tests/compat/filesystem_test.py @@ -329,6 +329,8 @@ class RealpathTest(test_util.TempDirTestCase): self.probe_path = _create_probe(self.tempdir) def test_symlink_resolution(self): + # Remove any symlinks already in probe_path + self.probe_path = filesystem.realpath(self.probe_path) # Absolute resolution link_path = os.path.join(self.tempdir, 'link_abs') os.symlink(self.probe_path, link_path) From 391f301dd88906c3c8320096cf412b468918df6f Mon Sep 17 00:00:00 2001 From: alexzorin Date: Thu, 25 Jul 2019 11:04:59 +1000 Subject: [PATCH 16/26] acme: Implement authz deactivation (#7254) Resolves #4945. First PR in order to address #5116. * acme: Implement authz deactivation Resolves #4945 * update AUTHORS and CHANGELOG * typos in mypy annotations * formatting: missing newline * improve test_deactivate_authorization * improve deactivate_authorization * test: s/STATUS_INVALID/STATUS_DEACTIVATED/ * simplify dict to keyword argument * acme: add UpdateAuthorization * acme: use UpdateAuthorization in deactivate_authz and add mypy annotation This allows deactivate_authorization to succeed for both ACME v1 and v2 servers. --- AUTHORS.md | 1 + CHANGELOG.md | 2 +- acme/acme/client.py | 15 +++++++++++++++ acme/acme/client_test.py | 8 ++++++++ acme/acme/messages.py | 9 ++++++++- 5 files changed, 33 insertions(+), 2 deletions(-) diff --git a/AUTHORS.md b/AUTHORS.md index 340a0a94b..45d00eda7 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -15,6 +15,7 @@ Authors * [Alex Gaynor](https://github.com/alex) * [Alex Halderman](https://github.com/jhalderm) * [Alex Jordan](https://github.com/strugee) +* [Alex Zorin](https://github.com/alexzorin) * [Amjad Mashaal](https://github.com/TheNavigat) * [Andrew Murray](https://github.com/radarhere) * [Anselm Levskaya](https://github.com/levskaya) diff --git a/CHANGELOG.md b/CHANGELOG.md index 091ec0493..6f0da8ce1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* +* acme: Authz deactivation added to `acme` module. ### Changed diff --git a/acme/acme/client.py b/acme/acme/client.py index 5a8fd88ae..9698c7609 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -123,6 +123,21 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes """ return self.update_registration(regr, update={'status': 'deactivated'}) + def deactivate_authorization(self, authzr): + # type: (messages.AuthorizationResource) -> messages.AuthorizationResource + """Deactivate authorization. + + :param messages.AuthorizationResource authzr: The Authorization resource + to be deactivated. + + :returns: The Authorization resource that was deactivated. + :rtype: `.AuthorizationResource` + + """ + body = messages.UpdateAuthorization(status='deactivated') + response = self._post(authzr.uri, body) + return self._authzr_from_response(response) + def _authzr_from_response(self, response, identifier=None, uri=None): authzr = messages.AuthorizationResource( body=messages.Authorization.from_json(response.json()), diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 406201751..4c762031b 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -637,6 +637,14 @@ class ClientTest(ClientTestBase): errors.PollError, self.client.poll_and_request_issuance, csr, authzrs, mintime=mintime, max_attempts=2) + def test_deactivate_authorization(self): + authzb = self.authzr.body.update(status=messages.STATUS_DEACTIVATED) + self.response.json.return_value = authzb.to_json() + authzr = self.client.deactivate_authorization(self.authzr) + self.assertEqual(authzb, authzr.body) + self.assertEqual(self.client.net.post.call_count, 1) + self.assertTrue(self.authzr.uri in self.net.post.call_args_list[0][0]) + def test_check_cert(self): self.response.headers['Location'] = self.certr.uri self.response.content = CERT_DER diff --git a/acme/acme/messages.py b/acme/acme/messages.py index df96b5f2b..2bfe688d2 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -168,6 +168,7 @@ STATUS_VALID = Status('valid') STATUS_INVALID = Status('invalid') STATUS_REVOKED = Status('revoked') STATUS_READY = Status('ready') +STATUS_DEACTIVATED = Status('deactivated') class IdentifierType(_Constant): @@ -471,7 +472,7 @@ class Authorization(ResourceBody): :ivar datetime.datetime expires: """ - identifier = jose.Field('identifier', decoder=Identifier.from_json) + identifier = jose.Field('identifier', decoder=Identifier.from_json, omitempty=True) challenges = jose.Field('challenges', omitempty=True) combinations = jose.Field('combinations', omitempty=True) @@ -501,6 +502,12 @@ class NewAuthorization(Authorization): resource = fields.Resource(resource_type) +class UpdateAuthorization(Authorization): + """Update authorization.""" + resource_type = 'authz' + resource = fields.Resource(resource_type) + + class AuthorizationResource(ResourceWithURI): """Authorization Resource. From bf9c681c4f3ad444a74cb935b59965c2f1edfb32 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 25 Jul 2019 01:20:52 -0700 Subject: [PATCH 17/26] fix backwards logic (#7265) --- tests/letstest/multitester.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/letstest/multitester.py b/tests/letstest/multitester.py index 2870aaa09..d63b7ab5a 100644 --- a/tests/letstest/multitester.py +++ b/tests/letstest/multitester.py @@ -375,7 +375,7 @@ def cleanup(cl_args, instances, targetlist): # If lengths of instances and targetlist aren't equal, instances failed to # start before running tests so leaving instances running for debugging # isn't very useful. Let's cleanup after ourselves instead. - if len(instances) == len(targetlist) or not cl_args.saveinstances: + if len(instances) != len(targetlist) or not cl_args.saveinstances: print('Terminating EC2 Instances') if cl_args.killboulder: boulder_server.terminate() From 40da709792d0e5907207ce1346020c72d8ec34be Mon Sep 17 00:00:00 2001 From: alexzorin Date: Thu, 25 Jul 2019 18:23:28 +1000 Subject: [PATCH 18/26] docs: s/certbot_tests/certbot_test/ (#7267) --- docs/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 715788332..8aeef54cc 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -176,7 +176,7 @@ that can be used once the virtual environment is activated: .. code-block:: shell - certbot_tests [ARGS...] + certbot_test [ARGS...] - Execute certbot with the provided arguments and other arguments useful for testing purposes, such as: verbose output, full tracebacks in case Certbot crashes, *etc.* From e6bf3fe7f81ff7651b3e8be3d530be725090ed2c Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Fri, 26 Jul 2019 00:25:36 +0200 Subject: [PATCH 19/26] [Windows] Security model for files permissions - STEP 3f (#7233) * Correct file permissions on TempHandler * Forbid os.chown and os.geteuid, as theses functions can be harmful to the security model on Windows. * Implement copy_ownership * Apply copy_ownership * Correct webroot tests (and activate another broken test !) * Correct lint and mypy * Ensure to apply mode in makedirs * Apply strict permissions on directories created with tempfile.mkdtemp(), like on Unix. * Ensure streamHandler has 0600 on Windows * Reactivate a test on windows * Pin oldest requirements to current internal libraries (acme and certbot) * Add dynamically pywin32 in dependencies: always except for certbot-oldest to avoid to break the relevant tests. * Administrative privileges are always required. * Correct security implementation (not the logic yet) * First correction. Allow to manipulate finely file permissions during their generation * Align to master + fix lint + resolve correctly symbolic links * Add a test for windows about default paths * Strenghthen the detection of Linux/Windows to check the standard files layout. * Fix lint and mypy * Reflect non usage of cache discovery from dns google plugin to its tests, solving Windows tests on the way * Apply suggestions from code review Co-Authored-By: adferrand * Add more details in a comment * Retrigger build. * Add documentation. * Fix a test * Correct RW clear down * Update util.py * Remove unused code * Fix code style * Adapt certbot coverage threshold on Linux due to Windows specific LOC addition. * Various optimizations around file owner and file mode * Fix last error * Fix copy_ownership_and_apply_mode * Fix lint * Correct mypy * Extract out first part from windows-file-permissions * Ignore new_compat in coverage for now * Create test package for compat * Add unit tests for security module. * Add pywin32 * Adapt linux coverages to the windows-specific LOCs added * Clean imports * Correct import * Trigger CI * Reactivate a test * Create the certbot.compat package. Move logic in certbot.compat.misc * Clean comment * Add doc * Fix lint * Correct mypy * Add executable permissions * Add the delegate certbot.compat.os module, add check coding style to enforce usage of certbot.compat.os instead of standard os * Load certbot.compat.os instead of os * Move existing compat test * Update local oldest requirements * Import sys * Fix some mocks * Update account_test.py * Update os.py * Update os.py * Update local oldest requirements * Implement the new linter_plugin * Fix remaining linting errors * Fix local oldest for nginx * Remove custom check in favor of pylint plugin * Remove check coding style * Update linter_plugin.py Co-Authored-By: adferrand * Add several comments * Update the setup.py * Add documentation * Update acme dependencies * Update certbot/compat/os.py Co-Authored-By: adferrand * Update certbot/compat/os.py Co-Authored-By: adferrand * Update certbot/compat/os.py Co-Authored-By: adferrand * Update docs/contributing.rst Co-Authored-By: adferrand * Update linter_plugin.py Co-Authored-By: adferrand * Update linter_plugin.py Co-Authored-By: adferrand * Update docs/contributing.rst Co-Authored-By: adferrand * Update docs/contributing.rst Co-Authored-By: adferrand * Corrections * Handle os.path. Simplify checker. * Add a comment to a reference implementation * Update changelog * Fix module registering * Update docs/contributing.rst Co-Authored-By: adferrand * Update docs/contributing.rst Co-Authored-By: adferrand * Update docs/contributing.rst Co-Authored-By: adferrand * Update config and changelog * Correction * Correct os * Fix merge * Disable pylint checks * Normalize imports * Simplify security * Corrections * Reorganize module * Clean code * Clean code * Remove coverage * No cover * Implement security.chmod * Disable a test for now * Disable hard error for now * Add a first test. Remove unused import * Recalibrate coverage * Modifications for misc * Correct function call * Add some types * Remove newline * Use os_rename * Implement security.open * Revert to windows-files-permissions approach * Fix lint * Implement security.mkdir and security.makedirs * Fix lint * Clean lint * Clean lint * Revert "Clean lint" This reverts commit 83bf81960ac6bf3f76c286ca065a5ac850c6870b. * Correct mock * Conditionally add pywin32 on setuptools versions that support environment markers. * Fix separator * Fix separator * Rename security into filesystem * Change module security to filesystem * Move rename into filesystem * Rename security into filesystem * Rename security into filesystem * Rerun CI * Fix import * Fix pylint * Implement copy_ownership_and_apply_mode * Fix pylint * Update certbot/compat/os.py Co-Authored-By: Brad Warren * Remove default values * Rewrite a comment. * Relaunch CI * Pass as keyword arguments * Update certbot/compat/filesystem.py Co-Authored-By: Brad Warren * Update certbot/compat/filesystem.py Co-Authored-By: Brad Warren * Update certbot/compat/filesystem.py Co-Authored-By: Brad Warren * Make the private key permissions transfer platform specific * Update certbot/compat/filesystem.py Co-Authored-By: Brad Warren * Rename variable * Fix comment0 * Add unit test for copy_ownership_and_apply_mode * Adapt coverage * Implement new methods. * Remove the old method * Reimplement make_or_verify_dir * Finish migration * Start to fix tests * Fix ownership when creating a file with filesystem.open * Fix security on TempHandler * Fix validation path permissions * Fix owner on mkdir * Use a proper workdir for crypto tests * Fix pylint * Adapt coverage * Update storage_test.py * Update util_test.py * Clean code * Update certbot/compat/filesystem.py Co-Authored-By: ohemorange * Add comment * Update certbot/compat/filesystem.py Co-Authored-By: ohemorange * Check permissions * Change test mode * Add unit test for filesystem.check_* functions * Update filesystem_test.py * Better logic for TempHandler * Adapt coverage --- .codecov.yml | 4 +- certbot-nginx/certbot_nginx/configurator.py | 11 +-- certbot/account.py | 7 +- certbot/cert_manager.py | 5 +- certbot/client.py | 5 +- certbot/compat/filesystem.py | 75 +++++++++++++++++++++ certbot/compat/misc.py | 26 ------- certbot/crypto_util.py | 7 +- certbot/log.py | 12 ++-- certbot/main.py | 10 +-- certbot/plugins/webroot.py | 3 +- certbot/plugins/webroot_test.py | 5 +- certbot/reverter.py | 10 +-- certbot/tests/cert_manager_test.py | 30 +++------ certbot/tests/compat/filesystem_test.py | 49 +++++++++++++- certbot/tests/crypto_util_test.py | 10 ++- certbot/tests/log_test.py | 5 +- certbot/tests/main_test.py | 10 ++- certbot/tests/storage_test.py | 19 ++---- certbot/tests/util_test.py | 52 ++------------ certbot/util.py | 30 ++------- 21 files changed, 194 insertions(+), 191 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 86813de6a..f4d4d1d6c 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -6,13 +6,13 @@ coverage: flags: linux # Fixed target instead of auto set by #7173, can # be removed when flags in Codecov are added back. - target: 97.6 + target: 97.5 threshold: 0.1 base: auto windows: flags: windows # Fixed target instead of auto set by #7173, can # be removed when flags in Codecov are added back. - target: 97.0 + target: 97.2 threshold: 0.1 base: auto diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index e078ad4cb..0c6eabf57 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -20,7 +20,6 @@ from certbot import crypto_util from certbot import errors from certbot import interfaces from certbot import util -from certbot.compat import misc from certbot.compat import os from certbot.plugins import common @@ -903,13 +902,9 @@ class NginxConfigurator(common.Installer): have permissions of root. """ - uid = misc.os_geteuid() - util.make_or_verify_dir( - self.config.work_dir, core_constants.CONFIG_DIRS_MODE, uid) - util.make_or_verify_dir( - self.config.backup_dir, core_constants.CONFIG_DIRS_MODE, uid) - util.make_or_verify_dir( - self.config.config_dir, core_constants.CONFIG_DIRS_MODE, uid) + util.make_or_verify_dir(self.config.work_dir, core_constants.CONFIG_DIRS_MODE) + util.make_or_verify_dir(self.config.backup_dir, core_constants.CONFIG_DIRS_MODE) + util.make_or_verify_dir(self.config.config_dir, core_constants.CONFIG_DIRS_MODE) def get_version(self): """Return version of Nginx Server. diff --git a/certbot/account.py b/certbot/account.py index bf5c131db..7a1e2de7a 100644 --- a/certbot/account.py +++ b/certbot/account.py @@ -20,7 +20,6 @@ from certbot import constants from certbot import errors from certbot import interfaces from certbot import util -from certbot.compat import misc from certbot.compat import os logger = logging.getLogger(__name__) @@ -139,8 +138,7 @@ class AccountFileStorage(interfaces.AccountStorage): """ def __init__(self, config): self.config = config - util.make_or_verify_dir(config.accounts_dir, 0o700, misc.os_geteuid(), - self.config.strict_permissions) + util.make_or_verify_dir(config.accounts_dir, 0o700, self.config.strict_permissions) def _account_dir_path(self, account_id): return self._account_dir_path_for_server_path(account_id, self.config.server_path) @@ -322,8 +320,7 @@ class AccountFileStorage(interfaces.AccountStorage): def _save(self, account, acme, regr_only): account_dir_path = self._account_dir_path(account.id) - util.make_or_verify_dir(account_dir_path, 0o700, misc.os_geteuid(), - self.config.strict_permissions) + util.make_or_verify_dir(account_dir_path, 0o700, self.config.strict_permissions) try: with open(self._regr_path(account_dir_path), "w") as regr_file: regr = account.regr diff --git a/certbot/cert_manager.py b/certbot/cert_manager.py index ab929b597..6d6d2e2e6 100644 --- a/certbot/cert_manager.py +++ b/certbot/cert_manager.py @@ -15,7 +15,6 @@ from certbot import interfaces from certbot import ocsp from certbot import storage from certbot import util -from certbot.compat import misc from certbot.compat import os from certbot.display import util as display_util @@ -106,7 +105,7 @@ def lineage_for_certname(cli_config, certname): """Find a lineage object with name certname.""" configs_dir = cli_config.renewal_configs_dir # Verify the directory is there - util.make_or_verify_dir(configs_dir, mode=0o755, uid=misc.os_geteuid()) + util.make_or_verify_dir(configs_dir, mode=0o755) try: renewal_file = storage.renewal_file_for_certname(cli_config, certname) except errors.CertStorageError: @@ -375,7 +374,7 @@ def _search_lineages(cli_config, func, initial_rv, *args): """ configs_dir = cli_config.renewal_configs_dir # Verify the directory is there - util.make_or_verify_dir(configs_dir, mode=0o755, uid=misc.os_geteuid()) + util.make_or_verify_dir(configs_dir, mode=0o755) rv = initial_rv for renewal_file in storage.renewal_conf_files(cli_config): diff --git a/certbot/client.py b/certbot/client.py index 4233a6343..7372d6d9d 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -30,7 +30,6 @@ from certbot import interfaces from certbot import reverter from certbot import storage from certbot import util -from certbot.compat import misc from certbot.compat import os from certbot.display import enhancements from certbot.display import ops as display_ops @@ -459,9 +458,7 @@ class Client(object): """ for path in cert_path, chain_path, fullchain_path: - util.make_or_verify_dir( - os.path.dirname(path), 0o755, misc.os_geteuid(), - self.config.strict_permissions) + util.make_or_verify_dir(os.path.dirname(path), 0o755, self.config.strict_permissions) cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path) diff --git a/certbot/compat/filesystem.py b/certbot/compat/filesystem.py index e2757eb2f..a38fbe760 100644 --- a/certbot/compat/filesystem.py +++ b/certbot/compat/filesystem.py @@ -77,6 +77,54 @@ def copy_ownership_and_apply_mode(src, dst, mode, copy_user, copy_group): chmod(dst, mode) +def check_mode(file_path, mode): + # type: (str, int) -> bool + """ + Check if the given mode matches the permissions of the given file. + On Linux, will make a direct comparison, on Windows, mode will be compared against + the security model. + :param str file_path: Path of the file + :param int mode: POSIX mode to test + :rtype: bool + :return: True if the POSIX mode matches the file permissions + """ + if POSIX_MODE: + return stat.S_IMODE(os.stat(file_path).st_mode) == mode + + return _check_win_mode(file_path, mode) + + +def check_owner(file_path): + # type: (str) -> bool + """ + Check if given file is owned by current user. + :param str file_path: File path to check + :rtype: bool + :return: True if given file is owned by current user, False otherwise. + """ + if POSIX_MODE: + return os.stat(file_path).st_uid == os.getuid() + + # Get owner sid of the file + security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION) + user = security.GetSecurityDescriptorOwner() + + # Compare sids + return _get_current_user() == user + + +def check_permissions(file_path, mode): + # type: (str, int) -> bool + """ + Check if given file has the given mode and is owned by current user. + :param str file_path: File path to check + :param int mode: POSIX mode to check + :rtype: bool + :return: True if file has correct mode and owner, False otherwise. + """ + return check_owner(file_path) and check_mode(file_path, mode) + + def open(file_path, flags, mode=0o777): # pylint: disable=redefined-builtin # type: (str, int, int) -> int """ @@ -107,6 +155,10 @@ def open(file_path, flags, mode=0o777): # pylint: disable=redefined-builtin security = attributes.SECURITY_DESCRIPTOR user = _get_current_user() dacl = _generate_dacl(user, mode) + # We set second parameter to 0 (`False`) to say that this security descriptor is + # NOT constructed from a default mechanism, but is explicitly set by the user. + # See https://docs.microsoft.com/en-us/windows/desktop/api/securitybaseapi/nf-securitybaseapi-setsecuritydescriptorowner # pylint: disable=line-too-long + security.SetSecurityDescriptorOwner(user, 0) # We set first parameter to 1 (`True`) to say that this security descriptor contains # a DACL. Otherwise second and third parameters are ignored. # We set third parameter to 0 (`False`) to say that this security descriptor is @@ -177,6 +229,7 @@ def mkdir(file_path, mode=0o777): security = attributes.SECURITY_DESCRIPTOR user = _get_current_user() dacl = _generate_dacl(user, mode) + security.SetSecurityDescriptorOwner(user, False) security.SetSecurityDescriptorDacl(1, dacl, 0) try: @@ -353,6 +406,28 @@ def _generate_windows_flags(rights_desc): return flag +def _check_win_mode(file_path, mode): + # Resolve symbolic links + file_path = realpath(file_path) + # Get current dacl file + security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION + | win32security.DACL_SECURITY_INFORMATION) + dacl = security.GetSecurityDescriptorDacl() + + # Get current file owner sid + user = security.GetSecurityDescriptorOwner() + + if not dacl: + # No DACL means full control to everyone + # This is not a deterministic permissions set. + return False + + # Calculate the target dacl + ref_dacl = _generate_dacl(user, mode) + + return _compare_dacls(dacl, ref_dacl) + + def _compare_dacls(dacl1, dacl2): """ This method compare the two given DACLs to check if they are identical. diff --git a/certbot/compat/misc.py b/certbot/compat/misc.py index c840c333c..693fceefb 100644 --- a/certbot/compat/misc.py +++ b/certbot/compat/misc.py @@ -42,22 +42,6 @@ def raise_for_non_administrative_windows_rights(): raise errors.Error('Error, certbot must be run on a shell with administrative rights.') -def os_geteuid(): - """ - Get current user uid - - :returns: The current user uid. - :rtype: int - - """ - try: - # Linux specific - return os.geteuid() - except AttributeError: - # Windows specific - return 0 - - def readline_with_timeout(timeout, prompt): # type: (float, str) -> str """ @@ -88,16 +72,6 @@ def readline_with_timeout(timeout, prompt): return sys.stdin.readline() -def compare_file_modes(mode1, mode2): - """Return true if the two modes can be considered as equals for this platform""" - if os.name != 'nt': - # Linux specific: standard compare - return oct(stat.S_IMODE(mode1)) == oct(stat.S_IMODE(mode2)) - # Windows specific: most of mode bits are ignored on Windows. Only check user R/W rights. - return (stat.S_IMODE(mode1) & stat.S_IREAD == stat.S_IMODE(mode2) & stat.S_IREAD - and stat.S_IMODE(mode1) & stat.S_IWRITE == stat.S_IMODE(mode2) & stat.S_IWRITE) - - WINDOWS_DEFAULT_FOLDERS = { 'config': 'C:\\Certbot', 'work': 'C:\\Certbot\\lib', diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 281a76668..12291af38 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -28,7 +28,6 @@ from acme.magic_typing import IO # pylint: disable=unused-import, no-name-in-mo from certbot import errors from certbot import interfaces from certbot import util -from certbot.compat import misc from certbot.compat import os logger = logging.getLogger(__name__) @@ -61,8 +60,7 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): config = zope.component.getUtility(interfaces.IConfig) # Save file - util.make_or_verify_dir(key_dir, 0o700, misc.os_geteuid(), - config.strict_permissions) + util.make_or_verify_dir(key_dir, 0o700, config.strict_permissions) key_f, key_path = util.unique_file( os.path.join(key_dir, keyname), 0o600, "wb") with key_f: @@ -92,8 +90,7 @@ def init_save_csr(privkey, names, path): privkey.pem, names, must_staple=config.must_staple) # Save CSR - util.make_or_verify_dir(path, 0o755, misc.os_geteuid(), - config.strict_permissions) + util.make_or_verify_dir(path, 0o755, config.strict_permissions) csr_f, csr_filename = util.unique_file( os.path.join(path, "csr-certbot.pem"), 0o644, "wb") with csr_f: diff --git a/certbot/log.py b/certbot/log.py index bf444de07..a16e2ef7e 100644 --- a/certbot/log.py +++ b/certbot/log.py @@ -17,6 +17,7 @@ from __future__ import print_function import functools import logging import logging.handlers +import shutil import sys import tempfile import traceback @@ -26,7 +27,6 @@ from acme import messages from certbot import constants from certbot import errors from certbot import util -from certbot.compat import misc from certbot.compat import os # Logging format @@ -134,8 +134,7 @@ def setup_log_file_handler(config, logfile, fmt): """ # TODO: logs might contain sensitive data such as contents of the # private key! #525 - util.set_up_core_dir( - config.logs_dir, 0o700, misc.os_geteuid(), config.strict_permissions) + util.set_up_core_dir(config.logs_dir, 0o700, config.strict_permissions) log_file_path = os.path.join(config.logs_dir, logfile) try: handler = logging.handlers.RotatingFileHandler( @@ -240,9 +239,10 @@ class TempHandler(logging.StreamHandler): """ def __init__(self): - stream = tempfile.NamedTemporaryFile('w', delete=False) + self._workdir = tempfile.mkdtemp() + self.path = os.path.join(self._workdir, 'log') + stream = util.safe_open(self.path, mode='w', chmod=0o600) super(TempHandler, self).__init__(stream) - self.path = stream.name self._delete = True def emit(self, record): @@ -266,7 +266,7 @@ class TempHandler(logging.StreamHandler): # stream like stderr to be used self.stream.close() if self._delete: - os.remove(self.path) + shutil.rmtree(self._workdir) self._delete = False super(TempHandler, self).close() finally: diff --git a/certbot/main.py b/certbot/main.py index 50470438f..fc91aca5f 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -1299,18 +1299,14 @@ def make_or_verify_needed_dirs(config): :rtype: None """ - util.set_up_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE, - misc.os_geteuid(), config.strict_permissions) - util.set_up_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE, - misc.os_geteuid(), config.strict_permissions) + util.set_up_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE, config.strict_permissions) + util.set_up_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE, config.strict_permissions) hook_dirs = (config.renewal_pre_hooks_dir, config.renewal_deploy_hooks_dir, config.renewal_post_hooks_dir,) for hook_dir in hook_dirs: - util.make_or_verify_dir(hook_dir, - uid=misc.os_geteuid(), - strict=config.strict_permissions) + util.make_or_verify_dir(hook_dir, strict=config.strict_permissions) def set_displayer(config): diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index f767bdf54..33e6530a1 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -24,6 +24,7 @@ from certbot.display import ops from certbot.display import util as display_util from certbot.plugins import common from certbot.plugins import util +from certbot.util import safe_open logger = logging.getLogger(__name__) @@ -207,7 +208,7 @@ to serve all files under specified web root ({0}).""" old_umask = os.umask(0o022) try: - with open(validation_path, "wb") as validation_file: + with safe_open(validation_path, mode="wb", chmod=0o644) as validation_file: validation_file.write(validation.encode()) finally: os.umask(old_umask) diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py index ba9950fae..a0b701cac 100644 --- a/certbot/plugins/webroot_test.py +++ b/certbot/plugins/webroot_test.py @@ -17,7 +17,6 @@ from acme import challenges from certbot import achallenges from certbot import errors -from certbot.compat import misc from certbot.compat import os from certbot.compat import filesystem from certbot.display import util as display_util @@ -168,14 +167,14 @@ class AuthenticatorTest(unittest.TestCase): # Remove exec bit from permission check, so that it # matches the file self.auth.perform([self.achall]) - self.assertTrue(misc.compare_file_modes(os.stat(self.validation_path).st_mode, 0o644)) + self.assertTrue(filesystem.check_mode(self.validation_path, 0o644)) # Check permissions of the directories for dirpath, dirnames, _ in os.walk(self.path): for directory in dirnames: full_path = os.path.join(dirpath, directory) - self.assertTrue(misc.compare_file_modes(os.stat(full_path).st_mode, 0o755)) + self.assertTrue(filesystem.check_mode(full_path, 0o755)) parent_gid = os.stat(self.path).st_gid parent_uid = os.stat(self.path).st_uid diff --git a/certbot/reverter.py b/certbot/reverter.py index 21d33806a..f4f1c8c67 100644 --- a/certbot/reverter.py +++ b/certbot/reverter.py @@ -15,7 +15,6 @@ from certbot import constants from certbot import errors from certbot import interfaces from certbot import util -from certbot.compat import misc from certbot.compat import os from certbot.compat import filesystem @@ -68,8 +67,7 @@ class Reverter(object): self.config = config util.make_or_verify_dir( - config.backup_dir, constants.CONFIG_DIRS_MODE, misc.os_geteuid(), - self.config.strict_permissions) + config.backup_dir, constants.CONFIG_DIRS_MODE, self.config.strict_permissions) def revert_temporary_config(self): """Reload users original configuration files after a temporary save. @@ -225,8 +223,7 @@ class Reverter(object): """ util.make_or_verify_dir( - cp_dir, constants.CONFIG_DIRS_MODE, misc.os_geteuid(), - self.config.strict_permissions) + cp_dir, constants.CONFIG_DIRS_MODE, self.config.strict_permissions) op_fd, existing_filepaths = self._read_and_append( os.path.join(cp_dir, "FILEPATHS")) @@ -445,8 +442,7 @@ class Reverter(object): cp_dir = self.config.in_progress_dir util.make_or_verify_dir( - cp_dir, constants.CONFIG_DIRS_MODE, misc.os_geteuid(), - self.config.strict_permissions) + cp_dir, constants.CONFIG_DIRS_MODE, self.config.strict_permissions) return cp_dir diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index b4cd946ee..83946a0f7 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -277,13 +277,12 @@ class SearchLineagesTest(BaseCertManagerTest): @mock.patch('certbot.storage.renewal_conf_files') @mock.patch('certbot.storage.RenewableCert') def test_cert_storage_error(self, mock_renewable_cert, mock_renewal_conf_files, - mock_make_or_verify_dir): + mock_make_or_verify_dir): mock_renewal_conf_files.return_value = ["badfile"] mock_renewable_cert.side_effect = errors.CertStorageError from certbot import cert_manager # pylint: disable=protected-access - self.assertEqual(cert_manager._search_lineages(self.config, lambda x: x, "check"), - "check") + self.assertEqual(cert_manager._search_lineages(self.config, lambda x: x, "check"), "check") self.assertTrue(mock_make_or_verify_dir.called) @@ -294,33 +293,28 @@ class LineageForCertnameTest(BaseCertManagerTest): @mock.patch('certbot.storage.renewal_file_for_certname') @mock.patch('certbot.storage.RenewableCert') def test_found_match(self, mock_renewable_cert, mock_renewal_conf_file, - mock_make_or_verify_dir): + mock_make_or_verify_dir): mock_renewal_conf_file.return_value = "somefile.conf" mock_match = mock.Mock(lineagename="example.com") mock_renewable_cert.return_value = mock_match from certbot import cert_manager - self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), - mock_match) + self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), mock_match) self.assertTrue(mock_make_or_verify_dir.called) @mock.patch('certbot.util.make_or_verify_dir') @mock.patch('certbot.storage.renewal_file_for_certname') - def test_no_match(self, mock_renewal_conf_file, - mock_make_or_verify_dir): + def test_no_match(self, mock_renewal_conf_file, mock_make_or_verify_dir): mock_renewal_conf_file.return_value = "other.com.conf" from certbot import cert_manager - self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), - None) + self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), None) self.assertTrue(mock_make_or_verify_dir.called) @mock.patch('certbot.util.make_or_verify_dir') @mock.patch('certbot.storage.renewal_file_for_certname') - def test_no_renewal_file(self, mock_renewal_conf_file, - mock_make_or_verify_dir): + def test_no_renewal_file(self, mock_renewal_conf_file, mock_make_or_verify_dir): mock_renewal_conf_file.side_effect = errors.CertStorageError() from certbot import cert_manager - self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), - None) + self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), None) self.assertTrue(mock_make_or_verify_dir.called) @@ -331,7 +325,7 @@ class DomainsForCertnameTest(BaseCertManagerTest): @mock.patch('certbot.storage.renewal_file_for_certname') @mock.patch('certbot.storage.RenewableCert') def test_found_match(self, mock_renewable_cert, mock_renewal_conf_file, - mock_make_or_verify_dir): + mock_make_or_verify_dir): mock_renewal_conf_file.return_value = "somefile.conf" mock_match = mock.Mock(lineagename="example.com") domains = ["example.com", "example.org"] @@ -344,12 +338,10 @@ class DomainsForCertnameTest(BaseCertManagerTest): @mock.patch('certbot.util.make_or_verify_dir') @mock.patch('certbot.storage.renewal_file_for_certname') - def test_no_match(self, mock_renewal_conf_file, - mock_make_or_verify_dir): + def test_no_match(self, mock_renewal_conf_file, mock_make_or_verify_dir): mock_renewal_conf_file.return_value = "somefile.conf" from certbot import cert_manager - self.assertEqual(cert_manager.domains_for_certname(self.config, "other.com"), - None) + self.assertEqual(cert_manager.domains_for_certname(self.config, "other.com"), None) self.assertTrue(mock_make_or_verify_dir.called) diff --git a/certbot/tests/compat/filesystem_test.py b/certbot/tests/compat/filesystem_test.py index 48ac04670..90ecd13d7 100644 --- a/certbot/tests/compat/filesystem_test.py +++ b/certbot/tests/compat/filesystem_test.py @@ -16,6 +16,7 @@ except ImportError: import certbot.tests.util as test_util from certbot import lock +from certbot import util from certbot.compat import os from certbot.compat import filesystem from certbot.tests.util import TempDirTestCase @@ -306,6 +307,52 @@ class CopyOwnershipTest(test_util.TempDirTestCase): mock_chmod.assert_called_once_with(self.probe_path, 0o700) +class CheckPermissionsTest(test_util.TempDirTestCase): + def setUp(self): + super(CheckPermissionsTest, self).setUp() + self.probe_path = _create_probe(self.tempdir) + + def test_check_mode(self): + self.assertTrue(filesystem.check_mode(self.probe_path, 0o744)) + + filesystem.chmod(self.probe_path, 0o700) + self.assertFalse(filesystem.check_mode(self.probe_path, 0o744)) + + @unittest.skipIf(POSIX_MODE, reason='Test specific to Windows security') + def test_check_owner_windows(self): + self.assertTrue(filesystem.check_owner(self.probe_path)) + + system = win32security.ConvertStringSidToSid(SYSTEM_SID) + security = win32security.SECURITY_ATTRIBUTES().SECURITY_DESCRIPTOR + security.SetSecurityDescriptorOwner(system, False) + + with mock.patch('win32security.GetFileSecurity') as mock_get: + mock_get.return_value = security + self.assertFalse(filesystem.check_owner(self.probe_path)) + + @unittest.skipUnless(POSIX_MODE, reason='Test specific to Linux security') + def test_check_owner_linux(self): + self.assertTrue(filesystem.check_owner(self.probe_path)) + + import os as std_os # pylint: disable=os-module-forbidden + uid = std_os.getuid() + + with mock.patch('os.getuid') as mock_uid: + mock_uid.return_value = uid + 1 + self.assertFalse(filesystem.check_owner(self.probe_path)) + + def test_check_permissions(self): + self.assertTrue(filesystem.check_permissions(self.probe_path, 0o744)) + + with mock.patch('certbot.compat.filesystem.check_mode') as mock_mode: + mock_mode.return_value = False + self.assertFalse(filesystem.check_permissions(self.probe_path, 0o744)) + + with mock.patch('certbot.compat.filesystem.check_owner') as mock_owner: + mock_owner.return_value = False + self.assertFalse(filesystem.check_permissions(self.probe_path, 0o744)) + + class OsReplaceTest(test_util.TempDirTestCase): """Test to ensure consistent behavior of rename method""" def test_os_replace_to_existing_file(self): @@ -381,7 +428,7 @@ def _set_owner(target, security_owner, user): def _create_probe(tempdir): filesystem.chmod(tempdir, 0o744) probe_path = os.path.join(tempdir, 'probe') - open(probe_path, 'w').close() + util.safe_open(probe_path, 'w', chmod=0o744).close() return probe_path diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index 5c1a07f8d..2673f4219 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -11,6 +11,7 @@ from certbot import errors from certbot import interfaces from certbot import util from certbot.compat import os +from certbot.compat import filesystem RSA256_KEY = test_util.load_vector('rsa256_key.pem') RSA256_KEY_PATH = test_util.vector_path('rsa256_key.pem') @@ -29,6 +30,9 @@ class InitSaveKeyTest(test_util.TempDirTestCase): def setUp(self): super(InitSaveKeyTest, self).setUp() + self.workdir = os.path.join(self.tempdir, 'workdir') + filesystem.mkdir(self.workdir, mode=0o700) + logging.disable(logging.CRITICAL) zope.component.provideUtility( mock.Mock(strict_permissions=True), interfaces.IConfig) @@ -46,15 +50,15 @@ class InitSaveKeyTest(test_util.TempDirTestCase): @mock.patch('certbot.crypto_util.make_key') def test_success(self, mock_make): mock_make.return_value = b'key_pem' - key = self._call(1024, self.tempdir) + key = self._call(1024, self.workdir) self.assertEqual(key.pem, b'key_pem') self.assertTrue('key-certbot.pem' in key.file) - self.assertTrue(os.path.exists(os.path.join(self.tempdir, key.file))) + self.assertTrue(os.path.exists(os.path.join(self.workdir, key.file))) @mock.patch('certbot.crypto_util.make_key') def test_key_failure(self, mock_make): mock_make.side_effect = ValueError - self.assertRaises(ValueError, self._call, 431, self.tempdir) + self.assertRaises(ValueError, self._call, 431, self.workdir) class InitSaveCSRTest(test_util.TempDirTestCase): diff --git a/certbot/tests/log_test.py b/certbot/tests/log_test.py index d203635a2..807c7ecaa 100644 --- a/certbot/tests/log_test.py +++ b/certbot/tests/log_test.py @@ -14,7 +14,7 @@ from acme.magic_typing import Optional # pylint: disable=unused-import, no-name from certbot import constants from certbot import errors from certbot import util -from certbot.compat import misc +from certbot.compat import filesystem from certbot.compat import os from certbot.tests import util as test_util @@ -260,8 +260,7 @@ class TempHandlerTest(unittest.TestCase): self.handler.close() def test_permissions(self): - self.assertTrue( - util.check_permissions(self.handler.path, 0o600, misc.os_geteuid())) + self.assertTrue(filesystem.check_permissions(self.handler.path, 0o600)) def test_delete(self): self.handler.close() diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index ff8a800ab..b7477e355 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -31,7 +31,6 @@ from certbot import interfaces # pylint: disable=unused-import from certbot import main from certbot import updater from certbot import util -from certbot.compat import misc from certbot.compat import os from certbot.compat import filesystem from certbot.plugins import disco @@ -809,9 +808,9 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met ifaces = [] # type: List[interfaces.IPlugin] plugins = mock_disco.PluginsRegistry.find_all() - def throw_error(directory, mode, uid, strict): + def throw_error(directory, mode, strict): """Raises error.Error.""" - _, _, _, _ = directory, mode, uid, strict + _, _, _ = directory, mode, strict raise errors.Error() stdout = six.StringIO() @@ -1593,7 +1592,7 @@ class MakeOrVerifyNeededDirs(test_util.ConfigTestCase): for core_dir in (self.config.config_dir, self.config.work_dir,): mock_util.set_up_core_dir.assert_any_call( core_dir, constants.CONFIG_DIRS_MODE, - misc.os_geteuid(), self.config.strict_permissions + self.config.strict_permissions ) hook_dirs = (self.config.renewal_pre_hooks_dir, @@ -1602,8 +1601,7 @@ class MakeOrVerifyNeededDirs(test_util.ConfigTestCase): for hook_dir in hook_dirs: # default mode of 755 is used mock_util.make_or_verify_dir.assert_any_call( - hook_dir, uid=misc.os_geteuid(), - strict=self.config.strict_permissions) + hook_dir, strict=self.config.strict_permissions) class EnhanceTest(test_util.ConfigTestCase): diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index fceef804a..2bc45873c 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -13,7 +13,6 @@ import six import certbot import certbot.tests.util as test_util from certbot import errors -from certbot.compat import misc from certbot.compat import os from certbot.compat import filesystem from certbot.storage import ALL_FOUR @@ -21,7 +20,6 @@ from certbot.storage import ALL_FOUR CERT = test_util.load_cert('cert_512.pem') - def unlink_all(rc_object): """Unlink all four items associated with this RenewableCert.""" for kind in ALL_FOUR: @@ -498,7 +496,6 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertTrue(self.test_rc.should_autorenew()) mock_ocsp.return_value = False - @test_util.broken_on_windows @mock.patch("certbot.storage.relevant_values") def test_save_successor(self, mock_rv): # Mock relevant_values() to claim that all values are relevant here @@ -562,7 +559,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.exists(temp_config_file)) - @test_util.broken_on_windows + @test_util.skip_on_windows('Group/everybody permissions are not maintained on Windows.') @mock.patch("certbot.storage.relevant_values") def test_save_successor_maintains_group_mode(self, mock_rv): # Mock relevant_values() to claim that all values are relevant here @@ -571,22 +568,18 @@ class RenewableCertTests(BaseRenewableCertTest): for kind in ALL_FOUR: self._write_out_kind(kind, 1) self.test_rc.update_all_links_to(1) - self.assertTrue(misc.compare_file_modes( - os.stat(self.test_rc.version("privkey", 1)).st_mode, 0o600)) + self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 1), 0o600)) filesystem.chmod(self.test_rc.version("privkey", 1), 0o444) # If no new key, permissions should be the same (we didn't write any keys) self.test_rc.save_successor(1, b"newcert", None, b"new chain", self.config) - self.assertTrue(misc.compare_file_modes( - os.stat(self.test_rc.version("privkey", 2)).st_mode, 0o444)) + self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 2), 0o444)) # If new key, permissions should be kept as 644 self.test_rc.save_successor(2, b"newcert", b"new_privkey", b"new chain", self.config) - self.assertTrue(misc.compare_file_modes( - os.stat(self.test_rc.version("privkey", 3)).st_mode, 0o644)) + self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 3), 0o644)) # If permissions reverted, next renewal will also revert permissions of new key filesystem.chmod(self.test_rc.version("privkey", 3), 0o400) self.test_rc.save_successor(3, b"newcert", b"new_privkey", b"new chain", self.config) - self.assertTrue(misc.compare_file_modes( - os.stat(self.test_rc.version("privkey", 4)).st_mode, 0o600)) + self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 4), 0o600)) @mock.patch("certbot.storage.relevant_values") @mock.patch("certbot.storage.filesystem.copy_ownership_and_apply_mode") @@ -622,7 +615,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.config.live_dir, "README"))) self.assertTrue(os.path.exists(os.path.join( self.config.live_dir, "the-lineage.com", "README"))) - self.assertTrue(misc.compare_file_modes(os.stat(result.key_path).st_mode, 0o600)) + self.assertTrue(filesystem.check_mode(result.key_path, 0o600)) with open(result.fullchain, "rb") as f: self.assertEqual(f.read(), b"cert" + b"chain") # Let's do it again and make sure it makes a different lineage diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index b96f8afd2..e65de3349 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -9,7 +9,6 @@ from six.moves import reload_module # pylint: disable=import-error import certbot.tests.util as test_util from certbot import errors -from certbot.compat import misc from certbot.compat import os from certbot.compat import filesystem @@ -120,15 +119,14 @@ class SetUpCoreDirTest(test_util.TempDirTestCase): @mock.patch('certbot.util.lock_dir_until_exit') def test_success(self, mock_lock): new_dir = os.path.join(self.tempdir, 'new') - self._call(new_dir, 0o700, misc.os_geteuid(), False) + self._call(new_dir, 0o700, False) self.assertTrue(os.path.exists(new_dir)) self.assertEqual(mock_lock.call_count, 1) @mock.patch('certbot.util.make_or_verify_dir') def test_failure(self, mock_make_or_verify): mock_make_or_verify.side_effect = OSError - self.assertRaises(errors.Error, self._call, - self.tempdir, 0o700, misc.os_geteuid(), False) + self.assertRaises(errors.Error, self._call, self.tempdir, 0o700, False) class MakeOrVerifyDirTest(test_util.TempDirTestCase): @@ -145,23 +143,20 @@ class MakeOrVerifyDirTest(test_util.TempDirTestCase): self.path = os.path.join(self.tempdir, "foo") filesystem.mkdir(self.path, 0o600) - self.uid = misc.os_geteuid() - def _call(self, directory, mode): from certbot.util import make_or_verify_dir - return make_or_verify_dir(directory, mode, self.uid, strict=True) + return make_or_verify_dir(directory, mode, strict=True) def test_creates_dir_when_missing(self): path = os.path.join(self.tempdir, "bar") self._call(path, 0o650) self.assertTrue(os.path.isdir(path)) - self.assertTrue(misc.compare_file_modes(os.stat(path).st_mode, 0o650)) + self.assertTrue(filesystem.check_mode(path, 0o650)) def test_existing_correct_mode_does_not_fail(self): self._call(self.path, 0o600) - self.assertTrue(misc.compare_file_modes(os.stat(self.path).st_mode, 0o600)) + self.assertTrue(filesystem.check_mode(self.path, 0o600)) - @test_util.skip_on_windows('Umask modes are mostly ignored on Windows.') def test_existing_wrong_mode_fails(self): self.assertRaises(errors.Error, self._call, self.path, 0o400) @@ -171,39 +166,6 @@ class MakeOrVerifyDirTest(test_util.TempDirTestCase): self.assertRaises(OSError, self._call, "bar", 12312312) -class CheckPermissionsTest(test_util.TempDirTestCase): - """Tests for certbot.util.check_permissions. - - Note that it is not possible to test for a wrong file owner, - as this testing script would have to be run as root. - - """ - - def setUp(self): - super(CheckPermissionsTest, self).setUp() - - self.uid = misc.os_geteuid() - - def _call(self, mode): - from certbot.util import check_permissions - return check_permissions(self.tempdir, mode, self.uid) - - def test_ok_mode(self): - filesystem.chmod(self.tempdir, 0o600) - self.assertTrue(self._call(0o600)) - - # TODO: reactivate the test when all logic from windows file permissions is merged. - @test_util.broken_on_windows - def test_wrong_mode(self): - filesystem.chmod(self.tempdir, 0o400) - try: - self.assertFalse(self._call(0o600)) - finally: - # Without proper write permissions, Windows is unable to delete a folder, - # even with admin permissions. Write access must be explicitly set first. - filesystem.chmod(self.tempdir, 0o700) - - class UniqueFileTest(test_util.TempDirTestCase): """Tests for certbot.util.unique_file.""" @@ -226,8 +188,8 @@ class UniqueFileTest(test_util.TempDirTestCase): def test_right_mode(self): fd1, name1 = self._call(0o700) fd2, name2 = self._call(0o600) - self.assertTrue(misc.compare_file_modes(0o700, os.stat(name1).st_mode)) - self.assertTrue(misc.compare_file_modes(0o600, os.stat(name2).st_mode)) + self.assertTrue(filesystem.check_mode(name1, 0o700)) + self.assertTrue(filesystem.check_mode(name2, 0o600)) fd1.close() fd2.close() diff --git a/certbot/util.py b/certbot/util.py index dfdc87346..8f553e51a 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -21,7 +21,6 @@ from acme.magic_typing import Tuple, Union # pylint: disable=unused-import, no- from certbot import constants from certbot import errors from certbot import lock -from certbot.compat import misc from certbot.compat import os from certbot.compat import filesystem @@ -144,12 +143,11 @@ def _release_locks(): _LOCKS.clear() -def set_up_core_dir(directory, mode, uid, strict): +def set_up_core_dir(directory, mode, strict): """Ensure directory exists with proper permissions and is locked. :param str directory: Path to a directory. :param int mode: Directory mode. - :param int uid: Directory owner. :param bool strict: require directory to be owned by current user :raises .errors.LockError: if the directory cannot be locked @@ -157,19 +155,18 @@ def set_up_core_dir(directory, mode, uid, strict): """ try: - make_or_verify_dir(directory, mode, uid, strict) + make_or_verify_dir(directory, mode, strict) lock_dir_until_exit(directory) except OSError as error: logger.debug("Exception was:", exc_info=True) raise errors.Error(PERM_ERR_FMT.format(error)) -def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False): +def make_or_verify_dir(directory, mode=0o755, strict=False): """Make sure directory exists with proper permissions. :param str directory: Path to a directory. :param int mode: Directory mode. - :param int uid: Directory owner. :param bool strict: require directory to be owned by current user :raises .errors.Error: if a directory already exists, @@ -184,29 +181,14 @@ def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False): filesystem.makedirs(directory, mode) except OSError as exception: if exception.errno == errno.EEXIST: - if strict and not check_permissions(directory, mode, uid): + if strict and not filesystem.check_permissions(directory, mode): raise errors.Error( - "%s exists, but it should be owned by user %d with" - "permissions %s" % (directory, uid, oct(mode))) + "%s exists, but it should be owned by current user with" + " permissions %s" % (directory, oct(mode))) else: raise -def check_permissions(filepath, mode, uid=0): - """Check file or directory permissions. - - :param str filepath: Path to the tested file (or directory). - :param int mode: Expected file mode. - :param int uid: Expected file owner. - - :returns: True if `mode` and `uid` match, False otherwise. - :rtype: bool - - """ - file_stat = os.stat(filepath) - return misc.compare_file_modes(file_stat.st_mode, mode) and file_stat.st_uid == uid - - def safe_open(path, mode="w", chmod=None): """Safely open a file. From d3da19919fe8c8a86bd1b1580f8142e35b37fe3d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 26 Jul 2019 13:37:16 -0700 Subject: [PATCH 20/26] Remove duplicate, failing oldest tests. (#7272) Nightly tests failed last night at https://travis-ci.com/certbot/certbot/builds/120816454. The cause was the oldest the version of Ubuntu used in the tests suddenly changed from Trusty to Xenial. You can see Xenial being used in the failing test at https://travis-ci.com/certbot/certbot/jobs/219873088#L9 and Trusty being used at the last passing test at https://travis-ci.com/certbot/certbot/jobs/218936290#L9. The change in the default doesn't seem to be documented (yet) at https://docs.travis-ci.com/user/reference/overview/. I started to pin Trusty in these tests, however, I noticed that we are running these same unit tests at https://github.com/certbot/certbot/blob/e6bf3fe7f81ff7651b3e8be3d530be725090ed2c/.travis.yml#L58. These other tests are still succeeding because it appears that including `sudo: required` causes Travis to still default to Trusty. Deleting these duplicated tests fixes our Travis failures and speeds things up ever so slightly. * Remove duplicate, failing oldest tests. * pin trusty --- .travis.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index baf385604..94eaf693e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,6 +55,10 @@ matrix: env: TOXENV=mypy <<: *not-on-master - python: "2.7" + # Ubuntu Trusty or older must be used because the oldest version of + # cryptography we support cannot be compiled against the version of + # OpenSSL in Xenial or newer. + dist: trusty env: TOXENV='py27-{acme,apache,certbot,dns,nginx}-oldest' sudo: required services: docker @@ -132,12 +136,6 @@ matrix: sudo: required services: docker <<: *extended-test-suite - - python: "2.7" - env: TOXENV=py27-certbot-oldest - <<: *extended-test-suite - - python: "2.7" - env: TOXENV=py27-nginx-oldest - <<: *extended-test-suite - python: "2.7" env: ACME_SERVER=boulder-v1 TOXENV=integration-certbot-oldest sudo: required From 81e0b92b4383d0f659f608e5e9d2ef9a9db5fb76 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Mon, 29 Jul 2019 19:27:09 +0200 Subject: [PATCH 21/26] Refer to ubuntu in install.rst (#6986) Fixes #5758 --- docs/install.rst | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/docs/install.rst b/docs/install.rst index 3ae7fa397..93a122e80 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -42,7 +42,7 @@ client as root, either `letsencrypt-nosudo The Apache plugin currently requires an OS with augeas version 1.0; currently `it supports `_ -modern OSes based on Debian, Fedora, SUSE, Gentoo and Darwin. +modern OSes based on Debian, Ubuntu, Fedora, SUSE, Gentoo and Darwin. Additional integrity verification of certbot-auto script can be done by verifying its digital signature. @@ -218,6 +218,31 @@ repo, if you have not already done so. Then run: sudo apt-get install certbot python-certbot-apache -t jessie-backports +**Ubuntu** + +If you run Ubuntu Trusty, Xenial, or Bionic, certbot is available through the official PPA, +that can be installed as followed: + +.. code-block:: shell + + sudo apt-get update + sudo apt-get install software-properties-common + sudo add-apt-repository universe + sudo add-apt-repository ppa:certbot/certbot + sudo apt-get update + +Then, certbot can be installed using: + +.. code-block:: shell + + sudo apt-get install certbot + +Optionally to install the Certbot Apache plugin, you can use: + +.. code-block:: shell + + sudo apt-get install python-certbot-apache + **Fedora** .. code-block:: shell From 9174c631d9965834f263ea7ff842d8d2087f47c7 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Mon, 29 Jul 2019 21:54:51 +0200 Subject: [PATCH 22/26] Disable TLS session tickets for Apache 2.4.11+ (#7191) * Implement the logic * Update tests * Fix lint and changelog * Update configurator.py * Move the TLS configs in a dedicated folder. Fix the formalism of their naming and location. * Improve existing test to check all TLS config have their hash registered in Certbot * Corrections after review * Improve a test * Remove commented useless lines in TLS configs * Add a nice warning. Because I am nice. * Fix lint * Add a test --- CHANGELOG.md | 1 + certbot-apache/MANIFEST.in | 1 + certbot-apache/certbot_apache/apache_util.py | 14 ++++++++++ certbot-apache/certbot_apache/configurator.py | 20 ++++++++++---- certbot-apache/certbot_apache/constants.py | 5 ++++ .../certbot_apache/override_arch.py | 4 --- .../certbot_apache/override_centos.py | 15 ++++++++--- .../certbot_apache/override_darwin.py | 4 --- .../certbot_apache/override_debian.py | 3 --- .../certbot_apache/override_fedora.py | 4 --- .../certbot_apache/override_gentoo.py | 4 --- .../certbot_apache/override_suse.py | 4 --- .../certbot_apache/tests/centos_test.py | 7 +++++ .../certbot_apache/tests/configurator_test.py | 27 ++++++++++++++----- .../centos-current-options-ssl-apache.conf} | 8 +----- .../centos-old-options-ssl-apache.conf | 18 +++++++++++++ .../current-options-ssl-apache.conf} | 8 +----- .../tls_configs/old-options-ssl-apache.conf | 19 +++++++++++++ 18 files changed, 114 insertions(+), 52 deletions(-) rename certbot-apache/certbot_apache/{centos-options-ssl-apache.conf => tls_configs/centos-current-options-ssl-apache.conf} (84%) create mode 100644 certbot-apache/certbot_apache/tls_configs/centos-old-options-ssl-apache.conf rename certbot-apache/certbot_apache/{options-ssl-apache.conf => tls_configs/current-options-ssl-apache.conf} (85%) create mode 100644 certbot-apache/certbot_apache/tls_configs/old-options-ssl-apache.conf diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f0da8ce1..0960eb4a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added +* Turn off session tickets for apache plugin by default * acme: Authz deactivation added to `acme` module. ### Changed diff --git a/certbot-apache/MANIFEST.in b/certbot-apache/MANIFEST.in index 3e594a953..a34e215e7 100644 --- a/certbot-apache/MANIFEST.in +++ b/certbot-apache/MANIFEST.in @@ -5,3 +5,4 @@ recursive-include certbot_apache/tests/testdata * include certbot_apache/centos-options-ssl-apache.conf include certbot_apache/options-ssl-apache.conf recursive-include certbot_apache/augeas_lens *.aug +recursive-include certbot_apache/tls_configs *.conf diff --git a/certbot-apache/certbot_apache/apache_util.py b/certbot-apache/certbot_apache/apache_util.py index 7a2ecf49b..f338c0407 100644 --- a/certbot-apache/certbot_apache/apache_util.py +++ b/certbot-apache/certbot_apache/apache_util.py @@ -1,6 +1,8 @@ """ Utility functions for certbot-apache plugin """ import binascii +import pkg_resources + from certbot import util from certbot.compat import os @@ -105,3 +107,15 @@ def parse_define_file(filepath, varname): def unique_id(): """ Returns an unique id to be used as a VirtualHost identifier""" return binascii.hexlify(os.urandom(16)).decode("utf-8") + + +def find_ssl_apache_conf(prefix): + """ + Find a TLS Apache config file in the dedicated storage. + :param str prefix: prefix of the TLS Apache config file to find + :return: the path the TLS Apache config file + :rtype: str + """ + return pkg_resources.resource_filename( + "certbot_apache", + os.path.join("tls_configs", "{0}-options-ssl-apache.conf".format(prefix))) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index f7c27bf76..ecc7c83ab 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -9,7 +9,6 @@ import time from collections import defaultdict -import pkg_resources import six import zope.component @@ -110,14 +109,24 @@ class ApacheConfigurator(common.Installer): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "options-ssl-apache.conf") ) def option(self, key): """Get a value from options""" return self.options.get(key) + def pick_apache_config(self): + """ + Pick the appropriate TLS Apache configuration file for current version of Apache and OS. + :return: the path to the TLS Apache configuration file to use + :rtype: str + """ + # Disabling TLS session tickets is supported by Apache 2.4.11+. + # So for old versions of Apache we pick a configuration without this option. + if self.version < (2, 4, 11): + return apache_util.find_ssl_apache_conf("old") + return apache_util.find_ssl_apache_conf("current") + def _prepare_options(self): """ Set the values possibly changed by command line parameters to @@ -2339,8 +2348,9 @@ class ApacheConfigurator(common.Installer): # XXX if we ever try to enforce a local privilege boundary (eg, running # certbot for unprivileged users via setuid), this function will need # to be modified. - return common.install_version_controlled_file(options_ssl, options_ssl_digest, - self.option("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES) + apache_config_path = self.pick_apache_config() + return common.install_version_controlled_file( + options_ssl, options_ssl_digest, apache_config_path, constants.ALL_SSL_OPTIONS_HASHES) def enable_autohsts(self, _unused_lineage, domains): """ diff --git a/certbot-apache/certbot_apache/constants.py b/certbot-apache/certbot_apache/constants.py index 23a7b7afd..f8184a42f 100644 --- a/certbot-apache/certbot_apache/constants.py +++ b/certbot-apache/certbot_apache/constants.py @@ -9,6 +9,7 @@ MOD_SSL_CONF_DEST = "options-ssl-apache.conf" UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-apache-conf-digest.txt" """Name of the hash of the updated or informed mod_ssl_conf as saved in `IConfig.config_dir`.""" +# NEVER REMOVE A SINGLE HASH FROM THIS LIST UNLESS YOU KNOW EXACTLY WHAT YOU ARE DOING! ALL_SSL_OPTIONS_HASHES = [ '2086bca02db48daf93468332543c60ac6acdb6f0b58c7bfdf578a5d47092f82a', '4844d36c9a0f587172d9fa10f4f1c9518e3bcfa1947379f155e16a70a728c21a', @@ -18,6 +19,10 @@ ALL_SSL_OPTIONS_HASHES = [ 'cfdd7c18d2025836ea3307399f509cfb1ebf2612c87dd600a65da2a8e2f2797b', '80720bd171ccdc2e6b917ded340defae66919e4624962396b992b7218a561791', 'c0c022ea6b8a51ecc8f1003d0a04af6c3f2bc1c3ce506b3c2dfc1f11ef931082', + '717b0a89f5e4c39b09a42813ac6e747cfbdeb93439499e73f4f70a1fe1473f20', + '0fcdc81280cd179a07ec4d29d3595068b9326b455c488de4b09f585d5dafc137', + '86cc09ad5415cd6d5f09a947fe2501a9344328b1e8a8b458107ea903e80baa6c', + '06675349e457eae856120cdebb564efe546f0b87399f2264baeb41e442c724c7', ] """SHA256 hashes of the contents of previous versions of all versions of MOD_SSL_CONF_SRC""" diff --git a/certbot-apache/certbot_apache/override_arch.py b/certbot-apache/certbot_apache/override_arch.py index c5620e9f9..02891548d 100644 --- a/certbot-apache/certbot_apache/override_arch.py +++ b/certbot-apache/certbot_apache/override_arch.py @@ -1,6 +1,4 @@ """ Distribution specific override class for Arch Linux """ -import pkg_resources - import zope.interface from certbot import interfaces @@ -26,6 +24,4 @@ class ArchConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/httpd/conf", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "options-ssl-apache.conf") ) diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py index 7c7492dbf..d4a7d7137 100644 --- a/certbot-apache/certbot_apache/override_centos.py +++ b/certbot-apache/certbot_apache/override_centos.py @@ -1,7 +1,6 @@ """ Distribution specific override class for CentOS family (RHEL, Fedora) """ import logging -import pkg_resources import zope.interface from certbot import errors @@ -39,8 +38,6 @@ class CentOSConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/httpd/conf.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "centos-options-ssl-apache.conf") ) def config_test(self): @@ -75,6 +72,18 @@ class CentOSConfigurator(configurator.ApacheConfigurator): # Finish with actual config check to see if systemctl restart helped super(CentOSConfigurator, self).config_test() + def pick_apache_config(self): + """ + Pick the appropriate TLS Apache configuration file for current version of Apache and OS. + :return: the path to the TLS Apache configuration file to use + :rtype: str + """ + # Disabling TLS session tickets is supported by Apache 2.4.11+. + # So for old versions of Apache we pick a configuration without this option. + if self.version < (2, 4, 11): + return apache_util.find_ssl_apache_conf("centos-old") + return apache_util.find_ssl_apache_conf("centos-current") + def _prepare_options(self): """ Override the options dictionary initialization in order to support diff --git a/certbot-apache/certbot_apache/override_darwin.py b/certbot-apache/certbot_apache/override_darwin.py index 4e2a6acac..e825b66b8 100644 --- a/certbot-apache/certbot_apache/override_darwin.py +++ b/certbot-apache/certbot_apache/override_darwin.py @@ -1,6 +1,4 @@ """ Distribution specific override class for macOS """ -import pkg_resources - import zope.interface from certbot import interfaces @@ -26,6 +24,4 @@ class DarwinConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2/other", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "options-ssl-apache.conf") ) diff --git a/certbot-apache/certbot_apache/override_debian.py b/certbot-apache/certbot_apache/override_debian.py index 58492bd01..1fc32670b 100644 --- a/certbot-apache/certbot_apache/override_debian.py +++ b/certbot-apache/certbot_apache/override_debian.py @@ -1,7 +1,6 @@ """ Distribution specific override class for Debian family (Ubuntu/Debian) """ import logging -import pkg_resources import zope.interface from certbot import errors @@ -35,8 +34,6 @@ class DebianConfigurator(configurator.ApacheConfigurator): handle_modules=True, handle_sites=True, challenge_location="/etc/apache2", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "options-ssl-apache.conf") ) def enable_site(self, vhost): diff --git a/certbot-apache/certbot_apache/override_fedora.py b/certbot-apache/certbot_apache/override_fedora.py index 786ada0fc..77f31efe8 100644 --- a/certbot-apache/certbot_apache/override_fedora.py +++ b/certbot-apache/certbot_apache/override_fedora.py @@ -1,5 +1,4 @@ """ Distribution specific override class for Fedora 29+ """ -import pkg_resources import zope.interface from certbot import errors @@ -31,9 +30,6 @@ class FedoraConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/httpd/conf.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - # TODO: eventually newest version of Fedora will need their own config - "certbot_apache", "centos-options-ssl-apache.conf") ) def config_test(self): diff --git a/certbot-apache/certbot_apache/override_gentoo.py b/certbot-apache/certbot_apache/override_gentoo.py index c358a10fa..6fa033857 100644 --- a/certbot-apache/certbot_apache/override_gentoo.py +++ b/certbot-apache/certbot_apache/override_gentoo.py @@ -1,6 +1,4 @@ """ Distribution specific override class for Gentoo Linux """ -import pkg_resources - import zope.interface from certbot import interfaces @@ -29,8 +27,6 @@ class GentooConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2/vhosts.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "options-ssl-apache.conf") ) def _prepare_options(self): diff --git a/certbot-apache/certbot_apache/override_suse.py b/certbot-apache/certbot_apache/override_suse.py index 3d0043afe..4baa57497 100644 --- a/certbot-apache/certbot_apache/override_suse.py +++ b/certbot-apache/certbot_apache/override_suse.py @@ -1,6 +1,4 @@ """ Distribution specific override class for OpenSUSE """ -import pkg_resources - import zope.interface from certbot import interfaces @@ -26,6 +24,4 @@ class OpenSUSEConfigurator(configurator.ApacheConfigurator): handle_modules=False, handle_sites=False, challenge_location="/etc/apache2/vhosts.d", - MOD_SSL_CONF_SRC=pkg_resources.resource_filename( - "certbot_apache", "options-ssl-apache.conf") ) diff --git a/certbot-apache/certbot_apache/tests/centos_test.py b/certbot-apache/certbot_apache/tests/centos_test.py index dddbf489e..5c8cff3b3 100644 --- a/certbot-apache/certbot_apache/tests/centos_test.py +++ b/certbot-apache/certbot_apache/tests/centos_test.py @@ -190,6 +190,13 @@ class MultipleVhostsTestCentOS(util.ApacheTest): errors.SubprocessError] self.assertRaises(errors.MisconfigurationError, self.config.restart) + def test_pick_correct_tls_config(self): + self.config.version = (2, 4, 10) + self.assertTrue('centos-old' in self.config.pick_apache_config()) + + self.config.version = (2, 4, 11) + self.assertTrue('centos-current' in self.config.pick_apache_config()) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 1eafae982..2bc2271a1 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -1706,7 +1706,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): self.config.updated_mod_ssl_conf_digest) def _current_ssl_options_hash(self): - return crypto_util.sha256sum(self.config.option("MOD_SSL_CONF_SRC")) + return crypto_util.sha256sum(self.config.pick_apache_config()) def _assert_current_file(self): self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) @@ -1742,7 +1742,7 @@ class InstallSslOptionsConfTest(util.ApacheTest): self.assertFalse(mock_logger.warning.called) self.assertTrue(os.path.isfile(self.config.mod_ssl_conf)) self.assertEqual(crypto_util.sha256sum( - self.config.option("MOD_SSL_CONF_SRC")), + self.config.pick_apache_config()), self._current_ssl_options_hash()) self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf), self._current_ssl_options_hash()) @@ -1758,18 +1758,31 @@ class InstallSslOptionsConfTest(util.ApacheTest): "%s has been manually modified; updated file " "saved to %s. We recommend updating %s for security purposes.") self.assertEqual(crypto_util.sha256sum( - self.config.option("MOD_SSL_CONF_SRC")), + self.config.pick_apache_config()), self._current_ssl_options_hash()) # only print warning once with mock.patch("certbot.plugins.common.logger") as mock_logger: self._call() self.assertFalse(mock_logger.warning.called) - def test_current_file_hash_in_all_hashes(self): + def test_ssl_config_files_hash_in_all_hashes(self): + """ + It is really critical that all TLS Apache config files have their SHA256 hash registered in + constants.ALL_SSL_OPTIONS_HASHES. Otherwise Certbot will mistakenly assume that the config + file has been manually edited by the user, and will refuse to update it. + This test ensures that all necessary hashes are present. + """ from certbot_apache.constants import ALL_SSL_OPTIONS_HASHES - self.assertTrue(self._current_ssl_options_hash() in ALL_SSL_OPTIONS_HASHES, - "Constants.ALL_SSL_OPTIONS_HASHES must be appended" - " with the sha256 hash of self.config.mod_ssl_conf when it is updated.") + import pkg_resources + tls_configs_dir = pkg_resources.resource_filename("certbot_apache", "tls_configs") + all_files = [os.path.join(tls_configs_dir, name) for name in os.listdir(tls_configs_dir) + if name.endswith('options-ssl-apache.conf')] + self.assertTrue(all_files) + for one_file in all_files: + file_hash = crypto_util.sha256sum(one_file) + self.assertTrue(file_hash in ALL_SSL_OPTIONS_HASHES, + "Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 " + "hash of {0} when it is updated.".format(one_file)) if __name__ == "__main__": diff --git a/certbot-apache/certbot_apache/centos-options-ssl-apache.conf b/certbot-apache/certbot_apache/tls_configs/centos-current-options-ssl-apache.conf similarity index 84% rename from certbot-apache/certbot_apache/centos-options-ssl-apache.conf rename to certbot-apache/certbot_apache/tls_configs/centos-current-options-ssl-apache.conf index 56c946a4e..2d99f6219 100644 --- a/certbot-apache/certbot_apache/centos-options-ssl-apache.conf +++ b/certbot-apache/certbot_apache/tls_configs/centos-current-options-ssl-apache.conf @@ -10,16 +10,10 @@ SSLEngine on SSLProtocol all -SSLv2 -SSLv3 SSLCipherSuite ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS SSLHonorCipherOrder on +SSLSessionTickets off SSLOptions +StrictRequire # Add vhost name to log entries: LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common - -#CustomLog /var/log/apache2/access.log vhost_combined -#LogLevel warn -#ErrorLog /var/log/apache2/error.log - -# Always ensure Cookies have "Secure" set (JAH 2012/1) -#Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4" diff --git a/certbot-apache/certbot_apache/tls_configs/centos-old-options-ssl-apache.conf b/certbot-apache/certbot_apache/tls_configs/centos-old-options-ssl-apache.conf new file mode 100644 index 000000000..277c8954a --- /dev/null +++ b/certbot-apache/certbot_apache/tls_configs/centos-old-options-ssl-apache.conf @@ -0,0 +1,18 @@ +# This file contains important security parameters. If you modify this file +# manually, Certbot will be unable to automatically provide future security +# updates. Instead, Certbot will print and log an error message with a path to +# the up-to-date file that you will need to refer to when manually updating +# this file. + +SSLEngine on + +# Intermediate configuration, tweak to your needs +SSLProtocol all -SSLv2 -SSLv3 +SSLCipherSuite ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS +SSLHonorCipherOrder on + +SSLOptions +StrictRequire + +# Add vhost name to log entries: +LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined +LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common diff --git a/certbot-apache/certbot_apache/options-ssl-apache.conf b/certbot-apache/certbot_apache/tls_configs/current-options-ssl-apache.conf similarity index 85% rename from certbot-apache/certbot_apache/options-ssl-apache.conf rename to certbot-apache/certbot_apache/tls_configs/current-options-ssl-apache.conf index 8113ee81e..c32e83148 100644 --- a/certbot-apache/certbot_apache/options-ssl-apache.conf +++ b/certbot-apache/certbot_apache/tls_configs/current-options-ssl-apache.conf @@ -11,16 +11,10 @@ SSLProtocol all -SSLv2 -SSLv3 SSLCipherSuite ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS SSLHonorCipherOrder on SSLCompression off +SSLSessionTickets off SSLOptions +StrictRequire # Add vhost name to log entries: LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common - -#CustomLog /var/log/apache2/access.log vhost_combined -#LogLevel warn -#ErrorLog /var/log/apache2/error.log - -# Always ensure Cookies have "Secure" set (JAH 2012/1) -#Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4" diff --git a/certbot-apache/certbot_apache/tls_configs/old-options-ssl-apache.conf b/certbot-apache/certbot_apache/tls_configs/old-options-ssl-apache.conf new file mode 100644 index 000000000..cd7c9bc4b --- /dev/null +++ b/certbot-apache/certbot_apache/tls_configs/old-options-ssl-apache.conf @@ -0,0 +1,19 @@ +# This file contains important security parameters. If you modify this file +# manually, Certbot will be unable to automatically provide future security +# updates. Instead, Certbot will print and log an error message with a path to +# the up-to-date file that you will need to refer to when manually updating +# this file. + +SSLEngine on + +# Intermediate configuration, tweak to your needs +SSLProtocol all -SSLv2 -SSLv3 +SSLCipherSuite ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS +SSLHonorCipherOrder on +SSLCompression off + +SSLOptions +StrictRequire + +# Add vhost name to log entries: +LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined +LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common From bfd4955bad0ea1771bb1d27be1359ec8798f9e6c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 30 Jul 2019 12:28:18 -0700 Subject: [PATCH 23/26] Bump timeout waiting for ACME server to 4 minutes. (#7284) * Bump timeout to 4 minutes. * address review comments --- .../certbot_integration_tests/utils/acme_server.py | 2 +- certbot-ci/certbot_integration_tests/utils/misc.py | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/certbot-ci/certbot_integration_tests/utils/acme_server.py b/certbot-ci/certbot_integration_tests/utils/acme_server.py index 8e3419f70..a41f6366a 100755 --- a/certbot-ci/certbot_integration_tests/utils/acme_server.py +++ b/certbot-ci/certbot_integration_tests/utils/acme_server.py @@ -159,7 +159,7 @@ class ACMEServer(object): # Wait for the ACME CA server to be up. print('=> Waiting for boulder instance to respond...') - misc.check_until_timeout(self.acme_xdist['directory_url']) + misc.check_until_timeout(self.acme_xdist['directory_url'], attempts=240) # Configure challtestsrv to answer any A record request with ip of the docker host. response = requests.post('http://localhost:{0}/set-default-ipv4'.format(CHALLTESTSRV_PORT), diff --git a/certbot-ci/certbot_integration_tests/utils/misc.py b/certbot-ci/certbot_integration_tests/utils/misc.py index 2144b9cee..0237bc01e 100644 --- a/certbot-ci/certbot_integration_tests/utils/misc.py +++ b/certbot-ci/certbot_integration_tests/utils/misc.py @@ -28,12 +28,13 @@ RSA_KEY_TYPE = 'rsa' ECDSA_KEY_TYPE = 'ecdsa' -def check_until_timeout(url): +def check_until_timeout(url, attempts=30): """ Wait and block until given url responds with status 200, or raise an exception - after 150 attempts. + after the specified number of attempts. :param str url: the URL to test - :raise ValueError: exception raised after 150 unsuccessful attempts to reach the URL + :param int attempts: the number of times to try to connect to the URL + :raise ValueError: exception raised if unable to reach the URL """ try: import urllib3 @@ -43,7 +44,7 @@ def check_until_timeout(url): from requests.packages.urllib3.exceptions import InsecureRequestWarning requests.packages.urllib3.disable_warnings(InsecureRequestWarning) - for _ in range(0, 150): + for _ in range(attempts): time.sleep(1) try: if requests.get(url, verify=False).status_code == 200: @@ -51,7 +52,7 @@ def check_until_timeout(url): except requests.exceptions.ConnectionError: pass - raise ValueError('Error, url did not respond after 150 attempts: {0}'.format(url)) + raise ValueError('Error, url did not respond after {0} attempts: {1}'.format(attempts, url)) class GracefulTCPServer(socketserver.TCPServer): From 2d3f3a042af6c5a4d2fb1a258e27868d34aef5ae Mon Sep 17 00:00:00 2001 From: Mikel Kew Date: Wed, 31 Jul 2019 18:31:05 +1000 Subject: [PATCH 24/26] Update dns-cloudflare docs regarding API Tokens (#7285) A quick update to the docs to explicitly mention that the Cloudflare Global API Key must me used instead of an API Token. --- certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py b/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py index 7e53f83ce..b08bc0968 100644 --- a/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py +++ b/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py @@ -22,7 +22,9 @@ Credentials Use of this plugin requires a configuration file containing Cloudflare API credentials, obtained from your Cloudflare -`account page `_. +`account page `_. This plugin +does not currently support Cloudflare's "API Tokens", so please ensure you use +the "Global API Key" for authentication. .. code-block:: ini :name: credentials.ini From 56f609d4f582f61274f93a807ec4b8a0bba43828 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 1 Aug 2019 19:39:46 +0200 Subject: [PATCH 25/26] Fix unit tests on Windows (#7270) Fixes #6850 This PR makes the last corrections needed to run all unit tests on Windows: add a function to check if a hook is executable in a cross-platform compatible way handle correctly the PATH surgery for Windows during hook execution handle correctly an account compatibility over both ACMEv1 and ACMEv2 remove (finally!) the @broken_on_windows decorator. * Fix account_tests * Fix hook executable test * Remove the temporary decorator @broken_on_windows * Fix util_test * No broken unit test on Windows anymore * More elegant mock * Fix context manager * Adapt coverage * Corrections * Adapt coverage * Forbid os.access --- .codecov.yml | 2 +- certbot/compat/filesystem.py | 36 ++++++++++++++ certbot/compat/misc.py | 4 ++ certbot/compat/os.py | 9 ++++ certbot/constants.py | 7 +-- certbot/hooks.py | 5 +- certbot/plugins/util.py | 6 ++- certbot/plugins/util_test.py | 17 ++++--- certbot/tests/account_test.py | 17 ++----- certbot/tests/compat/filesystem_test.py | 65 +++++++++++++++++++++++++ certbot/tests/compat/os_test.py | 3 +- certbot/tests/hook_test.py | 29 +++++------ certbot/tests/util.py | 9 ---- certbot/tests/util_test.py | 25 +++------- certbot/util.py | 16 +----- 15 files changed, 163 insertions(+), 87 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index f4d4d1d6c..8a1503da8 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -13,6 +13,6 @@ coverage: flags: windows # Fixed target instead of auto set by #7173, can # be removed when flags in Codecov are added back. - target: 97.2 + target: 97.6 threshold: 0.1 base: auto diff --git a/certbot/compat/filesystem.py b/certbot/compat/filesystem.py index a38fbe760..7a48e24f1 100644 --- a/certbot/compat/filesystem.py +++ b/certbot/compat/filesystem.py @@ -289,6 +289,42 @@ def realpath(file_path): return os.path.abspath(file_path) +# On Windows is_executable run from an unprivileged shell may claim that a path is +# executable when it is excutable only if run from a privileged shell. This result +# is due to the fact that GetEffectiveRightsFromAcl calculate effective rights +# without taking into consideration if the target user has currently required the +# elevated privileges or not. However this is not a problem since certbot always +# requires to be run under a privileged shell, so the user will always benefit +# from the highest (privileged one) set of permissions on a given file. +def is_executable(path): + """ + Is path an executable file? + :param str path: path to test + :returns: True if path is an executable file + :rtype: bool + """ + if POSIX_MODE: + return os.path.isfile(path) and os.access(path, os.X_OK) + + return _win_is_executable(path) + + +def _win_is_executable(path): + if not os.path.isfile(path): + return False + + security = win32security.GetFileSecurity(path, win32security.DACL_SECURITY_INFORMATION) + dacl = security.GetSecurityDescriptorDacl() + + mode = dacl.GetEffectiveRightsFromAcl({ + 'TrusteeForm': win32security.TRUSTEE_IS_SID, + 'TrusteeType': win32security.TRUSTEE_IS_USER, + 'Identifier': _get_current_user(), + }) + + return mode & ntsecuritycon.FILE_GENERIC_EXECUTE == ntsecuritycon.FILE_GENERIC_EXECUTE + + def _apply_win_mode(file_path, mode): """ This function converts the given POSIX mode into a Windows ACL list, and applies it to the diff --git a/certbot/compat/misc.py b/certbot/compat/misc.py index 693fceefb..5151e7156 100644 --- a/certbot/compat/misc.py +++ b/certbot/compat/misc.py @@ -30,6 +30,10 @@ else: MASK_FOR_PRIVATE_KEY_PERMISSIONS = 0 +# For Linux: define OS specific standard binary directories +STANDARD_BINARY_DIRS = ["/usr/sbin", "/usr/local/bin", "/usr/local/sbin"] if POSIX_MODE else [] + + def raise_for_non_administrative_windows_rights(): # type: () -> None """ diff --git a/certbot/compat/os.py b/certbot/compat/os.py index bb0e02eb3..2857ea408 100644 --- a/certbot/compat/os.py +++ b/certbot/compat/os.py @@ -107,3 +107,12 @@ def replace(*unused_args, **unused_kwargs): """Method os.replace() is forbidden""" raise RuntimeError('Usage of os.replace() is forbidden. ' 'Use certbot.compat.filesystem.replace() instead.') + + +# Results given by os.access are inconsistent or partial on Windows, because this platform is not +# following the POSIX approach. +def access(*unused_args, **unused_kwargs): + """Method os.access() is forbidden""" + raise RuntimeError('Usage of os.access() is forbidden. ' + 'Use certbot.compat.filesystem.check_mode() or ' + 'certbot.compat.filesystem.is_executable() instead.') diff --git a/certbot/constants.py b/certbot/constants.py index 5b268e157..10cd58ca1 100644 --- a/certbot/constants.py +++ b/certbot/constants.py @@ -169,9 +169,10 @@ ACCOUNTS_DIR = "accounts" """Directory where all accounts are saved.""" LE_REUSE_SERVERS = { - 'acme-v02.api.letsencrypt.org/directory': 'acme-v01.api.letsencrypt.org/directory', - 'acme-staging-v02.api.letsencrypt.org/directory': - 'acme-staging.api.letsencrypt.org/directory' + os.path.normpath('acme-v02.api.letsencrypt.org/directory'): + os.path.normpath('acme-v01.api.letsencrypt.org/directory'), + os.path.normpath('acme-staging-v02.api.letsencrypt.org/directory'): + os.path.normpath('acme-staging.api.letsencrypt.org/directory') } """Servers that can reuse accounts from other servers.""" diff --git a/certbot/hooks.py b/certbot/hooks.py index 34e06e0a3..1bb3a2eab 100644 --- a/certbot/hooks.py +++ b/certbot/hooks.py @@ -8,6 +8,7 @@ from acme.magic_typing import Set, List # pylint: disable=unused-import, no-nam from certbot import errors from certbot import util +from certbot.compat import filesystem from certbot.compat import os from certbot.plugins import util as plug_util @@ -254,7 +255,7 @@ def execute(cmd_name, shell_cmd): cmd_name, shell_cmd, cmd.returncode) if err: logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err) - return (err, out) + return err, out def list_hooks(dir_path): @@ -267,5 +268,5 @@ def list_hooks(dir_path): """ allpaths = (os.path.join(dir_path, f) for f in os.listdir(dir_path)) - hooks = [path for path in allpaths if util.is_exe(path) and not path.endswith('~')] + hooks = [path for path in allpaths if filesystem.is_executable(path) and not path.endswith('~')] return sorted(hooks) diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py index 61f811280..87eb45fe9 100644 --- a/certbot/plugins/util.py +++ b/certbot/plugins/util.py @@ -3,9 +3,11 @@ import logging from certbot import util from certbot.compat import os +from certbot.compat.misc import STANDARD_BINARY_DIRS logger = logging.getLogger(__name__) + def get_prefixes(path): """Retrieves all possible path prefixes of a path, in descending order of length. For instance, @@ -26,6 +28,7 @@ def get_prefixes(path): break return prefixes + def path_surgery(cmd): """Attempt to perform PATH surgery to find cmd @@ -35,10 +38,9 @@ def path_surgery(cmd): :returns: True if the operation succeeded, False otherwise """ - dirs = ("/usr/sbin", "/usr/local/bin", "/usr/local/sbin") path = os.environ["PATH"] added = [] - for d in dirs: + for d in STANDARD_BINARY_DIRS: if d not in path: path += os.pathsep + d added.append(d) diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index 6ec6f62f4..c41e55222 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -16,6 +16,7 @@ class GetPrefixTest(unittest.TestCase): self.assertEqual(get_prefixes('/'), [os.path.normpath('/')]) self.assertEqual(get_prefixes('a'), ['a']) + class PathSurgeryTest(unittest.TestCase): """Tests for certbot.plugins.path_surgery.""" @@ -29,13 +30,15 @@ class PathSurgeryTest(unittest.TestCase): self.assertEqual(path_surgery("eg"), True) self.assertEqual(mock_debug.call_count, 0) self.assertEqual(os.environ["PATH"], all_path["PATH"]) - no_path = {"PATH": "/tmp/"} - with mock.patch.dict('os.environ', no_path): - path_surgery("thingy") - self.assertEqual(mock_debug.call_count, 2) - self.assertTrue("Failed to find" in mock_debug.call_args[0][0]) - self.assertTrue("/usr/local/bin" in os.environ["PATH"]) - self.assertTrue("/tmp" in os.environ["PATH"]) + if os.name != 'nt': + # This part is specific to Linux since on Windows no PATH surgery is ever done. + no_path = {"PATH": "/tmp/"} + with mock.patch.dict('os.environ', no_path): + path_surgery("thingy") + self.assertEqual(mock_debug.call_count, 2 if os.name != 'nt' else 1) + self.assertTrue("Failed to find" in mock_debug.call_args[0][0]) + self.assertTrue("/usr/local/bin" in os.environ["PATH"]) + self.assertTrue("/tmp" in os.environ["PATH"]) if __name__ == "__main__": diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index 24a092cc8..c14500798 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -2,7 +2,6 @@ import datetime import json import shutil -import stat import unittest import josepy as jose @@ -13,6 +12,7 @@ from acme import messages import certbot.tests.util as test_util from certbot import errors +from certbot.compat import filesystem from certbot.compat import misc from certbot.compat import os @@ -116,7 +116,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): self.assertTrue(os.path.isdir( misc.underscores_for_unsupported_characters_in_path(self.config.accounts_dir))) - @test_util.broken_on_windows def test_save_and_restore(self): self.storage.save(self.acc, self.mock_client) account_path = os.path.join(self.config.accounts_dir, self.acc.id) @@ -124,8 +123,8 @@ class AccountFileStorageTest(test_util.ConfigTestCase): for file_name in "regr.json", "meta.json", "private_key.json": self.assertTrue(os.path.exists( os.path.join(account_path, file_name))) - self.assertTrue(oct(os.stat(os.path.join( - account_path, "private_key.json"))[stat.ST_MODE] & 0o777) in ("0400", "0o400")) + self.assertTrue( + filesystem.check_mode(os.path.join(account_path, "private_key.json"), 0o400)) # restore loaded = self.storage.load(self.acc.id) @@ -219,14 +218,12 @@ class AccountFileStorageTest(test_util.ConfigTestCase): self._set_server('https://acme-staging.api.letsencrypt.org/directory') self.assertEqual([], self.storage.find_all()) - @test_util.broken_on_windows def test_upgrade_version_staging(self): self._set_server('https://acme-staging.api.letsencrypt.org/directory') self.storage.save(self.acc, self.mock_client) self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory') self.assertEqual([self.acc], self.storage.find_all()) - @test_util.broken_on_windows def test_upgrade_version_production(self): self._set_server('https://acme-v01.api.letsencrypt.org/directory') self.storage.save(self.acc, self.mock_client) @@ -244,7 +241,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory') self.assertEqual([], self.storage.find_all()) - @test_util.broken_on_windows def test_upgrade_load(self): self._set_server('https://acme-staging.api.letsencrypt.org/directory') self.storage.save(self.acc, self.mock_client) @@ -253,7 +249,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): account = self.storage.load(self.acc.id) self.assertEqual(prev_account, account) - @test_util.broken_on_windows def test_upgrade_load_single_account(self): self._set_server('https://acme-staging.api.letsencrypt.org/directory') self.storage.save(self.acc, self.mock_client) @@ -278,7 +273,6 @@ class AccountFileStorageTest(test_util.ConfigTestCase): errors.AccountStorageError, self.storage.save, self.acc, self.mock_client) - @test_util.broken_on_windows def test_delete(self): self.storage.save(self.acc, self.mock_client) self.storage.delete(self.acc.id) @@ -313,12 +307,10 @@ class AccountFileStorageTest(test_util.ConfigTestCase): self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory') self.assertRaises(errors.AccountNotFound, self.storage.load, self.acc.id) - @test_util.broken_on_windows def test_delete_folders_up(self): self._test_delete_folders('https://acme-staging.api.letsencrypt.org/directory') self._assert_symlinked_account_removed() - @test_util.broken_on_windows def test_delete_folders_down(self): self._test_delete_folders('https://acme-staging-v02.api.letsencrypt.org/directory') self._assert_symlinked_account_removed() @@ -328,15 +320,14 @@ class AccountFileStorageTest(test_util.ConfigTestCase): with open(os.path.join(self.config.accounts_dir, 'foo'), 'w') as f: f.write('bar') - @test_util.broken_on_windows def test_delete_shared_account_up(self): self._set_server_and_stop_symlink('https://acme-staging-v02.api.letsencrypt.org/directory') self._test_delete_folders('https://acme-staging.api.letsencrypt.org/directory') - @test_util.broken_on_windows def test_delete_shared_account_down(self): self._set_server_and_stop_symlink('https://acme-staging-v02.api.letsencrypt.org/directory') self._test_delete_folders('https://acme-staging-v02.api.letsencrypt.org/directory') + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/tests/compat/filesystem_test.py b/certbot/tests/compat/filesystem_test.py index 90ecd13d7..11293fbfe 100644 --- a/certbot/tests/compat/filesystem_test.py +++ b/certbot/tests/compat/filesystem_test.py @@ -1,4 +1,5 @@ """Tests for certbot.compat.filesystem""" +import contextlib import errno import unittest @@ -411,6 +412,70 @@ class RealpathTest(test_util.TempDirTestCase): self.assertTrue('link1 is a loop!' in str(error.exception)) +class IsExecutableTest(test_util.TempDirTestCase): + """Tests for is_executable method""" + def test_not_executable(self): + file_path = os.path.join(self.tempdir, "foo") + + # On Windows a file created within Certbot will always have all permissions to the + # Administrators group set. Since the unit tests are typically executed under elevated + # privileges, it means that current user will always have effective execute rights on the + # hook script, and so the test will fail. To prevent that and represent a file created + # outside Certbot as typically a hook file is, we mock the _generate_dacl function in + # certbot.compat.filesystem to give rights only to the current user. This implies removing + # all ACEs except the first one from the DACL created by original _generate_dacl function. + + from certbot.compat.filesystem import _generate_dacl + + def _execute_mock(user_sid, mode): + dacl = _generate_dacl(user_sid, mode) + for _ in range(1, dacl.GetAceCount()): + dacl.DeleteAce(1) # DeleteAce dynamically updates the internal index mapping. + return dacl + + # create a non-executable file + with mock.patch("certbot.compat.filesystem._generate_dacl", side_effect=_execute_mock): + os.close(filesystem.open(file_path, os.O_CREAT | os.O_WRONLY, 0o666)) + + self.assertFalse(filesystem.is_executable(file_path)) + + @mock.patch("certbot.compat.filesystem.os.path.isfile") + @mock.patch("certbot.compat.filesystem.os.access") + def test_full_path(self, mock_access, mock_isfile): + with _fix_windows_runtime(): + mock_access.return_value = True + mock_isfile.return_value = True + self.assertTrue(filesystem.is_executable("/path/to/exe")) + + @mock.patch("certbot.compat.filesystem.os.path.isfile") + @mock.patch("certbot.compat.filesystem.os.access") + def test_rel_path(self, mock_access, mock_isfile): + with _fix_windows_runtime(): + mock_access.return_value = True + mock_isfile.return_value = True + self.assertTrue(filesystem.is_executable("exe")) + + @mock.patch("certbot.compat.filesystem.os.path.isfile") + @mock.patch("certbot.compat.filesystem.os.access") + def test_not_found(self, mock_access, mock_isfile): + with _fix_windows_runtime(): + mock_access.return_value = True + mock_isfile.return_value = False + self.assertFalse(filesystem.is_executable("exe")) + + +@contextlib.contextmanager +def _fix_windows_runtime(): + if os.name != 'nt': + yield + else: + with mock.patch('win32security.GetFileSecurity') as mock_get: + dacl_mock = mock_get.return_value.GetSecurityDescriptorDacl + mode_mock = dacl_mock.return_value.GetEffectiveRightsFromAcl + mode_mock.return_value = ntsecuritycon.FILE_GENERIC_EXECUTE + yield + + def _get_security_dacl(target): return win32security.GetFileSecurity(target, win32security.DACL_SECURITY_INFORMATION) diff --git a/certbot/tests/compat/os_test.py b/certbot/tests/compat/os_test.py index 754e97a10..e4928e4fb 100644 --- a/certbot/tests/compat/os_test.py +++ b/certbot/tests/compat/os_test.py @@ -8,7 +8,8 @@ class OsTest(unittest.TestCase): """Unit tests for os module.""" def test_forbidden_methods(self): # Checks for os module - for method in ['chmod', 'chown', 'open', 'mkdir', 'makedirs', 'rename', 'replace']: + for method in ['chmod', 'chown', 'open', 'mkdir', + 'makedirs', 'rename', 'replace', 'access']: self.assertRaises(RuntimeError, getattr(os, method)) # Checks for os.path module for method in ['realpath']: diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py index 079bc7f4c..017edc560 100644 --- a/certbot/tests/hook_test.py +++ b/certbot/tests/hook_test.py @@ -1,14 +1,14 @@ """Tests for certbot.hooks.""" -import stat import unittest import mock from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module from certbot import errors +from certbot import util from certbot.compat import os from certbot.compat import filesystem -from certbot.tests import util +from certbot.tests import util as test_util class ValidateHooksTest(unittest.TestCase): @@ -30,7 +30,7 @@ class ValidateHooksTest(unittest.TestCase): self.assertEqual("renew", types[-1]) -class ValidateHookTest(util.TempDirTestCase): +class ValidateHookTest(test_util.TempDirTestCase): """Tests for certbot.hooks.validate_hook.""" @classmethod @@ -38,22 +38,20 @@ class ValidateHookTest(util.TempDirTestCase): from certbot.hooks import validate_hook return validate_hook(*args, **kwargs) - @util.broken_on_windows - def test_not_executable(self): - file_path = os.path.join(self.tempdir, "foo") - # create a non-executable file - os.close(filesystem.open(file_path, os.O_CREAT | os.O_WRONLY, 0o666)) + def test_hook_not_executable(self): # prevent unnecessary modifications to PATH with mock.patch("certbot.hooks.plug_util.path_surgery"): - self.assertRaises(errors.HookCommandNotFound, - self._call, file_path, "foo") + # We just mock out filesystem.is_executable since on Windows, it is difficult + # to get a fully working test around executable permissions. See + # certbot.tests.compat.filesystem::NotExecutableTest for more in-depth tests. + with mock.patch("certbot.hooks.filesystem.is_executable", return_value=False): + self.assertRaises(errors.HookCommandNotFound, self._call, 'dummy', "foo") @mock.patch("certbot.hooks.util.exe_exists") def test_not_found(self, mock_exe_exists): mock_exe_exists.return_value = False with mock.patch("certbot.hooks.plug_util.path_surgery") as mock_ps: - self.assertRaises(errors.HookCommandNotFound, - self._call, "foo", "bar") + self.assertRaises(errors.HookCommandNotFound, self._call, "foo", "bar") self.assertTrue(mock_ps.called) @mock.patch("certbot.hooks._prog") @@ -62,7 +60,7 @@ class ValidateHookTest(util.TempDirTestCase): self.assertFalse(mock_prog.called) -class HookTest(util.ConfigTestCase): +class HookTest(test_util.ConfigTestCase): """Common base class for hook tests.""" @classmethod @@ -454,7 +452,7 @@ class ExecuteTest(unittest.TestCase): self.assertTrue(mock_logger.error.called) -class ListHooksTest(util.TempDirTestCase): +class ListHooksTest(test_util.TempDirTestCase): """Tests for certbot.hooks.list_hooks.""" @classmethod @@ -494,8 +492,7 @@ def create_hook(file_path): :param str file_path: path to create the file at """ - open(file_path, "w").close() - filesystem.chmod(file_path, os.stat(file_path).st_mode | stat.S_IXUSR) + util.safe_open(file_path, mode="w", chmod=0o744).close() if __name__ == '__main__': diff --git a/certbot/tests/util.py b/certbot/tests/util.py index 56fbd5458..7ee215c66 100644 --- a/certbot/tests/util.py +++ b/certbot/tests/util.py @@ -421,15 +421,6 @@ def skip_on_windows(reason): return wrapper -def broken_on_windows(function): - """Decorator to skip temporarily a broken test on Windows.""" - reason = 'Test is broken and ignored on windows but should be fixed.' - return unittest.skipIf( - sys.platform == 'win32' - and os.environ.get('SKIP_BROKEN_TESTS_ON_WINDOWS', 'true') == 'true', - reason)(function) - - def temp_join(path): """ Return the given path joined to the tempdir path for the current platform diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index e65de3349..cf4f31647 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -52,26 +52,13 @@ class ExeExistsTest(unittest.TestCase): from certbot.util import exe_exists return exe_exists(exe) - @mock.patch("certbot.util.os.path.isfile") - @mock.patch("certbot.util.os.access") - def test_full_path(self, mock_access, mock_isfile): - mock_access.return_value = True - mock_isfile.return_value = True - self.assertTrue(self._call("/path/to/exe")) + def test_exe_exists(self): + with mock.patch("certbot.util.filesystem.is_executable", return_value=True): + self.assertTrue(self._call("/path/to/exe")) - @mock.patch("certbot.util.os.path.isfile") - @mock.patch("certbot.util.os.access") - def test_on_path(self, mock_access, mock_isfile): - mock_access.return_value = True - mock_isfile.return_value = True - self.assertTrue(self._call("exe")) - - @mock.patch("certbot.util.os.path.isfile") - @mock.patch("certbot.util.os.access") - def test_not_found(self, mock_access, mock_isfile): - mock_access.return_value = False - mock_isfile.return_value = True - self.assertFalse(self._call("exe")) + def test_exe_not_exists(self): + with mock.patch("certbot.util.filesystem.is_executable", return_value=False): + self.assertFalse(self._call("/path/to/exe")) class LockDirUntilExit(test_util.TempDirTestCase): diff --git a/certbot/util.py b/certbot/util.py index 8f553e51a..d3297507e 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -86,18 +86,6 @@ def run_script(params, log=logger.error): return stdout, stderr -def is_exe(path): - """Is path an executable file? - - :param str path: path to test - - :returns: True iff path is an executable file - :rtype: bool - - """ - return os.path.isfile(path) and os.access(path, os.X_OK) - - def exe_exists(exe): """Determine whether path/name refers to an executable. @@ -109,10 +97,10 @@ def exe_exists(exe): """ path, _ = os.path.split(exe) if path: - return is_exe(exe) + return filesystem.is_executable(exe) else: for path in os.environ["PATH"].split(os.pathsep): - if is_exe(os.path.join(path, exe)): + if filesystem.is_executable(os.path.join(path, exe)): return True return False From 36b4c312c6c802c13dfde6f13f2b2a3004c913e0 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Fri, 2 Aug 2019 18:47:36 +0200 Subject: [PATCH 26/26] Upgrade virtualenv in dev/tests environments (#7287) AppVeyor recently upgrade the Python 3.7.x installed in their VM to 3.7.4. However, virtualenv 16.6.1 is broken on that specific version of Python for Windows. This PR upgrade virtualenv installed for a dev/test environment from 16.6.1 to 16.6.2 in order to fix this issue, and repair the CI jobs execute by AppVeyor on PRs. --- tools/dev_constraints.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/dev_constraints.txt b/tools/dev_constraints.txt index 1aa8414f9..660986da9 100644 --- a/tools/dev_constraints.txt +++ b/tools/dev_constraints.txt @@ -82,6 +82,6 @@ twine==1.11.0 typed-ast==1.1.0 typing==3.6.4 uritemplate==0.6 -virtualenv==16.6.1 +virtualenv==16.6.2 wcwidth==0.1.7 wrapt==1.11.1