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)