Automatically configure `config.connections.incoming` #53

christianbundy · 2019-05-08T16:59:57Z

This replaces the old hardcode method with a new automatic configuration
that iterates through os.networkInterfaces() and returns a valid
configuration that takes advantage of all possible interfaces.

This should give multiserver more addresses to broadcast.

Before

{ net:
   [ { host: '::',
       port: 8008,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' } ],
  ws:
   [ { host: '::',
       port: 8989,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' } ] }

After

{ net:
   [ { host: '127.0.0.1',
       port: 8008,
       scope: [ 'device' ],
       transform: 'shs' },
     { host: '::1', port: 8008, scope: [ 'device' ], transform: 'shs' },
     { host: '192.168.0.109',
       port: 8008,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' },
     { host: '172.18.0.1',
       port: 8008,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' },
     { host: 'fce2:9811:4862:81a7:bb08:91d6:2e41:d220',
       port: 8008,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' } ],
  ws:
   [ { host: '127.0.0.1',
       port: 8989,
       scope: [ 'device' ],
       transform: 'shs' },
     { host: '::1', port: 8989, scope: [ 'device' ], transform: 'shs' },
     { host: '192.168.0.109',
       port: 8989,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' },
     { host: '172.18.0.1',
       port: 8989,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' },
     { host: 'fce2:9811:4862:81a7:bb08:91d6:2e41:d220',
       port: 8989,
       scope: [ 'device', 'local', 'public' ],
       transform: 'shs' } ] }

One interesting side-effect here is that if you run a node in a mesh network (e.g. cjdns) then that address is now being broadcast over LAN. If two people use both Scuttlebutt and cjdns on the same LAN then when they're far away I think they'll continue to peer over cjdns because the address is saved to gossip.json. Someone please correct me if I'm wrong here!

Also Travis CI tests by filtering IPv6 addresses on Travis. 🎉

Resolves #47
Resolves #49
Resolves #52
Hopefully takes care of ssbc/patchwork#958 and ssbc/patchwork#1024
Paves the way to merging ssbc/multiserver#42

This replaces the old hardcode method with a new automatic configuration that iterates through `os.networkInterfaces()` and returns a valid configuration that takes advantage of all possible interfaces.

arj03 · 2019-05-08T18:32:30Z

It seems like this auto stuff only applies to shs, so I don't see that big of a problem. I mean it can't be combined with noauth, as mentioned here.

I guess this superseeds some of the other PRs?

christianbundy · 2019-05-08T22:55:53Z

Yeah, I've defaulted to always include the shs transform because I don't understand os.networkInterfaces() well enough to trust that { internal: true } actually means it's a private interface. Would love to learn more about this if anyone would prefer to use noauth. 📚

This definitely supersedes #47 and #49, and maybe #23 (? (cc: @regular)).

arj03 · 2019-05-09T06:18:25Z

Thats was what I meant, mostly. noauth should only be used with pure scope 'device'.

christianbundy · 2019-05-09T14:23:33Z

Sweet, are there any changes you'd like to see (or anyone you think should definitely see this)?

arj03 · 2019-05-09T16:54:16Z

I was just testing this and it works for me, both with existing config and without any incoming. Also for patchbay we change the incoming to allow unix + noauth so I don't see anything really bad coming out of this. So +1 for merging.

christianbundy · 2019-05-09T21:10:44Z

I want to point out a great observation that @black-puppydog made here:

I had to remove my hardcoded :: config entry (:man_facepalming:)

I'd like to avoid any mandatory config changes, I'm thinking this behavior might be better:

If config.host == null use the auto-detection to bind to all interfaces.
If config.host === '::' use the auto-detection to bind to all interfaces.
If config.host === '0.0.0.0' use the auto-detection to bind to all interfaces.
Otherwise, bind to the exact value in config.host with no automatic shenanigans.

christianbundy · 2019-05-10T16:29:28Z

Scratch that, it looks like util/fix-connections.js specifically checks to ensure that config.host is represented in config.connections.incoming, so we can only do this automated stuff when config.host == null.

Dominic mentioned that @regular might have some valuable feedback here, so I'm going to wait until I get a 👍 from him until merging this. I think this change might be more elegant upstream (see ssbc/multiserver-scopes#1), but I think this is a decently robust workaround until then.

mixmix · 2019-05-11T01:48:35Z

Yeah I put a bit of thought into that fix-connections. There should really be a test around it. The behaviour we do not want under any circumstances is that a person sets some incoming connections explicitly narrowly, as a then some auto smart junk adds connections. This is how it was and it's and its a great way to add security problems /rant

…

On Sat, 11 May 2019, 04:29 Christian Bundy, ***@***.***> wrote: Scratch that, it looks like util/fix-connections.js specifically checks to ensure that config.host is represented in config.connections.incoming, so we can only do this automated stuff when config.host == null. Dominic mentioned that @regular <https://github.com/regular> might have some valuable feedback here, so I'm going to wait until I get a 👍 from him until merging this. I think this change *might* be more elegant upstream (see ssbc/multiserver-scopes#1 <ssbc/multiserver-scopes#1>), but I think this is a decently robust workaround until then. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#53 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUK3HTSI7UINJ4YZHX7TP3PUWPGTANCNFSM4HLTPTWQ> .

christianbundy · 2019-05-11T16:45:58Z

Yep, I'm super appreciative that util/fix-connections.js throws when the config is funky, it acts as a great guard rail even if it's more of a runtime sanity check than a "test" in the dev-y sense of the word.

ahdinosaur

this makes sense! ❤️

i'm not sure what the implications are for those running inside Docker containers, but ssb-pub should hopefully not be affected since i decided to stop using defaults: ssbc/ssb-server#638

christianbundy · 2019-05-20T16:53:06Z

I've actually taken this code and moved it into multiserver where I think it belongs: ssbc/multiserver#49

This means that ssb-config can still listen on :: but when you stringify the address multiserver should output all of the actual addresses that it's listening on. Less code in ssb-config and a smarter multiserver, woo! Would love a review if/when folks have time.

dgs2019 · 2019-08-15T20:30:59Z

defaults.js

@@ -1,9 +1,9 @@
 var path = require('path')
 var home = require('os-homedir')
 var merge = require('deep-extend')
-var nonPrivate = require('non-private-ip')
 var ssbKeys = require('ssb-keys')
 var get = require('lodash.get')


regular · 2019-09-29T11:29:50Z

@christianbundy thanks for pinging me. The only thing I can add to this conversation is this:

Somewhere you wrote:

Node understands that :: means "listen on all interfaces"

:: is an IPv6 address that has all zeros. My understanding is that Node just passes :: down to the IP stack and that the IP stack's configuration (or socket options) determines if this means "all IPv6" addresses" or "all IPv6 and IPv4 addresses".

A program bound to :: will be given traffic for any actual IPv6 address assigned to the system - it may also receive IPv4 traffic too in the form of IPv6-mapped IPv4 addresses (::ffff:x.x.x.x) although this is dependent on the socket options set by the application.

https://serverfault.com/a/452279/171681

You might want to consider this when coding a special case for '::'. I'm afraid I don't have any more specific advise/opinion on this.

mixmix · 2019-10-24T08:07:08Z

@christianbundy you've done amazing work. I understand there could be a deeper solution in multiserver but that's beyond my comprehension for the moment.

Because I am more familiar with this, and have tested this working on my local network, I've decided to merge this as a great interim solution. I've factored out the body of your logic into a single file to make this easy to snip out in the future if multiserver things get resolved.

thanks so much for your love and attention ❤️ ❤️ ❤️

mixmix · 2019-10-24T08:26:18Z

updated the tests here, and travis says everything's good : #60

mixmix · 2019-10-24T08:30:29Z

Wrap up

Published as ssb-config@3.4.0 🍿 🎆

Closed #47
Closed #49
Closed #52

Future / TODO

Revisit this if ssbc/multiserver#49 is merged

christianbundy added 2 commits May 8, 2019 09:53

Automatically configure config.connections.incoming

d9cde0a

This replaces the old hardcode method with a new automatic configuration that iterates through `os.networkInterfaces()` and returns a valid configuration that takes advantage of all possible interfaces.

Have travis test on Node LTS, drop old Node 6

67a09fd

christianbundy added bug enhancement labels May 8, 2019

christianbundy requested review from staltz, clehner, cryptix, dominictarr, mixmix and pietgeursen May 8, 2019 16:59

christianbundy self-assigned this May 8, 2019

christianbundy requested a review from arj03 May 8, 2019 17:00

christianbundy mentioned this pull request May 8, 2019

Enable opts.ipv6Only by default ssbc/multiserver#42

Closed

christianbundy mentioned this pull request May 9, 2019

Use experimental ssb-config ssbc/patchwork#1027

Merged

Remove unused non-private-ip dependency

7709f30

christianbundy requested a review from regular May 10, 2019 15:18

christianbundy mentioned this pull request May 17, 2019

Correctly stringify meta-addresses, and bind to all addresses ssbc/multiserver#49

Closed

ahdinosaur reviewed May 20, 2019

View reviewed changes

christianbundy closed this May 21, 2019

christianbundy reopened this May 21, 2019

dgs2019 reviewed Aug 15, 2019

View reviewed changes

black-puppydog mentioned this pull request Oct 2, 2019

Connections need to be adjusted for viewer ahdinosaur/ssb-pub#31

Open

mixmix added 2 commits October 24, 2019 20:54

Merge branch 'master' into auto-config-incoming

1ef13d4

extract rad connections.incoming defaults into util!

1ae161b

mixmix merged commit 4985920 into master Oct 24, 2019

mixmix deleted the auto-config-incoming branch October 24, 2019 08:24

christianbundy restored the auto-config-incoming branch October 24, 2019 22:18

dominictarr mentioned this pull request Oct 28, 2019

scopes are broken in 3.4.0 #63

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically configure `config.connections.incoming` #53

Automatically configure `config.connections.incoming` #53

christianbundy commented May 8, 2019 •

edited

Loading

arj03 commented May 8, 2019

christianbundy commented May 8, 2019

arj03 commented May 9, 2019

christianbundy commented May 9, 2019

arj03 commented May 9, 2019

christianbundy commented May 9, 2019

christianbundy commented May 10, 2019

mixmix commented May 11, 2019 via email

christianbundy commented May 11, 2019

ahdinosaur left a comment

christianbundy commented May 20, 2019

dgs2019 Aug 15, 2019

regular commented Sep 29, 2019

mixmix commented Oct 24, 2019

mixmix commented Oct 24, 2019

mixmix commented Oct 24, 2019

Automatically configure config.connections.incoming #53

Automatically configure config.connections.incoming #53

Conversation

christianbundy commented May 8, 2019 • edited Loading

Before

After

arj03 commented May 8, 2019

christianbundy commented May 8, 2019

arj03 commented May 9, 2019

christianbundy commented May 9, 2019

arj03 commented May 9, 2019

christianbundy commented May 9, 2019

christianbundy commented May 10, 2019

mixmix commented May 11, 2019 via email

christianbundy commented May 11, 2019

ahdinosaur left a comment

Choose a reason for hiding this comment

christianbundy commented May 20, 2019

dgs2019 Aug 15, 2019

Choose a reason for hiding this comment

regular commented Sep 29, 2019

mixmix commented Oct 24, 2019

mixmix commented Oct 24, 2019

mixmix commented Oct 24, 2019

Wrap up

Future / TODO

Automatically configure `config.connections.incoming` #53

Automatically configure `config.connections.incoming` #53

christianbundy commented May 8, 2019 •

edited

Loading