diff --git a/Gemfile b/Gemfile
index ac7ab1c227ef31c2defe136d1a97396246908fba..8708f4a228a486ce37afbb67c128ed058dd528e9 100644
--- a/Gemfile
+++ b/Gemfile
@@ -682,9 +682,10 @@ gem 'telesignenterprise', '~> 2.2' # rubocop:todo Gemfile/MissingFeatureCategory
# BufferedIO patch
# Updating this version will require updating scripts/allowed_warnings.txt
gem 'net-protocol', '~> 0.1.3' # rubocop:todo Gemfile/MissingFeatureCategory
-# Lock this until we make DNS rebinding work with the updated net-http:
-# https://gitlab.com/gitlab-org/gitlab/-/issues/413528
-gem 'net-http', '= 0.1.1' # rubocop:todo Gemfile/MissingFeatureCategory
+
+# This is locked to 0.4.1 because we patch Net::HTTP#connect in
+# gems/gitlab-http/lib/net_http/connect_patch.rb.
+gem 'net-http', '= 0.4.1', feature_category: :shared
gem 'duo_api', '~> 1.3' # rubocop:todo Gemfile/MissingFeatureCategory
diff --git a/Gemfile.checksum b/Gemfile.checksum
index abf7193709dc721fa0e2fa0cab5aaa33ae792208..bdffbf048115cac2ad924dc0a6deb320b29efc61 100644
--- a/Gemfile.checksum
+++ b/Gemfile.checksum
@@ -395,7 +395,7 @@
{"name":"nap","version":"1.1.0","platform":"ruby","checksum":"949691660f9d041d75be611bb2a8d2fd559c467537deac241f4097d9b5eea576"},
{"name":"neighbor","version":"0.3.2","platform":"ruby","checksum":"b795bbcc24b1b9ae82d9f7e97a3461b0b3607d24a85a7acbed776bd498e7eba8"},
{"name":"nenv","version":"0.3.0","platform":"ruby","checksum":"d9de6d8fb7072228463bf61843159419c969edb34b3cef51832b516ae7972765"},
-{"name":"net-http","version":"0.1.1","platform":"ruby","checksum":"75a4e109b6f9af32fad0e98a6180c47aceb415927ca3bd70c8fc3e7dbbabbe86"},
+{"name":"net-http","version":"0.4.1","platform":"ruby","checksum":"a96efc5ea18bcb9715e24dda4159d10f67ff0345c8a980d04630028055b2c282"},
{"name":"net-http-persistent","version":"4.0.1","platform":"ruby","checksum":"2752f4cce05fd1c45e0537c6f3a98fa5a4899efd5f88e63c104ed5f05cbddef9"},
{"name":"net-imap","version":"0.3.4","platform":"ruby","checksum":"a82a59e2a429433dc54cae5a8b2979ffe49da8c66085740811bfa337dc3729b5"},
{"name":"net-ldap","version":"0.17.1","platform":"ruby","checksum":"52571b55f9157120833ac1667f2969ce0139251811d0a9b64657c1c135069cf9"},
diff --git a/Gemfile.lock b/Gemfile.lock
index 40a2741aa94d45b6a16da0d847565cb1df0a4546..6ba258f78aa26d9a5364795189d0fdf0002187ff 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -48,6 +48,7 @@ PATH
concurrent-ruby (~> 1.2)
httparty (~> 0.21.0)
ipaddress (~> 0.8.3)
+ net-http (= 0.4.1)
railties (~> 7)
PATH
@@ -1115,8 +1116,7 @@ GEM
neighbor (0.3.2)
activerecord (>= 6.1)
nenv (0.3.0)
- net-http (0.1.1)
- net-protocol
+ net-http (0.4.1)
uri
net-http-persistent (4.0.1)
connection_pool (~> 2.2)
@@ -2095,7 +2095,7 @@ DEPENDENCIES
minitest (~> 5.11.0)
multi_json (~> 1.14.1)
neighbor (~> 0.3.2)
- net-http (= 0.1.1)
+ net-http (= 0.4.1)
net-ldap (~> 0.17.1)
net-ntp
net-protocol (~> 0.1.3)
diff --git a/gems/gitlab-http/Gemfile.lock b/gems/gitlab-http/Gemfile.lock
index 149a6805f05515e321ccebec410182faa91c7862..afdf6f3dce28231a872c12b46f2dd5aae662cfae 100644
--- a/gems/gitlab-http/Gemfile.lock
+++ b/gems/gitlab-http/Gemfile.lock
@@ -23,6 +23,7 @@ PATH
concurrent-ruby (~> 1.2)
httparty (~> 0.21.0)
ipaddress (~> 0.8.3)
+ net-http (= 0.4.1)
railties (~> 7)
GEM
@@ -82,6 +83,8 @@ GEM
mini_mime (1.1.2)
minitest (5.18.1)
multi_xml (0.6.0)
+ net-http (0.4.1)
+ uri
nokogiri (1.15.4)
racc (~> 1.4)
parallel (1.23.0)
@@ -165,10 +168,12 @@ GEM
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
unicode-display_width (2.4.2)
+ uri (0.13.0)
webmock (3.18.1)
addressable (>= 2.8.0)
crack (>= 0.3.2)
hashdiff (>= 0.4.0, < 2.0.0)
+ webrick (1.8.1)
zeitwerk (2.6.8)
PLATFORMS
@@ -183,6 +188,7 @@ DEPENDENCIES
rubocop (~> 1.50.2)
rubocop-rspec (~> 2.22)
webmock (~> 3.18.1)
+ webrick (~> 1.8)
BUNDLED WITH
2.4.14
diff --git a/gems/gitlab-http/gitlab-http.gemspec b/gems/gitlab-http/gitlab-http.gemspec
index 27366246bb1a2d72297a5a8c6de5bab6f56cc9dd..2378945cbcc35954992cad6f108a7aa603484ebe 100644
--- a/gems/gitlab-http/gitlab-http.gemspec
+++ b/gems/gitlab-http/gitlab-http.gemspec
@@ -24,10 +24,13 @@ Gem::Specification.new do |spec|
spec.add_runtime_dependency 'httparty', '~> 0.21.0'
spec.add_runtime_dependency 'ipaddress', '~> 0.8.3'
spec.add_runtime_dependency "railties", "~> 7"
+ # See lib/net_http/connect_patch.rb
+ spec.add_runtime_dependency "net-http", "= 0.4.1"
spec.add_development_dependency 'gitlab-styles', '~> 10.1.0'
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"
spec.add_development_dependency 'webmock', '~> 3.18.1'
+ spec.add_development_dependency 'webrick', '~> 1.8'
end
diff --git a/gems/gitlab-http/lib/gitlab/http_v2/patches.rb b/gems/gitlab-http/lib/gitlab/http_v2/patches.rb
index 3d26fbc6447d3ec0c6edd116f02b541238a7bdd7..cd89b2e1b4446526a94a16dfc6c25ff39d497688 100644
--- a/gems/gitlab-http/lib/gitlab/http_v2/patches.rb
+++ b/gems/gitlab-http/lib/gitlab/http_v2/patches.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
require_relative "../../hostname_override_patch"
-require_relative "../../net_http/protocol_patch"
+require_relative "../../net_http/connect_patch"
require_relative "../../net_http/response_patch"
require_relative "../../httparty/response_patch"
diff --git a/gems/gitlab-http/lib/net_http/connect_patch.rb b/gems/gitlab-http/lib/net_http/connect_patch.rb
new file mode 100644
index 0000000000000000000000000000000000000000..e737f0fcedffef04a37d32df6f45bf8741679981
--- /dev/null
+++ b/gems/gitlab-http/lib/net_http/connect_patch.rb
@@ -0,0 +1,167 @@
+# frozen_string_literal: true
+
+# This patches Net::HTTP#connect to handle the hostname override patch,
+# which is needed for Server Side Request Forgery (SSRF)
+# protection. This stopped working in net-http v0.2.2 due to
+# https://github.com/ruby/net-http/pull/36.
+# https://github.com/ruby/net-http/issues/141 is outstanding to make
+# this less hacky, but for now we restore the previous behavior by
+# setting the SNI hostname with the hostname override, if available.
+require 'net/http'
+
+module Net
+ class HTTP < Protocol
+ # rubocop:disable Cop/LineBreakAroundConditionalBlock -- This is upstream code
+ # rubocop:disable Layout/ArgumentAlignment -- This is upstream code
+ # rubocop:disable Layout/AssignmentIndentation -- This is upstream code
+ # rubocop:disable Layout/LineEndStringConcatenationIndentation -- This is upstream code
+ # rubocop:disable Layout/MultilineOperationIndentation -- This is upstream code
+ # rubocop:disable Layout/SpaceInsideBlockBraces -- This is upstream code
+ # rubocop:disable Lint/UnusedBlockArgument -- This is upstream code
+ # rubocop:disable Metrics/AbcSize -- This is upstream code
+ # rubocop:disable Metrics/CyclomaticComplexity -- This is upstream code
+ # rubocop:disable Metrics/PerceivedComplexity -- This is upstream code
+ # rubocop:disable Naming/RescuedExceptionsVariableName -- This is upstream code
+ # rubocop:disable Style/AndOr -- This is upstream code
+ # rubocop:disable Style/BlockDelimiters -- This is upstream code
+ # rubocop:disable Style/EmptyLiteral -- This is upstream code
+ # rubocop:disable Style/IfUnlessModifier -- This is upstream code
+ # rubocop:disable Style/LineEndConcatenation -- This is upstream code
+ # rubocop:disable Style/MultilineIfThen -- This is upstream code
+ # rubocop:disable Style/Next -- This is upstream code
+ # rubocop:disable Style/RescueStandardError -- This is upstream code
+ # rubocop:disable Style/StringConcatenation -- This is upstream code
+ def connect
+ if use_ssl?
+ # reference early to load OpenSSL before connecting,
+ # as OpenSSL may take time to load.
+ @ssl_context = OpenSSL::SSL::SSLContext.new
+ end
+
+ if proxy? then
+ conn_addr = proxy_address
+ conn_port = proxy_port
+ else
+ conn_addr = conn_address
+ conn_port = port
+ end
+
+ debug "opening connection to #{conn_addr}:#{conn_port}..."
+ s = Timeout.timeout(@open_timeout, Net::OpenTimeout) {
+ begin
+ TCPSocket.open(conn_addr, conn_port, @local_host, @local_port)
+ rescue => e
+ raise e, "Failed to open TCP connection to " +
+ "#{conn_addr}:#{conn_port} (#{e.message})"
+ end
+ }
+ s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
+ debug "opened"
+ if use_ssl?
+ if proxy?
+ plain_sock = BufferedIO.new(s, read_timeout: @read_timeout,
+ write_timeout: @write_timeout,
+ continue_timeout: @continue_timeout,
+ debug_output: @debug_output)
+ buf = +"CONNECT #{conn_address}:#{@port} HTTP/#{HTTPVersion}\r\n" \
+ "Host: #{@address}:#{@port}\r\n"
+ if proxy_user
+ credential = ["#{proxy_user}:#{proxy_pass}"].pack('m0')
+ buf << "Proxy-Authorization: Basic #{credential}\r\n"
+ end
+ buf << "\r\n"
+ plain_sock.write(buf)
+ HTTPResponse.read_new(plain_sock).value
+ # assuming nothing left in buffers after successful CONNECT response
+ end
+
+ ssl_parameters = Hash.new
+ iv_list = instance_variables
+ SSL_IVNAMES.each_with_index do |ivname, i|
+ if iv_list.include?(ivname)
+ value = instance_variable_get(ivname)
+ unless value.nil?
+ ssl_parameters[SSL_ATTRIBUTES[i]] = value
+ end
+ end
+ end
+ @ssl_context.set_params(ssl_parameters)
+ unless @ssl_context.session_cache_mode.nil? # a dummy method on JRuby
+ @ssl_context.session_cache_mode =
+ OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT |
+ OpenSSL::SSL::SSLContext::SESSION_CACHE_NO_INTERNAL_STORE
+ end
+ if @ssl_context.respond_to?(:session_new_cb) # not implemented under JRuby
+ @ssl_context.session_new_cb = proc {|sock, sess| @ssl_session = sess }
+ end
+
+ # Still do the post_connection_check below even if connecting
+ # to IP address
+ verify_hostname = @ssl_context.verify_hostname
+
+ # This hack would not be needed with https://github.com/ruby/net-http/issues/141
+ address_to_verify = hostname_override || @address
+
+ # Server Name Indication (SNI) RFC 3546/6066
+ case address_to_verify
+ when Resolv::IPv4::Regex, Resolv::IPv6::Regex
+ # don't set SNI, as IP addresses in SNI is not valid
+ # per RFC 6066, section 3.
+
+ # Avoid openssl warning
+ @ssl_context.verify_hostname = false
+ else
+ ssl_host_address = address_to_verify
+ end
+
+ debug "starting SSL for #{conn_addr}:#{conn_port}..."
+ s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)
+ s.sync_close = true
+ s.hostname = ssl_host_address if s.respond_to?(:hostname=) && ssl_host_address
+
+ if @ssl_session and
+ Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
+ s.session = @ssl_session
+ end
+ ssl_socket_connect(s, @open_timeout)
+ if (@ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE) && verify_hostname
+ s.post_connection_check(@address)
+ end
+ debug "SSL established, protocol: #{s.ssl_version}, cipher: #{s.cipher[0]}"
+ end
+ @socket = BufferedIO.new(s, read_timeout: @read_timeout,
+ write_timeout: @write_timeout,
+ continue_timeout: @continue_timeout,
+ debug_output: @debug_output)
+ @last_communicated = nil
+ on_connect
+ rescue => exception
+ if s
+ debug "Conn close because of connect error #{exception}"
+ s.close
+ end
+ raise
+ end
+ private :connect
+ # rubocop:enable Cop/LineBreakAroundConditionalBlock
+ # rubocop:enable Layout/ArgumentAlignment
+ # rubocop:enable Layout/AssignmentIndentation
+ # rubocop:enable Layout/LineEndStringConcatenationIndentation
+ # rubocop:enable Layout/MultilineOperationIndentation
+ # rubocop:enable Layout/SpaceInsideBlockBraces
+ # rubocop:enable Lint/UnusedBlockArgument
+ # rubocop:enable Metrics/AbcSize
+ # rubocop:enable Metrics/CyclomaticComplexity
+ # rubocop:enable Metrics/PerceivedComplexity
+ # rubocop:enable Naming/RescuedExceptionsVariableName
+ # rubocop:enable Style/AndOr
+ # rubocop:enable Style/BlockDelimiters
+ # rubocop:enable Style/EmptyLiteral
+ # rubocop:enable Style/IfUnlessModifier
+ # rubocop:enable Style/LineEndConcatenation
+ # rubocop:enable Style/MultilineIfThen
+ # rubocop:enable Style/Next
+ # rubocop:enable Style/RescueStandardError
+ # rubocop:enable Style/StringConcatenation
+ end
+end
diff --git a/gems/gitlab-http/lib/net_http/protocol_patch.rb b/gems/gitlab-http/lib/net_http/protocol_patch.rb
deleted file mode 100644
index 8231423e1a518d8041bb0833a93c6c5876529393..0000000000000000000000000000000000000000
--- a/gems/gitlab-http/lib/net_http/protocol_patch.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-# frozen_string_literal: true
-
-# Monkey patch Net::HTTP to fix missing URL decoding for username and password in proxy settings
-#
-# See proposed upstream fix https://github.com/ruby/net-http/pull/5
-# See Ruby-lang issue https://bugs.ruby-lang.org/issues/17542
-# See issue on GitLab https://gitlab.com/gitlab-org/gitlab/-/issues/289836
-
-require 'net/http'
-
-# This file can be removed once Ruby 3.0 is no longer supported:
-# https://gitlab.com/gitlab-org/gitlab/-/issues/396223
-return if Gem::Version.new(Net::HTTP::VERSION) >= Gem::Version.new('0.2.0')
-
-module Net
- class HTTP < Protocol
- def proxy_user
- if environment_variable_is_multiuser_safe? && @proxy_from_env
- user = proxy_uri&.user
- CGI.unescape(user) unless user.nil?
- else
- @proxy_user
- end
- end
-
- def proxy_pass
- if environment_variable_is_multiuser_safe? && @proxy_from_env
- pass = proxy_uri&.password
- CGI.unescape(pass) unless pass.nil?
- else
- @proxy_pass
- end
- end
-
- def environment_variable_is_multiuser_safe?
- ENVIRONMENT_VARIABLE_IS_MULTIUSER_SAFE
- end
- end
-end
diff --git a/gems/gitlab-http/spec/gitlab/http_v2/net_http_connect_patch_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/net_http_connect_patch_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..548c33058bed03328cdafa630ad09172d2121abf
--- /dev/null
+++ b/gems/gitlab-http/spec/gitlab/http_v2/net_http_connect_patch_spec.rb
@@ -0,0 +1,102 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'webrick'
+require 'webrick/https'
+
+RSpec.describe 'Net::HTTP#connect DNS rebinding tests', feature_category: :shared do
+ let(:host) { 'localhost' }
+ let(:host_ip) { '127.0.0.1' }
+ let(:rack_app) do
+ proc do |_env|
+ ['200', { 'Content-Type' => 'text/plain' }, ['Hello, world!']]
+ end
+ end
+
+ let!(:http_server) do
+ Class.new do
+ attr_accessor :sni_hostname
+
+ def initialize
+ @server = WEBrick::HTTPServer.new(
+ Port: 0,
+ SSLEnable: true,
+ SSLCertName: [%w[CN localhost]],
+ SSLServerNameCallback: proc { |args| sni_callback(*args) },
+ Logger: WEBrick::Log.new('/dev/null'),
+ AccessLog: []
+ )
+
+ @server.mount_proc '/' do |_req, res|
+ res.body = 'Hello, world!'
+ end
+
+ Thread.new { @server.start }
+ end
+
+ def port
+ @server.config[:Port]
+ end
+
+ def shutdown
+ @server.shutdown
+ end
+
+ def sni_callback(sslsocket, hostname = nil)
+ @sni_hostname = hostname
+ @server.ssl_servername_callback(sslsocket, hostname)
+ end
+ end.new
+ end
+
+ describe '#connect' do
+ before do
+ WebMock.allow_net_connect!
+ end
+
+ after do
+ WebMock.disable_net_connect! # rubocop:disable RSpec/WebMockEnable -- method not available in gem
+ http_server.shutdown
+ end
+
+ shared_examples 'GET request' do
+ it 'makes a successful HTTPS connection' do
+ http = Net::HTTP.new(http_host, http_server.port)
+ http.use_ssl = true
+ http.verify_mode = OpenSSL::SSL::VERIFY_NONE
+ http.hostname_override = hostname_override if hostname_override
+
+ request = Net::HTTP::Get.new('/')
+
+ response = http.start { http.request(request) }
+ expect(response.code).to eq('200')
+ expect(response.body).to include('Hello, world!')
+ expect(http_server.sni_hostname).to eq(expected_sni)
+ end
+ end
+
+ context 'with hostname' do
+ let(:http_host) { host }
+ let(:expected_sni) { host }
+ let(:hostname_override) { nil }
+
+ it_behaves_like 'GET request'
+ end
+
+ context 'with IP address' do
+ let(:http_host) { host_ip }
+ let(:expected_sni) { nil }
+ let(:hostname_override) { nil }
+
+ it_behaves_like 'GET request'
+ end
+
+ context 'with hostname override' do
+ let(:http_host) { host_ip }
+ let(:hostname_override) { host }
+ let(:expected_sni) { host }
+
+ it_behaves_like 'GET request'
+ end
+ end
+end
diff --git a/gems/gitlab-http/spec/gitlab/http_v2/net_http_patch_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2/net_http_patch_spec.rb
deleted file mode 100644
index f34b0d9840359cd73ecb08f0d2be8d33e786fe15..0000000000000000000000000000000000000000
--- a/gems/gitlab-http/spec/gitlab/http_v2/net_http_patch_spec.rb
+++ /dev/null
@@ -1,92 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-require 'net/http'
-
-RSpec.describe 'Net::HTTP patch proxy user and password encoding', feature_category: :shared do
- let(:net_http) { Net::HTTP.new('hostname.example') }
-
- before do
- # This file can be removed once Ruby 3.0 is no longer supported:
- # https://gitlab.com/gitlab-org/gitlab/-/issues/396223
- skip if Gem::Version.new(Net::HTTP::VERSION) >= Gem::Version.new('0.2.0')
- end
-
- describe '#proxy_user' do
- subject { net_http.proxy_user }
-
- it { is_expected.to eq(nil) }
-
- context 'with http_proxy env' do
- let(:http_proxy) { 'http://proxy.example:8000' }
-
- before do
- stub_env('http_proxy', http_proxy)
- end
-
- it { is_expected.to eq(nil) }
-
- context 'and user:password authentication' do
- let(:http_proxy) { 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000' }
-
- context 'when on multiuser safe platform' do
- # linux, freebsd, darwin are considered multi user safe platforms
- # See https://github.com/ruby/net-http/blob/v0.1.1/lib/net/http.rb#L1174-L1178
-
- before do
- allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(true)
- end
-
- it { is_expected.to eq 'Y\\X' }
- end
-
- context 'when not on multiuser safe platform' do
- before do
- allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(false)
- end
-
- it { is_expected.to be_nil }
- end
- end
- end
- end
-
- describe '#proxy_pass' do
- subject { net_http.proxy_pass }
-
- it { is_expected.to eq(nil) }
-
- context 'with http_proxy env' do
- let(:http_proxy) { 'http://proxy.example:8000' }
-
- before do
- stub_env('http_proxy', http_proxy)
- end
-
- it { is_expected.to eq(nil) }
-
- context 'and user:password authentication' do
- let(:http_proxy) { 'http://Y%5CX:R%25S%5D%20%3FX@proxy.example:8000' }
-
- context 'when on multiuser safe platform' do
- # linux, freebsd, darwin are considered multi user safe platforms
- # See https://github.com/ruby/net-http/blob/v0.1.1/lib/net/http.rb#L1174-L1178
-
- before do
- allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(true)
- end
-
- it { is_expected.to eq 'R%S] ?X' }
- end
-
- context 'when not on multiuser safe platform' do
- before do
- allow(net_http).to receive(:environment_variable_is_multiuser_safe?).and_return(false)
- end
-
- it { is_expected.to be_nil }
- end
- end
- end
- end
-end