Block non-local connections on mdns server

[tomasciccola]

This should close #68 and #69

[tomasciccola]

For this to be merged I think we need a way to test it.
This would maybe mean a vps that makes a request to join a swarm id'ed via a topic. This request could be trigger from a request made inside the test to an endpoint.
I don't know enough about mDNS to know if this would suffice...

[sethvincent]

Yeah, that's a hard thing to write a test for, and in a way duplicates bogon's tests.

I think ensuring this works as expected is something that would be covered by an integration test suite (that doesn't exist yet and would take some effort to set up) so would be best to make an issue for making that test suite and referencing this pull request as one of the needed tests.

[sethvincent]

@tomasciccola Approved with the small caveat that bogon should be in package.json dependencies.

[gmaclennan]

Yes, testing is hard for this. One way forward is to set up a virtual network with mininet, but that would be a lot of work. I think it's ok to merge without tests for now, and we should ensure that we do manual integration tests for this (e.g. actually testing on real devices) at some point when the apps are ready.

[sethvincent]

It looks like we're now good to merge this.

[tomasciccola]

nop, I need to address this yet ->

Ooops, yeah, fixed that.
Additionally, socket.remoteAddress returns an IPv6 with the IPv4 emebed (is it always like that or does it depend on some flag?); so I need to strip the :ffff:: to avoid false positives on the check

This is probably not a good idea. By default server.listen() is listening on the unspecified IPv6 (::) address, then your OS, if not using IPv6, is mapping IPv4 to IPv6. IPv6 does not have any private addresses, so doing this regex will actually make some IPv6 addresses appear private, which is not what we want.

I think the solution to this is to remove the string.replace() here and to ensure that when we listen, we listen only on IPv4, e.g. with net.listen(0, '0.0.0.0')

[tomasciccola]

It seems that ports are assigned by the OS since we are not passing them explicitly. This means that if I do smth like
this.#tcp.listen(0,'0.0.0.0') then this.#tcp.address() will return null. I haven't find a way to pass '0.0.0.0' to handle only IPv4 (an alternative would be, I think, this.#tcp.listen({address:'0.0.0.0'}), but the same happens...

[tomasciccola]

I wrapped this.#tcp.listen in a promise and now it works and only listens to IPv4.
For some reason util.promisify provided by node didn't work.
The idea is that any fn that has a callback as a last parameter can be wrapped like so =>

const = promisify(this.#tcp?.listen)
await listen(0,'0.0.0.0')

Another issue is that in a promise resolve is typed as (value:any) => void while server.listen expects a () => void.
Is there any way to 'cast' a callback?

Additionally, the server.listen callback can't error so I'm not handling a rejection; I don't know if this is correct.
Otherwise I think this is ready to merge

[gmaclennan]

With this last commit, you would still get an error because server.address() could be null (if the listen() fails)

suggested changes were made

[sethvincent]

Based on a previous pr (#112) and some upcoming changes it looks like we can close this one. Feel free to reopen if I've got that wrong.