From fea8b627dd3edfc9289f8ced7b531ba5a8c012eb Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Mon, 29 Aug 2016 18:49:20 -0700 Subject: [PATCH] if tub.port is empty, don't listen Updated config docs. Added errors if we're not listening but were told to enable storage, helper, or if we're the Introducer server. closes ticket:2816 --- docs/configuration.rst | 21 ++++++--- src/allmydata/client.py | 6 +++ src/allmydata/introducer/server.py | 3 ++ src/allmydata/node.py | 27 +++++++----- src/allmydata/test/test_node.py | 70 ++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 17 deletions(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index bfe00be18..3ca9e98c9 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -152,14 +152,21 @@ set the ``tub.location`` option described below. willing to ask Twisted to allocate port numbers in this way). To automatically allocate a TCP port, leave ``tub.port`` blank. - If the ``tub.port`` config key is not provided, the node will look in - ``BASEDIR/client.port`` (or ``BASEDIR/introducer.port``, for introducers) - for the descriptor that was used last time. + If the ``tub.port`` key is empty (i.e. ``tub.port =``), the node will not + listen at all, and thus cannot accept connections from other nodes. If + ``[storage] enabled = true``, or ``[helper] enabled = true``, or the node + is an Introducer, then it is an error to have ``tub.port`` be empty. - If neither is available, the node will ask the kernel for any available - port (the moral equivalent of ``tcp:0``). The allocated port number will - be written into a descriptor string in ``BASEDIR/client.port`` (or - ``introducer.port``), so that subsequent runs will re-use the same port. + If the ``tub.port`` config key is not provided (e.g. ``tub.port`` appears + nowhere in the ``[node]`` section, or is commented out), the node will + look in ``BASEDIR/client.port`` (or ``BASEDIR/introducer.port``, for + introducers) for the descriptor that was used last time. + + If neither ``tub.port`` nor the port file is available, the node will ask + the kernel to allocate any available port (the moral equivalent of + ``tcp:0``). The allocated port number will be written into a descriptor + string in ``BASEDIR/client.port`` (or ``introducer.port``), so that + subsequent runs will re-use the same port. ``tub.location = (string, optional)`` diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 6b98c4329..b0b82e39d 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -141,6 +141,9 @@ class Client(node.Node, pollmixin.PollMixin): self.load_static_servers() self.helper = None if self.get_config("helper", "enabled", False, boolean=True): + if not self._tub_is_listening: + raise ValueError("config error: helper is enabled, but tub " + "is not listening ('tub.port=' is empty)") self.init_helper() self.init_ftp_server() self.init_sftp_server() @@ -246,6 +249,9 @@ class Client(node.Node, pollmixin.PollMixin): # should we run a storage server (and publish it for others to use)? if not self.get_config("storage", "enabled", True, boolean=True): return + if not self._tub_is_listening: + raise ValueError("config error: storage is enabled, but tub " + "is not listening ('tub.port=' is empty)") readonly = self.get_config("storage", "readonly", False, boolean=True) storedir = os.path.join(self.basedir, self.STOREDIR) diff --git a/src/allmydata/introducer/server.py b/src/allmydata/introducer/server.py index 560ae6450..b92a88c08 100644 --- a/src/allmydata/introducer/server.py +++ b/src/allmydata/introducer/server.py @@ -29,6 +29,9 @@ class IntroducerNode(node.Node): self.init_web(webport) # strports string def init_introducer(self): + if not self._tub_is_listening: + raise ValueError("config error: we are Introducer, but tub " + "is not listening ('tub.port=' is empty)") introducerservice = IntroducerService(self.basedir) self.add_service(introducerservice) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 21ba53928..fadf988a3 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -300,8 +300,11 @@ class Node(service.MultiService): def get_tub_port(self): # return a descriptor string - cfg_tubport = self.get_config("node", "tub.port", "") - if cfg_tubport: + MISSING = object() + cfg_tubport = self.get_config("node", "tub.port", MISSING) + if cfg_tubport is not MISSING: + if cfg_tubport.strip() == "": + return None # don't listen at all return self._convert_tub_port(cfg_tubport) # For 'tub.port', tahoe.cfg overrides the individual file on disk. So # only read self._portnumfile if tahoe.cfg doesn't provide a value. @@ -338,15 +341,19 @@ class Node(service.MultiService): self.write_config("my_nodeid", b32encode(self.nodeid).lower() + "\n") self.short_nodeid = b32encode(self.nodeid).lower()[:8] # ready for printing tubport = self.get_tub_port() - if tubport in ("0", "tcp:0"): - raise ValueError("tub.port cannot be 0: you must choose") - self.tub.listenOn(tubport) + if tubport: + if tubport in ("0", "tcp:0"): + raise ValueError("tub.port cannot be 0: you must choose") + self.tub.listenOn(tubport) + location = self.get_tub_location(tubport) + self.tub.setLocation(location) + self._tub_is_listening = True + self.log("Tub location set to %s" % (location,)) + # the Tub is now ready for tub.registerReference() + else: + self._tub_is_listening = False + self.log("Tub is not listening") - location = self.get_tub_location(tubport) - self.tub.setLocation(location) - self.log("Tub location set to %s" % (location,)) - - # the Tub is now ready for tub.registerReference() self.tub.setServiceParent(self) def create_control_tub(self): diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index 7a5ce5daf..2bc06d040 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -10,6 +10,8 @@ import foolscap.logging.log from twisted.application import service from allmydata.node import Node, formatTimeTahoeStyle, MissingConfigEntry +from allmydata.introducer.server import IntroducerNode +from allmydata.client import Client from allmydata.util import fileutil, iputil from allmydata.util.namespace import Namespace import allmydata.test.common_util as testutil @@ -178,3 +180,71 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): TestNode(basedir) self.failUnless(ns.called) + +NO_LISTEN_CONFIG = """ +[node] +tub.port = +#tub.location = +[client] +introducer.furl = empty +""" + +class ClientNotListening(unittest.TestCase): + def test_port_none(self): + basedir = "test_node/test_port_none" + fileutil.make_dirs(basedir) + f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt') + f.write(NO_LISTEN_CONFIG) + f.write("[storage]\n") + f.write("enabled = false\n") + f.close() + n = Client(basedir) + self.assertEqual(n.tub.getListeners(), []) + + def test_port_none_location_none(self): + basedir = "test_node/test_port_none_location_none" + fileutil.make_dirs(basedir) + f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt') + f.write(NO_LISTEN_CONFIG) + f.write("tub.location =\n") + f.write("[storage]\n") + f.write("enabled = false\n") + f.close() + n = Client(basedir) + self.assertEqual(n.tub.getListeners(), []) + + def test_port_none_storage(self): + basedir = "test_node/test_port_none_storage" + fileutil.make_dirs(basedir) + f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt') + f.write(NO_LISTEN_CONFIG) + f.write("[storage]\n") + f.write("enabled = true") + f.close() + e = self.assertRaises(ValueError, Client, basedir) + self.assertIn("storage is enabled, but tub is not listening", str(e)) + + def test_port_none_helper(self): + basedir = "test_node/test_port_none_helper" + fileutil.make_dirs(basedir) + f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt') + f.write(NO_LISTEN_CONFIG) + f.write("[storage]\n") + f.write("enabled = false\n") + f.write("[helper]\n") + f.write("enabled = true") + f.close() + e = self.assertRaises(ValueError, Client, basedir) + self.assertIn("helper is enabled, but tub is not listening", str(e)) + +class IntroducerNotListening(unittest.TestCase): + def test_port_none_introducer(self): + basedir = "test_node/test_port_none_introducer" + fileutil.make_dirs(basedir) + f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt') + f.write("[node]\n") + f.write("tub.port = \n") + f.write("#tub.location = \n") + f.close() + e = self.assertRaises(ValueError, IntroducerNode, basedir) + self.assertIn("we are Introducer, but tub is not listening", str(e))