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

Allow uid/gid < 1000.

Browse source
This commit is contained in:
Brad Warren 2019-04-26 16:02:23 -07:00
parent b4c6bf208e
commit b58b683cf5
3 changed files with 42 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
letsencrypt-auto-source/letsencrypt-auto
View file

@ -1505,9 +1505,9 @@ else
"""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 0
and if other users can modify the file, it prints a warning with a
suggestion on how to solve the problem.
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
@ -1534,7 +1534,21 @@ 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 0.
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 forum post 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.
[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
:param str path: filesystem path to check
:returns: True if the path has secure permissions, otherwise, False
@ -1544,9 +1558,9 @@ def has_safe_permissions(path):
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 != 0:
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 != 0:
if stat_result.st_mode & stat.S_IWUSR and stat_result.st_uid >= 1000:
return False
return True

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

@ -1,9 +1,9 @@
"""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 0
and if other users can modify the file, it prints a warning with a
suggestion on how to solve the problem.
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
@ -30,7 +30,21 @@ 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 0.
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 forum post 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.
[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
:param str path: filesystem path to check
:returns: True if the path has secure permissions, otherwise, False
@ -40,9 +54,9 @@ def has_safe_permissions(path):
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 != 0:
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 != 0:
if stat_result.st_mode & stat.S_IWUSR and stat_result.st_uid >= 1000:
return False
return True

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

@ -455,13 +455,13 @@ class AutoTests(TestCase):
self.assertTrue('insecure permissions' in out)
# Test group permissions
if stat_result.st_gid != 0:
if stat_result.st_gid >= 1000:
chmod(path, original_mode | S_IWGRP)
out, _ = run_le_auto_func()
self.assertTrue('insecure permissions' in out)
# Test owner permissions
if stat_result.st_uid != 0:
if stat_result.st_uid >= 1000:
chmod(path, original_mode | S_IWUSR)
out, _ = run_le_auto_func()
self.assertTrue('insecure permissions' in out)