Verify that LFS upload requests are genuine

LFS uploads are handled in concert by workhorse and rails. In normal
use, workhorse:

* Authorizes the request with rails (upload_authorize)
* Handles the upload of the file to a tempfile - disk or object storage
* Validates the file size and contents
* Hands off to rails to complete the upload (upload_finalize)

In `upload_finalize`, the LFS object is linked to the project. As LFS
objects are deduplicated across all projects, it may already exist. If
not, the temporary file is copied to the correct place, and will be
used by all future LFS objects with the same OID.

Workhorse uses the Content-Type of the request to decide to follow this
routine, as the URLs are ambiguous. If the Content-Type is anything but
"application/octet-stream", the request is proxied directly to rails,
on the assumption that this is a normal file edit request. If it's an
actual LFS request with a different content-type, however, it is routed
to the Rails `upload_finalize` action, which treats it as an LFS upload
just as it would a workhorse-modified request.

The outcome is that users can upload LFS objects that don't match the
declared size or OID. They can also create links to LFS objects they
don't really own, allowing them to read the contents of files if they
know just the size or OID.

We can close this hole by requiring requests to `upload_finalize` to be
sourced from Workhorse. The mechanism to do this already exists.

4 files changed, 25 insertions, 7 deletions

diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index da156181014..0e79152459e 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-8.1.0
\ No newline at end of file
+8.1.1
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 f1514e90eb2..1781759c54b 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,9 +1353,13 @@ describe 'Git LFS API and storage' do
 
         context 'when pushing the same lfs object to the second project' do
           before do
+            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}",
                 params: {},
-                headers: headers.merge('X-Gitlab-Lfs-Tmp' => lfs_tmp_file).compact
+                headers: finalize_headers
           end
 
           it 'responds with status 200' do
@@ -1370,7 +1380,7 @@ describe 'Git LFS API and storage' do
       put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}/authorize", params: {}, headers: 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
 
@@ -1384,11 +1394,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}", params: args, headers: 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}", params: args, headers: finalize_headers
     end
 
     def lfs_tmp_file