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

Listen only localhost by default #2387

Closed
KOLANICH opened this issue Jun 26, 2019 · 4 comments
Closed

Listen only localhost by default #2387

KOLANICH opened this issue Jun 26, 2019 · 4 comments

Comments

@KOLANICH
Copy link

KOLANICH commented Jun 26, 2019

By default tensorbord listens all interfaces, which may be a security issue. IMHO by default it should listen only on localhost, and if one uses it on a server, he usually configures the stuff, so it is not an issue for him.

@nfelt
Copy link
Collaborator

nfelt commented Jun 28, 2019

Thanks for the feedback, and yes, our current thinking is that we'll cut over to listening on localhost only by default for our 2.0 release, though there are a few details to work out around making sure it's easy for users who want the current behavior to still get it. Stay tuned.

@nfelt
Copy link
Collaborator

nfelt commented Aug 7, 2019

Update: as mentioned, we plan on changing the default to localhost for our 2.0 release, since this is the more conservative and safe behavior.

Users who specifically want to expose TensorBoard (e.g. in a lab context to let other users access the instance) can either pass --host=<interface> explicitly to bind to the desired interface, or configure some type of proxied access (for example, ssh port forwarding like ssh -L 6006:localhost:6006 <my-tensorboard-server>).

Strictly speaking the default behavior is actually to attempt to bind to :: and enable IPv6-mapped IPv4 for the socket, so that the same socket can serve both IPv4 and IPv6 traffic, which useful in cases where a hostname for the server (including sometimes localhost itself) has both IPv4 and IPv6 addresses associated with it, since this prevents spurious connection-denied errors from having chosen the wrong protocol. See #1449 for the details.

Since the existing behavior can't be achieved by specifying either 0.0.0.0 or :: explicitly, I propose that we accept an additional special host value, --host=*, which behave as though :: was passed but attempt to make the socket dual-bind to IPv4 as well.

This change should mostly resolve the following issues:

wchargin added a commit that referenced this issue Aug 22, 2019
Summary:
Prior to this change, TensorBoard would default to serving on the entire
local network; now, TensorBoard serves to the local machine only, and
the flag `--host='*'` can be used to dual-bind to IPv4 and IPv6 on the
entire local network (the previous default). See #2387 and comments
therein for details.

Test Plan:
On my Debian machine, running with `strace -e trace=%network`, and
testing connection with `curl -4 localhost:6006/data/logdir` (or `-6`):

  - running with no `--host` flag, or `--host=localhost`:
      - can connect to loopback on IPv4 only
      - cannot connect over LAN
      - `strace` shows binding on `AF_INET`
  - running with `--host='::1'`:
      - can connect to loopback on IPv6 only
      - cannot connect over LAN
      - `strace` shows binding on `AF_INET6`
  - running with `--host=0.0.0.0`:
      - can connect to loopback on IPv4 only
      - **can** connect over LAN
      - `strace` shows binding on `AF_INET`
  - running with `--host='*'`:
      - can connect on both IPv4 and IPv6
      - **can** connect over LAN
      - `strace` shows binding on `AF_INET6` with an additional syscall
        to `setsockopt(3, SOL_IPV6, IPV6_V6ONLY, [0], 4)` to facilitate
        the dual-binding, which is not present in any other tested case

wchargin-branch: localhost-only
wchargin added a commit that referenced this issue Sep 18, 2019
Summary:
Prior to this change, TensorBoard would default to serving on the entire
local network; now, TensorBoard serves to the local machine only, and
the flag `--bind_all` can be used to dual-bind to IPv4 and IPv6 on the
entire local network (the previous default). See #2387 and comments
therein for details.

Test Plan:
On my Debian machine, running with `strace -e trace=%network`:

  - running with no `--host` flag:
      - can connect to loopback on IPv4 only
      - cannot connect over LAN
      - `strace` shows binding on `AF_INET`
      - a notice about `--bind_all` is printed to stderr
  - running with `--host=localhost`:
      - same behavior as with no `--host` flag, but no notice is printed
  - running with `--host='::1'`:
      - can connect to loopback on IPv6 only
      - cannot connect over LAN
      - `strace` shows binding on `AF_INET6`
  - running with `--host=0.0.0.0`:
      - can connect to loopback on IPv4 only
      - **can** connect over LAN
      - `strace` shows binding on `AF_INET`
  - running with `--host='::0'`:
      - can connect on both IPv4 and IPv6
      - **can** connect over LAN
      - `strace` shows binding on `AF_INET6`
  - running with `--bind_all`:
      - can connect on both IPv4 and IPv6
      - **can** connect over LAN
      - `strace` shows binding on `AF_INET6` with an additional syscall
        to `setsockopt(3, SOL_IPV6, IPV6_V6ONLY, [0], 4)` to facilitate
        the dual-binding, which is not present in any other tested case

In all cases, the printed serving URL (“TensorBoard x.y.z running at…”)
bears the exact `--host` flag, or my full hostname if `--bind_all` was
given, or `localhost` if neither was given. In all cases, the URL is a
clickable link in my `gnome-terminal`.

Note that on my system dual binding to `::0` works without an explicit
syscall—i.e., `IPV6_V6ONLY` defaults to `0`—but this is not portable.

Connection testing was performed via

```shell
for ipv in 4 6; do
    if curl -sfL -"${ipv}" localhost:6006/data/logdir >/dev/null; then
        printf 'v%d OK\n' "${ipv}"
    else
        printf 'v%d FAIL\n' "${ipv}"
    fi
done
```

in all cases.

wchargin-branch: localhost-only
@wchargin
Copy link
Contributor

wchargin commented Sep 19, 2019

Fixed by #2589, which will be in TensorBoard 2.0.0.

@KOLANICH
Copy link
Author

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants