Skip to content
Permalink
Browse files Browse the repository at this point in the history
FEATURE: perform security checks earlier in the pipeline
This avoids needing to discard results and ensures no areas of mini profiler
can execute prior to a request being authorized

authorization token lasts for 30 minutes and is cycled after that
  • Loading branch information
SamSaffron committed May 18, 2016
1 parent bf33bd7 commit 4273771
Show file tree
Hide file tree
Showing 31 changed files with 435 additions and 94 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -2,4 +2,4 @@ source 'http://rubygems.org'

gemspec

gem 'codecov', :require => false, :group => :test
gem 'codecov', :require => false, :group => :test
30 changes: 30 additions & 0 deletions Guardfile
@@ -0,0 +1,30 @@
# A sample Guardfile
# More info at https://github.com/guard/guard#readme

directories %w(lib spec) \
.select{|d| Dir.exists?(d) ? d : UI.warning("Directory #{d} does not exist")}

## Note: if you are using the `directories` clause above and you are not
## watching the project directory ('.'), then you will want to move
## the Guardfile to a watched dir and symlink it back, e.g.
#
# $ mkdir config
# $ mv Guardfile config/
# $ ln -s config/Guardfile .
#
# and, you'll have to watch "config/Guardfile" instead of "Guardfile"

# Note: The cmd option is now required due to the increasing number of ways
# rspec may be run, below are examples of the most common uses.
# * bundler: 'bundle exec rspec'
# * bundler binstubs: 'bin/rspec'
# * spring: 'bin/rsspec' (This will use spring if running and you have
# installed the spring binstubs per the docs)
# * zeus: 'zeus rspec' (requires the server to be started separetly)
# * 'just' rspec: 'rspec'
guard :rspec, cmd: 'bundle exec rspec', failed_mode: :focus do
watch(%r{^spec/.+_spec\.rb$})
watch(%r{^lib/mini_profiler/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" }
watch('spec/spec_helper.rb') { "spec" }
end

2 changes: 0 additions & 2 deletions autotest/discover.rb

This file was deleted.

67 changes: 59 additions & 8 deletions lib/mini_profiler/client_settings.rb
Expand Up @@ -12,39 +12,90 @@ class ClientSettings
attr_accessor :backtrace_level


def initialize(env)
def initialize(env, store, start)
request = ::Rack::Request.new(env)
@cookie = request.cookies[COOKIE_NAME]
@store = store
@start = start

@allowed_tokens, @orig_auth_tokens = nil

if @cookie
@cookie.split(",").map{|pair| pair.split("=")}.each do |k,v|
@orig_disable_profiling = @disable_profiling = (v=='t') if k == "dp"
@backtrace_level = v.to_i if k == "bt"
@orig_auth_tokens = v.to_s.split("|") if k == "a"
end
end

@backtrace_level = nil if !@backtrace_level.nil? && (@backtrace_level == 0 || @backtrace_level > BACKTRACE_NONE)
if !@backtrace_level.nil? && (@backtrace_level == 0 || @backtrace_level > BACKTRACE_NONE)
@backtrace_level = nil
end

@orig_backtrace_level = @backtrace_level

end

def handle_cookie(result)
status,headers,_body = result

if (MiniProfiler.config.authorization_mode == :whitelist && !MiniProfiler.request_authorized?)
# this is non-obvious, don't kill the profiling cookie on errors or short requests
# this ensures that stuff that never reaches the rails stack does not kill profiling
if status.to_i >= 200 && status.to_i < 300 && ((Time.now - @start) > 0.1)
discard_cookie!(headers)
end
else
write!(headers)
end

result
end

def write!(headers)
if @orig_disable_profiling != @disable_profiling || @orig_backtrace_level != @backtrace_level || @cookie.nil?

tokens_changed = false

if MiniProfiler.request_authorized? && MiniProfiler.config.authorization_mode == :whitelist
@allowed_tokens ||= @store.allowed_tokens
tokens_changed = !@orig_auth_tokens || ((@allowed_tokens - @orig_auth_tokens).length > 0)
end

if @orig_disable_profiling != @disable_profiling ||
@orig_backtrace_level != @backtrace_level ||
@cookie.nil? ||
tokens_changed

settings = {"p" => "t" }
settings["dp"] = "t" if @disable_profiling
settings["bt"] = @backtrace_level if @backtrace_level
settings["dp"] = "t" if @disable_profiling
settings["bt"] = @backtrace_level if @backtrace_level
settings["a"] = @allowed_tokens.join("|") if @allowed_tokens && MiniProfiler.request_authorized?

settings_string = settings.map{|k,v| "#{k}=#{v}"}.join(",")
Rack::Utils.set_cookie_header!(headers, COOKIE_NAME, :value => settings_string, :path => '/')
end
end

def discard_cookie!(headers)
Rack::Utils.delete_cookie_header!(headers, COOKIE_NAME, :path => '/')
if @cookie
Rack::Utils.delete_cookie_header!(headers, COOKIE_NAME, :path => '/')
end
end

def has_cookie?
!@cookie.nil?
def has_valid_cookie?
valid_cookie = !@cookie.nil?

if (MiniProfiler.config.authorization_mode == :whitelist)
@allowed_tokens ||= @store.allowed_tokens

valid_cookie = (Array === @orig_auth_tokens) &&
((@allowed_tokens & @orig_auth_tokens).length > 0)
end

valid_cookie
end


def disable_profiling?
@disable_profiling
end
Expand Down
3 changes: 2 additions & 1 deletion lib/mini_profiler/context.rb
@@ -1,5 +1,6 @@
class Rack::MiniProfiler::Context
attr_accessor :inject_js,:current_timer,:page_struct,:skip_backtrace,:full_backtrace,:discard, :mpt_init, :measure
attr_accessor :inject_js,:current_timer,:page_struct,:skip_backtrace,
:full_backtrace,:discard, :mpt_init, :measure

def initialize(opts = {})
opts["measure"] = true unless opts.key? "measure"
Expand Down
55 changes: 21 additions & 34 deletions lib/mini_profiler/profiler.rb
Expand Up @@ -149,7 +149,9 @@ def config

def call(env)

client_settings = ClientSettings.new(env)
start = Time.now
client_settings = ClientSettings.new(env, @storage, start)
MiniProfiler.deauthorize_request if @config.authorization_mode == :whitelist

status = headers = body = nil
query_string = env['QUERY_STRING']
Expand All @@ -162,43 +164,39 @@ def call(env)
(@config.skip_paths && @config.skip_paths.any?{ |p| path.start_with?(p) }) ||
query_string =~ /pp=skip/

has_profiling_cookie = client_settings.has_cookie?

if skip_it || (@config.authorization_mode == :whitelist && !has_profiling_cookie)
status,headers,body = @app.call(env)
if !skip_it && @config.authorization_mode == :whitelist && !has_profiling_cookie && MiniProfiler.request_authorized?
client_settings.write!(headers)
end
return [status,headers,body]
if skip_it || (
@config.authorization_mode == :whitelist &&
!client_settings.has_valid_cookie?
)
return client_settings.handle_cookie(@app.call(env))
end

# handle all /mini-profiler requests here
return serve_html(env) if path.start_with? @config.base_url_path
return client_settings.handle_cookie(serve_html(env)) if path.start_with? @config.base_url_path

has_disable_cookie = client_settings.disable_profiling?
# manual session disable / enable
if query_string =~ /pp=disable/ || has_disable_cookie
skip_it = true
end

if query_string =~ /pp=enable/ && (@config.authorization_mode != :whitelist || MiniProfiler.request_authorized?)
if query_string =~ /pp=enable/
skip_it = false
config.enabled = true
end

if skip_it || !config.enabled
status,headers,body = @app.call(env)
client_settings.disable_profiling = true
client_settings.write!(headers)
return [status,headers,body]
return client_settings.handle_cookie([status,headers,body])
else
client_settings.disable_profiling = false
end

# profile gc
if query_string =~ /pp=profile-gc/
current.measure = false if current
return Rack::MiniProfiler::GCProfiler.new.profile_gc(@app, env)
return client_settings.handle_cookie(Rack::MiniProfiler::GCProfiler.new.profile_gc(@app, env))
end

# profile memory
Expand All @@ -215,11 +213,10 @@ def call(env)
body.close if body.respond_to? :close
end
report.pretty_print(result)
return text_result(result.string)
return client_settings.handle_cookie(text_result(result.string))
end

MiniProfiler.create_current(env, @config)
MiniProfiler.deauthorize_request if @config.authorization_mode == :whitelist

if query_string =~ /pp=normal-backtrace/
client_settings.backtrace_level = ClientSettings::BACKTRACE_DEFAULT
Expand All @@ -238,7 +235,6 @@ def call(env)
trace_exceptions = query_string =~ /pp=trace-exceptions/ && defined? TracePoint
status, headers, body, exceptions,trace = nil

start = Time.now

if trace_exceptions
exceptions = []
Expand Down Expand Up @@ -281,43 +277,37 @@ def call(env)
else
status,headers,body = @app.call(env)
end
client_settings.write!(headers)
ensure
trace.disable if trace
end

skip_it = current.discard

if (config.authorization_mode == :whitelist && !MiniProfiler.request_authorized?)
# this is non-obvious, don't kill the profiling cookie on errors or short requests
# this ensures that stuff that never reaches the rails stack does not kill profiling
if status.to_i >= 200 && status.to_i < 300 && ((Time.now - start) > 0.1)
client_settings.discard_cookie!(headers)
end
skip_it = true
end

return [status,headers,body] if skip_it
return client_settings.handle_cookie([status,headers,body]) if skip_it

# we must do this here, otherwise current[:discard] is not being properly treated
if trace_exceptions
body.close if body.respond_to? :close
return dump_exceptions exceptions
return client_settings.handle_cookie(dump_exceptions exceptions)
end

if query_string =~ /pp=env/ && !config.disable_env_dump
body.close if body.respond_to? :close
return dump_env env
return client_settings.handle_cookie(dump_env env)
end

if query_string =~ /pp=analyze-memory/
body.close if body.respond_to? :close
return analyze_memory
return client_settings.handle_cookie(analyze_memory)
end

if query_string =~ /pp=help/
body.close if body.respond_to? :close
return help(client_settings, env)
return client_settings.handle_cookie(help(client_settings, env))
end

page_struct = current.page_struct
Expand All @@ -326,7 +316,7 @@ def call(env)

if flamegraph
body.close if body.respond_to? :close
return self.flamegraph(flamegraph)
return client_settings.handle_cookie(self.flamegraph(flamegraph))
end


Expand All @@ -337,18 +327,16 @@ def call(env)

# inject headers, script
if status >= 200 && status < 300
client_settings.write!(headers)
result = inject_profiler(env,status,headers,body)
return result if result
return client_settings.handle_cookie(result) if result
end
rescue Exception => e
if @config.storage_failure != nil
@config.storage_failure.call(e)
end
end

client_settings.write!(headers)
[status, headers, body]
client_settings.handle_cookie([status, headers, body])

ensure
# Make sure this always happens
Expand Down Expand Up @@ -543,7 +531,6 @@ def help(client_settings, env)
</html>
"

client_settings.write!(headers)
[200, headers, [body]]
end

Expand Down
8 changes: 8 additions & 0 deletions lib/mini_profiler/storage/abstract_store.rb
Expand Up @@ -2,6 +2,9 @@ module Rack
class MiniProfiler
class AbstractStore

# maximum age of allowed tokens before cycling in seconds
MAX_TOKEN_AGE = 1800

def save(page_struct)
raise NotImplementedError.new("save is not implemented")
end
Expand Down Expand Up @@ -31,6 +34,11 @@ def diagnostics(user)
""
end

# a list of tokens that are permitted to access profiler in whitelist mode
def allowed_tokens
raise NotImplementedError.new("allowed_tokens is not implemented")
end

end
end
end
25 changes: 25 additions & 0 deletions lib/mini_profiler/storage/file_store.rb
Expand Up @@ -51,6 +51,9 @@ def initialize(args = nil)
@user_view_cache = FileCache.new(@path, "mp_views")
@user_view_lock = Mutex.new

@auth_token_cache = FileCache.new(@path, "tokens")
@auth_token_lock = Mutex.new

me = self
t = CacheCleanupThread.new do
interval = 10
Expand Down Expand Up @@ -126,6 +129,28 @@ def get_unviewed_ids(user)
}
end

def flush_tokens
@auth_token_lock.synchronize {
@auth_token_cache[""] = nil
}
end

def allowed_tokens
@auth_token_lock.synchronize {
token1, token2, cycle_at = @auth_token_cache[""]

unless cycle_at && (Time === cycle_at) && (cycle_at > Time.now)
token2 = token1
token1 = SecureRandom.hex
cycle_at = Time.now + Rack::MiniProfiler::AbstractStore::MAX_TOKEN_AGE
end

@auth_token_cache[""] = [token1, token2, cycle_at]

[token1, token2].compact
}
end

def cleanup_cache
files = Dir.entries(@path)
@timer_struct_lock.synchronize {
Expand Down

0 comments on commit 4273771

Please sign in to comment.