summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorblackst0ne <blackst0ne.ru@gmail.com>2017-06-16 11:04:24 +1100
committerDouwe Maan <douwe@selenight.nl>2017-07-26 11:05:44 +0200
commit29022350999ab3ddc4518f7a7647939ec2de8e09 (patch)
tree2b3e49cbe583ebf51b792318466880f6a208f11a
parentf2da36f19661353cd1bd6788fbdf1a65e2d70f8d (diff)
downloadgitlab-ce-29022350999ab3ddc4518f7a7647939ec2de8e09.tar.gz
Add CSRF token verification to API
-rw-r--r--changelogs/unreleased/33601-add-csrf-token-verification-to-api.yml4
-rw-r--r--lib/api/helpers.rb38
-rw-r--r--spec/lib/api/helpers/csrf_tokens_spec.rb42
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