Merge branch 'security-fix_jira_ssrf_vulnerability-12-2' into '12-2-stable'

Fix DNS rebind vulnerability for JIRA integration

See merge request gitlab/gitlabhq!3338

4 files changed, 82 insertions, 1 deletions

diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index d08fcd8954d..0728c83005e 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -64,7 +64,12 @@ class JiraService < IssueTrackerService
   end
 
   def client
-    @client ||= JIRA::Client.new(options)
+    @client ||= begin
+      JIRA::Client.new(options).tap do |client|
+        # Replaces JIRA default http client with our implementation
+        client.request_client = Gitlab::Jira::HttpClient.new(client.options)
+      end
+    end
   end
 
   def help
diff --git a/changelogs/unreleased/security-fix_jira_ssrf_vulnerability.yml b/changelogs/unreleased/security-fix_jira_ssrf_vulnerability.yml
new file mode 100644
index 00000000000..25518dd2d05
--- /dev/null
+++ b/changelogs/unreleased/security-fix_jira_ssrf_vulnerability.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent DNS rebind on JIRA service integration
+merge_request:
+author:
+type: security
diff --git a/lib/gitlab/jira/http_client.rb b/lib/gitlab/jira/http_client.rb
new file mode 100644
index 00000000000..11a33a7b358
--- /dev/null
+++ b/lib/gitlab/jira/http_client.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Jira
+    # Gitlab JIRA HTTP client to be used with jira-ruby gem, this subclasses JIRA::HTTPClient.
+    # Uses Gitlab::HTTP to make requests to JIRA REST API.
+    # The parent class implementation can be found at: https://github.com/sumoheavy/jira-ruby/blob/v1.4.0/lib/jira/http_client.rb
+    class HttpClient < JIRA::HttpClient
+      extend ::Gitlab::Utils::Override
+
+      override :request
+      def request(*args)
+        result = make_request(*args)
+
+        raise JIRA::HTTPError.new(result) unless result.response.is_a?(Net::HTTPSuccess)
+
+        result
+      end
+
+      override :make_cookie_auth_request
+      def make_cookie_auth_request
+        body = {
+          username: @options.delete(:username),
+          password: @options.delete(:password)
+        }.to_json
+
+        make_request(:post, @options[:context_path] + '/rest/auth/1/session', body, { 'Content-Type' => 'application/json' })
+      end
+
+      override :make_request
+      def make_request(http_method, path, body = '', headers = {})
+        request_params = { headers: headers }
+        request_params[:body] = body if body.present?
+        request_params[:headers][:Cookie] = get_cookies if options[:use_cookies]
+        request_params[:timeout] = options[:read_timeout] if options[:read_timeout]
+        request_params[:base_uri] = uri.to_s
+        request_params.merge!(auth_params)
+
+        result = Gitlab::HTTP.public_send(http_method, path, **request_params) # rubocop:disable GitlabSecurity/PublicSend
+        @authenticated = result.response.is_a?(Net::HTTPOK)
+        store_cookies(result) if options[:use_cookies]
+
+        result
+      end
+
+      def auth_params
+        return {} unless @options[:username] && @options[:password]
+
+        {
+          basic_auth: {
+            username: @options[:username],
+            password: @options[:password]
+          }
+        }
+      end
+
+      private
+
+      def get_cookies
+        cookie_array = @cookies.values.map { |cookie| "#{cookie.name}=#{cookie.value[0]}" }
+        cookie_array += Array(@options[:additional_cookies]) if @options.key?(:additional_cookies)
+        cookie_array.join('; ') if cookie_array.any?
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index 68eabce8513..22ae65ea2fb 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -11,6 +11,7 @@ describe Projects::ServicesController do
   before do
     sign_in(user)
     project.add_maintainer(user)
+    allow(Gitlab::UrlBlocker).to receive(:validate!).and_return([URI.parse('http://example.com'), nil])
   end
 
   describe '#test' do
@@ -56,6 +57,8 @@ describe Projects::ServicesController do
           stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
             .to_return(status: 200, body: '{}')
 
+          expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original
+
           put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
 
           expect(response.status).to eq(200)
@@ -66,6 +69,8 @@ describe Projects::ServicesController do
         stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
           .to_return(status: 200, body: '{}')
 
+        expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original
+
         put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
 
         expect(response.status).to eq(200)