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

Generate Random SSH key on `vagrant up` #4707

Merged
merged 5 commits into from Oct 24, 2014

Conversation

Projects
None yet
@mitchellh
Member

mitchellh commented Oct 24, 2014

This PR makes it so that on vagrant up, Vagrant will randomly generate a keypair and use that if the insecure keypair is detected. This adds an extra level of security when doing things such as bridge networking.

Fixes GH-2608

mitchellh added a commit that referenced this pull request Oct 24, 2014

Merge pull request #4707 from mitchellh/f-dynamic-rekey
Generate Random SSH key on `vagrant up`

@mitchellh mitchellh merged commit 6a5e56f into master Oct 24, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details

@mitchellh mitchellh deleted the f-dynamic-rekey branch Oct 24, 2014

@virtuald

This comment has been minimized.

Show comment
Hide comment
@virtuald

virtuald Oct 24, 2014

Should mention that this behavior is new as of Vagrant x.y ...

Should mention that this behavior is new as of Vagrant x.y ...

This comment has been minimized.

Show comment
Hide comment
@aspiers

aspiers replied Oct 29, 2014

+1

@mbrodala

This comment has been minimized.

Show comment
Hide comment
@mbrodala

mbrodala Oct 27, 2014

Contributor

Does this mean that one cannot use the same insecure private key to access arbitrary boxes anymore?

Contributor

mbrodala commented Oct 27, 2014

Does this mean that one cannot use the same insecure private key to access arbitrary boxes anymore?

@mitchellh

This comment has been minimized.

Show comment
Hide comment
@mitchellh

mitchellh Oct 27, 2014

Member

@mbbx6spp Correct. When it is packaged, the new key is put in (and the Vagrantfile with the box is configured with the new key). You can always disable this behavior with config.ssh.insert_key

Member

mitchellh commented Oct 27, 2014

@mbbx6spp Correct. When it is packaged, the new key is put in (and the Vagrantfile with the box is configured with the new key). You can always disable this behavior with config.ssh.insert_key

@mbbx6spp

This comment has been minimized.

Show comment
Hide comment
@mbbx6spp

mbbx6spp Oct 27, 2014

Contributor

I think you meant to mention @mbrodala. Cheers. :)

Contributor

mbbx6spp commented Oct 27, 2014

I think you meant to mention @mbrodala. Cheers. :)

@mitchellh

This comment has been minimized.

Show comment
Hide comment
@mitchellh

mitchellh Oct 28, 2014

Member

GitHub auto-correct fail. Thanks. :)

Member

mitchellh commented Oct 28, 2014

GitHub auto-correct fail. Thanks. :)

@glensc

This comment has been minimized.

Show comment
Hide comment
@glensc

glensc Dec 14, 2014

Contributor

Soon, someone will complain that rsa key with 2048 bits is too weak, and they want dsa or 4096 key :)

https://github.com/mitchellh/vagrant/blob/b37c031bc34656400b6fdc1192bb403b08341eb8/lib/vagrant/util/keypair.rb#L14

Contributor

glensc commented Dec 14, 2014

Soon, someone will complain that rsa key with 2048 bits is too weak, and they want dsa or 4096 key :)

https://github.com/mitchellh/vagrant/blob/b37c031bc34656400b6fdc1192bb403b08341eb8/lib/vagrant/util/keypair.rb#L14

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Dec 28, 2014

I fail to see the point in this change. The password of most (all?) public base boxes is still "vagrant/vagrant" with full passwordless sudo access.
No security has been gained by randomizing the keys.
There's just a loss of convenience and another system to maintain.

I just stumbled on this when I re-associated a VirtualBox that got lost for some time.
Vagrant finds the box again, but ever since, "vagrant ssh" requires a password and vagrant up/resume keep spamming "Authentication failure" until I kill the ruby process.

I think very few people will expose port 22 of the vagrant box to an insecure network anyway.
And if they simply bridge the machine to an insecure network, they'd have to deal with all the issues that this entails, beginning with installing security updates on every single box instance.

tl;dr
This change doesn't increase security, but reduces convenience.

Can this be turned into an option that defaults to using the well-known key, please?

TomyLobo commented Dec 28, 2014

I fail to see the point in this change. The password of most (all?) public base boxes is still "vagrant/vagrant" with full passwordless sudo access.
No security has been gained by randomizing the keys.
There's just a loss of convenience and another system to maintain.

I just stumbled on this when I re-associated a VirtualBox that got lost for some time.
Vagrant finds the box again, but ever since, "vagrant ssh" requires a password and vagrant up/resume keep spamming "Authentication failure" until I kill the ruby process.

I think very few people will expose port 22 of the vagrant box to an insecure network anyway.
And if they simply bridge the machine to an insecure network, they'd have to deal with all the issues that this entails, beginning with installing security updates on every single box instance.

tl;dr
This change doesn't increase security, but reduces convenience.

Can this be turned into an option that defaults to using the well-known key, please?

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Jan 2, 2015

@niclasgelin referenced this pull request in a commit that adds "config.ssh.insert_key = false" to a file that looks like a Vagrantfile.
Can someone confirm that this indeed a recommended way to disable the entire feature?

TomyLobo commented Jan 2, 2015

@niclasgelin referenced this pull request in a commit that adds "config.ssh.insert_key = false" to a file that looks like a Vagrantfile.
Can someone confirm that this indeed a recommended way to disable the entire feature?

@patrickheeney

This comment has been minimized.

Show comment
Hide comment
@patrickheeney

patrickheeney Jan 28, 2015

Lost 4 hours today because of this. config.ssh.insert_key = false should have a big warning somewhere to let everyone know how to disable this feature.

patrickheeney commented Jan 28, 2015

Lost 4 hours today because of this. config.ssh.insert_key = false should have a big warning somewhere to let everyone know how to disable this feature.

@mbrodala

This comment has been minimized.

Show comment
Hide comment
@mbrodala

mbrodala Jan 28, 2015

Contributor

@TomyLobo Yes, that's currently the only way. As @mitchellh pointed out, you don't even have to put this into every Vagrantfile since Vagrantfiles are also loaded from your home directory.

Contributor

mbrodala commented Jan 28, 2015

@TomyLobo Yes, that's currently the only way. As @mitchellh pointed out, you don't even have to put this into every Vagrantfile since Vagrantfiles are also loaded from your home directory.

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Jan 29, 2015

I cannot control my users' home directory contents, but i can control their Vagrantfiles.

I still think this is a bad idea, btw. It's snake oil, at best and, as @patrickheeney and I found out the hard way, a waste of time at worst.
Please make "config.ssh.insert_key = false" the default again.

TomyLobo commented Jan 29, 2015

I cannot control my users' home directory contents, but i can control their Vagrantfiles.

I still think this is a bad idea, btw. It's snake oil, at best and, as @patrickheeney and I found out the hard way, a waste of time at worst.
Please make "config.ssh.insert_key = false" the default again.

@tdk2fe

This comment has been minimized.

Show comment
Hide comment
@tdk2fe

tdk2fe Feb 27, 2015

Hey everybody -

Not sure if there has been any more discussion around this, but I did suffer some of the same problems as @TomyLobo and @patrickheeney described. I've been running vagrant for a while now, but I set up my boxes in directories out of a DAS location that's accessible via multiple users. So while my home directory (and vagrant config) is /home/user/.vagrant, I have a Vagrants folder in $HOME that links to /mnt/MSONIC_EXTARRAY/Virtualization/Vagrants, which is where I set up new boxes.

After not using it for a couple months, I spent some time trying to figure out the message regarding incorrect ownership of the private key used to connect.

I did verify that adding config.ssh.insert_key = false resolves this error.

tdk2fe commented Feb 27, 2015

Hey everybody -

Not sure if there has been any more discussion around this, but I did suffer some of the same problems as @TomyLobo and @patrickheeney described. I've been running vagrant for a while now, but I set up my boxes in directories out of a DAS location that's accessible via multiple users. So while my home directory (and vagrant config) is /home/user/.vagrant, I have a Vagrants folder in $HOME that links to /mnt/MSONIC_EXTARRAY/Virtualization/Vagrants, which is where I set up new boxes.

After not using it for a couple months, I spent some time trying to figure out the message regarding incorrect ownership of the private key used to connect.

I did verify that adding config.ssh.insert_key = false resolves this error.

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Feb 27, 2015

I still think it's pointless to default this to true. If you care about security, you can activate it and change the passwords of everything on the box while you're at it.

Confusing users is bad. Adding false security is bad.
I haven't heard a good side to this patch yet.

Btw, it could be argued that inserting any key reduces security

TomyLobo commented Feb 27, 2015

I still think it's pointless to default this to true. If you care about security, you can activate it and change the passwords of everything on the box while you're at it.

Confusing users is bad. Adding false security is bad.
I haven't heard a good side to this patch yet.

Btw, it could be argued that inserting any key reduces security

@hardyoyo

This comment has been minimized.

Show comment
Hide comment
@hardyoyo

hardyoyo Mar 2, 2015

While it has been closed as a duplicate, #5407 is worth mentioning here. From the looks of things, #4707 breaks the base box packager command.

hardyoyo commented Mar 2, 2015

While it has been closed as a duplicate, #5407 is worth mentioning here. From the looks of things, #4707 breaks the base box packager command.

highb added a commit to puppetlabs/beaker that referenced this pull request Mar 13, 2015

Stop vagrant from generating ssh key
Hashicorp added new behavior that generates a new SSH key
when generating a box, and will remove any existing "insecure"
key found on a box. See hashicorp/vagrant#4707

This has the unfortunate side effect of breaking the automation
used in Beaker to setup SSH keys, since it relied on the default
"insecure" vagrant keys being used. Current workaround is to disable
the new Vagrant behavior with a configuration option.

highb added a commit to puppetlabs/beaker that referenced this pull request Mar 16, 2015

(BRK-148) Stop vagrant from generating ssh key
Hashicorp added new behavior that generates a new SSH key
when generating a box, and will remove any existing "insecure"
key found on a box. See hashicorp/vagrant#4707

This has the unfortunate side effect of breaking the automation
used in Beaker to setup SSH keys, since it relied on the default
"insecure" vagrant keys being used. Current workaround is to disable
the new Vagrant behavior with a configuration option.

roooms added a commit to basho-labs/riak-ruby-vagrant that referenced this pull request Apr 9, 2015

Install vagrant insecure public key
Otherwise you need to "vagrant ssh" to the node from the repo directory or mess with new ssh key by passing it in to the ssh command or adding it with ssh-add. (hashicorp/vagrant#4707)
@obestwalter

This comment has been minimized.

Show comment
Hide comment
@obestwalter

obestwalter Apr 13, 2015

I agree with @TomyLobo and others here. I don't see the value of this either. Vagrant is for devboxes and they don't have to be secure. I think this will cause a lot of confusion and calls for help - e.g. https://groups.google.com/forum/#!topic/vagrant-up/FeXfcxpQlVA

This would not be a problem if the default was set to false.

obestwalter commented Apr 13, 2015

I agree with @TomyLobo and others here. I don't see the value of this either. Vagrant is for devboxes and they don't have to be secure. I think this will cause a lot of confusion and calls for help - e.g. https://groups.google.com/forum/#!topic/vagrant-up/FeXfcxpQlVA

This would not be a problem if the default was set to false.

@BattleBrisket

This comment has been minimized.

Show comment
Hide comment
@BattleBrisket

BattleBrisket Apr 21, 2015

Adding my voice to the choir; changing the default insecure key behavior was a bad decision. Vagrant is intended to build wide open boxes for development purposes. While I can appreciate the intent that @mitchellh describes in #5005....

The reason we made the change is because historically it hasn't mattered, but today more and more people use Vagrant with remote providers globally accessible via the internet (no longer host-only), and this security risk was still an issue with bridged networking (though, much smaller surface area).

By that logic, we should also randomize the password assigned to the vagrant user account; a cursory search of project PR's and issues indicates such a change isn't forthcoming.

Generating a random pair is security by obscurity at best. At worst, it's simply a waste of developer time, as I now have to go back and refactor every Vagrantfile in every project, and I have to be sure to add yet another config flag to new Vagrantfile's moving forward.

Again, I appreciate there's a new user base with higher security requirements, and everyone understands the value of security, but let's not forget the problem we're trying to solve here. If I want to use Vagrant in some kind of public environment, I would reasonably expect to do some extra legwork to make it more secure, given the design intentions.

BattleBrisket commented Apr 21, 2015

Adding my voice to the choir; changing the default insecure key behavior was a bad decision. Vagrant is intended to build wide open boxes for development purposes. While I can appreciate the intent that @mitchellh describes in #5005....

The reason we made the change is because historically it hasn't mattered, but today more and more people use Vagrant with remote providers globally accessible via the internet (no longer host-only), and this security risk was still an issue with bridged networking (though, much smaller surface area).

By that logic, we should also randomize the password assigned to the vagrant user account; a cursory search of project PR's and issues indicates such a change isn't forthcoming.

Generating a random pair is security by obscurity at best. At worst, it's simply a waste of developer time, as I now have to go back and refactor every Vagrantfile in every project, and I have to be sure to add yet another config flag to new Vagrantfile's moving forward.

Again, I appreciate there's a new user base with higher security requirements, and everyone understands the value of security, but let's not forget the problem we're trying to solve here. If I want to use Vagrant in some kind of public environment, I would reasonably expect to do some extra legwork to make it more secure, given the design intentions.

@rasputnik

This comment has been minimized.

Show comment
Hide comment
@rasputnik

rasputnik Jun 3, 2015

Random keys main issue is the breaking of automation. If someone can suggest a clean way to achieve that, I'd be grateful.

(and agree this is a pain with no real benefit until the vagrant/vagrant login details are resolved).

rasputnik commented Jun 3, 2015

Random keys main issue is the breaking of automation. If someone can suggest a clean way to achieve that, I'd be grateful.

(and agree this is a pain with no real benefit until the vagrant/vagrant login details are resolved).

@Gerst20051

This comment has been minimized.

Show comment
Hide comment
@Gerst20051

Gerst20051 Jun 29, 2015

👍 adding config.ssh.insert_key = false to the Vagrantfile and removing the vm private key automatically updated vagrant ssh-config with the correct private key ~/.vagrant.d/insecure_private_key. The last thing I had to do was update the authorized keys file on the vm. curl https://raw.githubusercontent.com/mitchellh/vagrant/master/keys/vagrant.pub > ~/.ssh/authorized_keys

Gerst20051 commented Jun 29, 2015

👍 adding config.ssh.insert_key = false to the Vagrantfile and removing the vm private key automatically updated vagrant ssh-config with the correct private key ~/.vagrant.d/insecure_private_key. The last thing I had to do was update the authorized keys file on the vm. curl https://raw.githubusercontent.com/mitchellh/vagrant/master/keys/vagrant.pub > ~/.ssh/authorized_keys

@rasputnik

This comment has been minimized.

Show comment
Hide comment
@rasputnik

rasputnik Jun 30, 2015

yep, config.ssh.insert_key = false prevents this becoming a problem. If that was the default, I'd have no issue with this feature (it's not for me but that's fine if someone thinks its useful).

rasputnik commented Jun 30, 2015

yep, config.ssh.insert_key = false prevents this becoming a problem. If that was the default, I'd have no issue with this feature (it's not for me but that's fine if someone thinks its useful).

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Jun 30, 2015

Last developer comment is from 28 Oct 2014.
I think they're sitting this one out...

TomyLobo commented Jun 30, 2015

Last developer comment is from 28 Oct 2014.
I think they're sitting this one out...

@ddelrio1986

This comment has been minimized.

Show comment
Hide comment
@ddelrio1986

ddelrio1986 Jul 20, 2015

Was just bitten by this! Wasted so much time on it!

ddelrio1986 commented Jul 20, 2015

Was just bitten by this! Wasted so much time on it!

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Jul 26, 2015

@mitchellh Can you please make config.ssh.insert_key default to false to stop wasting people's time.

TomyLobo commented Jul 26, 2015

@mitchellh Can you please make config.ssh.insert_key default to false to stop wasting people's time.

@bradisbell

This comment has been minimized.

Show comment
Hide comment
@bradisbell

bradisbell Jul 27, 2015

You simply need to add config.ssh.insert_key = false into your Vagrantfile as explained in https://docs.vagrantup.com/v2/vagrantfile/ssh_settings.html.

This method is not sufficient. I share a Vagrantfile with many other developers. This is a setting that should be configurable Vagrant-wide on my host, and not per Vagrantfile so that the hacks I have to use to make Vagrant work again do not affect them.

bradisbell commented Jul 27, 2015

You simply need to add config.ssh.insert_key = false into your Vagrantfile as explained in https://docs.vagrantup.com/v2/vagrantfile/ssh_settings.html.

This method is not sufficient. I share a Vagrantfile with many other developers. This is a setting that should be configurable Vagrant-wide on my host, and not per Vagrantfile so that the hacks I have to use to make Vagrant work again do not affect them.

@assunluis80

This comment has been minimized.

Show comment
Hide comment
@assunluis80

assunluis80 Aug 17, 2015

Where I can find the file to configure config.ssh.insert_key? I'm using Ubuntu 14.04 LTS.

assunluis80 commented Aug 17, 2015

Where I can find the file to configure config.ssh.insert_key? I'm using Ubuntu 14.04 LTS.

@TomyLobo

This comment has been minimized.

Show comment
Hide comment
@TomyLobo

TomyLobo Aug 17, 2015

There is no central file for this. You have to do this in every single bloody Vagrantfile.

TomyLobo commented Aug 17, 2015

There is no central file for this. You have to do this in every single bloody Vagrantfile.

@mbrodala

This comment has been minimized.

Show comment
Hide comment
@mbrodala

mbrodala Aug 20, 2015

Contributor

@TomyLobo Not exactly true, you can also put this in a ~/.vagrant.d/Vagrantfile to apply it to all machines on your desktop.

Contributor

mbrodala commented Aug 20, 2015

@TomyLobo Not exactly true, you can also put this in a ~/.vagrant.d/Vagrantfile to apply it to all machines on your desktop.

stackedsax added a commit to stackedsax/dcos-vagrant that referenced this pull request Jul 27, 2016

Fix the ssh identity issue with Vagrant > 1.7.0
See hashicorp/vagrant#4707 for history

It seems like making this change would help people spin up this demo just a little bit easier

xyu added a commit to xyu/heroku-wp that referenced this pull request Feb 21, 2017

Force User/Pass SSH
Don't use the more secure auto generated key because the default password auth
is enabled anyway so keys are not any more secure. Keys does end up causing
problems between different Vagrant versions as 1.7.x generates a new random key
per box as opposed to using the insecure key.

See:
hashicorp/vagrant#4707
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment