From 64aaf4cb54c4eadd410133d72810b0124e818ce2 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 18 Dec 2019 09:31:42 -0800
Subject: [PATCH] Squashed commit of the following:
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
commit d3e2f88c17dad2c7997e453d7ef518dd6e751ac8
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Dec 17 12:17:34 2019 -0800
making diff smaller
commit 99a8a8776513839b5da4af393b67afe95a9412d8
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Dec 17 12:13:00 2019 -0800
fix memcache tests on 1.6
commit f2cb48e50e507e638973f331d4a62099fae567ec
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Dec 17 11:48:45 2019 -0800
fix tests on 1.6
commit 7ff635c51d29f3e19377855f6010574fb2e8e593
Author: Rafael Mendon�a Fran�a <rafael@franca.dev>
Date: Mon Oct 21 16:39:00 2019 -0400
Introduce a new base class to avoid breaking when upgrading
Third-party session store would still need to be chaged to be more
secure but only upgrading rack will not break any application.
commit 3232f9370d099e784a16c01d32e8a2da4a953f18
Author: Rafael Mendon�a Fran�a <rafael@franca.dev>
Date: Wed Oct 16 14:07:36 2019 -0400
Add a version prefix to the private id to make easier to migrate old values
commit 15da2e5d95228d0b3fcdb38b2a562efc333402f0
Author: Rafael Mendon�a Fran�a <rafael@franca.dev>
Date: Wed Oct 9 19:14:08 2019 -0400
Fallback to the public id when reading the session in the pool adapter
commit 1a532d13eee9d5546349b5253a204187773de151
Author: Rafael Mendon�a Fran�a <rafael@franca.dev>
Date: Wed Oct 9 18:06:23 2019 -0400
Also drop the session with the public id when destroying sessions
commit 9fe40c68b514e0f4a947577e4b903a9ae477365e
Author: Rafael Mendon�a Fran�a <rafael@franca.dev>
Date: Wed Oct 9 17:50:45 2019 -0400
Fallback to the legacy id when the new id is not found
This will avoid all session to be invalidated.
commit b9565a90eea77960e552e3c86b0adb83ab034726
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 17:16:23 2019 -0400
Add the private id
commit 368effdbeca7955f4e46ea51bfb2d647bc79aa6b
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 16:48:41 2019 -0400
revert conditionals to master
commit d49aa811d6c8fe109c7fc5ca9bddb3d9f7eba796
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 16:45:04 2019 -0400
remove NullSession
commit 3e9cb660cc8bf9543b134b6f3ca35aadfe4e0611
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 16:38:01 2019 -0400
remove || raise and get closer to master
commit 442dba2362558e4a7a3e39d437b95d81f2479b31
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 16:31:06 2019 -0400
store hashed id, send public id
commit 3ab0277cd129f15059662451718048bcf23cb5d1
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 16:20:32 2019 -0400
use session id objects
commit 7237b6661ad98c1dac6ad799192262697e1a3559
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 15:43:58 2019 -0400
remove more nils
commit 511f809e80c3264af3a26d485a5137ac28f08ebe
Author: Aaron Patterson <aaron.patterson@gmail.com>
Date: Tue Aug 13 15:32:20 2019 -0400
try to ensure we always have some kind of object
---
lib/rack/session/abstract/id.rb | 82 ++++++++++++++++++++++++++++++--
lib/rack/session/cookie.rb | 13 ++++-
lib/rack/session/memcache.rb | 18 ++++---
lib/rack/session/pool.rb | 19 +++++---
test/spec_session_abstract_id.rb | 2 +-
test/spec_session_memcache.rb | 43 +++++++++++++++--
test/spec_session_pool.rb | 43 +++++++++++++++--
7 files changed, 196 insertions(+), 24 deletions(-)
diff --git a/lib/rack/session/abstract/id.rb b/lib/rack/session/abstract/id.rb
index 62bdb04..c13faf5 100644
--- a/lib/rack/session/abstract/id.rb
+++ b/lib/rack/session/abstract/id.rb
@@ -9,11 +9,38 @@ begin
rescue LoadError
# We just won't get securerandom
end
+require "digest/sha2"
module Rack
module Session
+ class SessionId
+ ID_VERSION = 2
+
+ attr_reader :public_id
+
+ def initialize(public_id)
+ @public_id = public_id
+ end
+
+ def private_id
+ "#{ID_VERSION}::#{hash_sid(public_id)}"
+ end
+
+ alias :cookie_value :public_id
+
+ def empty?; false; end
+ def to_s; raise; end
+ def inspect; public_id.inspect; end
+
+ private
+
+ def hash_sid(sid)
+ Digest::SHA256.hexdigest(sid)
+ end
+ end
+
module Abstract
ENV_SESSION_KEY = 'rack.session'.freeze
ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze
@@ -191,7 +218,7 @@ module Rack
# Not included by default; you must require 'rack/session/abstract/id'
# to use.
- class ID
+ class Persisted
DEFAULT_OPTIONS = {
:key => 'rack.session',
:path => '/',
@@ -342,10 +369,10 @@ module Rack
if not data = set_session(env, session_id, session_data, options)
env["rack.errors"].puts("Warning! #{self.class.name} failed to save session. Content dropped.")
elsif options[:defer] and not options[:renew]
- env["rack.errors"].puts("Deferring cookie for #{session_id}") if $VERBOSE
+ env["rack.errors"].puts("Deferring cookie for #{session_id.public_id}") if $VERBOSE
else
cookie = Hash.new
- cookie[:value] = data
+ cookie[:value] = cookie_value(data)
cookie[:expires] = Time.now + options[:expire_after] if options[:expire_after]
cookie[:expires] = Time.now + options[:max_age] if options[:max_age]
set_cookie(env, headers, cookie.merge!(options))
@@ -354,6 +381,10 @@ module Rack
[status, headers, body]
end
+ def cookie_value(data)
+ data
+ end
+
# Sets the cookie back to the client with session id. We skip the cookie
# setting if the value didn't change (sid is the same) or expires was given.
@@ -394,6 +425,51 @@ module Rack
raise '#destroy_session not implemented'
end
end
+
+ class PersistedSecure < Persisted
+ class SecureSessionHash < SessionHash
+ def [](key)
+ if key == "session_id"
+ load_for_read!
+ id.public_id
+ else
+ super
+ end
+ end
+ end
+
+ def generate_sid(*)
+ public_id = super
+
+ SessionId.new(public_id)
+ end
+
+ def extract_session_id(*)
+ public_id = super
+ public_id && SessionId.new(public_id)
+ end
+
+ private
+
+ def session_class
+ SecureSessionHash
+ end
+
+ def cookie_value(data)
+ data.cookie_value
+ end
+ end
+
+ class ID < Persisted
+ def self.inherited(klass)
+ k = klass.ancestors.find { |kl| kl.respond_to?(:superclass) && kl.superclass == ID }
+ unless k.instance_variable_defined?(:"@_rack_warned")
+ warn "#{klass} is inheriting from #{ID}. Inheriting from #{ID} is deprecated, please inherit from #{Persisted} instead" if $VERBOSE
+ k.instance_variable_set(:"@_rack_warned", true)
+ end
+ super
+ end
+ end
end
end
end
diff --git a/lib/rack/session/cookie.rb b/lib/rack/session/cookie.rb
index 9bea586..9ca9df2 100644
--- a/lib/rack/session/cookie.rb
+++ b/lib/rack/session/cookie.rb
@@ -44,7 +44,7 @@ module Rack
# })
#
- class Cookie < Abstract::ID
+ class Cookie < Abstract::PersistedSecure
# Encode session cookies as Base64
class Base64
def encode(str)
@@ -151,6 +151,15 @@ module Rack
data
end
+ class SessionId < DelegateClass(Session::SessionId)
+ attr_reader :cookie_value
+
+ def initialize(session_id, cookie_value)
+ super(session_id)
+ @cookie_value = cookie_value
+ end
+ end
+
def set_session(env, session_id, session, options)
session = session.merge("session_id" => session_id)
session_data = coder.encode(session)
@@ -163,7 +172,7 @@ module Rack
env["rack.errors"].puts("Warning! Rack::Session::Cookie data size exceeds 4K.")
nil
else
- session_data
+ SessionId.new(session_id, session_data)
end
end
diff --git a/lib/rack/session/memcache.rb b/lib/rack/session/memcache.rb
index c0e1f3e..1c6f256 100644
--- a/lib/rack/session/memcache.rb
+++ b/lib/rack/session/memcache.rb
@@ -19,7 +19,7 @@ module Rack
# Note that memcache does drop data before it may be listed to expire. For
# a full description of behaviour, please see memcache's documentation.
- class Memcache < Abstract::ID
+ class Memcache < Abstract::PersistedSecure
attr_reader :mutex, :pool
DEFAULT_OPTIONS = Abstract::ID::DEFAULT_OPTIONS.merge \
@@ -42,15 +42,15 @@ module Rack
def generate_sid
loop do
sid = super
- break sid unless @pool.get(sid, true)
+ break sid unless @pool.get(sid.private_id, true)
end
end
def get_session(env, sid)
with_lock(env) do
- unless sid and session = @pool.get(sid)
+ unless sid and session = get_session_with_fallback(sid)
sid, session = generate_sid, {}
- unless /^STORED/ =~ @pool.add(sid, session)
+ unless /^STORED/ =~ @pool.add(sid.private_id, session)
raise "Session collision on '#{sid.inspect}'"
end
end
@@ -63,14 +63,15 @@ module Rack
expiry = expiry.nil? ? 0 : expiry + 1
with_lock(env) do
- @pool.set session_id, new_session, expiry
+ @pool.set session_id.private_id, new_session, expiry
session_id
end
end
def destroy_session(env, session_id, options)
with_lock(env) do
- @pool.delete(session_id)
+ @pool.delete(session_id.public_id)
+ @pool.delete(session_id.private_id)
generate_sid unless options[:drop]
end
end
@@ -88,6 +89,11 @@ module Rack
@mutex.unlock if @mutex.locked?
end
+ private
+
+ def get_session_with_fallback(sid)
+ @pool.get(sid.private_id) || @pool.get(sid.public_id)
+ end
end
end
end
diff --git a/lib/rack/session/pool.rb b/lib/rack/session/pool.rb
index fcb34ec..e4afaaf 100644
--- a/lib/rack/session/pool.rb
+++ b/lib/rack/session/pool.rb
@@ -24,7 +24,7 @@ module Rack
# )
# Rack::Handler::WEBrick.run sessioned
- class Pool < Abstract::ID
+ class Pool < Abstract::PersistedSecure
attr_reader :mutex, :pool
DEFAULT_OPTIONS = Abstract::ID::DEFAULT_OPTIONS.merge :drop => false
@@ -37,15 +37,15 @@ module Rack
def generate_sid
loop do
sid = super
- break sid unless @pool.key? sid
+ break sid unless @pool.key? sid.private_id
end
end
def get_session(env, sid)
with_lock(env) do
- unless sid and session = @pool[sid]
+ unless sid and session = get_session_with_fallback(sid)
sid, session = generate_sid, {}
- @pool.store sid, session
+ @pool.store sid.private_id, session
end
[sid, session]
end
@@ -53,14 +53,15 @@ module Rack
def set_session(env, session_id, new_session, options)
with_lock(env) do
- @pool.store session_id, new_session
+ @pool.store session_id.private_id, new_session
session_id
end
end
def destroy_session(env, session_id, options)
with_lock(env) do
- @pool.delete(session_id)
+ @pool.delete(session_id.public_id)
+ @pool.delete(session_id.private_id)
generate_sid unless options[:drop]
end
end
@@ -71,6 +72,12 @@ module Rack
ensure
@mutex.unlock if @mutex.locked?
end
+
+ private
+
+ def get_session_with_fallback(sid)
+ @pool[sid.private_id] || @pool[sid.public_id]
+ end
end
end
end
diff --git a/test/spec_session_abstract_id.rb b/test/spec_session_abstract_id.rb
index 911f43b..99327a1 100644
--- a/test/spec_session_abstract_id.rb
+++ b/test/spec_session_abstract_id.rb
@@ -47,7 +47,7 @@ describe Rack::Session::Abstract::ID do
end
end
id = Rack::Session::Abstract::ID.new nil, :secure_random => secure_random.new
- id.send(:generate_sid).should.eql 'fake_hex'
+ id.send(:generate_sid).should.equal 'fake_hex'
end
end
diff --git a/test/spec_session_memcache.rb b/test/spec_session_memcache.rb
index 2b75980..5f50079 100644
--- a/test/spec_session_memcache.rb
+++ b/test/spec_session_memcache.rb
@@ -225,15 +225,52 @@ begin
req = Rack::MockRequest.new(pool)
res0 = req.get("/")
- session_id = (cookie = res0["Set-Cookie"])[session_match, 1]
- ses0 = pool.pool.get(session_id, true)
+ session_id = Rack::Session::SessionId.new (cookie = res0["Set-Cookie"])[session_match, 1]
+ ses0 = pool.pool.get(session_id.private_id, true)
req.get("/", "HTTP_COOKIE" => cookie)
- ses1 = pool.pool.get(session_id, true)
+ ses1 = pool.pool.get(session_id.private_id, true)
ses1.should.not.equal ses0
end
+ it "can read the session with the legacy id" do
+ pool = Rack::Session::Memcache.new(incrementor)
+ req = Rack::MockRequest.new(pool)
+
+ res0 = req.get("/")
+ cookie = res0["Set-Cookie"]
+ session_id = Rack::Session::SessionId.new cookie[session_match, 1]
+ ses0 = pool.pool.get(session_id.private_id, true)
+ pool.pool.set(session_id.public_id, ses0, 0, true)
+ pool.pool.delete(session_id.private_id)
+
+ res1 = req.get("/", "HTTP_COOKIE" => cookie)
+ res1["Set-Cookie"].should.be.nil
+ res1.body.should.equal '{"counter"=>2}'
+ pool.pool.get(session_id.private_id, true).should.not.be.nil
+ end
+
+ it "drops the session in the legacy id as well" do
+ pool = Rack::Session::Memcache.new(incrementor)
+ req = Rack::MockRequest.new(pool)
+ drop = Rack::Utils::Context.new(pool, drop_session)
+ dreq = Rack::MockRequest.new(drop)
+
+ res0 = req.get("/")
+ cookie = res0["Set-Cookie"]
+ session_id = Rack::Session::SessionId.new cookie[session_match, 1]
+ ses0 = pool.pool.get(session_id.private_id, true)
+ pool.pool.set(session_id.public_id, ses0, 0, true)
+ pool.pool.delete(session_id.private_id)
+
+ res2 = dreq.get("/", "HTTP_COOKIE" => cookie)
+ res2["Set-Cookie"].should.be.nil
+ res2.body.should.equal '{"counter"=>2}'
+ pool.pool.get(session_id.private_id, true).should.be.nil
+ pool.pool.get(session_id.public_id, true).should.be.nil
+ end
+
# anyone know how to do this better?
it "cleanly merges sessions when multithreaded" do
unless $DEBUG
diff --git a/test/spec_session_pool.rb b/test/spec_session_pool.rb
index 984f55a..ec9239e 100644
--- a/test/spec_session_pool.rb
+++ b/test/spec_session_pool.rb
@@ -5,7 +5,7 @@ require 'rack/session/pool'
describe Rack::Session::Pool do
session_key = Rack::Session::Pool::DEFAULT_OPTIONS[:key]
- session_match = /#{session_key}=[0-9a-fA-F]+;/
+ session_match = /#{session_key}=([0-9a-fA-F]+);/
incrementor = lambda do |env|
env["rack.session"]["counter"] ||= 0
@@ -13,7 +13,7 @@ describe Rack::Session::Pool do
Rack::Response.new(env["rack.session"].inspect).to_a
end
- session_id = Rack::Lint.new(lambda do |env|
+ get_session_id = Rack::Lint.new(lambda do |env|
Rack::Response.new(env["rack.session"].inspect).to_a
end)
@@ -142,6 +142,43 @@ describe Rack::Session::Pool do
pool.pool.size.should.equal 1
end
+ it "can read the session with the legacy id" do
+ pool = Rack::Session::Pool.new(incrementor)
+ req = Rack::MockRequest.new(pool)
+
+ res0 = req.get("/")
+ cookie = res0["Set-Cookie"]
+ session_id = Rack::Session::SessionId.new cookie[session_match, 1]
+ ses0 = pool.pool[session_id.private_id]
+ pool.pool[session_id.public_id] = ses0
+ pool.pool.delete(session_id.private_id)
+
+ res1 = req.get("/", "HTTP_COOKIE" => cookie)
+ res1["Set-Cookie"].should.be.nil
+ res1.body.should.equal '{"counter"=>2}'
+ pool.pool[session_id.private_id].should.not.be.nil
+ end
+
+ it "drops the session in the legacy id as well" do
+ pool = Rack::Session::Pool.new(incrementor)
+ req = Rack::MockRequest.new(pool)
+ drop = Rack::Utils::Context.new(pool, drop_session)
+ dreq = Rack::MockRequest.new(drop)
+
+ res0 = req.get("/")
+ cookie = res0["Set-Cookie"]
+ session_id = Rack::Session::SessionId.new cookie[session_match, 1]
+ ses0 = pool.pool[session_id.private_id]
+ pool.pool[session_id.public_id] = ses0
+ pool.pool.delete(session_id.private_id)
+
+ res2 = dreq.get("/", "HTTP_COOKIE" => cookie)
+ res2["Set-Cookie"].should.be.nil
+ res2.body.should.equal '{"counter"=>2}'
+ pool.pool[session_id.private_id].should.be.nil
+ pool.pool[session_id.public_id].should.be.nil
+ end
+
# anyone know how to do this better?
it "should merge sessions when multithreaded" do
unless $DEBUG
@@ -190,7 +227,7 @@ describe Rack::Session::Pool do
end
it "does not return a cookie if cookie was not written (only read)" do
- app = Rack::Session::Pool.new(session_id)
+ app = Rack::Session::Pool.new(get_session_id)
res = Rack::MockRequest.new(app).get("/")
res["Set-Cookie"].should.be.nil
end
--
2.21.0