Allow not resolvable urls when rebinding setting is disabled
Now, when the dns rebinging setting is disabled, we will allow urls that are not resolvable.
This commit is contained in:
parent
5512dc23de
commit
b4ea71f9ed
3 changed files with 85 additions and 45 deletions
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Allow not resolvable urls when dns rebind protection is disabled
|
||||||
|
merge_request: 32523
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -49,7 +49,7 @@ module Gitlab
|
||||||
hostname = uri.hostname
|
hostname = uri.hostname
|
||||||
port = get_port(uri)
|
port = get_port(uri)
|
||||||
|
|
||||||
address_info = get_address_info(hostname, port)
|
address_info = get_address_info(hostname, port, dns_rebind_protection)
|
||||||
return [uri, nil] unless address_info
|
return [uri, nil] unless address_info
|
||||||
|
|
||||||
ip_address = ip_address(address_info)
|
ip_address = ip_address(address_info)
|
||||||
|
@ -110,11 +110,15 @@ module Gitlab
|
||||||
validate_unicode_restriction(uri) if ascii_only
|
validate_unicode_restriction(uri) if ascii_only
|
||||||
end
|
end
|
||||||
|
|
||||||
def get_address_info(hostname, port)
|
def get_address_info(hostname, port, dns_rebind_protection)
|
||||||
Addrinfo.getaddrinfo(hostname, port, nil, :STREAM).map do |addr|
|
Addrinfo.getaddrinfo(hostname, port, nil, :STREAM).map do |addr|
|
||||||
addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
|
addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
|
||||||
end
|
end
|
||||||
rescue SocketError
|
rescue SocketError
|
||||||
|
# If the dns rebinding protection is not enabled, we allow
|
||||||
|
# urls that can't be resolved at this point.
|
||||||
|
return unless dns_rebind_protection
|
||||||
|
|
||||||
# In the test suite we use a lot of mocked urls that are either invalid or
|
# In the test suite we use a lot of mocked urls that are either invalid or
|
||||||
# don't exist. In order to avoid modifying a ton of tests and factories
|
# don't exist. In order to avoid modifying a ton of tests and factories
|
||||||
# we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS
|
# we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS
|
||||||
|
|
|
@ -4,80 +4,59 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
describe Gitlab::UrlBlocker do
|
describe Gitlab::UrlBlocker do
|
||||||
|
include StubRequests
|
||||||
|
|
||||||
describe '#validate!' do
|
describe '#validate!' do
|
||||||
|
subject { described_class.validate!(import_url) }
|
||||||
|
|
||||||
|
shared_examples 'validates URI and hostname' do
|
||||||
|
it 'runs the url validations' do
|
||||||
|
uri, hostname = subject
|
||||||
|
|
||||||
|
expect(uri).to eq(Addressable::URI.parse(expected_uri))
|
||||||
|
expect(hostname).to eq(expected_hostname)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'when URI is nil' do
|
context 'when URI is nil' do
|
||||||
let(:import_url) { nil }
|
let(:import_url) { nil }
|
||||||
|
|
||||||
it 'returns no URI and hostname' do
|
it_behaves_like 'validates URI and hostname' do
|
||||||
uri, hostname = described_class.validate!(import_url)
|
let(:expected_uri) { nil }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
expect(uri).to be(nil)
|
|
||||||
expect(hostname).to be(nil)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when URI is internal' do
|
context 'when URI is internal' do
|
||||||
let(:import_url) { 'http://localhost' }
|
let(:import_url) { 'http://localhost' }
|
||||||
|
|
||||||
it 'returns URI and no hostname' do
|
it_behaves_like 'validates URI and hostname' do
|
||||||
uri, hostname = described_class.validate!(import_url)
|
let(:expected_uri) { 'http://[::1]' }
|
||||||
|
let(:expected_hostname) { 'localhost' }
|
||||||
expect(uri).to eq(Addressable::URI.parse('http://[::1]'))
|
|
||||||
expect(hostname).to eq('localhost')
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when the URL hostname is a domain' do
|
context 'when the URL hostname is a domain' do
|
||||||
|
context 'when domain can be resolved' do
|
||||||
let(:import_url) { 'https://example.org' }
|
let(:import_url) { 'https://example.org' }
|
||||||
|
|
||||||
it 'returns URI and hostname' do
|
before do
|
||||||
uri, hostname = described_class.validate!(import_url)
|
stub_dns(import_url, ip_address: '93.184.216.34')
|
||||||
|
end
|
||||||
|
|
||||||
expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
|
it_behaves_like 'validates URI and hostname' do
|
||||||
expect(hostname).to eq('example.org')
|
let(:expected_uri) { 'https://93.184.216.34' }
|
||||||
|
let(:expected_hostname) { 'example.org' }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when the URL hostname is an IP address' do
|
context 'when domain cannot be resolved' do
|
||||||
let(:import_url) { 'https://93.184.216.34' }
|
|
||||||
|
|
||||||
it 'returns URI and no hostname' do
|
|
||||||
uri, hostname = described_class.validate!(import_url)
|
|
||||||
|
|
||||||
expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
|
|
||||||
expect(hostname).to be(nil)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'disabled DNS rebinding protection' do
|
|
||||||
context 'when URI is internal' do
|
|
||||||
let(:import_url) { 'http://localhost' }
|
|
||||||
|
|
||||||
it 'returns URI and no hostname' do
|
|
||||||
uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false)
|
|
||||||
|
|
||||||
expect(uri).to eq(Addressable::URI.parse('http://localhost'))
|
|
||||||
expect(hostname).to be(nil)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when the URL hostname is a domain' do
|
|
||||||
let(:import_url) { 'https://example.org' }
|
|
||||||
|
|
||||||
it 'returns URI and no hostname' do
|
|
||||||
uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false)
|
|
||||||
|
|
||||||
expect(uri).to eq(Addressable::URI.parse('https://example.org'))
|
|
||||||
expect(hostname).to eq(nil)
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when it cannot be resolved' do
|
|
||||||
let(:import_url) { 'http://foobar.x' }
|
let(:import_url) { 'http://foobar.x' }
|
||||||
|
|
||||||
it 'raises error' do
|
it 'raises an error' do
|
||||||
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
|
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
|
||||||
|
|
||||||
expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
|
expect { subject }.to raise_error(described_class::BlockedUrlError)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -85,20 +64,72 @@ describe Gitlab::UrlBlocker do
|
||||||
context 'when the URL hostname is an IP address' do
|
context 'when the URL hostname is an IP address' do
|
||||||
let(:import_url) { 'https://93.184.216.34' }
|
let(:import_url) { 'https://93.184.216.34' }
|
||||||
|
|
||||||
it 'returns URI and no hostname' do
|
it_behaves_like 'validates URI and hostname' do
|
||||||
uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false)
|
let(:expected_uri) { import_url }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
|
|
||||||
expect(hostname).to be(nil)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when it is invalid' do
|
context 'when the address is invalid' do
|
||||||
let(:import_url) { 'http://1.1.1.1.1' }
|
let(:import_url) { 'http://1.1.1.1.1' }
|
||||||
|
|
||||||
it 'raises an error' do
|
it 'raises an error' do
|
||||||
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
|
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
|
||||||
|
|
||||||
expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
|
expect { subject }.to raise_error(described_class::BlockedUrlError)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'disabled DNS rebinding protection' do
|
||||||
|
subject { described_class.validate!(import_url, dns_rebind_protection: false) }
|
||||||
|
|
||||||
|
context 'when URI is internal' do
|
||||||
|
let(:import_url) { 'http://localhost' }
|
||||||
|
|
||||||
|
it_behaves_like 'validates URI and hostname' do
|
||||||
|
let(:expected_uri) { import_url }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the URL hostname is a domain' do
|
||||||
|
let(:import_url) { 'https://example.org' }
|
||||||
|
|
||||||
|
before do
|
||||||
|
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when domain can be resolved' do
|
||||||
|
it_behaves_like 'validates URI and hostname' do
|
||||||
|
let(:expected_uri) { import_url }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when domain cannot be resolved' do
|
||||||
|
let(:import_url) { 'http://foobar.x' }
|
||||||
|
|
||||||
|
it_behaves_like 'validates URI and hostname' do
|
||||||
|
let(:expected_uri) { import_url }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the URL hostname is an IP address' do
|
||||||
|
let(:import_url) { 'https://93.184.216.34' }
|
||||||
|
|
||||||
|
it_behaves_like 'validates URI and hostname' do
|
||||||
|
let(:expected_uri) { import_url }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when it is invalid' do
|
||||||
|
let(:import_url) { 'http://1.1.1.1.1' }
|
||||||
|
|
||||||
|
it_behaves_like 'validates URI and hostname' do
|
||||||
|
let(:expected_uri) { import_url }
|
||||||
|
let(:expected_hostname) { nil }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue