Merge branch 'rc/whitelist_ports' into 'master'

Add ability to whitelist ports

Closes #31867

See merge request gitlab-org/gitlab!27025

app/models/application_setting_implementation.rb

@@ -361,18 +361,33 @@ module ApplicationSettingImplementation
 
   def separate_whitelists(string_array)
     string_array.reduce([[], []]) do |(ip_whitelist, domain_whitelist), string|
-      ip_obj = Gitlab::Utils.string_to_ip_object(string)
+      address, port = parse_addr_and_port(string)
+
+      ip_obj = Gitlab::Utils.string_to_ip_object(address)
 
       if ip_obj
-        ip_whitelist << ip_obj
+        ip_whitelist << Gitlab::UrlBlockers::IpWhitelistEntry.new(ip_obj, port: port)
       else
-        domain_whitelist << string
+        domain_whitelist << Gitlab::UrlBlockers::DomainWhitelistEntry.new(address, port: port)
       end
 
       [ip_whitelist, domain_whitelist]
     end
   end
 
+  def parse_addr_and_port(str)
+    case str
+    when /\A\[(?<address> .* )\]:(?<port> \d+ )\z/x      # string like "[::1]:80"
+      address, port = $~[:address], $~[:port]
+    when /\A(?<address> [^:]+ ):(?<port> \d+ )\z/x       # string like "127.0.0.1:80"
+      address, port = $~[:address], $~[:port]
+    else                                                 # string with no port number
+      address, port = str, nil
+    end
+
+    [address, port&.to_i]
+  end
+
   def array_to_string(arr)
     arr&.join("\n")
   end

changelogs/unreleased/rc-whitelist_ports.yml

+---
+title: Add ability to whitelist ports
+merge_request: 27025
+author:
+type: added

doc/security/webhooks.md

@@ -71,16 +71,24 @@ use IDNA encoding.
 The whitelist can hold a maximum of 1000 entries. Each entry can be a maximum of
 255 characters.
 
+You can whitelist a particular port by specifying it in the whitelist entry.
+For example `127.0.0.1:8080` will only allow connections to port 8080 on `127.0.0.1`.
+If no port is mentioned, all ports on that IP/domain are whitelisted. An IP range
+will whitelist all ports on all IPs in that range.
+
 Example:
 
 ```text
 example.com;gitlab.example.com
 127.0.0.1,1:0:0:0:0:0:0:1
 127.0.0.0/8 1:0:0:0:0:0:0:0/124
+[1:0:0:0:0:0:0:1]:8080
+127.0.0.1:8080
+example.com:8080
 ```
 
 NOTE: **Note:**
-Wildcards (`*.example.com`) and ports (`127.0.0.1:3000`) are not currently supported.
+Wildcards (`*.example.com`) are not currently supported.
 
 <!-- ## Troubleshooting
 

lib/gitlab/url_blocker.rb

@@ -49,7 +49,7 @@ module Gitlab
         return [uri, nil] unless address_info
 
         ip_address = ip_address(address_info)
-        return [uri, nil] if domain_whitelisted?(uri) || ip_whitelisted?(ip_address)
+        return [uri, nil] if domain_whitelisted?(uri) || ip_whitelisted?(ip_address, port: get_port(uri))
 
         protected_uri_with_hostname = enforce_uri_hostname(ip_address, uri, dns_rebind_protection)
 
@@ -254,11 +254,11 @@ module Gitlab
       end
 
       def domain_whitelisted?(uri)
-        Gitlab::UrlBlockers::UrlWhitelist.domain_whitelisted?(uri.normalized_host)
+        Gitlab::UrlBlockers::UrlWhitelist.domain_whitelisted?(uri.normalized_host, port: get_port(uri))
       end
 
-      def ip_whitelisted?(ip_address)
-        Gitlab::UrlBlockers::UrlWhitelist.ip_whitelisted?(ip_address)
+      def ip_whitelisted?(ip_address, port: nil)
+        Gitlab::UrlBlockers::UrlWhitelist.ip_whitelisted?(ip_address, port: port)
       end
 
       def config

lib/gitlab/url_blockers/domain_whitelist_entry.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module UrlBlockers
+    class DomainWhitelistEntry
+      attr_reader :domain, :port
+
+      def initialize(domain, port: nil)
+        @domain = domain
+        @port = port
+      end
+
+      def match?(requested_domain, requested_port = nil)
+        return false unless domain == requested_domain
+        return true if port.nil?
+
+        port == requested_port
+      end
+    end
+  end
+end

lib/gitlab/url_blockers/ip_whitelist_entry.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module UrlBlockers
+    class IpWhitelistEntry
+      attr_reader :ip, :port
+
+      # Argument ip should be an IPAddr object
+      def initialize(ip, port: nil)
+        @ip = ip
+        @port = port
+      end
+
+      def match?(requested_ip, requested_port = nil)
+        return false unless ip.include?(requested_ip)
+        return true if port.nil?
+
+        port == requested_port
+      end
+    end
+  end
+end

lib/gitlab/url_blockers/url_whitelist.rb

@@ -4,21 +4,25 @@ module Gitlab
   module UrlBlockers
     class UrlWhitelist
       class << self
-        def ip_whitelisted?(ip_string)
+        def ip_whitelisted?(ip_string, port: nil)
           return false if ip_string.blank?
 
           ip_whitelist, _ = outbound_local_requests_whitelist_arrays
           ip_obj = Gitlab::Utils.string_to_ip_object(ip_string)
 
-          ip_whitelist.any? { |ip| ip.include?(ip_obj) }
+          ip_whitelist.any? do |ip_whitelist_entry|
+            ip_whitelist_entry.match?(ip_obj, port)
+          end
         end
 
-        def domain_whitelisted?(domain_string)
+        def domain_whitelisted?(domain_string, port: nil)
           return false if domain_string.blank?
 
           _, domain_whitelist = outbound_local_requests_whitelist_arrays
 
-          domain_whitelist.include?(domain_string)
+          domain_whitelist.any? do |domain_whitelist_entry|
+            domain_whitelist_entry.match?(domain_string, port)
+          end
         end
 
         private

spec/lib/gitlab/url_blocker_spec.rb

@@ -501,6 +501,18 @@ describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
               it_behaves_like 'dns rebinding checks'
             end
           end
+
+          context 'with ports' do
+            let(:whitelist) do
+              ["127.0.0.1:2000"]
+            end
+
+            it 'allows domain with port when resolved ip has port whitelisted' do
+              stub_domain_resolv("www.resolve-domain.com", '127.0.0.1') do
+                expect(described_class).not_to be_blocked_url("http://www.resolve-domain.com:2000", url_blocker_attributes)
+              end
+            end
+          end
         end
       end
 

spec/lib/gitlab/url_blockers/domain_whitelist_entry_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::UrlBlockers::DomainWhitelistEntry do
+  let(:domain) { 'www.example.com' }
+
+  describe '#initialize' do
+    it 'initializes without port' do
+      domain_whitelist_entry = described_class.new(domain)
+
+      expect(domain_whitelist_entry.domain).to eq(domain)
+      expect(domain_whitelist_entry.port).to be(nil)
+    end
+
+    it 'initializes with port' do
+      port = 8080
+      domain_whitelist_entry = described_class.new(domain, port: port)
+
+      expect(domain_whitelist_entry.domain).to eq(domain)
+      expect(domain_whitelist_entry.port).to eq(port)
+    end
+  end
+
+  describe '#match?' do
+    it 'matches when domain and port are equal' do
+      port = 8080
+      domain_whitelist_entry = described_class.new(domain, port: port)
+
+      expect(domain_whitelist_entry).to be_match(domain, port)
+    end
+
+    it 'matches any port when port is nil' do
+      domain_whitelist_entry = described_class.new(domain)
+
+      expect(domain_whitelist_entry).to be_match(domain, 8080)
+      expect(domain_whitelist_entry).to be_match(domain, 9090)
+    end
+
+    it 'does not match when port is present but requested_port is nil' do
+      domain_whitelist_entry = described_class.new(domain, port: 8080)
+
+      expect(domain_whitelist_entry).not_to be_match(domain, nil)
+    end
+
+    it 'matches when port and requested_port are nil' do
+      domain_whitelist_entry = described_class.new(domain)
+
+      expect(domain_whitelist_entry).to be_match(domain)
+    end
+
+    it 'does not match if domain is not equal' do
+      domain_whitelist_entry = described_class.new(domain)
+
+      expect(domain_whitelist_entry).not_to be_match('www.gitlab.com', 8080)
+    end
+  end
+end

spec/lib/gitlab/url_blockers/ip_whitelist_entry_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::UrlBlockers::IpWhitelistEntry do
+  let(:ipv4) { IPAddr.new('192.168.1.1') }
+
+  describe '#initialize' do
+    it 'initializes without port' do
+      ip_whitelist_entry = described_class.new(ipv4)
+
+      expect(ip_whitelist_entry.ip).to eq(ipv4)
+      expect(ip_whitelist_entry.port).to be(nil)
+    end
+
+    it 'initializes with port' do
+      port = 8080
+      ip_whitelist_entry = described_class.new(ipv4, port: port)
+
+      expect(ip_whitelist_entry.ip).to eq(ipv4)
+      expect(ip_whitelist_entry.port).to eq(port)
+    end
+  end
+
+  describe '#match?' do
+    it 'matches with equivalent IP and port' do
+      port = 8080
+      ip_whitelist_entry = described_class.new(ipv4, port: port)
+
+      expect(ip_whitelist_entry).to be_match(ipv4.to_s, port)
+    end
+
+    it 'matches any port when port is nil' do
+      ip_whitelist_entry = described_class.new(ipv4)
+
+      expect(ip_whitelist_entry).to be_match(ipv4.to_s, 8080)
+      expect(ip_whitelist_entry).to be_match(ipv4.to_s, 9090)
+    end
+
+    it 'does not match when port is present but requested_port is nil' do
+      ip_whitelist_entry = described_class.new(ipv4, port: 8080)
+
+      expect(ip_whitelist_entry).not_to be_match(ipv4.to_s, nil)
+    end
+
+    it 'matches when port and requested_port are nil' do
+      ip_whitelist_entry = described_class.new(ipv4)
+
+      expect(ip_whitelist_entry).to be_match(ipv4.to_s)
+    end
+
+    it 'works with ipv6' do
+      ipv6 = IPAddr.new('fe80::c800:eff:fe74:8')
+      ip_whitelist_entry = described_class.new(ipv6)
+
+      expect(ip_whitelist_entry).to be_match(ipv6.to_s, 8080)
+    end
+
+    it 'matches ipv4 within IPv4 range' do
+      ipv4_range = IPAddr.new('127.0.0.0/28')
+      ip_whitelist_entry = described_class.new(ipv4_range)
+
+      expect(ip_whitelist_entry).to be_match(ipv4_range.to_range.last.to_s, 8080)
+      expect(ip_whitelist_entry).not_to be_match('127.0.1.1', 8080)
+    end
+
+    it 'matches IPv6 within IPv6 range' do
+      ipv6_range = IPAddr.new('fd84:6d02:f6d8:c89e::/124')
+      ip_whitelist_entry = described_class.new(ipv6_range)
+
+      expect(ip_whitelist_entry).to be_match(ipv6_range.to_range.last.to_s, 8080)
+      expect(ip_whitelist_entry).not_to be_match('fd84:6d02:f6d8:f::f', 8080)
+    end
+  end
+end

spec/lib/gitlab/url_blockers/url_whitelist_spec.rb

@@ -13,20 +13,17 @@ describe Gitlab::UrlBlockers::UrlWhitelist do
   end
 
   describe '#domain_whitelisted?' do
-    let(:whitelist) do
-      [
-        'www.example.com',
-        'example.com'
-      ]
-    end
+    let(:whitelist) { ['www.example.com', 'example.com'] }
 
     it 'returns true if domains present in whitelist' do
+      not_whitelisted = ['subdomain.example.com', 'example.org']
+
       aggregate_failures do
         whitelist.each do |domain|
           expect(described_class).to be_domain_whitelisted(domain)
         end
 
-        ['subdomain.example.com', 'example.org'].each do |domain|
+        not_whitelisted.each do |domain|
           expect(described_class).not_to be_domain_whitelisted(domain)
         end
       end
@@ -35,6 +32,28 @@ describe Gitlab::UrlBlockers::UrlWhitelist do
     it 'returns false when domain is blank' do
       expect(described_class).not_to be_domain_whitelisted(nil)
     end
+
+    context 'with ports' do
+      let(:whitelist) { ['example.io:3000'] }
+
+      it 'returns true if domain and ports present in whitelist' do
+        parsed_whitelist = [['example.io', { port: 3000 }]]
+        not_whitelisted = [
+          'example.io',
+          ['example.io', { port: 3001 }]
+        ]
+
+        aggregate_failures do
+          parsed_whitelist.each do |domain_and_port|
+            expect(described_class).to be_domain_whitelisted(*domain_and_port)
+          end
+
+          not_whitelisted.each do |domain_and_port|
+            expect(described_class).not_to be_domain_whitelisted(*domain_and_port)
+          end
+        end
+      end
+    end
   end
 
   describe '#ip_whitelisted?' do
@@ -114,5 +133,32 @@ describe Gitlab::UrlBlockers::UrlWhitelist do
         expect(described_class).not_to be_ip_whitelisted("127.0.1.15")
       end
     end
+
+    context 'with ports' do
+      let(:whitelist) { ['127.0.0.9:3000', '[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443'] }
+
+      it 'returns true if ip and ports present in whitelist' do
+        parsed_whitelist = [
+          ['127.0.0.9', { port: 3000 }],
+          ['[2001:db8:85a3:8d3:1319:8a2e:370:7348]', { port: 443 }]
+        ]
+        not_whitelisted = [
+          '127.0.0.9',
+          ['127.0.0.9', { port: 3001 }],
+          '[2001:db8:85a3:8d3:1319:8a2e:370:7348]',
+          ['[2001:db8:85a3:8d3:1319:8a2e:370:7348]', { port: 3001 }]
+        ]
+
+        aggregate_failures do
+          parsed_whitelist.each do |ip_and_port|
+            expect(described_class).to be_ip_whitelisted(*ip_and_port)
+          end
+
+          not_whitelisted.each do |ip_and_port|
+            expect(described_class).not_to be_ip_whitelisted(*ip_and_port)
+          end
+        end
+      end
+    end
   end
 end

spec/support/shared_examples/models/application_setting_shared_examples.rb

@@ -68,12 +68,12 @@ RSpec.shared_examples 'application settings examples' do
       setting.outbound_local_requests_whitelist_raw = 'example.com'
 
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
-        [], ['example.com']
+        [], [an_object_having_attributes(domain: 'example.com')]
       )
 
       setting.outbound_local_requests_whitelist_raw = 'gitlab.com'
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
-        [], ['gitlab.com']
+        [], [an_object_having_attributes(domain: 'gitlab.com')]
       )
     end
   end
@@ -81,15 +81,42 @@ RSpec.shared_examples 'application settings examples' do
   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'
+        '192.168.1.1',
+        '127.0.0.0/28',
+        '::ffff:a00:2',
+        '1:0:0:0:0:0:0:0/124',
+        'example.com',
+        'subdomain.example.com',
+        'www.example.com',
+        '::',
+        '1::',
+        '::1',
+        '1:2:3:4:5::7:8',
+        '[1:2:3:4:5::7:8]',
+        '[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443',
+        'www.example2.com:8080',
+        'example.com:8080'
       ]
 
       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')
+        an_object_having_attributes(ip: IPAddr.new('192.168.1.1')),
+        an_object_having_attributes(ip: IPAddr.new('127.0.0.0/8')),
+        an_object_having_attributes(ip: IPAddr.new('::ffff:a00:2')),
+        an_object_having_attributes(ip: IPAddr.new('1:0:0:0:0:0:0:0/124')),
+        an_object_having_attributes(ip: IPAddr.new('::')),
+        an_object_having_attributes(ip: IPAddr.new('1::')),
+        an_object_having_attributes(ip: IPAddr.new('::1')),
+        an_object_having_attributes(ip: IPAddr.new('1:2:3:4:5::7:8')),
+        an_object_having_attributes(ip: IPAddr.new('[1:2:3:4:5::7:8]')),
+        an_object_having_attributes(ip: IPAddr.new('[2001:db8:85a3:8d3:1319:8a2e:370:7348]'), port: 443)
+      ]
+      domain_whitelist = [
+        an_object_having_attributes(domain: 'example.com'),
+        an_object_having_attributes(domain: 'subdomain.example.com'),
+        an_object_having_attributes(domain: 'www.example.com'),
+        an_object_having_attributes(domain: 'www.example2.com', port: 8080),
+        an_object_having_attributes(domain: 'example.com', port: 8080)
       ]
-      domain_whitelist = ['www.example.com', 'example.com', 'subdomain.example.com']
 
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
         ip_whitelist, domain_whitelist
@@ -117,7 +144,7 @@ RSpec.shared_examples 'application settings examples' do
 
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
         [],
-        ['example.com']
+        [an_object_having_attributes(domain: 'example.com')]
       )
 
       setting.add_to_outbound_local_requests_whitelist(
@@ -126,7 +153,7 @@ RSpec.shared_examples 'application settings examples' do
 
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
         [],
-        ['example.com', 'gitlab.com']
+        [an_object_having_attributes(domain: 'example.com'), an_object_having_attributes(domain: 'gitlab.com')]
       )
     end
 
@@ -137,7 +164,7 @@ RSpec.shared_examples 'application settings examples' do
 
       expect(setting.outbound_local_requests_whitelist).to contain_exactly('gitlab.com')
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
-        [], ['gitlab.com']
+        [], [an_object_having_attributes(domain: 'gitlab.com')]
       )
     end