summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-03-15 20:09:08 +0000
committerRuben Davila <rdavila84@gmail.com>2017-03-18 13:51:30 -0500
commit026fc91c7b0afd5019590a936ea52168a1f9b0ff (patch)
tree449a532c99a985573b280e2415247f593420287c
parent2d05a0402c270117d71ef61b61e653b5edab6dcf (diff)
downloadgitlab-ce-026fc91c7b0afd5019590a936ea52168a1f9b0ff.tar.gz
Merge branch 'ssrf' into 'security'
Protect server against SSRF in project import URLs See merge request !2068
-rw-r--r--app/models/project.rb1
-rw-r--r--app/services/projects/import_service.rb3
-rw-r--r--app/validators/importable_url_validator.rb11
-rw-r--r--changelogs/unreleased/ssrf-protections.yml4
-rw-r--r--lib/gitlab/url_blocker.rb57
-rw-r--r--spec/lib/gitlab/url_blocker_spec.rb31
-rw-r--r--spec/models/project_spec.rb14
-rw-r--r--spec/services/projects/import_service_spec.rb20
8 files changed, 140 insertions, 1 deletions
diff --git a/app/models/project.rb b/app/models/project.rb
index 4a3e18f07d3..0059841be93 100644
--- a/app/models/project.rb
+++ b/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,
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index cd230528743..f060405630e 100644
--- a/app/services/projects/import_service.rb
+++ b/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
diff --git a/app/validators/importable_url_validator.rb b/app/validators/importable_url_validator.rb
new file mode 100644
index 00000000000..37a314adee6
--- /dev/null
+++ b/app/validators/importable_url_validator.rb
@@ -0,0 +1,11 @@
+# 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
diff --git a/changelogs/unreleased/ssrf-protections.yml b/changelogs/unreleased/ssrf-protections.yml
new file mode 100644
index 00000000000..8d803738009
--- /dev/null
+++ b/changelogs/unreleased/ssrf-protections.yml
@@ -0,0 +1,4 @@
+---
+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:
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
new file mode 100644
index 00000000000..bb2f4edc1a0
--- /dev/null
+++ b/lib/gitlab/url_blocker.rb
@@ -0,0 +1,57 @@
+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
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
new file mode 100644
index 00000000000..a504d299307
--- /dev/null
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -0,0 +1,31 @@
+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
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 2129bcbd74d..bd0353adeca 100644
--- a/spec/models/project_spec.rb
+++ b/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,
diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb
index ab6e8f537ba..e5917bb0b7a 100644
--- a/spec/services/projects/import_service_spec.rb
+++ b/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',