diff --git a/app/controllers/projects/packages/package_files_controller.rb b/app/controllers/projects/packages/package_files_controller.rb index 0222e09f54abddb145bc43c9c461380095c96795..0b3ab1ae1ad99968e51fbf94bf829f741b167436 100644 --- a/app/controllers/projects/packages/package_files_controller.rb +++ b/app/controllers/projects/packages/package_files_controller.rb @@ -13,7 +13,8 @@ def download package_file.package.touch_last_downloaded_at - send_upload(package_file.file, attachment: package_file.file_name_for_download) + send_upload(package_file.file, attachment: package_file.file_name_for_download, + ssrf_params: ::Packages::SsrfProtection.params_for(package_file.package)) end end end diff --git a/config/feature_flags/gitlab_com_derisk/generic_package_registry_ssrf_protection.yml b/config/feature_flags/gitlab_com_derisk/generic_package_registry_ssrf_protection.yml new file mode 100644 index 0000000000000000000000000000000000000000..2250ae9cb842edd4e07a78bb67f925ca19d50261 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/generic_package_registry_ssrf_protection.yml @@ -0,0 +1,10 @@ +--- +name: generic_package_registry_ssrf_protection +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/520294 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193902 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/547452 +milestone: '18.2' +group: group::package registry +type: gitlab_com_derisk +default_enabled: false diff --git a/lib/api/helpers/packages_helpers.rb b/lib/api/helpers/packages_helpers.rb index a7b2795253a56e33af659e03a1e7e39e0357fba6..36055a71ffa3a9f0ac2731754f8316a149bf8e06 100644 --- a/lib/api/helpers/packages_helpers.rb +++ b/lib/api/helpers/packages_helpers.rb @@ -142,7 +142,8 @@ def present_package_file!(package_file, supports_direct_download: true, content_ present_carrierwave_file!( package_file.file, supports_direct_download: supports_direct_download, - content_disposition: content_disposition + content_disposition: content_disposition, + extra_send_url_params: ::Packages::SsrfProtection.params_for(package_file.package) ) end diff --git a/lib/packages/ssrf_protection.rb b/lib/packages/ssrf_protection.rb new file mode 100644 index 0000000000000000000000000000000000000000..6b42c4decaa15f9944892ac619205d36d9313055 --- /dev/null +++ b/lib/packages/ssrf_protection.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Packages + class SsrfProtection + def self.params_for(package) + return {} unless package + return {} unless package_feature_enabled?(package) + + { + ssrf_filter: true, + allow_localhost: allow_localhost?, + allowed_endpoints: ObjectStoreSettings.enabled_endpoint_uris + } + end + + def self.allow_localhost? + Gitlab.dev_or_test_env? || Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? + end + + def self.package_feature_enabled?(package) + case package.package_type.to_sym + when :generic + Feature.enabled?(:generic_package_registry_ssrf_protection, package.project) + # Future package types can be added here + # when :npm + # Feature.enabled?(:npm_package_registry_ssrf_protection, package.project) + else + false + end + end + end +end diff --git a/spec/lib/packages/ssrf_protection_spec.rb b/spec/lib/packages/ssrf_protection_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4e3c9cc3290a485918ca442722564b8edd963270 --- /dev/null +++ b/spec/lib/packages/ssrf_protection_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::SsrfProtection, feature_category: :package_registry do + let_it_be(:project) { create(:project) } + let_it_be(:generic_package) { create(:generic_package, project: project) } + let_it_be(:unsupported_package) { create(:conan_package, project: project) } + + describe '.params_for' do + context 'when package is nil' do + it 'returns empty hash' do + expect(described_class.params_for(nil)).to eq({}) + end + end + + context 'when package type is not supported' do + it 'returns empty hash' do + expect(described_class.params_for(unsupported_package)).to eq({}) + end + end + + context 'when package type is supported' do + it 'returns SSRF protection params for generic package' do + result = described_class.params_for(generic_package) + + expect(result).to include( + ssrf_filter: true, + allow_localhost: true, + allowed_endpoints: ObjectStoreSettings.enabled_endpoint_uris + ) + end + + context 'when in production environment' do + before do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) + end + + context 'when local requests are allowed from webhooks' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + end + + it 'allows localhost' do + result = described_class.params_for(generic_package) + expect(result[:allow_localhost]).to be true + end + end + + context 'when local requests are not allowed from webhooks' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + end + + it 'does not allow localhost' do + result = described_class.params_for(generic_package) + expect(result[:allow_localhost]).to be false + end + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(generic_package_registry_ssrf_protection: false) + end + + it 'returns empty hash for generic package' do + expect(described_class.params_for(generic_package)).to eq({}) + end + end + end + end + + describe '.allow_localhost?' do + context 'when in development environment' do + before do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(true) + end + + it 'returns true' do + expect(described_class.allow_localhost?).to be true + end + end + + context 'when in production environment' do + before do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) + end + + context 'when local requests are allowed from webhooks' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + end + + it 'returns true' do + expect(described_class.allow_localhost?).to be true + end + end + + context 'when local requests are not allowed from webhooks' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + end + + it 'returns false' do + expect(described_class.allow_localhost?).to be false + end + end + end + end +end diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index 87f9d251094d79867685903f78966a2e578e4d28..312e23b51d18889277a413a120a530ffadb11d39 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -1131,23 +1131,36 @@ def upload_file( end context 'when direct download is disabled' do - let(:dispositon_header) do + let(:disposition_header) do "attachment; filename=\"#{package_file.file_name}\"; filename*=UTF-8\'\'#{package_file.file_name}" end + let(:expected_headers) do + { + allow_localhost: true, + allowed_endpoints: [], + response_headers: { + 'Content-Disposition' => disposition_header + }, + ssrf_filter: true + } + end + before do stub_package_file_object_storage(proxy_download: true) end it 'sends a file with response-content-disposition and filename' do expect(::Gitlab::Workhorse).to receive(:send_url) - .with(instance_of(String), { response_headers: { 'Content-Disposition' => dispositon_header } }) + .with(instance_of(String), expected_headers) .and_call_original download expect(response).to have_gitlab_http_status(:ok) end + + it_behaves_like 'package registry SSRF protection' end end diff --git a/spec/requests/projects/packages/package_files_controller_spec.rb b/spec/requests/projects/packages/package_files_controller_spec.rb index 5cb71e15619b4af96b6581495259d1c3aa294ebf..a68866de8931c5f48af6dbef888d85a2fd9d3b6f 100644 --- a/spec/requests/projects/packages/package_files_controller_spec.rb +++ b/spec/requests/projects/packages/package_files_controller_spec.rb @@ -25,33 +25,43 @@ .to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) end - context 'when the fog provider is Google and on .com', :saas do + context 'with remote object storage' do let(:package_file) { create(:package_file, :object_storage, package: package, file_name: filename) } - before do - stub_package_file_object_storage( - config: Gitlab.config.packages.object_store.merge(connection: { - provider: 'Google', - google_storage_access_key_id: 'test-access-id', - google_storage_secret_access_key: 'secret' - }), - proxy_download: true - ) - end + context 'when the fog provider is Google and on .com', :saas do + before do + stub_package_file_object_storage( + config: Gitlab.config.packages.object_store.merge(connection: { + provider: 'Google', + google_storage_access_key_id: 'test-access-id', + google_storage_secret_access_key: 'secret' + }), + proxy_download: true + ) + end - it 'send the correct headers' do - subject + it 'send the correct headers' do + subject + + command, encoded_params = response.headers[::Gitlab::Workhorse::SEND_DATA_HEADER].split(':') + params = Gitlab::Json.parse(Base64.urlsafe_decode64(encoded_params)) + + expect(command).to eq('send-url') + expect(params['URL']).to include( + %(response-content-disposition=attachment%3B%20filename%3D%22#{filename}), + 'x-goog-custom-audit-gitlab-project', + 'x-goog-custom-audit-gitlab-namespace', + 'x-goog-custom-audit-gitlab-size-bytes' + ) + end + end - command, encoded_params = response.headers[::Gitlab::Workhorse::SEND_DATA_HEADER].split(':') - params = Gitlab::Json.parse(Base64.urlsafe_decode64(encoded_params)) + context 'when direct download is disabled' do + before do + stub_package_file_object_storage(proxy_download: true) + end - expect(command).to eq('send-url') - expect(params['URL']).to include( - %(response-content-disposition=attachment%3B%20filename%3D%22#{filename}), - 'x-goog-custom-audit-gitlab-project', - 'x-goog-custom-audit-gitlab-namespace', - 'x-goog-custom-audit-gitlab-size-bytes' - ) + it_behaves_like 'package registry SSRF protection' end end diff --git a/spec/support/shared_examples/package_registry_ssrf_examples.rb b/spec/support/shared_examples/package_registry_ssrf_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..500011ba2287492517b1fbb6d4aa516dbac29135 --- /dev/null +++ b/spec/support/shared_examples/package_registry_ssrf_examples.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'package registry SSRF protection' do + context 'with custom object store endpoints' do + let(:custom_endpoints) { ['https://custom-storage.example.com', 'https://backup.example.com'] } + + before do + allow(ObjectStoreSettings).to receive(:enabled_endpoint_uris).and_return(custom_endpoints) + end + + it 'includes custom endpoints in allowed_endpoints' do + expect(Gitlab::Workhorse).to receive(:send_url).with( + an_instance_of(String), + hash_including(allowed_endpoints: custom_endpoints) + ).and_call_original + + subject + end + end + + context 'when local requests are not allowed' do + before do + allow(Gitlab).to receive(:dev_or_test_env?).and_return(false) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + end + + it 'sets allow_localhost to false' do + expect(Gitlab::Workhorse).to receive(:send_url).with( + an_instance_of(String), + hash_including(allow_localhost: false) + ).and_call_original + + subject + end + end + + context 'when generic_package_registry_ssrf_protection is disabled' do + before do + stub_feature_flags(generic_package_registry_ssrf_protection: false) + end + + it 'does not pass SSRF protection parameters' do + expect(Gitlab::Workhorse).to receive(:send_url).and_wrap_original do |method, *args| + expect(args.first).to be_an_instance_of(String) + expect(args.last).not_to include(:ssrf_filter, :allow_localhost, :allowed_endpoints) if args.last.is_a?(Hash) + + method.call(*args) + end + + subject + end + end +end