Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

Turn StrictHostKeyChecking on for SSH #534

Open
nathanleclaire opened this issue Feb 12, 2015 · 18 comments
Open

Turn StrictHostKeyChecking on for SSH #534

nathanleclaire opened this issue Feb 12, 2015 · 18 comments

Comments

@nathanleclaire
Copy link
Contributor

If you run something like docker-machine -D ssh dev ls you'll see the actual SSH command we are running behind the scenes:

$ docker-machine -D ssh dev ls
DEBU[0000] executing: ssh -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=quiet -p 65069 -i /Users/nathanleclaire/.docker/machines/dev/id_rsa docker@localhost ls

Some of this looks OK (although we probably don't need the port forward to localhost any more), but instead of turning off strict host key checking (a security feature to prevent MITM attacks) we should probably be automatically adding the host to the user's known_hosts file instead. The -o StrictHostKeyChecking=no was probably OK for local VMs, but the whole game changes when we're SSHing out to machines on the public internet.

@ehazlett
Copy link
Contributor

+1. I had the known_hosts stuff adding when we were using IdentityAuth.
I'll add this to the list. Thanks

On Thu, Feb 12, 2015 at 12:19 PM, Nathan LeClaire notifications@github.com
wrote:

If you run something like docker-machine -D ssh dev ls you'll see the
actual SSH command we are running behind the scenes:

$ docker-machine -D ssh dev lsDEBU[0000] executing: ssh -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=quiet -p 65069 -i /Users/nathanleclaire/.docker/machines/dev/id_rsa docker@localhost ls

Some of this looks OK (although we probably don't need the port forward to
localhost any more), but instead of turning off strict host key checking
(a security feature to prevent MITM attacks) we should probably be
automatically adding the host to the user's known_hosts file instead. The -o
StrictHostKeyChecking=no was probably OK for local VMs, but the whole
game changes when we're SSHing out to machines on the public internet.


Reply to this email directly or view it on GitHub
#534.

@sthulb
Copy link
Contributor

sthulb commented Feb 12, 2015

We could also drop shelling out to SSH.

@ehazlett
Copy link
Contributor

Yes I'm planning this as part of the core refactor.

@sthulb
Copy link
Contributor

sthulb commented Feb 12, 2015

Me too. Although it's a large bit of work in terms of security.

@sthulb
Copy link
Contributor

sthulb commented Feb 12, 2015

Short term should be to implement this issue and then long term remove it in favour of our own client.

@nathanleclaire
Copy link
Contributor Author

Wait, what would we do instead? I don't think we want to include a whole SSH / readline implementation in the machine code, why not use what's there?

@sthulb
Copy link
Contributor

sthulb commented Feb 12, 2015

The crypto/ssh package will do 90% of the job.

@SvenDowideit
Copy link
Contributor

or, pull ssh out entirely, and use docker run

@md5
Copy link

md5 commented Mar 2, 2015

@nathanleclaire how were you thinking the host's SSH key would be retrieved to be added to known_hosts?

I was thinking it might be interesting to add SSH host keys to new machine hosts instead of allowing random keys to be generated and accepting them blindly. It looks like cloud-init supports this via the ssh_keys setting.

@ehazlett
Copy link
Contributor

ehazlett commented Mar 2, 2015

@md5 yes cloudinit does this. as a matter of fact, i used this in my cloudinit branch.

@md5
Copy link

md5 commented Mar 2, 2015

👍

@sthulb
Copy link
Contributor

sthulb commented Mar 2, 2015

My SSH branch will undo any changes to the existing implementation. I have host key checking in my branch.

@ehazlett
Copy link
Contributor

ehazlett commented Mar 3, 2015

@sthulb +1 -- i think we should handle this as part of the crypto/ssh refactor. How close is your branch to being ready?

@sthulb
Copy link
Contributor

sthulb commented Mar 3, 2015

I just have to change 7 drivers and ensure host key checking actually works.

@ehazlett
Copy link
Contributor

ehazlett commented Mar 3, 2015

ah ok. thanks.

@jeanlaurent
Copy link
Member

What should we do when we remove a machine, it would be better to remove it.
But then it starts to be complicated...

are we still willing to add the host to the known_hosts file ? cc @nathanleclaire

@nathanleclaire
Copy link
Contributor Author

What should we do when we remove a machine, it would be better to remove it.
But then it starts to be complicated...

My suggestion would be that Machine should maintain and use its own known_hosts file when using OpenSSH (e.g. ~/.docker/machine/known_hosts). We could set it using -o UserKnownHosts and remove the relevant host line ourself when a user makes a rm call.

@dgageot dgageot added area/ssh and removed area/ssh labels Feb 12, 2016
@brettdh
Copy link

brettdh commented Apr 13, 2016

Any updates on this issue? It seems like a pretty big deal, and it's been open for a long time now.

tomeon pushed a commit to tomeon/machine that referenced this issue May 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants