Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee

12 files changed, 74 insertions, 13 deletions

diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb
index 82111c7322e..5f43eae1e8f 100644
--- a/app/models/integrations/bamboo.rb
+++ b/app/models/integrations/bamboo.rb
@@ -173,6 +173,7 @@ module Integrations
 
       query_params[:os_authType] = 'basic'
       params[:basic_auth] = basic_auth
+      params[:use_read_total_timeout] = true
       params
     end
 
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index ab1ba768a8f..581bd8077ad 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -50,9 +50,11 @@ class DroneCiService < CiService
   end
 
   def calculate_reactive_cache(sha, ref)
-    response = Gitlab::HTTP.try_get(commit_status_path(sha, ref),
+    response = Gitlab::HTTP.try_get(
+      commit_status_path(sha, ref),
       verify: enable_ssl_verification,
-      extra_log_info: { project_id: project_id })
+      extra_log_info: { project_id: project_id },
+      use_read_total_timeout: true)
 
     status =
       if response && response.code == 200 && response['status']
diff --git a/app/models/project_services/external_wiki_service.rb b/app/models/project_services/external_wiki_service.rb
index f49b008533d..6798b8fb695 100644
--- a/app/models/project_services/external_wiki_service.rb
+++ b/app/models/project_services/external_wiki_service.rb
@@ -38,7 +38,7 @@ class ExternalWikiService < Integration
   end
 
   def execute(_data)
-    response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true)
+    response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true, use_read_total_timeout: true)
     response.body if response.code == 200
   rescue StandardError
     nil
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index 099e3c336dd..ba78b1a8dc3 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -106,7 +106,7 @@ class IssueTrackerService < Integration
     result = false
 
     begin
-      response = Gitlab::HTTP.head(self.project_url, verify: true)
+      response = Gitlab::HTTP.head(self.project_url, verify: true, use_read_total_timeout: true)
 
       if response
         message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}"
diff --git a/app/models/project_services/mock_ci_service.rb b/app/models/project_services/mock_ci_service.rb
index bd6344c6e1a..ebe6179d742 100644
--- a/app/models/project_services/mock_ci_service.rb
+++ b/app/models/project_services/mock_ci_service.rb
@@ -56,7 +56,7 @@ class MockCiService < CiService
   #
   #
   def commit_status(sha, ref)
-    response = Gitlab::HTTP.get(commit_status_path(sha), verify: false)
+    response = Gitlab::HTTP.get(commit_status_path(sha), verify: false, use_read_total_timeout: true)
     read_commit_status(response)
   rescue Errno::ECONNREFUSED
     :error
diff --git a/app/models/project_services/slack_mattermost/notifier.rb b/app/models/project_services/slack_mattermost/notifier.rb
index 1a78cea5933..39ee5718248 100644
--- a/app/models/project_services/slack_mattermost/notifier.rb
+++ b/app/models/project_services/slack_mattermost/notifier.rb
@@ -17,7 +17,7 @@ module SlackMattermost
     class HTTPClient
       def self.post(uri, params = {})
         params.delete(:http_options) # these are internal to the client and we do not want them
-        Gitlab::HTTP.post(uri, body: params)
+        Gitlab::HTTP.post(uri, body: params, use_read_total_timeout: true)
       end
     end
   end
diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb
index 6fc24a4778c..c3390dd9c25 100644
--- a/app/models/project_services/teamcity_service.rb
+++ b/app/models/project_services/teamcity_service.rb
@@ -169,7 +169,7 @@ class TeamcityService < CiService
   end
 
   def get_path(path)
-    Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id })
+    Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id }, use_read_total_timeout: true)
   end
 
   def post_to_build_queue(data, branch)
@@ -179,7 +179,8 @@ class TeamcityService < CiService
             "<buildType id=#{build_type.encode(xml: :attr)}/>"\
             '</build>',
       headers: { 'Content-type' => 'application/xml' },
-      basic_auth: basic_auth
+      basic_auth: basic_auth,
+      use_read_total_timeout: true
     )
   end
 
diff --git a/app/models/project_services/unify_circuit_service.rb b/app/models/project_services/unify_circuit_service.rb
index 5f43388e1c9..f4e93cf8448 100644
--- a/app/models/project_services/unify_circuit_service.rb
+++ b/app/models/project_services/unify_circuit_service.rb
@@ -48,7 +48,8 @@ class UnifyCircuitService < ChatNotificationService
     response = Gitlab::HTTP.post(webhook, body: {
       subject: message.project_name,
       text: message.summary,
-      markdown: true
+      markdown: true,
+      use_read_total_timeout: true
     }.to_json)
 
     response if response.success?
diff --git a/app/models/project_services/webex_teams_service.rb b/app/models/project_services/webex_teams_service.rb
index 3d92d3bb85e..37f8a1af6f1 100644
--- a/app/models/project_services/webex_teams_service.rb
+++ b/app/models/project_services/webex_teams_service.rb
@@ -43,7 +43,7 @@ class WebexTeamsService < ChatNotificationService
 
   def notify(message, opts)
     header = { 'Content-Type' => 'application/json' }
-    response = Gitlab::HTTP.post(webhook, headers: header, body: { markdown: message.summary }.to_json)
+    response = Gitlab::HTTP.post(webhook, headers: header, body: { markdown: message.summary }.to_json, use_read_total_timeout: true)
 
     response if response.success?
   end
diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb
index 654d9356739..c2f12c133c1 100644
--- a/app/services/web_hook_service.rb
+++ b/app/services/web_hook_service.rb
@@ -41,6 +41,7 @@ class WebHookService
     @hook_name = hook_name.to_s
     @request_options = {
       timeout: Gitlab.config.gitlab.webhook_timeout,
+      use_read_total_timeout: true,
       allow_local_requests: hook.allow_local_requests?
     }
   end
@@ -67,7 +68,7 @@ class WebHookService
     {
       status: :success,
       http_status: response.code,
-      message: response.to_s
+      message: response.body
     }
   rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH,
          Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep,
diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb
index be87dcc0ff9..7e45cd216f5 100644
--- a/lib/gitlab/http.rb
+++ b/lib/gitlab/http.rb
@@ -8,9 +8,10 @@ module Gitlab
   class HTTP
     BlockedUrlError = Class.new(StandardError)
     RedirectionTooDeep = Class.new(StandardError)
+    ReadTotalTimeout = Class.new(Net::ReadTimeout)
 
     HTTP_TIMEOUT_ERRORS = [
-      Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout
+      Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Gitlab::HTTP::ReadTotalTimeout
     ].freeze
     HTTP_ERRORS = HTTP_TIMEOUT_ERRORS + [
       SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError,
@@ -23,6 +24,7 @@ module Gitlab
       read_timeout: 20,
       write_timeout: 30
     }.freeze
+    DEFAULT_READ_TOTAL_TIMEOUT = 20.seconds
 
     include HTTParty # rubocop:disable Gitlab/HTTParty
 
@@ -41,7 +43,19 @@ module Gitlab
           options
         end
 
-      httparty_perform_request(http_method, path, options_with_timeouts, &block)
+      unless options.has_key?(:use_read_total_timeout)
+        return httparty_perform_request(http_method, path, options_with_timeouts, &block)
+      end
+
+      start_time = Gitlab::Metrics::System.monotonic_time
+      read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT)
+
+      httparty_perform_request(http_method, path, options_with_timeouts) do |fragment|
+        elapsed = Gitlab::Metrics::System.monotonic_time - start_time
+        raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout
+
+        block.call fragment if block
+      end
     rescue HTTParty::RedirectionTooDeep
       raise RedirectionTooDeep
     rescue *HTTP_ERRORS => e
diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb
index 308f7f46251..71e80de9f89 100644
--- a/spec/lib/gitlab/http_spec.rb
+++ b/spec/lib/gitlab/http_spec.rb
@@ -27,6 +27,47 @@ RSpec.describe Gitlab::HTTP do
     end
   end
 
+  context 'when reading the response is too slow' do
+    before do
+      stub_const("#{described_class}::DEFAULT_READ_TOTAL_TIMEOUT", 0.001.seconds)
+
+      WebMock.stub_request(:post, /.*/).to_return do |request|
+        sleep 0.002.seconds
+        { body: 'I\m slow', status: 200 }
+      end
+    end
+
+    let(:options) { {} }
+
+    subject(:request_slow_responder) { described_class.post('http://example.org', **options) }
+
+    specify do
+      expect { request_slow_responder }.not_to raise_error
+    end
+
+    context 'with use_read_total_timeout option' do
+      let(:options) { { use_read_total_timeout: true } }
+
+      it 'raises a timeout error' do
+        expect { request_slow_responder }.to raise_error(Gitlab::HTTP::ReadTotalTimeout, /Request timed out after ?([0-9]*[.])?[0-9]+ seconds/)
+      end
+
+      context 'and timeout option' do
+        let(:options) { { use_read_total_timeout: true, timeout: 10.seconds } }
+
+        it 'overrides the default timeout when timeout option is present' do
+          expect { request_slow_responder }.not_to raise_error
+        end
+      end
+    end
+  end
+
+  it 'calls a block' do
+    WebMock.stub_request(:post, /.*/)
+
+    expect { |b| described_class.post('http://example.org', &b) }.to yield_with_args
+  end
+
   describe 'allow_local_requests_from_web_hooks_and_services is' do
     before do
       WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success')