Commit e0826b0c authored by Thong Kuah's avatar Thong Kuah

Override hostname when connecting via Kubeclient

Kubeclient uses rest-client. We hack into to access the net/http object
so that we can patch to connect to the resolved IP + set
hostname_override.

Add specs for discord. The discord integration also uses rest-client, so
since we patched rest-client, spec that the DNS rebinding protection
works
parent 80c57bf6
---
title: Fix SSRF via DNS rebinding in Kubernetes Integration
merge_request:
author:
type: security
# frozen_string_literal: true
module RestClient
class Request
attr_accessor :hostname_override
module UrlBlocker
def transmit(uri, req, payload, &block)
begin
ip, hostname_override = Gitlab::UrlBlocker.validate!(uri, allow_local_network: allow_settings_local_requests?,
allow_localhost: allow_settings_local_requests?,
dns_rebind_protection: dns_rebind_protection?)
self.hostname_override = hostname_override
rescue Gitlab::UrlBlocker::BlockedUrlError => e
raise ArgumentError, "URL '#{uri}' is blocked: #{e.message}"
end
# Gitlab::UrlBlocker returns a Addressable::URI which we need to coerce
# to URI so that rest-client can use it to determine if it's a
# URI::HTTPS or not. It uses it to set `net.use_ssl` to true or not:
#
# https://github.com/rest-client/rest-client/blob/f450a0f086f1cd1049abbef2a2c66166a1a9ba71/lib/restclient/request.rb#L656
ip_as_uri = URI.parse(ip)
super(ip_as_uri, req, payload, &block)
end
def net_http_object(hostname, port)
super.tap do |http|
http.hostname_override = hostname_override if hostname_override
end
end
private
def dns_rebind_protection?
return false if Gitlab.http_proxy_env?
Gitlab::CurrentSettings.dns_rebinding_protection_enabled?
end
def allow_settings_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
end
end
prepend UrlBlocker
end
end
require 'spec_helper'
describe 'rest-client dns rebinding protection' do
include StubRequests
context 'when local requests are not allowed' do
it 'allows an external request with http' do
request_stub = stub_full_request('http://example.com', ip_address: '93.184.216.34')
RestClient.get('http://example.com/')
expect(request_stub).to have_been_requested
end
it 'allows an external request with https' do
request_stub = stub_full_request('https://example.com', ip_address: '93.184.216.34')
RestClient.get('https://example.com/')
expect(request_stub).to have_been_requested
end
it 'raises error when it is a request that resolves to a local address' do
stub_full_request('https://example.com', ip_address: '172.16.0.0')
expect { RestClient.get('https://example.com') }
.to raise_error(ArgumentError,
"URL 'https://example.com' is blocked: Requests to the local network are not allowed")
end
it 'raises error when it is a request that resolves to a localhost address' do
stub_full_request('https://example.com', ip_address: '127.0.0.1')
expect { RestClient.get('https://example.com') }
.to raise_error(ArgumentError,
"URL 'https://example.com' is blocked: Requests to localhost are not allowed")
end
it 'raises error when it is a request to local address' do
expect { RestClient.get('http://172.16.0.0') }
.to raise_error(ArgumentError,
"URL 'http://172.16.0.0' is blocked: Requests to the local network are not allowed")
end
it 'raises error when it is a request to localhost address' do
expect { RestClient.get('http://127.0.0.1') }
.to raise_error(ArgumentError,
"URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
end
end
context 'when port different from URL scheme is used' do
it 'allows the request' do
request_stub = stub_full_request('https://example.com:8080', ip_address: '93.184.216.34')
RestClient.get('https://example.com:8080/')
expect(request_stub).to have_been_requested
end
it 'raises error when it is a request to local address' do
expect { RestClient.get('https://172.16.0.0:8080') }
.to raise_error(ArgumentError,
"URL 'https://172.16.0.0:8080' is blocked: Requests to the local network are not allowed")
end
it 'raises error when it is a request to localhost address' do
expect { RestClient.get('https://127.0.0.1:8080') }
.to raise_error(ArgumentError,
"URL 'https://127.0.0.1:8080' is blocked: Requests to localhost are not allowed")
end
end
context 'when DNS rebinding protection is disabled' do
before do
stub_application_setting(dns_rebinding_protection_enabled: false)
end
it 'allows the request' do
request_stub = stub_request(:get, 'https://example.com')
RestClient.get('https://example.com/')
expect(request_stub).to have_been_requested
end
end
context 'when http(s) proxy environment variable is set' do
before do
stub_env('https_proxy' => 'https://my.proxy')
end
it 'allows the request' do
request_stub = stub_request(:get, 'https://example.com')
RestClient.get('https://example.com/')
expect(request_stub).to have_been_requested
end
end
context 'when local requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
end
it 'allows an external request' do
request_stub = stub_full_request('https://example.com', ip_address: '93.184.216.34')
RestClient.get('https://example.com/')
expect(request_stub).to have_been_requested
end
it 'allows an external request that resolves to a local address' do
request_stub = stub_full_request('https://example.com', ip_address: '172.16.0.0')
RestClient.get('https://example.com/')
expect(request_stub).to have_been_requested
end
it 'allows an external request that resolves to a localhost address' do
request_stub = stub_full_request('https://example.com', ip_address: '127.0.0.1')
RestClient.get('https://example.com/')
expect(request_stub).to have_been_requested
end
it 'allows a local address request' do
request_stub = stub_request(:get, 'http://172.16.0.0')
RestClient.get('http://172.16.0.0')
expect(request_stub).to have_been_requested
end
it 'allows a localhost address request' do
request_stub = stub_request(:get, 'http://127.0.0.1')
RestClient.get('http://127.0.0.1')
expect(request_stub).to have_been_requested
end
end
end
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
require 'spec_helper' require 'spec_helper'
describe Gitlab::Kubernetes::KubeClient do describe Gitlab::Kubernetes::KubeClient do
include StubRequests
include KubernetesHelpers include KubernetesHelpers
let(:api_url) { 'https://kubernetes.example.com/prefix' } let(:api_url) { 'https://kubernetes.example.com/prefix' }
...@@ -14,6 +15,17 @@ describe Gitlab::Kubernetes::KubeClient do ...@@ -14,6 +15,17 @@ describe Gitlab::Kubernetes::KubeClient do
stub_kubeclient_discover(api_url) stub_kubeclient_discover(api_url)
end end
def method_call(client, method_name)
case method_name
when /\A(get_|delete_)/
client.public_send(method_name)
when /\A(create_|update_)/
client.public_send(method_name, {})
else
raise "Unknown method name #{method_name}"
end
end
shared_examples 'a Kubeclient' do shared_examples 'a Kubeclient' do
it 'is a Kubeclient::Client' do it 'is a Kubeclient::Client' do
is_expected.to be_an_instance_of Kubeclient::Client is_expected.to be_an_instance_of Kubeclient::Client
...@@ -25,28 +37,30 @@ describe Gitlab::Kubernetes::KubeClient do ...@@ -25,28 +37,30 @@ describe Gitlab::Kubernetes::KubeClient do
end end
shared_examples 'redirection not allowed' do |method_name| shared_examples 'redirection not allowed' do |method_name|
before do context 'api_url is redirected' do
redirect_url = 'https://not-under-our-control.example.com/api/v1/pods' before do
redirect_url = 'https://not-under-our-control.example.com/api/v1/pods'
stub_request(:get, %r{\A#{api_url}/}) stub_request(:get, %r{\A#{api_url}/})
.to_return(status: 302, headers: { location: redirect_url }) .to_return(status: 302, headers: { location: redirect_url })
stub_request(:get, redirect_url) stub_request(:get, redirect_url)
.to_return(status: 200, body: '{}') .to_return(status: 200, body: '{}')
end end
it 'does not follow redirects' do it 'does not follow redirects' do
method_call = -> do expect { method_call(client, method_name) }.to raise_error(Kubeclient::HttpError)
case method_name
when /\A(get_|delete_)/
client.public_send(method_name)
when /\A(create_|update_)/
client.public_send(method_name, {})
else
raise "Unknown method name #{method_name}"
end
end end
expect { method_call.call }.to raise_error(Kubeclient::HttpError) end
end
shared_examples 'dns rebinding not allowed' do |method_name|
it 'does not allow DNS rebinding' do
stub_dns(api_url, ip_address: '8.8.8.8')
client
stub_dns(api_url, ip_address: '192.168.2.120')
expect { method_call(client, method_name) }.to raise_error(ArgumentError, /is blocked/)
end end
end end
...@@ -160,6 +174,7 @@ describe Gitlab::Kubernetes::KubeClient do ...@@ -160,6 +174,7 @@ describe Gitlab::Kubernetes::KubeClient do
].each do |method| ].each do |method|
describe "##{method}" do describe "##{method}" do
include_examples 'redirection not allowed', method include_examples 'redirection not allowed', method
include_examples 'dns rebinding not allowed', method
it 'delegates to the core client' do it 'delegates to the core client' do
expect(client).to delegate_method(method).to(:core_client) expect(client).to delegate_method(method).to(:core_client)
...@@ -185,6 +200,7 @@ describe Gitlab::Kubernetes::KubeClient do ...@@ -185,6 +200,7 @@ describe Gitlab::Kubernetes::KubeClient do
].each do |method| ].each do |method|
describe "##{method}" do describe "##{method}" do
include_examples 'redirection not allowed', method include_examples 'redirection not allowed', method
include_examples 'dns rebinding not allowed', method
it 'delegates to the rbac client' do it 'delegates to the rbac client' do
expect(client).to delegate_method(method).to(:rbac_client) expect(client).to delegate_method(method).to(:rbac_client)
...@@ -203,6 +219,7 @@ describe Gitlab::Kubernetes::KubeClient do ...@@ -203,6 +219,7 @@ describe Gitlab::Kubernetes::KubeClient do
describe '#get_deployments' do describe '#get_deployments' do
include_examples 'redirection not allowed', 'get_deployments' include_examples 'redirection not allowed', 'get_deployments'
include_examples 'dns rebinding not allowed', 'get_deployments'
it 'delegates to the extensions client' do it 'delegates to the extensions client' do
expect(client).to delegate_method(:get_deployments).to(:extensions_client) expect(client).to delegate_method(:get_deployments).to(:extensions_client)
......
...@@ -8,4 +8,37 @@ describe DiscordService do ...@@ -8,4 +8,37 @@ describe DiscordService do
let(:client_arguments) { { url: webhook_url } } let(:client_arguments) { { url: webhook_url } }
let(:content_key) { :content } let(:content_key) { :content }
end end
describe '#execute' do
include StubRequests
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:webhook_url) { "https://example.gitlab.com/" }
let(:sample_data) do
Gitlab::DataBuilder::Push.build_sample(project, user)
end
before do
allow(subject).to receive_messages(
project: project,
project_id: project.id,
service_hook: true,
webhook: webhook_url
)
WebMock.stub_request(:post, webhook_url)
end
context 'DNS rebind to local address' do
before do
stub_dns(webhook_url, ip_address: '192.168.2.120')
end
it 'does not allow DNS rebinding' do
expect { subject.execute(sample_data) }.to raise_error(ArgumentError, /is blocked/)
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment