Commit 11a52f73 authored by Michael Kozono's avatar Michael Kozono

Merge branch 'ag-block-git-push-when-read-only' into 'master'

Block git push over HTTP when database is in read-only mode

See merge request gitlab-org/gitlab!47673
parents 2025e7da 805e9886
---
title: Block git push over HTTP when database is read-only
merge_request: 47673
author:
type: changed
...@@ -16,11 +16,15 @@ module EE ...@@ -16,11 +16,15 @@ module EE
'admin/geo/uploads' => %w{destroy} 'admin/geo/uploads' => %w{destroy}
}.freeze }.freeze
ALLOWLISTED_GIT_WRITE_ROUTES = {
'repositories/git_http' => %w{git_receive_pack}
}.freeze
private private
override :allowlisted_routes override :allowlisted_routes
def allowlisted_routes def allowlisted_routes
super || geo_node_update_route? || geo_proxy_git_ssh_route? || geo_api_route? super || geo_node_update_route? || geo_proxy_git_ssh_route? || geo_api_route? || geo_proxy_git_http_route?
end end
def geo_node_update_route? def geo_node_update_route?
...@@ -43,6 +47,12 @@ module EE ...@@ -43,6 +47,12 @@ module EE
end end
end end
def geo_proxy_git_http_route?
return unless request.path.end_with?('.git/git-receive-pack')
ALLOWLISTED_GIT_WRITE_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end
def geo_api_route? def geo_api_route?
::Gitlab::Middleware::ReadOnly::API_VERSIONS.any? do |version| ::Gitlab::Middleware::ReadOnly::API_VERSIONS.any? do |version|
request.path.include?("/api/v#{version}/geo_replication") request.path.include?("/api/v#{version}/geo_replication")
......
...@@ -43,5 +43,12 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do ...@@ -43,5 +43,12 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do
expect(response).not_to be_redirect expect(response).not_to be_redirect
expect(subject).not_to disallow_request expect(subject).not_to disallow_request
end end
it 'expects a POST request to git-receive-pack URL to be allowed' do
response = request.post('/root/rouge.git/git-receive-pack')
expect(response).not_to be_redirect
expect(subject).not_to disallow_request
end
end end
end end
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance' ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
ALLOWLISTED_GIT_ROUTES = { ALLOWLISTED_GIT_ROUTES = {
'repositories/git_http' => %w{git_upload_pack git_receive_pack} 'repositories/git_http' => %w{git_upload_pack}
}.freeze }.freeze
ALLOWLISTED_GIT_LFS_ROUTES = { ALLOWLISTED_GIT_LFS_ROUTES = {
...@@ -96,7 +96,7 @@ module Gitlab ...@@ -96,7 +96,7 @@ module Gitlab
def workhorse_passthrough_route? def workhorse_passthrough_route?
# Calling route_hash may be expensive. Only do it if we think there's a possible match # Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.post? && return false unless request.post? &&
request.path.end_with?('.git/git-upload-pack', '.git/git-receive-pack') request.path.end_with?('.git/git-upload-pack')
ALLOWLISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) ALLOWLISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
......
...@@ -128,7 +128,6 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do ...@@ -128,7 +128,6 @@ RSpec.shared_examples 'write access for a read-only GitLab instance' do
'LFS request to locks create' | '/root/rouge.git/info/lfs/locks' 'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock' 'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
'request to git-upload-pack' | '/root/rouge.git/git-upload-pack' 'request to git-upload-pack' | '/root/rouge.git/git-upload-pack'
'request to git-receive-pack' | '/root/rouge.git/git-receive-pack'
end end
with_them do with_them do
......
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