Skip to content
代码片段 群组 项目
未验证 提交 e8190bd3 编辑于 作者: Stan Hu's avatar Stan Hu
浏览文件

Skip IP lookups in validating URLs on certain conditions

Previously anytime `ApplicationSetting` were updated `UrlBlocker`
would attempt to resolve the IPs and determine whether they were
allowed by the current settings. However, in an offline environment,
services like Diagrams.net may not be resolved even if they are
enabled by default.

This commit skips the validations that require resolving IP addresses
if there are no restrictions on outgoing requests:

- Allow requests to the local network is checked
- Allow requests to the local network from system hooks is checked
- DNS rebinding attack protection is disabled
- Block all requests, except for IP addresses, IP ranges, and domain
  names defined in the allowlist (`deny_all_requests_except_allowed`)
  is disabled

Note that the URL validators in `ApplicationSetting` only pass in the
current `deny_all_requests_except_allowed` setting, so if that is
active then IP resolution will occur.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/467524

Changelog: fixed
上级 89f6f99e
No related branches found
No related tags found
无相关合并请求
......@@ -20,3 +20,7 @@ RSpec/MultipleMemoizedHelpers:
RSpec/FilePath:
CustomTransform:
HTTP_V2: http_v2
Lint/BinaryOperatorWithIdenticalOperands:
Exclude:
- 'spec/**/*.rb'
......@@ -55,11 +55,14 @@ GEM
addressable (2.8.4)
public_suffix (>= 2.0.2, < 6.0)
ast (2.4.2)
binding_of_caller (1.0.1)
debug_inspector (>= 1.2.0)
builder (3.2.4)
concurrent-ruby (1.2.2)
crack (0.4.5)
rexml
crass (1.0.6)
debug_inspector (1.2.0)
diff-lcs (1.5.0)
erubi (1.12.0)
gitlab-styles (10.1.0)
......@@ -91,6 +94,10 @@ GEM
parser (3.2.2.3)
ast (~> 2.4.1)
racc
proc_to_ast (0.2.0)
parser
rouge
unparser
public_suffix (5.0.1)
racc (1.7.1)
rack (2.2.7)
......@@ -113,6 +120,7 @@ GEM
rake (13.0.6)
regexp_parser (2.8.1)
rexml (3.2.5)
rouge (4.3.0)
rspec (3.12.0)
rspec-core (~> 3.12.0)
rspec-expectations (~> 3.12.0)
......@@ -125,6 +133,17 @@ GEM
rspec-mocks (3.12.5)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.12.0)
rspec-parameterized (1.0.2)
rspec-parameterized-core (< 2)
rspec-parameterized-table_syntax (< 2)
rspec-parameterized-core (1.0.1)
parser
proc_to_ast (>= 0.2.0)
rspec (>= 2.13, < 4)
unparser
rspec-parameterized-table_syntax (1.0.1)
binding_of_caller
rspec-parameterized-core (< 2)
rspec-rails (6.0.3)
actionpack (>= 6.1)
activesupport (>= 6.1)
......@@ -168,6 +187,9 @@ GEM
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
unicode-display_width (2.4.2)
unparser (0.6.8)
diff-lcs (~> 1.3)
parser (>= 3.2.0)
uri (0.13.0)
webmock (3.18.1)
addressable (>= 2.8.0)
......@@ -184,6 +206,7 @@ DEPENDENCIES
gitlab-rspec!
gitlab-styles (~> 10.1.0)
gitlab-utils!
rspec-parameterized (~> 1.0.2)
rspec-rails (~> 6.0.3)
rubocop (~> 1.50.2)
rubocop-rspec (~> 2.22)
......
......@@ -28,6 +28,7 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency "net-http", "= 0.4.1"
spec.add_development_dependency 'gitlab-styles', '~> 10.1.0'
spec.add_development_dependency 'rspec-parameterized', '~> 1.0.2'
spec.add_development_dependency 'rspec-rails', '~> 6.0.3'
spec.add_development_dependency "rubocop", "~> 1.50.2"
spec.add_development_dependency "rubocop-rspec", "~> 2.22"
......
......@@ -81,6 +81,10 @@ def validate_url_with_proxy!(
ascii_only: ascii_only
)
unless deny_all_requests_except_allowed || dns_rebind_protection || !allow_local_network || !allow_localhost
return Result.new(uri, nil, true)
end
validate_resolved_uri(uri,
allow_localhost: allow_localhost,
allow_local_network: allow_local_network,
......
# frozen_string_literal: true
require 'spec_helper'
require 'rspec-parameterized'
RSpec.describe Gitlab::HTTP_V2::UrlBlocker, :stub_invalid_dns_only, feature_category: :shared do
let(:schemes) { %w[http https] }
......@@ -37,6 +38,64 @@
end
end
end
context 'when one flag requring IP validation is enabled' do
using RSpec::Parameterized::TableSyntax
let(:import_url) { 'https://localhost' }
where(:deny_all_requests_except_allowed, :dns_rebind_protection, :allow_local_network, :allow_localhost) do
true | false | true | true
false | true | true | true
false | false | false | true
false | false | true | false
end
with_them do
let(:options) do
{
deny_all_requests_except_allowed: deny_all_requests_except_allowed,
dns_rebind_protection: dns_rebind_protection,
allow_local_network: allow_local_network,
allow_localhost: allow_localhost,
schemes: schemes
}
end
it 'attempts to validate IP' do
expect(described_class).to receive(:validate_resolved_uri).and_return(
described_class::Result.new(import_url, 'localhost', false))
subject
end
end
end
context 'when all forms of IP validation are disabled' do
let(:options) do
{
deny_all_requests_except_allowed: false,
dns_rebind_protection: false,
allow_local_network: true,
allow_localhost: true,
schemes: schemes
}
end
let(:import_url) { 'https://localhost' }
let(:expected_uri) { 'https://localhost' }
let(:expected_hostname) { nil }
let(:expected_use_proxy) { false }
it 'does not attempt to resolve IPs' do
expect(Addrinfo).not_to receive(:getaddrinfo)
uri, hostname = subject
expect(uri).to eq(Addressable::URI.parse(expected_uri))
expect(hostname).to eq(expected_hostname)
end
end
end
describe '#validate_url_with_proxy!' do
......
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册