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

Logging handler exposes values of CloudFormation NoEcho parameters #726

Closed
lwoggardner opened this issue Mar 2, 2015 · 4 comments
Closed
Labels
feature-request A feature should be added or improved.

Comments

@lwoggardner
Copy link

To replicate:

Enable logging with a pattern that include ':request_params' on AWS::CloudFormation::Client and create/update stack with a NoEcho parameter.

Have not tested whether they also leak in other logging patterns (request headers/body?)

Seahorse::Logging::Formatter could possibly deal with this, but may be safer to mask the parameters in the api context as soon as they are used.

@trevorrowe
Copy link
Member

This is an interesting issue. The NoEcho parameter is intended for CloudFormation to know when it should or should not return parameters back over the wire. From the SDK's perspective, the boolean has no meaning.

One option would be to add a customization to the log formatter that made it CloudFormation aware. I'm against this for a number of reasons. Instead, I think the better, long-term option is to make the Seahorse::Client::Logging::Formatter more flexible.

A user, should be able to register a list of parameter names to black-or-white-list. These would be omitted from request parameter logging. If response data logging were added, this list could be observed there as well. Filtering headers could be addressed in a similar manner, but with a different list.

Until a a feature is added to support this, you can work around it by either providing your own log formatter (which could potentially sub-class the existing one) or simply remove :request_params from the log pattern.

Thoughts?

@trevorrowe trevorrowe added feature-request A feature should be added or improved. Version 2 labels Mar 3, 2015
@lwoggardner
Copy link
Author

Yes, of course. The SDK may not even have the template to know that a given parameter is marked "NoEcho".

The feature sounds good - would suggest blacklist should be regexes - eg /password|no.*echo/i

Also, the filter does need to be CF aware in some respects because the filtering needs to occur deep into the parameters hash. (ie the request parameter is actually "parameters", whose value an array of hashes of the form { parameter_key: "myNoEchoParameter", parameter_value: "topsecret" }

(BTW I assume the reason it is done that way is to fit the basic API, as it would be much preferable from rubyist point of view to just pass the parameters as a straight hash)

I will attempt to create a custom formatter to implement this.

@lwoggardner
Copy link
Author

# @param [String] pattern The log format pattern should be a string
#   and may contain substitutions.
#
# @option options [Regex] :mask_filter (/password|no.*echo/i)
#   When summarizing hash values, any keys matching this mask filter
#   will have the value masked with "*******"
class MaskingFormatter < Seahorse::Client::Logging::Formatter

  # regex to match against has key names
 attr_reader :mask_filter

  def initialize(pattern, options = {})
    super
    @mask_filter = options[:mask_filter] || /password|no.*echo/i
  end

  private

  def summarize_string_hash(hash)
    hash.map do |key,v|
      masked_v = (key =~ mask_filter || ( key =~ /value/ && hash[key.gsub('value','key')] =~ mask_filter)) ? '********' : v
      "#{key.inspect}=>#{summarize_value(masked_v)}"
    end.join(",")
  end

  def summarize_symbol_hash(hash)
    hash.map do |key,v|
      masked_v = (key =~ mask_filter || ( key =~ /value/ && hash[key.to_s.gsub('value','key').to_sym] =~ mask_filter)) ? '*******' : v
      "#{key}:#{summarize_value(masked_v)}"
    end.join(",")
  end

end

@trevorrowe
Copy link
Member

This looks like a reasonable solution for your specific logging problem. I'll add a more generalized solution to the backlog of feature work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

2 participants