Commit 043c6649 authored by Steve Abrams's avatar Steve Abrams

Verify workhorse request for uploads

Verify PUT requests come from workhorse for
package upload and artifact endpoints using workhorse uploads.
parent e3bee019
---
title: Add workhorse request verification to package upload endpoints
merge_request:
author:
type: security
...@@ -179,10 +179,7 @@ module API ...@@ -179,10 +179,7 @@ module API
end end
route_setting :authentication, job_token_allowed: true route_setting :authentication, job_token_allowed: true
put ':id/packages/maven/*path/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do put ':id/packages/maven/*path/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do
authorize_create_package!(user_project) authorize_upload!
require_gitlab_workhorse!
Gitlab::Workhorse.verify_api_request!(headers)
status 200 status 200
content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
...@@ -205,8 +202,7 @@ module API ...@@ -205,8 +202,7 @@ module API
end end
route_setting :authentication, job_token_allowed: true route_setting :authentication, job_token_allowed: true
put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do
authorize_create_package!(user_project) authorize_upload!
require_gitlab_workhorse!
file_name, format = extract_format(params[:file_name]) file_name, format = extract_format(params[:file_name])
......
...@@ -586,6 +586,8 @@ describe API::ConanPackages do ...@@ -586,6 +586,8 @@ describe API::ConanPackages do
context 'without a file from workhorse' do context 'without a file from workhorse' do
let(:params) { { file: nil } } let(:params) { { file: nil } }
it_behaves_like 'package workhorse uploads'
it 'rejects the request' do it 'rejects the request' do
subject subject
...@@ -710,7 +712,7 @@ describe API::ConanPackages do ...@@ -710,7 +712,7 @@ describe API::ConanPackages do
subject subject
expect(response).to have_gitlab_http_status(:error) expect(response).to have_gitlab_http_status(:forbidden)
end end
context 'when using remote storage' do context 'when using remote storage' do
......
This diff is collapsed.
...@@ -172,8 +172,8 @@ describe API::NugetPackages do ...@@ -172,8 +172,8 @@ describe API::NugetPackages do
end end
describe 'PUT /api/v4/projects/:id/packages/nuget' do describe 'PUT /api/v4/projects/:id/packages/nuget' do
let_it_be(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
let_it_be(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
let_it_be(:file_name) { 'package.nupkg' } let_it_be(:file_name) { 'package.nupkg' }
let(:url) { "/projects/#{project.id}/packages/nuget" } let(:url) { "/projects/#{project.id}/packages/nuget" }
let(:headers) { {} } let(:headers) { {} }
......
...@@ -72,7 +72,7 @@ RSpec.shared_examples 'process nuget workhorse authorization' do |user_type, sta ...@@ -72,7 +72,7 @@ RSpec.shared_examples 'process nuget workhorse authorization' do |user_type, sta
project.add_maintainer(user) project.add_maintainer(user)
end end
it_behaves_like 'returning response status', :error it_behaves_like 'returning response status', :forbidden
end end
end end
end end
...@@ -104,6 +104,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member = ...@@ -104,6 +104,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
end end
context 'with correct params' do context 'with correct params' do
it_behaves_like 'package workhorse uploads'
it_behaves_like 'creates nuget package files' it_behaves_like 'creates nuget package files'
it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package'
end end
......
...@@ -115,3 +115,17 @@ RSpec.shared_examples 'background upload schedules a file migration' do ...@@ -115,3 +115,17 @@ RSpec.shared_examples 'background upload schedules a file migration' do
end end
end end
end end
shared_examples 'package workhorse uploads' do
context 'without a workhorse header' do
let(:workhorse_token) { JWT.encode({ 'iss' => 'invalid header' }, Gitlab::Workhorse.secret, 'HS256') }
it_behaves_like 'returning response status', :forbidden
it 'logs an error' do
expect(Gitlab::ErrorTracking).to receive(:track_exception).once
subject
end
end
end
...@@ -258,11 +258,21 @@ module API ...@@ -258,11 +258,21 @@ module API
end end
def require_gitlab_workhorse! def require_gitlab_workhorse!
verify_workhorse_api!
unless env['HTTP_GITLAB_WORKHORSE'].present? unless env['HTTP_GITLAB_WORKHORSE'].present?
forbidden!('Request should be executed via GitLab Workhorse') forbidden!('Request should be executed via GitLab Workhorse')
end end
end end
def verify_workhorse_api!
Gitlab::Workhorse.verify_api_request!(request.headers)
rescue => e
Gitlab::ErrorTracking.track_exception(e)
forbidden!
end
def require_pages_enabled! def require_pages_enabled!
not_found! unless user_project.pages_available? not_found! unless user_project.pages_available?
end end
......
...@@ -1545,7 +1545,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do ...@@ -1545,7 +1545,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
authorize_artifacts authorize_artifacts
expect(response).to have_gitlab_http_status(500) expect(response).to have_gitlab_http_status(:forbidden)
end end
context 'authorization token is invalid' do context 'authorization token is invalid' do
...@@ -1675,6 +1675,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do ...@@ -1675,6 +1675,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end end
end end
context 'Is missing GitLab Workhorse token headers' do
let(:jwt_token) { JWT.encode({ 'iss' => 'invalid-header' }, Gitlab::Workhorse.secret, 'HS256') }
it 'fails to post artifacts without GitLab-Workhorse' do
expect(Gitlab::ErrorTracking).to receive(:track_exception).once
upload_artifacts(file_upload, headers_with_token)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'when setting an expire date' do context 'when setting an expire date' do
let(:default_artifacts_expire_in) {} let(:default_artifacts_expire_in) {}
let(:post_data) do let(:post_data) do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment