Verify JWT messages from gitlab-workhorse

5 files changed, 134 insertions, 5 deletions

diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index c5c1402e8fc..63032d43027 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -4,7 +4,7 @@ describe Gitlab::Workhorse, lib: true do
   let(:project) { create(:project) }
   let(:subject) { Gitlab::Workhorse }
 
-  describe "#send_git_archive" do
+  describe ".send_git_archive" do
     context "when the repository doesn't have an archive file path" do
       before do
         allow(project.repository).to receive(:archive_metadata).and_return(Hash.new)
@@ -15,4 +15,88 @@ describe Gitlab::Workhorse, lib: true do
       end
     end
   end
+
+  describe ".secret" do
+    subject { described_class.secret }
+
+    before do
+      described_class.instance_variable_set(:@secret, nil)
+      described_class.write_secret
+    end
+
+    it 'returns 32 bytes' do
+      expect(subject).to be_a(String)
+      expect(subject.length).to eq(32)
+      expect(subject.encoding).to eq(Encoding::ASCII_8BIT)
+    end
+
+    it 'raises an exception if the secret file cannot be read' do
+      File.delete(described_class.secret_path)
+      expect { subject }.to raise_exception(Errno::ENOENT)
+    end
+
+    it 'raises an exception if the secret file contains the wrong number of bytes' do
+      File.truncate(described_class.secret_path, 0)
+      expect { subject }.to raise_exception(RuntimeError)
+    end
+  end
+
+  describe ".write_secret" do
+    let(:secret_path) { described_class.secret_path }
+    before do
+      begin
+        File.delete(secret_path)
+      rescue Errno::ENOENT
+      end
+
+      described_class.write_secret
+    end
+
+    it 'uses mode 0600' do
+      expect(File.stat(secret_path).mode & 0777).to eq(0600)
+    end
+
+    it 'writes base64 data' do
+      bytes = Base64.strict_decode64(File.read(secret_path))
+      expect(bytes).not_to be_empty
+    end
+  end
+
+  describe '#verify_api_request!' do
+    let(:header_key) { described_class.const_get('INTERNAL_API_REQUEST_HEADER') }
+    let(:payload) { { 'iss' => 'gitlab-workhorse' } }
+
+    it 'accepts a correct header' do
+      headers = { header_key => JWT.encode(payload, described_class.secret, 'HS256') }
+      expect { call_verify(headers) }.not_to raise_error
+    end
+
+    it 'raises an error when the header is not set' do
+      expect { call_verify({}) }.to raise_jwt_error
+    end
+
+    it 'raises an error when the header is not signed' do
+      headers = { header_key => JWT.encode(payload, nil, 'none') }
+      expect { call_verify(headers) }.to raise_jwt_error
+    end
+
+    it 'raises an error when the header is signed with the wrong key' do
+      headers = { header_key => JWT.encode(payload, 'wrongkey', 'HS256') }
+      expect { call_verify(headers) }.to raise_jwt_error
+    end
+
+    it 'raises an error when the issuer is incorrect' do
+      payload['iss'] = 'somebody else'
+      headers = { header_key => JWT.encode(payload, described_class.secret, 'HS256') }
+      expect { call_verify(headers) }.to raise_jwt_error
+    end
+
+    def raise_jwt_error
+      raise_error(JWT::DecodeError)
+    end
+
+    def call_verify(headers)
+      described_class.verify_api_request!(headers)
+    end
+  end
 end
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index ca7932dc5da..29a194b31f6 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -230,7 +230,8 @@ describe Ci::API::API do
       let(:post_url) { ci_api("/builds/#{build.id}/artifacts") }
       let(:delete_url) { ci_api("/builds/#{build.id}/artifacts") }
       let(:get_url) { ci_api("/builds/#{build.id}/artifacts") }
-      let(:headers) { { "GitLab-Workhorse" => "1.0" } }
+      let(:jwt_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
+      let(:headers) { { "GitLab-Workhorse" => "1.0", Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => jwt_token } }
       let(:headers_with_token) { headers.merge(Ci::API::Helpers::BUILD_TOKEN_HEADER => build.token) }
 
       before { build.run! }
@@ -240,14 +241,22 @@ describe Ci::API::API do
           it "using token as parameter" do
             post authorize_url, { token: build.token }, headers
             expect(response).to have_http_status(200)
+            expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
             expect(json_response["TempPath"]).not_to be_nil
           end
 
           it "using token as header" do
             post authorize_url, {}, headers_with_token
             expect(response).to have_http_status(200)
+            expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
             expect(json_response["TempPath"]).not_to be_nil
           end
+
+          it "reject requests that did not go through gitlab-workhorse" do
+            headers.delete('Gitlab-Workhorse-Api-Request')
+            post authorize_url, { token: build.token }, headers
+            expect(response).to have_http_status(500)
+          end
         end
 
         context "should fail to post too large artifact" do
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 9ca3b021aa2..b7001fede40 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -1,6 +1,8 @@
 require "spec_helper"
 
 describe 'Git HTTP requests', lib: true do
+  include WorkhorseHelpers
+
   let(:user)    { create(:user) }
   let(:project) { create(:project, path: 'project.git-project') }
 
@@ -48,6 +50,7 @@ describe 'Git HTTP requests', lib: true do
 
         expect(response).to have_http_status(200)
         expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace)
+        expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
       end
     end
   end
@@ -63,6 +66,7 @@ describe 'Git HTTP requests', lib: true do
       it "downloads get status 200" do
         download(path, {}) do |response|
           expect(response).to have_http_status(200)
+          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
         end
       end
 
@@ -101,6 +105,14 @@ describe 'Git HTTP requests', lib: true do
           end
         end
       end
+      
+      context 'when the request is not from gitlab-workhorse' do
+        it 'raises an exception' do
+          expect do
+            get("/#{project.path_with_namespace}.git/info/refs?service=git-upload-pack")
+          end.to raise_error(JWT::DecodeError)
+        end
+      end
     end
 
     context "when the project is private" do
@@ -170,11 +182,13 @@ describe 'Git HTTP requests', lib: true do
                 clone_get(path, env)
 
                 expect(response).to have_http_status(200)
+                expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
               end
 
               it "uploads get status 200" do
                 upload(path, env) do |response|
                   expect(response).to have_http_status(200)
+                  expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
                 end
               end
             end
@@ -189,6 +203,7 @@ describe 'Git HTTP requests', lib: true do
                 clone_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token
 
                 expect(response).to have_http_status(200)
+                expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
               end
 
               it "uploads get status 401 (no project existence information leak)" do
@@ -297,6 +312,7 @@ describe 'Git HTTP requests', lib: true do
           clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token
 
           expect(response).to have_http_status(200)
+          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
         end
 
         it "uploads get status 401 (no project existence information leak)" do
@@ -426,7 +442,7 @@ describe 'Git HTTP requests', lib: true do
   end
 
   def auth_env(user, password, spnego_request_token)
-    env = {}
+    env = workhorse_internal_api_request_header
     if user && password
       env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(user, password)
     elsif spnego_request_token
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index fcd6521317a..6e551bb65fa 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe 'Git LFS API and storage' do
+  include WorkhorseHelpers
+
   let(:user) { create(:user) }
   let!(:lfs_object) { create(:lfs_object, :with_file) }
 
@@ -715,6 +717,12 @@ describe 'Git LFS API and storage' do
             project.team << [user, :developer]
           end
 
+          context 'and the request bypassed workhorse' do
+            it 'raises an exception' do
+              expect { put_authorize(verified: false) }.to raise_error JWT::DecodeError
+            end
+          end
+
           context 'and request is sent by gitlab-workhorse to authorize the request' do
             before do
               put_authorize
@@ -724,6 +732,10 @@ describe 'Git LFS API and storage' do
               expect(response).to have_http_status(200)
             end
 
+            it 'uses the gitlab-workhorse content type' do
+              expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+            end
+
             it 'responds with status 200, location of lfs store and object details' do
               expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload")
               expect(json_response['LfsOid']).to eq(sample_oid)
@@ -863,8 +875,11 @@ describe 'Git LFS API and storage' do
       end
     end
 
-    def put_authorize
-      put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}/authorize", nil, headers
+    def put_authorize(verified: true)
+      authorize_headers = headers
+      authorize_headers.merge!(workhorse_internal_api_request_header) if verified
+
+      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)
diff --git a/spec/support/workhorse_helpers.rb b/spec/support/workhorse_helpers.rb
index 107b6e30924..47673cd4c3a 100644
--- a/spec/support/workhorse_helpers.rb
+++ b/spec/support/workhorse_helpers.rb
@@ -13,4 +13,9 @@ module WorkhorseHelpers
       ]
     end
   end
+
+  def workhorse_internal_api_request_header
+    jwt_token = JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256')
+    { 'HTTP_' + Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER.upcase.tr('-', '_') => jwt_token }
+  end
 end