By default the environment section leaks sensitive data, including Rails secret_token #182

Open
mattbrictson opened this Issue Aug 19, 2013 · 10 comments

Projects

None yet

4 participants

@mattbrictson

In Rails, the default parameter filter only excludes keys that include "password":

# application.rb generated by `rails new`
config.filter_parameters += [:password]

The default configuration for the exception_notification gem is to include the environment section, which dumps @request.filtered_env. This includes sensitive data like:

  • action_dispatch.secret_token
  • rack.request.cookie_hash
  • rack.session (includes CSRF token)
  • rack.session.options

This means the app's secret token could be sent plaintext in an email!

The solution is fairly simple: change the configuration of the Rails app so that those sensitive keys are filtered out.

# application.rb
config.filter_parameters += %w(password secret session cookie csrf)

Even though the workaround is easy, I feel like exception_notification should do more to prevent this data from leaking in the default configuration.

Perhaps the environment template should explicitly ignore the action_dispatch.secret_token key when iterating over @request.filtered_env?

@etipton
etipton commented Dec 3, 2014

bump on this - and I agree that exception_notification should have some reasonable default settings to avoid exposing this data.

I've just configured my app in the following way:

  1. I removed the "session" section of the email via the "sections" option:
    sections: %w{request environment backtrace}
  2. I added the following fields to filter_parameters:
# Env var names matching these need to be filtered from exception emails
config.filter_parameters += [:session, :warden, :secret, :salt]

!!!! see UPDATE note below !!!!! This differs from @mattbrictson's suggestion in that it leaves the HTTP_COOKIE env var unfiltered so that I can decrypt the session value if needed.

UPDATE: @xaviershay pointed out below that showing the cookie values, even if some are encrypted, leaves you vulnerable to replay attacks.

@etipton
etipton commented Dec 3, 2014

!!!! see UPDATE note below !!!!!

Here's how I decrypt the session: https://gist.github.com/etipton/a7b7ca15f4abfccc5f5d

(call CookieDecryptor.decrypt("cookie value here") from within the prod Rails console)

UPDATE: @xaviershay pointed out below that sending cookies in an insecure format such as email, even if some values are encrypted, leaves you vulnerable to replay attacks.

@etipton
etipton commented Dec 3, 2014

Oops, meant to include the ENV vars that I believe to contain sensitive data, in case someone decides to patch this gem:

  • action_dispatch.encrypted_cookie_salt : [FILTERED]
  • action_dispatch.encrypted_signed_cookie_salt : [FILTERED]
  • action_dispatch.http_auth_salt : [FILTERED]
  • action_dispatch.request.unsigned_session_cookie: [FILTERED]
  • action_dispatch.secret_key_base : [FILTERED]
  • action_dispatch.secret_token : [FILTERED]
  • action_dispatch.signed_cookie_salt : [FILTERED]
  • rack.session : [FILTERED]
  • rack.session.options : [FILTERED]
  • warden : [FILTERED]
@xaviershay

Just noticed this as well, seems like a dangerous default.

@xaviershay

Relevant fix for bugsnag notifier: bugsnag/bugsnag-ruby@7d40acb and also made these case-insensitive: bugsnag/bugsnag-ruby@b134164

@xaviershay

@etipton aren't you vulnerable to a replay attack if you leave the cookie exposed (even though it's encrypted)?

@etipton
etipton commented Apr 18, 2015

@xaviershay GREAT catch... yes, that is definitely a vulnerability with my current setup.

@xaviershay

My config:

Rails.application.config.filter_parameters +=
  [:password, :session, :warden, :secret, :salt, :cookie, :csrf]
@etipton
etipton commented Apr 18, 2015

In my case, I still like to get the cookie info so that I can know which user received the error... here is my quick-and-dirty solution for now... in case others might want to do something similar:

  1. Added app/views/exception_notifier/_environment.text.erb which will override the gem's view file.
  2. Copied and pasted from the gem, except I detect whether the key contains "cookie" and if so, I encrypt using a custom encryption class I already had (which is pretty straightforward to build... could also probably just use ActiveSupport::MessageEncryptor)
<% filtered_env = @request.filtered_env -%>
<% max = filtered_env.keys.map(&:to_s).max { |a, b| a.length <=> b.length } -%>
<% filtered_env.keys.map(&:to_s).sort.each do |key| -%>
<%#- <CUSTOM> -%>
<%- value = inspect_object(filtered_env[key]) -%>
<%- if value != '[FILTERED]' && key =~ /cookie/i -%>
<%- value = MyCustomMessageEncryptor.encrypt(value) -%>
<%- end -%>
<%#- </CUSTOM> -%>
* <%= raw("%-*s: %s" % [max.length, key, value]) %>
<% end -%>

For anyone thinking of doing this, know that this will only work if you are using the EmailNotifier, and please be sure that you have added the other stuff to your filter_parameter_logging setting...

It's not pretty but...

image

@xaviershay xaviershay added a commit to xaviershay/rails that referenced this issue Apr 18, 2015
@xaviershay xaviershay Add common secret names to filter parameters defaults.
This configuration is often used by extensions for filtering of Rails
session and environment data, for example:

* smartinez87/exception_notification#182
* bugsnag/bugsnag-ruby@7d40acb

As a result, they tend to be insecure until they are explicitly patched.
In the event of exception notification, this has not happened even after
the insecure defaults being present for many years.

The risk of these names being used for non-secret values is very low,
compared to the high and demonstrated risk of secrets leaking via a
common extension pattern.
30d3a5d
@xaviershay xaviershay added a commit to xaviershay/rails that referenced this issue Apr 18, 2015
@xaviershay xaviershay Add common secret names to filter parameters defaults.
This configuration is often used by extensions for filtering of Rails
session and environment data, for example:

* smartinez87/exception_notification#182
* bugsnag/bugsnag-ruby@7d40acb

As a result, they tend to be insecure until they are explicitly patched.
In the event of exception notification, this has not happened even after
the insecure defaults being present for many years.

The risk of these names being used for non-secret values is very low,
compared to the high and demonstrated risk of secrets leaking via a
common extension pattern.
9ba8f70
@xaviershay

Sample mail after my above config:

test failure
app/controllers/health_controller.rb:6:in `test_exceptions'
------------------------------- Request:

* URL        : http://redacted.example.com/_exception
* HTTP Method: GET
* IP address : 50.172.237.250
* Parameters : {"controller"=>"health", "action"=>"test_exceptions"}
* Timestamp  : 2015-04-19 08:19:53 +1000
* Server : c5cf4ef6-47fe-472e-a23e-4340a7aa9499
* Rails root : /app
* Process: 12
------------------------------- Environment:

* HTTP_ACCEPT                                    : text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
* HTTP_ACCEPT_ENCODING                           : gzip, deflate, sdch
* HTTP_ACCEPT_LANGUAGE                           : en-US,en;q=0.8
* HTTP_CONNECTION                                : close
* HTTP_CONNECT_TIME                              : 0
* HTTP_HOST                                      : redacted.example.com
* HTTP_TOTAL_ROUTE_TIME                          : 0
* HTTP_USER_AGENT                                : Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2311.90 Safari/537.36
* HTTP_VERSION                                   : HTTP/1.1
* HTTP_VIA                                       : 1.1 vegur
* HTTP_X_FORWARDED_FOR                           : 50.172.237.250
* HTTP_X_FORWARDED_PORT                          : 80
* HTTP_X_FORWARDED_PROTO                         : http
* HTTP_X_REQUEST_ID                              : 421fa364-7456-4426-a731-bf3ea7202ec6
* HTTP_X_REQUEST_START                           : 1429395592744
* ORIGINAL_FULLPATH                              : /_exception
* ORIGINAL_SCRIPT_NAME                           :
* PATH_INFO                                      : /_exception
* QUERY_STRING                                   :
* REMOTE_ADDR                                    : 10.169.148.51
* REQUEST_METHOD                                 : GET
* REQUEST_PATH                                   : /_exception
* REQUEST_URI                                    : /_exception
* ROUTES_70319103527460_SCRIPT_NAME              :
* SCRIPT_NAME                                    :
* SERVER_NAME                                    : redacted.example.com
* SERVER_PORT                                    : 80
* SERVER_PROTOCOL                                : HTTP/1.1
* SERVER_SOFTWARE                                : Unicorn 4.8.3
* action_controller.instance                     : #<HealthController:0x007fe8e5baf4e8>
* action_dispatch.backtrace_cleaner              : #<Rails::BacktraceCleaner:0x007fe8e22ac6c8>
* action_dispatch.cookies                        : [FILTERED]
* action_dispatch.cookies_digest                 : [FILTERED]
* action_dispatch.cookies_serializer             : [FILTERED]
* action_dispatch.encrypted_cookie_salt          : [FILTERED]
* action_dispatch.encrypted_signed_cookie_salt   : [FILTERED]
* action_dispatch.http_auth_salt                 : [FILTERED]
* action_dispatch.key_generator                  : #<ActiveSupport::CachingKeyGenerator:0x007fe8e4e560f8>
* action_dispatch.logger                         : #<Logger:0x007fe8e25caa80>
* action_dispatch.parameter_filter               : [:password, :session, :warden, :secret, :salt, :cookie, :csrf]
* action_dispatch.redirect_filter                : []
* action_dispatch.remote_ip                      : 50.172.237.250
* action_dispatch.request.content_type           :
* action_dispatch.request.formats                : [#<Mime::Type:0x007fe8e12491c8 @synonyms=["application/xhtml+xml"], @symbol=:html, @string="text/html">]
* action_dispatch.request.parameters             : {"controller"=>"health", "action"=>"test_exceptions"}
* action_dispatch.request.path_parameters        : {:controller=>"health", :action=>"test_exceptions"}
* action_dispatch.request.query_parameters       : {}
* action_dispatch.request.request_parameters     : {}
* action_dispatch.request.unsigned_session_cookie: [FILTERED]
* action_dispatch.request_id                     : 421fa364-7456-4426-a731-bf3ea7202ec6
* action_dispatch.routes                         : #<ActionDispatch::Routing::RouteSet:0x007fe8e25f9448>
* action_dispatch.secret_key_base                : [FILTERED]
* action_dispatch.secret_token                   : [FILTERED]
* action_dispatch.show_detailed_exceptions       : false
* action_dispatch.show_exceptions                : true
* action_dispatch.signed_cookie_salt             : [FILTERED]
* rack.errors                                    : #<IO:0x007fe8e055d608>
* rack.hijack                                    : #<Proc:0x007fe8e5ba4138@/app/vendor/bundle/ruby/2.2.0/gems/unicorn-4.8.3/lib/unicorn/http_request.rb:111>
* rack.hijack?                                   : true
* rack.input                                     : #<StringIO:0x007fe8e116e3e8>
* rack.logger                                    : #<Logger:0x007fe8e11eba50>
* rack.multiprocess                              : true
* rack.multithread                               : false
* rack.request.cookie_hash                       : [FILTERED]
* rack.request.query_hash                        : {}
* rack.request.query_string                      :
* rack.run_once                                  : false
* rack.session                                   : [FILTERED]
* rack.session.options                           : [FILTERED]
* rack.url_scheme                                : http
* rack.version                                   : [1, 2]
* warden                                         : [FILTERED]
------------------------------- Backtrace:

app/controllers/health_controller.rb:6:in `test_exceptions'
vendor/bundle/ruby/2.2.0/gems/actionpack-
@san650 san650 added the bug label Dec 19, 2015
@mwatt mwatt pushed a commit to mwatt/rails that referenced this issue Jun 27, 2016
@xaviershay xaviershay Add more common secret names to filter parameters defaults.
This configuration is often used by extensions for filtering of Rails
session and environment data, for example:

* smartinez87/exception_notification#182
* bugsnag/bugsnag-ruby@7d40acb

As a result, they tend to be insecure until they are explicitly patched.
In the event of exception notification, this has not happened even after
the insecure defaults being present for many years.

The risk of these names being used for non-secret values is very low,
compared to the high and demonstrated risk of secrets leaking via a
common extension pattern.
8c19717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment