[ADD] outbound requests whitelist

Signed-off-by: Istvan szalai <istvan.szalai@savoirfairelinux.com>

4 files changed, 272 insertions, 71 deletions

diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index f8b0cbfb6f6..93194de4a1b 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -220,53 +220,53 @@ describe Gitlab::UrlBlocker do
       end
       let(:fake_domain) { 'www.fakedomain.fake' }
 
-      context 'true (default)' do
+      shared_examples 'allows local requests' do |url_blocker_attributes|
         it 'does not block urls from private networks' do
           local_ips.each do |ip|
-            stub_domain_resolv(fake_domain, ip)
-
-            expect(described_class).not_to be_blocked_url("http://#{fake_domain}")
-
-            unstub_domain_resolv
+            stub_domain_resolv(fake_domain, ip) do
+              expect(described_class).not_to be_blocked_url("http://#{fake_domain}", url_blocker_attributes)
+            end
 
-            expect(described_class).not_to be_blocked_url("http://#{ip}")
+            expect(described_class).not_to be_blocked_url("http://#{ip}", url_blocker_attributes)
           end
         end
 
         it 'allows localhost endpoints' do
-          expect(described_class).not_to be_blocked_url('http://0.0.0.0', allow_localhost: true)
-          expect(described_class).not_to be_blocked_url('http://localhost', allow_localhost: true)
-          expect(described_class).not_to be_blocked_url('http://127.0.0.1', allow_localhost: true)
+          expect(described_class).not_to be_blocked_url('http://0.0.0.0', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://localhost', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://127.0.0.1', url_blocker_attributes)
         end
 
         it 'allows loopback endpoints' do
-          expect(described_class).not_to be_blocked_url('http://127.0.0.2', allow_localhost: true)
+          expect(described_class).not_to be_blocked_url('http://127.0.0.2', url_blocker_attributes)
         end
 
         it 'allows IPv4 link-local endpoints' do
-          expect(described_class).not_to be_blocked_url('http://169.254.169.254')
-          expect(described_class).not_to be_blocked_url('http://169.254.168.100')
+          expect(described_class).not_to be_blocked_url('http://169.254.169.254', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://169.254.168.100', url_blocker_attributes)
         end
 
         it 'allows IPv6 link-local endpoints' do
-          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]')
-          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]')
-          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]')
-          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]')
-          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]')
-          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]')
-          expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]')
+          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]', url_blocker_attributes)
+          expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]', url_blocker_attributes)
         end
       end
 
+      context 'true (default)' do
+        it_behaves_like 'allows local requests', { allow_localhost: true, allow_local_network: true }
+      end
+
       context 'false' do
         it 'blocks urls from private networks' do
           local_ips.each do |ip|
-            stub_domain_resolv(fake_domain, ip)
-
-            expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_local_network: false)
-
-            unstub_domain_resolv
+            stub_domain_resolv(fake_domain, ip) do
+              expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_local_network: false)
+            end
 
             expect(described_class).to be_blocked_url("http://#{ip}", allow_local_network: false)
           end
@@ -286,15 +286,169 @@ describe Gitlab::UrlBlocker do
           expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a864]', allow_local_network: false)
           expect(described_class).to be_blocked_url('http://[fe80::c800:eff:fe74:8]', allow_local_network: false)
         end
+
+        context 'when local domain/IP is whitelisted' do
+          let(:url_blocker_attributes) do
+            {
+              allow_localhost: false,
+              allow_local_network: false
+            }
+          end
+
+          before do
+            stub_application_setting(outbound_local_requests_whitelist: whitelist)
+          end
+
+          context 'with IPs in whitelist' do
+            let(:whitelist) do
+              [
+                '0.0.0.0',
+                '127.0.0.1',
+                '127.0.0.2',
+                '192.168.1.1',
+                '192.168.1.2',
+                '0:0:0:0:0:ffff:192.168.1.2',
+                '::ffff:c0a8:102',
+                '10.0.0.2',
+                '0:0:0:0:0:ffff:10.0.0.2',
+                '::ffff:a00:2',
+                '172.16.0.2',
+                '0:0:0:0:0:ffff:172.16.0.2',
+                '::ffff:ac10:20',
+                'feef::1',
+                'fee2::',
+                'fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa',
+                '0:0:0:0:0:ffff:169.254.169.254',
+                '::ffff:a9fe:a9fe',
+                '::ffff:169.254.168.100',
+                '::ffff:a9fe:a864',
+                'fe80::c800:eff:fe74:8',
+
+                # garbage IPs
+                '45645632345',
+                'garbage456:more345gar:bage'
+              ]
+            end
+
+            it_behaves_like 'allows local requests', { allow_localhost: false, allow_local_network: false }
+
+            it 'whitelists IP when dns_rebind_protection is disabled' do
+              stub_domain_resolv('example.com', '192.168.1.1') do
+                expect(described_class).not_to be_blocked_url("http://example.com",
+                  url_blocker_attributes.merge(dns_rebind_protection: false))
+              end
+            end
+          end
+
+          context 'with domains in whitelist' do
+            let(:whitelist) do
+              [
+                'www.example.com',
+                'example.com',
+                'xn--itlab-j1a.com',
+                'garbage$^$%#$^&$'
+              ]
+            end
+
+            it 'allows domains present in whitelist' do
+              domain = 'example.com'
+              subdomain1 = 'www.example.com'
+              subdomain2 = 'subdomain.example.com'
+
+              stub_domain_resolv(domain, '192.168.1.1') do
+                expect(described_class).not_to be_blocked_url("http://#{domain}",
+                  url_blocker_attributes)
+              end
+
+              stub_domain_resolv(subdomain1, '192.168.1.1') do
+                expect(described_class).not_to be_blocked_url("http://#{subdomain1}",
+                  url_blocker_attributes)
+              end
+
+              # subdomain2 is not part of the whitelist so it should be blocked
+              stub_domain_resolv(subdomain2, '192.168.1.1') do
+                expect(described_class).to be_blocked_url("http://#{subdomain2}",
+                  url_blocker_attributes)
+              end
+            end
+
+            it 'works with unicode and idna encoded domains' do
+              unicode_domain = 'ğitlab.com'
+              idna_encoded_domain = 'xn--itlab-j1a.com'
+
+              stub_domain_resolv(unicode_domain, '192.168.1.1') do
+                expect(described_class).not_to be_blocked_url("http://#{unicode_domain}",
+                  url_blocker_attributes)
+              end
+
+              stub_domain_resolv(idna_encoded_domain, '192.168.1.1') do
+                expect(described_class).not_to be_blocked_url("http://#{idna_encoded_domain}",
+                  url_blocker_attributes)
+              end
+            end
+          end
+
+          context 'with ip ranges in whitelist' do
+            let(:ipv4_range) { '127.0.0.0/28' }
+            let(:ipv6_range) { 'fd84:6d02:f6d8:c89e::/124' }
+
+            let(:whitelist) do
+              [
+                ipv4_range,
+                ipv6_range
+              ]
+            end
+
+            it 'blocks ipv4 range when not in whitelist' do
+              stub_application_setting(outbound_local_requests_whitelist: [])
+
+              IPAddr.new(ipv4_range).to_range.to_a.each do |ip|
+                expect(described_class).to be_blocked_url("http://#{ip}",
+                  url_blocker_attributes)
+              end
+            end
+
+            it 'allows all ipv4s in the range when in whitelist' do
+              IPAddr.new(ipv4_range).to_range.to_a.each do |ip|
+                expect(described_class).not_to be_blocked_url("http://#{ip}",
+                  url_blocker_attributes)
+              end
+            end
+
+            it 'blocks ipv6 range when not in whitelist' do
+              stub_application_setting(outbound_local_requests_whitelist: [])
+
+              IPAddr.new(ipv6_range).to_range.to_a.each do |ip|
+                expect(described_class).to be_blocked_url("http://[#{ip}]",
+                  url_blocker_attributes)
+              end
+            end
+
+            it 'allows all ipv6s in the range when in whitelist' do
+              IPAddr.new(ipv6_range).to_range.to_a.each do |ip|
+                expect(described_class).not_to be_blocked_url("http://[#{ip}]",
+                  url_blocker_attributes)
+              end
+            end
+
+            it 'blocks IPs outside the range' do
+              expect(described_class).to be_blocked_url("http://[fd84:6d02:f6d8:c89e:0:0:1:f]",
+                url_blocker_attributes)
+
+              expect(described_class).to be_blocked_url("http://127.0.1.15",
+                url_blocker_attributes)
+            end
+          end
+        end
       end
 
-      def stub_domain_resolv(domain, ip)
+      def stub_domain_resolv(domain, ip, &block)
         address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false, ipv4?: false)
         allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address])
         allow(address).to receive(:ipv6_v4mapped?).and_return(false)
-      end
 
-      def unstub_domain_resolv
+        yield
+
         allow(Addrinfo).to receive(:getaddrinfo).and_call_original
       end
     end
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 4645339f439..0c20b3aa4c8 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -231,4 +231,23 @@ describe Gitlab::Utils do
       end
     end
   end
+
+  describe '.string_to_ip_object' do
+    it 'returns nil when string is nil' do
+      expect(described_class.string_to_ip_object(nil)).to eq(nil)
+    end
+
+    it 'returns nil when string is invalid IP' do
+      expect(described_class.string_to_ip_object('invalid ip')).to eq(nil)
+      expect(described_class.string_to_ip_object('')).to eq(nil)
+    end
+
+    it 'returns IP object when string is valid IP' do
+      expect(described_class.string_to_ip_object('192.168.1.1')).to eq(IPAddr.new('192.168.1.1'))
+      expect(described_class.string_to_ip_object('::ffff:a9fe:a864')).to eq(IPAddr.new('::ffff:a9fe:a864'))
+      expect(described_class.string_to_ip_object('[::ffff:a9fe:a864]')).to eq(IPAddr.new('::ffff:a9fe:a864'))
+      expect(described_class.string_to_ip_object('127.0.0.0/28')).to eq(IPAddr.new('127.0.0.0/28'))
+      expect(described_class.string_to_ip_object('1:0:0:0:0:0:0:0/124')).to eq(IPAddr.new('1:0:0:0:0:0:0:0/124'))
+    end
+  end
 end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index ab6f6dfe720..bd87bbd8d68 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -37,6 +37,17 @@ describe ApplicationSetting do
     it { is_expected.not_to allow_value("myemail@example.com").for(:lets_encrypt_notification_email) }
     it { is_expected.to allow_value("myemail@test.example.com").for(:lets_encrypt_notification_email) }
 
+    it { is_expected.to allow_value(['192.168.1.1'] * 1_000).for(:outbound_local_requests_whitelist) }
+    it { is_expected.not_to allow_value(['192.168.1.1'] * 1_001).for(:outbound_local_requests_whitelist) }
+    it { is_expected.to allow_value(['1' * 255]).for(:outbound_local_requests_whitelist) }
+    it { is_expected.not_to allow_value(['1' * 256]).for(:outbound_local_requests_whitelist) }
+    it { is_expected.not_to allow_value(['ğitlab.com']).for(:outbound_local_requests_whitelist) }
+    it { is_expected.to allow_value(['xn--itlab-j1a.com']).for(:outbound_local_requests_whitelist) }
+    it { is_expected.not_to allow_value(['<h1></h1>']).for(:outbound_local_requests_whitelist) }
+    it { is_expected.to allow_value(['gitlab.com']).for(:outbound_local_requests_whitelist) }
+    it { is_expected.to allow_value(nil).for(:outbound_local_requests_whitelist) }
+    it { is_expected.to allow_value([]).for(:outbound_local_requests_whitelist) }
+
     context "when user accepted let's encrypt terms of service" do
       before do
         setting.update(lets_encrypt_terms_of_service_accepted: true)
diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb
index e7ec24c5b7e..2c600785ad3 100644
--- a/spec/support/shared_examples/application_setting_examples.rb
+++ b/spec/support/shared_examples/application_setting_examples.rb
@@ -1,58 +1,54 @@
 # frozen_string_literal: true
 
-RSpec.shared_examples 'application settings examples' do
-  context 'restricted signup domains' do
-    it 'sets single domain' do
-      setting.domain_whitelist_raw = 'example.com'
-      expect(setting.domain_whitelist).to eq(['example.com'])
-    end
+RSpec.shared_examples 'string of domains' do |attribute|
+  it 'sets single domain' do
+    setting.method("#{attribute}_raw=").call('example.com')
+    expect(setting.method(attribute).call).to eq(['example.com'])
+  end
 
-    it 'sets multiple domains with spaces' do
-      setting.domain_whitelist_raw = 'example.com *.example.com'
-      expect(setting.domain_whitelist).to eq(['example.com', '*.example.com'])
-    end
+  it 'sets multiple domains with spaces' do
+    setting.method("#{attribute}_raw=").call('example.com *.example.com')
+    expect(setting.method(attribute).call).to eq(['example.com', '*.example.com'])
+  end
 
-    it 'sets multiple domains with newlines and a space' do
-      setting.domain_whitelist_raw = "example.com\n *.example.com"
-      expect(setting.domain_whitelist).to eq(['example.com', '*.example.com'])
-    end
+  it 'sets multiple domains with newlines and a space' do
+    setting.method("#{attribute}_raw=").call("example.com\n *.example.com")
+    expect(setting.method(attribute).call).to eq(['example.com', '*.example.com'])
+  end
 
-    it 'sets multiple domains with commas' do
-      setting.domain_whitelist_raw = "example.com, *.example.com"
-      expect(setting.domain_whitelist).to eq(['example.com', '*.example.com'])
-    end
+  it 'sets multiple domains with commas' do
+    setting.method("#{attribute}_raw=").call("example.com, *.example.com")
+    expect(setting.method(attribute).call).to eq(['example.com', '*.example.com'])
   end
 
-  context 'blacklisted signup domains' do
-    it 'sets single domain' do
-      setting.domain_blacklist_raw = 'example.com'
-      expect(setting.domain_blacklist).to contain_exactly('example.com')
-    end
+  it 'sets multiple domains with semicolon' do
+    setting.method("#{attribute}_raw=").call("example.com; *.example.com")
+    expect(setting.method(attribute).call).to contain_exactly('example.com', '*.example.com')
+  end
 
-    it 'sets multiple domains with spaces' do
-      setting.domain_blacklist_raw = 'example.com *.example.com'
-      expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com')
-    end
+  it 'sets multiple domains with mixture of everything' do
+    setting.method("#{attribute}_raw=").call("example.com; *.example.com\n test.com\sblock.com   yes.com")
+    expect(setting.method(attribute).call).to contain_exactly('example.com', '*.example.com', 'test.com', 'block.com', 'yes.com')
+  end
 
-    it 'sets multiple domains with newlines and a space' do
-      setting.domain_blacklist_raw = "example.com\n *.example.com"
-      expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com')
-    end
+  it 'removes duplicates' do
+    setting.method("#{attribute}_raw=").call("example.com; example.com; 127.0.0.1; 127.0.0.1")
+    expect(setting.method(attribute).call).to contain_exactly('example.com', '127.0.0.1')
+  end
 
-    it 'sets multiple domains with commas' do
-      setting.domain_blacklist_raw = "example.com, *.example.com"
-      expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com')
-    end
+  it 'does not fail with garbage values' do
+    setting.method("#{attribute}_raw=").call("example;34543:garbage:fdh5654;")
+    expect(setting.method(attribute).call).to contain_exactly('example', '34543:garbage:fdh5654')
+  end
+end
 
-    it 'sets multiple domains with semicolon' do
-      setting.domain_blacklist_raw = "example.com; *.example.com"
-      expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com')
-    end
+RSpec.shared_examples 'application settings examples' do
+  context 'restricted signup domains' do
+    it_behaves_like 'string of domains', :domain_whitelist
+  end
 
-    it 'sets multiple domains with mixture of everything' do
-      setting.domain_blacklist_raw = "example.com; *.example.com\n test.com\sblock.com   yes.com"
-      expect(setting.domain_blacklist).to contain_exactly('example.com', '*.example.com', 'test.com', 'block.com', 'yes.com')
-    end
+  context 'blacklisted signup domains' do
+    it_behaves_like 'string of domains', :domain_blacklist
 
     it 'sets multiple domain with file' do
       setting.domain_blacklist_file = File.open(Rails.root.join('spec/fixtures/', 'domain_blacklist.txt'))
@@ -60,6 +56,27 @@ RSpec.shared_examples 'application settings examples' do
     end
   end
 
+  context 'outbound_local_requests_whitelist' do
+    it_behaves_like 'string of domains', :outbound_local_requests_whitelist
+  end
+
+  context 'outbound_local_requests_whitelist_arrays' do
+    it 'separates the IPs and domains' do
+      setting.outbound_local_requests_whitelist = [
+        '192.168.1.1', '127.0.0.0/28', 'www.example.com', 'example.com',
+        '::ffff:a00:2', '1:0:0:0:0:0:0:0/124', 'subdomain.example.com'
+      ]
+
+      ip_whitelist = [
+        IPAddr.new('192.168.1.1'), IPAddr.new('127.0.0.0/8'),
+        IPAddr.new('::ffff:a00:2'), IPAddr.new('1:0:0:0:0:0:0:0/124')
+      ]
+      domain_whitelist = ['www.example.com', 'example.com', 'subdomain.example.com']
+
+      expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(ip_whitelist, domain_whitelist)
+    end
+  end
+
   describe 'usage ping settings' do
     context 'when the usage ping is disabled in gitlab.yml' do
       before do