Merge branch 'master' into candidate-0.34.1-2

This commit is contained in:
Brad Warren 2019-05-06 15:56:22 -07:00
commit b86f553586
7 changed files with 115 additions and 113 deletions

View file

@ -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

View file

@ -14,7 +14,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
### Fixed
*
* 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

View file

@ -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.')

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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')