hashicorp / packer

Packer is a tool for creating identical machine images for multiple platforms from a single source configuration.
http://www.packer.io
Other
15.05k stars 3.32k forks source link

Proposal: Ephemeral SSH key pair for VirtualBox builders #7225

Closed stephen-fox closed 5 years ago

stephen-fox commented 5 years ago

Hello,

My team wants to avoid using a known secret when building VirtualBox images. I have seen several packer examples create a new Linux account and set its password to a value stored in the packer template file. This creates a few problems for our team:

I noticed that the debugging documentation states the following:

In debug mode once the remote instance is instantiated, Packer will emit to the
current directory an ephemeral private ssh key as a .pem file.
(...)
The key will only be emitted for cloud-based builders.

This appears to indicate that packer creates an ephemeral SSH key pair for cloud-based builders.

My question is: Would the packer maintainers be open to an enhancement to the VirtualBox builder so that, like the cloud builders, it creates a fresh SSH key pair during the build? The public key would be accessible via a template variable so that it can be injected into the machine image.

To be clear, I am not asking for Hashicorp to write this feature - I am happy to take a crack at it. I just wanted to see if the maintainers are open to the feature proposal.

Thank you, Stephen

SwampDragons commented 5 years ago

That sounds fine to me! I'd welcome the PR.

stephen-fox commented 5 years ago

Awesome, thank you for the quick response!

I will get working on it soon(tm) :)

rickard-von-essen commented 5 years ago

This would be very good and has been on mind for a long time.

But how do you purpose to implement this?

For virtualbox-ovf there is no way to pass the key befor SSH. For virtualbox-iso you can already do this by preprocessing the kickstart/preseed file and insert the pubkey for a ephemeral ssh key. This could be handled more gracefully by Packer by automating this for you, but that would require templating of http_directory files something we before avoided.

One option that would be to automatically create an ISO and attach to the VM containing a cloud-init config drive. Then the only requirement for the VM would be to have cloud-init installed. This could work for both virtualbox-iso and virtualbox-ovf and for all other local builders.

Or do you have any other suggestion?

chrismarget commented 5 years ago

I'm working with @stephen-fox on this, might as well chime in.

For virtualbox-iso you can already do this by preprocessing the kickstart/preseed file and insert the pubkey for a ephemeral ssh key.

We're using virtualbox-iso, and are pre-generating the key already. We just don't love the dependency on wrapping/preprocessing, would like packer to just do this part. We saw a pattern in the cloud builders, liked the idea of adding key-generating parity to virtualbox-iso.

This could be handled more gracefully by Packer by automating this for you

Yes

but that would require templating of http_directory files something we before avoided

Snagging the trusted public key via HTTP is one possibility. We deliberately didn't mention the mechanism in the original query because, hey, why be prescriptive about it?

Right now, we're actually typing the key on the kernel command line :)

For this strategy to work the only requirement is that the public key be available via template variable. We're including the prepared public key via user variable:

{
  "builders": [
    {
      "type": "virtualbox-iso",
      "boot_command": [
        "<up><wait><tab> text ks=http://{{ .HTTPIP }}:{{ .HTTPPort}}/ks.cfg PACKER_USER={{ user `username` }} PACKER_ECDSA={{ user `ssh-key` }}<enter>"
    }
  ],
  "variables": {
    "username": "packer",
    "ssh-key": "AAAAE2VjZHN<snip>yHpdz9Z13toZseXg="
  }

}
rickard-von-essen commented 5 years ago

A slight variant of that, instead of passing the whole pubkey in the boot command could be to just create a temp file in the http_directory that you can access with a template key, say {{ .SSHPubKey }}, this would expand to something like http://<http ip>:<http port>//id_rsa-packer-5417.pub.

chrismarget commented 5 years ago

@rickard-von-essen Yep, we understand that approach.

Our initial thought on the matter (while trying to avoid wrapping packer with a preprocessor script) was to leverage a pre-build hook that we assumed was available in packer. We then found some feature requests for pre-build modules that were shot down because they weren't aligned with the sorts of jobs that packer should be doing.

With that in mind, we're trying to tread lightly feature-wise.

If having the builder drop a trusted key on disk is acceptable (@SwampDragons ?) , then we're interested in exploring that.

Two approaches spring to mind:

  1. You proposed having a variable that reveals where packer has created the public key (presumably in the http_directory.)
  2. Use a builder config key like ssh_pubkey_outfile to specify the file where packer should write the public key.

Option 2 is my preference:

So, I'm leaning toward having packer generate the keypair, and make the public key available two ways:

Additionally, we plan on writing private key to disk when debug is enabled (same behavior as the cloud builders).

The only outstanding question about the ssh_pubkey_outfile behavior as I've proposed it: Should the builder clean up after itself?

rickard-von-essen commented 5 years ago

But how should the VM reach the pub key unless it's in the http_directory. I think that is the only place that makes sense.

chrismarget commented 5 years ago

But how should the VM reach the pub key unless it's in the http_directory

There are plenty of options here. Bear with me for a moment while I go off the deep end...

Pass the build host's webcam through to the VM. After the pubkey file appears in the specified location, a script on the build host contracts a skywriting company to paint the key data in the camera's field of view. Then it's just a simple matter of OCR...

That ridiculous scheme isn't appropriate to my use case, but there are lots of network services other than packer's webserver that could express the key to the new VM.

Writing the key data in a location that only packer knows seems needlessly limiting. Is the http_directory even writable? It seems like a bold assumption.

I'm still hoping @SwampDragons will weigh in on whether writing the key to disk is acceptable in the first place.

Keeping it in memory is a lighter lift and satisfies our use case just fine.

SwampDragons commented 5 years ago

Pass the build host's webcam through to the VM. After the pubkey file appears in the specified location, a script on the build host contracts a skywriting company to paint the key data in the camera's field of view. Then it's just a simple matter of OCR...

😂

So you've already noticed that we generally only drop the key on the disk during debug mode, when we're using the cloud builders. But that's the private key, not the public one.

I think writing the public key to disk is fine. Normally I'd say to use the packer tempdir functions to do this, but since you want it accessible to the vm, rickard's right that http_dir is the way to go.

I don't see anything particularly wrong with this; We should certainly stat the http_dir for a similarly named file and fail if there's a name collision. And we should add a note in the docs saying this is the mechanism by which we pass they key. If you're envisioning this key being temporary (which I suspect you are because that's how we do it with the cloud builders) then we also need to make sure we remove the key as part of the cleanup of the ssh key generating step.

chrismarget commented 5 years ago

Thanks, @SwampDragons.

But that's the private key, not the public one

Indeed, I'd never have proposed that a skywriter publish a private key. I'm not a madman!

:)

My main concern about writing the key to disk at all wasn't a security angle, but a packer philosophy angle. As I said, we want to tread lightly, thought you might not accept a builder that mucks around on disk anywhere other than the cache/output directories.

So, you're leaning toward option 1: The builder writes the key to http_dir using an unpredictable name?

For the reasons I've already articulated, I'd much rather specify (option 2) where the key should be written rather than discover where it's been written. Practically speaking, I'll almost certainly specify that the key file be written within the http_dir, but would like to have the option of dropping it elsewhere if, for example, the packer environment is shared to me via read-only NFS.

I think @stephen-fox would like to have the option of just not writing the write-to-disk code, and keeping everything in memory.

We're on the same page with respect to filename collision and cleanup.

SwampDragons commented 5 years ago

Oh, thanks for asking for clarification -- I'm actually leaning towards @stephen-fox's opinion on this. I tend to prefer keeping things in memory where I can, especially for temporary things that don't need to be shared outside the program.

However, since you're the one implementing, you're the one with the real-life use case and the well-developed opinions about how this makes the most sense to implement.

But here are a couple of trade-offs to consider:

I think I'm happy with any of these solutions; none of them sound particularly heavy-handed or requiring of intense architectural changes.

chrismarget commented 5 years ago

Awesome, thanks for elaborating.

Keeping it all in memory and banging it out with keypresses is our favorite option. It's what we're doing now, and it seems to be working fine, at least on the one macbook where we've actually done it. Thanks for the heads-up about the risks there. We'll be watching for it.

Writing to disk at a predictable location, with the option to override makes sense too. Your position about reasonable defaults makes good sense.

SwampDragons commented 5 years ago

Either way, it is probably a good idea to do the same check that the amazon builder does, where you write the private key to disk when debug mode is set.

chrismarget commented 5 years ago

Yup, that's part of the plan.

chrismarget commented 5 years ago

However, since you're the one implementing

@stephen-fox would like me to point out that he's the brains behind this operation.

rickard-von-essen commented 5 years ago

You could also instead of writing the pub key to disk just serve it from memory. This might need some more work to modify the HTTP server.

On Fri, Jan 25, 2019, 20:13 Chris Marget <notifications@github.com wrote:

However, since you're the one implementing

@stephen-fox https://github.com/stephen-fox would like me to point out that he's the brains behind this operation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hashicorp/packer/issues/7225#issuecomment-457690101, or mute the thread https://github.com/notifications/unsubscribe-auth/AAiCg8Cg_vshbHkoAijHoPUfMaHO8JsVks5vG1dSgaJpZM4aRT3f .

chrismarget commented 5 years ago

Magic HTTP file location that serves the SSH key?

We talked about something along these lines: Rather than serve the SSH key, serve a blob of cloud-init data from an embedded URL in the http service. It'd be a little bit like EC2's magic web server that feeds identity info into EC2 instances. I'm not sure how we'd steer the VM's cloud-init over to that magic service, but it sounded like a fun idea.

At any rate, that sort of capability is way beyond our aspirations.

stephen-fox commented 5 years ago

Don't listen to @chrismarget. This is 99% his fault!

@SwampDragons, I have been looking through the code the past few days. I noticed there is quite a bit of copy-pasta'd SSH key pair code between the builders (including the "write the private key to disk if debug" business logic).

I know very little about the history of the code base... Are there any plans to coalesce the copy pasted code? I feel pretty bad following this pattern. I could write some basic common code for generating a SSH key pair and stick it in the (repo-root)/common package for future use and refactoring efforts if that is something you are interested in.

Do you have any thoughts on this?

rickard-von-essen commented 5 years ago

@stephen-fox that sounds great (to remove some duplicated code). @azr did a great job doing exactly this for the ssh connection part of the code, but the "keypair" part was left for another time, see #6613. I would recommend doing this in a separate PR (from adding auto generating ssh key for VirtualBox).

SwampDragons commented 5 years ago

I agree with Rickard -- we do want to deduplicate that code, and if you want to do it I'd be eternally grateful -- but for my sanity's sake as the person who will probably be reviewing, I'd like it in a different PR than the one adding the new functionality to Virtualbox.

stephen-fox commented 5 years ago

I agree - replacing the old code with a new implementation should be done separately (that's going to be an eye-burner).

@rickard-von-essen, thank you for the info. That PR has some good history in it. I did not realize there was a helper package.

stephen-fox commented 5 years ago

@SwampDragons,

I have reached a point where we can write some end-user facing documentation about how the feature works. I noticed that the documentation website's source code is in the packer project (website/source/docs).

Should @chrismarget and myself take a stab at writing something before we submit a PR? Or would you like us to wait until we submit a PR, and finish code review?

SwampDragons commented 5 years ago

It's your choice what order you want to do it in; we don't merge the PR until the documentation is complete. I often like writing the docs first because they help me think through the problem from a user perspective, but it's totally up to you.

stephen-fox commented 5 years ago

Got it. We have a pretty good idea how this will work (well, at least we think we do). Chris and I will put some documentation together and go from there.

ghost commented 4 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.