Merge pull request #737 from binarylogic/session_fixation

Defend against Session Fixation attack

Author: jaredbeck

CHANGELOG.md

@@ -10,7 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 * Breaking Changes
   * None
 * Added
-  * None
+  * `Authlogic::Session::Base.session_fixation_defense` - Reset the Rack
+    session ID after authentication, to protect against Session Fixation
+    attacks. (https://guides.rubyonrails.org/security.html#session-fixation)
+    Default: true
 * Fixed
   * None
 

lib/authlogic/controller_adapters/abstract_adapter.rb

@@ -8,6 +8,7 @@ module ControllerAdapters # :nodoc:
     class AbstractAdapter
       E_COOKIE_DOMAIN_ADAPTER = "The cookie_domain method has not been " \
         "implemented by the controller adapter"
+      ENV_SESSION_OPTIONS = "rack.session.options"
 
       attr_accessor :controller
 
@@ -44,6 +45,26 @@ def request_content_type
         request.content_type
       end
 
+      # Inform Rack that we would like a new session ID to be assigned. Changes
+      # the ID, but not the contents of the session.
+      #
+      # The `:renew` option is read by `rack/session/abstract/id.rb`.
+      #
+      # This is how Devise (via warden) implements defense against Session
+      # Fixation. Our implementation is copied directly from the warden gem
+      # (set_user in warden/proxy.rb)
+      def renew_session_id
+        env = request.env
+        options = env[ENV_SESSION_OPTIONS]
+        if options
+          if options.frozen?
+            env[ENV_SESSION_OPTIONS] = options.merge(renew: true).freeze
+          else
+            options[:renew] = true
+          end
+        end
+      end
+
       def session
         controller.session
       end

lib/authlogic/session/base.rb

@@ -424,6 +424,7 @@ def self.#{method}(*filter_list, &block)
       after_save :reset_perishable_token!
       after_save :save_cookie, if: :cookie_enabled?
       after_save :update_session
+      after_create :renew_session_id
 
       after_destroy :destroy_cookie, if: :cookie_enabled?
       after_destroy :update_session
@@ -976,6 +977,16 @@ def secure(value = nil)
         end
         alias secure= secure
 
+        # Should the Rack session ID be reset after authentication, to protect
+        # against Session Fixation attacks?
+        #
+        # * <tt>Default:</tt> true
+        # * <tt>Accepts:</tt> Boolean
+        def session_fixation_defense(value = nil)
+          rw_config(:session_fixation_defense, value, true)
+        end
+        alias session_fixation_defense= session_fixation_defense
+
         # Should the cookie be signed? If the controller adapter supports it, this is a
         # measure against cookie tampering.
         def sign_cookie(value = nil)
@@ -1681,6 +1692,13 @@ def configure_password_methods
         define_password_field_methods
       end
 
+      # Assign a new controller-session ID, to defend against Session Fixation.
+      # https://guides.rubyonrails.org/v6.0/security.html#session-fixation
+      def renew_session_id
+        return unless self.class.session_fixation_defense
+        controller.renew_session_id
+      end
+
       def define_login_field_methods
         return unless login_field
         self.class.send(:attr_writer, login_field) unless respond_to?("#{login_field}=")

lib/authlogic/test_case/mock_request.rb

@@ -9,6 +9,12 @@ def initialize(controller)
         self.controller = controller
       end
 
+      def env
+        @env ||= {
+          ControllerAdapters::AbstractAdapter::ENV_SESSION_OPTIONS => {}
+        }
+      end
+
       def format
         controller.request_content_type if controller.respond_to? :request_content_type
       end

test/session_test/password_test.rb

@@ -117,6 +117,7 @@ def test_save_with_credentials
         assert_equal 1, session.record.login_count
         assert Time.now >= session.record.current_login_at
         assert_equal "1.1.1.1", session.record.current_login_ip
+        assert_equal env_session_options[:renew], true
       end
     end
   end

test/session_test/session_test.rb

@@ -21,9 +21,12 @@ def test_persist_persist_by_session
         assert session = UserSession.find
         assert_equal ben, session.record
         assert_equal ben.persistence_token, controller.session["user_credentials"]
+        refute_includes env_session_options, :renew
       end
 
-      def test_persist_persist_by_session_with_session_fixation_attack
+      # A SQL injection attack to steal the persistence_token.
+      # TODO: Explain how `:select` is used, and sanitized.
+      def test_persist_persist_by_session_with_sql_injection_attack
         ben = users(:ben)
         controller.session["user_credentials"] = "neo"
         controller.session["user_credentials_id"] = {
@@ -33,7 +36,7 @@ def test_persist_persist_by_session_with_session_fixation_attack
         assert @user_session.blank?
       end
 
-      def test_persist_persist_by_session_with_sql_injection_attack
+      def test_persist_persist_by_session_with_sql_injection_attack_2
         controller.session["user_credentials"] = { select: "ABRA CADABRA" }
         controller.session["user_credentials_id"] = nil
         assert_nothing_raised do
@@ -49,6 +52,7 @@ def test_persist_persist_by_session_with_token_only
         session = UserSession.find
         assert_equal ben, session.record
         assert_equal ben.persistence_token, controller.session["user_credentials"]
+        refute_includes env_session_options, :renew
       end
 
       def test_after_save_update_session
@@ -57,6 +61,7 @@ def test_after_save_update_session
         assert controller.session["user_credentials"].blank?
         assert session.save
         assert_equal ben.persistence_token, controller.session["user_credentials"]
+        assert_equal env_session_options[:renew], true
       end
 
       def test_after_destroy_update_session
@@ -66,6 +71,7 @@ def test_after_destroy_update_session
         assert session = UserSession.find
         assert session.destroy
         assert controller.session["user_credentials"].blank?
+        refute_includes env_session_options, :renew
       end
 
       def test_after_persisting_update_session
@@ -74,6 +80,7 @@ def test_after_persisting_update_session
         assert controller.session["user_credentials"].blank?
         assert UserSession.find
         assert_equal ben.persistence_token, controller.session["user_credentials"]
+        refute_includes env_session_options, :renew
       end
     end
   end

test/test_helper.rb

@@ -238,6 +238,13 @@ def config_teardown
       end
     end
 
+    def env_session_options
+      controller.request.env.fetch(
+        ::Authlogic::ControllerAdapters::AbstractAdapter::ENV_SESSION_OPTIONS,
+        {}
+      ).with_indifferent_access
+    end
+
     def password_for(user)
       case user
       when users(:ben)