diff --git a/gems/gitlab-http/.rubocop.yml b/gems/gitlab-http/.rubocop.yml index 8bc6b6a4cfbedea1c2b6afc72646cf916b29a112..75941091b893572a39fd821738eebfb1321e7cc5 100644 --- a/gems/gitlab-http/.rubocop.yml +++ b/gems/gitlab-http/.rubocop.yml @@ -20,3 +20,7 @@ RSpec/MultipleMemoizedHelpers: RSpec/FilePath: CustomTransform: HTTP_V2: http_v2 + +Lint/BinaryOperatorWithIdenticalOperands: + Exclude: + - 'spec/**/*.rb' diff --git a/gems/gitlab-http/Gemfile.lock b/gems/gitlab-http/Gemfile.lock index afdf6f3dce28231a872c12b46f2dd5aae662cfae..91cd6eca94f8e25677958e3ff7a6f1dbcd4d52cf 100644 --- a/gems/gitlab-http/Gemfile.lock +++ b/gems/gitlab-http/Gemfile.lock @@ -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) diff --git a/gems/gitlab-http/gitlab-http.gemspec b/gems/gitlab-http/gitlab-http.gemspec index 2378945cbcc35954992cad6f108a7aa603484ebe..89306c0b21e2c12f82a66073e2c630540dae575d 100644 --- a/gems/gitlab-http/gitlab-http.gemspec +++ b/gems/gitlab-http/gitlab-http.gemspec @@ -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" diff --git a/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb b/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb index 7c669a0d9c6f8db1d1251221fc877a85a26b0691..bfee1b2b40b624c117988c052e46f0fab9bb9f2f 100644 --- a/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb +++ b/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb @@ -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, diff --git a/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb index 904fed9baefca6392c53cb1a9a9c1277c96d8711..75f57e731222bd1ccd42428db02762656f6eedfa 100644 --- a/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb +++ b/gems/gitlab-http/spec/gitlab/http_v2/url_blocker_spec.rb @@ -1,6 +1,7 @@ # 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