crypt/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-01-26 07:41:33 +03:00
Code Activity

Preserve other-read bit on private keys too (#6544)

Browse Source 
Not updating the guidelines in using.rst-- I don't want to encourage people to use this. now that Certbot preserves gid, the better way to set permissions is to change the group permisisons!

* Preserve other-read bit on private keys too

* fix integration test

* fix and rename permission routines in integration tests
This commit is contained in:
sydneyli
2018-12-04 10:59:23 -08:00
committed by Brad Warren
parent f5aad1440f
commit 9c0b27de68
3 changed files with 31 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
certbot/storage.py
View File

@@ -1141,11 +1141,11 @@ class RenewableCert(object):
with util.safe_open(target["privkey"], "wb", chmod=BASE_PRIVKEY_MODE) as f:
logger.debug("Writing new private key to %s.", target["privkey"])
f.write(new_privkey)
# If the previous privkey in this lineage has an existing gid or group mode > 0,
# let's keep those.
group_mode = stat.S_IMODE(os.stat(old_privkey).st_mode) & \
(stat.S_IRGRP | stat.S_IWGRP | stat.S_IXGRP)
mode = BASE_PRIVKEY_MODE | group_mode
# Preserve gid and (mode & 074) from previous privkey in this lineage.
old_mode = stat.S_IMODE(os.stat(old_privkey).st_mode) & \
(stat.S_IRGRP | stat.S_IWGRP | stat.S_IXGRP | \
stat.S_IROTH)
mode = BASE_PRIVKEY_MODE | old_mode
os.chown(target["privkey"], -1, os.stat(old_privkey).st_gid)
os.chmod(target["privkey"], mode)

6
certbot/tests/storage_test.py
View File

@@ -562,12 +562,12 @@ class RenewableCertTests(BaseRenewableCertTest):
self.test_rc.save_successor(1, b"newcert", None, b"new chain", self.config)
self.assertTrue(compat.compare_file_modes(
os.stat(self.test_rc.version("privkey", 2)).st_mode, 0o444))
# If new key, permissions should be rest to 600 + preserved group
# 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(compat.compare_file_modes(
os.stat(self.test_rc.version("privkey", 3)).st_mode, 0o640))
os.stat(self.test_rc.version("privkey", 3)).st_mode, 0o644))
# If permissions reverted, next renewal will also revert permissions of new key
os.chmod(self.test_rc.version("privkey", 3), 0o404)
os.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(compat.compare_file_modes(
os.stat(self.test_rc.version("privkey", 4)).st_mode, 0o600))

35
tests/certbot-boulder-integration.sh
View File

@@ -280,13 +280,20 @@ CheckCertCount() {
fi
}
CheckGroup() {
group_mode() { echo $((0`stat -c %a $1` & 070)); }
group_owner() { echo `stat -c %G $1`; }
if [ `group_mode $1` -ne `group_mode $2` ] ; then
echo "Expected group permission `group_mode $1`, got `group_mode $2` on file $2"
CheckPermissions() {
# Args: <filepath_1> <filepath_2> <mask>
# Checks mode of two files match under <mask>
masked_mode() { echo $((0`stat -c %a $1` & 0$2)); }
if [ `masked_mode $1 $3` -ne `masked_mode $2 $3` ] ; then
echo "With $3 mask, expected mode `masked_mode $1 $3`, got `masked_mode $2 $3` on file $2"
exit 1
fi
}
CheckGID() {
# Args: <filepath_1> <filepath_2>
# Checks group owner of two files match
group_owner() { echo `stat -c %G $1`; }
if [ `group_owner $1` != `group_owner $2` ] ; then
echo "Expected group owner `group_owner $1`, got `group_owner $2` on file $2"
exit 1
@@ -294,9 +301,11 @@ CheckGroup() {
}
CheckOthersPermission() {
# Args: <filepath_1> <expected mode>
# Tests file's other/world permission against expected mode
other_permission=$((0`stat -c %a $1` & 07))
if [ $other_permission -ne 0 ] ; then
echo "Expected file $1 not to be accessible by others"
if [ $other_permission -ne $2 ] ; then
echo "Expected file $1 to have others mode $2, got $other_permission instead"
exit 1
fi
}
@@ -316,9 +325,10 @@ rm -rf "$renewal_hooks_root"
common renew --cert-name le.wtf --authenticator manual
CheckCertCount "le.wtf" 2
CheckOthersPermission "${root}/conf/archive/le.wtf/privkey1.pem"
CheckOthersPermission "${root}/conf/archive/le.wtf/privkey2.pem"
CheckGroup "${root}/conf/archive/le.wtf/privkey1.pem" "${root}/conf/archive/le.wtf/privkey2.pem"
CheckOthersPermission "${root}/conf/archive/le.wtf/privkey1.pem" 0
CheckOthersPermission "${root}/conf/archive/le.wtf/privkey2.pem" 0
CheckPermissions "${root}/conf/archive/le.wtf/privkey1.pem" "${root}/conf/archive/le.wtf/privkey2.pem" 074
CheckGID "${root}/conf/archive/le.wtf/privkey1.pem" "${root}/conf/archive/le.wtf/privkey2.pem"
chmod 0444 "${root}/conf/archive/le.wtf/privkey2.pem"
# test renewal with no executables in hook directories
@@ -337,8 +347,9 @@ CreateDirHooks
sed -i "4arenew_before_expiry = 4 years" "$root/conf/renewal/le.wtf.conf"
common_no_force_renew renew --rsa-key-size 2048 --no-directory-hooks
CheckCertCount "le.wtf" 3
CheckGroup "${root}/conf/archive/le.wtf/privkey2.pem" "${root}/conf/archive/le.wtf/privkey3.pem"
CheckOthersPermission "${root}/conf/archive/le.wtf/privkey3.pem"
CheckGID "${root}/conf/archive/le.wtf/privkey2.pem" "${root}/conf/archive/le.wtf/privkey3.pem"
CheckPermissions "${root}/conf/archive/le.wtf/privkey2.pem" "${root}/conf/archive/le.wtf/privkey3.pem" 074
CheckOthersPermission "${root}/conf/archive/le.wtf/privkey3.pem" 04
if [ -s "$HOOK_DIRS_TEST" ]; then
echo "Directory hooks were executed with --no-directory-hooks!" >&2