protected variables leak in logs during migrate

[inopinatus]

Expected Behavior

OpsWorks protected variables should not appear in logs, because those are readable back in the console.

Actual Behavior

A deploy with a migrate spews its entire set of configured environment variables into the logs. This includes protected variables, which can then be read back in plaintext in the OpsWorks console. This behaviour is hard-coded in Chef, so it's Chef's fault, but it's our concern, because it undermines an effective promise from the OpsWorks platform.

Offending line is lib/chef/provider/deploy.rb:202 in the OpsWorks-bundled release ofChef.

Chef log including error

[2018-07-05T18:25:41+00:00] INFO: deploy[testapp] migrating deploy with environment SECRET_KEY_BASE='oh_dear_that_got_disclosed' MALLOC_ARENA_MAX='2' RAILS_ENV='test'

[inopinatus]

Reported to AWS. Their response (received this morning) was to update the documentation to confirm that it leaks.

[inopinatus]

My temporary workaround is to place the following in libraries/fix-chef-environment-leak.rb of my cookbooks:

# Monkey-patch the Deploy provider to not spew environment variables into the logs

class Chef::Provider::Deploy
  def migrate
    run_symlinks_before_migrate

    if @new_resource.migrate
      enforce_ownership

      environment = @new_resource.environment
      env_info = environment && environment.map do |key_and_val|
        "#{key_and_val.first}='#{key_and_val.last}'"
      end.join(" ")

      converge_by("execute migration command #{@new_resource.migration_command}") do
        Chef::Log.info "#{@new_resource} migrating #{@new_resource.user} without environment info"
        shell_out!(@new_resource.migration_command, run_options(:cwd => release_path, :log_level => :info))
      end
    end
  end
end

[inopinatus]

Not sure of a longer-term fix yet, but probably something like "Don't trust OpsWorks to protect your so-called protected values - keep secrets elsewhere".

The suggestion made in the doco of keeping secrets in S3 seems weak to me. Rails 5.2 has a new secrets mechanism that could strengthen it, but not everyone uses Rails. The AWS platform also has three services that could help:

• AWS Secrets Manager
• AWS Systems Manager Parameter Store
• AWS Key Management Service

Later this month I'll have time to evaluate which one is best fitted.

[ajgon]

Thanks for this investigation! Unfortunately I don't see a simple solution for that as well.

I'm thinking about adding another config option, like app['global']['secret_env_variables'] or whatever, with lists all of environment variables that should be considered as "private". For now, this would mean "skip them during deploy". Usually secret keys, api keys and stuff like that are required in running application, not during typical deployment tasks like migration or symlinking - and since all running-related variables are configured in appserver (which is done in configure phase) - this should do the trick.

What do you think about this approach?

[inopinatus]

I worry it has edge cases. Yes it can reduce the disclosure. However, application deploys can still have a use for secrets. Maybe not that Stripe key super-secret, but my deploy hooks use API keys to (for example) notify services like Rollbar that a deploy just happened. They could fetch that themselves from a YAML environment file, but that's yet more code for a problem that no-one should have.

[ajgon]

Yeah, I really hate monkey-patching, but your solution resolves precisely this problem without any side-effects (since Chef 12 is not developed anymore, I don't think this method would change anytime soon) and it fits nicely to the cookbook.

On the other hand, question is - should opsworks_ruby even care, about thing which didn't break?

[inopinatus]

In principal, no - I actually think this is an AWS problem, not the problem of opsworks_ruby, but they've declined to fix it.

In practice, however, I think almost every user of opsworks_ruby with a production-grade application would want to know that this happens and have some remedy.

[inopinatus]

I do think it would be good to have a blacklist (or whitelist) for environment variables used in recipes. I have also noticed them appearing in the output of ps which is a second-level concern for systems managers.

[ajgon]

For which process? Appserver? Or during migration?

[inopinatus]

the example I have is during delayed_job stop/start:

testadminrole-joshua@test-app1:~$ ps wwwaux | grep disclosed
root     29682  0.0  0.1  51004  3548 ?        Ss   19:59   0:00 /bin/su - deploy -c cd /srv/www/testapp/current && SECRET_KEY_BASE="oh_dear_that_got_disclosed" MALLOC_ARENA_MAX="2" RAILS_ENV="test" HOME="/home/deploy" USER="deploy" bin/delayed_job stop --pid-dir=/run/lock/testapp/ -i 0

I haven't checked for other cases.

Now that I look at it again, a recipe-level filter wouldn't help in this case. I think we might consider that a separate issue. Related but separate. The best fix is again likely to be "don't put your secrets in OpsWorks protected variables".

[olbrich]

I actually prefer to only use the opsworks environment variables to override configs. For the rest I use a combination of .env files and parameter store. Part of the reason I do this is because I want as much of the application config as possible to be under version control, and parameter store also provides secure strings and an audit log.