diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index bfc7b6a67..503599423 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -405,7 +405,8 @@ class NginxConfigurator(common.Plugin): cert = acme_crypto_util.gen_ss_cert(key, domains=[socket.gethostname()]) cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, cert) - cert_file, cert_path = util.unique_file(os.path.join(tmp_dir, "cert.pem")) + cert_file, cert_path = util.unique_file( + os.path.join(tmp_dir, "cert.pem"), mode="wb") with cert_file: cert_file.write(cert_pem) return cert_path, le_key.file diff --git a/certbot/client.py b/certbot/client.py index 55f3d5e67..880cfe7df 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -322,7 +322,7 @@ class Client(object): self.config.strict_permissions) cert_pem = OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped).decode('ascii') + OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path) @@ -595,10 +595,10 @@ def _open_pem_file(cli_arg_path, pem_path): """ if cli.set_by_cli(cli_arg_path): - return util.safe_open(pem_path, chmod=0o644),\ + return util.safe_open(pem_path, chmod=0o644, mode="wb"),\ os.path.abspath(pem_path) else: - uniq = util.unique_file(pem_path, 0o644) + uniq = util.unique_file(pem_path, 0o644, "wb") return uniq[0], os.path.abspath(uniq[1]) def _save_chain(chain_pem, chain_file): diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 7253742b0..65e3de345 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -53,7 +53,8 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): # Save file util.make_or_verify_dir(key_dir, 0o700, os.geteuid(), config.strict_permissions) - key_f, key_path = util.unique_file(os.path.join(key_dir, keyname), 0o600) + key_f, key_path = util.unique_file( + os.path.join(key_dir, keyname), 0o600, "wb") with key_f: key_f.write(key_pem) @@ -85,7 +86,7 @@ def init_save_csr(privkey, names, path, csrname="csr-certbot.pem"): util.make_or_verify_dir(path, 0o755, os.geteuid(), config.strict_permissions) csr_f, csr_filename = util.unique_file( - os.path.join(path, csrname), 0o644) + os.path.join(path, csrname), 0o644, "wb") csr_f.write(csr_pem) csr_f.close() @@ -351,11 +352,11 @@ def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM): if isinstance(cert, jose.ComparableX509): # pylint: disable=protected-access cert = cert.wrapped - return OpenSSL.crypto.dump_certificate(filetype, cert).decode('ascii') + return OpenSSL.crypto.dump_certificate(filetype, cert) # assumes that OpenSSL.crypto.dump_certificate includes ending # newline character - return "".join(_dump_cert(cert) for cert in chain) + return b"".join(_dump_cert(cert) for cert in chain) def notBefore(cert_path): diff --git a/certbot/storage.py b/certbot/storage.py index 2134cd90b..5d7eeb88a 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -817,17 +817,17 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes for kind in ALL_FOUR: os.symlink(os.path.join(archive, kind + "1.pem"), target[kind]) - with open(target["cert"], "w") as f: + with open(target["cert"], "wb") as f: logger.debug("Writing certificate to %s.", target["cert"]) f.write(cert) - with open(target["privkey"], "w") as f: + with open(target["privkey"], "wb") as f: logger.debug("Writing private key to %s.", target["privkey"]) f.write(privkey) # XXX: Let's make sure to get the file permissions right here - with open(target["chain"], "w") as f: + with open(target["chain"], "wb") as f: logger.debug("Writing chain to %s.", target["chain"]) f.write(chain) - with open(target["fullchain"], "w") as f: + with open(target["fullchain"], "wb") as f: # assumes that OpenSSL.crypto.dump_certificate includes # ending newline character logger.debug("Writing full chain to %s.", target["fullchain"]) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index c0dc1de3a..4832e2869 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -40,9 +40,9 @@ class InitSaveKeyTest(unittest.TestCase): @mock.patch('certbot.crypto_util.make_key') def test_success(self, mock_make): - mock_make.return_value = 'key_pem' + mock_make.return_value = b'key_pem' key = self._call(1024, self.key_dir) - self.assertEqual(key.pem, 'key_pem') + self.assertEqual(key.pem, b'key_pem') self.assertTrue('key-certbot.pem' in key.file) @mock.patch('certbot.crypto_util.make_key') @@ -67,13 +67,13 @@ class InitSaveCSRTest(unittest.TestCase): def test_it(self, unused_mock_verify, mock_csr): from certbot.crypto_util import init_save_csr - mock_csr.return_value = ('csr_pem', 'csr_der') + mock_csr.return_value = (b'csr_pem', b'csr_der') csr = init_save_csr( mock.Mock(pem='dummy_key'), 'example.com', self.csr_dir, 'csr-certbot.pem') - self.assertEqual(csr.data, 'csr_der') + self.assertEqual(csr.data, b'csr_der') self.assertTrue('csr-certbot.pem' in csr.file) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index bfbcd885e..fb33a1864 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -573,18 +573,18 @@ class RenewableCertTests(BaseRenewableCertTest): from certbot import storage result = storage.RenewableCert.new_lineage( - "the-lineage.com", "cert", "privkey", "chain", self.cli_config) + "the-lineage.com", b"cert", b"privkey", b"chain", self.cli_config) # This consistency check tests most relevant properties about the # newly created cert lineage. # pylint: disable=protected-access self.assertTrue(result._consistent()) self.assertTrue(os.path.exists(os.path.join( self.cli_config.renewal_configs_dir, "the-lineage.com.conf"))) - with open(result.fullchain) as f: - self.assertEqual(f.read(), "cert" + "chain") + with open(result.fullchain, "rb") as f: + self.assertEqual(f.read(), b"cert" + b"chain") # Let's do it again and make sure it makes a different lineage result = storage.RenewableCert.new_lineage( - "the-lineage.com", "cert2", "privkey2", "chain2", self.cli_config) + "the-lineage.com", b"cert2", b"privkey2", b"chain2", self.cli_config) self.assertTrue(os.path.exists(os.path.join( self.cli_config.renewal_configs_dir, "the-lineage.com-0001.conf"))) # Now trigger the detection of already existing files @@ -592,15 +592,15 @@ class RenewableCertTests(BaseRenewableCertTest): self.cli_config.live_dir, "the-lineage.com-0002")) self.assertRaises(errors.CertStorageError, storage.RenewableCert.new_lineage, "the-lineage.com", - "cert3", "privkey3", "chain3", self.cli_config) + b"cert3", b"privkey3", b"chain3", self.cli_config) os.mkdir(os.path.join(self.cli_config.default_archive_dir, "other-example.com")) self.assertRaises(errors.CertStorageError, storage.RenewableCert.new_lineage, - "other-example.com", "cert4", - "privkey4", "chain4", self.cli_config) + "other-example.com", b"cert4", + b"privkey4", b"chain4", self.cli_config) # Make sure it can accept renewal parameters result = storage.RenewableCert.new_lineage( - "the-lineage.com", "cert2", "privkey2", "chain2", self.cli_config) + "the-lineage.com", b"cert2", b"privkey2", b"chain2", self.cli_config) # TODO: Conceivably we could test that the renewal parameters actually # got saved @@ -617,7 +617,7 @@ class RenewableCertTests(BaseRenewableCertTest): shutil.rmtree(self.cli_config.live_dir) storage.RenewableCert.new_lineage( - "the-lineage.com", "cert2", "privkey2", "chain2", self.cli_config) + "the-lineage.com", b"cert2", b"privkey2", b"chain2", self.cli_config) self.assertTrue(os.path.exists( os.path.join( self.cli_config.renewal_configs_dir, "the-lineage.com.conf"))) diff --git a/certbot/util.py b/certbot/util.py index f3a74d47d..220795237 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -151,11 +151,11 @@ def safe_open(path, mode="w", chmod=None, buffering=None): mode, *fdopen_args) -def _unique_file(path, filename_pat, count, mode): +def _unique_file(path, filename_pat, count, chmod, mode): while True: current_path = os.path.join(path, filename_pat(count)) try: - return safe_open(current_path, chmod=mode),\ + return safe_open(current_path, chmod=chmod, mode=mode),\ os.path.abspath(current_path) except OSError as err: # "File exists," is okay, try a different name. @@ -164,11 +164,12 @@ def _unique_file(path, filename_pat, count, mode): count += 1 -def unique_file(path, mode=0o777): +def unique_file(path, chmod=0o777, mode="w"): """Safely finds a unique file. :param str path: path/filename.ext - :param int mode: File mode + :param int chmod: File mode + :param str mode: Open mode :returns: tuple of file object and file name @@ -176,15 +177,16 @@ def unique_file(path, mode=0o777): path, tail = os.path.split(path) return _unique_file( path, filename_pat=(lambda count: "%04d_%s" % (count, tail)), - count=0, mode=mode) + count=0, chmod=chmod, mode=mode) -def unique_lineage_name(path, filename, mode=0o777): +def unique_lineage_name(path, filename, chmod=0o777, mode="w"): """Safely finds a unique file using lineage convention. :param str path: directory path :param str filename: proposed filename - :param int mode: file mode + :param int chmod: file mode + :param str mode: open mode :returns: tuple of file object and file name (which may be modified from the requested one by appending digits to ensure uniqueness) @@ -196,13 +198,13 @@ def unique_lineage_name(path, filename, mode=0o777): """ preferred_path = os.path.join(path, "%s.conf" % (filename)) try: - return safe_open(preferred_path, chmod=mode), preferred_path + return safe_open(preferred_path, chmod=chmod), preferred_path except OSError as err: if err.errno != errno.EEXIST: raise return _unique_file( path, filename_pat=(lambda count: "%s-%04d.conf" % (filename, count)), - count=1, mode=mode) + count=1, chmod=chmod, mode=mode) def safely_remove(path):