From 08eeba0dfcddd0c6504c6ca60e489340331698b2 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 19 Mar 2019 23:24:10 +0200 Subject: [PATCH] Geo: Enforce repository JWT scopes It was already implemented but we reverted it in http://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9597 We have to bring it back in 11.10. --- .../geo/replication/security_review.md | 1 - .../ee/projects/git_http_controller.rb | 9 ++++ ee/app/services/geo/base_sync_service.rb | 2 +- .../80-geo-enforce-repository-jwt-scopes.yml | 5 ++ ee/spec/requests/git_http_geo_spec.rb | 50 ++++++++++++++++++- 5 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 ee/changelogs/unreleased/80-geo-enforce-repository-jwt-scopes.yml diff --git a/doc/administration/geo/replication/security_review.md b/doc/administration/geo/replication/security_review.md index f77527ae8a7100..3dcc4774f5f9b8 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 c9c512642cfc7d..bd1086fc566ebf 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 d99e0af1ce70d9..980323696db896 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 00000000000000..d8dd86b91ac19e --- /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 cc2bad4300c26b..c3a45a8fac8885 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 -- GitLab