Merge branch '32358-add-additional-stats-for-modsecurity-installations' into 'master'

Add usage statistics for modsecurity for packets/anomalous

See merge request gitlab-org/gitlab!28535

app/models/clusters/applications/elastic_stack.rb

@@ -53,7 +53,7 @@ module Clusters
         super.merge('wait-for-elasticsearch.sh': File.read("#{Rails.root}/vendor/elastic_stack/wait-for-elasticsearch.sh"))
       end
 
-      def elasticsearch_client
+      def elasticsearch_client(timeout: nil)
         strong_memoize(:elasticsearch_client) do
           next unless kube_client
 
@@ -65,6 +65,7 @@ module Clusters
             # ensure TLS certs are properly verified
             faraday.ssl[:verify] = kube_client.ssl_options[:verify_ssl]
             faraday.ssl[:cert_store] = kube_client.ssl_options[:cert_store]
+            faraday.options.timeout = timeout unless timeout.nil?
           end
 
         rescue Kubeclient::HttpError => error

app/models/clusters/cluster.rb

@@ -36,6 +36,8 @@ module Clusters
     has_one :cluster_project, -> { order(id: :desc) }, class_name: 'Clusters::Project'
     has_many :deployment_clusters
     has_many :deployments, inverse_of: :cluster
+    has_many :successful_deployments, -> { success }, class_name: 'Deployment'
+    has_many :environments, -> { distinct }, through: :deployments
 
     has_many :cluster_groups, class_name: 'Clusters::Group'
     has_many :groups, through: :cluster_groups, class_name: '::Group'
@@ -125,6 +127,12 @@ module Clusters
     scope :gcp_installed, -> { gcp_provided.joins(:provider_gcp).merge(Clusters::Providers::Gcp.with_status(:created)) }
     scope :aws_installed, -> { aws_provided.joins(:provider_aws).merge(Clusters::Providers::Aws.with_status(:created)) }
 
+    scope :with_enabled_modsecurity, -> { joins(:application_ingress).merge(::Clusters::Applications::Ingress.modsecurity_enabled) }
+    scope :with_available_elasticstack, -> { joins(:application_elastic_stack).merge(::Clusters::Applications::ElasticStack.available) }
+    scope :distinct_with_deployed_environments, -> { joins(:environments).merge(::Deployment.success).distinct }
+    scope :preload_elasticstack, -> { preload(:application_elastic_stack) }
+    scope :preload_environments, -> { preload(:environments) }
+
     scope :managed, -> { where(managed: true) }
     scope :with_persisted_applications, -> { eager_load(*APPLICATIONS_ASSOCIATIONS) }
     scope :default_environment, -> { where(environment_scope: DEFAULT_ENVIRONMENT) }

config/initializers/1_settings.rb

@@ -567,6 +567,9 @@ Gitlab.ee do
   Settings.cron_jobs['sync_seat_link_worker'] ||= Settingslogic.new({})
   Settings.cron_jobs['sync_seat_link_worker']['cron'] ||= "#{rand(60)} 0 * * *"
   Settings.cron_jobs['sync_seat_link_worker']['job_class'] = 'SyncSeatLinkWorker'
+  Settings.cron_jobs['web_application_firewall_metrics_worker'] ||= Settingslogic.new({})
+  Settings.cron_jobs['web_application_firewall_metrics_worker']['cron'] ||= '0 1 * * 0'
+  Settings.cron_jobs['web_application_firewall_metrics_worker']['job_class'] = 'IngressModsecurityCounterMetricsWorker'
 end
 
 #

ee/app/finders/ee/notes_finder.rb

@@ -10,7 +10,7 @@ module EE
       if noteable_type == 'epic'
         return EpicsFinder.new(@current_user, group_id: @params[:group_id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
       elsif noteable_type == 'vulnerability'
-        return Security::VulnerabilitiesFinder.new(@project) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+        return ::Security::VulnerabilitiesFinder.new(@project) # rubocop:disable Gitlab/ModuleWithInstanceVariables
       end
 
       super

ee/app/services/ee/security/ingress_modsecurity_usage_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Security
+    ##
+    # This service measures usage of the Modsecurity Web Application Firewall across the entire
+    # instance's deployed environments.
+    #
+    ##
+    class IngressModsecurityUsageService
+      BATCH_SIZE = 1
+
+      def initialize
+        @statistics_unavailable_count = 0
+        @packets_processed_count = 0
+        @packets_anomalous_count = 0
+      end
+
+      # rubocop: disable CodeReuse/ActiveRecord
+      def execute
+        clusters_with_enabled_modsecurity.find_each(batch_size: BATCH_SIZE) do |cluster|
+          cluster.environments.each do |environment|
+            result = anomaly_results_for_cluster_and_environment(cluster, environment)
+            if result.nil?
+              @statistics_unavailable_count += 1
+            else
+              @packets_processed_count += result[:total_traffic]
+              @packets_anomalous_count += result[:total_anomalous_traffic]
+            end
+          end
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        {
+          statistics_unavailable: @statistics_unavailable_count.to_i,
+          packets_processed: @packets_processed_count.to_i,
+          packets_anomalous: @packets_anomalous_count.to_i
+        }
+      end
+
+      private
+
+      def anomaly_results_for_cluster_and_environment(cluster, environment)
+        # As defined in config/initializers/1_settings.rb#562, IngressModsecurityCounterMetricsWorker will be executed
+        # once a week. That is why when we are collecting data from clusters we are querying for the last 7 days.
+        ::Security::WafAnomalySummaryService
+          .new(environment: environment, cluster: cluster, from: 7.days.ago.iso8601, options: { timeout: 10 })
+          .execute(totals_only: true)
+      rescue => e
+        ::Gitlab::ErrorTracking.track_exception(e, environment_id: environment&.id, cluster_id: cluster&.id)
+        nil
+      end
+
+      def clusters_with_enabled_modsecurity
+        ::Clusters::Cluster
+          .with_enabled_modsecurity
+          .with_available_elasticstack
+          .distinct_with_deployed_environments
+          .preload_elasticstack
+          .preload_environments
+      end
+    end
+  end
+end

ee/app/services/security/waf_anomaly_summary_service.rb

@@ -5,14 +5,16 @@ module Security
   # Queries ES and retrieves both total nginx requests & modsec violations
   #
   class WafAnomalySummaryService < ::BaseService
-    def initialize(environment:, interval: 'day', from: 30.days.ago.iso8601, to: Time.zone.now.iso8601)
+    def initialize(environment:, cluster: environment.deployment_platform&.cluster, interval: 'day', from: 30.days.ago.iso8601, to: Time.zone.now.iso8601, options: {})
       @environment = environment
+      @cluster = cluster
       @interval = interval
       @from = from
       @to = to
+      @options = options
     end
 
-    def execute
+    def execute(totals_only: false)
       return if elasticsearch_client.nil?
       return unless @environment.external_url
 
@@ -29,6 +31,8 @@ module Security
         modsec_total_requests = modsec_results.dig('hits', 'total').to_f
       end
 
+      return { total_traffic: nginx_total_requests, total_anomalous_traffic: modsec_total_requests } if totals_only
+
       anomalous_traffic_count = nginx_total_requests.zero? ? 0 : (modsec_total_requests / nginx_total_requests).round(2)
 
       {
@@ -46,13 +50,13 @@ module Security
     end
 
     def elasticsearch_client
-      @elasticsearch_client ||= application_elastic_stack&.elasticsearch_client
+      @elasticsearch_client ||= application_elastic_stack&.elasticsearch_client(timeout: @options[:timeout])
     end
 
     private
 
     def application_elastic_stack
-      @application_elastic_stack ||= @environment.deployment_platform&.cluster&.application_elastic_stack
+      @application_elastic_stack ||= @cluster&.application_elastic_stack
     end
 
     def chart_above_v3?

ee/app/workers/all_queues.yml

@@ -179,6 +179,14 @@
   :weight: 1
   :idempotent: 
   :tags: []
+- :name: cronjob:ingress_modsecurity_counter_metrics
+  :feature_category: :web_firewall
+  :has_external_dependencies: true
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: 
+  :tags: []
 - :name: cronjob:ldap_all_groups_sync
   :feature_category: :authentication_and_authorization
   :has_external_dependencies: true

ee/app/workers/ingress_modsecurity_counter_metrics_worker.rb

+# frozen_string_literal: true
+
+class IngressModsecurityCounterMetricsWorker # rubocop:disable Scalability/IdempotentWorker
+  include ApplicationWorker
+  include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
+  include ExclusiveLeaseGuard
+
+  feature_category :web_firewall
+
+  worker_has_external_dependencies!
+
+  LEASE_TIMEOUT = 1.hour
+
+  def perform
+    return unless Feature.enabled?(:usage_ingress_modsecurity_counter, default_enabled: true)
+
+    try_obtain_lease do
+      cluster_app_metrics = EE::Security::IngressModsecurityUsageService.new.execute
+
+      Gitlab::UsageDataCounters::IngressModsecurityCounter.add(
+        cluster_app_metrics[:statistics_unavailable],
+        cluster_app_metrics[:packets_processed],
+        cluster_app_metrics[:packets_anomalous]
+      )
+    end
+  end
+
+  private
+
+  def lease_timeout
+    LEASE_TIMEOUT
+  end
+
+  def lease_release?
+    false
+  end
+end

ee/changelogs/unreleased/32358-add-additional-stats-for-modsecurity-installations.yml

+---
+title: Add usage statistics for modsecurity total packets/anomalous packets
+merge_request: 28535
+author:
+type: added

ee/lib/ee/gitlab/usage_data.rb

@@ -34,7 +34,7 @@ module EE
 
         override :usage_data_counters
         def usage_data_counters
-          super + [::Gitlab::UsageDataCounters::LicensesList]
+          super + [::Gitlab::UsageDataCounters::LicensesList, ::Gitlab::UsageDataCounters::IngressModsecurityCounter]
         end
 
         override :uncached_data

ee/lib/gitlab/usage_data_counters/ingress_modsecurity_counter.rb

+# frozen_string_literal: true
+
+module Gitlab::UsageDataCounters
+  class IngressModsecurityCounter < BaseCounter
+    KNOWN_EVENTS = %w[statistics_unavailable packets_processed packets_anomalous].freeze
+    PREFIX = 'ingress_modsecurity'
+
+    class << self
+      def add(statistics_unavailable, packets_processed, packets_anomalous)
+        return unless Gitlab::CurrentSettings.usage_ping_enabled?
+
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.multi do
+            redis.set(redis_key(:statistics_unavailable), statistics_unavailable)
+            redis.incrby(redis_key(:packets_processed), packets_processed)
+            redis.incrby(redis_key(:packets_anomalous), packets_anomalous)
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/usage_data_counters/ingress_modsecurity_counter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::UsageDataCounters::IngressModsecurityCounter, :clean_gitlab_redis_shared_state do
+  describe '.add' do
+    it 'increases packets_processed and packets_anomalous counters and sets statistics_unavailable counter' do
+      described_class.add(3, 10_200, 2_500)
+      expect(described_class.totals).to eq(
+        ingress_modsecurity_packets_anomalous: 2_500,
+        ingress_modsecurity_packets_processed: 10_200,
+        ingress_modsecurity_statistics_unavailable: 3
+      )
+
+      described_class.add(2, 800, 500)
+      expect(described_class.totals).to eq(
+        ingress_modsecurity_packets_anomalous: 3_000,
+        ingress_modsecurity_packets_processed: 11_000,
+        ingress_modsecurity_statistics_unavailable: 2
+      )
+    end
+  end
+end

ee/spec/services/ee/security/ingress_modsecurity_usage_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::Security::IngressModsecurityUsageService do
+  describe '#execute' do
+    let(:environment) { create(:environment) }
+    let(:ingress_mode) { :modsecurity_blocking }
+    let(:deployments) { [] }
+    let!(:cluster) { create(:cluster, deployments: deployments) }
+    let!(:ingress) { create(:clusters_applications_ingress, ingress_mode, cluster: cluster) }
+    let!(:elastic_stack) { create(:clusters_applications_elastic_stack, :installed, cluster: cluster) }
+
+    subject { described_class.new.execute }
+
+    before do
+      allow_any_instance_of(::Security::WafAnomalySummaryService).to receive(:execute)
+    end
+
+    context 'when cluster is disabled' do
+      let(:cluster) { create(:cluster, :disabled, deployments: deployments) }
+
+      it 'gathers ingress data' do
+        expect(subject[:statistics_unavailable]).to eq(0)
+        expect(subject[:packets_processed]).to eq(0)
+        expect(subject[:packets_anomalous]).to eq(0)
+      end
+    end
+
+    context 'when environment is not available' do
+      let(:environment) { create(:environment, state: :stopped) }
+
+      it 'gathers ingress data' do
+        expect(subject[:statistics_unavailable]).to eq(0)
+        expect(subject[:packets_processed]).to eq(0)
+        expect(subject[:packets_anomalous]).to eq(0)
+      end
+    end
+
+    context 'when environment is available' do
+      context 'when deployment is unsuccessful' do
+        let(:deployments) { [deployment] }
+        let!(:deployment) { create(:deployment, :failed, environment: environment) }
+
+        it 'gathers ingress data' do
+          expect(subject[:statistics_unavailable]).to eq(0)
+          expect(subject[:packets_processed]).to eq(0)
+          expect(subject[:packets_anomalous]).to eq(0)
+        end
+      end
+
+      context 'when deployment is successful' do
+        let(:deployments) { [deployment] }
+        let!(:deployment) { create(:deployment, :success, environment: environment) }
+        let(:waf_anomaly_summary) { { total_traffic: 1000, total_anomalous_traffic: 200 } }
+
+        before do
+          allow_any_instance_of(::Security::WafAnomalySummaryService).to receive(:execute).and_return(waf_anomaly_summary)
+          allow(::Gitlab::ErrorTracking).to receive(:track_exception)
+        end
+
+        context 'when modsecurity statistics are available' do
+          it 'gathers ingress data' do
+            expect(subject[:statistics_unavailable]).to eq(0)
+            expect(subject[:packets_processed]).to eq(1000)
+            expect(subject[:packets_anomalous]).to eq(200)
+          end
+        end
+
+        context 'when modsecurity statistics are not available' do
+          let(:waf_anomaly_summary) { nil }
+
+          it 'gathers ingress data' do
+            expect(subject[:statistics_unavailable]).to eq(1)
+            expect(subject[:packets_processed]).to eq(0)
+            expect(subject[:packets_anomalous]).to eq(0)
+          end
+        end
+
+        context 'when modsecurity statistics process is raising exception' do
+          before do
+            allow_any_instance_of(::Security::WafAnomalySummaryService).to receive(:execute).and_raise(StandardError)
+          end
+
+          it 'gathers ingress data' do
+            expect(subject[:statistics_unavailable]).to eq(1)
+            expect(subject[:packets_processed]).to eq(0)
+            expect(subject[:packets_anomalous]).to eq(0)
+          end
+
+          it 'tracks exception' do
+            expect(::Gitlab::ErrorTracking).to receive(:track_exception).with(StandardError, environment_id: environment.id, cluster_id: cluster.id)
+            subject
+          end
+        end
+
+        context 'with multiple environments' do
+          let!(:environment_2) { create(:environment) }
+          let!(:cluster_2) { create(:cluster, deployments: [deployment_2]) }
+          let!(:deployment_2) { create(:deployment, :success, environment: environment_2) }
+
+          let!(:ingress_2) { create(:clusters_applications_ingress, ingress_mode, cluster: cluster_2) }
+          let!(:elastic_stack_2) { create(:clusters_applications_elastic_stack, :installed, cluster: cluster_2) }
+
+          it 'gathers ingress data from multiple environments' do
+            expect(subject[:statistics_unavailable]).to eq(0)
+            expect(subject[:packets_processed]).to eq(2000)
+            expect(subject[:packets_anomalous]).to eq(400)
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/services/security/waf_anomaly_summary_service_spec.rb

@@ -149,6 +149,14 @@ RSpec.describe Security::WafAnomalySummaryService do
           expect(results.fetch(:total_traffic)).to eq 0
           expect(results.fetch(:anomalous_traffic)).to eq 0.0
         end
+
+        context 'when totals_only is set to true' do
+          it 'returns totals only', :aggregate_failures do
+            results = subject.execute(totals_only: true)
+
+            expect(results).to eq(total_traffic: 0.0, total_anomalous_traffic: 0.0)
+          end
+        end
       end
 
       context 'no violations' do
@@ -163,6 +171,14 @@ RSpec.describe Security::WafAnomalySummaryService do
           expect(results.fetch(:total_traffic)).to eq 3
           expect(results.fetch(:anomalous_traffic)).to eq 0.0
         end
+
+        context 'when totals_only is set to true' do
+          it 'returns totals only', :aggregate_failures do
+            results = subject.execute(totals_only: true)
+
+            expect(results).to eq(total_traffic: 3.0, total_anomalous_traffic: 0.0)
+          end
+        end
       end
 
       context 'with violations' do
@@ -177,6 +193,14 @@ RSpec.describe Security::WafAnomalySummaryService do
           expect(results.fetch(:total_traffic)).to eq 3
           expect(results.fetch(:anomalous_traffic)).to eq 0.33
         end
+
+        context 'when totals_only is set to true' do
+          it 'returns totals only', :aggregate_failures do
+            results = subject.execute(totals_only: true)
+
+            expect(results).to eq(total_traffic: 3.0, total_anomalous_traffic: 1.0)
+          end
+        end
       end
 
       context 'with legacy es6 cluster' do

ee/spec/workers/ingress_modsecurity_counter_metrics_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe IngressModsecurityCounterMetricsWorker, :clean_gitlab_redis_shared_state do
+  include ExclusiveLeaseHelpers
+
+  subject(:worker) { described_class.new }
+
+  let(:ingress_usage_service) { instance_double('EE::Security::IngressModsecurityUsageService', execute: usage_statistics) }
+  let(:usage_statistics) do
+    {
+      statistics_unavailable: 2,
+      packets_processed: 10_200,
+      packets_anomalous: 2_500
+    }
+  end
+
+  before do
+    allow(EE::Security::IngressModsecurityUsageService).to receive(:new) { ingress_usage_service }
+  end
+
+  describe '#perform' do
+    context 'when feature flag is disabled' do
+      before do
+        stub_feature_flags(usage_ingress_modsecurity_counter: false)
+      end
+
+      it 'does not update the usae counter' do
+        worker.perform
+        expect(Gitlab::UsageDataCounters::IngressModsecurityCounter.totals).to eq(
+          ingress_modsecurity_packets_anomalous: 0,
+          ingress_modsecurity_packets_processed: 0,
+          ingress_modsecurity_statistics_unavailable: 0
+        )
+      end
+    end
+
+    context 'with exclusive lease' do
+      let(:lease_key) { "#{described_class.name.underscore}" }
+
+      before do
+        stub_exclusive_lease_taken(lease_key)
+      end
+
+      it 'does not allow to add counters concurrently' do
+        expect(Gitlab::UsageDataCounters::IngressModsecurityCounter).not_to receive(:add)
+
+        worker.perform
+      end
+    end
+
+    it 'updates usage counter' do
+      worker.perform
+      expect(Gitlab::UsageDataCounters::IngressModsecurityCounter.totals).to eq(
+        ingress_modsecurity_packets_anomalous: 2_500,
+        ingress_modsecurity_packets_processed: 10_200,
+        ingress_modsecurity_statistics_unavailable: 2
+      )
+    end
+  end
+end

spec/models/clusters/applications/elastic_stack_spec.rb

@@ -175,6 +175,7 @@ describe Clusters::Applications::ElasticStack do
         expect(faraday_connection.headers["Authorization"]).to eq(kube_client.headers[:Authorization])
         expect(faraday_connection.ssl.cert_store).to be_instance_of(OpenSSL::X509::Store)
         expect(faraday_connection.ssl.verify).to eq(1)
+        expect(faraday_connection.options.timeout).to be_nil
       end
 
       context 'when cluster is not reachable' do
@@ -186,6 +187,15 @@ describe Clusters::Applications::ElasticStack do
           expect(subject.elasticsearch_client).to be_nil
         end
       end
+
+      context 'when timeout is provided' do
+        it 'sets timeout in elasticsearch_client' do
+          client = subject.elasticsearch_client(timeout: 123)
+          faraday_connection = client.transport.connections.first.connection
+
+          expect(faraday_connection.options.timeout).to eq(123)
+        end
+      end
     end
   end
 end

spec/models/clusters/cluster_spec.rb

@@ -28,6 +28,8 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
   it { is_expected.to have_one(:cluster_project) }
   it { is_expected.to have_many(:deployment_clusters) }
   it { is_expected.to have_many(:metrics_dashboard_annotations) }
+  it { is_expected.to have_many(:successful_deployments) }
+  it { is_expected.to have_many(:environments).through(:deployments) }
 
   it { is_expected.to delegate_method(:status).to(:provider) }
   it { is_expected.to delegate_method(:status_reason).to(:provider) }
@@ -190,6 +192,73 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '.with_enabled_modsecurity' do
+    subject { described_class.with_enabled_modsecurity }
+
+    let_it_be(:cluster) { create(:cluster) }
+
+    context 'cluster has ingress application with enabled modsecurity' do
+      let!(:application) { create(:clusters_applications_ingress, :installed, :modsecurity_logging, cluster: cluster) }
+
+      it { is_expected.to include(cluster) }
+    end
+
+    context 'cluster has ingress application with disabled modsecurity' do
+      let!(:application) { create(:clusters_applications_ingress, :installed, :modsecurity_disabled, cluster: cluster) }
+
+      it { is_expected.not_to include(cluster) }
+    end
+
+    context 'cluster does not have ingress application' do
+      it { is_expected.not_to include(cluster) }
+    end
+  end
+
+  describe '.with_available_elasticstack' do
+    subject { described_class.with_available_elasticstack }
+
+    let_it_be(:cluster) { create(:cluster) }
+
+    context 'cluster has ElasticStack application' do
+      let!(:application) { create(:clusters_applications_elastic_stack, :installed, cluster: cluster) }
+
+      it { is_expected.to include(cluster) }
+    end
+
+    context 'cluster does not have ElasticStack application' do
+      it { is_expected.not_to include(cluster) }
+    end
+  end
+
+  describe '.distinct_with_deployed_environments' do
+    subject { described_class.distinct_with_deployed_environments }
+
+    let_it_be(:cluster) { create(:cluster) }
+
+    context 'cluster has multiple successful deployment with environment' do
+      let!(:environment) { create(:environment) }
+      let!(:deployment) { create(:deployment, :success, cluster: cluster, environment: environment) }
+      let!(:deployment_2) { create(:deployment, :success, cluster: cluster, environment: environment) }
+
+      it { is_expected.to include(cluster) }
+
+      it 'lists only distinct environments' do
+        expect(subject.first.environments.count).to eq(1)
+      end
+    end
+
+    context 'cluster has only failed deployment with environment' do
+      let!(:environment) { create(:environment) }
+      let!(:deployment) { create(:deployment, :failed, cluster: cluster, environment: environment) }
+
+      it { is_expected.not_to include(cluster) }
+    end
+
+    context 'cluster does not have any deployment' do
+      it { is_expected.not_to include(cluster) }
+    end
+  end
+
   describe '.with_project_alert_service_data' do
     subject { described_class.with_project_alert_service_data(project_id) }