Fix DNS rebind vulnerability for JIRA integration

Uses Gitlab::HTTP for JIRA requests instead of Net::Http.
Gitlab::Http comes with some built in SSRF protections.
This commit is contained in:
Felipe Artur 2019-07-16 16:49:47 -03:00
parent 1dfbb27f6e
commit 492a7e753d
4 changed files with 82 additions and 1 deletions

View file

@ -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

View file

@ -0,0 +1,5 @@
---
title: Prevent DNS rebind on JIRA service integration
merge_request:
author:
type: security

View file

@ -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

View file

@ -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)