Handle net-http changes (#54)

.github/workflows/build-test.yml

@@ -1,14 +1,14 @@
 name: Build-test
 
-on: 
+on:
   push:
   workflow_call:
 
 jobs:
   build-test:
     strategy:
       matrix:
-        ruby-version: [2.6.0, 2.7.0, 3.0.0, 3.1.0]
+        ruby-version: [2.6.0, 2.7.0, 3.0.0, 3.1.0, head]
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2

Dockerfile

@@ -1,4 +1,4 @@
-FROM ruby:2.6.0
+FROM ruby:3.0.0
 
 RUN apt update && apt-get install -y vim
 

README.md

@@ -19,7 +19,7 @@ This gem provides a safe and easy way to fetch content from user-submitted urls.
 - handles URIs/IPv4/IPv6, redirects, DNS, etc, correctly
 - has 0 runtime dependencies
 - has a comprehensive test suite (100% code coverage)
-- is tested against ruby `2.6`, `2.7`, `3.0`, and `3.1`
+- is tested against ruby `2.6`, `2.7`, `3.0`, `3.1`, and `ruby-head`
 
 ### Quick start
 

lib/ssrf_filter.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require 'ssrf_filter/patch/resolv'
 require 'ssrf_filter/patch/ssl_socket'
 require 'ssrf_filter/ssrf_filter'
 require 'ssrf_filter/version'

lib/ssrf_filter/patch/resolv.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'resolv'
+
+class SsrfFilter
+  module Patch
+    module Resolv
+      # As described in ssl_socket.rb, we want to patch ruby's http connection code to allow us to make outbound network
+      # requests while ensuring that both:
+      # 1) we're connecting to a public / non-private ip address
+      # 2) https connections continue to work
+      #
+      # This used to work fine prior to this change in ruby's net/http library:
+      # https://github.com/ruby/net-http/pull/36
+      # After this changed was introduced our patch no longer works - we need to set the hostname to the correct
+      # value on the SSLSocket (`s.hostname = ssl_host_address`), but that code path no longer executes due to the
+      # modification in the linked pull request.
+      #
+      # To work around this we introduce the patch below, which forces our ip address string to not match against the
+      # Resolv IPv4/IPv6 regular expressions. This is ugly and cumbersome but I didn't see any better path.
+      class PatchedRegexp < Regexp
+        def ===(other)
+          if ::Thread.current.key?(::SsrfFilter::FIBER_ADDRESS_KEY) &&
+             other.object_id.equal?(::Thread.current[::SsrfFilter::FIBER_ADDRESS_KEY].object_id)
+            false
+          else
+            super(other)
+          end
+        end
+      end
+
+      def self.apply!
+        return if instance_variable_defined?(:@patched_resolv)
+
+        @patched_resolv = true
+
+        old_ipv4 = ::Resolv::IPv4.send(:remove_const, :Regex)
+        old_ipv6 = ::Resolv::IPv6.send(:remove_const, :Regex)
+        ::Resolv::IPv4.const_set(:Regex, PatchedRegexp.new(old_ipv4))
+        ::Resolv::IPv6.const_set(:Regex, PatchedRegexp.new(old_ipv6))
+      end
+    end
+  end
+end

lib/ssrf_filter/patch/ssl_socket.rb

@@ -35,13 +35,14 @@ def self.apply!
         ::OpenSSL::SSL::SSLSocket.class_eval do
           original_post_connection_check = instance_method(:post_connection_check)
           define_method(:post_connection_check) do |hostname|
-            original_post_connection_check.bind(self).call(::Thread.current[::SsrfFilter::FIBER_LOCAL_KEY] || hostname)
+            original_post_connection_check.bind(self).call(::Thread.current[::SsrfFilter::FIBER_HOSTNAME_KEY] ||
+              hostname)
           end
 
           if method_defined?(:hostname=)
             original_hostname = instance_method(:hostname=)
             define_method(:hostname=) do |hostname|
-              original_hostname.bind(self).call(::Thread.current[::SsrfFilter::FIBER_LOCAL_KEY] || hostname)
+              original_hostname.bind(self).call(::Thread.current[::SsrfFilter::FIBER_HOSTNAME_KEY] || hostname)
             end
           end
         end

lib/ssrf_filter/ssrf_filter.rb

@@ -83,7 +83,8 @@ def self.prefixlen_from_ipaddr(ipaddr)
     patch: ::Net::HTTP::Patch
   }.freeze
 
-  FIBER_LOCAL_KEY = :__ssrf_filter_hostname
+  FIBER_HOSTNAME_KEY = :__ssrf_filter_hostname
+  FIBER_ADDRESS_KEY = :__ssrf_filter_address
 
   class Error < ::StandardError
   end
@@ -106,6 +107,7 @@ class CRLFInjection < Error
   %i[get put post delete head patch].each do |method|
     define_singleton_method(method) do |url, options = {}, &block|
       ::SsrfFilter::Patch::SSLSocket.apply!
+      ::SsrfFilter::Patch::Resolv.apply!
 
       original_url = url
       scheme_whitelist = options[:scheme_whitelist] || DEFAULT_SCHEME_WHITELIST
@@ -187,7 +189,7 @@ def self.fetch_once(uri, ip, verb, options, &block)
     http_options = options[:http_options] || {}
     http_options[:use_ssl] = (uri.scheme == 'https')
 
-    with_forced_hostname(hostname) do
+    with_forced_hostname(hostname, ip) do
       ::Net::HTTP.start(uri.hostname, uri.port, **http_options) do |http|
         http.request(request) do |response|
           case response
@@ -219,11 +221,13 @@ def self.validate_request(request)
   end
   private_class_method :validate_request
 
-  def self.with_forced_hostname(hostname, &_block)
-    ::Thread.current[FIBER_LOCAL_KEY] = hostname
+  def self.with_forced_hostname(hostname, ip, &_block)
+    ::Thread.current[FIBER_HOSTNAME_KEY] = hostname
+    ::Thread.current[FIBER_ADDRESS_KEY] = ip
     yield
   ensure
-    ::Thread.current[FIBER_LOCAL_KEY] = nil
+    ::Thread.current[FIBER_HOSTNAME_KEY] = nil
+    ::Thread.current[FIBER_ADDRESS_KEY] = nil
   end
   private_class_method :with_forced_hostname
 end

spec/lib/patch/resolv_spec.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+describe ::SsrfFilter::Patch::Resolv do
+  describe 'apply' do
+    before do
+      if described_class.instance_variable_defined?(:@patched_resolv)
+        described_class.remove_instance_variable(:@patched_resolv)
+      end
+    end
+
+    it 'only patches once' do
+      expect(::Resolv::IPv4).to receive(:remove_const).once.and_call_original
+      expect(::Resolv::IPv6).to receive(:remove_const).once.and_call_original
+      described_class.apply!
+      described_class.apply!
+    end
+  end
+
+  describe ::SsrfFilter::Patch::Resolv::PatchedRegexp do
+    it 'forces the ip regex to not match the supplied address' do
+      # rubocop:disable Style/CaseEquality
+      ipaddress1 = '1.2.3.4'
+      ipaddress2 = '5.6.7.8'
+      SsrfFilter.send(:with_forced_hostname, nil, ipaddress1) do
+        expect(described_class.new(Resolv::IPv4::Regex) === ipaddress1).to be false
+        expect(described_class.new(Resolv::IPv4::Regex) === ipaddress2).to be true
+      end
+      expect(described_class.new(Resolv::IPv4::Regex) === ipaddress1).to be true
+      expect(described_class.new(Resolv::IPv4::Regex) === ipaddress2).to be true
+      # rubocop:enable Style/CaseEquality
+    end
+  end
+end

spec/lib/ssrf_filter_spec.rb

@@ -145,22 +145,28 @@
 
   describe 'with_forced_hostname' do
     it 'sets the value for the block and clear it afterwards' do
-      expect(Thread.current[described_class::FIBER_LOCAL_KEY]).to be_nil
-      described_class.with_forced_hostname('test') do
-        expect(Thread.current[described_class::FIBER_LOCAL_KEY]).to eq('test')
+      expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_ADDRESS_KEY]).to be_nil
+      described_class.with_forced_hostname('test', '1.2.3.4') do
+        expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to eq('test')
+        expect(Thread.current[described_class::FIBER_ADDRESS_KEY]).to eq('1.2.3.4')
       end
-      expect(Thread.current[described_class::FIBER_LOCAL_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_ADDRESS_KEY]).to be_nil
     end
 
     it 'clears the value even if an exception is raised' do
-      expect(Thread.current[described_class::FIBER_LOCAL_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_ADDRESS_KEY]).to be_nil
       expect do
-        described_class.with_forced_hostname('test') do
-          expect(Thread.current[described_class::FIBER_LOCAL_KEY]).to eq('test')
+        described_class.with_forced_hostname('test', '1.2.3.4') do
+          expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to eq('test')
+          expect(Thread.current[described_class::FIBER_ADDRESS_KEY]).to eq('1.2.3.4')
           raise StandardError
         end
       end.to raise_error(StandardError)
-      expect(Thread.current[described_class::FIBER_LOCAL_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_HOSTNAME_KEY]).to be_nil
+      expect(Thread.current[described_class::FIBER_ADDRESS_KEY]).to be_nil
     end
   end
 

spec/spec_helper.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require 'pry-byebug'
 require 'simplecov'
 require 'simplecov-lcov'
 SimpleCov.start do