From 71451dd54b6d2e7eab6ba8f075240a13b303e86e Mon Sep 17 00:00:00 2001 From: Zach Shepherd Date: Thu, 11 May 2017 15:49:34 -0700 Subject: [PATCH] security: preserve permissions on renewal conf (#4430) Ensure that permissions are preserved when renewal data is written to conf files. This allows users to limit access to the file, if they wish. Testing done: * `tox -e py27` * `tox -e lint` * Manual Testing * Got a new certificate. Restricted the permissions on the renewal conf. Renewed the certificate. Verified that the new renewal conf permissions matched. --- certbot/storage.py | 11 +++++++++++ certbot/tests/storage_test.py | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/certbot/storage.py b/certbot/storage.py index c35a268e3..4f167d4ea 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -4,6 +4,7 @@ import glob import logging import os import re +import stat import configobj import parsedatetime @@ -117,10 +118,20 @@ def write_renewal_config(o_filename, n_filename, archive_dir, target, relevant_d # TODO: add human-readable comments explaining other available # parameters logger.debug("Writing new config %s.", n_filename) + + # Ensure that the file exists + open(n_filename, 'a').close() + + # Copy permissions from the old version of the file, if it exists. + if os.path.exists(o_filename): + current_permissions = stat.S_IMODE(os.lstat(o_filename).st_mode) + os.chmod(n_filename, current_permissions) + with open(n_filename, "wb") as f: config.write(outfile=f) return config + def rename_renewal_config(prev_name, new_name, cli_config): """Renames cli_config.certname's config to cli_config.new_certname. diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index d8fe98536..e6e2b25ff 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -3,6 +3,7 @@ import datetime import os import shutil +import stat import unittest import configobj @@ -758,13 +759,16 @@ class RenewableCertTests(BaseRenewableCertTest): with open(temp, "w") as f: f.write("[renewalparams]\nuseful = value # A useful value\n" "useless = value # Not needed\n") + os.chmod(temp, 0o640) target = {} for x in ALL_FOUR: target[x] = "somewhere" archive_dir = "the_archive" relevant_data = {"useful": "new_value"} + from certbot import storage storage.write_renewal_config(temp, temp2, archive_dir, target, relevant_data) + with open(temp2, "r") as f: content = f.read() # useful value was updated @@ -775,6 +779,9 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertTrue("useless" not in content) # check version was stored self.assertTrue("version = {0}".format(certbot.__version__) in content) + # ensure permissions are copied + self.assertEqual(stat.S_IMODE(os.lstat(temp).st_mode), + stat.S_IMODE(os.lstat(temp2).st_mode)) def test_update_symlinks(self): from certbot import storage