Make RethinkDB listen on localhost by default and add an option to listen on another network interface.

[skorokithakis]

Most (all?) server applications listen to localhost only by default, for security. Most people will be expecting that, and will have a security hole when their RethinkDB instance is open to the entire web.

It would be much better to do the expected thing and have it listen to localhost, with an option to change that.

[danielmewes]

I agree. It's probably not so much of a problem on a production system, where you can easily setup a firewall. But it is a problem on developer machines. Having to set up and administer a firewall on a personal computer is a pain, and really shouldn't be required.

For example I must be careful to not have it running on my private notebook when I'm at university, or using my 3G to connect to the Internet etc.

I'm also not sure if this could be a problem for inclusion into e.g. Debian, which have a "secure by default configuration" policy. I haven't checked how strongly this is enforced though.

[coffeemug]

Temporarily moving to backlog -- there are more pressing issues to work out first.

[coffeemug]

Update (according to @Tryneus) -- "well, issue 28 is code complete and (almost) compiling, testing tomorrow!"

[coffeemug]

Also, I'd like to review what configuration looks like, I think good user experience for this is very important.

[Tryneus]

Well, the current state is that there is a new network option: [--local-address ('all', 'loopback', )]

'all' - listen on all found local addresses
'loopback' - listen on all found local loopback addresses
- add a local ip address to listen on

This option can be specified multiple times. If not specified at all, it will default to 'all'.

[jdoliner]

Hmm, calling this flag --local-address doesn't tell me much about what it's used for. Could we call it --listen instead?

Also isn't one of the points of this issue that it should default to loopback only?

[coffeemug]

Agree on defaulting to loopback. I also think --local-address is confusing, but so is --listen. What do nginx and apache use to name this flag, we should just use the same name.

We also need to integrate this with frank's startup/config scripts.

[al3xandru]

afair Apache calls it listen. Redis calls it bind

[jdoliner]

Hmm, bind actually seems the best to me although I don't think listen is really confusing.

[Tryneus]

Oh, sorry, it actually already is --listen-address, is that ok? Also, I'll change it to default to loopback only. The other option would be to make this a required flag, but I think that would be too cumbersome for the quickstart.

[jdoliner]

Yeah I think listen-address is fine. Required flag seems bad to me too. We should definitely print something at startup about only listening on localhost so people don't get confused.

[Tryneus]

Right, that's on the TODO list for this issue as well. Basically, we'll just printout which addresses we're going to be listening on, right at startup.

[skorokithakis]

I'll chime in too. I think "listen" is pretty clear, and "bind" even more so. I strongly suggest that this flag is optional and defaults to localhost, as that's where other services bind (redis, postgres, etc) by default. It would be unexpected to bind to anything other than localhost, at least for me.

I hope this helps.

[coffeemug]

Sorry, I think listen-address is strictly worse than bind, and we should rename it to bind. I pretty much agree with everything else (flag should be optional and default to loopback, and we should print out on startup which interfaces we're listening on).

[skorokithakis]

I agree with the above, mainly because the "address" part is misleading, as someone might think it implies a port. I think listen-interface or bind-interface are clearer. In my opinion, people are accustomed to all of the above and will understand what they mean, though, so I don't feel very strongly about any of the alternatives, they all sound acceptable to me.

[Tryneus]

Ok, so here's how it stands at the moment:

• The new option is [--bind (all | <ip address>)]
• This applies only to rethinkdb serve and rethinkdb proxy
• rethinkdb admin and rethinkdb import will only listen on loopback addresses
• This option can be used multiple times
• All loopback addresses are listened on by default.
• Any addresses specified in a --bind option must be found, or rethinkdb prints an error and exits. This applies even if --bind all is used

The code is done and working in my branch, just awaiting code review now.

[jdoliner]

So what happens if I start a machine A and listen on all interfaces and a machine B which listens on only local and I tell B to connect to A will it make its connection successfully while A fails to contact B?

[Tryneus]

I consider that user error, @jdoliner, and we can't cover every non-sensical setup a user may come up with. In this case, it will likely work, because machine B will connect to machine A, at which point machine A no longer needs to connect to B. If the connection ever goes down, machine B will have to be the one to reconnect.

[jdoliner]

Fair enough.

[coffeemug]

Before we close this bug, if the user starts with default settings, could we add a log message that says something like "Listening only on localhost for security, use bind=all to access the server on other network interfaces"?

The user experience can be very annoying and confusing without it.

[Tryneus]

So, I currently have it logging which ip addresses it's listening on at startup:

By default:

info: Listening on addresses: 127.0.0.1, 127.0.1.1.

Or with --bind all:

info: Listening on addresses: 127.0.0.1, 127.0.1.1, 192.168.0.7, 192.168.1.7.

I suppose I could have it reference the --bind option, how about this:

info: Listening on addresses (add more using '--bind'): 127.0.0.1, 127.0.1.1.

As for closing this, I've taken care of all the code review comments, but it isn't in next yet. I'd like to wait until the revamped heartbeat gets pushed before pushing this, as they touched some of the same code, and I would like not to have to do the same work over again. In either case, I think we should have both of them taken care of sometime Monday.

[coffeemug]

I like your proposal to reference --bind. Let's do that.

[Tryneus]

Ok, this is finally in as of commit 0b584ea, closing.