-
Notifications
You must be signed in to change notification settings - Fork 935
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
Is there a way to hide the password in the log? #349
Comments
+1 |
I ended up writing a wrapper for the RestClient that logs what I need to log, but is specific to the API I'm working with. # Private: Wrapper for the Gooddata REST client that performs logging and simplifies
# setting REST parameters.
#
# method - Symbol speciying the REST method to use. Curerntly only support :get and :put.
# url - Full URL to make the request against.
# log - Array that specifies whether requests and/or responses should be logged.
# Sometimes it is necessary to turn of response logging for large responses
# (e.g., data files). (default: [:request, :response]).
# &block - This method only works by specifying a block. It yeilds
# a RestParams object where the user sets the REST headers and values.
#
# Examples
#
# rest_client :post, "#{url_base}/gdc/account/login" do |params|
# params.headers = {
# content_type: 'application/json',
# accept: 'application/json'
# }
#
# params.values = {
# postUserLogin: {
# login: "#{username}",
# password: "#{password}",
# remember: 1
# }
# }
# end
#
# Returns a RestClient::Response when successful.
def rest_client(method, url, log: [:request, :response], &block)
params = RestParams.new
yield params
if Array(log).include? :request
masked_values = params.values.to_json
if params.values[:postUserLogin] && params.values[:postUserLogin][:password]
masked_values = masked_values.gsub(/#{params.values[:postUserLogin][:password]}/,'******')
end
@rest_log.info "#{method.upcase}: #{url}"
@rest_log.info "HEADERS: #{JSON.pretty_generate params.headers rescue params.headers}"
@rest_log.info "VALUES: #{JSON.pretty_generate JSON.parse(masked_values) rescue masked_values}"
end
response = case method
when :get
RestClient.get url, params.headers
when :post
RestClient.post url, params.values.to_json, params.headers
else
raise "Unkown RestClient method #{method}"
end
@rest_log.info "RESPONSE (headers): #{JSON.pretty_generate response.headers rescue response.headers}"
@rest_log.info "RESPONSE: #{JSON.pretty_generate JSON.parse(response) rescue response}" if Array(log).include? :response
response
end
# Public: Simple class to hold headers and values for REST queries.
class RestParams
# Public: Initializes the REST parameters (headers and values). Headers
# and values may either be a hash or a JSON-formatted string. Strings
# are converted and stored as a hash.
#
# headers - The REST headers parameters. May be a hash or a JSON-formatting string.
# values - The REST values parameters. May be a hash or a JSON-formatting string.
def initialize(headers: {}, values: {})
self.headers = headers
self.values = values
end
# Public: Returns the headers parameters as a hash.
attr_reader :headers
# Public: Returns the values parameters as a hash.
attr_reader :values
# Public: Setter for the headers. Strings are parsed with JSON. Otherwise,
# it assumes the headers are stored as a hash.
def headers=(val)
@headers = val.is_a?(String) ? JSON.parse(val) : val
end
# Public: Setter for the values. Strings are parsed with JSON. Otherwise,
# it assumes the values are stored as a hash.
def values=(val)
@values = val.is_a?(String) ? JSON.parse(val) : val
end
end |
Heads up that OSVDB-117461 was filed for this. That caused bundler-audit to recommend removing rest-client from an app's Gemfile.
|
It looks like logging is opt in, so if I'm correct you wouldn't be impacted unless you enabled it. |
I don't have the time to implement a fix right now, but if someone is looking to jump in, this seems to be the point at which you'd have to implement log filtering: https://github.com/rest-client/rest-client/blob/master/lib/restclient/request.rb#L535 The response could also potentially contain filterable params: https://github.com/rest-client/rest-client/blob/master/lib/restclient/request.rb#L545 |
To elaborate on @derekprior's point, if I've been speed-reading the code correctly, rest-client doesn't log unless RestClient.log is set. |
Repro: ENV['RESTCLIENT_LOG'] = "stdout"
require 'rest-client'
RestClient.get 'https://user:password@example.com/private/resource' output:
|
Fixes rest-client#349 and OSVDB-117461.
See referenced PR for a fix. |
This is merged in release 1.7.3: Thanks, everyone, for bearing with this report. |
* OSVDB-119878 - Session fixation vulnerability via Set-Cookie headers - rest-client/rest-client#369 * OSVDB-117461 - Log plaintext password local disclosure - rest-client/rest-client#349
* OSVDB-119878 - Session fixation vulnerability via Set-Cookie headers - rest-client/rest-client#369 * OSVDB-117461 - Log plaintext password local disclosure - rest-client/rest-client#349
Would a PR be accepted to also redact the Bearer token in the |
I was wondering if there was a good way to hide the login password from the log? Alternatively, could we disable logging for certain actions (like log in)?
The text was updated successfully, but these errors were encountered: