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

Turn secrets.yml* into credentials.yml.enc #30006

Closed
dhh opened this issue Jul 30, 2017 · 12 comments
Closed

Turn secrets.yml* into credentials.yml.enc #30006

dhh opened this issue Jul 30, 2017 · 12 comments

Comments

@dhh
Copy link
Member

dhh commented Jul 30, 2017

The combination of config/secrets.yml, config/secrets.yml.enc, and SECRET_BASE_KEY is confusing. It's not clear what you should be putting in these secrets and whether the SECRET_BASE_KEY is related to the setup in general.

I'd like to propose that we deprecate secrets.yml* and instead adopt config/credentials.yml.enc to signify what these secrets are specifically for: Keeping API keys, database passwords, and any other integration credentials in one place.

This new file would have a flat format, not divided by environments, like secrets.yml has been. Most of the time, these credentials are only relevant in production, and if someone does need to have some keys duplicated for different environments, it can be done by hand.

This leaves us with what to do with SECRET_BASE_KEY. Here, I'd like to propose that we for test and development simply derive values for both based off a known constant (it could simply be the application name). It doesn't really matter. These secrets are not supposed to withstand any sort of attack in test or development.

It's only in production (and derivative environments, like exposed betas) where the secret actually needs to be secret. So we can simply insert this secret into the new flat credentials.yml.enc file. I'd suggest we think of a better name, though. Maybe something like app_key_generator_secret or the likes.

This change would also enable us to setup the encrypted credentials along with the new application skeleton, because even if you make a mistake and forget your key, it's only production that's impacted. So it won't slow down anyone just getting started with development or test.

Finally, this would leave us with a single RAILS_CREDENTIALS_KEY for services like Heroku to set. Once this has become the standard, they could even ask for this key when it's missing on the rails buildkit deploy, such that the setup is all smooth.

Note: We should just keep Rails.secrets and friends around. The Rails.credentials setup would be a new, concurrent approach. All new apps would use it, but we wouldn't need to screw existing apps.

@dhh dhh added the railties label Jul 30, 2017
@dhh
Copy link
Member Author

dhh commented Jul 30, 2017

cc @kaspth

@ccasabona
Copy link

It would be an improvement to allow all secrets to be in one file. We essentially do this now by using entries in the encrypted secrets file to populate entries in database.yml. We would not mourn the death of config/database.yml.

The division into environments in the current config/secrets.yml file is very convenient for us as we have test keys with the various 3rd party API's that we use. Changing to a flat file format would have to somehow accommodate this practice.

Insofar as SECRET_KEY_BASE is concerned, it would not matter to us if this was generated automatically in the test and development environments as suggested. Changing the name of this key isn't necessary from a semantic point of view but it would help document the historical change in Rails. How about APP_SECRET_KEY for a new name?

@kirs
Copy link
Member

kirs commented Jul 31, 2017

IMO an advantage that secrets.yml has over config/credentials.yml.enc is that in any environment, you could refer to Rails.secrets[:facebook_api_key] and it return the environment-local value. It kind of forced you to always put your credential into the secrets storage.

Correct me if I get the idea wrong, but it sounds like with a single-environment config/credentials.yml.enc developers would be more likely to hardcode conditions like:

api_key = Rails.env.production? ? Rails.credentials[:api_key] : "testtest"

@dhh
Copy link
Member Author

dhh commented Jul 31, 2017

Kirs, I don't think that's the case. You can well do Rails.credentials.dig(:facebook, Rails.env, :api_key), if you want to use a facebook: production/test: api_key format. But in many cases there won't be a test environment, so enforcing the structure on all keys isn't worth it.

@pavel-jurasek
Copy link

pavel-jurasek commented Aug 1, 2017

@dhh well in our case we using secrets in 3 environments for 3 different api keys and / or configuration specifics (different AWS API gateways) credentials would make sense but right know we have setup like this:

secrets.yml

test:
  foo: <%= ENV['FOO'] || 'default' %>

so in our test we can change configuration of application without need for gems like DotEnv

for configurations we using rails encrypted secrets and leveraging composition of secrets. E.g. for developers we do not need to do any changes in source code of the app only in our deployment pipe line we adding secrets.yml.enc who overrides defaults to final secrets per environments.

Environment: Development / Test

  • Only plain rails secrets.yml with possibility to re-configure rails secrets via ENV for unit tests.

Environment: Staging / UAT / Production

  • Using same secrets.yml as is describe above + adding environment specific overrides for specific environments (e.g. secrets.yml.enc).

Introducing credentials.yml would not be straight forward as it is now for us.

Alternative solution would be enabled multiple encrypted files and let developer make a decision?

@dhh
Copy link
Member Author

dhh commented Aug 1, 2017

The point with encrypted credentials would be to move away from ENVs. Having something called secrets.yml that isn't actually secret because it isn't actually encrypted has bugged me for a long time as well.

You can still do per-env statements through Rails.credentials.dig(:facebook, Rails.env, :api_key).

Anyway, I'm also happy to have the secrets infrastructure available in such a way that people can roll their own files by hand. But the initial focus will be on getting the flat, shared Rails.credentials approach boostrapped.

@morgoth
Copy link
Member

morgoth commented Aug 2, 2017

@dhh related to @kirs question #30006 (comment) how would you handle the case when in an example you have Rails.credentials.facebook_api_key pointing to test app in the development environment that should be accessible for all dev, which means it doesn't have to be encrypted, but it would be nice to not have to do conditionals like: Rails.credentials.facebook_api_key || "my-development-api-key" in the code?

About multiple encrypted files - I started bringing it to secrets in #29909 but I guess it could be addressed here instead?

@dhh
Copy link
Member Author

dhh commented Aug 2, 2017

@morgoth I don't think this has to solve all permutations of credential handling. We should seek to offer a great, default path that delivers on the number #1 problem: Keeping real credentials encrypted, so they don't float around in the source code, and give them a single place to live.

Having to do an either/or if you have a setup where only some developers have the secret key, and thus no power to deploy to production, seems fine to me.

@dhh
Copy link
Member Author

dhh commented Aug 4, 2017

Started the work on this in #30067

@freibuis
Copy link

I would like to make a suggestion here..

I really liked the approach you took with ActiveJob. this gave us 1 interface to use many styles of worker engines like sidekiq,sucker punch, delayed_job. May be make an credentials api like rails has done with ActiveJob. that way we could choice a method of choice but only have to learn 1 style of coding..

call it ActiveKms or ActiveSecrets <---naming is hard. lol

example. using hashicorp vault or aws's kms system. or rails credential file that is being worked on right now.

Just thinking out loud.

@kaspth
Copy link
Contributor

kaspth commented Sep 28, 2017

We have added ActiveSupport::EncryptedFile and ActiveSupport::EncryptedConfiguration that's encrypted via the config/master.key. Maybe that would help you to make your desired integration :)

@rails-bot rails-bot bot added the stale label Dec 28, 2017
@rails-bot
Copy link

rails-bot bot commented Dec 28, 2017

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-1-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants