diff options
| -rw-r--r-- | changelogs/unreleased/33601-add-csrf-token-verification-to-api.yml | 4 | ||||
| -rw-r--r-- | lib/api/helpers.rb | 38 | ||||
| -rw-r--r-- | spec/lib/api/helpers/csrf_tokens_spec.rb | 42 |
3 files changed, 80 insertions, 4 deletions
diff --git a/changelogs/unreleased/33601-add-csrf-token-verification-to-api.yml b/changelogs/unreleased/33601-add-csrf-token-verification-to-api.yml new file mode 100644 index 00000000000..fa1a77ebcc1 --- /dev/null +++ b/changelogs/unreleased/33601-add-csrf-token-verification-to-api.yml @@ -0,0 +1,4 @@ +--- +title: Add CSRF token verification to API +merge_request: 12154 +author: @blackst0ne diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 57e3e93500f..ab5f4c865e0 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -328,6 +328,33 @@ module API private + def xor_byte_strings(s1, s2) + s2_bytes = s2.bytes + s1.each_byte.with_index { |c1, i| s2_bytes[i] ^= c1 } + s2_bytes.pack('C*') + end + + # Check if CSRF tokens are equal. + # The header token is masked. + # So, before the comparison it must be unmasked. + def csrf_tokens_valid?(request) + session_token = request.session['_csrf_token'] + header_token = request.headers['X-Csrf-Token'] + + session_token = Base64.strict_decode64(session_token) + header_token = Base64.strict_decode64(header_token) + + # Decoded CSRF token passed from the frontend has to be 64 symbols long. + return false if header_token.size != 64 + + header_token = xor_byte_strings(header_token[0...32], header_token[32..-1]) + + ActiveSupport::SecurityUtils.secure_compare(session_token, header_token) + + rescue + false + end + def private_token params[APIGuard::PRIVATE_TOKEN_PARAM] || env[APIGuard::PRIVATE_TOKEN_HEADER] end @@ -336,12 +363,15 @@ module API env['warden'] end + def verified_request? + request = Grape::Request.new(env) + + request.head? || request.get? || csrf_tokens_valid?(request) + end + # Check the Rails session for valid authentication details - # - # Until CSRF protection is added to the API, disallow this method for - # state-changing endpoints def find_user_from_warden - warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) + warden.try(:authenticate) if verified_request? end def initial_current_user diff --git a/spec/lib/api/helpers/csrf_tokens_spec.rb b/spec/lib/api/helpers/csrf_tokens_spec.rb new file mode 100644 index 00000000000..d16db6c9064 --- /dev/null +++ b/spec/lib/api/helpers/csrf_tokens_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe API::Helpers do + subject do + Class.new.include(described_class).new + end + + let(:header_token) { 'WblCcheb1qQLHFVhlMtwOhxJr5613vUT05vCvToRvfJ68UPT7+eV5xpaY9CjubnF3VGbTfIhQYkZWmWTfvZAWQ==' } + let(:session_token) { 'I0gBofh8Q0MRRjaxN3LJ/8EYNNNH/7SaysGnLkTn/as=' } + + before do + class Request + attr_reader :headers + attr_reader :session + + def initialize(header_token = nil, session_token = nil) + @headers = { 'X-Csrf-Token' => header_token } + @session = { '_csrf_token' => session_token } + end + end + end + + it 'should return false if header token is invalid' do + request = Request.new(nil, session_token) + expect(subject.send(:csrf_tokens_valid?, request)).to be false + end + + it 'should return false if session_token token is invalid' do + request = Request.new(header_token, nil) + expect(subject.send(:csrf_tokens_valid?, request)).to be false + end + + it 'should return false if header_token is not 64 symbols long' do + request = Request.new(header_token[0..16], session_token) + expect(subject.send(:csrf_tokens_valid?, request)).to be false + end + + it 'should return true if both header_token and session_token are correct' do + request = Request.new(header_token, session_token) + expect(subject.send(:csrf_tokens_valid?, request)).to be true + end +end |