summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2019-01-24 12:39:00 +0000
committerYorick Peterse <yorickpeterse@gmail.com>2019-01-24 12:39:03 +0000
commit1493ede681d12c037f2ad2ef92ce56fcb456ae42 (patch)
treed5fc73cbcac7b79dfb699c66e2fad25ed238b9fa
parentd8916379ed24df335eedd0f7b4c7f3db38d22d2b (diff)
downloadgitlab-ce-1493ede681d12c037f2ad2ef92ce56fcb456ae42.tar.gz
Merge branch 'security-2767-verify-lfs-finalize-from-workhorse-11-5' into 'security-11-5'
[11.5] Verify that LFS upload requests are genuine See merge request gitlab/gitlabhq!2864 (cherry picked from commit 5c3d4d012e734b12140ecc527ade0f5ae8a26049) dd634b25 Verify that LFS upload requests are genuine
-rw-r--r--GITLAB_WORKHORSE_VERSION2
-rw-r--r--app/controllers/projects/lfs_storage_controller.rb2
-rw-r--r--changelogs/unreleased/security-2767-verify-lfs-finalize-from-workhorse.yml5
-rw-r--r--spec/requests/lfs_http_spec.rb25
4 files changed, 26 insertions, 8 deletions
diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index 1996c504476..b7f8ee41e69 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-7.1.3
+7.1.4
diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb
index babeee48ef3..013e01b82aa 100644
--- a/app/controllers/projects/lfs_storage_controller.rb
+++ b/app/controllers/projects/lfs_storage_controller.rb
@@ -5,7 +5,7 @@ class Projects::LfsStorageController < Projects::GitHttpClientController
include WorkhorseRequest
include SendFileUpload
- skip_before_action :verify_workhorse_api!, only: [:download, :upload_finalize]
+ skip_before_action :verify_workhorse_api!, only: :download
def download
lfs_object = LfsObject.find_by_oid(oid)
diff --git a/changelogs/unreleased/security-2767-verify-lfs-finalize-from-workhorse.yml b/changelogs/unreleased/security-2767-verify-lfs-finalize-from-workhorse.yml
new file mode 100644
index 00000000000..e79e3263df7
--- /dev/null
+++ b/changelogs/unreleased/security-2767-verify-lfs-finalize-from-workhorse.yml
@@ -0,0 +1,5 @@
+---
+title: Verify that LFS upload requests are genuine
+merge_request:
+author:
+type: security
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index e349181b794..1b0388444c3 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -1086,6 +1086,12 @@ describe 'Git LFS API and storage' do
end
end
+ context 'and request to finalize the upload is not sent by gitlab-workhorse' do
+ it 'fails with a JWT decode error' do
+ expect { put_finalize(lfs_tmp_file, verified: false) }.to raise_error(JWT::DecodeError)
+ end
+ end
+
context 'and workhorse requests upload finalize for a new lfs object' do
before do
lfs_object.destroy
@@ -1347,8 +1353,12 @@ describe 'Git LFS API and storage' do
context 'when pushing the same lfs object to the second project' do
before do
- put "#{second_project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}", nil,
- headers.merge('X-Gitlab-Lfs-Tmp' => lfs_tmp_file).compact
+ finalize_headers = headers
+ .merge('X-Gitlab-Lfs-Tmp' => lfs_tmp_file)
+ .merge(workhorse_internal_api_request_header)
+
+ put "#{second_project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}",
+ nil, finalize_headers
end
it 'responds with status 200' do
@@ -1369,7 +1379,7 @@ describe 'Git LFS API and storage' do
put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}/authorize", nil, authorize_headers
end
- def put_finalize(lfs_tmp = lfs_tmp_file, with_tempfile: false, args: {})
+ def put_finalize(lfs_tmp = lfs_tmp_file, with_tempfile: false, verified: true, args: {})
upload_path = LfsObjectUploader.workhorse_local_upload_path
file_path = upload_path + '/' + lfs_tmp if lfs_tmp
@@ -1383,11 +1393,14 @@ describe 'Git LFS API and storage' do
'file.name' => File.basename(file_path)
}
- put_finalize_with_args(args.merge(extra_args).compact)
+ put_finalize_with_args(args.merge(extra_args).compact, verified: verified)
end
- def put_finalize_with_args(args)
- put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}", args, headers
+ def put_finalize_with_args(args, verified:)
+ finalize_headers = headers
+ finalize_headers.merge!(workhorse_internal_api_request_header) if verified
+
+ put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}", args, finalize_headers
end
def lfs_tmp_file