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 credentials using a generic EncryptedConfiguration class #30067

Merged
merged 48 commits into from Sep 11, 2017

Conversation

Projects
None yet
@dhh
Member

dhh commented Aug 3, 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.

This PR will 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 is 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. For test and development, we'll instead simply derive values for both based off a known constant. 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 will birth the new credentials.yml.enc file with secret_key_base set instead on new apps.

This change will 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_MASTER_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 will just keep Rails.secrets and friends around. The Rails.application.credentials setup will be a new, concurrent approach. All new apps would use it, but we wouldn't need to screw existing apps.

Work left:

  • Implement config.require_master_key instead of config.read_encrypted_secrets.
  • Alert at the end of "rails new app" that there is a master key, what it is, and that you should put it in your password manager.
  • Update all documentation to talk about credentials rather than secrets.
  • Update all railties tests to deal with missing secrets.yml and new master.key and credentials.yml.enc

@dhh dhh changed the title from WIP: Add credentials using a generic EncryptedConfiguration class to Add credentials using a generic EncryptedConfiguration class Aug 4, 2017

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Aug 4, 2017

Member
Member

dhh commented Aug 4, 2017

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Aug 4, 2017

Member

Thinking about this further, I wonder if there's even greater room for simplification. Maybe we can combine the secret_key_base with the RAILS_MASTER_KEY and then simply rely on a single master key that can live in that ENV or in config/master.key. Then we wouldn't have a separate secret_key_base, and all apps would be required to have either RAILS_MASTER_KEY env or config/master.key set. We could further more easily allow other encrypted configurations to piggy back off that env/file key, kinda like we do with the message_verifier generator.

Are there any security issues with using the RAILS_MASTER_KEY as the secret_key_base as well?

Member

dhh commented Aug 4, 2017

Thinking about this further, I wonder if there's even greater room for simplification. Maybe we can combine the secret_key_base with the RAILS_MASTER_KEY and then simply rely on a single master key that can live in that ENV or in config/master.key. Then we wouldn't have a separate secret_key_base, and all apps would be required to have either RAILS_MASTER_KEY env or config/master.key set. We could further more easily allow other encrypted configurations to piggy back off that env/file key, kinda like we do with the message_verifier generator.

Are there any security issues with using the RAILS_MASTER_KEY as the secret_key_base as well?

@dhh dhh requested a review from kaspth Aug 7, 2017

kaspth added some commits Aug 7, 2017

[ci skip] Keep wording to `key base`; prefer defaults.
Usually we say we change defaults, not "spec" out a release.

Can't use backticks in our sdoc generated documentation either.
Show outdated Hide outdated activesupport/lib/active_support/encrypted_configuration.rb
end
def config
@config ||= deserialize(read).deep_symbolize_keys

This comment has been minimized.

@kaspth

kaspth Aug 7, 2017

Member

Should we use ActiveSupport::InheritedOptions here on top of this? It would allow things like Rails.credentials.aws_access_key?

@kaspth

kaspth Aug 7, 2017

Member

Should we use ActiveSupport::InheritedOptions here on top of this? It would allow things like Rails.credentials.aws_access_key?

This comment has been minimized.

@dhh

dhh Aug 17, 2017

Member

👍

@dhh

dhh Aug 17, 2017

Member

👍

Show outdated Hide outdated activesupport/lib/active_support/encrypted_file.rb
test "change content by key file" do
@encrypted_file.write(@content)
@encrypted_file.change do |file|
file.write(file.read + " and went by the lake")

This comment has been minimized.

@kaspth

kaspth Aug 7, 2017

Member

Really dig the flow that Pathnames and the change method bring! 👏

@kaspth

kaspth Aug 7, 2017

Member

Really dig the flow that Pathnames and the change method bring! 👏

This comment has been minimized.

@dhh

dhh Aug 7, 2017

Member

Pathnames are mint!

@dhh

dhh Aug 7, 2017

Member

Pathnames are mint!

@@ -404,6 +404,32 @@ def secrets=(secrets) #:nodoc:
@secrets = secrets
end
# The secret_key_base is used as the input secret to the application's key generator, which in turn

This comment has been minimized.

@kaspth

kaspth Aug 7, 2017

Member

Had this lined up to deprecate secrets but looks like it's a bit more involved than that. Let's defer the deprecation to after this is merged.

ActiveSupport::Deprecation.warn(<<-end_of_message.strip_heredoc)
  `Rails.application.secrets` has been deprecated in favor of `Rails.application.credentials`
  and will be removed in Rails 6.0.

  See `bin/rails credentials --help` for more.
end_of_message

(Most trouble is just that our test suite relies on secrets, so will be seeing some deprecation warnings.)

@kaspth

kaspth Aug 7, 2017

Member

Had this lined up to deprecate secrets but looks like it's a bit more involved than that. Let's defer the deprecation to after this is merged.

ActiveSupport::Deprecation.warn(<<-end_of_message.strip_heredoc)
  `Rails.application.secrets` has been deprecated in favor of `Rails.application.credentials`
  and will be removed in Rails 6.0.

  See `bin/rails credentials --help` for more.
end_of_message

(Most trouble is just that our test suite relies on secrets, so will be seeing some deprecation warnings.)

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Aug 7, 2017

Member

I'd flip this now: config.present? ? YAML.dump(config) : ""

I'd flip this now: config.present? ? YAML.dump(config) : ""

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Aug 7, 2017

Member

Same: config.present? ? YAML.load(config) : {}

Same: config.present? ? YAML.load(config) : {}

kaspth added some commits Aug 7, 2017

Add require_master_key to abort the boot process.
In case the master key is required in a certain environment
we should inspect that the key is there and abort if it isn't.
Print missing key message and exit immediately.
Spares us a lengthy backtrace and prevents further execution.

I've verified the behavior in a test app, but couldn't figure the
test out as loading the app just exits immediately with:

```
/Users/kasperhansen/Documents/code/rails/activesupport/lib/active_support/testing/isolation.rb:23:in `load': marshal data too short (ArgumentError)
	from /Users/kasperhansen/Documents/code/rails/activesupport/lib/active_support/testing/isolation.rb:23:in `run'
	from /Users/kasperhansen/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/minitest-5.10.2/lib/minitest.rb:830:in `run_one_method'
	from /Users/kasperhansen/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/minitest-5.10.2/lib/minitest/parallel.rb:32:in `block (2 levels) in start'
```

It's likely we need to capture and prevent the exit somehow.
Kernel.stub(:exit) didn't work. Leaving it for tomorrow.
@tenderlove

This comment has been minimized.

Show comment
Hide comment
@tenderlove

tenderlove Aug 7, 2017

Member

It's likely we need to capture and prevent the exit somehow. Kernel.stub(:exit) didn't work. Leaving it for tomorrow.

@kaspth Can't we load the app inside a fork and check the return value from Process.waitpid?

Member

tenderlove commented Aug 7, 2017

It's likely we need to capture and prevent the exit somehow. Kernel.stub(:exit) didn't work. Leaving it for tomorrow.

@kaspth Can't we load the app inside a fork and check the return value from Process.waitpid?

@claudiob

This comment has been minimized.

Show comment
Hide comment
@claudiob

claudiob Aug 7, 2017

Member

If you create a brand new Rails <= 5.1 app from scratch, and then you manually delete the file config/secrets.yml, then you get this error once you run your server locally:

screen shot 2017-08-07 at 3 52 45 pm

This existing error message is correct in telling you to "set this value in config/secrets.yml".


If you create a brand new Rails app from this credentials branch then there is config/secrets.yml file anymore. But if you delete config/master.key, the error message is the same as before:

screen shot 2017-08-07 at 3 54 49 pm


Do you think we should just change the message in that error?
Or should we change the code for rails s to work in development even without a master key?

Member

claudiob commented Aug 7, 2017

If you create a brand new Rails <= 5.1 app from scratch, and then you manually delete the file config/secrets.yml, then you get this error once you run your server locally:

screen shot 2017-08-07 at 3 52 45 pm

This existing error message is correct in telling you to "set this value in config/secrets.yml".


If you create a brand new Rails app from this credentials branch then there is config/secrets.yml file anymore. But if you delete config/master.key, the error message is the same as before:

screen shot 2017-08-07 at 3 54 49 pm


Do you think we should just change the message in that error?
Or should we change the code for rails s to work in development even without a master key?

@kaspth kaspth referenced this pull request Aug 8, 2017

Closed

secrets.rb #30117

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh May 30, 2018

Member
Member

dhh commented May 30, 2018

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu May 30, 2018

Contributor

The web-console attack doesn't strike me as anything unique.

The difference here is that this is app-production credentials, not just some developers's data.

Contributor

printercu commented May 30, 2018

The web-console attack doesn't strike me as anything unique.

The difference here is that this is app-production credentials, not just some developers's data.

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh May 30, 2018

Member
Member

dhh commented May 30, 2018

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu May 30, 2018

Contributor

You can use credentials without having them decrypted in development mode
by not putting the master key in a local file.

Can you please describe the scenario? For example with Devise: one have to set secret_key, and write like this Devise.secret_key = Rails.application.credentials.devise_secret_key. Without master.key he'll get an error Devise.secret_key was not set.. This is from issue: #30717

Contributor

printercu commented May 30, 2018

You can use credentials without having them decrypted in development mode
by not putting the master key in a local file.

Can you please describe the scenario? For example with Devise: one have to set secret_key, and write like this Devise.secret_key = Rails.application.credentials.devise_secret_key. Without master.key he'll get an error Devise.secret_key was not set.. This is from issue: #30717

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu May 30, 2018

Contributor

If your machines are infected by malware

web-console issue is not related to malware. If local firewall is not setup properly, then anybody in the local network is able to read production credentials. I think it's possible that someone enable remote access to local dev server to show local changes, in office for example.

Contributor

printercu commented May 30, 2018

If your machines are infected by malware

web-console issue is not related to malware. If local firewall is not setup properly, then anybody in the local network is able to read production credentials. I think it's possible that someone enable remote access to local dev server to show local changes, in office for example.

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh May 30, 2018

Member
Member

dhh commented May 30, 2018

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu May 30, 2018

Contributor

Is it what you mean?

credentials = Rails.application.credentials
Devise.secret_key = credentials.available? ? credentials.devise_secret_key : 'stub'

I don't think this is a good solution. I think good code has as less as possible differences between environments. 'stub' value is ok here, but it does not work with api keys (ex., localhost-only oauth key).

About the web-console. I think most of developers unaware of this issue (I really think most even don't know who secret_key_base is used). I think everybody should be warned, to be able to chose appropriate way of preventing possible bad consequences: either stop using credentials, or use master.key even more carefully. If rails team don't want to post this warning, I'll do it myself.

UPD.

You can use credentials without having them decrypted in development mode
by not putting the master key in a local file.

But it's there by default in new app. And I haven't found anything like "Don't keep master.key in ./config all the time. Copy it there only when you need to edit credentials." in rails source. So it's insecure by default.

Contributor

printercu commented May 30, 2018

Is it what you mean?

credentials = Rails.application.credentials
Devise.secret_key = credentials.available? ? credentials.devise_secret_key : 'stub'

I don't think this is a good solution. I think good code has as less as possible differences between environments. 'stub' value is ok here, but it does not work with api keys (ex., localhost-only oauth key).

About the web-console. I think most of developers unaware of this issue (I really think most even don't know who secret_key_base is used). I think everybody should be warned, to be able to chose appropriate way of preventing possible bad consequences: either stop using credentials, or use master.key even more carefully. If rails team don't want to post this warning, I'll do it myself.

UPD.

You can use credentials without having them decrypted in development mode
by not putting the master key in a local file.

But it's there by default in new app. And I haven't found anything like "Don't keep master.key in ./config all the time. Copy it there only when you need to edit credentials." in rails source. So it's insecure by default.

@thomasnal

This comment has been minimized.

Show comment
Hide comment
@thomasnal

thomasnal May 30, 2018

The discussion above strikes me with the following idea.

  1. To keep differences between the environments minimal, none in the flowing case, credentials would be accessed by:
Rails.application.credentials
  1. To encrypt credential files, Rails would use the key read from RAILS_MASTER_KEY or master.key if env var not present.

  2. The difference would be on the storage level. The credentials would be organized in files à la environment configuration files. Based on the environment the app runs in, the credentials would be read from,

config/credentials/production.yml.enc
config/credentials/development.yml.enc
...

A smart error message will solve the situation if the master key can't decrypt the file Rails is attempting to read. Master key can't decrypt credentials in development environment.

This is a flexible way of organizing the files for likely all the usage cases I have seen referred to in this thread.

  • Those teams who store credentials in encrypted files will have credentials organized in separate files for different environments while each environment can be encrypted with a separate master key if desired. Otherwise, simply using the same key decrypts credentials in multiple environments, such as development and staging for an example.
  • Still others can use credentials in development only. While their production credentials are managed by operations team and read from environment variables.
  • Perhaps those teams who want to use exactly the same set of credentials over multiple environments would want to duplicate the file, e.g. copy development.yml.enc to staging.yml.enc if credentials change.

Summary, the same code to access credentials in all environments, option to manage multiple credential files, multiple keys per best practices of the team. And it looks to me easy to adapt from the current Rails 5.2 credentials.

Seems that all animals and creatures would be fed and stay alive. Any likes or dislikes for the above?

thomasnal commented May 30, 2018

The discussion above strikes me with the following idea.

  1. To keep differences between the environments minimal, none in the flowing case, credentials would be accessed by:
Rails.application.credentials
  1. To encrypt credential files, Rails would use the key read from RAILS_MASTER_KEY or master.key if env var not present.

  2. The difference would be on the storage level. The credentials would be organized in files à la environment configuration files. Based on the environment the app runs in, the credentials would be read from,

config/credentials/production.yml.enc
config/credentials/development.yml.enc
...

A smart error message will solve the situation if the master key can't decrypt the file Rails is attempting to read. Master key can't decrypt credentials in development environment.

This is a flexible way of organizing the files for likely all the usage cases I have seen referred to in this thread.

  • Those teams who store credentials in encrypted files will have credentials organized in separate files for different environments while each environment can be encrypted with a separate master key if desired. Otherwise, simply using the same key decrypts credentials in multiple environments, such as development and staging for an example.
  • Still others can use credentials in development only. While their production credentials are managed by operations team and read from environment variables.
  • Perhaps those teams who want to use exactly the same set of credentials over multiple environments would want to duplicate the file, e.g. copy development.yml.enc to staging.yml.enc if credentials change.

Summary, the same code to access credentials in all environments, option to manage multiple credential files, multiple keys per best practices of the team. And it looks to me easy to adapt from the current Rails 5.2 credentials.

Seems that all animals and creatures would be fed and stay alive. Any likes or dislikes for the above?

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu May 30, 2018

Contributor
  • I think it should be possible to not use encryption. For test and dev at least.
  • Having all envs in separate files makes it impossible to use yaml inheritance. We usually have test: << *development
Contributor

printercu commented May 30, 2018

  • I think it should be possible to not use encryption. For test and dev at least.
  • Having all envs in separate files makes it impossible to use yaml inheritance. We usually have test: << *development
@thomasnal

This comment has been minimized.

Show comment
Hide comment
@thomasnal

thomasnal May 30, 2018

Having all envs in separate files makes it impossible to use yaml inheritance. We usually have test: << *development

That would be managed by copying development.yml.enc to test.yml.enc whenever dev credentials change.

I see that mix of encrypted and not encrypted values is what disrupts our desire for pretty and straightforward solution. If desired to mix them, you are better to adopt the following approach with inheritance and a single configuration file.

# config/my_config.yml:
development: &development
  access_key_id: my_non_encrypted_value
test: 
  << *development
production:
  access_key_id: <%= Rails.application.credentials.dig(:access_key_id) %>
# config/initializers/my_config.rb:
Rails.application.config.x.my_config = OpenStruct.new(
  Rails.application.config_for(:my_config).deep_symbolize_keys
)

Then, regardless of the environment, access the key by,

Rails.configuration.my_config.access_key_id

I think it should be possible to not use encryption. For test and dev at least.

Anything that is referred to as a credential and left plain open can't justify to be called a credential regardless of the environment. In other way, anything that is referred to as credentials is not to be committed to version control plain as a discouragement from wrong practices. Rails needs to stand strong and clear in this regards!

With the proposed approach, should you wish to manage such practice, you can commit the master.key that decrypts dev and test credentials to solve your situation.

Note:

Should purist decide to allow "plain credentials", then,

config.require_master_key = false

and the file read would be config/credentials/development.yml. Wasn't that discouraged before, by @dhh perhaps?

thomasnal commented May 30, 2018

Having all envs in separate files makes it impossible to use yaml inheritance. We usually have test: << *development

That would be managed by copying development.yml.enc to test.yml.enc whenever dev credentials change.

I see that mix of encrypted and not encrypted values is what disrupts our desire for pretty and straightforward solution. If desired to mix them, you are better to adopt the following approach with inheritance and a single configuration file.

# config/my_config.yml:
development: &development
  access_key_id: my_non_encrypted_value
test: 
  << *development
production:
  access_key_id: <%= Rails.application.credentials.dig(:access_key_id) %>
# config/initializers/my_config.rb:
Rails.application.config.x.my_config = OpenStruct.new(
  Rails.application.config_for(:my_config).deep_symbolize_keys
)

Then, regardless of the environment, access the key by,

Rails.configuration.my_config.access_key_id

I think it should be possible to not use encryption. For test and dev at least.

Anything that is referred to as a credential and left plain open can't justify to be called a credential regardless of the environment. In other way, anything that is referred to as credentials is not to be committed to version control plain as a discouragement from wrong practices. Rails needs to stand strong and clear in this regards!

With the proposed approach, should you wish to manage such practice, you can commit the master.key that decrypts dev and test credentials to solve your situation.

Note:

Should purist decide to allow "plain credentials", then,

config.require_master_key = false

and the file read would be config/credentials/development.yml. Wasn't that discouraged before, by @dhh perhaps?

@thomasnal

This comment has been minimized.

Show comment
Hide comment
@thomasnal

thomasnal May 30, 2018

To unite encrypted and non encrypted approaches, either a single config file is needed or yml and yml.enc needs to be allowed according to config.require_master_key.

For a single config file though Rails could provide a facility that exposes credentials, if available and configured, through a config file:

# config/application.yml
default: &default
  <% Rails.application.credentials.config.each do |key, value| %>
  <%= "#{key}: #{value}" %>
  <# credentials read from config/credentials/#{Rails.env}.yml.enc if config.require_master_key %>
  <% end %>

development: &development
  << *default
  access_key_id: my_non_encrypted_value
test: 
  access_key_id: <%= SecureRandom.hex %>
staging:
  << *development
production:
  << *default

I like that this approach provides a single way to access the values Rails.configuration.x.application (this key and name has been picked for illustration only).

thomasnal commented May 30, 2018

To unite encrypted and non encrypted approaches, either a single config file is needed or yml and yml.enc needs to be allowed according to config.require_master_key.

For a single config file though Rails could provide a facility that exposes credentials, if available and configured, through a config file:

# config/application.yml
default: &default
  <% Rails.application.credentials.config.each do |key, value| %>
  <%= "#{key}: #{value}" %>
  <# credentials read from config/credentials/#{Rails.env}.yml.enc if config.require_master_key %>
  <% end %>

development: &development
  << *default
  access_key_id: my_non_encrypted_value
test: 
  access_key_id: <%= SecureRandom.hex %>
staging:
  << *development
production:
  << *default

I like that this approach provides a single way to access the values Rails.configuration.x.application (this key and name has been picked for illustration only).

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu May 31, 2018

Contributor

@thomasnal what's wrong with #30067 (comment) ?

Contributor

printercu commented May 31, 2018

@thomasnal what's wrong with #30067 (comment) ?

@tehtorq

This comment has been minimized.

Show comment
Hide comment
@tehtorq

tehtorq May 31, 2018

I've been using Rails for 7+ years now, and this is the first time I'm struggling to understand the benefits of a new feature in light of what's seemingly going to be taken away. I don't understand how this replaces the current secrets.yml implementation. With the current implementation, my production server never needs to see development credentials and vice versa.

  1. Perhaps I'm misunderstanding, but how can I access credentials required for a development or staging server without using the same production master key? I have numerous sets of credentials currently split by environment, and that's only growing with the increased trend towards usage of 3rd-party services.

  2. For the above use-case, is the convention to roll our own solution? Or continue to use secrets.yml (rails 5.2.0)?

tehtorq commented May 31, 2018

I've been using Rails for 7+ years now, and this is the first time I'm struggling to understand the benefits of a new feature in light of what's seemingly going to be taken away. I don't understand how this replaces the current secrets.yml implementation. With the current implementation, my production server never needs to see development credentials and vice versa.

  1. Perhaps I'm misunderstanding, but how can I access credentials required for a development or staging server without using the same production master key? I have numerous sets of credentials currently split by environment, and that's only growing with the increased trend towards usage of 3rd-party services.

  2. For the above use-case, is the convention to roll our own solution? Or continue to use secrets.yml (rails 5.2.0)?

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Jun 11, 2018

Contributor

I've created secure_credentials gem based on ideas from #30067 (comment)

Contributor

printercu commented Jun 11, 2018

I've created secure_credentials gem based on ideas from #30067 (comment)

@musaffa

This comment has been minimized.

Show comment
Hide comment
@musaffa

musaffa Jun 23, 2018

I don't like this change. Rails really needs a RFC process like Ember and Rust so that it can move forward as a community. The majority would have voted against this change had there been a RFC process.

Please don't deprecate secrets.yml so that the rest of us can continue to use it.

musaffa commented Jun 23, 2018

I don't like this change. Rails really needs a RFC process like Ember and Rust so that it can move forward as a community. The majority would have voted against this change had there been a RFC process.

Please don't deprecate secrets.yml so that the rest of us can continue to use it.

@alexwebgr

This comment has been minimized.

Show comment
Hide comment
@alexwebgr

alexwebgr Jun 30, 2018

i think the solution that @kaspth is suggesting with config_for solves the problem of per-env credentials and master key pairs thus eliminating the need for adding yet another gem. It never occurred to me and perhaps to other people that is possible to generate multiple credentials .yml.enc files and master.key pairs bin/rails encrypted:edit config/development.yml.enc --key config/development.key
Can't wait to see what version 6 brings !

alexwebgr commented Jun 30, 2018

i think the solution that @kaspth is suggesting with config_for solves the problem of per-env credentials and master key pairs thus eliminating the need for adding yet another gem. It never occurred to me and perhaps to other people that is possible to generate multiple credentials .yml.enc files and master.key pairs bin/rails encrypted:edit config/development.yml.enc --key config/development.key
Can't wait to see what version 6 brings !

@noctivityinc

This comment has been minimized.

Show comment
Hide comment
@noctivityinc

noctivityinc Jul 5, 2018

Maybe Im missing something obvious here but is there a reason why you wouldn't just use the standard multi-env notation in the credentials file and then just create a SUPER SIMPLE method in an initializer called cred.rb or whatever that does this:

def cred(key)
   return Rails.application.credentials[Rails.env.to_sym][key]
end

and then just call cred(:whatever_key) when you want the value? I just tried it and it works great.

noctivityinc commented Jul 5, 2018

Maybe Im missing something obvious here but is there a reason why you wouldn't just use the standard multi-env notation in the credentials file and then just create a SUPER SIMPLE method in an initializer called cred.rb or whatever that does this:

def cred(key)
   return Rails.application.credentials[Rails.env.to_sym][key]
end

and then just call cred(:whatever_key) when you want the value? I just tried it and it works great.

@noctivityinc

This comment has been minimized.

Show comment
Hide comment
@noctivityinc

noctivityinc Jul 5, 2018

Small modification. I added the code in application.rb so I can load it in other initializers:

require_relative 'boot'

class Creds
  def self.get(key)
    return Rails.application.credentials[Rails.env.to_sym][key]
  end
end

require 'rails/all'

# Require the gems listed in Gemfile, including any gems
# you've limited to :test, :development, or :production.
Bundler.require(*Rails.groups)

module Five
  class Application < Rails::Application
    # Initialize configuration defaults for originally generated Rails version.
    config.load_defaults 5.2

    # Settings in config/environments/* take precedence over those specified here.
    # Application configuration can go into files in config/initializers
    # -- all .rb files in that directory are automatically loaded after loading
    # the framework and any gems in your application.
  end
end

noctivityinc commented Jul 5, 2018

Small modification. I added the code in application.rb so I can load it in other initializers:

require_relative 'boot'

class Creds
  def self.get(key)
    return Rails.application.credentials[Rails.env.to_sym][key]
  end
end

require 'rails/all'

# Require the gems listed in Gemfile, including any gems
# you've limited to :test, :development, or :production.
Bundler.require(*Rails.groups)

module Five
  class Application < Rails::Application
    # Initialize configuration defaults for originally generated Rails version.
    config.load_defaults 5.2

    # Settings in config/environments/* take precedence over those specified here.
    # Application configuration can go into files in config/initializers
    # -- all .rb files in that directory are automatically loaded after loading
    # the framework and any gems in your application.
  end
end
@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Jul 5, 2018

Contributor

@noctivityinc problem in this scenario is that every developer gets access to production credentials. Also this making credentials less secure even if you trust all the developers. I've posted a note about this with some details: https://github.com/printercu/secure_credentials/wiki/Rails-5.2-credentials-are-not-secure

BTW given cred method will not work in every env without config.read_encrypted_secrets = true (#30717). Seems like it'll be fixed in next releases.

Contributor

printercu commented Jul 5, 2018

@noctivityinc problem in this scenario is that every developer gets access to production credentials. Also this making credentials less secure even if you trust all the developers. I've posted a note about this with some details: https://github.com/printercu/secure_credentials/wiki/Rails-5.2-credentials-are-not-secure

BTW given cred method will not work in every env without config.read_encrypted_secrets = true (#30717). Seems like it'll be fixed in next releases.

@noctivityinc

This comment has been minimized.

Show comment
Hide comment
@noctivityinc

noctivityinc Jul 5, 2018

@printercu I do agree your solution with the secure_credentials gem is better. I just dont like having to run to remember to run rails encrypted path/to/file.yml.enc -k path/to/key.key every time or using a third party dependency.

noctivityinc commented Jul 5, 2018

@printercu I do agree your solution with the secure_credentials gem is better. I just dont like having to run to remember to run rails encrypted path/to/file.yml.enc -k path/to/key.key every time or using a third party dependency.

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Jul 5, 2018

Contributor

It's still possible to keep config/master.key in local repo, and use just rails encrypted:edit path/to/file vs rails credentials:edit.

Contributor

printercu commented Jul 5, 2018

It's still possible to keep config/master.key in local repo, and use just rails encrypted:edit path/to/file vs rails credentials:edit.

@rensanity

This comment has been minimized.

Show comment
Hide comment
@rensanity

rensanity Jul 12, 2018

The text editor i chose do not appear when i type EDITOR='subl --wait' rails credentials:edit on terminal :(

rensanity commented Jul 12, 2018

The text editor i chose do not appear when i type EDITOR='subl --wait' rails credentials:edit on terminal :(

@Fedcomp

This comment has been minimized.

Show comment
Hide comment
@Fedcomp

Fedcomp Jul 14, 2018

can someone explain how to use rails without secrets.yml.enc when SECRET_KEY_BASE will be removed from rails? Keeping rest of framework working of course.

Fedcomp commented Jul 14, 2018

can someone explain how to use rails without secrets.yml.enc when SECRET_KEY_BASE will be removed from rails? Keeping rest of framework working of course.

@kaspth

This comment has been minimized.

Show comment
Hide comment
@kaspth

kaspth Jul 15, 2018

Member

@Fedcomp not sure what you mean? SECRET_KEY_BASE isn't going anywhere. We'll derive it from the app name in development and test environments. In production, credentials can have it or encrypted secrets can have it or the ENV variable can have it.

Member

kaspth commented Jul 15, 2018

@Fedcomp not sure what you mean? SECRET_KEY_BASE isn't going anywhere. We'll derive it from the app name in development and test environments. In production, credentials can have it or encrypted secrets can have it or the ENV variable can have it.

@memiux

This comment has been minimized.

Show comment
Hide comment
@memiux

memiux Jul 30, 2018

RIP the good ol' days.

shared:
  third_party_api_key: <%= ENV["THIRD_PARTY_API_KEY"] || "sandbox-123" %>

I remember when 12factor was a thing, secrets were fine now I'm confused. 😐

memiux commented Jul 30, 2018

RIP the good ol' days.

shared:
  third_party_api_key: <%= ENV["THIRD_PARTY_API_KEY"] || "sandbox-123" %>

I remember when 12factor was a thing, secrets were fine now I'm confused. 😐

@BerkhanBerkdemir

This comment has been minimized.

Show comment
Hide comment
@BerkhanBerkdemir

BerkhanBerkdemir Aug 2, 2018

Today, I faced "the problem", env based encryption, when I trying to integrate Algolia. I think for now, the best practice is using secure_credentials gem. Is there any best practice?

BerkhanBerkdemir commented Aug 2, 2018

Today, I faced "the problem", env based encryption, when I trying to integrate Algolia. I think for now, the best practice is using secure_credentials gem. Is there any best practice?

@kaspth

This comment has been minimized.

Show comment
Hide comment
@kaspth

kaspth Aug 2, 2018

Member

I'd like to make some things clear: Credentials is a conceptual rethink from Secrets and Encrypted Secrets. It charters a new path and thus solves fewer use cases than the old way did. The Core team is not in anyway opposed to supporting multiple environment credentials as long as it doesn't complicate the existing use case as @dhh made clear #30067 (comment).

However, the suggestions we've seen for multi environment credentials so far haven't been up to par. For instance, the secure_credentials gem flies right in the face of the motivation stated in the PR description:

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.

Do volunteer some PRs yourselves! I'm pretty confident to say that multi env credentials are a lot closer than you'd think. The biggest hurdle that I see is people continuing to conceptualize and compare with the secrets way. That's the hard way. 😄

@BerkhanBerkdemir my suggestion is here #30067 (comment). True multi env credentials could perhaps smooth this over.

Thanks for all the feedback and ideas! Those who feel the burn know best how to put out the fire, so let's level Rails up together ❤️

Member

kaspth commented Aug 2, 2018

I'd like to make some things clear: Credentials is a conceptual rethink from Secrets and Encrypted Secrets. It charters a new path and thus solves fewer use cases than the old way did. The Core team is not in anyway opposed to supporting multiple environment credentials as long as it doesn't complicate the existing use case as @dhh made clear #30067 (comment).

However, the suggestions we've seen for multi environment credentials so far haven't been up to par. For instance, the secure_credentials gem flies right in the face of the motivation stated in the PR description:

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.

Do volunteer some PRs yourselves! I'm pretty confident to say that multi env credentials are a lot closer than you'd think. The biggest hurdle that I see is people continuing to conceptualize and compare with the secrets way. That's the hard way. 😄

@BerkhanBerkdemir my suggestion is here #30067 (comment). True multi env credentials could perhaps smooth this over.

Thanks for all the feedback and ideas! Those who feel the burn know best how to put out the fire, so let's level Rails up together ❤️

@sidot3291

This comment has been minimized.

Show comment
Hide comment
@sidot3291

sidot3291 Aug 2, 2018

Thanks for the rallying cry @kaspth

What do you think of initializing a key and credentials file for each environment within config/environments?

e.g:

> ls config/environments:
    development.rb
    development.key
    development.credentials.yml.enc
    production.rb
    production.key
    production.credentials.yml.enc
    test.rb
    test.key
    test.credentials.yml.enc

Then:

  • All keys are .gitignore'd
  • Update CLI so it's rails credentials:edit production to edit production credentials

This adds to the config directory a bit but still allows for atomic deploys of environment variables.

My hesitation with this is that my setup would require "staging" credentials, which intuitively would require a fourth "staging" environment. CLI could be built so generating this is easy, e.g:
rails environments:add staging

It's worth noting, though, Heroku explicitly pushes me away from this approach:

It may be tempting to create another custom environment such as “staging” and create a config/environments/staging.rb and deploy to a Heroku app with RAILS_ENV=staging. This is not a good practice. Instead we recommend always running in production mode and modifying any behavior by setting your config vars.
https://devcenter.heroku.com/articles/deploying-to-a-custom-rails-environment

That raises two options:

  1. Don't care. The credentials concept is intentionally pushing atomic, checked-in environment variables instead of system environment variables.
  2. Make environment settings for the credentials key and file, e.g. in production.rb:
config.credentials.key = ENV['CREDENTIALS_KEY'] || Rails.root.join('config/credentials/production.key').read
config.credentials.file = Rails.root.join(ENV['CREDENTIALS_FILE'] || 'config/credentials/production.credentials.yml.enc')

sidot3291 commented Aug 2, 2018

Thanks for the rallying cry @kaspth

What do you think of initializing a key and credentials file for each environment within config/environments?

e.g:

> ls config/environments:
    development.rb
    development.key
    development.credentials.yml.enc
    production.rb
    production.key
    production.credentials.yml.enc
    test.rb
    test.key
    test.credentials.yml.enc

Then:

  • All keys are .gitignore'd
  • Update CLI so it's rails credentials:edit production to edit production credentials

This adds to the config directory a bit but still allows for atomic deploys of environment variables.

My hesitation with this is that my setup would require "staging" credentials, which intuitively would require a fourth "staging" environment. CLI could be built so generating this is easy, e.g:
rails environments:add staging

It's worth noting, though, Heroku explicitly pushes me away from this approach:

It may be tempting to create another custom environment such as “staging” and create a config/environments/staging.rb and deploy to a Heroku app with RAILS_ENV=staging. This is not a good practice. Instead we recommend always running in production mode and modifying any behavior by setting your config vars.
https://devcenter.heroku.com/articles/deploying-to-a-custom-rails-environment

That raises two options:

  1. Don't care. The credentials concept is intentionally pushing atomic, checked-in environment variables instead of system environment variables.
  2. Make environment settings for the credentials key and file, e.g. in production.rb:
config.credentials.key = ENV['CREDENTIALS_KEY'] || Rails.root.join('config/credentials/production.key').read
config.credentials.file = Rails.root.join(ENV['CREDENTIALS_FILE'] || 'config/credentials/production.credentials.yml.enc')
@thomasnal

This comment has been minimized.

Show comment
Hide comment
@thomasnal

thomasnal Aug 2, 2018

I would give it its own folder such as config/environments or config/credentials is a great way to go. My five cents, as explained earlier, seems to support all variety of requests we have seen.

thomasnal commented Aug 2, 2018

I would give it its own folder such as config/environments or config/credentials is a great way to go. My five cents, as explained earlier, seems to support all variety of requests we have seen.

@sidot3291

This comment has been minimized.

Show comment
Hide comment
@sidot3291

sidot3291 Aug 2, 2018

As is normally the case after clicking send, I find myself leaning toward the second option, since the entirety of my fourth "staging" environment would be copy of production, except for credentials.

So to round things out:

  1. Add a config/credentials folder with:
ls config/credentials
    development.key
    development.yml.enc
    production.key
    production.yml.enc
    test.key
    test.yml.enc
  1. Update the CLI
    rails credentials:edit {token} edits the corresponding file in the credentials folder
    rails credentials:add {token} generates a new key/file pair

  2. Add two environment settings to the existing environments/{env}.rb files:

config.credentials.key = ENV['CREDENTIALS_KEY'] || Rails.root.join('config/credentials/production.key').read
config.credentials.file = Rails.root.join(ENV['CREDENTIALS_FILE'] || 'config/credentials/production.credentials.yml.enc')

Would this work?

sidot3291 commented Aug 2, 2018

As is normally the case after clicking send, I find myself leaning toward the second option, since the entirety of my fourth "staging" environment would be copy of production, except for credentials.

So to round things out:

  1. Add a config/credentials folder with:
ls config/credentials
    development.key
    development.yml.enc
    production.key
    production.yml.enc
    test.key
    test.yml.enc
  1. Update the CLI
    rails credentials:edit {token} edits the corresponding file in the credentials folder
    rails credentials:add {token} generates a new key/file pair

  2. Add two environment settings to the existing environments/{env}.rb files:

config.credentials.key = ENV['CREDENTIALS_KEY'] || Rails.root.join('config/credentials/production.key').read
config.credentials.file = Rails.root.join(ENV['CREDENTIALS_FILE'] || 'config/credentials/production.credentials.yml.enc')

Would this work?

@Fedcomp

This comment has been minimized.

Show comment
Hide comment
@Fedcomp

Fedcomp Aug 2, 2018

Seems to be over complicated, instead it''s easier to use plain old ENV variables.

Fedcomp commented Aug 2, 2018

Seems to be over complicated, instead it''s easier to use plain old ENV variables.

@morgoth

This comment has been minimized.

Show comment
Hide comment
@morgoth

morgoth Aug 2, 2018

Member

hey all - please take a look at #33521 if it would be enough for your usecases

Member

morgoth commented Aug 2, 2018

hey all - please take a look at #33521 if it would be enough for your usecases

@christophermlne

This comment has been minimized.

Show comment
Hide comment
@christophermlne

christophermlne Aug 2, 2018

I didn't find the old system confusing but this new system is very confusing and it confused everyone in our office. The devs are now passing the master key around through Slack.

christophermlne commented Aug 2, 2018

I didn't find the old system confusing but this new system is very confusing and it confused everyone in our office. The devs are now passing the master key around through Slack.

@printercu

This comment has been minimized.

Show comment
Hide comment
@printercu

printercu Aug 3, 2018

Contributor

@kaspth

For instance, the secure_credentials gem flies right in the face of the motivation stated in the PR description

There is no secrets.yml and secrets.yml.enc in secure_credentials gem. There are multienv secrets.yml and secrets.:env.yml.enc for any other envs. I've explained why having encrypted credentials for dev/test envs is a bad practice on this wiki page: because it gives illusion of security and encourages sharing secret key across all developers.

After trying many different ways of managing credentials, I've came to not checking secrets.yml to vcs but using secrets.sample.yml. This allows to share dev credentials selectively (yes, they are secret in some cases too) and prevent occasional check-ins of this files with sensitive data. And I like the idea of storing encrypted production credentials in vcs. So I've combined both approaches in secure_credentials.

Contributor

printercu commented Aug 3, 2018

@kaspth

For instance, the secure_credentials gem flies right in the face of the motivation stated in the PR description

There is no secrets.yml and secrets.yml.enc in secure_credentials gem. There are multienv secrets.yml and secrets.:env.yml.enc for any other envs. I've explained why having encrypted credentials for dev/test envs is a bad practice on this wiki page: because it gives illusion of security and encourages sharing secret key across all developers.

After trying many different ways of managing credentials, I've came to not checking secrets.yml to vcs but using secrets.sample.yml. This allows to share dev credentials selectively (yes, they are secret in some cases too) and prevent occasional check-ins of this files with sensitive data. And I like the idea of storing encrypted production credentials in vcs. So I've combined both approaches in secure_credentials.

@soteras soteras referenced this pull request Aug 3, 2018

Closed

Kind of idea. Credentials #2993

@prashantham

This comment has been minimized.

Show comment
Hide comment
@prashantham

prashantham Aug 4, 2018

I had to implement a CI/CD environment for a rails 4.2 app on google cloud. The server image needed to have all the source including secrets but in encrypted .env files. The encrypted files would then be decrypted when instance was created using kms. Now this required different files for our stage/prod envs i.e .env.stage.enc and .env.prod.env. Google project would hold metadata related to cipher file name which are available to instances. systemd task would then decrypt them as .env.

So i think we need to have 2 env variables one for master key and another for cipher file.

prashantham commented Aug 4, 2018

I had to implement a CI/CD environment for a rails 4.2 app on google cloud. The server image needed to have all the source including secrets but in encrypted .env files. The encrypted files would then be decrypted when instance was created using kms. Now this required different files for our stage/prod envs i.e .env.stage.enc and .env.prod.env. Google project would hold metadata related to cipher file name which are available to instances. systemd task would then decrypt them as .env.

So i think we need to have 2 env variables one for master key and another for cipher file.

@kaspth

This comment has been minimized.

Show comment
Hide comment
@kaspth

kaspth Aug 12, 2018

Member

Thanks for the further feedback everyone! @morgoth even submitted a PR for us to hammer things out on! I've submitted my thoughts about a potential file structure here: #33521 (comment)

@christophermlne can you elaborate on what you and your team found confusing about this setup? Why wasn't it confusing with the old secrets.yml + secrets.yml.enc?

@printercu sorry I wasn't clear enough. It's not the files being present, it's having two similarly named files and only one being the encrypted one.


I'm less in favor of the ENV variables suggestions to do lookup. Credentials is an attempt to cut down on the number of ENV variables an app should manage. Though we could expose config.credentials.key_path or config.credentials.key and config.credentials.content_path, which you can override via ENV variables yourselves.

Member

kaspth commented Aug 12, 2018

Thanks for the further feedback everyone! @morgoth even submitted a PR for us to hammer things out on! I've submitted my thoughts about a potential file structure here: #33521 (comment)

@christophermlne can you elaborate on what you and your team found confusing about this setup? Why wasn't it confusing with the old secrets.yml + secrets.yml.enc?

@printercu sorry I wasn't clear enough. It's not the files being present, it's having two similarly named files and only one being the encrypted one.


I'm less in favor of the ENV variables suggestions to do lookup. Credentials is an attempt to cut down on the number of ENV variables an app should manage. Though we could expose config.credentials.key_path or config.credentials.key and config.credentials.content_path, which you can override via ENV variables yourselves.

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