Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add canonicalize function, add ipv6 option to host() (now returns v4 … #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

regular
Copy link
Contributor

@regular regular commented Sep 29, 2019

I ran into an issue on macOS, where multiserver's net plugin would try to bind to my private ipv6 address and failed with EADDRUNAVAIL. The reason could be that, implicitly, we create an ipv4 net server that cannot bind to v6 addresses. (We could create a v6 server however ... but I have not investigated this further)

Anyway, I was surprised that multiserver-scopes would return ipv6 addresses by default, it both (v4 and v6) are available and figured it probably shouldn't. With this PR host() returns v4 addresses. However, i you want an ipv6 address, you can use the new options argument, e.g.: host('private', {ipv6: true})

  • add canonicalize function
  • add ipv6 option to host()
  • host() now returns v4 by default)
  • add tests

@regular
Copy link
Contributor Author

regular commented Sep 29, 2019

@cryptix
Copy link
Member

cryptix commented Sep 30, 2019

I read over this and it seems like a good change. Inconsistent binding and dialing has been an issue in the last and I feel like v4 Loopback will be around for a while.

@staltz
Copy link
Member

staltz commented Oct 8, 2019

I'm related to this issue in many ways so I need to comment:

The reason could be that, implicitly, we create an ipv4 net server that cannot bind to v6 addresses. (We could create a v6 server however ... but I have not investigated this further)

I'm quite sure net supports IPv6 addresses, I'm doing that in Manyverse and it's been working fine.

The PR ssbc/multiserver#51 that @cryptix referred to is fixing a different issue than this, though, it's simply propagating initialization errors up, whether they are related to IPv6 or not shouldn't matter. Much more relevant is the PR ssbc/multiserver#52 which fixes multiserver to bind to private IPv6 addresses.

Anyway, I was surprised that multiserver-scopes would return ipv6 addresses by default, it both (v4 and v6)

Yes, I discovered this too, and ended up fixing this in non-private-ip. The module multiserver-scopes doesn't concern about IP addresses, it just asks non-private-ip to give it a private IP address. So I ended up forking that and making non-private-ip-android: https://github.com/staltz/non-private-ip/blob/fork/index.js it goes through all network interfaces and addresses, and prioritizes returning an IPv4 address. But if an IPv4 does not exist, but IPv6 exists, it returns IPv6. Maybe we should consider merging non-private-ip-android back into non-private-ip?

@regular
Copy link
Contributor Author

regular commented Oct 9, 2019

@staltz I dug a little deeper. This fails with EADDRNOTAVAIL on macOS

const ip = require('non-private-ip')
const net = require('net')
net.createServer().listen(0, ip.private.v6, err=>console.log(err))

ifconfig reports my actual private ipv6 address to be what ip.private.v6 reports, but with the name of the interface postfixed, separated by %, in my case: %en0.

And in fact this works:

net.createServer().listen(0, `${ip.private.v6}%en0`, err=>console.log(err))

Not sure how and where to fix this though. Moving forward, it would be nice to support v6.

EDIT ok, that's exactly what you address in ssbc/multiserver#52 I see!

@regular
Copy link
Contributor Author

regular commented Oct 9, 2019

@staltz The question remains: How do we get those private IPv6 addresses with zone id (the suffix)?

I was hoping that the numeric scopeid returned by os.networkInterfaces() for each address would do the trick. But a quick test shows that the addresses produced by

function getPrivateIPv6() {
  return Object.values(os.networkInterfaces())
    .reduce((acc, x)=>{return acc.concat(x)}, [])
    .filter(iface=>iface.address.toLowerCase().startsWith('fe80'))
    .map(iface=>{
      return `${iface.address}%${iface.scopeid}`
    })
}

console.log(getPrivateIPv6())

/* ==>
[
  'fe80::1%1',
  'fe80::c68:c821:a021:676d%5',
  'fe80::14f3:23ff:feb5:1251%7',
  'fe80::a858:f8e9:aadd:ec4c%10'
]
*/

also yield EADDRNOTAVAIL when passed to listen(). At least on macOS.

@staltz
Copy link
Member

staltz commented Oct 9, 2019

EDIT ok, that's exactly what you address in ssbc/multiserver#52 I see!

Yes! But on Android ip.private.v6 returns the address with the scopeid suffixed, I think (can't quite remember where it came from). By the way, try using the name of the interface as the scopeid instead of the numeric id, e.g. fe80::14f3:23ff:feb5:1251%wlan0 not fe80::14f3:23ff:feb5:1251%7.

@staltz
Copy link
Member

staltz commented Oct 9, 2019

In the worst case, you could still fork non-private-ip and check if the about-to-be-returned address is of family IPv6, and if it is, then suffix % plus the name of the interface to the end of the address.

@regular
Copy link
Contributor Author

regular commented Oct 9, 2019

@staltz yes, using the interface name as suffix works on macOS, as described above. The stack overflow article you linked to in your PR though sounds like Windows is using numeric scope ids (like %16). So i am afraid this is one of those cases where it's hard to find (and test!) a solution that works cross-platform. :( Also, it really sounds like if networkInterfaces()' scopeid should exactly serve this purpose ....

@staltz
Copy link
Member

staltz commented Oct 9, 2019

Also, it really sounds like if networkInterfaces()' scopeid should exactly serve this purpose ...

Yep, OS-level quirks should have been sorted out underneath os.networkInterfaces(), after all the module is called os right? :D

@christianbundy
Copy link
Contributor

Maybe relevant, but in my multiserver PR I've removed all scoped IPv6 addresses and the build is passing on win/mac/linux:

https://github.com/ssbc/multiserver/blob/3fd74cf070bf7ebd4d1aec0b7ada992e9baa0b22/lib/util.js#L21-L22

@christianbundy
Copy link
Contributor

More context: the latest PR obsoletes multiserver-scopes in general, so I'd love thoughts/feels/opinions about this comment/PR: ssbc/multiserver#49 (comment)

@dominictarr
Copy link

dominictarr commented Nov 22, 2019

More context: the latest PR obsoletes multiserver-scopes in general

@christianbundy do you mean your PR removes just that module or the idea of scopes?

@christianbundy
Copy link
Contributor

Just removes this module ("multiserver-scopes"), not "multiserver scopes". 😬

@stale
Copy link

stale bot commented Feb 20, 2020

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

@stale stale bot added the stale label Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants