Refactor and clean up CSRF protection code

1. Remove unused methods
2. Remove unused and unnecessary features
3. Add basic tests

The session token is stored server-side and never given to the client.
A masked token is generated for each request which can be inserted into forms to be POSTed.

lib/sidekiq/web.rb

@@ -10,7 +10,7 @@ require "sidekiq/web/helpers"
 require "sidekiq/web/router"
 require "sidekiq/web/action"
 require "sidekiq/web/application"
-require "sidekiq/web/authenticity_token"
+require "sidekiq/web/csrf_protection"
 
 require "rack/content_length"
 
@@ -155,8 +155,8 @@ module Sidekiq
     def build_sessions
       middlewares = self.middlewares
 
-      unless using?(AuthenticityToken) || ENV["RACK_ENV"] == "test"
+      unless using?(CsrfProtection) || ENV["RACK_ENV"] == "test"
-        middlewares.unshift [[AuthenticityToken], nil]
+        middlewares.unshift [[CsrfProtection], nil]
       end
 
       s = sessions

lib/sidekiq/web/authenticity_token.rb

@@ -1,254 +0,0 @@
-# this file came from the sinatra/rack-protection project
-#
-# The MIT License (MIT)
-#
-# Copyright (c) 2011-2017 Konstantin Haase
-# Copyright (c) 2015-2017 Zachary Scott
-#
-# Permission is hereby granted, free of charge, to any person obtaining
-# a copy of this software and associated documentation files (the
-# 'Software'), to deal in the Software without restriction, including
-# without limitation the rights to use, copy, modify, merge, publish,
-# distribute, sublicense, and/or sell copies of the Software, and to
-# permit persons to whom the Software is furnished to do so, subject to
-# the following conditions:
-#
-# The above copyright notice and this permission notice shall be
-# included in all copies or substantial portions of the Software.
-#
-# THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND,
-# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
-# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
-# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
-# CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
-# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
-# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-
-require "securerandom"
-require "base64"
-require "rack/request"
-
-module Sidekiq
-  class Web
-    class AuthenticityToken
-      DEFAULT_OPTIONS = {
-        reaction: :default_reaction, logging: true,
-        message: "Forbidden", encryptor: Digest::SHA1,
-        session_key: "rack.session", status: 403,
-        allow_empty_referrer: true,
-        report_key: "protection.failed",
-        html_types: %w[text/html application/xhtml text/xml application/xml]
-      }
-
-      attr_reader :app, :options
-
-      def self.default_options(options)
-        define_method(:default_options) { DEFAULT_OPTIONS.merge(options) }
-      end
-
-      def self.default_reaction(reaction)
-        alias_method(:default_reaction, reaction)
-      end
-
-      def default_options
-        DEFAULT_OPTIONS
-      end
-
-      def initialize(app, options = {})
-        @app, @options = app, default_options.merge(options)
-      end
-
-      def safe?(env)
-        %w[GET HEAD OPTIONS TRACE].include? env["REQUEST_METHOD"]
-      end
-
-      def call(env)
-        unless accepts? env
-          instrument env
-          result = react env
-        end
-        result || app.call(env)
-      end
-
-      def react(env)
-        result = send(options[:reaction], env)
-        result if (Array === result) && (result.size == 3)
-      end
-
-      def warn(env, message)
-        return unless options[:logging]
-        l = options[:logger] || env["rack.logger"] || ::Logger.new(env["rack.errors"])
-        l.warn(message)
-      end
-
-      def instrument(env)
-        return unless (i = options[:instrumenter])
-        env["rack.protection.attack"] = self.class.name.split("::").last.downcase
-        i.instrument("rack.protection", env)
-      end
-
-      def deny(env)
-        warn env, "attack prevented by #{self.class}"
-        [options[:status], {"Content-Type" => "text/plain"}, [options[:message]]]
-      end
-
-      def report(env)
-        warn env, "attack reported by #{self.class}"
-        env[options[:report_key]] = true
-      end
-
-      def session?(env)
-        env.include? options[:session_key]
-      end
-
-      def session(env)
-        return env[options[:session_key]] if session? env
-        fail "you need to set up a session middleware *before* #{self.class}"
-      end
-
-      def drop_session(env)
-        session(env).clear if session? env
-      end
-
-      def referrer(env)
-        ref = env["HTTP_REFERER"].to_s
-        return if !options[:allow_empty_referrer] && ref.empty?
-        URI.parse(ref).host || Rack::Request.new(env).host
-      rescue URI::InvalidURIError
-      end
-
-      def origin(env)
-        env["HTTP_ORIGIN"] || env["HTTP_X_ORIGIN"]
-      end
-
-      def random_string(secure = defined? SecureRandom)
-        secure ? SecureRandom.hex(16) : "%032x" % rand(2**128 - 1)
-      rescue NotImplementedError
-        random_string false
-      end
-
-      def encrypt(value)
-        options[:encryptor].hexdigest value.to_s
-      end
-
-      def secure_compare(a, b)
-        Rack::Utils.secure_compare(a.to_s, b.to_s)
-      end
-
-      def html?(headers)
-        return false unless (header = headers.detect { |k, v| k.downcase == "content-type" })
-        options[:html_types].include? header.last[/^\w+\/\w+/]
-      end
-
-      TOKEN_LENGTH = 32
-
-      default_options authenticity_param: "authenticity_token",
-                      allow_if: nil
-
-      def self.token(session)
-        new(nil).mask_authenticity_token(session)
-      end
-
-      def self.random_token
-        SecureRandom.base64(TOKEN_LENGTH)
-      end
-
-      def accepts?(env)
-        session = session env
-        set_token(session)
-
-        safe?(env) ||
-          valid_token?(session, env["HTTP_X_CSRF_TOKEN"]) ||
-          valid_token?(session, Rack::Request.new(env).params[options[:authenticity_param]]) ||
-          options[:allow_if]&.call(env)
-      end
-
-      def mask_authenticity_token(session)
-        token = set_token(session)
-        mask_token(token)
-      end
-
-      private
-
-      def set_token(session)
-        session[:csrf] ||= self.class.random_token
-      end
-
-      # Checks the client's masked token to see if it matches the
-      # session token.
-      def valid_token?(session, token)
-        return false if token.nil? || token.empty?
-
-        begin
-          token = decode_token(token)
-        rescue ArgumentError # encoded_masked_token is invalid Base64
-          return false
-        end
-
-        # See if it's actually a masked token or not. We should be able
-        # to handle any unmasked tokens that we've issued without error.
-
-        if unmasked_token?(token)
-          compare_with_real_token token, session
-
-        elsif masked_token?(token)
-          token = unmask_token(token)
-
-          compare_with_real_token token, session
-
-        else
-          false # Token is malformed
-        end
-      end
-
-      # Creates a masked version of the authenticity token that varies
-      # on each request. The masking is used to mitigate SSL attacks
-      # like BREACH.
-      def mask_token(token)
-        token = decode_token(token)
-        one_time_pad = SecureRandom.random_bytes(token.length)
-        encrypted_token = xor_byte_strings(one_time_pad, token)
-        masked_token = one_time_pad + encrypted_token
-        encode_token(masked_token)
-      end
-
-      # Essentially the inverse of +mask_token+.
-      def unmask_token(masked_token)
-        # Split the token into the one-time pad and the encrypted
-        # value and decrypt it
-        token_length = masked_token.length / 2
-        one_time_pad = masked_token[0...token_length]
-        encrypted_token = masked_token[token_length..-1]
-        xor_byte_strings(one_time_pad, encrypted_token)
-      end
-
-      def unmasked_token?(token)
-        token.length == TOKEN_LENGTH
-      end
-
-      def masked_token?(token)
-        token.length == TOKEN_LENGTH * 2
-      end
-
-      def compare_with_real_token(token, session)
-        secure_compare(token, real_token(session))
-      end
-
-      def real_token(session)
-        decode_token(session[:csrf])
-      end
-
-      def encode_token(token)
-        Base64.strict_encode64(token)
-      end
-
-      def decode_token(token)
-        Base64.strict_decode64(token)
-      end
-
-      def xor_byte_strings(s1, s2)
-        s1.bytes.zip(s2.bytes).map { |(c1, c2)| c1 ^ c2 }.pack("c*")
-      end
-    end
-  end
-end

lib/sidekiq/web/csrf_protection.rb

@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+# this file originally based on authenticity_token.rb from the sinatra/rack-protection project
+#
+# The MIT License (MIT)
+#
+# Copyright (c) 2011-2017 Konstantin Haase
+# Copyright (c) 2015-2017 Zachary Scott
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# 'Software'), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+# CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+require "securerandom"
+require "base64"
+require "rack/request"
+
+module Sidekiq
+  class Web
+    class CsrfProtection
+      def initialize(app, options = nil)
+        @app = app
+      end
+
+      def call(env)
+        accept?(env) ? admit(env) : deny(env)
+      end
+
+      private
+
+      def admit(env)
+        # On each successful request, we create a fresh masked token
+        # which will be used in any forms rendered for this request.
+        s = session(env)
+        s[:csrf] ||= SecureRandom.base64(TOKEN_LENGTH)
+        env[:csrf_token] = mask_token(s[:csrf])
+        @app.call(env)
+      end
+
+      def safe?(env)
+        %w[GET HEAD OPTIONS TRACE].include? env["REQUEST_METHOD"]
+      end
+
+      def logger(env)
+        @logger ||= (env["rack.logger"] || ::Logger.new(env["rack.errors"]))
+      end
+
+      def deny(env)
+        logger(env).warn "attack prevented by #{self.class}"
+        [403, {"Content-Type" => "text/plain"}, ["Forbidden"]]
+      end
+
+      def session(env)
+        env["rack.session"] || fail("you need to set up a session middleware *before* #{self.class}")
+      end
+
+      def accept?(env)
+        return true if safe?(env)
+
+        giventoken = Rack::Request.new(env).params["authenticity_token"]
+        valid_token?(env, giventoken)
+      end
+
+      TOKEN_LENGTH = 32
+
+      # Checks that the token given to us as a parameter matches
+      # the token stored in the session.
+      def valid_token?(env, giventoken)
+        return false if giventoken.nil? || giventoken.empty?
+
+        begin
+          token = decode_token(giventoken)
+        rescue ArgumentError # client input is invalid
+          return false
+        end
+
+        sess = session(env)
+        localtoken = sess[:csrf]
+
+        # Rotate the session token after every use
+        sess[:csrf] = SecureRandom.base64(TOKEN_LENGTH)
+
+        # See if it's actually a masked token or not. We should be able
+        # to handle any unmasked tokens that we've issued without error.
+
+        if unmasked_token?(token)
+          compare_with_real_token token, localtoken
+        elsif masked_token?(token)
+          unmasked = unmask_token(token)
+          compare_with_real_token unmasked, localtoken
+        else
+          false # Token is malformed
+        end
+      end
+
+      # Creates a masked version of the authenticity token that varies
+      # on each request. The masking is used to mitigate SSL attacks
+      # like BREACH.
+      def mask_token(token)
+        token = decode_token(token)
+        one_time_pad = SecureRandom.random_bytes(token.length)
+        encrypted_token = xor_byte_strings(one_time_pad, token)
+        masked_token = one_time_pad + encrypted_token
+        Base64.strict_encode64(masked_token)
+      end
+
+      # Essentially the inverse of +mask_token+.
+      def unmask_token(masked_token)
+        # Split the token into the one-time pad and the encrypted
+        # value and decrypt it
+        token_length = masked_token.length / 2
+        one_time_pad = masked_token[0...token_length]
+        encrypted_token = masked_token[token_length..-1]
+        xor_byte_strings(one_time_pad, encrypted_token)
+      end
+
+      def unmasked_token?(token)
+        token.length == TOKEN_LENGTH
+      end
+
+      def masked_token?(token)
+        token.length == TOKEN_LENGTH * 2
+      end
+
+      def compare_with_real_token(token, local)
+        Rack::Utils.secure_compare(token.to_s, decode_token(local).to_s)
+      end
+
+      def decode_token(token)
+        Base64.strict_decode64(token)
+      end
+
+      def xor_byte_strings(s1, s2)
+        s1.bytes.zip(s2.bytes).map { |(c1, c2)| c1 ^ c2 }.pack("c*")
+      end
+    end
+  end
+end

lib/sidekiq/web/helpers.rb

@@ -230,7 +230,7 @@ module Sidekiq
     end
 
     def csrf_tag
-      "<input type='hidden' name='authenticity_token' value='#{session[:csrf]}'/>"
+      "<input type='hidden' name='authenticity_token' value='#{env[:csrf_token]}'/>"
     end
 
     def to_display(arg)

test/test_csrf.rb

@@ -0,0 +1,93 @@
+require_relative './helper'
+require 'sidekiq/web/csrf_protection'
+
+class TestCsrf < Minitest::Test
+  def session
+    @session ||= {}
+  end
+
+  def env(opts={})
+    imp =  StringIO.new("")
+    {
+      "REQUEST_METHOD" => "GET",
+      "rack.session" => session,
+      "rack.logger" => ::Logger.new(@logio ||= StringIO.new("")),
+      "rack.input" => imp,
+      "rack.request.form_input" => imp,
+      "rack.request.form_hash" => {},
+    }.merge(opts)
+  end
+
+  def call(env, &block)
+    Sidekiq::Web::CsrfProtection.new(block).call(env)
+  end
+
+  def test_get
+    ok = [200, {}, ["OK"]]
+    first = 1
+    second = 1
+    result = call(env) do |envy|
+      refute_nil envy[:csrf_token]
+      assert_equal 88, envy[:csrf_token].size
+      first = envy[:csrf_token]
+      ok
+    end
+    assert_equal ok, result
+
+    result = call(env) do |envy|
+      refute_nil envy[:csrf_token]
+      assert_equal 88, envy[:csrf_token].size
+      second = envy[:csrf_token]
+      ok
+    end
+    assert_equal ok, result
+
+    # verify masked token changes on every valid request
+    refute_equal first, second
+  end
+
+  def test_bad_post
+    result = call(env("REQUEST_METHOD" => "POST")) do
+      raise "Shouldnt be called"
+    end
+    refute_nil result
+    assert_equal 403, result[0]
+    assert_equal ["Forbidden"], result[2]
+
+    @logio.rewind
+    assert_match(/attack prevented/, @logio.string)
+  end
+
+  def test_good_and_bad_posts
+    goodtoken = nil
+    # Make a GET to set up the session with a good token
+    goodtoken = call(env) do |envy|
+      envy[:csrf_token]
+    end
+    assert goodtoken
+
+    # Make a POST with the known good token
+    result = call(
+      env({
+        "REQUEST_METHOD" => "POST",
+        "rack.request.form_hash" => { "authenticity_token"=>goodtoken }
+      })) do
+      [200, {}, ["OK"]]
+    end
+    refute_nil result
+    assert_equal 200, result[0]
+    assert_equal ["OK"], result[2]
+
+    # Make a POST with a known bad token
+    result = call(
+      env({
+        "REQUEST_METHOD" => "POST",
+        "rack.request.form_hash" => { "authenticity_token"=>"N0QRBD34tU61d7fi+0ZaF/35JLW/9K+8kk8dc1TZoK/0pTl7GIHap5gy7BWGsoKlzbMLRp1yaDpCDFwTJtxWAg==", },
+      })) do
+      raise "shouldnt be called"
+    end
+    refute_nil result
+    assert_equal 403, result[0]
+    assert_equal ["Forbidden"], result[2]
+  end
+end