From 47bc87034fbe196f7aac65415e67ef562f41d246 Mon Sep 17 00:00:00 2001 From: Mike Perham Date: Wed, 3 Jun 2020 16:06:36 -0700 Subject: [PATCH] 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 | 6 +- lib/sidekiq/web/authenticity_token.rb | 254 -------------------------- lib/sidekiq/web/csrf_protection.rb | 153 ++++++++++++++++ lib/sidekiq/web/helpers.rb | 2 +- test/test_csrf.rb | 93 ++++++++++ 5 files changed, 250 insertions(+), 258 deletions(-) delete mode 100644 lib/sidekiq/web/authenticity_token.rb create mode 100644 lib/sidekiq/web/csrf_protection.rb create mode 100644 test/test_csrf.rb diff --git a/lib/sidekiq/web.rb b/lib/sidekiq/web.rb index 5407ce51..9de36231 100644 --- a/lib/sidekiq/web.rb +++ b/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" - middlewares.unshift [[AuthenticityToken], nil] + unless using?(CsrfProtection) || ENV["RACK_ENV"] == "test" + middlewares.unshift [[CsrfProtection], nil] end s = sessions diff --git a/lib/sidekiq/web/authenticity_token.rb b/lib/sidekiq/web/authenticity_token.rb deleted file mode 100644 index 80253d8b..00000000 --- a/lib/sidekiq/web/authenticity_token.rb +++ /dev/null @@ -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 diff --git a/lib/sidekiq/web/csrf_protection.rb b/lib/sidekiq/web/csrf_protection.rb new file mode 100644 index 00000000..fea42c98 --- /dev/null +++ b/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 diff --git a/lib/sidekiq/web/helpers.rb b/lib/sidekiq/web/helpers.rb index f387ed33..f845d724 100644 --- a/lib/sidekiq/web/helpers.rb +++ b/lib/sidekiq/web/helpers.rb @@ -230,7 +230,7 @@ module Sidekiq end def csrf_tag - "" + "" end def to_display(arg) diff --git a/test/test_csrf.rb b/test/test_csrf.rb new file mode 100644 index 00000000..9d6b174a --- /dev/null +++ b/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