From 7aa749174bd404e6efde358d09e84594048c769b Mon Sep 17 00:00:00 2001
From: Jakub Warmuz <jakub@warmuz.org>
Date: Thu, 9 Jul 2015 11:17:22 +0000
Subject: [PATCH] Fix achall response key chmods security bug.

---
 letsencrypt/plugins/common.py      |  6 +++---
 letsencrypt/plugins/common_test.py | 26 +++++++++++++++-----------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py
index cd148fa5e..460af1b15 100644
--- a/letsencrypt/plugins/common.py
+++ b/letsencrypt/plugins/common.py
@@ -11,6 +11,7 @@ from acme.jose import util as jose_util
 
 from letsencrypt import constants
 from letsencrypt import interfaces
+from letsencrypt import le_util
 
 
 def option_namespace(name):
@@ -173,7 +174,7 @@ class Dvsni(object):
         cert_pem, response = achall.gen_cert_and_response(s)
 
         # Write out challenge cert
-        with open(cert_path, "w") as cert_chall_fd:
+        with open(cert_path, "wb") as cert_chall_fd:
             cert_chall_fd.write(cert_pem)
 
         key_path = self.get_key_path(achall)
@@ -181,8 +182,7 @@ class Dvsni(object):
             encoding=serialization.Encoding.PEM,
             format=serialization.PrivateFormat.TraditionalOpenSSL,
             encryption_algorithm=serialization.NoEncryption())
-        # XXX: key_file chmods!  (SEC)
-        with open(key_path, 'w') as key_file:
+        with le_util.safe_open(key_path, 'wb', chmod=0o400) as key_file:
             key_file.write(key_pem)
         self.configurator.reverter.register_file_creation(True, key_path)
 
diff --git a/letsencrypt/plugins/common_test.py b/letsencrypt/plugins/common_test.py
index 340cfcf0d..8688c36b1 100644
--- a/letsencrypt/plugins/common_test.py
+++ b/letsencrypt/plugins/common_test.py
@@ -150,24 +150,28 @@ class DvsniTest(unittest.TestCase):
         # open context managers more elegantly. It avoids dealing with
         # __enter__ and __exit__ calls.
         # http://www.voidspace.org.uk/python/mock/helpers.html#mock.mock_open
-        m_open = mock.mock_open()
+        mock_open, mock_safe_open = mock.mock_open(), mock.mock_open()
 
         response = challenges.DVSNIResponse(s="randomS1")
         achall = mock.MagicMock(nonce=self.achalls[0].nonce,
                                 nonce_domain=self.achalls[0].nonce_domain)
         achall.gen_cert_and_response.return_value = ("pem", response)
 
-        with mock.patch("letsencrypt.plugins.common.open", m_open, create=True):
-            # pylint: disable=protected-access
-            self.assertEqual(response, self.sni._setup_challenge_cert(
-                achall, "randomS1"))
+        with mock.patch("letsencrypt.plugins.common.open",
+                        mock_open, create=True):
+            with mock.patch("letsencrypt.plugins.common.le_util.safe_open",
+                            mock_safe_open):
+                # pylint: disable=protected-access
+                self.assertEqual(response, self.sni._setup_challenge_cert(
+                    achall, "randomS1"))
 
-            self.assertTrue(m_open.called)
-            self.assertEqual(
-                m_open.call_args[0], (self.sni.get_cert_path(achall), "w"))
-            self.assertEqual(m_open().write.mock_calls[0][1][0], "pem")
-            self.assertEqual(m_open().write.mock_calls[1][1][0],
-                             achall.key.key.private_bytes())
+        # pylint: disable=no-member
+        mock_open.assert_called_once_with(self.sni.get_cert_path(achall), "wb")
+        mock_open.return_value.write.assert_called_once_with("pem")
+        mock_safe_open.assert_called_once_with(
+            self.sni.get_key_path(achall), "wb", chmod=0o400)
+        mock_safe_open.return_value.write.assert_called_once_with(
+            achall.key.key.private_bytes())
 
 
 if __name__ == "__main__":