diff --git a/config/audit_events/types/security_policy_access_token_push_bypass.yml b/config/audit_events/types/security_policy_access_token_push_bypass.yml new file mode 100644 index 0000000000000000000000000000000000000000..b6a6830fdedfe5d2414d22ad544dd38607242e7b --- /dev/null +++ b/config/audit_events/types/security_policy_access_token_push_bypass.yml @@ -0,0 +1,10 @@ +--- +name: security_policy_access_token_push_bypass +description: Branch push that is blocked by a security policy is bypassed for configured access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549644 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249 +feature_category: security_policy_management +milestone: '18.2' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/config/audit_events/types/security_policy_service_account_push_bypass.yml b/config/audit_events/types/security_policy_service_account_push_bypass.yml new file mode 100644 index 0000000000000000000000000000000000000000..926a02cecc72da0cd2ddd02691924a6ec4c13953 --- /dev/null +++ b/config/audit_events/types/security_policy_service_account_push_bypass.yml @@ -0,0 +1,10 @@ +--- +name: security_policy_service_account_push_bypass +description: Branch push that is blocked by a security policy is bypassed for configured service account +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549644 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249 +feature_category: security_policy_management +milestone: '18.2' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index cfa97ca3e2d47fd30be3d2ac2b3914469ec88153..883bd0be7119dfe23184ea5777d0196fbb36e5bd 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -546,6 +546,8 @@ Audit event types belong to the following product categories. | [`policy_violations_detected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violation is detected in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549811) | Project | | [`policy_violations_resolved`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violations are resolved in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549812) | Project | | [`policy_yaml_invalidated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196721) | The policy YAML is invalidated in security policy project | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/550892) | Project | +| [`security_policy_access_token_push_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249) | Branch push that is blocked by a security policy is bypassed for configured access token | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549644) | Project | +| [`security_policy_service_account_push_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249) | Branch push that is blocked by a security policy is bypassed for configured service account | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549644) | Project | ### Security testing configuration diff --git a/ee/app/models/security/policy.rb b/ee/app/models/security/policy.rb index fc61c728f94b9407e1a95bd19e8f8b9334c4b4db..604842faca364f8811f9c38257e492197b71d334 100644 --- a/ee/app/models/security/policy.rb +++ b/ee/app/models/security/policy.rb @@ -74,6 +74,10 @@ class Policy < ApplicationRecord where("content->'actions' @> ?", [{ role_approvers: [custom_role_id] }].to_json) end + scope :with_bypass_settings, -> do + where("content->'bypass_settings' IS NOT NULL").where("content->'bypass_settings' <> ?", '{}') + end + delegate :namespace?, :namespace, :project?, :project, to: :security_orchestration_policy_configuration def self.checksum(policy_hash) @@ -314,6 +318,11 @@ def enforced_scans=(scans) metadata['enforced_scans'] = scans end + def bypass_settings + Security::ScanResultPolicies::BypassSettings.new(policy_content[:bypass_settings]) + end + strong_memoize_attr :bypass_settings + private def content_by_type diff --git a/ee/lib/ee/gitlab/checks/security/policy_check.rb b/ee/lib/ee/gitlab/checks/security/policy_check.rb index 4189428d9045e3801b625b8bd4e9e99364aa3e3a..903fddd8e35995d38cbeb8fb997b33bc7a10a881 100644 --- a/ee/lib/ee/gitlab/checks/security/policy_check.rb +++ b/ee/lib/ee/gitlab/checks/security/policy_check.rb @@ -14,6 +14,7 @@ def validate! logger.log_timed(LOG_MESSAGE) do break unless branch_name_affected_by_policy? + break if policies_bypass_applied? if force_push? raise ::Gitlab::GitAccess::ForbiddenError, FORCE_PUSH_ERROR_MESSAGE @@ -32,6 +33,14 @@ def branch_name_affected_by_policy? branch_name.in? affected_branches end + def policies_bypass_applied? + return false if ::Feature.disabled?(:security_policies_bypass_options_tokens_accounts, project) + + ::Security::ScanResultPolicies::PushBypassChecker.new( + project: project, user_access: user_access, branch_name: branch_name + ).check_bypass! + end + def force_push? ::Gitlab::Checks::ForcePush.force_push?(project, oldrev, newrev) end diff --git a/ee/lib/security/scan_result_policies/bypass_settings.rb b/ee/lib/security/scan_result_policies/bypass_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..7022d805871b296716696099d8c05bfbf43062a6 --- /dev/null +++ b/ee/lib/security/scan_result_policies/bypass_settings.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class BypassSettings + include Gitlab::Utils::StrongMemoize + + def initialize(bypass_settings) + @bypass_settings = bypass_settings || {} + end + + def access_token_ids + bypass_settings[:access_tokens]&.pluck(:id) + end + strong_memoize_attr :access_token_ids + + def service_account_ids + bypass_settings[:service_accounts]&.pluck(:id) + end + strong_memoize_attr :service_account_ids + + private + + attr_reader :bypass_settings + end + end +end diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb new file mode 100644 index 0000000000000000000000000000000000000000..69f68b3278502a0f26a6b16dec330e11e98841f5 --- /dev/null +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class PolicyBypassChecker + include Gitlab::Utils::StrongMemoize + + def initialize(security_policy:, project:, user_access:, branch_name:) + @security_policy = security_policy + @project = project + @user = user_access.user + @branch_name = branch_name + end + + def bypass_allowed? + return false unless user + + bypass_with_access_token? || bypass_with_service_account? + end + + def bypass_with_access_token? + policy_token_ids = security_policy.bypass_settings.access_token_ids + return false if policy_token_ids.blank? + + return false unless user.project_bot? + + user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key + return false if user_token_ids.blank? + + log_bypass_audit!(:access_token, user_token_ids) + true + end + + def bypass_with_service_account? + policy_service_account_ids = security_policy.bypass_settings.service_account_ids + return false if policy_service_account_ids.blank? + + return false unless user.service_account? + return false unless policy_service_account_ids.include?(user.id) + + log_bypass_audit!(:service_account, user.id) + true + end + + private + + attr_reader :security_policy, :project, :user, :branch_name + + def log_bypass_audit!(type, id) + Gitlab::Audit::Auditor.audit( + name: "security_policy_#{type}_push_bypass", + author: user, + scope: project, + target: security_policy, + message: + "Blocked branch push is bypassed by security policy '#{security_policy.name}' for #{type} with ID: #{id}", + additional_details: { + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + } + ) + end + end + end +end diff --git a/ee/lib/security/scan_result_policies/push_bypass_checker.rb b/ee/lib/security/scan_result_policies/push_bypass_checker.rb new file mode 100644 index 0000000000000000000000000000000000000000..92d6dc600869d03824cd85558567a8df5cf1ed39 --- /dev/null +++ b/ee/lib/security/scan_result_policies/push_bypass_checker.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class PushBypassChecker + include Gitlab::Utils::StrongMemoize + + def initialize(project:, user_access:, branch_name:) + @project = project + @user_access = user_access + @branch_name = branch_name + end + + def check_bypass! + return unless project.licensed_feature_available?(:security_orchestration_policies) + + policies = project.security_policies.with_bypass_settings + return if policies.empty? + + policies.any? { |policy| bypass_allowed?(policy) } + end + + private + + attr_reader :project, :user_access, :branch_name + + def bypass_allowed?(policy) + Security::ScanResultPolicies::PolicyBypassChecker.new( + security_policy: policy, + project: project, + user_access: user_access, + branch_name: branch_name + ).bypass_allowed? + end + end + end +end diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index 57d7504ff4c8021d6bf831d9b63a54bc88c045fb..239fdaa90f214f36a8644311c5c3591c46079e09 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -47,6 +47,27 @@ end end + transient do + bypass_access_token_ids { [] } + bypass_service_account_ids { [] } + end + + after(:build) do |policy, evaluator| + next if evaluator.bypass_access_token_ids.blank? + + policy.content ||= {} + policy.content[:bypass_settings] ||= {} + policy.content[:bypass_settings][:access_tokens] = evaluator.bypass_access_token_ids.map do |token_id| + { id: token_id } + end + + next if evaluator.bypass_service_account_ids.blank? + + policy.content[:bypass_settings] ||= {} + policy.content[:bypass_settings][:service_accounts] = evaluator + .bypass_service_account_ids.map { |service_account_id| { id: service_account_id } } + end + trait :deleted do policy_index { -1 } end diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index a8363c351081a8b61f7186ec4c17f9d30e70bfdd..c242699ded0f27e184b53001a321aae63d74f6f8 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -101,5 +101,46 @@ expect { policy_check! }.not_to raise_error end end + + context 'with bypass_settings' do + let_it_be(:user) { create(:user, :project_bot) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + context 'when policies_bypass_applied? allows bypass with access token' do + before do + create(:security_policy, linked_projects: [project], bypass_access_token_ids: [personal_access_token.id]) + end + + it 'does not raise' do + expect { policy_check! }.not_to raise_error + end + end + + context 'when policies_bypass_applied? does not allow bypass with access token' do + before do + # Create a policy with a different access token, so the current token is not allowed + another_token = create(:personal_access_token) + create(:security_policy, linked_projects: [project], bypass_access_token_ids: [another_token.id]) + end + + it 'raises error' do + expect { policy_check! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, described_class::FORCE_PUSH_ERROR_MESSAGE + ) + end + end + end + + context 'when the security_policies_bypass_options_tokens_accounts feature flag is disabled' do + before do + stub_feature_flags(security_policies_bypass_options_tokens_accounts: false) + end + + it 'does not bypass and raises error' do + expect { policy_check! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, described_class::FORCE_PUSH_ERROR_MESSAGE + ) + end + end end end diff --git a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a5edd9e29c3fb296ef8081c076fb0e55bec301ee --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::BypassSettings, feature_category: :security_policy_management do + let_it_be(:bypass_settings) do + { + access_tokens: [ + { id: 1 }, + { id: 2 } + ], + service_accounts: [ + { id: 10 }, + { id: 20 } + ] + } + end + + subject(:bypass_settings_instance) { described_class.new(bypass_settings) } + + describe '#access_token_ids' do + it 'returns the ids of access tokens' do + expect(bypass_settings_instance.access_token_ids).to match_array([1, 2]) + end + + context 'when access_tokens is nil' do + let_it_be(:bypass_settings) { {} } + + it 'returns nil' do + expect(bypass_settings_instance.access_token_ids).to be_nil + end + end + end + + describe '#service_account_ids' do + it 'returns the ids of service accounts' do + expect(bypass_settings_instance.service_account_ids).to match_array([10, 20]) + end + + context 'when service_accounts is nil' do + let_it_be(:bypass_settings) { {} } + + it 'returns nil' do + expect(bypass_settings_instance.service_account_ids).to be_nil + end + end + end +end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..375f327696727547e029e9fcf48d07c24e02b909 --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::PolicyBypassChecker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + let_it_be(:branch_name) { 'main' } + let_it_be(:user) { create(:user, :project_bot) } + let_it_be(:service_account) { create(:service_account) } + + let_it_be_with_refind(:security_policy) do + create(:security_policy, linked_projects: [project], content: { bypass_settings: {} }) + end + + let_it_be(:service_account_access) { Gitlab::UserAccess.new(service_account, container: project) } + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + + describe '#bypass_allowed?' do + subject(:bypass_allowed?) do + described_class.new( + security_policy: security_policy, project: project, user_access: user_access, branch_name: branch_name + ).bypass_allowed? + end + + before do + allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original + end + + shared_examples 'bypass is not allowed and audit log is not created' do + it 'returns false and does not create an audit log' do + result = bypass_allowed? + + expect(result).to be false + expect(Gitlab::Audit::Auditor).not_to have_received(:audit).with( + hash_including(message: a_string_including("Blocked branch push is bypassed by security policy")) + ) + end + end + + context 'when bypass_settings is blank' do + before do + security_policy.update!(content: { actions: [] }) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when bypass_settings has access_tokens' do + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + before do + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } }) + end + + context 'when the access token is inactive' do + before do + personal_access_token.update!(expires_at: 1.day.ago) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when the access token is active' do + before do + personal_access_token.update!(expires_at: nil) + end + + it 'returns true and creates an audit log' do + result = bypass_allowed? + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( + name: 'security_policy_access_token_push_bypass', + author: user, + scope: project, + target: security_policy, + message: a_string_including("Blocked branch push is bypassed by security policy"), + additional_details: hash_including( + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + ) + )) + end + end + + context 'when the access token is not allowed to bypass' do + before do + another_access_token = create(:personal_access_token) + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: another_access_token.id }] } }) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user_access is not a project bot' do + let_it_be(:user_access) do + normal_user = create(:user) + Gitlab::UserAccess.new(normal_user, container: project) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + end + + context 'when bypass_settings has service_accounts' do + before do + security_policy.update!(content: { bypass_settings: { service_accounts: [{ id: service_account.id }] } }) + end + + context 'when user_access is the allowed service account' do + let_it_be(:user_access) { service_account_access } + + it 'returns true and creates an audit log' do + result = bypass_allowed? + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( + name: 'security_policy_service_account_push_bypass', + author: service_account, + scope: project, + target: security_policy, + message: a_string_including("Blocked branch push is bypassed by security policy"), + additional_details: hash_including( + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + ) + )) + end + end + + context 'when user_access is a different service account' do + let_it_be(:user_access) do + other_service_account = create(:service_account) + Gitlab::UserAccess.new(other_service_account, container: project) + end + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + + context 'when user_access is not a service account' do + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + + it_behaves_like 'bypass is not allowed and audit log is not created' + end + end + end +end diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..302a507c2bf88f527d06a0d7d5edec5f1d257573 --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::PushBypassChecker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + + let_it_be(:branch_name) { 'main' } + let_it_be(:user) { create(:user, :project_bot) } + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + let_it_be(:checker) { described_class.new(project: project, user_access: user_access, branch_name: branch_name) } + + describe '#check_bypass!' do + context 'when the feature is not available' do + before do + stub_licensed_features(security_orchestration_policies: false) + end + + it 'returns nil' do + expect(checker.check_bypass!).to be_nil + end + end + + context 'when the feature is available' do + before do + stub_licensed_features(security_orchestration_policies: true) + end + + context 'when there are no policies with bypass settings' do + it 'returns nil' do + expect(checker.check_bypass!).to be_nil + end + end + + context 'when there is a policy with bypass settings for access token' do + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be_with_reload(:security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [personal_access_token.id]) + end + + it 'returns true' do + expect(checker.check_bypass!).to be true + end + + context 'when the access token is not allowed to bypass' do + before do + another_access_token = create(:personal_access_token) + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: another_access_token.id }] } }) + end + + it 'returns false' do + expect(checker.check_bypass!).to be false + end + end + end + + context 'with multiple security policies' do + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + context 'when multiple policies have bypass_settings' do + let_it_be_with_reload(:security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [personal_access_token.id]) + end + + let_it_be_with_reload(:non_matching_security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [999_999]) + end + + it 'returns true if any policy allows bypass' do + expect(checker.check_bypass!).to be true + end + end + + context 'when only one policy has bypass_settings' do + let_it_be_with_reload(:security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [personal_access_token.id]) + end + + let_it_be_with_reload(:non_matching_security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], content: {}) + end + + it 'returns true if the policy with bypass_settings allows bypass' do + expect(checker.check_bypass!).to be true + end + end + + context 'when multiple policies have no bypass_settings' do + let_it_be_with_reload(:security_policy1) do + create(:security_policy, :approval_policy, linked_projects: [project], content: {}) + end + + let_it_be_with_reload(:security_policy2) do + create(:security_policy, :approval_policy, linked_projects: [project], content: {}) + end + + it 'returns nil' do + expect(checker.check_bypass!).to be_nil + end + end + end + end + end +end diff --git a/ee/spec/models/security/policy_spec.rb b/ee/spec/models/security/policy_spec.rb index 0f5ab394f68d70523051620a5098b30ce52201e3..bab7571c489b040cf56c872eeacb90bed0cee702 100644 --- a/ee/spec/models/security/policy_spec.rb +++ b/ee/spec/models/security/policy_spec.rb @@ -1259,4 +1259,58 @@ }) end end + + describe '.with_bypass_settings' do + let_it_be(:policy_with_bypass) do + create(:security_policy, bypass_access_token_ids: [1]) + end + + let_it_be(:policy_without_bypass) do + create(:security_policy, :require_approval) + end + + let_it_be(:policy_with_empty_bypass) { create(:security_policy, content: { bypass_settings: {} }) } + + it 'returns only policies with non-empty bypass_settings' do + result = described_class.with_bypass_settings + expect(result).to contain_exactly(policy_with_bypass) + end + end + + describe '#bypass_settings' do + let(:access_token_id) { 42 } + let(:service_account_id) { 99 } + + context 'when bypass_settings is nil' do + let(:policy) { build(:security_policy, content: {}) } + + it 'returns a BypassSettings object with nil ids' do + expect(policy.bypass_settings.access_token_ids).to be_nil + expect(policy.bypass_settings.service_account_ids).to be_nil + end + end + + context 'when bypass_settings is empty' do + let(:policy) { build(:security_policy, content: { bypass_settings: {} }) } + + it 'returns a BypassSettings object with nil ids' do + expect(policy.bypass_settings.access_token_ids).to be_nil + expect(policy.bypass_settings.service_account_ids).to be_nil + end + end + + context 'when bypass_settings has access_tokens and service_accounts' do + let(:policy) do + build(:security_policy, + bypass_access_token_ids: [access_token_id], + bypass_service_account_ids: [service_account_id] + ) + end + + it 'returns the correct ids' do + expect(policy.bypass_settings.access_token_ids).to contain_exactly(access_token_id) + expect(policy.bypass_settings.service_account_ids).to contain_exactly(service_account_id) + end + end + end end