diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-25 17:28:16 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-25 17:28:16 +0000 |
commit | c22ba132adda00c9910ab9155a39a82370813003 (patch) | |
tree | 2eb67e09b545c3e74bda5a6fd7828f69db41b8a1 | |
parent | 00553eb342317257e7d694f571f7cbd4c846d161 (diff) | |
download | gitlab-ce-c22ba132adda00c9910ab9155a39a82370813003.tar.gz |
Add latest changes from gitlab-org/security/gitlab@12-8-stable-ee
-rw-r--r-- | app/controllers/concerns/hotlink_interceptor.rb | 15 | ||||
-rw-r--r-- | app/controllers/projects/repositories_controller.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/security-repository-archive-hotlinking.yml | 5 | ||||
-rw-r--r-- | changelogs/unreleased/security-rf-vulnerability-metadata-fix.yml | 5 | ||||
-rw-r--r-- | lib/api/helpers.rb | 4 | ||||
-rw-r--r-- | lib/api/repositories.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/hotlinking_detector.rb | 52 | ||||
-rw-r--r-- | spec/controllers/projects/repositories_controller_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/hotlinking_detector_spec.rb | 75 | ||||
-rw-r--r-- | spec/requests/api/repositories_spec.rb | 12 | ||||
-rw-r--r-- | spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb | 87 | ||||
-rwxr-xr-x[-rw-r--r--] | vendor/gitignore/C++.gitignore | 0 | ||||
-rwxr-xr-x[-rw-r--r--] | vendor/gitignore/Java.gitignore | 0 |
13 files changed, 265 insertions, 0 deletions
diff --git a/app/controllers/concerns/hotlink_interceptor.rb b/app/controllers/concerns/hotlink_interceptor.rb new file mode 100644 index 00000000000..712a10eab98 --- /dev/null +++ b/app/controllers/concerns/hotlink_interceptor.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module HotlinkInterceptor + extend ActiveSupport::Concern + + def intercept_hotlinking! + return render_406 if Gitlab::HotlinkingDetector.intercept_hotlinking?(request) + end + + private + + def render_406 + head :not_acceptable + end +end diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index d0fb814948f..a97f123d8d7 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -3,11 +3,13 @@ class Projects::RepositoriesController < Projects::ApplicationController include ExtractsPath include StaticObjectExternalStorage + include HotlinkInterceptor prepend_before_action(only: [:archive]) { authenticate_sessionless_user!(:archive) } # Authorize before_action :require_non_empty_project, except: :create + before_action :intercept_hotlinking!, only: :archive before_action :assign_archive_vars, only: :archive before_action :assign_append_sha, only: :archive before_action :authorize_download_code! diff --git a/changelogs/unreleased/security-repository-archive-hotlinking.yml b/changelogs/unreleased/security-repository-archive-hotlinking.yml new file mode 100644 index 00000000000..cf87ea488f0 --- /dev/null +++ b/changelogs/unreleased/security-repository-archive-hotlinking.yml @@ -0,0 +1,5 @@ +--- +title: Block hotlinking to repository archives +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-rf-vulnerability-metadata-fix.yml b/changelogs/unreleased/security-rf-vulnerability-metadata-fix.yml new file mode 100644 index 00000000000..5de5fc761fd --- /dev/null +++ b/changelogs/unreleased/security-rf-vulnerability-metadata-fix.yml @@ -0,0 +1,5 @@ +--- +title: vulnerability_feedback records should be restricted to a dev role and above +merge_request: +author: +type: security diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 001fb92ec52..4b51bbff804 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -359,6 +359,10 @@ module API render_api_error!('405 Method Not Allowed', 405) end + def not_acceptable! + render_api_error!('406 Not Acceptable', 406) + end + def conflict!(message = nil) render_api_error!(message || '409 Conflict', 409) end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 00473db1ff1..9953d3138f5 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -89,6 +89,8 @@ module API optional :format, type: String, desc: 'The archive format' end get ':id/repository/archive', requirements: { format: Gitlab::PathRegex.archive_formats_regex } do + not_acceptable! if Gitlab::HotlinkingDetector.intercept_hotlinking?(request) + send_git_archive user_project.repository, ref: params[:sha], format: params[:format], append_sha: true rescue not_found!('File') diff --git a/lib/gitlab/hotlinking_detector.rb b/lib/gitlab/hotlinking_detector.rb new file mode 100644 index 00000000000..44901297870 --- /dev/null +++ b/lib/gitlab/hotlinking_detector.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Gitlab + class HotlinkingDetector + IMAGE_FORMATS = %w(image/jpeg image/apng image/png image/webp image/svg+xml image/*).freeze + MEDIA_FORMATS = %w(video/webm video/ogg video/* application/ogg audio/webm audio/ogg audio/wav audio/*).freeze + CSS_FORMATS = %w(text/css).freeze + INVALID_FORMATS = (IMAGE_FORMATS + MEDIA_FORMATS + CSS_FORMATS).freeze + INVALID_FETCH_MODES = %w(cors no-cors websocket).freeze + + class << self + def intercept_hotlinking?(request) + request_accepts = parse_request_accepts(request) + + return false unless Feature.enabled?(:repository_archive_hotlinking_interception, default_enabled: true) + + # Block attempts to embed as JS + return true if sec_fetch_invalid?(request) + + # If no Accept header was set, skip the rest + return false if request_accepts.empty? + + # Workaround for IE8 weirdness + return false if IMAGE_FORMATS.include?(request_accepts.first) && request_accepts.include?("application/x-ms-application") + + # Block all other media requests if the first format is a media type + return true if INVALID_FORMATS.include?(request_accepts.first) + + false + end + + private + + def sec_fetch_invalid?(request) + fetch_mode = request.headers["Sec-Fetch-Mode"] + + return if fetch_mode.blank? + return true if INVALID_FETCH_MODES.include?(fetch_mode) + end + + def parse_request_accepts(request) + # Rails will already have parsed the Accept header + return request.accepts if request.respond_to?(:accepts) + + # Grape doesn't parse it, so we can use the Rails system for this + return Mime::Type.parse(request.headers["Accept"]) if request.respond_to?(:headers) && request.headers["Accept"].present? + + [] + end + end + end +end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index d4a81f24d9c..820ac594b26 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -24,6 +24,12 @@ describe Projects::RepositoriesController do sign_in(user) end + it_behaves_like "hotlink interceptor" do + let(:http_request) do + get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" + end + end + it "uses Gitlab::Workhorse" do get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master" }, format: "zip" diff --git a/spec/lib/gitlab/hotlinking_detector_spec.rb b/spec/lib/gitlab/hotlinking_detector_spec.rb new file mode 100644 index 00000000000..536d744c197 --- /dev/null +++ b/spec/lib/gitlab/hotlinking_detector_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::HotlinkingDetector do + describe ".intercept_hotlinking?" do + using RSpec::Parameterized::TableSyntax + + subject { described_class.intercept_hotlinking?(request) } + + let(:request) { double("request", headers: headers) } + let(:headers) { {} } + + context "hotlinked as media" do + where(:return_value, :accept_header) do + # These are default formats in modern browsers, and IE + false | "*/*" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + false | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + false | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*" + false | "text/html, application/xhtml+xml, image/jxr, */*" + false | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + + # These are image request formats + true | "image/webp,*/*" + true | "image/png,image/*;q=0.8,*/*;q=0.5" + true | "image/webp,image/apng,image/*,*/*;q=0.8" + true | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5" + + # Video request formats + true | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5" + + # Audio request formats + true | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5" + + # CSS request formats + true | "text/css,*/*;q=0.1" + true | "text/css" + true | "text/css,*/*;q=0.1" + end + + with_them do + let(:headers) do + { "Accept" => accept_header } + end + + it { is_expected.to be(return_value) } + end + end + + context "hotlinked as a script" do + where(:return_value, :fetch_mode) do + # Standard navigation fetch modes + false | "navigate" + false | "nested-navigate" + false | "same-origin" + + # Fetch modes when linking as JS + true | "cors" + true | "no-cors" + true | "websocket" + end + + with_them do + let(:headers) do + { "Sec-Fetch-Mode" => fetch_mode } + end + + it { is_expected.to be(return_value) } + end + end + end +end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 8bca458bece..cea08aa8767 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -263,6 +263,18 @@ describe API::Repositories do let(:message) { '404 File Not Found' } end end + + context "when hotlinking detection is enabled" do + before do + Feature.enable(:repository_archive_hotlinking_interception) + end + + it_behaves_like "hotlink interceptor" do + let(:http_request) do + get api(route, current_user), headers: headers + end + end + end end context 'when unauthenticated', 'and project is public' do diff --git a/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb new file mode 100644 index 00000000000..93a394387a3 --- /dev/null +++ b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +RSpec.shared_examples "hotlink interceptor" do + let(:http_request) { nil } + let(:headers) { nil } + + describe "DDOS prevention" do + using RSpec::Parameterized::TableSyntax + + context "hotlinked as media" do + where(:response_status, :accept_header) do + # These are default formats in modern browsers, and IE + :ok | "*/*" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" + :ok | "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8" + :ok | "image/jpeg, application/x-ms-application, image/gif, application/xaml+xml, image/pjpeg, application/x-ms-xbap, application/x-shockwave-flash, application/msword, */*" + :ok | "text/html, application/xhtml+xml, image/jxr, */*" + :ok | "text/html, application/xml;q=0.9, application/xhtml+xml, image/png, image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1" + + # These are image request formats + :not_acceptable | "image/webp,*/*" + :not_acceptable | "image/png,image/*;q=0.8,*/*;q=0.5" + :not_acceptable | "image/webp,image/apng,image/*,*/*;q=0.8" + :not_acceptable | "image/png,image/svg+xml,image/*;q=0.8, */*;q=0.5" + + # Video request formats + :not_acceptable | "video/webm,video/ogg,video/*;q=0.9,application/ogg;q=0.7,audio/*;q=0.6,*/*;q=0.5" + + # Audio request formats + :not_acceptable | "audio/webm,audio/ogg,audio/wav,audio/*;q=0.9,application/ogg;q=0.7,video/*;q=0.6,*/*;q=0.5" + + # CSS request formats + :not_acceptable | "text/css,*/*;q=0.1" + :not_acceptable | "text/css" + :not_acceptable | "text/css,*/*;q=0.1" + end + + with_them do + let(:headers) do + { "Accept" => accept_header } + end + + before do + request.headers.merge!(headers) if request.present? + end + + it "renders the response" do + http_request + + expect(response).to have_gitlab_http_status(response_status) + end + end + end + + context "hotlinked as a script" do + where(:response_status, :fetch_mode) do + # Standard navigation fetch modes + :ok | "navigate" + :ok | "nested-navigate" + :ok | "same-origin" + + # Fetch modes when linking as JS + :not_acceptable | "cors" + :not_acceptable | "no-cors" + :not_acceptable | "websocket" + end + + with_them do + let(:headers) do + { "Sec-Fetch-Mode" => fetch_mode } + end + + before do + request.headers.merge!(headers) if request.present? + end + + it "renders the response" do + http_request + + expect(response).to have_gitlab_http_status(response_status) + end + end + end + end +end diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore index 259148fa18f..259148fa18f 100644..100755 --- a/vendor/gitignore/C++.gitignore +++ b/vendor/gitignore/C++.gitignore diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore index a1c2a238a96..a1c2a238a96 100644..100755 --- a/vendor/gitignore/Java.gitignore +++ b/vendor/gitignore/Java.gitignore |