Merge branch 'ssrf' into 'security'

Protect server against SSRF in project import URLs

See merge request !2068

app/models/project.rb

@@ -193,6 +193,7 @@ class Project < ActiveRecord::Base
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
   validates :import_url, addressable_url: true, if: :external_import?
+  validates :import_url, importable_url: true, if: [:external_import?, :import_url_changed?]
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :avatar_type,

app/services/projects/import_service.rb

@@ -33,6 +33,7 @@ module Projects
     def import_repository
       begin
+        raise Error, "Blocked import URL." if Gitlab::UrlBlocker.blocked_url?(project.import_url)
         gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url)
       rescue => e
         # Expire cache to prevent scenarios such as:
@@ -40,7 +41,7 @@ module Projects
         # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
         project.repository.before_import if project.repository_exists?
-        raise Error,  "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}"
+        raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}"
       end
     end

app/validators/importable_url_validator.rb

+# ImportableUrlValidator
+#
+# This validator blocks projects from using dangerous import_urls to help
+# protect against Server-side Request Forgery (SSRF).
+class ImportableUrlValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    if Gitlab::UrlBlocker.blocked_url?(value)
+      record.errors.add(attribute, "imports are not allowed from that URL")
+    end
+  end
+end

changelogs/unreleased/ssrf-protections.yml

+---
+title: To protect against Server-side Request Forgery project import URLs are now prohibited against localhost or the server IP except for the assigned instance URL and port. Imports are also prohibited from ports below 1024 with the exception of ports 22, 80, and 443.
+merge_request:
+author:

lib/gitlab/url_blocker.rb

+require 'resolv'
+
+module Gitlab
+  class UrlBlocker
+    class << self
+      # Used to specify what hosts and port numbers should be prohibited for project
+      # imports.
+      VALID_PORTS = [22, 80, 443].freeze
+
+      def blocked_url?(url)
+        blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"]
+        blocked_ips.concat(Socket.ip_address_list.map(&:ip_address))
+
+        begin
+          uri = Addressable::URI.parse(url)
+          # Allow imports from the GitLab instance itself but only from the configured ports
+          return false if internal?(uri)
+
+          return true if blocked_port?(uri.port)
+
+          server_ips = Resolv.getaddresses(uri.hostname)
+          return true if (blocked_ips & server_ips).any?
+        rescue Addressable::URI::InvalidURIError
+          return true
+        end
+
+        false
+      end
+
+      private
+
+      def blocked_port?(port)
+        return false if port.blank?
+
+        port < 1024 && !VALID_PORTS.include?(port)
+      end
+
+      def internal?(uri)
+        internal_web?(uri) || internal_shell?(uri)
+      end
+
+      def internal_web?(uri)
+        uri.hostname == config.gitlab.host &&
+          (uri.port.blank? || uri.port == config.gitlab.port)
+      end
+
+      def internal_shell?(uri)
+        uri.hostname == config.gitlab_shell.ssh_host &&
+          (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
+      end
+
+      def config
+        Gitlab.config
+      end
+    end
+  end
+end

spec/lib/gitlab/url_blocker_spec.rb

+require 'spec_helper'
+
+describe Gitlab::UrlBlocker, lib: true do
+  describe '#blocked_url?' do
+    it 'allows imports from configured web host and port' do
+      import_url = "http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git"
+      expect(described_class.blocked_url?(import_url)).to be false
+    end
+
+    it 'allows imports from configured SSH host and port' do
+      import_url = "http://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
+      expect(described_class.blocked_url?(import_url)).to be false
+    end
+
+    it 'returns true for bad localhost hostname' do
+      expect(described_class.blocked_url?('https://localhost:65535/foo/foo.git')).to be true
+    end
+
+    it 'returns true for bad port' do
+      expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true
+    end
+
+    it 'returns true for invalid URL' do
+      expect(described_class.blocked_url?('http://:8080')).to be true
+    end
+
+    it 'returns false for legitimate URL' do
+      expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
+    end
+  end
+end

spec/models/project_spec.rb

@@ -218,6 +218,20 @@ describe Project, models: true do
       expect(project2.import_data).to be_nil
     end
+    it "does not allow blocked import_url localhost" do
+      project2 = build(:empty_project, import_url: 'http://localhost:9000/t.git')
+
+      expect(project2).to be_invalid
+      expect(project2.errors[:import_url]).to include('imports are not allowed from that URL')
+    end
+
+    it "does not allow blocked import_url port" do
+      project2 = build(:empty_project, import_url: 'http://github.com:25/t.git')
+
+      expect(project2).to be_invalid
+      expect(project2.errors[:import_url]).to include('imports are not allowed from that URL')
+    end
+
     describe 'project pending deletion' do
       let!(:project_pending_deletion) do
         create(:empty_project,

spec/services/projects/import_service_spec.rb

@@ -120,6 +120,26 @@ describe Projects::ImportService, services: true do
       end
     end
+    context 'with blocked import_URL' do
+      it 'fails with localhost' do
+        project.import_url = 'https://localhost:9000/vim/vim.git'
+
+        result = described_class.new(project, user).execute
+
+        expect(result[:status]).to eq :error
+        expect(result[:message]).to end_with 'Blocked import URL.'
+      end
+
+      it 'fails with port 25' do
+        project.import_url = "https://github.com:25/vim/vim.git"
+
+        result = described_class.new(project, user).execute
+
+        expect(result[:status]).to eq :error
+        expect(result[:message]).to end_with 'Blocked import URL.'
+      end
+    end
+
     def stub_github_omniauth_provider
       provider = OpenStruct.new(
         'name' => 'github',