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

Add support for encrypted environment variables #909

Closed
wants to merge 7 commits into from
Closed

Add support for encrypted environment variables #909

wants to merge 7 commits into from

Conversation

siliconcow
Copy link

Resolves #908 which I just opened.

Seemed simple enough to implement for our purposes though I understand if it can't be merged due to the added dependency etc.

David Challoner added 2 commits February 1, 2015 00:48
Signed-off-by: David Challoner <dchalloner@gmail.com>
Signed-off-by: David Challoner <dchalloner@gmail.com>
@hacfi
Copy link

hacfi commented Feb 1, 2015

👍 Would be so useful!!

Signed-off-by: David Challoner <dec@zestfinance.com>
@thaJeztah
Copy link
Member

If I understand correctly, the use case here is being able to store encrypted ENV-vars in your docker-compose.yml, and have compose decrypt it before using (if you pass the decryption key / secret)?

Personally I'm -1 on this; IMO it gives the impression that those values are "safe" (because they are encrypted), whereas they still get sent to the daemon (and added to the container) unencrypted.

If you want to keep out your "secrets" from your docker-compose.yml, compose offers other ways to do this. You can set the variables before running docker-compose up (see environment) or use an env_file that is not checked into version control.

I can see the convenience of this PR, but I think that encrypting/decrypting values in a docker-compose.yml could also be handled by an external application / helper tool that decrypts the values before passing it to compose in some way.

Just my 0.02c, I'm not a maintainer.

@hacfi
Copy link

hacfi commented Feb 1, 2015

@thaJeztah I get your point but docker-compose is a tool which helps you start your multi-container app with a single command so you don’t have to write your own tool. It’s a common use-case to have credentials in the environment and you don’t want to store them in plaintext. Imho docker-compose is a tool for convience and it would be great not to need an additional external application / helper tool for such an essential feature.

@thaJeztah
Copy link
Member

I fully understand that just about any system will need secrets, but keeping them in source control (even encrypted), just isn't the way to handle that. Handling secrets for deployment is not something new, and various solutions for that exist. You can Google for "keep passwords out of source control", here's an example; Handling secret credentials in Ruby on Rails

Besides, if you use an env-file and keep that out of source control, you only have to copy that file to your project directory before starting/deploying and compose will handle this automatically, without external tools.

@siliconcow
Copy link
Author

I understand there are different opinions on this but you need to keep your credentials somewhere. If not in version control then some other repository (i.e lastpass). This uses AES-256 with a fairly ridiculous number of rounds of PBKDF2 plus a salt so it's probably more secure than most roll-your-own solutions and eliminates the need to use a separate service. You got to keep things simple and low overhead otherwise people will do dumb things like use email or god forbid postit notes to pass credentials around.

@relwell
Copy link

relwell commented Feb 10, 2015

This looks like a useful PR, but I'd really like to see this get introduced on top of #845, instead of introducing additional complexity that we'll have to accommodate in order to provide a top-requested feature from the community. It shouldn't be too hard to add this logic into the new config module that that PR introduces.

@aanand
Copy link

aanand commented Feb 10, 2015

I'm -1 on this. It introduces a lot of complexity to serve a use case that, so far, no-one has asked for. I don't want to start encrypting anything on the user's behalf unless there's a really compelling argument for it.

@relwell
Copy link

relwell commented Feb 10, 2015

I only particularly care that it not be introduced before environment variable support.

I agree with the -1ers about the complexity it introduces.

@dnephin
Copy link

dnephin commented Feb 10, 2015

-1

Any "encrypted" value added this way would be exposed to everyone through docker inspect, so it's really not providing any kind of security. Encrypted values need to be decrypted inside the container itself.

@aanand
Copy link

aanand commented Feb 10, 2015

@dnephin also true, I hadn't considered that.

@hacfi
Copy link

hacfi commented Feb 10, 2015

I think the main use-case is that you keep the fig.yml in git/vcs without storing plain-text passwords in it so it doesn’t matter that they are exposed via docker inspect. But I accept that you don’t want to include this in fig so I’ll create a tool which will update the fig.yml before running fig up. #845 also looks promising.

@dnephin
Copy link

dnephin commented Feb 10, 2015

Fig and docker both support environment variables as only keys, and they both take the value from the environment, so I don't think you'd need to modify fig.yml. Just add the variable names to the environment list, and set the environment variables before running fig.

@aanand
Copy link

aanand commented Feb 10, 2015

They also both support environment variable files (docker: --env-file=myenvfile, fig.yml: env_file: myenvfile), so alternatively you can generate that on-the-fly before running fig up.

@hacfi
Copy link

hacfi commented Feb 10, 2015

Oh, sorry - wasn’t aware of that! In that case I can solve this in another way - thanks!

@thaJeztah
Copy link
Member

Oh, sorry - wasn’t aware of that! In that case I can solve this in another way - thanks!

You can find links to the documentation for them in my earlier comment: #909 (comment). Hope you get it working for your situation!

@hacfi
Copy link

hacfi commented Feb 10, 2015

Thanks..must have overseen that. Going through other issues/PRs I thought env_file was not merged yet.

@GordonTheTurtle
Copy link

Can you please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:Katlean/fig.git somewhere
$ cd somewhere
$ git rebase -i HEAD~6
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

This will update the existing PR, so you do not need to open a new one.

@dnephin dnephin closed this May 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for encrypted environment variables
7 participants