Create additional incomings for localhost

[regular]

In the old days, multiserver servers bound to all interfaces. Now they only bind to hosts that are explicitly configured in either config.host or config.connections.incoming.**.host, or the host indirectly selected via a scope.

The old behaviour of binding to all interfaces was convenient, because sbot would gossip on the LAN/Internet and would also allow the local cli client to connect without any configuration.

This restores this convenient behaviour by creating additional local incomings for net and ws as needed.

If you have a public IP, without any configuration, you end up with these incomings

• net/shs on public ip
• net/shs on localhost
• ws/shs on public ip
• ws/shs on localhost

and if you don't have a public IP:

• net/shs on private (lan) ip
• net/shs on localhost
• ws/shs on private (lan) ip
• ws/shs on localhost

[dominictarr]

hmm, my first reaction is I dislike how complicated this is getting. I thought you couldn't listen on the same port twice on two interfaces but I tested this and it worked. So I learnt something... but that it took me this long to learn that also shows how complicated this networking stuff is.

as I think a bit more, I start to realize that you have a point here... but I don't think we've found the best way to do it yet. There is a growing amount of code that sets up the configuration... and I'd much rather just have it work with completely static code.

One thing that is highly important: a config created before scuttlebot@13 should still be able to run sbot whoami which breaks if you use explicit addresses.

[dominictarr]

a problem with this configuration style, is that it only sets these defaults if the user has not set an incomings, and if they want to add something, they must copy the whole default incomings config to their setup.

but then, if we want to change something about the default incomings, it won't just be merged, it'll have to work with their own config. This will make it difficult for people who want to add one thing to maintain their setup. We need a design were you can override, enable, disable a single config - for example, disable net on local, but not device

[dominictarr]

public as 127.0.0.1 doesn't make sense either, if it doesn't add a device address because the localhost server is on the wrong interface, then you can't connect, although you could if you realized that the public address is actually a device address

I tried running scuttlebot tests for this with https://github.com/ssbc/multiserver/pull/28 and it breaks

[regular]

@dominictarr I feel the need to explain my intentions in a broader context, what I am trying to achieve with this whole set of PRs, which is easy to lose track of, because it's so spread out across so many different repos.

One thing that is highly important: a config created before scuttlebot@13 should still be able to run sbot whoami

I agree. Even more so, I am actually trying to make it work with a config created for scuttlebot<=11.3, before config.connections was introduced. That change (I believe it was in 11.4) left a lot of dangling ends, in particular traditional config values to be ignored entirely and introducing noauth, which now makes the former default behaviour of listening on all interfaces very dangerous. So, I am actually trying to make a minimal set of changes to make the new config system work in a compatible way, but also safe. That's actually all I want before moving on to application code again. I am surprised how hard this turned out to be :( and it is possible, that I am on the wrong track entirely.

mm, my first reaction is I dislike how complicated this is getting

I agree

There is a growing amount of code that sets up the configuration... and I'd much rather just have it work with completely static code

Yes, me too, as long as it is safe and doesn't violate reasonable expectations (which it currently does). I'd totally prefer a simpler solution that still does the correct thing, I just wasn't able to come up with one.

public as 127.0.0.1 doesn't make sense either, if it doesn't add a device address because the localhost server is on the wrong interface, then you can't connect, although you could if you realized that the public address is actually a device address

I don't understand this paragraph. The only addresses in device scope should be localhost and ipv6 aliases thereof. ms.stringify currently has knowledge of private and public scope hardcoded and don't know anything about device, which might cause the trouble. I think that knowledge needs to move into multiserver-scopes, that than would also know about device. (I am still learning the semantics of scope ..., I have a feeling that it is incomplete, put can't put my finger on it yet)

I tried running scuttlebot tests for this with ssbc/multiserver#28 and it breaks

Yes, I can confirm this, I will look into it. It might be that some tests assume that sbot listens on all interfaces, but I'll have to investigate.

[regular]

@dominictarr

I tried running scuttlebot tests for this with ssbc/multiserver#28 and it breaks

So, for me three tests in test/invite.js fail:

• test invite.accept api with ipv6
• test invite.create with modern
• test invite.accepts doesnt follow if already followed

The first one passes, if Alice gets host: '::' in her config. The test assumes that binding to localhost, also listens on ::1, which doesn't seem to be the case (or it depends on the specifics of the IP stack). We can also get rid of the string replacement here https://github.com/ssbc/scuttlebot/blob/master/test/invite.js#L170 because the invite will already be correct. So, this does not need any change once we make host='::' the default.

The third one fails because plugins/invite.js callls getAddress() without arguments, relying on it defaulting to 'public' https://github.com/ssbc/scuttlebot/blob/master/plugins/invite.js#L77 https://github.com/ssbc/scuttlebot/blob/master/plugins/invite.js#L112 (see https://github.com/ssbc/multiserver/pull/28#issue-226467645) Since invite codes only make sense with a public address, it probably should call it with 'public' explicitly, but I also would not be against changing the default back.

One thing that makes these tests weird is that creating invite codes really only make sense on a machine with a public IP, but the tests need to run on machines without public IPs. So we expect getAddress() or getAddress('public') to return an address that isn't actually public ....

The 2nd test is the hardest one. It still fails after applying the changes that make the 3rd tests pass (it also depends on these changes). I tracked it down to multiserver/plugins/ws.js calling server.address() before the http server actually started listening. address() then returns null. Same problem as here https://github.com/ssbc/ssb-client/pull/34#discussion_r229125075 I also talked about it here https://github.com/ssbc/multiserver/commit/eb3b027bace7ba150e1a26173ec2977e39ca8de8 any function that creates a server really should be async, so we can be sure that it is actually listening when before we run code that relies on it to listen.

Adding a setTimeout() around alice.invite.create(), so that server.address() is called after server.listen() is done, returns the correct address. I don't know why this worked until now.

However, the test still wouldn't pass, because now sbot would crash with the infamous 'end of parent stream' and no useful stack trace. This is where I gave up for now.

[dominictarr]

One thing that makes these tests weird is that creating invite codes really only make sense on a machine with a public IP, but the tests need to run on machines without public IPs. So we expect getAddress() or getAddress('public') to return an address that isn't actually public ....

note: there is an --allowPrivate option, that allows you to create an invite code for a private address. Sometimes this is useful - also it makes the tests work, using this option could cause getAddress('local')

[dominictarr]

oh, the end-of-parent-stream seems to be caused by when getAddress returns an empty string. that's fixed in secret-stack master branch, which I havn't released yet because I havn't been able to get scuttlebot tests passing. if you link it in I think it should work.

[dominictarr]

@dominictarr I feel the need to explain my intentions in a broader context

Yes! We definitely have the same end-goals - we have just experienced different edge cases so we need to find a way out that works for everyone. I need getAddress(scope) to return an address usable from that scope (especially public) so that I can release http://github.com/ssbc/ssb-device-address (which is the last blocker for https://github.com/dominictarr/user-invites AND https://github.com/dominictarr/ssb-tunnel

[regular]

regarding sync/async server.listen(): see also: %lPTvlvFSyZnTUbQk+rEjis0L6cuf87hAOFXTWGpT8GQ=.sha256

it's the same problem

[dominictarr]

btw, this script always seems to work:

var os = require('os')
var net = require('net')
;(function retry () {
  console.log('try')
  var port = 1024 + ~~(Math.random() * 4000)
  var host = os.networkInterfaces().wlp2s0[0].address
//  var host = 'localhost'
  var server = net.createServer(function (stream) {
    console.log('connected')
    stream.destroy()
    server.close(retry)
  }).listen(port, host)
  net.connect(port, host)
})()

if you change it host to 'localhost' then you get ECONNREFUSED, if you add setImmediate you get it sometimes if you add setTimeout without a time, you get it less often. curiously, if you bind to all addresses and connect to localhost you never get the error!

Although, if the port is not available, that error is async. So, I guess we need to take a second argument to ms.server(onConnection, cb) for that.

[regular]

@dominictarr also keep in mind that the outcome of such experiments is likely to be different on Windows, Mac OS, FreeBSD and even different versions of the Linux kernel. Agreed, the callback is the only clean solution.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cryptix]

I think this shouldn’t be closed.

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

[mixmix]

I think this has been supersceded by ChristianBundies comprehensive default PR that was merged in 3.4.0. I'm going to close this, please correct me if I'm wrong