make sure tub.location and tub.port can be empty #2816

New issue

Closed

opened 2016-08-29 23:15:54 +00:00 by warner · 9 comments

warner commented

2016-08-29 23:15:54 +00:00

tub.port being empty means the node shouldn't do Tub.listenOn. We need to see how foolcap (and tahoe) reacts to either or both of these being empty.. I think tub.location needs to be set, but can be an empty string. I know it's fine to not call Tub.listenOn(), but I want to be sure that tahoe will do exactly that when tub.port is empty, and that it isn't going to allocate a listening port.

This ties into the anonymous mode (#1010 and #517). Anonymous-mode clients should leave both empty. Anonymous-mode servers, though, will populate them (with a tor:ONION.onion:PORT and tcp:LOCALPORT:interface=127.0.0.1).

tub.port being empty means the node shouldn't do `Tub.listenOn`. We need to see how foolcap (and tahoe) reacts to either or both of these being empty.. I think `tub.location` needs to be set, but can be an empty string. I know it's fine to not call `Tub.listenOn()`, but I want to be sure that tahoe will do exactly that when `tub.port` is empty, and that it isn't going to allocate a listening port. This ties into the anonymous mode (#1010 and #517). Anonymous-mode clients should leave both empty. Anonymous-mode servers, though, will populate them (with a `tor:ONION.onion:PORT` and `tcp:LOCALPORT:interface=127.0.0.1`).

warner added the

labels 2016-08-29 23:15:54 +00:00

warner added this to the 1.12.0 milestone 2016-08-29 23:15:54 +00:00

warner self-assigned this 2016-08-29 23:15:54 +00:00

warner commented

2016-08-30 01:36:41 +00:00

Author

(https://tahoe-lafs.org/trac/tahoe-lafs/ticket/517#comment:43) discusses the expected behavior of tub.location.

(https://tahoe-lafs.org/trac/tahoe-lafs/ticket/517#comment:43) discusses the expected behavior of `tub.location`.

warner commented

2016-08-30 01:45:21 +00:00

Author

Current ([2d8a1e865], 29-Aug-2016, pre-1.12) behavior of tub.port is:

tub.port = STRING:
- if string is all-numeric PORT, use tcp:PORT
- else the string must be an endpoint like tcp:HOST or tcp:HOST:interface=127.0.0.1
#tub.port = (omitted entirely):
- if NODEDIR/tub.port is present, treat as PORT or endpoint string
- if not present, allocate a TCP port, write tcp:PORT to NODEDIR/tub.port for future use
tub.port = (empty value): same as tub.port = (empty value)

Current tahoe create-client/create-node will write #tub.port = into tahoe.cfg, which means for most nodes that haven't been explicitly configured (probably to a deliberate port number), the key is missing entirely.

The proposed change is to treat tub.port = (empty value) as instructions to not start up a Listener. While this is a change in behavior, I don't think many client nodes are likely to be configured with an empty string.

Current ([2d8a1e865], 29-Aug-2016, pre-1.12) behavior of `tub.port` is: * `tub.port = STRING`: * if string is all-numeric PORT, use `tcp:PORT` * else the string must be an endpoint like `tcp:HOST` or `tcp:HOST:interface=127.0.0.1` * `#tub.port =` (omitted entirely): * if `NODEDIR/tub.port` is present, treat as PORT or endpoint string * if not present, allocate a TCP port, write `tcp:PORT` to `NODEDIR/tub.port` for future use * `tub.port =` (empty value): same as `tub.port =` (empty value) Current `tahoe create-client`/`create-node` will write `#tub.port =` into `tahoe.cfg`, which means for most nodes that haven't been explicitly configured (probably to a deliberate port number), the key is missing entirely. The proposed change is to treat `tub.port =` (empty value) as instructions to not start up a Listener. While this is a change in behavior, I don't think many client nodes are likely to be configured with an empty string.

warner commented

2016-08-30 05:34:18 +00:00

Author

(https://github.com/tahoe-lafs/tahoe-lafs/pull/324)

warner commented

2016-08-30 06:58:32 +00:00

Author

Note that the way to configure a client to not listen at all is to have this:

# disable listening
[node]
tub.port =
tub.location =

But don't do it like this, because it won't do the right thing:

# doesn't work!
[node]

or:

# doesn't work!
[node]
#tub.port =
#tub.location =

(if they're missing entirely, then tub.location will autodetect, and tub.port will either read the NODEDIR/client.port file or allocate a new port)

I'm willing to believe that we need a more obvious "disabled" syntax here. Maybe something like:

# possible alternate syntax
[node]
tub.port = none
tub.location = none

but it's a bit awkward, given that tub.port is usually an endpoint string (so it's fairly open-ended), and tub.location is a comma-separated hints string (also open-ended). We could steal a few names from those namespaces to mean "disabled", but there's always going to be a bit of tension/syntax-violating going on.

Note that the way to configure a client to not listen at all is to have this: ``` # disable listening [node] tub.port = tub.location = ``` But don't do it like this, because it won't do the right thing: ``` # doesn't work! [node] ``` or: ``` # doesn't work! [node] #tub.port = #tub.location = ``` (if they're missing entirely, then tub.location will autodetect, and tub.port will either read the `NODEDIR/client.port` file or allocate a new port) I'm willing to believe that we need a more obvious "disabled" syntax here. Maybe something like: ``` # possible alternate syntax [node] tub.port = none tub.location = none ``` but it's a bit awkward, given that tub.port is usually an endpoint string (so it's fairly open-ended), and tub.location is a comma-separated hints string (also open-ended). We could steal a few names from those namespaces to mean "disabled", but there's always going to be a bit of tension/syntax-violating going on.

Brian Warner <warner@lothar.com> commented

2016-08-30 15:42:16 +00:00

Owner

In fea8b62/trunk:

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

In [fea8b62/trunk](/tahoe-lafs/trac/commit/fea8b627dd3edfc9289f8ced7b531ba5a8c012eb): ``` 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 ```

tahoe-lafs added the

r/fixed

label 2016-08-30 15:42:16 +00:00

Brian Warner <warner@lothar.com> closed this issue

2016-08-30 15:42:16 +00:00

warner commented

2016-08-30 19:07:37 +00:00

Author

[node]
tub.port = disabled
tub.location = disabled

and we'll reject any confusing cases that aren't already in use. tahoe create-node creates commented-out lines, which must continue to do automatic port/location (for backwards compatibility) for the "key missing" case:

# default created by 'tahoe create-node'
[node]
#tub.port =
#tub.location =

.. but which means we can reject "key present but value empty" cases, and throw an error at startup if we see:

# rejected!
[node]
tub.port =
tub.location =

I'll also add a constraint that if tub.port = disabled, then tub.location must be disabled too, so they always show up as a pair. (and if tub.location = disabled, then tub.port must be too). I think that'll catch a lot of the confusing cases. "key is missing" and "value is missing" will mean different things, but instead of "X vs not X", it'll be "X vs error", which is probably safer.

At today's devchat, we agreed that an empty value is confusing, especially because it introduces a critical difference between "key is missing" and "key is present but value is empty" (one turns off the listener, the other turns on automatic port assignment). So I've reopened this, and I'm going to change it to: ``` [node] tub.port = disabled tub.location = disabled ``` and we'll reject any confusing cases that aren't already in use. `tahoe create-node` creates commented-out lines, which must continue to do automatic port/location (for backwards compatibility) for the "key missing" case: ``` # default created by 'tahoe create-node' [node] #tub.port = #tub.location = ``` .. but which means we can reject "key present but value empty" cases, and throw an error at startup if we see: ``` # rejected! [node] tub.port = tub.location = ``` I'll also add a constraint that if `tub.port = disabled`, then `tub.location` must be `disabled` too, so they always show up as a pair. (and if `tub.location = disabled`, then `tub.port` must be too). I think that'll catch a lot of the confusing cases. "key is missing" and "value is missing" will mean different things, but instead of "X vs not X", it'll be "X vs error", which is probably safer.

warner removed the

r/fixed

label 2016-08-30 19:07:37 +00:00

warner reopened this issue

2016-08-30 19:07:37 +00:00

warner commented

2016-08-30 19:08:14 +00:00

Author

Oh, and we'll also change tahoe create-node to add a comment in tahoe.cfg, which explains the possible values for these keys.

Oh, and we'll also change `tahoe create-node` to add a comment in `tahoe.cfg`, which explains the possible values for these keys.

warner commented

2016-08-31 23:18:40 +00:00

Author

Reminding myself what the valid combinations are:

tub.port				tub.location
	missing	empty	"disabled"	hintstring
missing	alloc/auto	ERR2	ERR4	alloc/file
empty	ERR1	ERR1	ERR1	ERR1
"disabled"	ERR3	ERR2	no-listen	ERR3
endpoint	auto	ERR2	ERR4	manual

ERR1: tub.port is not allowed to be an empty string
ERR2: tub.location is not allowed to be an empty string
ERR3: if tub.port is "disabled", tub.location must be too
ERR4: if tub.location is "disabled", tub.port must be too
alloc/auto: (legacy default): tub.port is either read from the portfile (e.g. NODEDIR/client.port), or allocated (and then written into the portfile). tub.location is automatically set by scanning for addresses and combining with the port number
auto: tub.port comes from tahoe.cfg, and must be in the form tcp:12345 or tcp:12345:morestuff. tub.location is automatically set as above
manual: both tub.port and tub.location are specified in tahoe.cfg, although tub.location can contain AUTO, which is treated as above
alloc/file: tub.port is either read from portfile or allocated. tub.location is set by tahoe.cfg, but can contain AUTO
no-listen: the Tub does not run a Listener, and does not have a location

Reminding myself what the valid combinations are: | tub.port |||| tub.location | |---|---|---|---|---| | | missing | empty | "disabled" | hintstring | | missing | alloc/auto | ERR2 | ERR4 | alloc/file | | empty | ERR1 | ERR1 | ERR1 | ERR1 | | "disabled" | ERR3 | ERR2 | no-listen | ERR3 | | endpoint | auto | ERR2 | ERR4 | manual | * ERR1: tub.port is not allowed to be an empty string * ERR2: tub.location is not allowed to be an empty string * ERR3: if tub.port is "disabled", tub.location must be too * ERR4: if tub.location is "disabled", tub.port must be too * alloc/auto: (legacy default): tub.port is either read from the portfile (e.g. NODEDIR/client.port), or allocated (and then written into the portfile). tub.location is automatically set by scanning for addresses and combining with the port number * auto: tub.port comes from tahoe.cfg, and must be in the form `tcp:12345` or `tcp:12345:morestuff`. tub.location is automatically set as above * manual: both tub.port and tub.location are specified in tahoe.cfg, although tub.location can contain `AUTO`, which is treated as above * alloc/file: tub.port is either read from portfile or allocated. tub.location is set by tahoe.cfg, but can contain `AUTO` * no-listen: the Tub does not run a Listener, and does not have a location

Brian Warner <warner@lothar.com> commented

2016-09-02 05:32:18 +00:00

Owner

In 076b389/trunk:

config: change syntax of no-listen mode

We now use::

  tub.port = disabled
  tub.location = disabled

instead of using an empty value (but the key still being present, since
if the key is missing entirely, that means "be automatic").

closes ticket:2816

In [076b389/trunk](/tahoe-lafs/trac/commit/076b3895dc0270330892ff033b0fd6a8ded65096): ``` config: change syntax of no-listen mode We now use:: tub.port = disabled tub.location = disabled instead of using an empty value (but the key still being present, since if the key is missing entirely, that means "be automatic"). closes ticket:2816 ```