diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-08-10 19:54:54 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-08-10 19:54:54 +0300 |
commit | b1aac0382c406b3856db90e15df8b2a9ea7ff6cd (patch) | |
tree | 2db352e89f59c6ad5fb0dd7c89c40a001c590509 /config | |
parent | 4ccba6bf2ddd48d66cd9cd8c6cee5eae19691cbb (diff) | |
parent | 2785a56e7d1968dfda03850a14af296d71b06503 (diff) |
Merge branch 'decouple-secret-keys' into 'master'
Store OTP secret key in secrets.yml
## What does this MR do?
Migrate the value of `.secret` to `config/secrets.yml` if present, so that `.secret` can be rotated without preventing all users with 2FA from logging in. (On a clean setup, generate different keys for each.)
## Are there points in the code the reviewer needs to double check?
I'm not sure we actually need `.secret` at all after this, but it seems safer not to touch it.
## Why was this MR needed?
We have some DB encryption keys in `config/secrets.yml`, and one in `.secret`. They should all be in the same place.
## What are the relevant issue numbers?
#3963, which isn't closed until I make the relevant changes in Omnibus too.
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- ~~API support added~~
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5274
Diffstat (limited to 'config')
-rw-r--r-- | config/initializers/secret_token.rb | 103 |
1 files changed, 70 insertions, 33 deletions
diff --git a/config/initializers/secret_token.rb b/config/initializers/secret_token.rb index dae3a4a9a93..291fa6c0abc 100644 --- a/config/initializers/secret_token.rb +++ b/config/initializers/secret_token.rb @@ -2,49 +2,86 @@ require 'securerandom' -# Your secret key for verifying the integrity of signed cookies. -# If you change this key, all old signed cookies will become invalid! -# Make sure the secret is at least 30 characters and all random, -# no regular words or you'll be exposed to dictionary attacks. - -def find_secure_token - token_file = Rails.root.join('.secret') - if ENV.key?('SECRET_KEY_BASE') - ENV['SECRET_KEY_BASE'] - elsif File.exist? token_file - # Use the existing token. - File.read(token_file).chomp - else - # Generate a new token of 64 random hexadecimal characters and store it in token_file. - token = SecureRandom.hex(64) - File.write(token_file, token) - token +# Transition material in .secret to the secret_key_base key in config/secrets.yml. +# Historically, ENV['SECRET_KEY_BASE'] takes precedence over .secret, so we maintain that +# behavior. +# +# It also used to be the case that the key material in ENV['SECRET_KEY_BASE'] or .secret +# was used to encrypt OTP (two-factor authentication) data so if present, we copy that key +# material into config/secrets.yml under otp_key_base. +# +# Finally, if we have successfully migrated all secrets to config/secrets.yml, delete the +# .secret file to avoid confusion. +# +def create_tokens + secret_file = Rails.root.join('.secret') + file_secret_key = File.read(secret_file).chomp if File.exist?(secret_file) + env_secret_key = ENV['SECRET_KEY_BASE'] + + # Ensure environment variable always overrides secrets.yml. + Rails.application.secrets.secret_key_base = env_secret_key if env_secret_key.present? + + defaults = { + secret_key_base: file_secret_key || generate_new_secure_token, + otp_key_base: env_secret_key || file_secret_key || generate_new_secure_token, + db_key_base: generate_new_secure_token + } + + missing_secrets = set_missing_keys(defaults) + write_secrets_yml(missing_secrets) unless missing_secrets.empty? + + begin + File.delete(secret_file) if file_secret_key + rescue => e + warn "Error deleting useless .secret file: #{e}" end end -Rails.application.config.secret_token = find_secure_token -Rails.application.config.secret_key_base = find_secure_token - -# CI def generate_new_secure_token SecureRandom.hex(64) end -if Rails.application.secrets.db_key_base.blank? - warn "Missing `db_key_base` for '#{Rails.env}' environment. The secrets will be generated and stored in `config/secrets.yml`" +def warn_missing_secret(secret) + warn "Missing Rails.application.secrets.#{secret} for #{Rails.env} environment. The secret will be generated and stored in config/secrets.yml." +end - all_secrets = YAML.load_file('config/secrets.yml') if File.exist?('config/secrets.yml') - all_secrets ||= {} +def set_missing_keys(defaults) + defaults.stringify_keys.each_with_object({}) do |(key, default), missing| + if Rails.application.secrets[key].blank? + warn_missing_secret(key) - # generate secrets - env_secrets = all_secrets[Rails.env.to_s] || {} - env_secrets['db_key_base'] ||= generate_new_secure_token - all_secrets[Rails.env.to_s] = env_secrets + missing[key] = Rails.application.secrets[key] = default + end + end +end + +def write_secrets_yml(missing_secrets) + secrets_yml = Rails.root.join('config/secrets.yml') + rails_env = Rails.env.to_s + secrets = YAML.load_file(secrets_yml) if File.exist?(secrets_yml) + secrets ||= {} + secrets[rails_env] ||= {} + + secrets[rails_env].merge!(missing_secrets) do |key, old, new| + # Previously, it was possible this was set to the literal contents of an Erb + # expression that evaluated to an empty value. We don't want to support that + # specifically, just ensure we don't break things further. + # + if old.present? + warn <<EOM +Rails.application.secrets.#{key} was blank, but the literal value in config/secrets.yml was: + #{old} - # save secrets - File.open('config/secrets.yml', 'w', 0600) do |file| - file.write(YAML.dump(all_secrets)) +This probably isn't the expected value for this secret. To keep using a literal Erb string in config/secrets.yml, replace `<%` with `<%%`. +EOM + + exit 1 # rubocop:disable Rails/Exit + end + + new end - Rails.application.secrets.db_key_base = env_secrets['db_key_base'] + File.write(secrets_yml, YAML.dump(secrets), mode: 'w', perm: 0600) end + +create_tokens |