diff --git a/doc/administration/geo/replication/security_review.md b/doc/administration/geo/replication/security_review.md index f77527ae8a7100a2064d48a06b9aedeeb6362abc..3dcc4774f5f9b89fdb1a4477d19bbeaaa3860463 100644 --- a/doc/administration/geo/replication/security_review.md +++ b/doc/administration/geo/replication/security_review.md @@ -271,7 +271,6 @@ questions from [owasp.org](https://www.owasp.org). - LFS and File ID. - Upload and File ID. - Job Artifact and File ID. -- Geo JWTs scopes are not enforced for Git Access yet, but will be in a future version (currently scheduled for GitLab 11.10). ### What access requirements have been defined for URI and Service calls? diff --git a/ee/app/controllers/ee/projects/git_http_controller.rb b/ee/app/controllers/ee/projects/git_http_controller.rb index c9c512642cfc7d2ce0983a1678bf816a1679ca99..bd1086fc566ebf9c4b5b438674cd22c395870c00 100644 --- a/ee/app/controllers/ee/projects/git_http_controller.rb +++ b/ee/app/controllers/ee/projects/git_http_controller.rb @@ -48,6 +48,7 @@ def access_actor def authenticate_user return super unless geo_request? return render_bad_geo_auth('Bad token') unless decoded_authorization + return render_bad_geo_auth('Unauthorized scope') unless jwt_scope_valid? # grant access @authentication_result = ::Gitlab::Auth::Result.new(nil, project, :geo, [:download_code, :push_code]) # rubocop:disable Gitlab/ModuleWithInstanceVariables @@ -57,6 +58,14 @@ def authenticate_user render_bad_geo_auth("Invalid signature time ") end + def jwt_scope_valid? + decoded_authorization[:scope] == repository.full_path + end + + def repository + wiki? ? project.wiki.repository : project.repository + end + def decoded_authorization strong_memoize(:decoded_authorization) do ::Gitlab::Geo::JwtRequestDecoder.new(request.headers['Authorization']).decode diff --git a/ee/app/services/geo/base_sync_service.rb b/ee/app/services/geo/base_sync_service.rb index d99e0af1ce70d99a04022cba27de0750cf3791d5..980323696db896e102d81a9f412cc22d0ac81909 100644 --- a/ee/app/services/geo/base_sync_service.rb +++ b/ee/app/services/geo/base_sync_service.rb @@ -101,7 +101,7 @@ def fetch_geo_mirror(repository) # Build a JWT header for authentication def jwt_authentication_header authorization = ::Gitlab::Geo::RepoSyncRequest.new( - scope: project.repository.full_path + scope: repository.full_path ).authorization { "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" } diff --git a/ee/changelogs/unreleased/80-geo-enforce-repository-jwt-scopes.yml b/ee/changelogs/unreleased/80-geo-enforce-repository-jwt-scopes.yml new file mode 100644 index 0000000000000000000000000000000000000000..d8dd86b91ac19e7c9ac3b1793a178f95e980b98a --- /dev/null +++ b/ee/changelogs/unreleased/80-geo-enforce-repository-jwt-scopes.yml @@ -0,0 +1,5 @@ +--- +title: Enforce Geo JWT tokens scope for repository sync +merge_request: 10303 +author: +type: changed diff --git a/ee/spec/requests/git_http_geo_spec.rb b/ee/spec/requests/git_http_geo_spec.rb index cc2bad4300c26b761b01773fe6ca6070a570f7ce..c3a45a8fac88852f0c5861c43c168ca1f47c8254 100644 --- a/ee/spec/requests/git_http_geo_spec.rb +++ b/ee/spec/requests/git_http_geo_spec.rb @@ -12,7 +12,7 @@ set(:secondary) { create(:geo_node) } # Ensure the token always comes from the real time of the request - let!(:auth_token) { Gitlab::Geo::BaseRequest.new(scope: "repository-#{project.id}").authorization } + let!(:auth_token) { Gitlab::Geo::BaseRequest.new(scope: project.full_path).authorization } let!(:user) { create(:user) } let!(:user_without_any_access) { create(:user) } let!(:user_without_push_access) { create(:user) } @@ -21,6 +21,7 @@ let!(:key_for_user_without_push_access) { create(:key, user: user_without_push_access) } let(:env) { valid_geo_env } + let(:auth_token_with_invalid_scope) { Gitlab::Geo::BaseRequest.new(scope: "invalid").authorization } before do project.add_maintainer(user) @@ -346,6 +347,53 @@ def make_request end end end + + context 'invalid scope' do + subject do + make_request + response + end + + def make_request + get "/#{repository_path}.git/info/refs", params: { service: 'git-upload-pack' }, headers: env + end + + shared_examples_for 'unauthorized because of invalid scope' do + it { is_expected.to have_gitlab_http_status(:unauthorized) } + + it 'returns correct error' do + expect(subject.parsed_body).to eq('Geo JWT authentication failed: Unauthorized scope') + end + end + + context 'invalid scope of Geo JWT token' do + let(:repository_path) { project.full_path } + let(:env) { geo_env(auth_token_with_invalid_scope) } + + include_examples 'unauthorized because of invalid scope' + end + + context 'Geo JWT token scopes for wiki and repository are not interchangeable' do + context 'for a repository but using a wiki scope' do + let(:repository_path) { project.full_path } + let(:scope) { project.wiki.full_path } + let(:auth_token_with_valid_wiki_scope) { Gitlab::Geo::BaseRequest.new(scope: scope).authorization } + let(:env) { geo_env(auth_token_with_valid_wiki_scope) } + + include_examples 'unauthorized because of invalid scope' + end + + context 'for a wiki but using a repository scope' do + let(:project) { create(:project, :wiki_repo) } + let(:repository_path) { project.wiki.full_path } + let(:scope) { project.full_path } + let(:auth_token_with_valid_repository_scope) { Gitlab::Geo::BaseRequest.new(scope: scope).authorization } + let(:env) { geo_env(auth_token_with_valid_repository_scope) } + + include_examples 'unauthorized because of invalid scope' + end + end + end end def valid_geo_env