From fdf8519ed5a66a94caf551cf5f853a3a327b4ff3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 16:29:52 -0400 Subject: [PATCH 01/23] Define a protocol for listener/transport providers --- src/allmydata/listeners.py | 66 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 src/allmydata/listeners.py diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py new file mode 100644 index 000000000..00ac9df4d --- /dev/null +++ b/src/allmydata/listeners.py @@ -0,0 +1,66 @@ +""" +Define a protocol for listening on a transport such that Tahoe-LAFS can +communicate over it, manage configuration for it in its configuration file, +detect when it is possible to use it, etc. +""" + +from __future__ import annotations + +from typing import Any, Protocol + +from attrs import frozen + +from .interfaces import IAddressFamily + +@frozen +class ListenerConfig: + """ + :ivar tub_ports: Entries to merge into ``[node]tub.port``. + + :ivar tub_locations: Entries to merge into ``[node]tub.location``. + + :ivar node_config: Entries to merge into the overall Tahoe-LAFS + configuration. XXX Note: Sections currently merge by overwriting + existing items with overlapping keys. In the future it would be nice + to merge every item sensibly (or error). + """ + tub_ports: list[str] + tub_locations: list[str] + node_config: dict[str, dict[str, str]] + +class Listener(Protocol): + """ + An object which can listen on a transport and allow Tahoe-LAFS + communication to happen over it. + """ + def is_available(self) -> bool: + """ + Can this type of listener actually be used in this runtime + environment? + """ + + def can_hide_ip(self) -> bool: + """ + Can the transport supported by this type of listener conceal the + node's public internet address from peers? + """ + + def create_config(self, reactor: Any, cli_config: Any) -> ListenerConfig: + """ + Set up an instance of this listener according to the given + configuration parameters. + + This may also allocate ephemeral resources if necessary. + + :return: The created configuration which can be merged into the + overall *tahoe.cfg* configuration file. + """ + + def create(self, config: Any, reactor: Any) -> IAddressFamily: + """ + Instantiate this listener according to the given + previously-generated configuration. + + :return: A handle on the listener which can be used to integrate it + into the Tahoe-LAFS node. + """ From cbfbfe8b1e3075b5c131f93c3c9e5b02b03b2ac1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 16:30:55 -0400 Subject: [PATCH 02/23] top-of-file cleanups --- src/allmydata/util/i2p_provider.py | 10 +--------- src/allmydata/util/tor_provider.py | 10 +--------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py index 071245adf..08a67c271 100644 --- a/src/allmydata/util/i2p_provider.py +++ b/src/allmydata/util/i2p_provider.py @@ -1,14 +1,6 @@ # -*- coding: utf-8 -*- -""" -Ported to Python 3. -""" -from __future__ import absolute_import, print_function, with_statement -from __future__ import division -from __future__ import unicode_literals -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations import os diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index 4ca19c01c..6d8278aad 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -1,14 +1,6 @@ # -*- coding: utf-8 -*- -""" -Ported to Python 3. -""" -from __future__ import absolute_import, print_function, with_statement -from __future__ import division -from __future__ import unicode_literals -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations import os From ed237b0dba674a7bac023163542cd6d904e20d77 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 09:26:13 -0400 Subject: [PATCH 03/23] improve the Listener protocol somewhat --- src/allmydata/listeners.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 00ac9df4d..34b447868 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -6,11 +6,13 @@ detect when it is possible to use it, etc. from __future__ import annotations -from typing import Any, Protocol +from typing import Any, Awaitable, Protocol, Sequence, Mapping, Optional +from typing_extensions import Literal -from attrs import frozen +from attrs import frozen, define from .interfaces import IAddressFamily +from .util.iputil import allocate_tcp_port @frozen class ListenerConfig: @@ -19,14 +21,12 @@ class ListenerConfig: :ivar tub_locations: Entries to merge into ``[node]tub.location``. - :ivar node_config: Entries to merge into the overall Tahoe-LAFS - configuration. XXX Note: Sections currently merge by overwriting - existing items with overlapping keys. In the future it would be nice - to merge every item sensibly (or error). + :ivar node_config: Entries to add into the overall Tahoe-LAFS + configuration beneath a section named after this listener. """ - tub_ports: list[str] - tub_locations: list[str] - node_config: dict[str, dict[str, str]] + tub_ports: Sequence[str] + tub_locations: Sequence[str] + node_config: Mapping[str, Sequence[tuple[str, str]]] class Listener(Protocol): """ @@ -45,7 +45,7 @@ class Listener(Protocol): node's public internet address from peers? """ - def create_config(self, reactor: Any, cli_config: Any) -> ListenerConfig: + async def create_config(self, reactor: Any, cli_config: Any) -> Optional[ListenerConfig]: """ Set up an instance of this listener according to the given configuration parameters. @@ -56,7 +56,7 @@ class Listener(Protocol): overall *tahoe.cfg* configuration file. """ - def create(self, config: Any, reactor: Any) -> IAddressFamily: + def create(self, reactor: Any, config: Any) -> IAddressFamily: """ Instantiate this listener according to the given previously-generated configuration. From e15970a484031cb2d4673c03f06b314a19c25b43 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 09:26:59 -0400 Subject: [PATCH 04/23] Add a couple simple Listeners that we need --- src/allmydata/listeners.py | 51 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 34b447868..149721bbc 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -64,3 +64,54 @@ class Listener(Protocol): :return: A handle on the listener which can be used to integrate it into the Tahoe-LAFS node. """ + +class TCPProvider: + """ + Support plain TCP connections. + """ + def is_available(self) -> Literal[True]: + return True + + def can_hide_ip(self) -> Literal[False]: + return False + + async def create_config(self, reactor: Any, cli_config: Any) -> ListenerConfig: + tub_ports = [] + tub_locations = [] + if cli_config["port"]: # --port/--location are a pair + tub_ports.append(cli_config["port"]) + tub_locations.append(cli_config["location"]) + else: + assert "hostname" in cli_config + hostname = cli_config["hostname"] + new_port = allocate_tcp_port() + tub_ports.append(f"tcp:{new_port}") + tub_locations.append(f"tcp:{hostname}:{new_port}") + + return ListenerConfig(tub_ports, tub_locations, {}) + + def create(self, reactor: Any, config: Any) -> IAddressFamily: + raise NotImplementedError() + + +@define +class StaticProvider: + """ + A provider that uses all pre-computed values. + """ + _available: bool + _hide_ip: bool + _config: Any + _address: IAddressFamily + + def is_available(self) -> bool: + return self._available + + def can_hide_ip(self) -> bool: + return self._hide_ip + + async def create_config(self, reactor: Any, cli_config: Any) -> None: + return await self._config + + def create(self, reactor: Any, config: Any) -> IAddressFamily: + return self._address From c52eb695055014821bdb3c5916b1278524ba448e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 09:27:26 -0400 Subject: [PATCH 05/23] Make the I2P and Tor providers implement the Listener protocol --- src/allmydata/test/test_i2p_provider.py | 8 ++-- src/allmydata/test/test_tor_provider.py | 20 +++++----- src/allmydata/util/i2p_provider.py | 43 ++++++++++++++++------ src/allmydata/util/tor_provider.py | 49 ++++++++++++++++--------- 4 files changed, 76 insertions(+), 44 deletions(-) diff --git a/src/allmydata/test/test_i2p_provider.py b/src/allmydata/test/test_i2p_provider.py index 364a85c5b..072b17647 100644 --- a/src/allmydata/test/test_i2p_provider.py +++ b/src/allmydata/test/test_i2p_provider.py @@ -177,7 +177,7 @@ class CreateDest(unittest.TestCase): with mock.patch("allmydata.util.i2p_provider.clientFromString", return_value=ep) as cfs: d = i2p_provider.create_config(reactor, cli_config) - tahoe_config_i2p, i2p_port, i2p_location = self.successResultOf(d) + i2p_config = self.successResultOf(d) connect_to_i2p.assert_called_with(reactor, cli_config, txi2p) cfs.assert_called_with(reactor, "goodport") @@ -189,9 +189,9 @@ class CreateDest(unittest.TestCase): "dest.private_key_file": os.path.join("private", "i2p_dest.privkey"), } - self.assertEqual(tahoe_config_i2p, expected) - self.assertEqual(i2p_port, "listen:i2p") - self.assertEqual(i2p_location, "i2p:FOOBAR.b32.i2p:3457") + self.assertEqual(dict(i2p_config.node_config["i2p"]), expected) + self.assertEqual(i2p_config.tub_ports, ["listen:i2p"]) + self.assertEqual(i2p_config.tub_locations, ["i2p:FOOBAR.b32.i2p:3457"]) _None = object() class FakeConfig(dict): diff --git a/src/allmydata/test/test_tor_provider.py b/src/allmydata/test/test_tor_provider.py index 86d54803a..65495fa0c 100644 --- a/src/allmydata/test/test_tor_provider.py +++ b/src/allmydata/test/test_tor_provider.py @@ -199,7 +199,7 @@ class CreateOnion(unittest.TestCase): with mock.patch("allmydata.util.tor_provider.allocate_tcp_port", return_value=999999): d = tor_provider.create_config(reactor, cli_config) - tahoe_config_tor, tor_port, tor_location = self.successResultOf(d) + tor_config = self.successResultOf(d) launch_tor.assert_called_with(reactor, executable, os.path.abspath(private_dir), txtorcon) @@ -216,10 +216,10 @@ class CreateOnion(unittest.TestCase): } if executable: expected["tor.executable"] = executable - self.assertEqual(tahoe_config_tor, expected) - self.assertEqual(tor_port, "tcp:999999:interface=127.0.0.1") - self.assertEqual(tor_location, "tor:ONION.onion:3457") - fn = os.path.join(basedir, tahoe_config_tor["onion.private_key_file"]) + self.assertEqual(dict(tor_config.node_config["tor"]), expected) + self.assertEqual(tor_config.tub_ports, ["tcp:999999:interface=127.0.0.1"]) + self.assertEqual(tor_config.tub_locations, ["tor:ONION.onion:3457"]) + fn = os.path.join(basedir, dict(tor_config.node_config["tor"])["onion.private_key_file"]) with open(fn, "rb") as f: privkey = f.read() self.assertEqual(privkey, b"privkey") @@ -253,7 +253,7 @@ class CreateOnion(unittest.TestCase): with mock.patch("allmydata.util.tor_provider.allocate_tcp_port", return_value=999999): d = tor_provider.create_config(reactor, cli_config) - tahoe_config_tor, tor_port, tor_location = self.successResultOf(d) + tor_config = self.successResultOf(d) connect_to_tor.assert_called_with(reactor, cli_config, txtorcon) txtorcon.EphemeralHiddenService.assert_called_with("3457 127.0.0.1:999999") @@ -267,10 +267,10 @@ class CreateOnion(unittest.TestCase): "onion.private_key_file": os.path.join("private", "tor_onion.privkey"), } - self.assertEqual(tahoe_config_tor, expected) - self.assertEqual(tor_port, "tcp:999999:interface=127.0.0.1") - self.assertEqual(tor_location, "tor:ONION.onion:3457") - fn = os.path.join(basedir, tahoe_config_tor["onion.private_key_file"]) + self.assertEqual(dict(tor_config.node_config["tor"]), expected) + self.assertEqual(tor_config.tub_ports, ["tcp:999999:interface=127.0.0.1"]) + self.assertEqual(tor_config.tub_locations, ["tor:ONION.onion:3457"]) + fn = os.path.join(basedir, dict(tor_config.node_config["tor"])["onion.private_key_file"]) with open(fn, "rb") as f: privkey = f.read() self.assertEqual(privkey, b"privkey") diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py index 08a67c271..4d997945f 100644 --- a/src/allmydata/util/i2p_provider.py +++ b/src/allmydata/util/i2p_provider.py @@ -2,6 +2,9 @@ from __future__ import annotations +from typing import Any +from typing_extensions import Literal + import os from zope.interface import ( @@ -13,11 +16,12 @@ from twisted.internet.endpoints import clientFromString from twisted.internet.error import ConnectionRefusedError, ConnectError from twisted.application import service +from ..listeners import ListenerConfig from ..interfaces import ( IAddressFamily, ) -def create(reactor, config): +def create(reactor: Any, config: Any) -> IAddressFamily: """ Create a new Provider service (this is an IService so must be hooked up to a parent or otherwise started). @@ -47,6 +51,21 @@ def _import_txi2p(): except ImportError: # pragma: no cover return None +def is_available() -> bool: + """ + Can this type of listener actually be used in this runtime + environment? + + If its dependencies are missing then it cannot be. + """ + return not (_import_i2p() is None or _import_txi2p() is None) + +def can_hide_ip() -> Literal[True]: + """ + Can the transport supported by this type of listener conceal the + node's public internet address from peers? + """ + return True def _try_to_connect(reactor, endpoint_desc, stdout, txi2p): # yields True or None @@ -89,29 +108,28 @@ def _connect_to_i2p(reactor, cli_config, txi2p): else: raise ValueError("unable to reach any default I2P SAM port") -@inlineCallbacks -def create_config(reactor, cli_config): +async def create_config(reactor: Any, cli_config: Any) -> ListenerConfig: txi2p = _import_txi2p() if not txi2p: raise ValueError("Cannot create I2P Destination without txi2p. " "Please 'pip install tahoe-lafs[i2p]' to fix this.") - tahoe_config_i2p = {} # written into tahoe.cfg:[i2p] + tahoe_config_i2p = [] # written into tahoe.cfg:[i2p] private_dir = os.path.abspath(os.path.join(cli_config["basedir"], "private")) stdout = cli_config.stdout if cli_config["i2p-launch"]: raise NotImplementedError("--i2p-launch is under development.") else: print("connecting to I2P (to allocate .i2p address)..", file=stdout) - sam_port = yield _connect_to_i2p(reactor, cli_config, txi2p) + sam_port = await _connect_to_i2p(reactor, cli_config, txi2p) print("I2P connection established", file=stdout) - tahoe_config_i2p["sam.port"] = sam_port + tahoe_config_i2p.append(("sam.port", sam_port)) external_port = 3457 # TODO: pick this randomly? there's no contention. privkeyfile = os.path.join(private_dir, "i2p_dest.privkey") sam_endpoint = clientFromString(reactor, sam_port) print("allocating .i2p address...", file=stdout) - dest = yield txi2p.generateDestination(reactor, privkeyfile, 'SAM', sam_endpoint) + dest = await txi2p.generateDestination(reactor, privkeyfile, 'SAM', sam_endpoint) print(".i2p address allocated", file=stdout) i2p_port = "listen:i2p" # means "see [i2p]", calls Provider.get_listener() i2p_location = "i2p:%s:%d" % (dest.host, external_port) @@ -124,10 +142,11 @@ def create_config(reactor, cli_config): # * "private_key_file" points to the on-disk copy of the private key # material (although we always write it to the same place) - tahoe_config_i2p["dest"] = "true" - tahoe_config_i2p["dest.port"] = str(external_port) - tahoe_config_i2p["dest.private_key_file"] = os.path.join("private", - "i2p_dest.privkey") + tahoe_config_i2p.extend([ + ("dest", "true"), + ("dest.port", str(external_port)), + ("dest.private_key_file", os.path.join("private", "i2p_dest.privkey")), + ]) # tahoe_config_i2p: this is a dictionary of keys/values to add to the # "[i2p]" section of tahoe.cfg, which tells the new node how to launch @@ -141,7 +160,7 @@ def create_config(reactor, cli_config): # at both create-node and startup time. The data directory is not # recorded in tahoe.cfg - returnValue((tahoe_config_i2p, i2p_port, i2p_location)) + return ListenerConfig([i2p_port], [i2p_location], {"i2p": tahoe_config_i2p}) @implementer(IAddressFamily) diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index 6d8278aad..a30eaebc9 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -2,6 +2,9 @@ from __future__ import annotations +from typing import Any +from typing_extensions import Literal + import os from zope.interface import ( @@ -18,6 +21,7 @@ from .iputil import allocate_tcp_port from ..interfaces import ( IAddressFamily, ) +from ..listeners import ListenerConfig def _import_tor(): try: @@ -33,7 +37,13 @@ def _import_txtorcon(): except ImportError: # pragma: no cover return None -def create(reactor, config, import_tor=None, import_txtorcon=None): +def can_hide_ip() -> Literal[True]: + return True + +def is_available() -> bool: + return not (_import_tor() is None or _import_txtorcon() is None) + +def create(reactor: Any, config: Any, import_tor=None, import_txtorcon=None) -> IAddressFamily: """ Create a new _Provider service (this is an IService so must be hooked up to a parent or otherwise started). @@ -146,30 +156,29 @@ def _connect_to_tor(reactor, cli_config, txtorcon): else: raise ValueError("unable to reach any default Tor control port") -@inlineCallbacks -def create_config(reactor, cli_config): +async def create_config(reactor: Any, cli_config: Any) -> ListenerConfig: txtorcon = _import_txtorcon() if not txtorcon: raise ValueError("Cannot create onion without txtorcon. " "Please 'pip install tahoe-lafs[tor]' to fix this.") - tahoe_config_tor = {} # written into tahoe.cfg:[tor] + tahoe_config_tor = [] # written into tahoe.cfg:[tor] private_dir = os.path.abspath(os.path.join(cli_config["basedir"], "private")) stdout = cli_config.stdout if cli_config["tor-launch"]: - tahoe_config_tor["launch"] = "true" + tahoe_config_tor.append(("launch", "true")) tor_executable = cli_config["tor-executable"] if tor_executable: - tahoe_config_tor["tor.executable"] = tor_executable + tahoe_config_tor.append(("tor.executable", tor_executable)) print("launching Tor (to allocate .onion address)..", file=stdout) - (_, tor_control_proto) = yield _launch_tor( + (_, tor_control_proto) = await _launch_tor( reactor, tor_executable, private_dir, txtorcon) print("Tor launched", file=stdout) else: print("connecting to Tor (to allocate .onion address)..", file=stdout) - (port, tor_control_proto) = yield _connect_to_tor( + (port, tor_control_proto) = await _connect_to_tor( reactor, cli_config, txtorcon) print("Tor connection established", file=stdout) - tahoe_config_tor["control.port"] = port + tahoe_config_tor.append(("control.port", port)) external_port = 3457 # TODO: pick this randomly? there's no contention. @@ -178,12 +187,12 @@ def create_config(reactor, cli_config): "%d 127.0.0.1:%d" % (external_port, local_port) ) print("allocating .onion address (takes ~40s)..", file=stdout) - yield ehs.add_to_tor(tor_control_proto) + await ehs.add_to_tor(tor_control_proto) print(".onion address allocated", file=stdout) tor_port = "tcp:%d:interface=127.0.0.1" % local_port tor_location = "tor:%s:%d" % (ehs.hostname, external_port) privkey = ehs.private_key - yield ehs.remove_from_tor(tor_control_proto) + await ehs.remove_from_tor(tor_control_proto) # in addition to the "how to launch/connect-to tor" keys above, we also # record information about the onion service into tahoe.cfg. @@ -195,12 +204,12 @@ def create_config(reactor, cli_config): # * "private_key_file" points to the on-disk copy of the private key # material (although we always write it to the same place) - tahoe_config_tor["onion"] = "true" - tahoe_config_tor["onion.local_port"] = str(local_port) - tahoe_config_tor["onion.external_port"] = str(external_port) - assert privkey - tahoe_config_tor["onion.private_key_file"] = os.path.join("private", - "tor_onion.privkey") + tahoe_config_tor.extend([ + ("onion", "true"), + ("onion.local_port", str(local_port)), + ("onion.external_port", str(external_port)), + ("onion.private_key_file", os.path.join("private", "tor_onion.privkey")), + ]) privkeyfile = os.path.join(private_dir, "tor_onion.privkey") with open(privkeyfile, "wb") as f: if isinstance(privkey, str): @@ -219,7 +228,11 @@ def create_config(reactor, cli_config): # at both create-node and startup time. The data directory is not # recorded in tahoe.cfg - returnValue((tahoe_config_tor, tor_port, tor_location)) + return ListenerConfig( + [tor_port], + [tor_location], + {"tor": tahoe_config_tor}, + ) @implementer(IAddressFamily) From 74ebda771a0b0e403dd6167081cb5516cc9c124e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 09:46:54 -0400 Subject: [PATCH 06/23] Make `tahoe create-node` use the new listener protocol --- src/allmydata/scripts/create_node.py | 135 ++++++++++++++++---------- src/allmydata/test/cli/test_create.py | 55 ++++------- 2 files changed, 103 insertions(+), 87 deletions(-) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 7d15b95ec..85d1c46cd 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -1,3 +1,8 @@ + +from __future__ import annotations + +from typing import Optional + import io import os @@ -21,7 +26,37 @@ from allmydata.scripts.common import ( from allmydata.scripts.default_nodedir import _default_nodedir from allmydata.util.assertutil import precondition from allmydata.util.encodingutil import listdir_unicode, argv_to_unicode, quote_local_unicode_path, get_io_encoding -from allmydata.util import fileutil, i2p_provider, iputil, tor_provider, jsonbytes as json + +i2p_provider: Listener +tor_provider: Listener + +from allmydata.util import fileutil, i2p_provider, tor_provider, jsonbytes as json + +from ..listeners import ListenerConfig, Listener, TCPProvider, StaticProvider + +def _get_listeners() -> dict[str, Listener]: + """ + Get all of the kinds of listeners we might be able to use. + """ + return { + "tor": tor_provider, + "i2p": i2p_provider, + "tcp": TCPProvider(), + "none": StaticProvider( + available=True, + hide_ip=False, + config=defer.succeed(None), + # This is supposed to be an IAddressFamily but we have none for + # this kind of provider. We could implement new client and server + # endpoint types that always fail and pass an IAddressFamily here + # that uses those. Nothing would ever even ask for them (at + # least, yet), let alone try to use them, so that's a lot of extra + # work for no practical result so I'm not doing it now. + address=None, # type: ignore[arg-type] + ), + } + +_LISTENERS = _get_listeners() dummy_tac = """ import sys @@ -98,8 +133,11 @@ def validate_where_options(o): if o['listen'] != "none" and o.get('join', None) is None: listeners = o['listen'].split(",") for l in listeners: - if l not in ["tcp", "tor", "i2p"]: - raise UsageError("--listen= must be none, or one/some of: tcp, tor, i2p") + if l not in _LISTENERS: + raise UsageError( + "--listen= must be one/some of: " + f"{', '.join(sorted(_LISTENERS))}", + ) if 'tcp' in listeners and not o['hostname']: raise UsageError("--listen=tcp requires --hostname=") if 'tcp' not in listeners and o['hostname']: @@ -108,7 +146,7 @@ def validate_where_options(o): def validate_tor_options(o): use_tor = "tor" in o["listen"].split(",") if use_tor or any((o["tor-launch"], o["tor-control-port"])): - if tor_provider._import_txtorcon() is None: + if not _LISTENERS["tor"].is_available(): raise UsageError( "Specifying any Tor options requires the 'txtorcon' module" ) @@ -123,7 +161,7 @@ def validate_tor_options(o): def validate_i2p_options(o): use_i2p = "i2p" in o["listen"].split(",") if use_i2p or any((o["i2p-launch"], o["i2p-sam-port"])): - if i2p_provider._import_txi2p() is None: + if not _LISTENERS["i2p"].is_available(): raise UsageError( "Specifying any I2P options requires the 'txi2p' module" ) @@ -145,7 +183,7 @@ class _CreateBaseOptions(BasedirOptions): def postOptions(self): super(_CreateBaseOptions, self).postOptions() if self['hide-ip']: - if tor_provider._import_txtorcon() is None and i2p_provider._import_txi2p() is None: + if not (_LISTENERS["tor"].is_available() or _LISTENERS["i2p"].is_available()): raise UsageError( "--hide-ip was specified but neither 'txtorcon' nor 'txi2p' " "are installed.\nTo do so:\n pip install tahoe-lafs[tor]\nor\n" @@ -218,8 +256,20 @@ class CreateIntroducerOptions(NoDefaultBasedirOptions): validate_i2p_options(self) -@defer.inlineCallbacks -def write_node_config(c, config): +def merge_config( + left: Optional[ListenerConfig], + right: Optional[ListenerConfig], +) -> Optional[ListenerConfig]: + if left is None or right is None: + return None + return ListenerConfig( + list(left.tub_ports) + list(right.tub_ports), + list(left.tub_locations) + list(right.tub_locations), + dict(list(left.node_config.items()) + list(right.node_config.items())), + ) + + +async def write_node_config(c, config): # this is shared between clients and introducers c.write("# -*- mode: conf; coding: {c.encoding} -*-\n".format(c=c)) c.write("\n") @@ -232,9 +282,10 @@ def write_node_config(c, config): if config["hide-ip"]: c.write("[connections]\n") - if tor_provider._import_txtorcon(): + if _LISTENERS["tor"].is_available(): c.write("tcp = tor\n") else: + # XXX What about i2p? c.write("tcp = disabled\n") c.write("\n") @@ -253,38 +304,23 @@ def write_node_config(c, config): c.write("web.port = %s\n" % (webport,)) c.write("web.static = public_html\n") - listeners = config['listen'].split(",") + listener_config = ListenerConfig([], [], {}) + for listener_name in config['listen'].split(","): + listener = _LISTENERS[listener_name] + listener_config = merge_config( + (await listener.create_config(reactor, config)), + listener_config, + ) - tor_config = {} - i2p_config = {} - tub_ports = [] - tub_locations = [] - if listeners == ["none"]: - c.write("tub.port = disabled\n") - c.write("tub.location = disabled\n") + if listener_config is None: + tub_ports = ["disabled"] + tub_locations = ["disabled"] else: - if "tor" in listeners: - (tor_config, tor_port, tor_location) = \ - yield tor_provider.create_config(reactor, config) - tub_ports.append(tor_port) - tub_locations.append(tor_location) - if "i2p" in listeners: - (i2p_config, i2p_port, i2p_location) = \ - yield i2p_provider.create_config(reactor, config) - tub_ports.append(i2p_port) - tub_locations.append(i2p_location) - if "tcp" in listeners: - if config["port"]: # --port/--location are a pair - tub_ports.append(config["port"]) - tub_locations.append(config["location"]) - else: - assert "hostname" in config - hostname = config["hostname"] - new_port = iputil.allocate_tcp_port() - tub_ports.append("tcp:%s" % new_port) - tub_locations.append("tcp:%s:%s" % (hostname, new_port)) - c.write("tub.port = %s\n" % ",".join(tub_ports)) - c.write("tub.location = %s\n" % ",".join(tub_locations)) + tub_ports = listener_config.tub_ports + tub_locations = listener_config.tub_locations + + c.write("tub.port = %s\n" % ",".join(tub_ports)) + c.write("tub.location = %s\n" % ",".join(tub_locations)) c.write("\n") c.write("#log_gatherer.furl =\n") @@ -294,17 +330,12 @@ def write_node_config(c, config): c.write("#ssh.authorized_keys_file = ~/.ssh/authorized_keys\n") c.write("\n") - if tor_config: - c.write("[tor]\n") - for key, value in list(tor_config.items()): - c.write("%s = %s\n" % (key, value)) - c.write("\n") - - if i2p_config: - c.write("[i2p]\n") - for key, value in list(i2p_config.items()): - c.write("%s = %s\n" % (key, value)) - c.write("\n") + if listener_config is not None: + for section, items in listener_config.node_config.items(): + c.write(f"[{section}]\n") + for k, v in items: + c.write(f"{k} = {v}\n") + c.write("\n") def write_client_config(c, config): @@ -445,7 +476,7 @@ def create_node(config): fileutil.make_dirs(os.path.join(basedir, "private"), 0o700) cfg_name = os.path.join(basedir, "tahoe.cfg") with io.open(cfg_name, "w", encoding='utf-8') as c: - yield write_node_config(c, config) + yield defer.Deferred.fromCoroutine(write_node_config(c, config)) write_client_config(c, config) print("Node created in %s" % quote_local_unicode_path(basedir), file=out) @@ -488,7 +519,7 @@ def create_introducer(config): fileutil.make_dirs(os.path.join(basedir, "private"), 0o700) cfg_name = os.path.join(basedir, "tahoe.cfg") with io.open(cfg_name, "w", encoding='utf-8') as c: - yield write_node_config(c, config) + yield defer.Deferred.fromCoroutine(write_node_config(c, config)) print("Introducer created in %s" % quote_local_unicode_path(basedir), file=out) defer.returnValue(0) diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index 1d1576082..d100f481f 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -17,6 +17,7 @@ from ..common import ( disable_modules, ) from ...scripts import create_node +from ...listeners import ListenerConfig, StaticProvider from ... import client def read_config(basedir): @@ -45,7 +46,7 @@ class Config(unittest.TestCase): e = self.assertRaises(usage.UsageError, parse_cli, verb, *args) self.assertIn("option %s not recognized" % (option,), str(e)) - def test_create_client_config(self): + async def test_create_client_config(self): d = self.mktemp() os.mkdir(d) fname = os.path.join(d, 'tahoe.cfg') @@ -59,7 +60,7 @@ class Config(unittest.TestCase): "shares-happy": "1", "shares-total": "1", } - create_node.write_node_config(f, opts) + await create_node.write_node_config(f, opts) create_node.write_client_config(f, opts) # should succeed, no exceptions @@ -245,7 +246,7 @@ class Config(unittest.TestCase): parse_cli, "create-node", "--listen=tcp,none", basedir) - self.assertEqual(str(e), "--listen= must be none, or one/some of: tcp, tor, i2p") + self.assertEqual(str(e), "--listen=tcp requires --hostname=") def test_node_listen_bad(self): basedir = self.mktemp() @@ -253,7 +254,7 @@ class Config(unittest.TestCase): parse_cli, "create-node", "--listen=XYZZY,tcp", basedir) - self.assertEqual(str(e), "--listen= must be none, or one/some of: tcp, tor, i2p") + self.assertEqual(str(e), "--listen= must be one/some of: i2p, none, tcp, tor") def test_node_listen_tor_hostname(self): e = self.assertRaises(usage.UsageError, @@ -287,24 +288,15 @@ class Config(unittest.TestCase): self.assertIn("To avoid clobbering anything, I am going to quit now", err) @defer.inlineCallbacks - def test_node_slow_tor(self): - basedir = self.mktemp() + def test_node_slow(self): d = defer.Deferred() - self.patch(tor_provider, "create_config", lambda *a, **kw: d) - d2 = run_cli("create-node", "--listen=tor", basedir) - d.callback(({}, "port", "location")) - rc, out, err = yield d2 - self.assertEqual(rc, 0) - self.assertIn("Node created", out) - self.assertEqual(err, "") + slow = StaticProvider(True, False, d, None) + create_node._LISTENERS["xxyzy"] = slow + self.addCleanup(lambda: create_node._LISTENERS.pop("xxyzy")) - @defer.inlineCallbacks - def test_node_slow_i2p(self): basedir = self.mktemp() - d = defer.Deferred() - self.patch(i2p_provider, "create_config", lambda *a, **kw: d) - d2 = run_cli("create-node", "--listen=i2p", basedir) - d.callback(({}, "port", "location")) + d2 = run_cli("create-node", "--listen=xxyzy", basedir) + d.callback(None) rc, out, err = yield d2 self.assertEqual(rc, 0) self.assertIn("Node created", out) @@ -369,10 +361,12 @@ def fake_config(testcase: unittest.TestCase, module: Any, result: Any) -> list[t class Tor(unittest.TestCase): def test_default(self): basedir = self.mktemp() - tor_config = {"abc": "def"} + tor_config = {"tor": [("abc", "def")]} tor_port = "ghi" tor_location = "jkl" - config_d = defer.succeed( (tor_config, tor_port, tor_location) ) + config_d = defer.succeed( + ListenerConfig([tor_port], [tor_location], tor_config) + ) calls = fake_config(self, tor_provider, config_d) rc, out, err = self.successResultOf( @@ -391,10 +385,7 @@ class Tor(unittest.TestCase): def test_launch(self): basedir = self.mktemp() - tor_config = {"abc": "def"} - tor_port = "ghi" - tor_location = "jkl" - config_d = defer.succeed( (tor_config, tor_port, tor_location) ) + config_d = defer.succeed(None) calls = fake_config(self, tor_provider, config_d) rc, out, err = self.successResultOf( @@ -410,10 +401,7 @@ class Tor(unittest.TestCase): def test_control_port(self): basedir = self.mktemp() - tor_config = {"abc": "def"} - tor_port = "ghi" - tor_location = "jkl" - config_d = defer.succeed( (tor_config, tor_port, tor_location) ) + config_d = defer.succeed(None) calls = fake_config(self, tor_provider, config_d) rc, out, err = self.successResultOf( @@ -451,10 +439,10 @@ class Tor(unittest.TestCase): class I2P(unittest.TestCase): def test_default(self): basedir = self.mktemp() - i2p_config = {"abc": "def"} + i2p_config = {"i2p": [("abc", "def")]} i2p_port = "ghi" i2p_location = "jkl" - dest_d = defer.succeed( (i2p_config, i2p_port, i2p_location) ) + dest_d = defer.succeed(ListenerConfig([i2p_port], [i2p_location], i2p_config)) calls = fake_config(self, i2p_provider, dest_d) rc, out, err = self.successResultOf( @@ -479,10 +467,7 @@ class I2P(unittest.TestCase): def test_sam_port(self): basedir = self.mktemp() - i2p_config = {"abc": "def"} - i2p_port = "ghi" - i2p_location = "jkl" - dest_d = defer.succeed( (i2p_config, i2p_port, i2p_location) ) + dest_d = defer.succeed(None) calls = fake_config(self, i2p_provider, dest_d) rc, out, err = self.successResultOf( From 00ecb65c01478e59f9dd543e849c085556c8b767 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 09:47:25 -0400 Subject: [PATCH 07/23] remove unused import --- src/allmydata/listeners.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 149721bbc..3c4e71b9c 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -6,7 +6,7 @@ detect when it is possible to use it, etc. from __future__ import annotations -from typing import Any, Awaitable, Protocol, Sequence, Mapping, Optional +from typing import Any, Protocol, Sequence, Mapping, Optional from typing_extensions import Literal from attrs import frozen, define From e8bcfea4f3c318a907926de7cc507549c291b059 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 09:56:30 -0400 Subject: [PATCH 08/23] news fragment --- newsfragments/4004.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/4004.minor diff --git a/newsfragments/4004.minor b/newsfragments/4004.minor new file mode 100644 index 000000000..e69de29bb From 2f3091a065fa3cb49183807b81785d8bd121f807 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 29 Mar 2023 10:00:38 -0400 Subject: [PATCH 09/23] pass mypy strict on the new module --- mypy.ini | 2 +- src/allmydata/listeners.py | 8 +++++--- src/allmydata/util/iputil.py | 12 +++--------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/mypy.ini b/mypy.ini index 7acc0ddc5..22b8a52f9 100644 --- a/mypy.ini +++ b/mypy.ini @@ -9,7 +9,7 @@ no_implicit_optional = True warn_redundant_casts = True strict_equality = True -[mypy-allmydata.test.cli.wormholetesting] +[mypy-allmydata.test.cli.wormholetesting,allmydata.listeners] disallow_any_generics = True disallow_subclassing_any = True disallow_untyped_calls = True diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 3c4e71b9c..667f984e5 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -6,7 +6,7 @@ detect when it is possible to use it, etc. from __future__ import annotations -from typing import Any, Protocol, Sequence, Mapping, Optional +from typing import Any, Protocol, Sequence, Mapping, Optional, Union, Awaitable from typing_extensions import Literal from attrs import frozen, define @@ -101,7 +101,7 @@ class StaticProvider: """ _available: bool _hide_ip: bool - _config: Any + _config: Union[Awaitable[ListenerConfig], ListenerConfig] _address: IAddressFamily def is_available(self) -> bool: @@ -110,7 +110,9 @@ class StaticProvider: def can_hide_ip(self) -> bool: return self._hide_ip - async def create_config(self, reactor: Any, cli_config: Any) -> None: + async def create_config(self, reactor: Any, cli_config: Any) -> ListenerConfig: + if isinstance(self._config, ListenerConfig): + return self._config return await self._config def create(self, reactor: Any, config: Any) -> IAddressFamily: diff --git a/src/allmydata/util/iputil.py b/src/allmydata/util/iputil.py index fd3e88c7f..e71e514e8 100644 --- a/src/allmydata/util/iputil.py +++ b/src/allmydata/util/iputil.py @@ -1,17 +1,10 @@ """ Utilities for getting IP addresses. - -Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals +from future.utils import native_str -from future.utils import PY2, native_str -if PY2: - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from typing import Callable import os, socket @@ -39,6 +32,7 @@ from .gcutil import ( fcntl = requireModule("fcntl") +allocate_tcp_port: Callable[[], int] from foolscap.util import allocate_tcp_port # re-exported try: From e6b3b658106359f6157d79dfe6b4974af4e49c93 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 19 Jul 2023 11:35:51 -0400 Subject: [PATCH 10/23] add some missing docstrings --- src/allmydata/test/cli/test_create.py | 19 +++++++++++++++++++ src/allmydata/util/i2p_provider.py | 5 +++++ 2 files changed, 24 insertions(+) diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index d100f481f..c6caf3395 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -47,6 +47,13 @@ class Config(unittest.TestCase): self.assertIn("option %s not recognized" % (option,), str(e)) async def test_create_client_config(self): + """ + ``create_node.write_client_config`` writes a configuration file + that can be parsed. + + TODO Maybe we should test that we can recover the given configuration + from the parse, too. + """ d = self.mktemp() os.mkdir(d) fname = os.path.join(d, 'tahoe.cfg') @@ -289,6 +296,10 @@ class Config(unittest.TestCase): @defer.inlineCallbacks def test_node_slow(self): + """ + A node can be created using a listener type that returns an + unfired Deferred from its ``create_config`` method. + """ d = defer.Deferred() slow = StaticProvider(True, False, d, None) create_node._LISTENERS["xxyzy"] = slow @@ -384,6 +395,10 @@ class Tor(unittest.TestCase): self.assertEqual(cfg.get("node", "tub.location"), "jkl") def test_launch(self): + """ + The ``--tor-launch`` command line option sets ``tor-launch`` to + ``True``. + """ basedir = self.mktemp() config_d = defer.succeed(None) @@ -400,6 +415,10 @@ class Tor(unittest.TestCase): self.assertEqual(args[1]["tor-control-port"], None) def test_control_port(self): + """ + The ``--tor-control-port`` command line parameter's value is + passed along as the ``tor-control-port`` value. + """ basedir = self.mktemp() config_d = defer.succeed(None) diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py index 4d997945f..996237873 100644 --- a/src/allmydata/util/i2p_provider.py +++ b/src/allmydata/util/i2p_provider.py @@ -109,6 +109,11 @@ def _connect_to_i2p(reactor, cli_config, txi2p): raise ValueError("unable to reach any default I2P SAM port") async def create_config(reactor: Any, cli_config: Any) -> ListenerConfig: + """ + For a given set of command-line options, construct an I2P listener. + + This includes allocating a new I2P address. + """ txi2p = _import_txi2p() if not txi2p: raise ValueError("Cannot create I2P Destination without txi2p. " From c1c0b60862855c1966d8aee058e7398b6fd2d93c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 19 Jul 2023 11:42:38 -0400 Subject: [PATCH 11/23] remove hard-coded tor/i2p in hide-ip support --- src/allmydata/scripts/create_node.py | 15 +++++++++++---- src/allmydata/util/dictutil.py | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 85d1c46cd..69215bcde 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -24,6 +24,7 @@ from allmydata.scripts.common import ( write_introducer, ) from allmydata.scripts.default_nodedir import _default_nodedir +from allmydata.util import dictutil from allmydata.util.assertutil import precondition from allmydata.util.encodingutil import listdir_unicode, argv_to_unicode, quote_local_unicode_path, get_io_encoding @@ -183,11 +184,17 @@ class _CreateBaseOptions(BasedirOptions): def postOptions(self): super(_CreateBaseOptions, self).postOptions() if self['hide-ip']: - if not (_LISTENERS["tor"].is_available() or _LISTENERS["i2p"].is_available()): + ip_hiders = dictutil.filter(lambda v: v.can_hide_ip(), _LISTENERS) + available = dictutil.filter(lambda v: v.is_available(), ip_hiders) + if not available: raise UsageError( - "--hide-ip was specified but neither 'txtorcon' nor 'txi2p' " - "are installed.\nTo do so:\n pip install tahoe-lafs[tor]\nor\n" - " pip install tahoe-lafs[i2p]" + "--hide-ip was specified but no IP-hiding listener is installed.\n" + "Try one of these:\n" + + "".join([ + f"\tpip install tahoe-lafs[{name}]\n" + for name + in ip_hiders + ]) ) class CreateClientOptions(_CreateBaseOptions): diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py index 0a7df0a38..277fc30e9 100644 --- a/src/allmydata/util/dictutil.py +++ b/src/allmydata/util/dictutil.py @@ -2,6 +2,23 @@ Tools to mess with dicts. """ +from __future__ import annotations +from typing import Callable, TypeVar + +K = TypeVar("K") +V = TypeVar("V") + +def filter(pred: Callable[[V], bool], orig: dict[K, V]) -> dict[K, V]: + """ + Filter out key/value pairs that fail to match a predicate. + """ + return { + k: v + for (k, v) + in orig.items() + if pred(v) + } + class DictOfSets(dict): def add(self, key, value): if key in self: From 72c18579e245b2be317e3e83b317d2584f33c58b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 19 Jul 2023 11:54:18 -0400 Subject: [PATCH 12/23] another docstring --- src/allmydata/scripts/create_node.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 69215bcde..0cd5c577b 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -267,6 +267,13 @@ def merge_config( left: Optional[ListenerConfig], right: Optional[ListenerConfig], ) -> Optional[ListenerConfig]: + """ + Merge two listener configurations into one configuration representing + both of them. + + If either is ``None`` then the result is ``None``. This supports the + "disable listeners" functionality. + """ if left is None or right is None: return None return ListenerConfig( From 911b54267be080ece9dc3f436a082834b1de3ae8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 19 Jul 2023 11:54:22 -0400 Subject: [PATCH 13/23] StaticProviders don't need to change --- src/allmydata/listeners.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 667f984e5..17f9ebd3b 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -94,7 +94,7 @@ class TCPProvider: raise NotImplementedError() -@define +@frozen class StaticProvider: """ A provider that uses all pre-computed values. From ee8155729dac54d1deaa5b5ab8ef835b5b667262 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 19 Jul 2023 12:03:18 -0400 Subject: [PATCH 14/23] clean up some type annotations --- src/allmydata/listeners.py | 8 ++++---- src/allmydata/util/tor_provider.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 17f9ebd3b..93eecf09f 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -9,7 +9,7 @@ from __future__ import annotations from typing import Any, Protocol, Sequence, Mapping, Optional, Union, Awaitable from typing_extensions import Literal -from attrs import frozen, define +from attrs import frozen from .interfaces import IAddressFamily from .util.iputil import allocate_tcp_port @@ -101,7 +101,7 @@ class StaticProvider: """ _available: bool _hide_ip: bool - _config: Union[Awaitable[ListenerConfig], ListenerConfig] + _config: Union[Awaitable[Optional[ListenerConfig]], Optional[ListenerConfig]] _address: IAddressFamily def is_available(self) -> bool: @@ -110,8 +110,8 @@ class StaticProvider: def can_hide_ip(self) -> bool: return self._hide_ip - async def create_config(self, reactor: Any, cli_config: Any) -> ListenerConfig: - if isinstance(self._config, ListenerConfig): + async def create_config(self, reactor: Any, cli_config: Any) -> Optional[ListenerConfig]: + if self._config is None or isinstance(self._config, ListenerConfig): return self._config return await self._config diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index f22371399..18a3281f4 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -42,7 +42,7 @@ def can_hide_ip() -> Literal[True]: def is_available() -> bool: return not (_import_tor() is None or _import_txtorcon() is None) -def create(reactor, config, import_tor=None, import_txtorcon=None) -> Optional[_Provider]: +def create(reactor, config, import_tor=None, import_txtorcon=None) -> _Provider: """ Create a new _Provider service (this is an IService so must be hooked up to a parent or otherwise started). From 40665d824d056fba0f929d636f7214d1eda8450b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 19 Jul 2023 12:04:16 -0400 Subject: [PATCH 15/23] remove unused import --- src/allmydata/util/tor_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index 18a3281f4..0e3090578 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import annotations -from typing import Any, Optional +from typing import Any from typing_extensions import Literal import os From 57facc6335ae2787998890320569ce2c51a7e2bf Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 20 Jul 2023 14:19:12 -0400 Subject: [PATCH 16/23] narrow the type of cli_config a bit This has unfortunate interactions with the "stdout" attribute but I'm punting on that. --- src/allmydata/listeners.py | 7 ++++--- src/allmydata/util/i2p_provider.py | 7 +++++-- src/allmydata/util/tor_provider.py | 7 +++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 93eecf09f..6af19d1b9 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -10,6 +10,7 @@ from typing import Any, Protocol, Sequence, Mapping, Optional, Union, Awaitable from typing_extensions import Literal from attrs import frozen +from twisted.python.usage import Options from .interfaces import IAddressFamily from .util.iputil import allocate_tcp_port @@ -45,7 +46,7 @@ class Listener(Protocol): node's public internet address from peers? """ - async def create_config(self, reactor: Any, cli_config: Any) -> Optional[ListenerConfig]: + async def create_config(self, reactor: Any, cli_config: Options) -> Optional[ListenerConfig]: """ Set up an instance of this listener according to the given configuration parameters. @@ -75,7 +76,7 @@ class TCPProvider: def can_hide_ip(self) -> Literal[False]: return False - async def create_config(self, reactor: Any, cli_config: Any) -> ListenerConfig: + async def create_config(self, reactor: Any, cli_config: Options) -> ListenerConfig: tub_ports = [] tub_locations = [] if cli_config["port"]: # --port/--location are a pair @@ -110,7 +111,7 @@ class StaticProvider: def can_hide_ip(self) -> bool: return self._hide_ip - async def create_config(self, reactor: Any, cli_config: Any) -> Optional[ListenerConfig]: + async def create_config(self, reactor: Any, cli_config: Options) -> Optional[ListenerConfig]: if self._config is None or isinstance(self._config, ListenerConfig): return self._config return await self._config diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py index 996237873..e9188c6ae 100644 --- a/src/allmydata/util/i2p_provider.py +++ b/src/allmydata/util/i2p_provider.py @@ -15,6 +15,7 @@ from twisted.internet.defer import inlineCallbacks, returnValue from twisted.internet.endpoints import clientFromString from twisted.internet.error import ConnectionRefusedError, ConnectError from twisted.application import service +from twisted.python.usage import Options from ..listeners import ListenerConfig from ..interfaces import ( @@ -108,7 +109,7 @@ def _connect_to_i2p(reactor, cli_config, txi2p): else: raise ValueError("unable to reach any default I2P SAM port") -async def create_config(reactor: Any, cli_config: Any) -> ListenerConfig: +async def create_config(reactor: Any, cli_config: Options) -> ListenerConfig: """ For a given set of command-line options, construct an I2P listener. @@ -120,7 +121,9 @@ async def create_config(reactor: Any, cli_config: Any) -> ListenerConfig: "Please 'pip install tahoe-lafs[i2p]' to fix this.") tahoe_config_i2p = [] # written into tahoe.cfg:[i2p] private_dir = os.path.abspath(os.path.join(cli_config["basedir"], "private")) - stdout = cli_config.stdout + # XXX We shouldn't carry stdout around by jamming it into the Options + # value. See https://tahoe-lafs.org/trac/tahoe-lafs/ticket/4048 + stdout = cli_config.stdout # type: ignore[attr-defined] if cli_config["i2p-launch"]: raise NotImplementedError("--i2p-launch is under development.") else: diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index 0e3090578..c40e65f42 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -13,6 +13,7 @@ from twisted.internet.defer import inlineCallbacks, returnValue from twisted.internet.endpoints import clientFromString, TCP4ServerEndpoint from twisted.internet.error import ConnectionRefusedError, ConnectError from twisted.application import service +from twisted.python.usage import Options from .observer import OneShotObserverList from .iputil import allocate_tcp_port @@ -154,14 +155,16 @@ def _connect_to_tor(reactor, cli_config, txtorcon): else: raise ValueError("unable to reach any default Tor control port") -async def create_config(reactor: Any, cli_config: Any) -> ListenerConfig: +async def create_config(reactor: Any, cli_config: Options) -> ListenerConfig: txtorcon = _import_txtorcon() if not txtorcon: raise ValueError("Cannot create onion without txtorcon. " "Please 'pip install tahoe-lafs[tor]' to fix this.") tahoe_config_tor = [] # written into tahoe.cfg:[tor] private_dir = os.path.abspath(os.path.join(cli_config["basedir"], "private")) - stdout = cli_config.stdout + # XXX We shouldn't carry stdout around by jamming it into the Options + # value. See https://tahoe-lafs.org/trac/tahoe-lafs/ticket/4048 + stdout = cli_config.stdout # type: ignore[attr-defined] if cli_config["tor-launch"]: tahoe_config_tor.append(("launch", "true")) tor_executable = cli_config["tor-executable"] From 024b5e428a84c3a78eda4b163bb47693224b6067 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 20 Jul 2023 14:23:31 -0400 Subject: [PATCH 17/23] narrow the type annotation for another Listener method param --- src/allmydata/listeners.py | 7 ++++--- src/allmydata/util/i2p_provider.py | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/allmydata/listeners.py b/src/allmydata/listeners.py index 6af19d1b9..f97f699b4 100644 --- a/src/allmydata/listeners.py +++ b/src/allmydata/listeners.py @@ -14,6 +14,7 @@ from twisted.python.usage import Options from .interfaces import IAddressFamily from .util.iputil import allocate_tcp_port +from .node import _Config @frozen class ListenerConfig: @@ -57,7 +58,7 @@ class Listener(Protocol): overall *tahoe.cfg* configuration file. """ - def create(self, reactor: Any, config: Any) -> IAddressFamily: + def create(self, reactor: Any, config: _Config) -> IAddressFamily: """ Instantiate this listener according to the given previously-generated configuration. @@ -91,7 +92,7 @@ class TCPProvider: return ListenerConfig(tub_ports, tub_locations, {}) - def create(self, reactor: Any, config: Any) -> IAddressFamily: + def create(self, reactor: Any, config: _Config) -> IAddressFamily: raise NotImplementedError() @@ -116,5 +117,5 @@ class StaticProvider: return self._config return await self._config - def create(self, reactor: Any, config: Any) -> IAddressFamily: + def create(self, reactor: Any, config: _Config) -> IAddressFamily: return self._address diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py index e9188c6ae..c480cd2f1 100644 --- a/src/allmydata/util/i2p_provider.py +++ b/src/allmydata/util/i2p_provider.py @@ -21,8 +21,9 @@ from ..listeners import ListenerConfig from ..interfaces import ( IAddressFamily, ) +from ..node import _Config -def create(reactor: Any, config: Any) -> IAddressFamily: +def create(reactor: Any, config: _Config) -> IAddressFamily: """ Create a new Provider service (this is an IService so must be hooked up to a parent or otherwise started). From 4713573621470dc0043587699b63ccf788bf1a40 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 20 Jul 2023 14:27:30 -0400 Subject: [PATCH 18/23] test for dictutil.filter --- src/allmydata/test/test_dictutil.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/allmydata/test/test_dictutil.py b/src/allmydata/test/test_dictutil.py index 7e26a6ed9..ce1c4a74c 100644 --- a/src/allmydata/test/test_dictutil.py +++ b/src/allmydata/test/test_dictutil.py @@ -1,17 +1,9 @@ """ Tests for allmydata.util.dictutil. - -Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals +from __future__ import annotations from future.utils import PY2, PY3 -if PY2: - # dict omitted to match dictutil.py. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, list, object, range, str, max, min # noqa: F401 from unittest import skipIf @@ -168,3 +160,18 @@ class TypedKeyDictPython2(unittest.TestCase): # Demonstration of how bytes and unicode can be mixed: d = {u"abc": 1} self.assertEqual(d[b"abc"], 1) + + +class FilterTests(unittest.TestCase): + """ + Tests for ``dictutil.filter``. + """ + def test_filter(self) -> None: + """ + ``dictutil.filter`` returns a ``dict`` that contains the key/value + pairs for which the value is matched by the given predicate. + """ + self.assertEqual( + {1: 2}, + dictutil.filter(lambda v: v == 2, {1: 2, 2: 3}), + ) From da43acf52e25bad8c3158f9b6f1d56672d1ea5dd Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 20 Jul 2023 14:27:50 -0400 Subject: [PATCH 19/23] more accurate docstring for dictutil.filter --- src/allmydata/util/dictutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py index 277fc30e9..58820993f 100644 --- a/src/allmydata/util/dictutil.py +++ b/src/allmydata/util/dictutil.py @@ -10,7 +10,7 @@ V = TypeVar("V") def filter(pred: Callable[[V], bool], orig: dict[K, V]) -> dict[K, V]: """ - Filter out key/value pairs that fail to match a predicate. + Filter out key/value pairs whose value fails to match a predicate. """ return { k: v From 02a696d73beec90e8d1d1071016440ee0d3e6d8a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 21 Jul 2023 08:13:33 -0400 Subject: [PATCH 20/23] Make `merge_config` fail on overlapping configs This isn't expected to happen. If it does it would be nice to see it instead of silently continue working with some config dropped on the floor. --- src/allmydata/scripts/create_node.py | 7 ++++ src/allmydata/test/cli/test_create.py | 54 +++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 0cd5c577b..4357abb49 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -273,9 +273,16 @@ def merge_config( If either is ``None`` then the result is ``None``. This supports the "disable listeners" functionality. + + :raise ValueError: If the keys in the node configs overlap. """ if left is None or right is None: return None + + overlap = set(left.node_config) & set(right.node_config) + if overlap: + raise ValueError(f"Node configs overlap: {overlap}") + return ListenerConfig( list(left.tub_ports) + list(right.tub_ports), list(left.tub_locations) + list(right.tub_locations), diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index c6caf3395..bc6aab760 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -25,6 +25,60 @@ def read_config(basedir): config = configutil.get_config(tahoe_cfg) return config +class MergeConfigTests(unittest.TestCase): + """ + Tests for ``create_node.merge_config``. + """ + def test_disable_left(self) -> None: + """ + If the left argument to ``create_node.merge_config`` is ``None`` + then the return value is ``None``. + """ + conf = ListenerConfig([], [], {}) + self.assertEqual(None, create_node.merge_config(None, conf)) + + def test_disable_right(self) -> None: + """ + If the right argument to ``create_node.merge_config`` is ``None`` + then the return value is ``None``. + """ + conf = ListenerConfig([], [], {}) + self.assertEqual(None, create_node.merge_config(conf, None)) + + def test_disable_both(self) -> None: + """ + If both arguments to ``create_node.merge_config`` are ``None`` + then the return value is ``None``. + """ + self.assertEqual(None, create_node.merge_config(None, None)) + + def test_overlapping_keys(self) -> None: + """ + If there are any keys in the ``node_config`` of the left and right + parameters that are shared then ``ValueError`` is raised. + """ + left = ListenerConfig([], [], {"foo": "bar"}) + right = ListenerConfig([], [], {"foo": "baz"}) + self.assertRaises(ValueError, lambda: create_node.merge_config(left, right)) + + def test_merge(self) -> None: + """ + ``create_node.merge_config`` returns a ``ListenerConfig`` that has + all of the ports, locations, and node config from each of the two + ``ListenerConfig`` values given. + """ + left = ListenerConfig(["left-port"], ["left-location"], {"left": "foo"}) + right = ListenerConfig(["right-port"], ["right-location"], {"right": "bar"}) + result = create_node.merge_config(left, right) + self.assertEqual( + ListenerConfig( + ["left-port", "right-port"], + ["left-location", "right-location"], + {"left": "foo", "right": "bar"}, + ), + result, + ) + class Config(unittest.TestCase): def test_client_unrecognized_options(self): tests = [ From 2d688df29924cea72c43535b49f11627a0843234 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 21 Jul 2023 08:19:27 -0400 Subject: [PATCH 21/23] get the node config types right --- src/allmydata/test/cli/test_create.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index bc6aab760..406aebd48 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -57,8 +57,8 @@ class MergeConfigTests(unittest.TestCase): If there are any keys in the ``node_config`` of the left and right parameters that are shared then ``ValueError`` is raised. """ - left = ListenerConfig([], [], {"foo": "bar"}) - right = ListenerConfig([], [], {"foo": "baz"}) + left = ListenerConfig([], [], {"foo": [("b", "ar")]}) + right = ListenerConfig([], [], {"foo": [("ba", "z")]}) self.assertRaises(ValueError, lambda: create_node.merge_config(left, right)) def test_merge(self) -> None: @@ -67,14 +67,22 @@ class MergeConfigTests(unittest.TestCase): all of the ports, locations, and node config from each of the two ``ListenerConfig`` values given. """ - left = ListenerConfig(["left-port"], ["left-location"], {"left": "foo"}) - right = ListenerConfig(["right-port"], ["right-location"], {"right": "bar"}) + left = ListenerConfig( + ["left-port"], + ["left-location"], + {"left": [("f", "oo")]}, + ) + right = ListenerConfig( + ["right-port"], + ["right-location"], + {"right": [("ba", "r")]}, + ) result = create_node.merge_config(left, right) self.assertEqual( ListenerConfig( ["left-port", "right-port"], ["left-location", "right-location"], - {"left": "foo", "right": "bar"}, + {"left": [("f", "oo")], "right": [("ba", "r")]}, ), result, ) From feb9643dfe1022663d339c622e7a0e9e94c1ca90 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 20 Jul 2023 11:36:30 -0400 Subject: [PATCH 22/23] skip permission-related tests if the environment is not suitable posix superuser can do anything on the filesystem --- src/allmydata/test/cli/test_grid_manager.py | 3 ++- src/allmydata/test/test_client.py | 25 +++++++------------- src/allmydata/test/test_node.py | 26 ++++++--------------- 3 files changed, 17 insertions(+), 37 deletions(-) diff --git a/src/allmydata/test/cli/test_grid_manager.py b/src/allmydata/test/cli/test_grid_manager.py index 2ed738544..604cd6b7b 100644 --- a/src/allmydata/test/cli/test_grid_manager.py +++ b/src/allmydata/test/cli/test_grid_manager.py @@ -222,7 +222,8 @@ class GridManagerCommandLine(TestCase): result.output, ) - @skipIf(not platform.isLinux(), "I only know how permissions work on linux") + @skipIf(platform.isWindows(), "We don't know how to set permissions on Windows.") + @skipIf(os.getuid() == 0, "cannot test as superuser with all permissions") def test_sign_bad_perms(self): """ Error reported if we can't create certificate file diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index 04d8e6160..86c95a310 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -1,17 +1,7 @@ -""" -Ported to Python 3. -""" -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations import os -import sys +from unittest import skipIf from functools import ( partial, ) @@ -42,6 +32,7 @@ from twisted.internet import defer from twisted.python.filepath import ( FilePath, ) +from twisted.python.runtime import platform from testtools.matchers import ( Equals, AfterPreprocessing, @@ -156,12 +147,12 @@ class Basic(testutil.ReallyEqualMixin, unittest.TestCase): yield client.create_client(basedir) self.assertIn("[client]helper.furl", str(ctx.exception)) + # if somebody knows a clever way to do this (cause + # EnvironmentError when reading a file that really exists), on + # windows, please fix this + @skipIf(platform.isWindows(), "We don't know how to set permissions on Windows.") + @skipIf(os.getuid() == 0, "cannot test as superuser with all permissions") def test_unreadable_config(self): - if sys.platform == "win32": - # if somebody knows a clever way to do this (cause - # EnvironmentError when reading a file that really exists), on - # windows, please fix this - raise unittest.SkipTest("can't make unreadable files on windows") basedir = "test_client.Basic.test_unreadable_config" os.mkdir(basedir) fn = os.path.join(basedir, "tahoe.cfg") diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py index 0b371569a..1469ec5b2 100644 --- a/src/allmydata/test/test_node.py +++ b/src/allmydata/test/test_node.py @@ -1,14 +1,4 @@ -""" -Ported to Python 3. -""" -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations import base64 import os @@ -31,6 +21,7 @@ from unittest import skipIf from twisted.python.filepath import ( FilePath, ) +from twisted.python.runtime import platform from twisted.trial import unittest from twisted.internet import defer @@ -333,10 +324,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): default = [("hello", "world")] self.assertEqual(config.items("nosuch", default), default) - @skipIf( - "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower(), - "We don't know how to set permissions on Windows.", - ) + @skipIf(platform.isWindows(), "We don't know how to set permissions on Windows.") + @skipIf(os.getuid() == 0, "cannot test as superuser with all permissions") def test_private_config_unreadable(self): """ Asking for inaccessible private config is an error @@ -351,10 +340,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): with self.assertRaises(Exception): config.get_or_create_private_config("foo") - @skipIf( - "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower(), - "We don't know how to set permissions on Windows.", - ) + @skipIf(platform.isWindows(), "We don't know how to set permissions on Windows.") + @skipIf(os.getuid() == 0, "cannot test as superuser with all permissions") def test_private_config_unreadable_preexisting(self): """ error if reading private config data fails @@ -411,6 +398,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase): self.assertEqual(len(counter), 1) # don't call unless necessary self.assertEqual(value, "newer") + @skipIf(os.getuid() == 0, "cannot test as superuser with all permissions") def test_write_config_unwritable_file(self): """ Existing behavior merely logs any errors upon writing From 60b361df2b711e83bb113cc5ce8da98a63870e56 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 21 Jul 2023 08:39:15 -0400 Subject: [PATCH 23/23] news fragment --- newsfragments/4049.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/4049.minor diff --git a/newsfragments/4049.minor b/newsfragments/4049.minor new file mode 100644 index 000000000..e69de29bb