Kernel can only be bound to localhost #169

Closed
minrk opened this Issue Oct 13, 2010 · 8 comments

Projects

None yet

2 participants

@minrk
Member
minrk commented Oct 13, 2010

Has anyone actually tried to connect a frontend to a remote kernel (on a different machine)? Because I'm pretty sure it's impossible with the current setup.

KernelManager.start_kernel() enforces that the kernel be bound to localhost, which can only be connected via the local machine. This is wrong, and prevents ever allowing external managers from connecting to the kernel (part of the point). It should not check that it's localhost; it should check that it refers to this machine, which, on my laptop, includes at least 5 valid addresses.

The correct logic should be:
import socket
LOCAL_IPS = socket.gethostbyname_ex('localhost')[2]
LOCAL_IPS.extend(socket.gethostbyname_ex(socket.gethostname())[2]
# on my machine, this is now: ['127.0.0.1', '192.168.1.101', '192.168.1.111', '192.168.2.1', '192.168.151.1']

and all
addr == LOCALHOST
tests should be:
addr in LOCAL_IPS

*There should probably be some try/excepts in constructing LOCAL_IPS, since gethostname might fail on some systems.

@minrk
Member
minrk commented Oct 13, 2010

The ipythonqt script has similar logic preventing binding to a local non-loopback interface (it fails silently, which is worse, but easily fixable).

@minrk
Member
minrk commented Oct 13, 2010

We also need to allow binding to multiple interfaces. At the very least, support the catch-all aliases, which can be specified as: '0.0.0.0' or '', since these are likely the most common choice after localhost.

@fperez
Member
fperez commented Oct 13, 2010

No, I hadn't tried. I had assumed it was possible (the generic machinery for it is there), but I'd never actually tested it.

Go ahead and make the fixes.

Though since right now we have zero security/authentication/encryption, I'd say that allowing remote clients should be something explicitly required on startup with --allow-remote or somesuch. At least users will be aware that they are opening effectively a remote shell on their system.

On something like the UCB-wide network, I tend to keep long-running sessions on a machine that has a publicly addressable IP. I don't want to have to worry about remote connections all the time.

It would be even better if this could be toggled at runtime: you could decide you want to invite someone, then you allow their IP, then you close again.

So I think the ideal design would be:

  • by default, closed to remotes (like now)
  • --allow-remote=IP: allows this IP to connect.
  • --allow-remote='all': fully open to any remote connection.

And having the allow/disable remotes be an internal IPython method would be great, so that one can do this at runtime too (both allowing and closing).

How does that sound?

@minrk
Member
minrk commented Oct 13, 2010

I pushed an implementation of this into my newkernel. It involves adding a file into IPython.utils that builds LOCALHOST and a list of IPs pointing to the current machine called LOCAL_IPS which can be used for checking whether a connection is really local.

http://github.com/minrk/ipython/commit/0641149f37f1de87743c4338cda4d3715b9da38d

The rest of the implementation involves propagating the existing --ip flag in base_launch_kernel up through the various launch/start_kernel methods, so that the frontend '--ip' flag actually makes sense when the kernel is not external.

http://github.com/minrk/ipython/commit/cd37162666ee713076f79c77c4e49e555fbacc6a

It sounds like you want to be able to allow connections from specific clients, but that just isn't possible at the socket level. We would have to build all of the identifying machinery into our messaging system (e.g. via uuid1 based session IDs), and implementing security for the entire messaging system is a bit beyond the scope of this interface bugfix. Are we ready to start that discussion?

All I did was fix the '--ip' flag for the listening kernel, which didn't work unless it was loopback before. That means you can now listen on 192.168.x.y (enet) OR 127.0.0.1 (loopback) OR 0.0.0.0 (all interfaces). What you can't do is listen on more than one interface but less than all of them. If we change the ip argument from an interface to a list of interfaces, then we can reasonably build in the option to have multiple interfaces, and then runtime-removable interfaces make sense.

The default is still loopback, so unless users specify to listen on an externally visible IP, it's not vulnerable.

@fperez
Member
fperez commented Oct 14, 2010

Mmh, this is weird. I'd written a long comment about this a couple of days ago, and now I don't see it! Very odd.

Reconstructing from memory, I think my main points were:

  • implementation looks good to me.
  • I'm a little worried the logic isn't trivial and we have no real documentation for this other than in --help. I know it's not your fault because we don't have yet any docs for the qtconsole, but if you think you can start even a sketch of the docs that could be a dump of the --help output along with the explanation above, that would be great.

Otherwise, I'm happy to merge #164 and close this one. Let me know if you think you can do the little doc stub (whatever you have time for) and we'll proceed.

Thanks for the great work!

@minrk
Member
minrk commented Oct 14, 2010

Yes, I thought I submitted the above comment before, but it didn't post, and I had to redo it a few hours later when I noticed. I assumed I just forgot to hit the button.

I think the logic is fairly straightforward, but there does seem to be an increasing number of states to switch between.

I just added a little bit of text to --help in #164, and I'll start a skeleton doc, hopefully by this weekend.

@fperez
Member
fperez commented Oct 22, 2010

What's the status of this one? It seems to me that since we've closed #164 with these enhancements, this one can be closed, no?

@minrk
Member
minrk commented Oct 22, 2010

Yes, this is fixed as part of #164. Closing...

@jtriley jtriley pushed a commit to jtriley/ipython that referenced this issue Jul 30, 2011
@minrk minrk Possible fix for GH-169 cd37162
@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
@minrk minrk Possible fix for GH-169 f4c76df
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment