From f16621a38b17d53c76600c12d67031544c055d74 Mon Sep 17 00:00:00 2001 From: Fred Miller Date: Sun, 21 Oct 2018 00:05:24 +0800 Subject: Make private keys readable only by the owner --- mitmproxy/certs.py | 20 ++++++++++++++++++-- test/mitmproxy/test_certs.py | 7 +++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/mitmproxy/certs.py b/mitmproxy/certs.py index 58aea6d5..8b8ba6f2 100644 --- a/mitmproxy/certs.py +++ b/mitmproxy/certs.py @@ -5,6 +5,7 @@ import datetime import ipaddress import sys import typing +import contextlib from pyasn1.type import univ, constraint, char, namedtype, tag from pyasn1.codec.der.decoder import decode @@ -195,6 +196,21 @@ class CertStore: dh = cls.load_dhparam(dh_path) return cls(key, ca, ca_path, dh) + @staticmethod + @contextlib.contextmanager + def umask_secret(): + """ + Context to temporarily set umask to its original value bitor 0o77. + Useful when writing private keys to disk so that only the owner + will be able to read them. + """ + original_umask = os.umask(0) + os.umask(original_umask | 0o77) + try: + yield + finally: + os.umask(original_umask) + @staticmethod def create_store(path, basename, o=None, cn=None, expiry=DEFAULT_EXP): if not os.path.exists(path): @@ -205,7 +221,7 @@ class CertStore: key, ca = create_ca(o=o, cn=cn, exp=expiry) # Dump the CA plus private key - with open(os.path.join(path, basename + "-ca.pem"), "wb") as f: + with CertStore.umask_secret(), open(os.path.join(path, basename + "-ca.pem"), "wb") as f: f.write( OpenSSL.crypto.dump_privatekey( OpenSSL.crypto.FILETYPE_PEM, @@ -236,7 +252,7 @@ class CertStore: f.write(p12.export()) # Dump the certificate and key in a PKCS12 format for Windows devices - with open(os.path.join(path, basename + "-ca.p12"), "wb") as f: + with CertStore.umask_secret(), open(os.path.join(path, basename + "-ca.p12"), "wb") as f: p12 = OpenSSL.crypto.PKCS12() p12.set_certificate(ca) p12.set_privatekey(key) diff --git a/test/mitmproxy/test_certs.py b/test/mitmproxy/test_certs.py index 12d3dc96..82b9cdbb 100644 --- a/test/mitmproxy/test_certs.py +++ b/test/mitmproxy/test_certs.py @@ -111,6 +111,13 @@ class TestCertStore: certs.CertStore.load_dhparam(filename) assert os.path.exists(filename) + def test_umask_secret(self, tmpdir): + filename = str(tmpdir.join("secret")) + with certs.CertStore.umask_secret(), open(filename, "wb"): + pass + # TODO: How do we actually attempt to read that file as another user? + assert os.stat(filename).st_mode & 0o77 == 0 + class TestDummyCert: -- cgit v1.2.3 From bf3570b3b987c27701df65753666a83b52ba8752 Mon Sep 17 00:00:00 2001 From: Fred Miller Date: Tue, 23 Oct 2018 08:28:09 +0800 Subject: Skip file permission test on Windows --- test/mitmproxy/test_certs.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/mitmproxy/test_certs.py b/test/mitmproxy/test_certs.py index 82b9cdbb..884927cb 100644 --- a/test/mitmproxy/test_certs.py +++ b/test/mitmproxy/test_certs.py @@ -1,4 +1,6 @@ import os +import sys +import pytest from mitmproxy import certs # class TestDNTree: @@ -111,6 +113,8 @@ class TestCertStore: certs.CertStore.load_dhparam(filename) assert os.path.exists(filename) + @pytest.mark.skipif(sys.platform in ["win32", "cygwin"], + reason="Unix file permissions are not applicable on Windows") def test_umask_secret(self, tmpdir): filename = str(tmpdir.join("secret")) with certs.CertStore.umask_secret(), open(filename, "wb"): -- cgit v1.2.3 From 28551e96552bc3077d772f8459ddb74e9c0f5253 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Tue, 23 Oct 2018 15:24:59 +0200 Subject: use skip_windows decorator consistently --- test/mitmproxy/test_certs.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/mitmproxy/test_certs.py b/test/mitmproxy/test_certs.py index 884927cb..8421ec58 100644 --- a/test/mitmproxy/test_certs.py +++ b/test/mitmproxy/test_certs.py @@ -1,7 +1,6 @@ import os -import sys -import pytest from mitmproxy import certs +from ..conftest import skip_windows # class TestDNTree: # def test_simple(self): @@ -113,8 +112,7 @@ class TestCertStore: certs.CertStore.load_dhparam(filename) assert os.path.exists(filename) - @pytest.mark.skipif(sys.platform in ["win32", "cygwin"], - reason="Unix file permissions are not applicable on Windows") + @skip_windows def test_umask_secret(self, tmpdir): filename = str(tmpdir.join("secret")) with certs.CertStore.umask_secret(), open(filename, "wb"): -- cgit v1.2.3