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

More granular address announcements #3613

Closed
ghost opened this issue Jan 20, 2017 · 9 comments
Closed

More granular address announcements #3613

ghost opened this issue Jan 20, 2017 · 9 comments
Labels
status/in-progress In progress topic/interop Interoperability topic/libp2p Topic libp2p topic/repo Topic repo

Comments

@ghost
Copy link

ghost commented Jan 20, 2017

Sometimes a go-ipfs listener can not directly bind to its external address. This is e.g. the case with NAT, where it binds to an IP address in the local network, while other nodes dial it using the local router's public IP address. We jump through all kinds of hoops to work around that, and one of these is "observed addresses", where other nodes tell us which addresses they've seen us using.

Another example are HTTP-based transports like /ws, which will in many cases be behind an SSL-terminating reverse proxy. Bound address: /ws, external address: /wss. There is a way to discover that we're behind a reverse proxy (by passing a special HTTP header), but that would only work after the first request has come in. Until then, the node would be unaware of its /wss address.

We could just as well have an ExternalAddresses config option though (or ideally both). This might also be useful for announcing /dns addresses. The default bootstrappers will eventually be bootstrap.libp2p.io or so, and they'd be able to announce it.

@ghost ghost added kind/enhancement A net-new feature or improvement to an existing feature topic/interop Interoperability topic/libp2p Topic libp2p labels Jan 20, 2017
@mateon1
Copy link
Contributor

mateon1 commented Jan 20, 2017

Related: ipfs-inactive/faq#185 (using SOCKS with IPFS)

@ghost ghost changed the title Listeners behind reverse proxies More granular address announcements Apr 18, 2017
@ghost
Copy link
Author

ghost commented Apr 18, 2017

@diasdavid @Kubuxu @whyrusleeping what do you think of this?

What I was thinking of is something like a Addresses.Announce array option. @diasdavid also brought up the inverse use case, excluding certain addresses from announcement (Addresses.NoAnnounce).

If Addresses.Announce is empty, we'll infer the addresses to announce as usual. If Addresses.NoAnnounce is non-empty, we'll exclude any matching address from the set of to-be-announced addresses.

Addresses.NoAnnounce should allow for filtering based on /ipcidr. It would probably be useful to allow for /ipcidr in Addresses.Swarm too.

Examples

Websocket listener behind an HTTPS reverse proxy:

{
  "Addresses": {
    "Swarm": [
      "/ip6/::/tcp/80/ws"
    ],
    "Announce": [
      "/dns6/example.net/tcp/4001/wss"
    ]
  }
}

Normal TCP listener, but don't announce our 172.16.0.0/12 addresses.

{
  "Addresses": {
    "Swarm": [
      "/ip4/0.0.0.0/tcp/4001"
    ],
    "NoAnnounce": [
      "/ip4/172.16.0.0/ipcidr/12"
    ]
  }
}

@ghost
Copy link
Author

ghost commented Apr 18, 2017

The bootstrap nodes would announce their IP, and their dnsaddr.

{
  "Addresses": {
    "Swarm": [
      "/ip4/0.0.0.0/tcp/4001"
    ],
    "Announce": [
      "/ip4/1.2.3.4/tcp/4001",
      "/dnsaddr/libp2p.io"
    ]
  }
}

@whyrusleeping
Copy link
Member

This all looks good to me. 👍

@daviddias
Copy link
Member

Thanks @lgierth, this pretty much covers my concerns.

While we are at it, can we piggy back a old proposed change of putting libp2p things under libp2p key?

libp2p: {
  addresses: {
    swarm:
    announce:
    noAnnounce:
  }
  discovery
  ...
}

@whyrusleeping
Copy link
Member

@diasdavid We can do that, but it doesnt need to be coupled to the change @lgierth is proposing. This change doesnt require a migration, renaming those keys does.

That said, we have needed a config file migration to update defaults for a good while

@ghost
Copy link
Author

ghost commented Apr 18, 2017

That said, we have needed a config file migration to update defaults for a good while

Yeah I can take care of it when patching the new bootstrap addrs into the config.

@ghost
Copy link
Author

ghost commented Apr 18, 2017

I have a hunch this is gonna get ugly unless I refactor things a bit. Listened-on addresses and their announcement are currently pretty strongly coupled. Maybe it's worth introducing a broker-like object which would control the logic around our node's addresses.

@ghost ghost unassigned daviddias and whyrusleeping Apr 25, 2017
@ghost ghost added topic/repo Topic repo and removed kind/enhancement A net-new feature or improvement to an existing feature labels Apr 25, 2017
@ghost
Copy link
Author

ghost commented May 29, 2017

I have a hunch this is gonna get ugly unless I refactor things a bit

No, actually it's gonna be just fine. See libp2p/go-libp2p#196 for the few changes that are needed on go-libp2p's part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/in-progress In progress topic/interop Interoperability topic/libp2p Topic libp2p topic/repo Topic repo
Projects
None yet
Development

No branches or pull requests

3 participants