diff --git a/lib/cc/fixed_resolv.rb b/lib/cc/fixed_resolv.rb deleted file mode 100644 index 7ce21bc..0000000 --- a/lib/cc/fixed_resolv.rb +++ /dev/null @@ -1,29 +0,0 @@ -require "resolv-replace" - -module CC - class FixedResolv < Resolv::DNS - def self.enable! - new.tap do |instance| - Resolv::DefaultResolver.replace_resolvers([instance]) - end - end - - def initialize - @addresses = {} - end - - def setaddress(name, address) - addresses[name] = address - end - - def each_address(name) - if addresses.key?(name) - yield addresses.fetch(name) - end - end - - private - - attr_reader :addresses - end -end diff --git a/lib/cc/resolv.rb b/lib/cc/resolv.rb new file mode 100644 index 0000000..1a5b064 --- /dev/null +++ b/lib/cc/resolv.rb @@ -0,0 +1,39 @@ +require "resolv-replace" + +module CC + class Resolv + def self.with_fixed_dns(dns = ::Resolv::DNS.new) + ::Resolv::DefaultResolver.replace_resolvers([Fixed.new(dns)]) + + yield if block_given? + ensure + # There's no way to ask what the current values are before we override + # them; hopefully going by the source is good enough. + # https://docs.ruby-lang.org/en/2.0.0/Resolv.html#method-c-new + default_resolvers = [::Resolv::Hosts.new, ::Resolv::DNS.new] + ::Resolv::DefaultResolver.replace_resolvers(default_resolvers) + end + + class Fixed + def initialize(fallback) + @addresses = {} + @fallback = fallback + end + + def each_address(name) + if addresses.key?(name) + yield addresses.fetch(name) + else + fallback.each_address(name) do |address| + addresses[name] ||= address + yield address + end + end + end + + private + + attr_reader :addresses, :fallback + end + end +end diff --git a/lib/cc/service/http.rb b/lib/cc/service/http.rb index f0d163c..4e33bd8 100644 --- a/lib/cc/service/http.rb +++ b/lib/cc/service/http.rb @@ -1,4 +1,5 @@ require "active_support/concern" +require "cc/resolv" require "cc/service/response_check" require "cc/service/safe_webhook" @@ -53,13 +54,15 @@ def raw_post(url = nil, body = nil, headers = nil) def http_method(method, url = nil, body = nil, headers = nil) block = Proc.new if block_given? - CC::Service::SafeWebhook.ensure_safe!(url) + CC::Resolv.with_fixed_dns do + CC::Service::SafeWebhook.ensure_safe!(url) - http.send(method) do |req| - req.url(url) if url - req.headers.update(headers) if headers - req.body = body if body - block.call req if block + http.send(method) do |req| + req.url(url) if url + req.headers.update(headers) if headers + req.body = body if body + block.call req if block + end end end diff --git a/lib/cc/service/safe_webhook.rb b/lib/cc/service/safe_webhook.rb index de0490b..4a84c67 100644 --- a/lib/cc/service/safe_webhook.rb +++ b/lib/cc/service/safe_webhook.rb @@ -1,8 +1,6 @@ require "ipaddr" require "uri" -require "cc/fixed_resolv" - module CC class Service class SafeWebhook @@ -22,16 +20,6 @@ def self.ensure_safe!(url) instance.ensure_safe! end - def self.getaddress(host) - @dns ||= Resolv::DNS.new - @dns.getaddress(host) - end - - def self.setaddress(host, address) - @fixed_resolv ||= CC::FixedResolv.enable! - @fixed_resolv.setaddress(host, address) - end - def initialize(url) @url = url end @@ -49,14 +37,12 @@ def ensure_safe! attr_reader :url def internal?(host) - address = self.class.getaddress(host) - - self.class.setaddress(host, address) + address = ::Resolv.getaddress(host) PRIVATE_ADDRESS_SUBNETS.any? do |subnet| subnet === IPAddr.new(address.to_s) end - rescue Resolv::ResolvError + rescue ::Resolv::ResolvError true # localhost end diff --git a/spec/cc/resolve_spec.rb b/spec/cc/resolve_spec.rb new file mode 100644 index 0000000..d83b40c --- /dev/null +++ b/spec/cc/resolve_spec.rb @@ -0,0 +1,43 @@ +require "spec_helper" + +module CC + describe Resolv do + describe ".with_fixed_dns" do + it "replaces the default resolver for the duration of the block" do + fallback = double + + expect(fallback).to receive(:each_address). + with("google.com").and_yield("overridden") + + Resolv.with_fixed_dns(fallback) do + expect(::Resolv.getaddress("google.com")).to eq "overridden" + expect(::Resolv.getaddress("google.com")).to eq "overridden" + end + + expect(::Resolv.getaddress("google.com")).not_to eq "overridden" + end + end + + describe Resolv::Fixed do + describe "#each_address" do + it "delegates to the fallback resolver and caches the first address" do + fallback = double + fixed = Resolv::Fixed.new(fallback) + + allow(fallback).to receive(:each_address). + with("host").once. + and_yield("address-1"). + and_yield("address-2") + + yielded_1 = [] + yielded_2 = [] + fixed.each_address("host") { |a| yielded_1 << a } + fixed.each_address("host") { |a| yielded_2 << a } + + expect(yielded_1).to eq ["address-1", "address-2"] + expect(yielded_2).to eq ["address-1"] + end + end + end + end +end diff --git a/spec/cc/service/safe_webhook_spec.rb b/spec/cc/service/safe_webhook_spec.rb index b7094da..0723506 100644 --- a/spec/cc/service/safe_webhook_spec.rb +++ b/spec/cc/service/safe_webhook_spec.rb @@ -5,7 +5,7 @@ class CC::Service describe ".ensure_safe!" do it "does not allow internal URLs" do %w[ 127.0.0.1 192.168.0.1 10.0.1.18 ].each do |address| - stub_resolv("github.com", address) + stub_resolv_getaddress("github.com", address) expect do SafeWebhook.ensure_safe!("https://github.com/api/v1/user") @@ -13,29 +13,35 @@ class CC::Service end end + it "does not allow URLs that don't resolve via DNS" do + allow(::Resolv).to receive(:getaddress). + with("localhost").and_raise(::Resolv::ResolvError) + + expect do + SafeWebhook.ensure_safe!("https://localhost/api/v1/user") + end.to raise_error(SafeWebhook::InternalWebhookError) + end + it "allows internal URLs when configured to do so" do allow(ENV).to receive(:[]). with("CODECLIMATE_ALLOW_INTERNAL_WEBHOOKS"). and_return("1") - stub_resolv("github.com", "10.0.1.18") + stub_resolv_getaddress("github.com", "10.0.1.18") SafeWebhook.ensure_safe!("https://github.com/api/v1/user") end it "allows non-internal URLs" do - stub_resolv("github.com", "1.1.1.2") + stub_resolv_getaddress("github.com", "1.1.1.2") SafeWebhook.ensure_safe!("https://github.com/api/v1/user") end + end - it "ensures future dns queries get the same answer" do - stub_resolv("github.com", "1.1.1.3") - - SafeWebhook.ensure_safe!("https://github.com/api/v1/user") - - expect(Resolv.getaddress("github.com").to_s).to eq "1.1.1.3" - end + def stub_resolv_getaddress(host, ip) + allow(::Resolv).to receive(:getaddress). + with(host).and_return(::Resolv::IPv4.create(ip)) end end end diff --git a/spec/support/resolv_helpers.rb b/spec/support/resolv_helpers.rb index b035ab3..5330650 100644 --- a/spec/support/resolv_helpers.rb +++ b/spec/support/resolv_helpers.rb @@ -1,7 +1,9 @@ module ResolvHelpers def stub_resolv(name, address) - allow(CC::Service::SafeWebhook).to receive(:getaddress). - with(name).and_return(Resolv::IPv4.create(address)) + dns = double + allow(::Resolv::DNS).to receive(:new).and_return(dns) + allow(dns).to receive(:each_address). + with(name).and_yield(Resolv::IPv4.create(address)) end end