By default accept requests from localhost only

[zhouer]

The default behavior of nodejs http server when not specifying hostname
is listen to 0.0.0.0 for IPv4 or :: for IPv6, which means accept
connections on all IP address. This patch specifes the hostname to
localhost when the hostname was not specified in command line argument
(-h and --hostname), because:

• Let RPC port opening to world is a bad idea.
• The default behavier of geth is listening to localhost only.
• I believe it is the original intention, see the message from L189.

[mikeseese]

Thanks for contributing @zhouer! I agree that we should not default to all IP addresses.

Unfortunately localhost is not always a valid hostname (we've had numerous issues where users are receiving ENOTFOUND localhost). I'd rather the default to be an IP address (i.e. 127.0.0.1) than a hostname.

[zhouer]

Thanks for your review, how about this.

[mikeseese]

This is better @zhouer, thanks for the quick turn around. I'm going to discuss this offline with the other devs to see if this is the best approach to the problem at hand

[benjamincburns]

Hi @zhouer - thanks for submitting!

Normally I wouldn't hesitate to accept this, but in essence it's a breaking change. For example this change might break scripts or containerized setups which rely on bridged networking if either fail to specify a hostname explicitly.

If you experiencing some acute pain as a result of this default being set the way it is, I'm eager to do something about it. Otherwise, I worry about the maintenance burden that this change will cause.

Typically we like to plan out major releases a bit, whereas we're willing to make minor/patch releases fairly ad-hoc. However I think our stance on major release numbers is softening. Thinking aloud here, I think the right strategy might be to open/announce a merge window for breaking changes so that others may propose similar breaking changes. If you have any thoughts on this, I'd love to hear them.

[zhouer]

Hi @benjamincburns,

Just do whatever make you guys comfortable. I can always use -h localhost to workaround this issue.

However, I did suffer from this issue. I noticed this issue because sometimes my dapp didn't work as expect, but after restarting ganache my dapp became normal again. At beginning I thought it's a bug from ganache, so I just keep restarting ganache again and again, but then I found there were still logging messages came out even when I was not using ganache. And then I figured out there were somebody trying to steal all the balance, and caused my dapp failed at random point.

Moreover, the Listening on localhost:8454 message shows at starting up is misleading, it makes me thought it's impossible that someone else can access the RPC port. I didn't know the ganache listens on all IP until I ran netstat! My dev platform does have public IP, and more than one guys were trying to steal the balance when I was debugging.

[benjamincburns]

Hmm... strange that we write Listening on localhost:8545 to the output. Given that's the case, I wonder if we can't make this a patch fix, as we've expressed some intention of listening on the loopback interface but missed our mark.

[mikeseese]

A separate note @zhouer: I highly suggest you use a firewall to close all ports for applications that don't need to be open ports. 8545 should not be open to prevent these kinds of issues. Further, you can specify the firewall rules to be interface-specific.

[tcoulter]

👍 for patch fix -- the number of people using this on an external interface is really low. Also 👍 for 127.0.0.1 as the default instead of 0.0.0.0.

[tcoulter]

To that I should say, thanks @zhouer!

[zhouer]

Thank you guys!

[dragos-vlad]

Hello @zhouerm, @tcoulter, @seesemichaelj ,

This PR breaks Docker integration. Using -h 0.0.0.0 or -h localhost does not seem to help. When I try to run a container which is connected to a private network, in my case the network created by docker-compose, other containers can not connect to the ganache-cli container.

Please advise,
Best regards.

Edit: OK so using -h 0.0.0.0 after rebuilding all containers worked, sorry for the false alarm. However the issue still stands that this PR breaks Docker integrations. I recommend updating the Dockerfile to bind to 0.0.0.0 or to localhost since this is the expected behaviour when dealing with Docker.

[zhouer]

Hi @dragos-vlad
Using -h 0.0.0.0 should bind on all IPv4 IP, and -h :: should bind on all IPv6 IP,
does -h :: work for you (if your docker is IPv6 only environment)?

[mikeseese]

@dragos-vlad Sorry for the delay! Using localhost is the same thing as 127.0.0.1, so if I'm understanding properly, you'd really just want to use 0.0.0.0?

[dragos-vlad]

@seesemichaelj Yeap, this is how I'm using it now. What I'm trying to explain is that this should be the default in the Dockerfile, since this is the expected behaviour when running a server in Docker.