From 1f1e58e221b19efd8a6519d2ac62978b05da8b3d Mon Sep 17 00:00:00 2001 From: Konstantin Haase Date: Fri, 2 Sep 2011 14:54:49 -0600 Subject: [PATCH] add rack-protection, fixes #310 --- CHANGES | 3 +++ Gemfile | 1 + README.rdoc | 23 ++++++++++++++++++++++ lib/sinatra/base.rb | 15 +++++++++++++-- sinatra.gemspec | 5 +++-- test/helpers_test.rb | 2 +- test/settings_test.rb | 45 +++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 89 insertions(+), 5 deletions(-) diff --git a/CHANGES b/CHANGES index 3f955388..d299b048 100644 --- a/CHANGES +++ b/CHANGES @@ -11,6 +11,9 @@ * Added support for HTTP PATCH requests. (Konstantin Haase) + * Use rack-protection to defend against common opportunistic attacks. + (Konstantin Haase) + * Support for Creole templates, Creole is a standardized wiki markup, supported by many wiki implementations. (Konstanin Haase) diff --git a/Gemfile b/Gemfile index d5363c0d..bbb687ea 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,7 @@ RUBY_ENGINE = 'ruby' unless defined? RUBY_ENGINE source :rubygems unless ENV['QUICK'] +gemspec gem 'rake' gem 'rack-test', '>= 0.5.6' diff --git a/README.rdoc b/README.rdoc index 2d7da48a..36e5858e 100644 --- a/README.rdoc +++ b/README.rdoc @@ -104,6 +104,9 @@ Route patterns may have optional parameters: # matches "GET /posts" and any extension "GET /posts.json", "GET /posts.xml" etc. end +By the way, unless you disable the path traversal attack protection (see below), +the request path might be modified before matching against your routes. + === Conditions Routes may include a variety of matching conditions, such as the user agent: @@ -1238,6 +1241,23 @@ You can access those options via settings: ... end +=== Configuring attack protection + +Sinatra is using +{Rack::Protection}[https://github.com/rkh/rack-protection#readme] to defend +you application against common, opportunistic attacks. You can easily disable +this behavior (which should result in performance gains): + + disable :protection + +To skip a single defense layer, set +protection+ to an options hash: + + set :protection, :except => :path_traversal + +You can also hand in an array in order to disable a list of protections: + + set :protections, :except => [:path_traversal, :session_hijacking] + === Available Settings [absolute_redirects] If disabled, Sinatra will allow relative redirects, @@ -1290,6 +1310,9 @@ You can access those options via settings: redirect '/foo' would behave like redirect to('/foo'). Disabled per default. +[protection] Whether or not to enable web attack protections. See + protection section above. + [public_folder] folder public files are served from [reload_templates] whether or not to reload templates between requests. diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index c82f1207..d9a7ce24 100644 --- a/lib/sinatra/base.rb +++ b/lib/sinatra/base.rb @@ -1,6 +1,7 @@ # external dependencies require 'rack' require 'tilt' +require "rack/protection" # stdlib dependencies require 'thread' @@ -1308,8 +1309,9 @@ module Sinatra builder.use ShowExceptions if show_exceptions? builder.use Rack::MethodOverride if method_override? builder.use Rack::Head - setup_logging builder - setup_sessions builder + setup_logging builder + setup_sessions builder + setup_protection builder end def setup_middleware(builder) @@ -1329,6 +1331,14 @@ module Sinatra end end + def setup_protection(builder) + return unless protection? + options = Hash === protection ? protection.dup : {} + options[:except] = Array options[:except] + options[:except] += [:session_hijacking, :remote_token] unless sessions? + builder.use Rack::Protection, options + end + def setup_sessions(builder) return unless sessions? options = {} @@ -1436,6 +1446,7 @@ module Sinatra set :show_exceptions, Proc.new { development? } set :sessions, false set :logging, false + set :protection, true set :method_override, false set :default_encoding, "utf-8" set :add_charset, %w[javascript xml xhtml+xml json].map { |t| "application/#{t}" } diff --git a/sinatra.gemspec b/sinatra.gemspec index e6c0a781..dfe7ada0 100644 --- a/sinatra.gemspec +++ b/sinatra.gemspec @@ -12,6 +12,7 @@ Gem::Specification.new 'sinatra', Sinatra::VERSION do |s| s.extra_rdoc_files = s.files.select { |p| p =~ /^README/ } << 'LICENSE' s.rdoc_options = %w[--line-numbers --inline-source --title Sinatra --main README.rdoc] - s.add_dependency 'rack', '~> 1.3' - s.add_dependency 'tilt', '~> 1.3' + s.add_dependency 'rack', '~> 1.3' + s.add_dependency 'rack-protection', '~> 1.0' + s.add_dependency 'tilt', '~> 1.3' end diff --git a/test/helpers_test.rb b/test/helpers_test.rb index eeb6e339..ca6d1c82 100644 --- a/test/helpers_test.rb +++ b/test/helpers_test.rb @@ -380,7 +380,7 @@ class HelpersTest < Test::Unit::TestCase enable :sessions get '/' do - assert session.empty? + assert session[:foo].nil? session[:foo] = 'bar' redirect '/hi' end diff --git a/test/settings_test.rb b/test/settings_test.rb index 4654f1ac..b2e58de3 100644 --- a/test/settings_test.rb +++ b/test/settings_test.rb @@ -490,4 +490,49 @@ class SettingsTest < Test::Unit::TestCase assert ! @application.lock? end end + + describe 'protection' do + class MiddlewareTracker < Rack::Builder + def self.track + Rack.send :remove_const, :Builder + Rack.const_set :Builder, MiddlewareTracker + MiddlewareTracker.used.clear + yield + ensure + Rack.send :remove_const, :Builder + Rack.const_set :Builder, MiddlewareTracker.superclass + end + + def self.used + @used ||= [] + end + + def use(middleware, *) + MiddlewareTracker.used << middleware + super + end + end + + it 'sets up Rack::Protection' do + MiddlewareTracker.track do + Sinatra::Base.new + assert_include MiddlewareTracker.used, Rack::Protection + end + end + + it 'sets up Rack::Protection::PathTraversal' do + MiddlewareTracker.track do + Sinatra::Base.new + assert_include MiddlewareTracker.used, Rack::Protection::PathTraversal + end + end + + it 'does not set up Rack::Protection::PathTraversal when disabling it' do + MiddlewareTracker.track do + Sinatra.new { set :protection, :except => :path_traversal }.new + assert_include MiddlewareTracker.used, Rack::Protection + assert !MiddlewareTracker.used.include?(Rack::Protection::PathTraversal) + end + end + end end