Verified Commit b95918dd authored by Douwe Maan's avatar Douwe Maan 💒

Make error messages even more descriptive

parent 7143bb88
......@@ -10,28 +10,27 @@ module Gitlab
begin
uri = Addressable::URI.parse(url)
# Allow imports from the GitLab instance itself but only from the configured ports
return true if internal?(uri)
raise BlockedUrlError, "Port is blocked" if blocked_port?(uri.port, valid_ports)
raise BlockedUrlError, "User is blocked" if blocked_user_or_hostname?(uri.user)
raise BlockedUrlError, "Hostname is blocked" if blocked_user_or_hostname?(uri.hostname)
rescue Addressable::URI::InvalidURIError
raise BlockedUrlError, "URI is invalid"
end
addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM)
# Allow imports from the GitLab instance itself but only from the configured ports
return true if internal?(uri)
if !allow_localhost && localhost?(addrs_info)
raise BlockedUrlError, "Requests to localhost are blocked"
end
port = uri.port || uri.default_port
validate_port!(port, valid_ports) if valid_ports.any?
validate_user!(uri.user)
validate_hostname!(uri.hostname)
if !allow_private_networks && private_network?(addrs_info)
raise BlockedUrlError, "Requests to the private local network are blocked"
end
rescue Addressable::URI::InvalidURIError
raise BlockedUrlError, "URI is invalid"
begin
addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM)
rescue SocketError
return
return true
end
validate_localhost!(addrs_info) unless allow_localhost
validate_local_network!(addrs_info) unless allow_private_networks
true
end
......@@ -45,16 +44,42 @@ module Gitlab
private
def blocked_port?(port, valid_ports)
return false if port.blank? || valid_ports.blank?
def validate_port!(port, valid_ports)
return if port.blank?
# Only ports under 1024 are restricted
return if port >= 1024
return if valid_ports.include?(port)
raise BlockedUrlError, "Only allowed ports are #{valid_ports.join(', ')}, and any over 1024"
end
def validate_user!(value)
return if value.blank?
return if value =~ /\A\p{Alnum}/
port < 1024 && !valid_ports.include?(port)
raise BlockedUrlError, "Username needs to start with an alphanumeric character"
end
def blocked_user_or_hostname?(value)
return false if value.blank?
def validate_hostname!(value)
return if value.blank?
return if value =~ /\A\p{Alnum}/
value !~ /\A\p{Alnum}/
raise BlockedUrlError, "Hostname needs to start with an alphanumeric character"
end
def validate_localhost!(addrs_info)
local_ips = ["127.0.0.1", "::1", "0.0.0.0"]
local_ips.concat(Socket.ip_address_list.map(&:ip_address))
return if (local_ips & addrs_info.map(&:ip_address)).empty?
raise BlockedUrlError, "Requests to localhost are not allowed"
end
def validate_local_network!(addrs_info)
return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
raise BlockedUrlError, "Requests to the local network are not allowed"
end
def internal?(uri)
......@@ -71,17 +96,6 @@ module Gitlab
(uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
end
def localhost?(addrs_info)
blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"]
blocked_ips.concat(Socket.ip_address_list.map(&:ip_address))
(blocked_ips & addrs_info.map(&:ip_address)).any?
end
def private_network?(addrs_info)
addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
end
def config
Gitlab.config
end
......
......@@ -224,14 +224,14 @@ describe Project do
project2 = build(: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')
expect(project2.errors[:import_url].first).to include('Requests to localhost are not allowed')
end
it "does not allow blocked import_url port" do
project2 = build(: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')
expect(project2.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443')
end
describe 'project pending deletion' do
......
......@@ -156,7 +156,7 @@ describe Projects::ImportService do
result = described_class.new(project, user).execute
expect(result[:status]).to eq :error
expect(result[:message]).to end_with 'Blocked import URL.'
expect(result[:message]).to include('Requests to localhost are not allowed')
end
it 'fails with port 25' do
......@@ -165,7 +165,7 @@ describe Projects::ImportService do
result = described_class.new(project, user).execute
expect(result[:status]).to eq :error
expect(result[:message]).to end_with 'Blocked import URL.'
expect(result[:message]).to include('Only allowed ports are 22, 80, 443')
end
end
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment