upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-06-04 06:15:36 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Merge branch 'auto-permissions2' into test-auto-permissions2

Browse source
This commit is contained in:
Brad Warren 2019-04-29 16:08:57 -07:00
commit 176e9583d8
7 changed files with 73 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
CHANGELOG.md
View file

@ -22,6 +22,15 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
`malformed` error to be received from the ACME server.
* Linode DNS plugin now supports api keys created from their new panel
at [cloud.linode.com](https://cloud.linode.com)
* Adding a warning noting that future versions of Certbot will automatically configure the
webserver so that all requests redirect to secure HTTPS access. You can control this
behavior and disable this warning with the --redirect and --no-redirect flags.
* certbot-auto now prints warnings when run as root with insecure file system
permissions. If you see these messages, you should fix the problem by
following the instructions at
https://community.letsencrypt.org/t/certbot-auto-deployment-best-practices/91979/,
however, these warnings can be disabled as necessary with the flag
--no-permissions-check.
### Fixed

5
certbot/client.py
View file

@ -549,6 +549,11 @@ class Client(object):
if ask_redirect:
if config_name == "redirect" and config_value is None:
config_value = enhancements.ask(enhancement_name)
if not config_value:
logger.warning("Future versions of Certbot will automatically "
"configure the webserver so that all requests redirect to secure "
"HTTPS access. You can control this behavior and disable this "
"warning with the --redirect and --no-redirect flags.")
if config_value:
self.apply_enhancement(domains, enhancement_name, option)
enhanced = True

15
certbot/tests/client_test.py
View file

@ -564,6 +564,21 @@ class EnhanceConfigTest(ClientTestCommon):
self.assertEqual(mock_log.warning.call_args[0][1],
'redirect')
@mock.patch("certbot.client.logger")
def test_config_set_no_warning_redirect(self, mock_log):
self.config.redirect = False
self._test_with_already_existing()
self.assertFalse(mock_log.warning.called)
@mock.patch("certbot.client.enhancements.ask")
@mock.patch("certbot.client.logger")
def test_warn_redirect(self, mock_log, mock_ask):
self.config.redirect = None
mock_ask.return_value = False
self._test_with_already_existing()
self.assertTrue(mock_log.warning.called)
self.assertTrue("disable" in mock_log.warning.call_args[0][0])
def test_no_ask_hsts(self):
self.config.hsts = True
self._test_with_all_supported()

29
letsencrypt-auto-source/letsencrypt-auto
View file

@ -1514,11 +1514,11 @@ 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.
If the absolute path of certbot-auto contains a symlink, it is not
handled specially and the symlink is followed. Due to this, there could
be permissions problems unreported by this script, however, issues like
this were not caused by our documentation and are ignored for the sake
of simplicity.
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
@ -1543,23 +1543,24 @@ def has_safe_permissions(path):
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 forum post which is /usr/local/bin. 1000
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 0-500 is reserved[2]. Debian recommends normal uids start at
uid 1000 and RHEL recommends uid 5000 to allow them to increase the
range used for system IDs in the future. It's possible that the
threshold of 1000 is too high, however, this seems unlikely and
avoids printing warnings when we're not confident there is a
problem.
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/7/html/system_administrators_guide/ch-managing_users_and_groups
[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
@ -1571,7 +1572,7 @@ def has_safe_permissions(path):
def main(certbot_auto_path):
current_path = os.path.abspath(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('/') == '/'.

29
letsencrypt-auto-source/pieces/check_permissions.py
View file

@ -5,11 +5,11 @@ 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.
If the absolute path of certbot-auto contains a symlink, it is not
handled specially and the symlink is followed. Due to this, there could
be permissions problems unreported by this script, however, issues like
this were not caused by our documentation and are ignored for the sake
of simplicity.
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
@ -34,23 +34,24 @@ def has_safe_permissions(path):
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 forum post which is /usr/local/bin. 1000
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 0-500 is reserved[2]. Debian recommends normal uids start at
uid 1000 and RHEL recommends uid 5000 to allow them to increase the
range used for system IDs in the future. It's possible that the
threshold of 1000 is too high, however, this seems unlikely and
avoids printing warnings when we're not confident there is a
problem.
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/7/html/system_administrators_guide/ch-managing_users_and_groups
[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
@ -62,7 +63,7 @@ def has_safe_permissions(path):
def main(certbot_auto_path):
current_path = os.path.abspath(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('/') == '/'.

4
letsencrypt-auto-source/tests/auto_test.py
View file

@ -192,7 +192,7 @@ def install_le_auto(contents, install_path):
chmod(install_path, S_IRUSR | S_IXUSR)
def run_le_auto(le_auto_path, venv_dir, base_url=None, le_auto_args_str=None, **kwargs):
def run_le_auto(le_auto_path, venv_dir, base_url=None, le_auto_args_str='--version', **kwargs):
"""Run the prebuilt version of letsencrypt-auto, returning stdout and
stderr strings.
@ -223,8 +223,6 @@ iQIDAQAB
env.update(d)
if le_auto_args_str is None:
le_auto_args_str = '--version'
return out_and_err(
le_auto_path + ' ' + le_auto_args_str,
shell=True,

14
tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh
View file

@ -9,7 +9,13 @@ set -eo pipefail
#private_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/local-ipv4)
cd letsencrypt
export PATH="$PWD/letsencrypt-auto-source:$PATH"
LE_AUTO_DIR="/usr/local/bin"
LE_AUTO_PATH="$LE_AUTO_DIR/letsencrypt-auto"
sudo cp letsencrypt-auto-source/letsencrypt-auto "$LE_AUTO_PATH"
sudo chown root "$LE_AUTO_PATH"
sudo chmod 0755 "$LE_AUTO_PATH"
export PATH="$LE_AUTO_DIR:$PATH"
letsencrypt-auto --os-packages-only --debug --version
# Create a venv-like layout at the old virtual environment path to test that a
@ -35,3 +41,9 @@ if ! letsencrypt-auto --help --no-self-upgrade | grep -F "letsencrypt-auto [SUBC
echo "letsencrypt-auto not included in help output!"
exit 1
fi
OUTPUT=$(letsencrypt-auto --install-only --no-self-upgrade --quiet 2>&1)
if [ -n "$OUTPUT" ]; then
echo letsencrypt-auto produced unexpected output!
exit 1
fi