From 9734be69220168588780430880fe410cfc6a2ec8 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Wed, 1 May 2019 14:07:30 -0700 Subject: [PATCH 1/9] Add contents to CHANGELOG.md for next version --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82eac94cb..77351c84b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). +## 0.35.0 - master + +### Added + +* + +### Changed + +* + +### Fixed + +* + +Despite us having broken lockstep, we are continuing to release new versions of +all Certbot components during releases for the time being, however, the only +package with changes other than its version number was: + +* + +More details about these changes can be found on our GitHub repo. + ## 0.34.0 - 2019-05-01 ### Changed From 7711da9fc21aea72325c3d730b706c12fc1ffe94 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Wed, 1 May 2019 14:07:30 -0700 Subject: [PATCH 2/9] Bump version to 0.35.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 85e9a642a..56a9a63f3 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.34.0' +version = '0.35.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 3161402a5..e14bcb3b6 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.34.0' +version = '0.35.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 fc03fd971..c95864e09 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.34.0' +version = '0.35.0.dev0' install_requires = [ 'certbot', diff --git a/certbot-dns-cloudflare/setup.py b/certbot-dns-cloudflare/setup.py index 64efd115b..afdfb09e1 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.34.0' +version = '0.35.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 df79af91d..4883150fb 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.34.0' +version = '0.35.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 3444a6f8c..07d406fd9 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.34.0' +version = '0.35.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 588541821..f781bd0a5 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.34.0' +version = '0.35.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 4f1f9d59c..41c1b75a5 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.34.0' +version = '0.35.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 e27d0e154..f009df8e8 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.34.0' +version = '0.35.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 fc95cc06b..e1d6aeed0 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.34.0' +version = '0.35.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 e1238ab07..ae8739d61 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.34.0' +version = '0.35.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 9c4c74f96..b2e14869e 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.34.0' +version = '0.35.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 8a75f6d9d..e839cd71d 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.34.0' +version = '0.35.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 a4da5976f..a6a52d648 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.34.0' +version = '0.35.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 c37660aaf..e05104e5d 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.34.0' +version = '0.35.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 4177da095..09cd4acd2 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.34.0' +version = '0.35.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 3d75a0279..29f458542 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.34.0' +version = '0.35.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 1bf6f1825..51055ce64 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.34.0' +version = '0.35.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 4157090a5..abec68040 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.34.0' +__version__ = '0.35.0.dev0' diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 0d9606372..4e1503715 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.34.0" +LE_AUTO_VERSION="0.35.0.dev0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates From 82f64126d9a1105da17f37386e1f40fcd3e97ea6 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Thu, 2 May 2019 12:46:59 -0400 Subject: [PATCH 3/9] Grammar (#7013) * spelling: these * grammar: either-or * spelling: e.g. --- certbot/compat/misc.py | 4 ++-- certbot/compat/os.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot/compat/misc.py b/certbot/compat/misc.py index 2f4ba0c6b..4f0e22078 100644 --- a/certbot/compat/misc.py +++ b/certbot/compat/misc.py @@ -31,7 +31,7 @@ def raise_for_non_administrative_windows_rights(subcommand): # Because windll exists only on a Windows runtime, and static code analysis engines # do not like at all non existent objects when run from Linux (even if we handle properly # all the cases in the code). - # So we access windll only by reflection to trick theses engines. + # So we access windll only by reflection to trick these engines. if hasattr(ctypes, 'windll') and subcommand not in UNPRIVILEGED_SUBCOMMANDS_ALLOWED: windll = getattr(ctypes, 'windll') if windll.shell32.IsUserAnAdmin() == 0: @@ -73,7 +73,7 @@ def os_rename(src, dst): raise if not hasattr(os, 'replace'): # pragma: no cover # We should never go on this line. Either we are on Linux and os.rename has succeeded, - # either we are on Windows, and only Python >= 3.4 is supported where os.replace is + # or we are on Windows, and only Python >= 3.4 is supported where os.replace is # available. raise RuntimeError('Error: tried to run os_rename on Python < 3.3. ' 'Certbot supports only Python 3.4 >= on Windows.') diff --git a/certbot/compat/os.py b/certbot/compat/os.py index 0112fbc73..8a139a141 100644 --- a/certbot/compat/os.py +++ b/certbot/compat/os.py @@ -1,6 +1,6 @@ """ This compat modules is a wrapper of the core os module that forbids usage of specific operations -(eg. chown, chmod, getuid) that would be harmful to the Windows file security model of Certbot. +(e.g. chown, chmod, getuid) that would be harmful to the Windows file security model of Certbot. This module is intended to replace standard os module throughout certbot projects (except acme). """ from __future__ import absolute_import From 862577fffc5ea6b29a9bb519d7b850b454d23e84 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 2 May 2019 11:32:49 -0700 Subject: [PATCH 4/9] Bump initial version to 0.33.1. (#7017) We made this change locally yesterday while preparing the release. I tested this change on all AMIs currently in the test farm as well as Fedora 29 and this test passed on all instances. --- tests/letstest/scripts/test_leauto_upgrades.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/letstest/scripts/test_leauto_upgrades.sh b/tests/letstest/scripts/test_leauto_upgrades.sh index d565aa268..d5133ba38 100755 --- a/tests/letstest/scripts/test_leauto_upgrades.sh +++ b/tests/letstest/scripts/test_leauto_upgrades.sh @@ -15,8 +15,8 @@ if ! command -v git ; then exit 1 fi fi -# 0.18.0 is the oldest version of letsencrypt-auto that works on Fedora 26+. -INITIAL_VERSION="0.18.0" +# 0.33.x is the oldest version of letsencrypt-auto that works on Fedora 29+. +INITIAL_VERSION="0.33.1" git checkout -f "v$INITIAL_VERSION" letsencrypt-auto if ! ./letsencrypt-auto -v --debug --version --no-self-upgrade 2>&1 | tail -n1 | grep "^certbot $INITIAL_VERSION$" ; then echo initial installation appeared to fail From e15e848474a5c180b2f22cd059c44905a11568ef Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 2 May 2019 11:36:47 -0700 Subject: [PATCH 5/9] Stop certbot-auto from printing blank lines (#7016) Fixes #7012. Apparently, the previous test we had here doesn't catch the case when certbot-auto prints blank lines. (I don't yet understand why so if someone does, please let me know!) Regardless, I fixed up the test and verified it fails with the version of letsencrypt-auto in master and then fixed letsencrypt-auto so the test passes. I ran test farm tests on the changes here and they passed on all instances. * correct test * fixes #7012 --- letsencrypt-auto-source/letsencrypt-auto | 8 +++++--- letsencrypt-auto-source/letsencrypt-auto.template | 8 +++++--- .../scripts/test_letsencrypt_auto_certonly_standalone.sh | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 4e1503715..f9f02da62 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -1593,12 +1593,14 @@ UNLIKELY_EOF # --------------------------------------------------------------------------- # If the script fails for some reason, don't break certbot-auto. set +e - # Suppress unexpected error output and only print the script's output if it - # ran successfully. + # Suppress unexpected error output. CHECK_PERM_OUT=$("$LE_PYTHON" "$TEMP_DIR/check_permissions.py" "$0" 2>/dev/null) CHECK_PERM_STATUS="$?" set -e - if [ "$CHECK_PERM_STATUS" = 0 ]; then + # Only print output if the script ran successfully and it actually produced + # output. The latter check resolves + # https://github.com/certbot/certbot/issues/7012. + if [ "$CHECK_PERM_STATUS" = 0 -a -n "$CHECK_PERM_OUT" ]; then error "$CHECK_PERM_OUT" fi fi diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template index 21db0f908..bff4173d4 100755 --- a/letsencrypt-auto-source/letsencrypt-auto.template +++ b/letsencrypt-auto-source/letsencrypt-auto.template @@ -670,12 +670,14 @@ UNLIKELY_EOF # --------------------------------------------------------------------------- # If the script fails for some reason, don't break certbot-auto. set +e - # Suppress unexpected error output and only print the script's output if it - # ran successfully. + # Suppress unexpected error output. CHECK_PERM_OUT=$("$LE_PYTHON" "$TEMP_DIR/check_permissions.py" "$0" 2>/dev/null) CHECK_PERM_STATUS="$?" set -e - if [ "$CHECK_PERM_STATUS" = 0 ]; then + # Only print output if the script ran successfully and it actually produced + # output. The latter check resolves + # https://github.com/certbot/certbot/issues/7012. + if [ "$CHECK_PERM_STATUS" = 0 -a -n "$CHECK_PERM_OUT" ]; then error "$CHECK_PERM_OUT" fi fi diff --git a/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh b/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh index 035512ef7..0973bbc03 100755 --- a/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh +++ b/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh @@ -42,8 +42,8 @@ if ! letsencrypt-auto --help --no-self-upgrade | grep -F "letsencrypt-auto [SUBC exit 1 fi -OUTPUT=$(letsencrypt-auto --install-only --no-self-upgrade --quiet 2>&1) -if [ -n "$OUTPUT" ]; then +OUTPUT_LEN=$(letsencrypt-auto --install-only --no-self-upgrade --quiet 2>&1 | wc -c) +if [ "$OUTPUT_LEN" != 0 ]; then echo letsencrypt-auto produced unexpected output! exit 1 fi From b19d4801c9dea2898402c5b388da4bd10b103d01 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 2 May 2019 23:32:02 +0200 Subject: [PATCH 6/9] Fix oldest tests when local dependencies are used (#7019) Fixes #7014. Using a --force-reinstall (only for oldest tests), dependencies are properly reinstalled. Since this action significantly increases the execution time of oldest tests, I split them into two parts to allow their parallel execution by Travis. We will need to find a better way to solve this in the future. An example of successful execution of oldest tests in the situation of a point release can be found here: https://travis-ci.org/adferrand/certbot/builds/527475532 * Fix for oldest requirements * Split oldest tests * Update a comment --- .travis.yml | 7 ++++++- tools/pip_install.py | 11 +++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index b23164a19..e1859cc0e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -60,7 +60,12 @@ matrix: env: TOXENV=mypy <<: *not-on-master - python: "2.7" - env: TOXENV='py27-{acme,apache,certbot,dns,nginx,postfix}-oldest' + env: TOXENV='py27-{acme,apache,certbot,nginx,postfix}-oldest' + sudo: required + services: docker + <<: *not-on-master + - python: "2.7" + env: TOXENV='py27-dns-oldest' sudo: required services: docker <<: *not-on-master diff --git a/tools/pip_install.py b/tools/pip_install.py index 68268e298..abc7baa91 100755 --- a/tools/pip_install.py +++ b/tools/pip_install.py @@ -99,8 +99,15 @@ def main(args): else: # Otherwise, we merge requirements to build the constraints and pin dependencies requirements = None + reinstall = False if os.environ.get('CERTBOT_OLDEST') == '1': requirements = certbot_oldest_processing(tools_path, args, test_constraints) + # We need to --force-reinstall the tested distribution when using oldest + # requirements because of an error in these tests in particular situations + # described in https://github.com/certbot/certbot/issues/7014. + # However this slows down considerably the oldest tests (5 min -> 10 min), + # so we need to find a better mitigation in the future. + reinstall = True else: certbot_normal_processing(tools_path, test_constraints) @@ -109,8 +116,8 @@ def main(args): pip_install_with_print('--constraint "{0}" --requirement "{1}"' .format(all_constraints, requirements)) - pip_install_with_print('--constraint "{0}" {1}' - .format(all_constraints, ' '.join(args))) + pip_install_with_print('--constraint "{0}" {1} {2}'.format( + all_constraints, '--force-reinstall' if reinstall else '', ' '.join(args))) finally: if os.environ.get('TRAVIS'): print('travis_fold:end:install_certbot_deps') From 4bf6eb2091e3190282b0e2c6540186e64bf4d846 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 2 May 2019 14:52:36 -0700 Subject: [PATCH 7/9] Update changelog for 0.34.1. (#7021) --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77351c84b..84f28cea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* certbot-auto no longer prints a blank line when there are no permissions + problems. Despite us having broken lockstep, we are continuing to release new versions of all Certbot components during releases for the time being, however, the only From 6a970f74d0fe13c739a105e20dc8892c49b677b1 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Fri, 3 May 2019 00:17:11 +0200 Subject: [PATCH 8/9] Try another approach (#7022) In #7019, a solution has been integrated to fix oldest tests execution in the corner cases described in #7014. However this solution was not very satisfactory, as it consists in making a --force-reinstall for all requirements on each oldest tests (apache, certbot, acme, each dns plugin ...). As a consequence, the overall execution time of these tests increased from 5 min to 10 min. In this PR I propose a more elegant solution: instead of reinstalling all dependencies, we force reinstall only the requirements themselves describe in the relevant oldest-requirements.txt files. This way only the packages that are potentially ignored by pip because they exists locally (acme, certbot, ...) are reinstalled. The result is the same than in #7019 (we are sure that all packages are really installed by pip), but the very limited number of force reinstalled package here make the impact on execution time negligible. As a consequence, I revert back also the tox environments to execute all oldest tests together. A successful execution of oldest tests using this PR material in the context of a point release can be seen here: https://travis-ci.org/adferrand/certbot/builds/527513101 --- .travis.yml | 7 +------ tools/pip_install.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index e1859cc0e..b23164a19 100644 --- a/.travis.yml +++ b/.travis.yml @@ -60,12 +60,7 @@ matrix: env: TOXENV=mypy <<: *not-on-master - python: "2.7" - env: TOXENV='py27-{acme,apache,certbot,nginx,postfix}-oldest' - sudo: required - services: docker - <<: *not-on-master - - python: "2.7" - env: TOXENV='py27-dns-oldest' + env: TOXENV='py27-{acme,apache,certbot,dns,nginx,postfix}-oldest' sudo: required services: docker <<: *not-on-master diff --git a/tools/pip_install.py b/tools/pip_install.py index abc7baa91..cf0a7aee5 100755 --- a/tools/pip_install.py +++ b/tools/pip_install.py @@ -99,25 +99,25 @@ def main(args): else: # Otherwise, we merge requirements to build the constraints and pin dependencies requirements = None - reinstall = False if os.environ.get('CERTBOT_OLDEST') == '1': requirements = certbot_oldest_processing(tools_path, args, test_constraints) - # We need to --force-reinstall the tested distribution when using oldest - # requirements because of an error in these tests in particular situations - # described in https://github.com/certbot/certbot/issues/7014. - # However this slows down considerably the oldest tests (5 min -> 10 min), - # so we need to find a better mitigation in the future. - reinstall = True else: certbot_normal_processing(tools_path, test_constraints) merge_requirements(tools_path, requirements, test_constraints, all_constraints) - if requirements: + if requirements: # This branch is executed during the oldest tests + # First step, install the transitive dependencies of oldest requirements + # in respect with oldest constraints. pip_install_with_print('--constraint "{0}" --requirement "{1}"' .format(all_constraints, requirements)) + # Second step, ensure that oldest requirements themselves are effectively + # installed using --force-reinstall, and avoid corner cases like the one described + # in https://github.com/certbot/certbot/issues/7014. + pip_install_with_print('--force-reinstall --no-deps --requirement "{0}"' + .format(requirements)) - pip_install_with_print('--constraint "{0}" {1} {2}'.format( - all_constraints, '--force-reinstall' if reinstall else '', ' '.join(args))) + pip_install_with_print('--constraint "{0}" {1}'.format( + all_constraints, ' '.join(args))) finally: if os.environ.get('TRAVIS'): print('travis_fold:end:install_certbot_deps') From 71b1b8c2d9b99f1ca42b7ae08a1ef577341ce485 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Tue, 7 May 2019 00:49:47 +0200 Subject: [PATCH 9/9] Fix check permissions logic (#7034) Fixes #7031 I use the same approach than in `CreateVenv()` and `CompareVersions()`: a new bash function `CheckPathPermissions()` is declared an execute a python script passed to the interpreter through stdin. This allows: * to not require the temp_dir that holds a temporary script to be executed * to reduce at the bare minimum the change to make on the order of bash command to execute (including when the temp_dir is created) * Fix check permissions logic in certbot-auto by making a temp dir useless * Update CHANGELOG.md --- CHANGELOG.md | 2 + letsencrypt-auto-source/letsencrypt-auto | 177 +++++++++--------- .../letsencrypt-auto.template | 15 +- 3 files changed, 101 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84f28cea7..408118ad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * certbot-auto no longer prints a blank line when there are no permissions problems. +* certbot-auto no longer writes a check_permissions.py script at the root + of the filesystem. Despite us having broken lockstep, we are continuing to release new versions of all Certbot components during releases for the time being, however, the only diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index f9f02da62..da3fbe30f 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -953,6 +953,95 @@ if __name__ == '__main__': UNLIKELY_EOF } +# Check that the given PATH_TO_CHECK has secured permissions. +# Parameters: LE_PYTHON, PATH_TO_CHECK +CheckPathPermissions() { + "$1" - "$2" << "UNLIKELY_EOF" +"""Verifies certbot-auto cannot be modified by unprivileged users. + +This script takes the path to certbot-auto as its only command line +argument. It then checks that the file can only be modified by uid/gid +< 1000 and if other users can modify the file, it prints a warning with +a suggestion on how to solve the problem. + +Permissions on symlinks in the absolute path of certbot-auto are ignored +and only the canonical path to certbot-auto is checked. There could be +permissions problems due to the symlinks that are unreported by this +script, however, issues like this were not caused by our documentation +and are ignored for the sake of simplicity. + +All warnings are printed to stdout rather than stderr so all stderr +output from this script can be suppressed to avoid printing messages if +this script fails for some reason. + +""" +from __future__ import print_function + +import os +import stat +import sys + + +FORUM_POST_URL = 'https://community.letsencrypt.org/t/certbot-auto-deployment-best-practices/91979/' + + +def has_safe_permissions(path): + """Returns True if the given path has secure permissions. + + The permissions are considered safe if the file is only writable by + uid/gid < 1000. + + The reason we allow more IDs than 0 is because on some systems such + as Debian, system users/groups other than uid/gid 0 are used for the + path we recommend in our instructions which is /usr/local/bin. 1000 + was chosen because on Debian 0-999 is reserved for system IDs[1] and + on RHEL either 0-499 or 0-999 is reserved depending on the + version[2][3]. Due to these differences across different OSes, this + detection isn't perfect so we only determine permissions are + insecure when we can be reasonably confident there is a problem + regardless of the underlying OS. + + [1] https://www.debian.org/doc/debian-policy/ch-opersys.html#uid-and-gid-classes + [2] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/ch-managing_users_and_groups + [3] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/system_administrators_guide/ch-managing_users_and_groups + + :param str path: filesystem path to check + :returns: True if the path has secure permissions, otherwise, False + :rtype: bool + + """ + # os.stat follows symlinks before obtaining information about a file. + stat_result = os.stat(path) + if stat_result.st_mode & stat.S_IWOTH: + return False + if stat_result.st_mode & stat.S_IWGRP and stat_result.st_gid >= 1000: + return False + if stat_result.st_mode & stat.S_IWUSR and stat_result.st_uid >= 1000: + return False + return True + + +def main(certbot_auto_path): + current_path = os.path.realpath(certbot_auto_path) + last_path = None + permissions_ok = True + # This loop makes use of the fact that os.path.dirname('/') == '/'. + while current_path != last_path and permissions_ok: + permissions_ok = has_safe_permissions(current_path) + last_path = current_path + current_path = os.path.dirname(current_path) + + if not permissions_ok: + print('{0} has insecure permissions!'.format(certbot_auto_path)) + print('To learn how to fix them, visit {0}'.format(FORUM_POST_URL)) + + +if __name__ == '__main__': + main(sys.argv[1]) + +UNLIKELY_EOF +} + if [ "$1" = "--le-auto-phase2" ]; then # Phase 2: Create venv, install LE, and run. @@ -1505,96 +1594,10 @@ else # Don't warn about file permissions if the user disabled the check or we # can't find an up-to-date Python. if [ "$PYVER" -ge "$MIN_PYVER" -a "$NO_PERMISSIONS_CHECK" != 1 ]; then - # --------------------------------------------------------------------------- - cat << "UNLIKELY_EOF" > "$TEMP_DIR/check_permissions.py" -"""Verifies certbot-auto cannot be modified by unprivileged users. - -This script takes the path to certbot-auto as its only command line -argument. It then checks that the file can only be modified by uid/gid -< 1000 and if other users can modify the file, it prints a warning with -a suggestion on how to solve the problem. - -Permissions on symlinks in the absolute path of certbot-auto are ignored -and only the canonical path to certbot-auto is checked. There could be -permissions problems due to the symlinks that are unreported by this -script, however, issues like this were not caused by our documentation -and are ignored for the sake of simplicity. - -All warnings are printed to stdout rather than stderr so all stderr -output from this script can be suppressed to avoid printing messages if -this script fails for some reason. - -""" -from __future__ import print_function - -import os -import stat -import sys - - -FORUM_POST_URL = 'https://community.letsencrypt.org/t/certbot-auto-deployment-best-practices/91979/' - - -def has_safe_permissions(path): - """Returns True if the given path has secure permissions. - - The permissions are considered safe if the file is only writable by - uid/gid < 1000. - - The reason we allow more IDs than 0 is because on some systems such - as Debian, system users/groups other than uid/gid 0 are used for the - path we recommend in our instructions which is /usr/local/bin. 1000 - was chosen because on Debian 0-999 is reserved for system IDs[1] and - on RHEL either 0-499 or 0-999 is reserved depending on the - version[2][3]. Due to these differences across different OSes, this - detection isn't perfect so we only determine permissions are - insecure when we can be reasonably confident there is a problem - regardless of the underlying OS. - - [1] https://www.debian.org/doc/debian-policy/ch-opersys.html#uid-and-gid-classes - [2] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/ch-managing_users_and_groups - [3] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/system_administrators_guide/ch-managing_users_and_groups - - :param str path: filesystem path to check - :returns: True if the path has secure permissions, otherwise, False - :rtype: bool - - """ - # os.stat follows symlinks before obtaining information about a file. - stat_result = os.stat(path) - if stat_result.st_mode & stat.S_IWOTH: - return False - if stat_result.st_mode & stat.S_IWGRP and stat_result.st_gid >= 1000: - return False - if stat_result.st_mode & stat.S_IWUSR and stat_result.st_uid >= 1000: - return False - return True - - -def main(certbot_auto_path): - current_path = os.path.realpath(certbot_auto_path) - last_path = None - permissions_ok = True - # This loop makes use of the fact that os.path.dirname('/') == '/'. - while current_path != last_path and permissions_ok: - permissions_ok = has_safe_permissions(current_path) - last_path = current_path - current_path = os.path.dirname(current_path) - - if not permissions_ok: - print('{0} has insecure permissions!'.format(certbot_auto_path)) - print('To learn how to fix them, visit {0}'.format(FORUM_POST_URL)) - - -if __name__ == '__main__': - main(sys.argv[1]) - -UNLIKELY_EOF - # --------------------------------------------------------------------------- # If the script fails for some reason, don't break certbot-auto. set +e # Suppress unexpected error output. - CHECK_PERM_OUT=$("$LE_PYTHON" "$TEMP_DIR/check_permissions.py" "$0" 2>/dev/null) + CHECK_PERM_OUT=$(CheckPathPermissions "$LE_PYTHON" "$0" 2>/dev/null) CHECK_PERM_STATUS="$?" set -e # Only print output if the script ran successfully and it actually produced diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template index bff4173d4..c064580bd 100755 --- a/letsencrypt-auto-source/letsencrypt-auto.template +++ b/letsencrypt-auto-source/letsencrypt-auto.template @@ -501,6 +501,14 @@ CreateVenv() { UNLIKELY_EOF } +# Check that the given PATH_TO_CHECK has secured permissions. +# Parameters: LE_PYTHON, PATH_TO_CHECK +CheckPathPermissions() { + "$1" - "$2" << "UNLIKELY_EOF" +{{ check_permissions.py }} +UNLIKELY_EOF +} + if [ "$1" = "--le-auto-phase2" ]; then # Phase 2: Create venv, install LE, and run. @@ -663,15 +671,10 @@ else # Don't warn about file permissions if the user disabled the check or we # can't find an up-to-date Python. if [ "$PYVER" -ge "$MIN_PYVER" -a "$NO_PERMISSIONS_CHECK" != 1 ]; then - # --------------------------------------------------------------------------- - cat << "UNLIKELY_EOF" > "$TEMP_DIR/check_permissions.py" -{{ check_permissions.py }} -UNLIKELY_EOF - # --------------------------------------------------------------------------- # If the script fails for some reason, don't break certbot-auto. set +e # Suppress unexpected error output. - CHECK_PERM_OUT=$("$LE_PYTHON" "$TEMP_DIR/check_permissions.py" "$0" 2>/dev/null) + CHECK_PERM_OUT=$(CheckPathPermissions "$LE_PYTHON" "$0" 2>/dev/null) CHECK_PERM_STATUS="$?" set -e # Only print output if the script ran successfully and it actually produced