Avoid binding to all addresses #74
Conversation
|
Yeah, this is a good change, but could you think up some tests perhaps? might be tricky but you could poke at |
Telling the user that we're listening (only) on 127.0.0.1 when in fact we are listening on all available addresses can confuse users into thinking that running the service is more secure than it actually is. Better to tell the user what host exactly we are listening on. As far as I can tell, browsers accept having the zero address (:: or 0.0.0.0) pasted into their location bar, so it should still be possible to copy & paste the URL. One benefit is that we now only report the listening line when we have actually started listening, avoiding confusion when the web server is slow to start or when it fails to start after the message got printed. Another benefit is that we can report the port number actually in use if the user set the port to zero.
The default behavior can be achieved by specifying `*`. It will correspond to `::` on an IPv6-enabled machine and `0.0.0.0` otherwise, which is the reason we don't use either of these as the default.
This increases the chances of lazy people still using a security-relevant setting.
|
There appear to be zero tests for the command line interface at the moment. I'll have to write the first, I think. And I noticed that the short option |
|
Now here is some basic testing of the command line interface, along with specific tests for the newly added features. I tried to make the framework flexible enough so that other command line switches would be reasonably easy to test, too. The design is quite different from that of the rest of the test suite, because in order to verify the command's standard output, we need to run the process to completion, which means we can't start the process in a first Due to this approach, there is no The address selection is pretty ad-hoc: skip known loopback addresses, and skip link-local addresses since these didn't work for me when I tested this. The first other address we encounter will be used in these tests. Do we need configurability here? Should we exclude the loopback device altogether, even if it contains non-loopback addresses? Are there more address classes to exclude, like e.g. multicast addresses and the likes? Are there any chances to get at the “default” address, i.e. the one used for a typical outgoing package on the default route? Should we check with https://www.whatismyip.com/ or similar, and prefer that address if it is one of those configured? |
|
Ping? |
|
all, good, merged and released as 1.2.0, thanks @gagern! |
This was made possible by isaacs/st#74 and is good for security since it reduces the risk of accidentially exposing sensitive information.
Always binding to all addresses, the way the command line server currently does, can be a security problem. The situation is made worse by the fact that the command line server tells the user only about listening on
127.0.0.1, so the user may mistakenly think that only their local machine can access the server.This pull request addresses these problems by making the host name configurable, by providing flags to only bind to
localhost, and by always reporting the actual host name or address in the printed URL.I would suggest changing the default host name to
localhostin the next major version, so I suggested as much in the README. For now the change should be backwards-compatible, so I think a minor version bump would suffice.I wrote these changes in three commits, addressing different aspects. Feel free to commit only some of them if you disagree with some of the later, e.g. if you don't want a
--localhostflag. If you want me to, I can of course squash the commits.