Permalink
Browse files

Switch back to Rack::Session::Cookie, but generate a secret.

  • Loading branch information...
1 parent 7b393f3 commit 93d6e1f35a9b96242224bd227f5728800934b2a1 @rkh rkh committed Mar 12, 2011
Showing with 13 additions and 8 deletions.
  1. +2 −4 CHANGES
  2. +11 −4 lib/sinatra/base.rb
View
@@ -1,9 +1,7 @@
= 1.2.1 / Not Yet Release
- * Switched default session middleware from `Rack::Session::Cookies` to
- `Rack::Session::Pool`, to improve security. Using `Rack::Session::Cookies`
- without a secret allows injecting arbitrary objects into sessions an, in a
- worst case scenario, might lead to code injection. (Konstantin Haase)
+ * Use a generated session secret when using `enable :sessions`. (Konstantin
+ Haase)
= 1.2.0 / 2011-03-03
View
@@ -1232,10 +1232,9 @@ def new(*args, &bk)
# an instance of this class as end point.
def build(*args, &bk)
builder = Rack::Builder.new
- builder.use Rack::Session::Pool if sessions?
- builder.use Rack::CommonLogger if logging?
- builder.use Rack::MethodOverride if method_override?
- builder.use ShowExceptions if show_exceptions?
+ builder.use Rack::CommonLogger if logging?
+ builder.use Rack::MethodOverride if method_override?
+ builder.use ShowExceptions if show_exceptions?
middleware.each { |c,a,b| builder.use(c, *a, &b) }
builder.run new!(*args, &bk)
builder
@@ -1246,6 +1245,11 @@ def call(env)
end
private
+ def setup_sessions(builder)
+ return unless sessions?
+ builder.use Rack::Session::Cookie, :secret => session_secret
+ end
+
def detect_rack_handler
servers = Array(server)
servers.each do |server_name|
@@ -1342,6 +1346,9 @@ def self.force_encoding(data, *) data end
set :default_encoding, "utf-8"
set :add_charset, [/^text\//, 'application/javascript', 'application/xml', 'application/xhtml+xml']
+ # explicitly generating this eagerly to play nice with preforking
+ set :session_secret, '%x' % rand(2**255)
+
class << self
alias_method :methodoverride?, :method_override?
alias_method :methodoverride=, :method_override=

1 comment on commit 93d6e1f

Thank you so very much. I railed against the default no-secret a year or two ago.. it was very dangerous out of the box IMHO and there were numerous sites using it incorrectly that were not aware of this.

I would personally recommend requiring a secret for enable :sessions, or some kind of a warning or something. Which of course may require some refactoring of the enable stuff, but I think it's worth it. I really can't think of a situation where you would want to enable this without a secret.

Please sign in to comment.