From f94648886070958a5a7ff9524cb044238d0ae24e Mon Sep 17 00:00:00 2001 From: FelixTheDev <126066594+felixthedevgit@users.noreply.github.com> Date: Fri, 9 Oct 2026 12:50:01 +0200 Subject: [PATCH] Fixes #6430 - Webhook fails to fallback to additional IPs, only first tried (especially for dualstack). Requests with safety validation pin the connection to the validated address, so Net::HTTP no longer falls back to the other addresses of the hostname, e.g. from an unreachable IPv6 to IPv4. If the connection to the pinned address cannot be opened, the other addresses of the hostname are now tried in resolver order. Each one has to pass the same checks as the pinned one (HostnameSafetyCheck .safe_addresses), and only the connection is retried, so a request is never sent twice. --- lib/hostname_safety_check.rb | 33 ++++++++++--- lib/user_agent.rb | 36 ++++++++++++++ spec/lib/hostname_safety_check_spec.rb | 30 ++++++++++++ spec/lib/user_agent_spec.rb | 66 ++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 6 deletions(-) diff --git a/lib/hostname_safety_check.rb b/lib/hostname_safety_check.rb index 815973f36b27..8839915b399a 100644 --- a/lib/hostname_safety_check.rb +++ b/lib/hostname_safety_check.rb @@ -23,9 +23,33 @@ module HostnameSafetyCheck # @raise [StandardError] if hostname is not safe or cannot be resolved def self.validate!(hostname, allow_private: false, allow_loopback: false, allow_link_local: false) resolved = IPSocket.getaddress(hostname) + check_address!(hostname, resolved, allow_private:, allow_loopback:, allow_link_local:) + + resolved + rescue => e + raise e if e.is_a?(SafetyError) + + raise SafetyError.new(hostname) # rubocop:disable Style/RaiseArgs + end + + # Returns all addresses the hostname resolves to that pass the same checks as .validate!, in resolver order. + # Unsafe addresses are left out, so a connection can fall back to another address without ever reaching them. + # + # @return [Array] the safe addresses, empty if the hostname cannot be resolved + def self.safe_addresses(hostname, allow_private: false, allow_loopback: false, allow_link_local: false) + Addrinfo.getaddrinfo(hostname, nil, nil, :STREAM).map(&:ip_address).uniq.select do |address| + check_address!(hostname, address, allow_private:, allow_loopback:, allow_link_local:) + rescue SafetyError, IPAddr::Error + false + end + rescue SocketError + [] + end + + def self.check_address!(hostname, address, allow_private:, allow_loopback:, allow_link_local:) # An IPv4 address in IPv6 notation (::ffff:169.254.169.254, ::169.254.169.254) is judged as the # IPv4 address it stands for. - ip = IPAddr.new(resolved).native + ip = IPAddr.new(address).native if METADATA_ENDPOINTS.any? { |network| network.include?(ip) } raise MetadataIpError.new(hostname, ip) @@ -43,12 +67,9 @@ def self.validate!(hostname, allow_private: false, allow_loopback: false, allow_ raise LinkLocalIpError.new(hostname, ip) end - resolved - rescue => e - raise e if e.is_a?(SafetyError) - - raise SafetyError.new(hostname) # rubocop:disable Style/RaiseArgs + true end + private_class_method :check_address! class SafetyError < StandardError def initialize(hostname, ip = nil) diff --git a/lib/user_agent.rb b/lib/user_agent.rb index 565d86d8e864..4283098c1abf 100644 --- a/lib/user_agent.rb +++ b/lib/user_agent.rb @@ -5,6 +5,16 @@ class UserAgent + # Raised while opening a connection, before anything has been sent. + CONNECT_ERRORS = [ + Net::OpenTimeout, + Errno::ECONNREFUSED, + Errno::EHOSTUNREACH, + Errno::ENETUNREACH, + Errno::EADDRNOTAVAIL, + Errno::ETIMEDOUT, + ].freeze + # Make HTTP request via GET method # # @see .make_connection @@ -256,6 +266,26 @@ def self.handled_open_timeout(tries) end end + # Opens the connection to the pinned address and falls back to the other safe addresses of the hostname, + # e.g. from an unreachable IPv6 to IPv4 - pinning keeps Net::HTTP from doing that on its own. + # Only the connection is retried, so the request itself is never sent twice. + def self.connect_with_address_fallback(http, hostname, validate_safety_options) + http.start + rescue *CONNECT_ERRORS => e + pinned_ip = http.ipaddr + fallback = HostnameSafetyCheck.safe_addresses(hostname, **validate_safety_options) - [pinned_ip] + + fallback.each do |address| + http.ipaddr = address + return http.start + rescue *CONNECT_ERRORS + next + end + + http.ipaddr = pinned_ip + raise e + end + # Base method for making connection # # @param method [Symbol] HTTP request method style to use. Must be Net::HTTP::Class @@ -305,6 +335,8 @@ def self.make_connection(method, url, params = {}, options = {}, count = 10) http.ipaddr = resolved_ip if !http.proxy? end + address_pinned = options[:validate_safety] && !http.proxy? + # set headers request = set_headers(request, options) @@ -328,6 +360,8 @@ def self.make_connection(method, url, params = {}, options = {}, count = 10) handled_open_timeout(options[:open_socket_tries]) do Timeout.timeout(total_timeout) do + connect_with_address_fallback(http, uri.hostname, validate_safety_options) if address_pinned + response = if (send_as_raw_body = options[:send_as_raw_body]) http.request(request, send_as_raw_body) else @@ -343,6 +377,8 @@ def self.make_connection(method, url, params = {}, options = {}, count = 10) success: false, code: 0, ) + ensure + http.finish if address_pinned && http.started? end end diff --git a/spec/lib/hostname_safety_check_spec.rb b/spec/lib/hostname_safety_check_spec.rb index 2132b8be3678..2d23d1b7a0b2 100644 --- a/spec/lib/hostname_safety_check_spec.rb +++ b/spec/lib/hostname_safety_check_spec.rb @@ -186,4 +186,34 @@ end end end + + describe '.safe_addresses' do + let(:hostname) { 'dualstack.example.com' } + let(:addresses) { ['2001:db8::1', '203.0.113.10', '169.254.169.254', 'fd00:ec2::254', '203.0.113.10'] } + + before do + allow(Addrinfo).to receive(:getaddrinfo) + .with(hostname, nil, nil, :STREAM) + .and_return(addresses.map { |address| Addrinfo.tcp(address, 0) }) + end + + it 'returns the distinct safe addresses in resolver order' do + expect(described_class.safe_addresses(hostname)).to eq(['2001:db8::1', '203.0.113.10']) + end + + it 'applies the given options' do + expect(described_class.safe_addresses(hostname, allow_link_local: true)) + .to eq(['2001:db8::1', '203.0.113.10', '169.254.169.254']) + end + + context 'when the hostname cannot be resolved' do + before do + allow(Addrinfo).to receive(:getaddrinfo).and_raise(SocketError) + end + + it 'returns no addresses' do + expect(described_class.safe_addresses(hostname)).to eq([]) + end + end + end end diff --git a/spec/lib/user_agent_spec.rb b/spec/lib/user_agent_spec.rb index ad3aa4471787..33a1d4e5b1dc 100644 --- a/spec/lib/user_agent_spec.rb +++ b/spec/lib/user_agent_spec.rb @@ -736,6 +736,8 @@ def stop_server :open_timeout= => nil, :read_timeout= => nil, :set_debug_output => nil, + :start => nil, + :started? => false, :request => nil, :proxy? => false, ) @@ -938,6 +940,70 @@ def base_host end end + describe '.connect_with_address_fallback' do + let(:hostname) { 'dualstack.example.com' } + let(:http) { Net::HTTP.new(hostname, 443) } + let(:attempts) { [] } + let(:unreachable) { [] } + + before do + http.ipaddr = '2001:db8::1' + + allow(HostnameSafetyCheck).to receive(:safe_addresses).with(hostname).and_return(['2001:db8::1', '203.0.113.10']) + + allow(http).to receive(:start) do + attempts << http.ipaddr + raise Errno::ENETUNREACH if unreachable.include?(http.ipaddr) + + http + end + end + + context 'when the pinned address is reachable' do + it 'connects to it without resolving further addresses' do + described_class.connect_with_address_fallback(http, hostname, nil) + + expect(attempts).to eq(['2001:db8::1']) + expect(HostnameSafetyCheck).not_to have_received(:safe_addresses) + end + end + + context 'when the pinned address is unreachable' do + let(:unreachable) { ['2001:db8::1'] } + + it 'connects to the next safe address' do + described_class.connect_with_address_fallback(http, hostname, nil) + + expect(attempts).to eq(['2001:db8::1', '203.0.113.10']) + end + end + + context 'when no address is reachable' do + let(:unreachable) { ['2001:db8::1', '203.0.113.10'] } + + it 'raises the error and keeps the pinned address' do + expect { described_class.connect_with_address_fallback(http, hostname, nil) } + .to raise_error(Errno::ENETUNREACH) + + expect(attempts).to eq(['2001:db8::1', '203.0.113.10']) + expect(http.ipaddr).to eq('2001:db8::1') + end + end + + context 'when the connection fails for another reason' do + before do + allow(http).to receive(:start).and_raise(OpenSSL::SSL::SSLError) + end + + it 'does not try other addresses' do + expect { described_class.connect_with_address_fallback(http, hostname, nil) } + .to raise_error(OpenSSL::SSL::SSLError) + + expect(HostnameSafetyCheck).not_to have_received(:safe_addresses) + end + end + end + describe '.log' do before do allow(HttpLog).to receive(:create)