From 8c65bdcf9dc69ff1d8317f59c497f1f24e867bb2 Mon Sep 17 00:00:00 2001 From: Zooko O'Whielacronx Date: Mon, 17 Dec 2007 16:39:54 -0700 Subject: [PATCH] put all private state in $BASEDIR/private fixes #219 The only part of #219 that this doesn't include is the part about logpublisher, which has been moved out of tahoe into foolscap. --- src/allmydata/client.py | 10 ++---- src/allmydata/node.py | 55 +++++++++++++++++++----------- src/allmydata/test/check_memory.py | 2 +- src/allmydata/test/check_speed.py | 2 +- src/allmydata/test/test_client.py | 4 +-- src/allmydata/test/test_node.py | 22 +++++++++++- src/allmydata/test/test_system.py | 4 +-- src/allmydata/test/test_web.py | 8 +++-- src/allmydata/util/fileutil.py | 35 ++++--------------- src/allmydata/webish.py | 3 +- 10 files changed, 77 insertions(+), 68 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index fe07ec72..61c7983f 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -66,7 +66,7 @@ class Client(node.Node, Referenceable, testutil.PollMixin): def _init_start_page(self, privdiruri): ws = self.getServiceNamed("webish") - startfile = os.path.join(self.basedir, "start.html") + startfile = os.path.join(self.basedir, "private", "start.html") nodeurl_file = os.path.join(self.basedir, "node.url") return ws.create_start_html(privdiruri, startfile, nodeurl_file) @@ -82,8 +82,7 @@ class Client(node.Node, Referenceable, testutil.PollMixin): def init_secret(self): def make_secret(): return idlib.b2a(os.urandom(16)) + "\n" - secret_s = self.get_or_create_config("secret", make_secret, - filemode=0600) + secret_s = self.get_or_create_private_config("secret", make_secret) self._secret = idlib.a2b(secret_s) def init_storage(self): @@ -206,10 +205,7 @@ class Client(node.Node, Referenceable, testutil.PollMixin): c = ControlServer() c.setServiceParent(self) control_url = self.tub.registerReference(c) - control_furl_file = os.path.join(self.basedir, "control.furl") - open(control_furl_file, "w").write(control_url + "\n") - os.chmod(control_furl_file, 0600) - + self.write_private_config("control.furl", control_url + "\n") def remote_get_versions(self): return str(allmydata.__version__), str(self.OLDEST_SUPPORTED_VERSION) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index e955849c..29e5b24a 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -8,7 +8,7 @@ from twisted.internet import defer, reactor from foolscap import Tub, eventual from allmydata import get_package_versions_string from allmydata.util import log as tahoe_log -from allmydata.util import iputil, observer, humanreadable +from allmydata.util import fileutil, iputil, observer, humanreadable from allmydata.util.assertutil import precondition # Just to get their versions: @@ -36,6 +36,12 @@ def formatTimeTahoeStyle(self, when): else: return d.isoformat(" ") + ".000Z" +PRIV_README=""" +This directory contains files which contain private data for the Tahoe node, +such as private keys. On Unix-like systems, the permissions on this directory +are set to disallow users other than its owner from reading the contents of +the files. See the 'configuration.txt' documentation file for details.""" + class Node(service.MultiService): # this implements common functionality of both Client nodes and Introducer # nodes. @@ -48,9 +54,10 @@ class Node(service.MultiService): service.MultiService.__init__(self) self.basedir = os.path.abspath(basedir) self._tub_ready_observerlist = observer.OneShotObserverList() - certfile = os.path.join(self.basedir, self.CERTFILE) + fileutil.make_dirs(os.path.join(self.basedir, "private"), 0700) + open(os.path.join(self.basedir, "private", "README"), "w").write(PRIV_README) + certfile = os.path.join(self.basedir, "private", self.CERTFILE) self.tub = Tub(certFile=certfile) - os.chmod(certfile, 0600) self.tub.setOption("logLocalFailures", True) self.tub.setOption("logRemoteFailures", True) self.nodeid = b32decode(self.tub.tubID.upper()) # binary format @@ -83,41 +90,49 @@ class Node(service.MultiService): self.log("Node constructed. " + get_package_versions_string()) iputil.increase_rlimits() - def get_config(self, name, mode="r", required=False): + def get_config(self, name, required=False): """Get the (string) contents of a config file, or None if the file did not exist. If required=True, raise an exception rather than returning None. Any leading or trailing whitespace will be stripped from the data.""" fn = os.path.join(self.basedir, name) try: - return open(fn, mode).read().strip() + return open(fn, "r").read().strip() except EnvironmentError: if not required: return None raise - def get_or_create_config(self, name, default_fn, mode="w", filemode=None): - """Try to get the (string) contents of a config file, and return it. - Any leading or trailing whitespace will be stripped from the data. + def write_private_config(self, name, value): + """Write the (string) contents of a private config file (which is a + config file that resides within the subdirectory named 'private'), and + return it. Any leading or trailing whitespace will be stripped from + the data. + """ + privname = os.path.join(self.basedir, "private", name) + open(privname, "w").write(value.strip()) + + def get_or_create_private_config(self, name, default): + """Try to get the (string) contents of a private config file (which + is a config file that resides within the subdirectory named + 'private'), and return it. Any leading or trailing whitespace will be + stripped from the data. - If the file does not exist, try to create it using default_fn, and - then return the value that was written. If 'default_fn' is a string, + If the file does not exist, try to create it using default, and + then return the value that was written. If 'default' is a string, use it as a default value. If not, treat it as a 0-argument callable which is expected to return a string. """ - value = self.get_config(name) + privname = os.path.join("private", name) + value = self.get_config(privname) if value is None: - if isinstance(default_fn, (str, unicode)): - value = default_fn + if isinstance(default, (str, unicode)): + value = default else: - value = default_fn() - fn = os.path.join(self.basedir, name) + value = default() + fn = os.path.join(self.basedir, privname) try: - f = open(fn, mode) - f.write(value) - f.close() - if filemode is not None: - os.chmod(fn, filemode) + open(fn, "w").write(value) except EnvironmentError, e: self.log("Unable to write config file '%s'" % fn) self.log(e) diff --git a/src/allmydata/test/check_memory.py b/src/allmydata/test/check_memory.py index 460ca315..e45fabb3 100644 --- a/src/allmydata/test/check_memory.py +++ b/src/allmydata/test/check_memory.py @@ -270,7 +270,7 @@ this file are ignored. # now we wait for the client to get started. we're looking for the # control.furl file to appear. - furl_file = os.path.join(clientdir, "control.furl") + furl_file = os.path.join(clientdir, "private", "control.furl") def _check(): if pp.ended and pp.ended.value.status != 0: # the twistd process ends normally (with rc=0) if the child diff --git a/src/allmydata/test/check_speed.py b/src/allmydata/test/check_speed.py index c9196cd4..a382bf4a 100644 --- a/src/allmydata/test/check_speed.py +++ b/src/allmydata/test/check_speed.py @@ -12,7 +12,7 @@ class SpeedTest: def __init__(self, test_client_dir): #self.real_stderr = sys.stderr log.startLogging(open("st.log", "a"), setStdout=False) - f = open(os.path.join(test_client_dir, "control.furl"), "r") + f = open(os.path.join(test_client_dir, "private", "control.furl"), "r") self.control_furl = f.read().strip() f.close() self.base_service = service.MultiService() diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index f199bcdd..36b2eef7 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -36,8 +36,8 @@ class Basic(unittest.TestCase): open(os.path.join(basedir, "introducer.furl"), "w").write("") open(os.path.join(basedir, "vdrive.furl"), "w").write("") c = client.Client(basedir) - secret_file = os.path.join(basedir, "secret") - self.failUnless(os.path.exists(secret_file)) + secret_fname = os.path.join(basedir, "private", "secret") + self.failUnless(os.path.exists(secret_fname), secret_fname) renew_secret = c.get_renewal_secret() self.failUnless(idlib.b2a(renew_secret)) cancel_secret = c.get_cancel_secret() diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index 352ae6ba..5e8182af 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -1,5 +1,5 @@ -import os, time +import os, stat, sys, time from twisted.trial import unittest from twisted.internet import defer from twisted.python import log @@ -55,3 +55,23 @@ class TestCase(unittest.TestCase, testutil.SignalMixin): self.failUnless("Z" in t) t2 = formatTimeTahoeStyle("ignored", int(time.time())) self.failUnless("Z" in t2) + + def test_secrets_dir(self): + basedir = "test_node/test_secrets_dir" + fileutil.make_dirs(basedir) + n = TestNode(basedir) + self.failUnless(os.path.exists(os.path.join(basedir, "private"))) + + def test_secrets_dir_protected(self): + if "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower(): + # We don't know how to test that unprivileged users can't read this + # thing. (Also we don't know exactly how to set the permissions so + # that unprivileged users can't read this thing.) + raise unittest.SkipTest("We don't know how to set permissions on Windows.") + basedir = "test_node/test_secrets_dir_protected" + fileutil.make_dirs(basedir) + n = TestNode(basedir) + privdir = os.path.join(basedir, "private") + st = os.stat(privdir) + bits = stat.S_IMODE(st[stat.ST_MODE]) + self.failUnless(bits & 0001 == 0, bits) diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py index 2d807b94..e9e7ad50 100644 --- a/src/allmydata/test/test_system.py +++ b/src/allmydata/test/test_system.py @@ -917,7 +917,7 @@ class SystemTest(testutil.SignalMixin, unittest.TestCase): def _test_web_start(self, res): basedir = self.clients[0].basedir - startfile = os.path.join(basedir, "start.html") + startfile = os.path.join(basedir, "private", "start.html") self.failUnless(os.path.exists(startfile)) start_html = open(startfile, "r").read() self.failUnless(self.webish_url in start_html) @@ -978,7 +978,7 @@ class SystemTest(testutil.SignalMixin, unittest.TestCase): # exercise the remote-control-the-client foolscap interfaces in # allmydata.control (mostly used for performance tests) c0 = self.clients[0] - control_furl_file = os.path.join(c0.basedir, "control.furl") + control_furl_file = os.path.join(c0.basedir, "private", "control.furl") control_furl = open(control_furl_file, "r").read().strip() # it doesn't really matter which Tub we use to connect to the client, # so let's just use our IntroducerNode's diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index 45163c04..c878383e 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -251,15 +251,16 @@ class Web(WebMixin, unittest.TestCase): self.s.basedir = 'web/test_welcome' fileutil.make_dirs("web/test_welcome") + fileutil.make_dirs("web/test_welcome/private") self.ws.create_start_html("private_uri", - "web/test_welcome/start.html", + "web/test_welcome/private/start.html", "web/test_welcome/node.url") return self.GET("/") d.addCallback(_check) def _check2(res): self.failUnless('To view your personal private non-shared' in res) self.failUnless('from your local filesystem:' in res) - self.failUnless(os.path.abspath('web/test_welcome/start.html') + self.failUnless(os.path.abspath('web/test_welcome/private/start.html') in res) d.addCallback(_check2) return d @@ -324,7 +325,8 @@ class Web(WebMixin, unittest.TestCase): def test_start_html(self): fileutil.make_dirs("web") - startfile = "web/start.html" + fileutil.make_dirs("web/private") + startfile = "web/private/start.html" nodeurlfile = "web/node.url" self.ws.create_start_html("private_uri", startfile, nodeurlfile) diff --git a/src/allmydata/util/fileutil.py b/src/allmydata/util/fileutil.py index ac2391a9..9b3ef9a6 100644 --- a/src/allmydata/util/fileutil.py +++ b/src/allmydata/util/fileutil.py @@ -96,21 +96,13 @@ class NamedTemporaryDirectory: if self.cleanup and hasattr(self, 'name'): rm_dir(self.name) -def make_dirs(dirname, mode=0777, strictmode=False): +def make_dirs(dirname, mode=0777): """ - A threadsafe and idempotent version of os.makedirs(). If the dir already - exists, do nothing and return without raising an exception. If this call - creates the dir, return without raising an exception. If there is an - error that prevents creation or if the directory gets deleted after - make_dirs() creates it and before make_dirs() checks that it exists, raise - an exception. - - @param strictmode if true, then make_dirs() will raise an exception if the - directory doesn't have the desired mode. For example, if the - directory already exists, and has a different mode than the one - specified by the mode parameter, then if strictmode is true, - make_dirs() will raise an exception, else it will ignore the - discrepancy. + An idempotent version of os.makedirs(). If the dir already exists, do + nothing and return without raising an exception. If this call creates the + dir, return without raising an exception. If there is an error that + prevents creation or if the directory gets deleted after make_dirs() creates + it and before make_dirs() checks that it exists, raise an exception. """ tx = None try: @@ -123,21 +115,6 @@ def make_dirs(dirname, mode=0777, strictmode=False): raise tx raise exceptions.IOError, "unknown error prevented creation of directory, or deleted the directory immediately after creation: %s" % dirname # careful not to construct an IOError with a 2-tuple, as that has a special meaning... - tx = None - if hasattr(os, 'chmod'): - try: - os.chmod(dirname, mode) - except OSError, x: - tx = x - - if strictmode and hasattr(os, 'stat'): - s = os.stat(dirname) - resmode = stat.S_IMODE(s.st_mode) - if resmode != mode: - if tx: - raise tx - raise exceptions.IOError, "unknown error prevented setting correct mode of directory, or changed mode of the directory immediately after creation. dirname: %s, mode: %04o, resmode: %04o" % (dirname, mode, resmode,) # careful not to construct an IOError with a 2-tuple, as that has a special meaning... - def rm_dir(dirname): """ A threadsafe and idempotent version of shutil.rmtree(). If the dir is diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 921632f9..0832f61c 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1335,7 +1335,7 @@ class Root(rend.Page): def render_private_vdrive(self, ctx, data): basedir = IClient(ctx).basedir - start_html = os.path.abspath(os.path.join(basedir, "start.html")) + start_html = os.path.abspath(os.path.join(basedir, "private", "start.html")) if os.path.exists(start_html): return T.p["To view your personal private non-shared filestore, ", "use this browser to open the following file from ", @@ -1408,7 +1408,6 @@ class WebishServer(service.MultiService): def _create_start_html(self, dummy, private_uri, startfile, nodeurl_file): f = open(startfile, "w") - os.chmod(startfile, 0600) template = open(util.sibpath(__file__, "web/start.html"), "r").read() # what is our webport? s = self.listener -- 2.45.2