[CI-Test] fix/6430-webhook-address-fallback

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<String>] 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)

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
 

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

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)