From 9c147640d0af8d5149d85814e2949cd06741e52f Mon Sep 17 00:00:00 2001 From: Patrik Ragnarsson Date: Mon, 15 Jan 2024 16:22:22 +0100 Subject: [PATCH] Remove `Rack::Protection::EncryptedCookie` (#1989) The rack-session gem has equivalent functionality. Close https://github.com/sinatra/sinatra/issues/1945 --- .github/workflows/test.yml | 30 +- Gemfile | 6 + lib/sinatra/base.rb | 3 +- rack-protection/Gemfile | 5 - rack-protection/lib/rack/protection.rb | 3 - .../lib/rack/protection/authenticity_token.rb | 1 + .../lib/rack/protection/encrypted_cookie.rb | 273 --------- .../lib/rack/protection/encryptor.rb | 62 -- rack-protection/rack-protection.gemspec | 1 - .../protection/authenticity_token_spec.rb | 2 +- .../rack/protection/encrypted_cookie_spec.rb | 562 ------------------ .../lib/rack/protection/encryptor_spec.rb | 52 -- sinatra.gemspec | 1 + test/integration_start_test.rb | 10 + 14 files changed, 37 insertions(+), 974 deletions(-) delete mode 100644 rack-protection/lib/rack/protection/encrypted_cookie.rb delete mode 100644 rack-protection/lib/rack/protection/encryptor.rb delete mode 100644 rack-protection/spec/lib/rack/protection/encrypted_cookie_spec.rb delete mode 100644 rack-protection/spec/lib/rack/protection/encryptor_spec.rb diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e1ab9a34ca..cfdd26c013 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,7 +11,7 @@ permissions: jobs: rack-protection: - name: rack-protection (${{ matrix.ruby }}, rack ${{ matrix.rack }}, rack-session ${{ matrix.rack_session }}) + name: rack-protection (${{ matrix.ruby }}, rack ${{ matrix.rack }}) runs-on: ubuntu-latest timeout-minutes: 5 strategy: @@ -19,8 +19,6 @@ jobs: matrix: rack: - stable - rack_session: - - stable ruby: - "2.6" - "2.7" @@ -32,10 +30,9 @@ jobs: - "truffleruby" include: # Rack - - { ruby: 3.2, rack: head, rack_session: stable } - - { ruby: 3.2, rack: stable, rack_session: head } - # https://github.com/sinatra/sinatra/issues/1988 - - { ruby: ruby-head, rack: stable, rack_session: stable, allow-failure: true } + - { ruby: 3.2, rack: head } + # Never fail our build due to problems with head + - { ruby: ruby-head, rack: stable, allow-failure: true } env: rack: ${{ matrix.rack }} steps: @@ -65,7 +62,7 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} webhook: ${{ secrets.DISCORD_WEBHOOK }} sinatra: - name: ${{ matrix.ruby }} (Rack ${{ matrix.rack }}, Puma ${{ matrix.puma }}, Tilt ${{ matrix.tilt }}) + name: ${{ matrix.ruby }} (Rack ${{ matrix.rack }}, Rack::Session ${{ matrix.rack_session }}, Puma ${{ matrix.puma }}, Tilt ${{ matrix.tilt }}) runs-on: ubuntu-latest timeout-minutes: 15 strategy: @@ -75,23 +72,28 @@ jobs: - stable rack: - stable + rack_session: + - stable tilt: - stable # Due to https://github.com/actions/runner/issues/849, we have to use quotes for '3.0' ruby: [2.6, 2.7, '3.0', 3.1, 3.2, 3.3, jruby, truffleruby] include: # Rack - - { ruby: 3.2, rack: head, puma: stable, tilt: stable, allow-failure: true } + - { ruby: 3.2, rack: head, puma: stable, tilt: stable, rack_session: stable, allow-failure: true } + # Rack::Session + - { ruby: 3.2, rack: stable, puma: stable, tilt: stable, rack_session: head, allow-failure: true } # Puma - - { ruby: 3.2, rack: stable, puma: head, tilt: stable, allow-failure: true } + - { ruby: 3.2, rack: stable, puma: head, tilt: stable, rack_session: stable, allow-failure: true } # Tilt - - { ruby: 3.2, rack: stable, puma: stable, tilt: head, allow-failure: true } + - { ruby: 3.2, rack: stable, puma: stable, tilt: head, rack_session: stable, allow-failure: true } # Never fail our build due to problems with head - - { ruby: ruby-head, rack: stable, puma: stable, tilt: stable, allow-failure: true } - - { ruby: jruby-head, rack: stable, puma: stable, tilt: stable, allow-failure: true } - - { ruby: truffleruby-head, rack: stable, puma: stable, tilt: stable, allow-failure: true } + - { ruby: ruby-head, rack: stable, puma: stable, tilt: stable, rack_session: stable, allow-failure: true } + - { ruby: jruby-head, rack: stable, puma: stable, tilt: stable, rack_session: stable, allow-failure: true } + - { ruby: truffleruby-head, rack: stable, puma: stable, tilt: stable, rack_session: stable, allow-failure: true } env: rack: ${{ matrix.rack }} + rack_session: ${{ matrix.rack_session }} puma: ${{ matrix.puma }} tilt: ${{ matrix.tilt }} diff --git a/Gemfile b/Gemfile index 6d56a2c8b9..95da9acf5b 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,12 @@ rack_version = ENV['rack'].to_s rack_version = nil if rack_version.empty? || (rack_version == 'stable') rack_version = { github: 'rack/rack' } if rack_version == 'head' gem 'rack', rack_version + +rack_session_version = ENV['rack_session'].to_s +rack_session_version = nil if rack_session_version.empty? || (rack_session_version == 'stable') +rack_session_version = { github: 'rack/rack-session' } if rack_session_version == 'head' +gem 'rack-session', rack_session_version + gem 'rackup' puma_version = ENV['puma'].to_s diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index 929df4db22..5fd259a0ac 100644 --- a/lib/sinatra/base.rb +++ b/lib/sinatra/base.rb @@ -8,6 +8,7 @@ end require 'tilt' require 'rack/protection' +require 'rack/session' require 'mustermann' require 'mustermann/sinatra' require 'mustermann/regular' @@ -1929,7 +1930,7 @@ def force_encoding(*args) set :dump_errors, proc { !test? } set :show_exceptions, proc { development? } set :sessions, false - set :session_store, Rack::Protection::EncryptedCookie + set :session_store, Rack::Session::Cookie set :logging, false set :protection, true set :method_override, false diff --git a/rack-protection/Gemfile b/rack-protection/Gemfile index c5f4b42f85..58814f4363 100644 --- a/rack-protection/Gemfile +++ b/rack-protection/Gemfile @@ -11,8 +11,3 @@ rack_version = ENV['rack'].to_s rack_version = nil if rack_version.empty? || (rack_version == 'stable') rack_version = { github: 'rack/rack' } if rack_version == 'head' gem 'rack', rack_version - -rack_session_version = ENV['rack_session'].to_s -rack_session_version = nil if rack_session_version.empty? || (rack_session_version == 'stable') -rack_session_version = { github: 'rack/rack-session' } if rack_session_version == 'head' -gem 'rack-session', rack_session_version diff --git a/rack-protection/lib/rack/protection.rb b/rack-protection/lib/rack/protection.rb index 3ddf3d74f5..a5ae5d0b93 100644 --- a/rack-protection/lib/rack/protection.rb +++ b/rack-protection/lib/rack/protection.rb @@ -2,7 +2,6 @@ require 'rack/protection/version' require 'rack' -require 'rack/session' module Rack module Protection @@ -10,8 +9,6 @@ module Protection autoload :Base, 'rack/protection/base' autoload :CookieTossing, 'rack/protection/cookie_tossing' autoload :ContentSecurityPolicy, 'rack/protection/content_security_policy' - autoload :Encryptor, 'rack/protection/encryptor' - autoload :EncryptedCookie, 'rack/protection/encrypted_cookie' autoload :EscapedParams, 'rack/protection/escaped_params' autoload :FormToken, 'rack/protection/form_token' autoload :FrameOptions, 'rack/protection/frame_options' diff --git a/rack-protection/lib/rack/protection/authenticity_token.rb b/rack-protection/lib/rack/protection/authenticity_token.rb index f5c8db62d4..ffc6126651 100644 --- a/rack-protection/lib/rack/protection/authenticity_token.rb +++ b/rack-protection/lib/rack/protection/authenticity_token.rb @@ -51,6 +51,7 @@ module Protection # Here is server.rb: # # require 'rack/protection' + # require 'rack/session' # # app = Rack::Builder.app do # use Rack::Session::Cookie, secret: 'secret' diff --git a/rack-protection/lib/rack/protection/encrypted_cookie.rb b/rack-protection/lib/rack/protection/encrypted_cookie.rb deleted file mode 100644 index 006d496a75..0000000000 --- a/rack-protection/lib/rack/protection/encrypted_cookie.rb +++ /dev/null @@ -1,273 +0,0 @@ -# frozen_string_literal: true - -require 'openssl' -require 'zlib' -require 'json' -require 'rack/request' -require 'rack/response' -require 'rack/session/abstract/id' - -module Rack - module Protection - # Rack::Protection::EncryptedCookie provides simple cookie based session management. - # By default, the session is a Ruby Hash stored as base64 encoded marshalled - # data set to :key (default: rack.session). The object that encodes the - # session data is configurable and must respond to +encode+ and +decode+. - # Both methods must take a string and return a string. - # - # When the secret key is set, cookie data is checked for data integrity. - # The old_secret key is also accepted and allows graceful secret rotation. - # A legacy_hmac_secret is also accepted and is used to upgrade existing - # sessions to the new encryption scheme. - # - # There is also a legacy_hmac_coder option which can be set if a non-default - # coder was used for legacy session cookies. - # - # Example: - # - # use Rack::Protection::EncryptedCookie, - # :key => 'rack.session', - # :domain => 'foo.com', - # :path => '/', - # :expire_after => 2592000, - # :secret => 'change_me', - # :old_secret => 'old_secret' - # - # All parameters are optional. - # - # Example using legacy HMAC options - # - # Rack::Protection:EncryptedCookie.new(application, { - # # The secret used for legacy HMAC cookies - # legacy_hmac_secret: 'legacy secret', - # # legacy_hmac_coder will default to Rack::Protection::EncryptedCookie::Base64::Marshal - # legacy_hmac_coder: Rack::Protection::EncryptedCookie::Identity.new, - # # legacy_hmac will default to OpenSSL::Digest::SHA1 - # legacy_hmac: OpenSSL::Digest::SHA256 - # }) - # - # Example of a cookie with no encoding: - # - # Rack::Protection::EncryptedCookie.new(application, { - # :coder => Rack::Protection::EncryptedCookie::Identity.new - # }) - # - # Example of a cookie with custom encoding: - # - # Rack::Protection::EncryptedCookie.new(application, { - # :coder => Class.new { - # def encode(str); str.reverse; end - # def decode(str); str.reverse; end - # }.new - # }) - # - class EncryptedCookie < Rack::Session::Abstract::Persisted - # Encode session cookies as Base64 - class Base64 - def encode(str) - [str].pack('m0') - end - - def decode(str) - str.unpack1('m') - end - - # Encode session cookies as Marshaled Base64 data - class Marshal < Base64 - def encode(str) - super(::Marshal.dump(str)) - end - - def decode(str) - return unless str - - begin - ::Marshal.load(super(str)) - rescue StandardError - nil - end - end - end - - # N.B. Unlike other encoding methods, the contained objects must be a - # valid JSON composite type, either a Hash or an Array. - class JSON < Base64 - def encode(obj) - super(::JSON.dump(obj)) - end - - def decode(str) - return unless str - - begin - ::JSON.parse(super(str)) - rescue StandardError - nil - end - end - end - - class ZipJSON < Base64 - def encode(obj) - super(Zlib::Deflate.deflate(::JSON.dump(obj))) - end - - def decode(str) - return unless str - - ::JSON.parse(Zlib::Inflate.inflate(super(str))) - rescue StandardError - nil - end - end - end - - # Use no encoding for session cookies - class Identity - def encode(str); str; end - def decode(str); str; end - end - - class Marshal - def encode(str) - ::Marshal.dump(str) - end - - def decode(str) - ::Marshal.load(str) if str - end - end - - attr_reader :coder - - def initialize(app, options = {}) - # Assume keys are hex strings and convert them to raw byte strings for - # actual key material - @secrets = options.values_at(:secret, :old_secret).compact.map do |secret| - [secret].pack('H*') - end - - warn <<-MSG unless secure?(options) - SECURITY WARNING: No secret option provided to Rack::Protection::EncryptedCookie. - This poses a security threat. It is strongly recommended that you - provide a secret to prevent exploits that may be possible from crafted - cookies. This will not be supported in future versions of Rack, and - future versions will even invalidate your existing user cookies. - - Called from: #{caller[0]}. - MSG - - warn <<-MSG if @secrets.first && @secrets.first.length < 32 - SECURITY WARNING: Your secret is not long enough. It must be at least - 32 bytes long and securely random. To generate such a key for use - you can run the following command: - - ruby -rsecurerandom -e 'p SecureRandom.hex(32)' - - Called from: #{caller[0]}. - MSG - - if options.key?(:legacy_hmac_secret) - @legacy_hmac = options.fetch(:legacy_hmac, OpenSSL::Digest::SHA1) - - # Multiply the :digest_length: by 2 because this value is the length of - # the digest in bytes but session digest strings are encoded as hex - # strings - @legacy_hmac_length = @legacy_hmac.new.digest_length * 2 - @legacy_hmac_secret = options[:legacy_hmac_secret] - @legacy_hmac_coder = (options[:legacy_hmac_coder] ||= Base64::Marshal.new) - else - @legacy_hmac = false - end - - # If encryption is used we can just use a default Marshal encoder - # without Base64 encoding the results. - # - # If no encryption is used, rely on the previous default (Base64::Marshal) - @coder = (options[:coder] ||= (@secrets.any? ? Marshal.new : Base64::Marshal.new)) - - super(app, options.merge!(cookie_only: true)) - end - - private - - def find_session(req, _sid) - data = unpacked_cookie_data(req) - data = persistent_session_id!(data) - [data['session_id'], data] - end - - def extract_session_id(request) - unpacked_cookie_data(request)['session_id'] - end - - def unpacked_cookie_data(request) - request.fetch_header(Session::RACK_SESSION_UNPACKED_COOKIE_DATA) do |k| - session_data = cookie_data = request.cookies[@key] - - # Try to decrypt with the first secret, if that returns nil, try - # with old_secret - unless @secrets.empty? - session_data = Rack::Protection::Encryptor.decrypt_message(cookie_data, @secrets.first) - session_data ||= Rack::Protection::Encryptor.decrypt_message(cookie_data, @secrets[1]) if @secrets.size > 1 - end - - # If session_data is still nil, are there is a legacy HMAC - # configured, try verify and parse the cookie that way - if !session_data && @legacy_hmac - digest = cookie_data.slice!(-@legacy_hmac_length..-1) - cookie_data.slice!(-2..-1) # remove double dash - session_data = cookie_data if digest_match?(cookie_data, digest) - - # Decode using legacy HMAC decoder - request.set_header(k, @legacy_hmac_coder.decode(session_data) || {}) - else - request.set_header(k, coder.decode(session_data) || {}) - end - end - end - - def persistent_session_id!(data, sid = nil) - data ||= {} - data['session_id'] ||= sid || generate_sid - data - end - - def write_session(req, session_id, session, _options) - session = session.merge('session_id' => session_id) - session_data = coder.encode(session) - - unless @secrets.empty? - session_data = Rack::Protection::Encryptor.encrypt_message(session_data, @secrets.first) - end - - if session_data.size > (4096 - @key.size) - req.get_header(RACK_ERRORS).puts('Warning! Rack::Protection::EncryptedCookie data size exceeds 4K.') - nil - else - session_data - end - end - - def delete_session(_req, _session_id, options) - # Nothing to do here, data is in the client - generate_sid unless options[:drop] - end - - def digest_match?(data, digest) - return false unless data && digest - - Rack::Utils.secure_compare(digest, generate_hmac(data)) - end - - def generate_hmac(data) - OpenSSL::HMAC.hexdigest(@legacy_hmac.new, @legacy_hmac_secret, data) - end - - def secure?(options) - @secrets.size >= 1 || - (options[:coder] && options[:let_coder_handle_secure_encoding]) - end - end - end -end diff --git a/rack-protection/lib/rack/protection/encryptor.rb b/rack-protection/lib/rack/protection/encryptor.rb deleted file mode 100644 index ded10c7e8e..0000000000 --- a/rack-protection/lib/rack/protection/encryptor.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -require 'openssl' - -module Rack - module Protection - module Encryptor - CIPHER = 'aes-256-gcm' - DELIMITER = '--' - - def self.base64_encode(str) - [str].pack('m0') - end - - def self.base64_decode(str) - str.unpack1('m0') - end - - def self.encrypt_message(data, secret, auth_data = '') - raise ArgumentError, 'data cannot be nil' if data.nil? - - cipher = OpenSSL::Cipher.new(CIPHER) - cipher.encrypt - cipher.key = secret[0, cipher.key_len] - - # Rely on OpenSSL for the initialization vector - iv = cipher.random_iv - - # This must be set to properly use AES GCM for the OpenSSL module - cipher.auth_data = auth_data - - cipher_text = cipher.update(data) - cipher_text << cipher.final - - "#{base64_encode cipher_text}#{DELIMITER}#{base64_encode iv}#{DELIMITER}#{base64_encode cipher.auth_tag}" - end - - def self.decrypt_message(data, secret) - return unless data - - cipher = OpenSSL::Cipher.new(CIPHER) - cipher_text, iv, auth_tag = data.split(DELIMITER, 3).map! { |v| base64_decode(v) } - - # This check is from ActiveSupport::MessageEncryptor - # see: https://github.com/ruby/openssl/issues/63 - return if auth_tag.nil? || auth_tag.bytes.length != 16 - - cipher.decrypt - cipher.key = secret[0, cipher.key_len] - cipher.iv = iv - cipher.auth_tag = auth_tag - cipher.auth_data = '' - - decrypted_data = cipher.update(cipher_text) - decrypted_data << cipher.final - decrypted_data - rescue OpenSSL::Cipher::CipherError, TypeError, ArgumentError - nil - end - end - end -end diff --git a/rack-protection/rack-protection.gemspec b/rack-protection/rack-protection.gemspec index bc1899ffd7..b95d118573 100644 --- a/rack-protection/rack-protection.gemspec +++ b/rack-protection/rack-protection.gemspec @@ -41,5 +41,4 @@ RubyGems 2.0 or newer is required to protect against public gem pushes. You can # dependencies s.add_dependency 'base64', '>= 0.1.0' s.add_dependency 'rack', '>= 3.0.0', '< 4' - s.add_dependency 'rack-session', '>= 2.0.0', '< 3' end diff --git a/rack-protection/spec/lib/rack/protection/authenticity_token_spec.rb b/rack-protection/spec/lib/rack/protection/authenticity_token_spec.rb index 07b448597b..af8fa95b51 100644 --- a/rack-protection/spec/lib/rack/protection/authenticity_token_spec.rb +++ b/rack-protection/spec/lib/rack/protection/authenticity_token_spec.rb @@ -75,7 +75,7 @@ it 'allows for a custom token session key' do mock_app do - use Rack::Session::Cookie, key: 'rack.session', secret: SecureRandom.hex(32) + use(Rack::Config) { |e| e['rack.session'] ||= {} } use Rack::Protection::AuthenticityToken, key: :_csrf run DummyApp end diff --git a/rack-protection/spec/lib/rack/protection/encrypted_cookie_spec.rb b/rack-protection/spec/lib/rack/protection/encrypted_cookie_spec.rb deleted file mode 100644 index de2a659bc1..0000000000 --- a/rack-protection/spec/lib/rack/protection/encrypted_cookie_spec.rb +++ /dev/null @@ -1,562 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Rack::Protection::EncryptedCookie do - let(:incrementor) do - lambda do |env| - env['rack.session']['counter'] ||= 0 - env['rack.session']['counter'] += 1 - hash = env['rack.session'].dup - hash.delete('session_id') - Rack::Response.new(hash.inspect).to_a - end - end - - let(:session_id) do - lambda do |env| - Rack::Response.new(env['rack.session'].to_hash.inspect).to_a - end - end - - let(:session_option) do - lambda do |opt| - lambda do |env| - Rack::Response.new(env['rack.session.options'][opt].inspect).to_a - end - end - end - - let(:nothing) do - lambda do |_env| - Rack::Response.new('Nothing').to_a - end - end - - let(:renewer) do - lambda do |env| - env['rack.session.options'][:renew] = true - Rack::Response.new('Nothing').to_a - end - end - - let(:only_session_id) do - lambda do |env| - Rack::Response.new(env['rack.session']['session_id'].to_s).to_a - end - end - - let(:bigcookie) do - lambda do |env| - env['rack.session']['cookie'] = 'big' * 3000 - Rack::Response.new(env['rack.session'].inspect).to_a - end - end - - let(:destroy_session) do - lambda do |env| - env['rack.session'].destroy - Rack::Response.new('Nothing').to_a - end - end - - def response_for(options = {}) - request_options = options.fetch(:request, {}) - cookie = if options[:cookie].is_a?(Rack::Response) - options[:cookie]['Set-Cookie'] - else - options[:cookie] - end - request_options['HTTP_COOKIE'] = cookie || '' - - app_with_cookie = Rack::Protection::EncryptedCookie.new(*options[:app]) - app_with_cookie = Rack::Lint.new(app_with_cookie) - Rack::MockRequest.new(app_with_cookie).get('/', request_options) - end - - def random_cipher_secret - OpenSSL::Cipher.new('aes-256-gcm').random_key.unpack1('H*') - end - - let(:secret) { random_cipher_secret } - let(:warnings) { [] } - - before do - local_warnings = warnings - - Rack::Protection::EncryptedCookie.class_eval do - define_method(:warn) { |m| local_warnings << m } - end - end - - after do - Rack::Protection::EncryptedCookie.class_eval { remove_method :warn } - end - - describe 'Base64' do - it 'uses base64 to encode' do - coder = Rack::Protection::EncryptedCookie::Base64.new - str = 'fuuuuu' - expect(coder.encode(str)).to eq([str].pack('m0')) - end - - it 'uses base64 to decode' do - coder = Rack::Protection::EncryptedCookie::Base64.new - str = ['fuuuuu'].pack('m0') - expect(coder.decode(str)).to eq(str.unpack1('m0')) - end - - it 'handles non-strict base64 encoding' do - coder = Rack::Protection::EncryptedCookie::Base64.new - str = ['A' * 256].pack('m') - expect(coder.decode(str)).to eq('A' * 256) - end - - describe 'Marshal' do - it 'marshals and base64 encodes' do - coder = Rack::Protection::EncryptedCookie::Base64::Marshal.new - str = 'fuuuuu' - expect(coder.encode(str)).to eq([::Marshal.dump(str)].pack('m0')) - end - - it 'marshals and base64 decodes' do - coder = Rack::Protection::EncryptedCookie::Base64::Marshal.new - str = [::Marshal.dump('fuuuuu')].pack('m0') - expect(coder.decode(str)).to eq(::Marshal.load(str.unpack1('m0'))) - end - - it 'rescues failures on decode' do - coder = Rack::Protection::EncryptedCookie::Base64::Marshal.new - expect(coder.decode('lulz')).to be_nil - end - end - - describe 'JSON' do - it 'JSON and base64 encodes' do - coder = Rack::Protection::EncryptedCookie::Base64::JSON.new - obj = %w[fuuuuu] - expect(coder.encode(obj)).to eq([::JSON.dump(obj)].pack('m0')) - end - - it 'JSON and base64 decodes' do - coder = Rack::Protection::EncryptedCookie::Base64::JSON.new - str = [::JSON.dump(%w[fuuuuu])].pack('m0') - expect(coder.decode(str)).to eq(::JSON.parse(str.unpack1('m0'))) - end - - it 'rescues failures on decode' do - coder = Rack::Protection::EncryptedCookie::Base64::JSON.new - expect(coder.decode('lulz')).to be_nil - end - end - - describe 'ZipJSON' do - it 'jsons, deflates, and base64 encodes' do - coder = Rack::Protection::EncryptedCookie::Base64::ZipJSON.new - obj = %w[fuuuuu] - json = JSON.dump(obj) - expect(coder.encode(obj)).to eq([Zlib::Deflate.deflate(json)].pack('m0')) - end - - it 'base64 decodes, inflates, and decodes json' do - coder = Rack::Protection::EncryptedCookie::Base64::ZipJSON.new - obj = %w[fuuuuu] - json = JSON.dump(obj) - b64 = [Zlib::Deflate.deflate(json)].pack('m0') - expect(coder.decode(b64)).to eq(obj) - end - - it 'rescues failures on decode' do - coder = Rack::Protection::EncryptedCookie::Base64::ZipJSON.new - expect(coder.decode('lulz')).to be_nil - end - end - end - - it 'warns if no secret is given' do - Rack::Protection::EncryptedCookie.new(incrementor) - expect(warnings.first).to match(/no secret/i) - warnings.clear - Rack::Protection::EncryptedCookie.new(incrementor, secret: secret) - expect(warnings).to be_empty - end - - it 'warns if secret is to short' do - Rack::Protection::EncryptedCookie.new(incrementor, secret: secret[0, 16]) - expect(warnings.first).to match(/secret is not long enough/i) - warnings.clear - Rack::Protection::EncryptedCookie.new(incrementor, secret: secret) - expect(warnings).to be_empty - end - - it "doesn't warn if coder is configured to handle encoding" do - Rack::Protection::EncryptedCookie.new( - incrementor, coder: Object.new, let_coder_handle_secure_encoding: true - ) - expect(warnings).to be_empty - end - - it 'still warns if coder is not set' do - Rack::Protection::EncryptedCookie.new( - incrementor, - let_coder_handle_secure_encoding: true - ) - expect(warnings.first).to match(/no secret/i) - end - - it 'uses a coder' do - identity = Class.new do - attr_reader :calls - - def initialize - @calls = [] - end - - def encode(str) - @calls << :encode - str - end - - def decode(str) - @calls << :decode - str - end - end.new - response = response_for(app: [incrementor, { coder: identity }]) - - expect(response['Set-Cookie']).to include('rack.session=') - expect(response.body).to eq('{"counter"=>1}') - expect(identity.calls).to eq(%i[decode encode]) - end - - it 'creates a new cookie' do - response = response_for(app: incrementor) - expect(response['Set-Cookie']).to include('rack.session=') - expect(response.body).to eq('{"counter"=>1}') - end - - it 'loads from a cookie' do - response = response_for(app: incrementor) - - response = response_for(app: incrementor, cookie: response) - expect(response.body).to eq('{"counter"=>2}') - - response = response_for(app: incrementor, cookie: response) - expect(response.body).to eq('{"counter"=>3}') - end - - it 'renew session id' do - response = response_for(app: incrementor) - cookie = response['Set-Cookie'] - response = response_for(app: only_session_id, cookie: cookie) - cookie = response['Set-Cookie'] if response['Set-Cookie'] - - expect(response.body).to_not eq('') - old_session_id = response.body - - response = response_for(app: renewer, cookie: cookie) - cookie = response['Set-Cookie'] if response['Set-Cookie'] - response = response_for(app: only_session_id, cookie: cookie) - - expect(response.body).to_not eq('') - expect(response.body).to_not eq(old_session_id) - end - - it 'destroys session' do - response = response_for(app: incrementor) - response = response_for(app: only_session_id, cookie: response) - - expect(response.body).to_not eq('') - old_session_id = response.body - - response = response_for(app: destroy_session, cookie: response) - response = response_for(app: only_session_id, cookie: response) - - expect(response.body).to_not eq('') - expect(response.body).to_not eq(old_session_id) - end - - it 'survives broken cookies' do - response = response_for( - app: incrementor, - cookie: 'rack.session=blarghfasel' - ) - expect(response.body).to eq('{"counter"=>1}') - - response = response_for( - app: [incrementor, { secret: secret }], - cookie: 'rack.session=' - ) - expect(response.body).to eq('{"counter"=>1}') - end - - it 'barks on too big cookies' do - expect do - response_for(app: bigcookie, request: { fatal: true }) - end.to raise_error Rack::MockRequest::FatalWarning - end - - it 'loads from a cookie with integrity hash' do - app = [incrementor, { secret: secret }] - - response = response_for(app: app) - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>2}') - - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>3}') - - app = [incrementor, { secret: random_cipher_secret }] - - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>1}') - end - - it 'loads from a cookie with accept-only integrity hash for graceful key rotation' do - response = response_for(app: [incrementor, { secret: secret }]) - - new_secret = random_cipher_secret - - app = [incrementor, { secret: new_secret, old_secret: secret }] - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>2}') - - newer_secret = random_cipher_secret - - app = [incrementor, { secret: newer_secret, old_secret: new_secret }] - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>3}') - end - - it 'loads from a legacy hmac cookie' do - legacy_session = Rack::Protection::EncryptedCookie::Base64::Marshal.new.encode({ 'counter' => 1, 'session_id' => 'abcdef' }) - legacy_secret = 'test legacy secret' - legacy_digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('SHA1'), legacy_secret, legacy_session) - - legacy_cookie = "rack.session=#{legacy_session}--#{legacy_digest}; path=/; HttpOnly" - - app = [incrementor, { secret: secret, legacy_hmac_secret: legacy_secret }] - response = response_for(app: app, cookie: legacy_cookie) - expect(response.body).to eq('{"counter"=>2}') - end - - it 'ignores tampered with session cookies' do - app = [incrementor, { secret: secret }] - response = response_for(app: app) - expect(response.body).to eq('{"counter"=>1}') - - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>2}') - - ctxt, iv, auth_tag = response['Set-Cookie'].split('--', 3) - tampered_with_cookie = [ctxt, iv, auth_tag.reverse].join('--') - - response = response_for(app: app, cookie: tampered_with_cookie) - expect(response.body).to eq('{"counter"=>1}') - end - - it 'ignores tampered with legacy hmac cookie' do - legacy_session = Rack::Protection::EncryptedCookie::Base64::Marshal.new.encode({ 'counter' => 1, 'session_id' => 'abcdef' }) - legacy_secret = 'test legacy secret' - legacy_digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('SHA1'), legacy_secret, legacy_session).reverse - - legacy_cookie = "rack.session=#{legacy_session}--#{legacy_digest}; path=/; HttpOnly" - - app = [incrementor, { secret: secret, legacy_hmac_secret: legacy_secret }] - response = response_for(app: app, cookie: legacy_cookie) - expect(response.body).to eq('{"counter"=>1}') - end - - it 'supports either of secret or old_secret' do - app = [incrementor, { secret: secret }] - response = response_for(app: app) - expect(response.body).to eq('{"counter"=>1}') - - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>2}') - - app = [incrementor, { old_secret: secret }] - response = response_for(app: app) - expect(response.body).to eq('{"counter"=>1}') - - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>2}') - end - - it 'supports custom digest class for legacy hmac cookie' do - legacy_hmac = OpenSSL::Digest::SHA256 - legacy_session = Rack::Protection::EncryptedCookie::Base64::Marshal.new.encode({ 'counter' => 1, 'session_id' => 'abcdef' }) - legacy_secret = 'test legacy secret' - legacy_digest = OpenSSL::HMAC.hexdigest(legacy_hmac.new, legacy_secret, legacy_session) - legacy_cookie = "rack.session=#{Rack::Utils.escape legacy_session}--#{legacy_digest}; path=/; HttpOnly" - - app = [incrementor, { - secret: secret, legacy_hmac_secret: legacy_secret, legacy_hmac: legacy_hmac - }] - - response = response_for(app: app, cookie: legacy_cookie) - expect(response.body).to eq('{"counter"=>2}') - - response = response_for(app: app, cookie: response) - expect(response.body).to eq('{"counter"=>3}') - end - - it 'can handle Rack::Lint middleware' do - response = response_for(app: incrementor) - - lint = Rack::Lint.new(session_id) - response = response_for(app: lint, cookie: response) - expect(response.body).to_not be_nil - end - - it 'can handle middleware that inspects the env' do - class TestEnvInspector - def initialize(app) - @app = app - end - - def call(env) - env.inspect - @app.call(env) - end - end - - response = response_for(app: incrementor) - - inspector = TestEnvInspector.new(session_id) - response = response_for(app: inspector, cookie: response) - expect(response.body).to_not be_nil - end - - it 'returns the session id in the session hash' do - response = response_for(app: incrementor) - expect(response.body).to eq('{"counter"=>1}') - - response = response_for(app: session_id, cookie: response) - expect(response.body).to match(/"session_id"=>/) - expect(response.body).to match(/"counter"=>1/) - end - - it 'does not return a cookie if set to secure but not using ssl' do - app = [incrementor, { secure: true }] - - response = response_for(app: app) - expect(response['Set-Cookie']).to be_nil - - response = response_for(app: app, request: { 'HTTPS' => 'on' }) - expect(response['Set-Cookie']).to_not be_nil - expect(response['Set-Cookie']).to match(/secure/) - end - - it 'does not return a cookie if cookie was not read/written' do - response = response_for(app: nothing) - expect(response['Set-Cookie']).to be_nil - end - - it 'does not return a cookie if cookie was not written (only read)' do - response = response_for(app: session_id) - expect(response['Set-Cookie']).to be_nil - end - - it 'returns even if not read/written if :expire_after is set' do - app = [nothing, { expire_after: 3600 }] - request = { 'rack.session' => { 'not' => 'empty' } } - response = response_for(app: app, request: request) - expect(response['Set-Cookie']).to_not be_nil - end - - it 'returns no cookie if no data was written and no session was created previously, even if :expire_after is set' do - app = [nothing, { expire_after: 3600 }] - response = response_for(app: app) - expect(response['Set-Cookie']).to be_nil - end - - it "exposes :secret in env['rack.session.option']" do - response = response_for(app: [session_option[:secret], { secret: secret }]) - expect(response.body).to eq(secret.inspect) - end - - it "exposes :coder in env['rack.session.option']" do - response = response_for(app: session_option[:coder]) - expect(response.body).to match(/Base64::Marshal/) - end - - it 'exposes correct :coder when a secret is used' do - response = response_for(app: session_option[:coder], secret: secret) - expect(response.body).to match(/Marshal/) - end - - it 'allows passing in a hash with session data from middleware in front' do - request = { 'rack.session' => { foo: 'bar' } } - response = response_for(app: session_id, request: request) - expect(response.body).to match(/foo/) - end - - it 'allows modifying session data with session data from middleware in front' do - request = { 'rack.session' => { foo: 'bar' } } - response = response_for(app: incrementor, request: request) - expect(response.body).to match(/counter/) - expect(response.body).to match(/foo/) - end - - it "allows more than one '--' in the cookie when calculating legacy digests" do - @counter = 0 - app = lambda do |env| - env['rack.session']['message'] ||= '' - env['rack.session']['message'] << "#{@counter += 1}--" - hash = env['rack.session'].dup - hash.delete('session_id') - Rack::Response.new(hash['message']).to_a - end - # another example of an unsafe coder is Base64.urlsafe_encode64 - unsafe_coder = Class.new do - def encode(hash); hash.inspect end - def decode(str); eval(str) if str; end - end.new - - legacy_session = unsafe_coder.encode('message' => "#{@counter += 1}--#{@counter += 1}--", 'session_id' => 'abcdef') - legacy_secret = 'test legacy secret' - legacy_digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('SHA1'), legacy_secret, legacy_session) - legacy_cookie = "rack.session=#{Rack::Utils.escape legacy_session}--#{legacy_digest}; path=/; HttpOnly" - - _app = [app, { - secret: secret, legacy_hmac_secret: legacy_secret, - legacy_hmac_coder: unsafe_coder - }] - - response = response_for(app: _app, cookie: legacy_cookie) - expect(response.body).to eq('1--2--3--') - end - - it 'allows for non-strict encoded cookie' do - long_session_app = lambda do |env| - env['rack.session']['value'] = 'A' * 256 - env['rack.session']['counter'] = 1 - hash = env['rack.session'].dup - hash.delete('session_id') - Rack::Response.new(hash.inspect).to_a - end - - non_strict_coder = Class.new do - def encode(str) - [Marshal.dump(str)].pack('m') - end - - def decode(str) - return unless str - - Marshal.load(str.unpack1('m')) - end - end.new - - non_strict_response = response_for(app: [ - long_session_app, { coder: non_strict_coder } - ]) - - response = response_for(app: [ - incrementor - ], cookie: non_strict_response) - - expect(response.body).to match(%("value"=>"#{'A' * 256}")) - expect(response.body).to match('"counter"=>2') - expect(response.body).to match(/\A{[^}]+}\z/) - end -end diff --git a/rack-protection/spec/lib/rack/protection/encryptor_spec.rb b/rack-protection/spec/lib/rack/protection/encryptor_spec.rb deleted file mode 100644 index cb7a2a401f..0000000000 --- a/rack-protection/spec/lib/rack/protection/encryptor_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe Rack::Protection::Encryptor do - let(:secret) do - OpenSSL::Cipher.new(Rack::Protection::Encryptor::CIPHER).random_key - end - - it 'encrypted message contains ciphertext iv and auth_tag' do - msg = Rack::Protection::Encryptor.encrypt_message('hello world', secret) - - ctxt, iv, auth_tag = msg.split(Rack::Protection::Encryptor::DELIMITER, 3) - - expect(ctxt).not_to be_empty - expect(iv).not_to be_empty - expect(auth_tag).not_to be_empty - end - - it 'encrypted message is decryptable' do - cmsg = Rack::Protection::Encryptor.encrypt_message('hello world', secret) - pmsg = Rack::Protection::Encryptor.decrypt_message(cmsg, secret) - - expect(pmsg).to eql('hello world') - end - - it 'encryptor and decryptor handles overly long keys' do - new_secret = "#{secret}abcdef123456" - - # These methos should truncate the long key (so OpenSSL raise exceptions) - cmsg = Rack::Protection::Encryptor.encrypt_message('hello world', new_secret) - pmsg = Rack::Protection::Encryptor.decrypt_message(cmsg, new_secret) - - expect(pmsg).to eq('hello world') - end - - it 'decrypt returns nil for junk messages' do - pmsg = Rack::Protection::Encryptor.decrypt_message('aaa--bbb-ccc', secret) - - expect(pmsg).to be_nil - end - - it 'decrypt returns nil for tampered messages' do - cmsg = Rack::Protection::Encryptor.encrypt_message('hello world', secret) - - csplit = cmsg.split(Rack::Protection::Encryptor::DELIMITER, 3) - csplit[2] = csplit.last.reverse - - tampered_msg = csplit.join(Rack::Protection::Encryptor::DELIMITER) - pmsg = Rack::Protection::Encryptor.decrypt_message(tampered_msg, secret) - - expect(pmsg).to be_nil - end -end diff --git a/sinatra.gemspec b/sinatra.gemspec index 4b74db4c78..7df5871ad2 100644 --- a/sinatra.gemspec +++ b/sinatra.gemspec @@ -47,6 +47,7 @@ RubyGems 2.0 or newer is required to protect against public gem pushes. You can s.add_dependency 'mustermann', '~> 3.0' s.add_dependency 'rack', '>= 3.0.0', '< 4' + s.add_dependency 'rack-session', '>= 2.0.0', '< 3' s.add_dependency 'rack-protection', version s.add_dependency 'tilt', '~> 2.0' end diff --git a/test/integration_start_test.rb b/test/integration_start_test.rb index f5a3f310ce..8863edf697 100644 --- a/test/integration_start_test.rb +++ b/test/integration_start_test.rb @@ -4,7 +4,17 @@ class IntegrationStartTest < Minitest::Test include IntegrationStartHelper def test_app_start_without_rackup + # Why we skip head versions: The Gemfile used here would have to support + # the ENVs and we would need to bundle before starting the app + # + # Example from locally playing with this: + # + # root@df8b1e7cb106:/app# rack_session=head BUNDLE_GEMFILE=./test/integration/gemfile_without_rackup.rb ruby ./test/integration/simple_app.rb -p 0 -s puma + # The git source https://github.com/rack/rack-session.git is not yet checked out. Please run `bundle install` before trying to start your application + # + # Using bundler/inline is an idea, but it would add to the startup time skip "So much work to run with rack head branch" if ENV['rack'] == 'head' + skip "So much work to run with rack-session head branch" if ENV['rack_session'] == 'head' app_file = File.join(__dir__, "integration", "simple_app.rb") gem_file = File.join(__dir__, "integration", "gemfile_without_rackup.rb")