kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch 'fix-unathorized-cloning' into 'security'

Browse source 
Ensure external users are not able to clone disabled repositories.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23788

See merge request !2017

Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
Douwe Maan 2016-11-02 21:50:44 +00:00 committed by Rémy Coutable
parent a14ee68fe4
commit b0bf92140f
No known key found for this signature in database
GPG key ID: 46DF07E5CD9E96AB
11 changed files with 187 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
app/controllers/projects/git_http_client_controller.rb
View file

@ -21,10 +21,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
def authenticate_user
@authentication_result = Gitlab::Auth::Result.new
if project && project.public? && download_request?
return # Allow access
end
if allow_basic_auth? && basic_auth_provided?
login, password = user_name_and_password(request)
@ -41,6 +37,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController
send_final_spnego_response
return # Allow access
end
elsif project && download_request? && Guest.can?(:download_code, project)
@authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code])
return # Allow access
end
send_challenges

6
app/controllers/projects/git_http_controller.rb
View file

@ -78,11 +78,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
def upload_pack_allowed?
return false unless Gitlab.config.gitlab_shell.upload_pack
if user
access_check.allowed?
else
ci? || project.public?
end
access_check.allowed? || ci?
end
def access

4
app/helpers/lfs_helper.rb
View file

@ -1,6 +1,6 @@
module LfsHelper
include Gitlab::Routing.url_helpers
def require_lfs_enabled!
return if Gitlab.config.lfs.enabled
@ -27,7 +27,7 @@ module LfsHelper
def lfs_download_access?
return false unless project.lfs_enabled?
project.public? || ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
end
def user_can_download_code?

7
app/models/guest.rb Normal file
View file

@ -0,0 +1,7 @@
class Guest
class << self
def can?(action, subject)
Ability.allowed?(nil, action, subject)
end
end
end

2
app/services/auth/container_registry_authentication_service.rb
View file

@ -76,7 +76,7 @@ module Auth
case requested_action
when 'pull'
requested_project.public? || build_can_pull?(requested_project) || user_can_pull?(requested_project)
build_can_pull?(requested_project) || user_can_pull?(requested_project)
when 'push'
build_can_push?(requested_project) || user_can_push?(requested_project)
else

91
lib/gitlab/git_access.rb
View file

@ -2,8 +2,18 @@
# class return an instance of `GitlabAccessStatus`
module Gitlab
class GitAccess
UnauthorizedError = Class.new(StandardError)
ERROR_MESSAGES = {
upload: 'You are not allowed to upload code for this project.',
download: 'You are not allowed to download code from this project.',
deploy_key: 'Deploy keys are not allowed to push code.',
no_repo: 'A repository for this project does not exist yet.'
}
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
PUSH_COMMANDS = %w{ git-receive-pack }
ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS
attr_reader :actor, :project, :protocol, :user_access, :authentication_abilities
@ -16,56 +26,43 @@ module Gitlab
end
def check(cmd, changes)
return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed?
unless actor
return build_status_object(false, "No user or key was provided.")
end
if user && !user_access.allowed?
return build_status_object(false, "Your account has been blocked.")
end
unless project && (user_access.can_read_project? || deploy_key_can_read_project?)
return build_status_object(false, 'The project you were looking for could not be found.')
end
check_protocol!
check_active_user!
check_project_accessibility!
check_command_existence!(cmd)
case cmd
when *DOWNLOAD_COMMANDS
download_access_check
when *PUSH_COMMANDS
push_access_check(changes)
else
build_status_object(false, "The command you're trying to execute is not allowed.")
end
build_status_object(true)
rescue UnauthorizedError => ex
build_status_object(false, ex.message)
end
def download_access_check
if user
user_download_access_check
elsif deploy_key
build_status_object(true)
else
raise 'Wrong actor'
elsif deploy_key.nil? && !Guest.can?(:download_code, project)
raise UnauthorizedError, ERROR_MESSAGES[:download]
end
end
def push_access_check(changes)
if user
user_push_access_check(changes)
elsif deploy_key
build_status_object(false, "Deploy keys are not allowed to push code.")
else
raise 'Wrong actor'
raise UnauthorizedError, ERROR_MESSAGES[deploy_key ? :deploy_key : :upload]
end
end
def user_download_access_check
unless user_can_download_code? || build_can_download_code?
return build_status_object(false, "You are not allowed to download code from this project.")
raise UnauthorizedError, ERROR_MESSAGES[:download]
end
build_status_object(true)
end
def user_can_download_code?
@ -78,15 +75,15 @@ module Gitlab
def user_push_access_check(changes)
unless authentication_abilities.include?(:push_code)
return build_status_object(false, "You are not allowed to upload code for this project.")
raise UnauthorizedError, ERROR_MESSAGES[:upload]
end
if changes.blank?
return build_status_object(true)
return # Allow access.
end
unless project.repository.exists?
return build_status_object(false, "A repository for this project does not exist yet.")
raise UnauthorizedError, ERROR_MESSAGES[:no_repo]
end
changes_list = Gitlab::ChangesList.new(changes)
@ -96,11 +93,9 @@ module Gitlab
status = change_access_check(change)
unless status.allowed?
# If user does not have access to make at least one change - cancel all push
return status
raise UnauthorizedError, status.message
end
end
build_status_object(true)
end
def change_access_check(change)
@ -113,6 +108,30 @@ module Gitlab
private
def check_protocol!
unless protocol_allowed?
raise UnauthorizedError, "Git access over #{protocol.upcase} is not allowed"
end
end
def check_active_user!
if user && !user_access.allowed?
raise UnauthorizedError, "Your account has been blocked."
end
end
def check_project_accessibility!
if project.blank? || !can_read_project?
raise UnauthorizedError, 'The project you were looking for could not be found.'
end
end
def check_command_existence!(cmd)
unless ALL_COMMANDS.include?(cmd)
raise UnauthorizedError, "The command you're trying to execute is not allowed."
end
end
def matching_merge_request?(newrev, branch_name)
Checks::MatchingMergeRequest.new(newrev, branch_name, project).match?
end
@ -130,6 +149,16 @@ module Gitlab
end
end
def can_read_project?
if user
user_access.can_read_project?
elsif deploy_key
deploy_key_can_read_project?
else
Guest.can?(:read_project, project)
end
end
protected
def user

10
spec/factories/projects.rb
View file

@ -49,13 +49,17 @@ FactoryGirl.define do
end
after(:create) do |project, evaluator|
# Builds and MRs can't have higher visibility level than repository access level.
builds_access_level = [evaluator.builds_access_level, evaluator.repository_access_level].min
merge_requests_access_level = [evaluator.merge_requests_access_level, evaluator.repository_access_level].min
project.project_feature.
update_attributes(
update_attributes!(
wiki_access_level: evaluator.wiki_access_level,
builds_access_level: evaluator.builds_access_level,
builds_access_level: builds_access_level,
snippets_access_level: evaluator.snippets_access_level,
issues_access_level: evaluator.issues_access_level,
merge_requests_access_level: evaluator.merge_requests_access_level,
merge_requests_access_level: merge_requests_access_level,
repository_access_level: evaluator.repository_access_level
)
end

25
spec/lib/gitlab/git_access_spec.rb
View file

@ -66,6 +66,7 @@ describe Gitlab::GitAccess, lib: true do
context 'pull code' do
it { expect(subject.allowed?).to be_falsey }
it { expect(subject.message).to match(/You are not allowed to download code/) }
end
end
@ -77,6 +78,7 @@ describe Gitlab::GitAccess, lib: true do
context 'pull code' do
it { expect(subject.allowed?).to be_falsey }
it { expect(subject.message).to match(/Your account has been blocked/) }
end
end
@ -84,6 +86,29 @@ describe Gitlab::GitAccess, lib: true do
context 'pull code' do
it { expect(subject.allowed?).to be_falsey }
end
context 'when project is public' do
let(:public_project) { create(:project, :public) }
let(:guest_access) { Gitlab::GitAccess.new(nil, public_project, 'web', authentication_abilities: []) }
subject { guest_access.check('git-upload-pack', '_any') }
context 'when repository is enabled' do
it 'give access to download code' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::ENABLED)
expect(subject.allowed?).to be_truthy
end
end
context 'when repository is disabled' do
it 'does not give access to download code' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
expect(subject.allowed?).to be_falsey
expect(subject.message).to match(/You are not allowed to download code/)
end
end
end
end
describe 'deploy key permissions' do

2
spec/lib/gitlab/git_access_wiki_spec.rb
View file

@ -18,7 +18,7 @@ describe Gitlab::GitAccessWiki, lib: true do
project.team << [user, :developer]
end
subject { access.push_access_check(changes) }
subject { access.check('git-receive-pack', changes) }
it { expect(subject.allowed?).to be_truthy }
end

47
spec/models/guest_spec.rb Normal file
View file

@ -0,0 +1,47 @@
require 'spec_helper'
describe Guest, lib: true do
let(:public_project) { create(:project, :public) }
let(:private_project) { create(:project, :private) }
let(:internal_project) { create(:project, :internal) }
describe '.can_pull?' do
context 'when project is private' do
it 'does not allow to pull the repo' do
expect(Guest.can?(:download_code, private_project)).to eq(false)
end
end
context 'when project is internal' do
it 'does not allow to pull the repo' do
expect(Guest.can?(:download_code, internal_project)).to eq(false)
end
end
context 'when project is public' do
context 'when repository is disabled' do
it 'does not allow to pull the repo' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
expect(Guest.can?(:download_code, public_project)).to eq(false)
end
end
context 'when repository is accessible only by team members' do
it 'does not allow to pull the repo' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::PRIVATE)
expect(Guest.can?(:download_code, public_project)).to eq(false)
end
end
context 'when repository is enabled' do
it 'allows to pull the repo' do
public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::ENABLED)
expect(Guest.can?(:download_code, public_project)).to eq(true)
end
end
end
end
end

32
spec/requests/git_http_spec.rb
View file

@ -115,6 +115,38 @@ describe 'Git HTTP requests', lib: true do
end.to raise_error(JWT::DecodeError)
end
end
context 'when the repo is public' do
context 'but the repo is disabled' do
it 'does not allow to clone the repo' do
project = create(:project, :public, repository_access_level: ProjectFeature::DISABLED)
download("#{project.path_with_namespace}.git", {}) do |response|
expect(response).to have_http_status(:unauthorized)
end
end
end
context 'but the repo is enabled' do
it 'allows to clone the repo' do
project = create(:project, :public, repository_access_level: ProjectFeature::ENABLED)
download("#{project.path_with_namespace}.git", {}) do |response|
expect(response).to have_http_status(:ok)
end
end
end
context 'but only project members are allowed' do
it 'does not allow to clone the repo' do
project = create(:project, :public, repository_access_level: ProjectFeature::PRIVATE)
download("#{project.path_with_namespace}.git", {}) do |response|
expect(response).to have_http_status(:unauthorized)
end
end
end
end
end
context "when the project is private" do