From 9dcf0aa4506c5f2dacc3769c45a1d52b29b92920 Mon Sep 17 00:00:00 2001 From: Zamir Martins Filho Date: Tue, 24 Sep 2024 20:34:58 +0200 Subject: [PATCH 1/5] Add cyclonedx data as security findings --- .../vue_shared/security_reports/constants.js | 1 + app/finders/security/security_jobs_finder.rb | 2 +- config/bounded_contexts.yml | 7 +- doc/api/graphql/reference/index.md | 18 +++- .../vulnerability_location.fragment.graphql | 3 + ...line_security_report_summary.query.graphql | 4 + .../security_dashboard/store/constants.js | 1 + .../vue_shared/security_reports/constants.js | 3 + .../vulnerability_location/cyclonedx_type.rb | 18 ++++ .../types/vulnerability_location_type.rb | 5 +- .../concerns/ee/enums/ci/job_artifact.rb | 2 +- .../models/concerns/ee/enums/vulnerability.rb | 1 + ee/app/models/ee/ci/job_artifact.rb | 23 ++++- ee/app/models/ee/ci/pipeline.rb | 3 +- .../models/gitlab_subscriptions/features.rb | 1 + ee/app/models/security/scan.rb | 3 +- .../sbom/create_vulnerabilities_service.rb | 8 +- .../services/security/store_scans_service.rb | 9 +- .../build_finding_map_service.rb | 11 +-- .../advisory_scanner.rb | 8 +- .../security_report_builder.rb | 97 ++++++++++++------- locale/gitlab.pot | 3 + 22 files changed, 171 insertions(+), 60 deletions(-) create mode 100644 ee/app/graphql/types/vulnerability_location/cyclonedx_type.rb diff --git a/app/assets/javascripts/vue_shared/security_reports/constants.js b/app/assets/javascripts/vue_shared/security_reports/constants.js index 2db98346526f0b..d55b0917f69417 100644 --- a/app/assets/javascripts/vue_shared/security_reports/constants.js +++ b/app/assets/javascripts/vue_shared/security_reports/constants.js @@ -29,6 +29,7 @@ export const REPORT_TYPE_CLUSTER_IMAGE_SCANNING = 'cluster_image_scanning'; export const REPORT_TYPE_COVERAGE_FUZZING = 'coverage_fuzzing'; export const REPORT_TYPE_CORPUS_MANAGEMENT = 'corpus_management'; export const REPORT_TYPE_API_FUZZING = 'api_fuzzing'; +export const REPORT_TYPE_CYCLONEDX = 'cyclonedx'; /** * SecurityReportTypeEnum values for use with GraphQL. diff --git a/app/finders/security/security_jobs_finder.rb b/app/finders/security/security_jobs_finder.rb index 8209ab9e0b655a..c1057f7d3534f6 100644 --- a/app/finders/security/security_jobs_finder.rb +++ b/app/finders/security/security_jobs_finder.rb @@ -13,7 +13,7 @@ module Security class SecurityJobsFinder < JobsFinder def self.allowed_job_types - [:sast, :sast_advanced, :sast_iac, :breach_and_attack_simulation, :dast, :dependency_scanning, :container_scanning, :secret_detection, :coverage_fuzzing, :api_fuzzing, :cluster_image_scanning] + [:sast, :sast_advanced, :sast_iac, :breach_and_attack_simulation, :dast, :dependency_scanning, :container_scanning, :secret_detection, :coverage_fuzzing, :api_fuzzing, :cluster_image_scanning, :cyclonedx] end end end diff --git a/config/bounded_contexts.yml b/config/bounded_contexts.yml index a47afeb213083b..c464c09394ad4d 100644 --- a/config/bounded_contexts.yml +++ b/config/bounded_contexts.yml @@ -30,7 +30,7 @@ domains: - duo_chat - ai_abstraction_layer - code_suggestions # Also in CodeSuggestions - - cloud_connector # Also in CloudConnector + - cloud_connector # Also in CloudConnector - duo_workflow Analytics: @@ -340,6 +340,11 @@ domains: feature_categories: - vulnerability_management + VulnerabilityLocation: + description: Vulnerability location based on the various report types. + feature_categories: + - vulnerability_management + WebHooks: description: Outbound integration of GitLab with external tools and workflows feature_categories: diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index b646b5b826f64f..116f696af24faa 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -33553,6 +33553,7 @@ Represents summary of a security report. | `containerScanning` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `container_scanning` scan. | | `containerScanningForRegistry` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `container_scanning_for_registry` scan. | | `coverageFuzzing` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `coverage_fuzzing` scan. | +| `cyclonedx` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `cyclonedx` scan. | | `dast` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `dast` scan. | | `dependencyScanning` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `dependency_scanning` scan. | | `generic` | [`SecurityReportSummarySection`](#securityreportsummarysection) | Aggregated counts for the `generic` scan. | @@ -35248,7 +35249,7 @@ Represents a vulnerability. | `presentOnDefaultBranch` | [`Boolean!`](#boolean) | Indicates whether the vulnerability is present on the default branch or not. | | `primaryIdentifier` | [`VulnerabilityIdentifier`](#vulnerabilityidentifier) | Primary identifier of the vulnerability. | | `project` | [`Project`](#project) | Project on which the vulnerability was found. | -| `reportType` | [`VulnerabilityReportType`](#vulnerabilityreporttype) | Type of the security report that found the vulnerability (SAST, DEPENDENCY_SCANNING, CONTAINER_SCANNING, DAST, SECRET_DETECTION, COVERAGE_FUZZING, API_FUZZING, CLUSTER_IMAGE_SCANNING, CONTAINER_SCANNING_FOR_REGISTRY, GENERIC). `Scan Type` in the UI. | +| `reportType` | [`VulnerabilityReportType`](#vulnerabilityreporttype) | Type of the security report that found the vulnerability (SAST, DEPENDENCY_SCANNING, CONTAINER_SCANNING, DAST, SECRET_DETECTION, COVERAGE_FUZZING, API_FUZZING, CLUSTER_IMAGE_SCANNING, CONTAINER_SCANNING_FOR_REGISTRY, CYCLONEDX, GENERIC). `Scan Type` in the UI. | | `resolvedAt` | [`Time`](#time) | Timestamp of when the vulnerability state was changed to resolved. | | `resolvedBy` | [`UserCore`](#usercore) | User that resolved the vulnerability. | | `resolvedOnDefaultBranch` | [`Boolean!`](#boolean) | Indicates whether the vulnerability is fixed on the default branch or not. | @@ -35681,6 +35682,17 @@ Represents the location of a vulnerability found by a Coverage Fuzzing scan. | `vulnerableClass` | [`String`](#string) | Class containing the vulnerability. | | `vulnerableMethod` | [`String`](#string) | Method containing the vulnerability. | +### `VulnerabilityLocationCyclonedx` + +Represents the location of a vulnerability found by a CVS scan. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `dependency` | [`VulnerableDependency`](#vulnerabledependency) | Dependency containing the vulnerability. | +| `file` | [`String`](#string) | Path to the vulnerable file. | + ### `VulnerabilityLocationDast` Represents the location of a vulnerability found by a DAST scan. @@ -39347,6 +39359,7 @@ Type of search. | `CLUSTER_IMAGE_SCANNING` | CLUSTER IMAGE SCANNING scan report. | | `CONTAINER_SCANNING` | CONTAINER SCANNING scan report. | | `COVERAGE_FUZZING` | COVERAGE FUZZING scan report. | +| `CYCLONEDX` | CYCLONEDX scan report. | | `DAST` | DAST scan report. | | `DEPENDENCY_SCANNING` | DEPENDENCY SCANNING scan report. | | `SAST` | SAST scan report. | @@ -39365,6 +39378,7 @@ The type of the security scanner. | `CLUSTER_IMAGE_SCANNING` | Cluster Image Scanning scanner. | | `CONTAINER_SCANNING` | Container Scanning scanner. | | `COVERAGE_FUZZING` | Coverage Fuzzing scanner. | +| `CYCLONEDX` | Cyclonedx scanner. | | `DAST` | DAST scanner. | | `DEPENDENCY_SCANNING` | Dependency Scanning scanner. | | `SAST` | SAST scanner. | @@ -39966,6 +39980,7 @@ The type of the security scan that found the vulnerability. | `CONTAINER_SCANNING` | Container Scanning report. | | `CONTAINER_SCANNING_FOR_REGISTRY` | Container Scanning For Registry report. | | `COVERAGE_FUZZING` | Coverage Fuzzing report. | +| `CYCLONEDX` | Cyclonedx report. | | `DAST` | DAST report. | | `DEPENDENCY_SCANNING` | Dependency Scanning report. | | `GENERIC` | Generic report. | @@ -41374,6 +41389,7 @@ One of: - [`VulnerabilityLocationClusterImageScanning`](#vulnerabilitylocationclusterimagescanning) - [`VulnerabilityLocationContainerScanning`](#vulnerabilitylocationcontainerscanning) - [`VulnerabilityLocationCoverageFuzzing`](#vulnerabilitylocationcoveragefuzzing) +- [`VulnerabilityLocationCyclonedx`](#vulnerabilitylocationcyclonedx) - [`VulnerabilityLocationDast`](#vulnerabilitylocationdast) - [`VulnerabilityLocationDependencyScanning`](#vulnerabilitylocationdependencyscanning) - [`VulnerabilityLocationGeneric`](#vulnerabilitylocationgeneric) diff --git a/ee/app/assets/javascripts/security_dashboard/graphql/fragments/vulnerability_location.fragment.graphql b/ee/app/assets/javascripts/security_dashboard/graphql/fragments/vulnerability_location.fragment.graphql index f44cf834e5aae5..5dc96479cf4938 100644 --- a/ee/app/assets/javascripts/security_dashboard/graphql/fragments/vulnerability_location.fragment.graphql +++ b/ee/app/assets/javascripts/security_dashboard/graphql/fragments/vulnerability_location.fragment.graphql @@ -30,4 +30,7 @@ fragment VulnerabilityLocation on VulnerabilityLocation { ... on VulnerabilityLocationDast { path } + ... on VulnerabilityLocationCyclonedx { + file + } } diff --git a/ee/app/assets/javascripts/security_dashboard/graphql/queries/pipeline_security_report_summary.query.graphql b/ee/app/assets/javascripts/security_dashboard/graphql/queries/pipeline_security_report_summary.query.graphql index 98104be01b0007..021de942ed979b 100644 --- a/ee/app/assets/javascripts/security_dashboard/graphql/queries/pipeline_security_report_summary.query.graphql +++ b/ee/app/assets/javascripts/security_dashboard/graphql/queries/pipeline_security_report_summary.query.graphql @@ -55,6 +55,10 @@ query pipelineSecuritySummary( vulnerabilitiesCount ...SecurityReportSummaryScans } + cyclonedx { + vulnerabilitiesCount + ...SecurityReportSummaryScans + } } } } diff --git a/ee/app/assets/javascripts/security_dashboard/store/constants.js b/ee/app/assets/javascripts/security_dashboard/store/constants.js index 11331af2e106bd..0c39488b111691 100644 --- a/ee/app/assets/javascripts/security_dashboard/store/constants.js +++ b/ee/app/assets/javascripts/security_dashboard/store/constants.js @@ -30,6 +30,7 @@ export const REPORT_TYPES_DEFAULT = { dependency_scanning: s__('ciReport|Dependency Scanning'), sast: s__('ciReport|SAST'), secret_detection: s__('ciReport|Secret Detection'), + cyclonedx: s__('ciReport|CVS'), }; export const REPORT_TYPES_DEFAULT_KEYS = Object.keys(REPORT_TYPES_DEFAULT); diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/constants.js b/ee/app/assets/javascripts/vue_shared/security_reports/constants.js index af2f1422bab8d1..d43cc06898cb72 100644 --- a/ee/app/assets/javascripts/vue_shared/security_reports/constants.js +++ b/ee/app/assets/javascripts/vue_shared/security_reports/constants.js @@ -10,6 +10,7 @@ import { REPORT_TYPE_DEPENDENCY_SCANNING, REPORT_TYPE_CONTAINER_SCANNING, REPORT_TYPE_CLUSTER_IMAGE_SCANNING, + REPORT_TYPE_CYCLONEDX, } from '~/vue_shared/security_reports/constants'; export * from '~/vue_shared/security_reports/constants'; @@ -27,6 +28,7 @@ export const SECURITY_REPORT_TYPE_ENUM_DAST = 'DAST'; export const SECURITY_REPORT_TYPE_ENUM_DEPENDENCY_SCANNING = 'DEPENDENCY_SCANNING'; export const SECURITY_REPORT_TYPE_ENUM_CONTAINER_SCANNING = 'CONTAINER_SCANNING'; export const SECURITY_REPORT_TYPE_ENUM_CLUSTER_IMAGE_SCANNING = 'CLUSTER_IMAGE_SCANNING'; +export const SECURITY_REPORT_TYPE_ENUM_CYCLONEDX = 'CYCLONEDX'; /* Override CE Definitions */ @@ -43,6 +45,7 @@ export const reportTypeToSecurityReportTypeEnum = { [REPORT_TYPE_DEPENDENCY_SCANNING]: SECURITY_REPORT_TYPE_ENUM_DEPENDENCY_SCANNING, [REPORT_TYPE_CONTAINER_SCANNING]: SECURITY_REPORT_TYPE_ENUM_CONTAINER_SCANNING, [REPORT_TYPE_CLUSTER_IMAGE_SCANNING]: SECURITY_REPORT_TYPE_ENUM_CLUSTER_IMAGE_SCANNING, + [REPORT_TYPE_CYCLONEDX]: SECURITY_REPORT_TYPE_ENUM_CYCLONEDX, }; /** diff --git a/ee/app/graphql/types/vulnerability_location/cyclonedx_type.rb b/ee/app/graphql/types/vulnerability_location/cyclonedx_type.rb new file mode 100644 index 00000000000000..00bcf5ba79d4a6 --- /dev/null +++ b/ee/app/graphql/types/vulnerability_location/cyclonedx_type.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Types + module VulnerabilityLocation + # rubocop: disable Graphql/AuthorizeTypes -- Vulnerability locations have their authorization enforced by VulnerabilityType + class CyclonedxType < BaseObject + graphql_name 'VulnerabilityLocationCyclonedx' + description 'Represents the location of a vulnerability found by a CVS scan' + + field :dependency, ::Types::VulnerableDependencyType, + null: true, description: 'Dependency containing the vulnerability.' + + field :file, GraphQL::Types::String, + null: true, description: 'Path to the vulnerable file.' + end + # rubocop: enable Graphql/AuthorizeTypes + end +end diff --git a/ee/app/graphql/types/vulnerability_location_type.rb b/ee/app/graphql/types/vulnerability_location_type.rb index 742042aea2fc3e..0c4f60cd63b99c 100644 --- a/ee/app/graphql/types/vulnerability_location_type.rb +++ b/ee/app/graphql/types/vulnerability_location_type.rb @@ -14,7 +14,8 @@ class VulnerabilityLocationType < BaseUnion VulnerabilityLocation::SastType, VulnerabilityLocation::SecretDetectionType, VulnerabilityLocation::CoverageFuzzingType, - VulnerabilityLocation::GenericType + VulnerabilityLocation::GenericType, + VulnerabilityLocation::CyclonedxType def self.resolve_type(object, context) case object[:report_type] @@ -34,6 +35,8 @@ def self.resolve_type(object, context) VulnerabilityLocation::CoverageFuzzingType when 'generic' VulnerabilityLocation::GenericType + when 'cyclonedx' + VulnerabilityLocation::CyclonedxType else raise UnexpectedReportType, "Report type must be one of #{::Enums::Vulnerability.report_types.keys}" end diff --git a/ee/app/models/concerns/ee/enums/ci/job_artifact.rb b/ee/app/models/concerns/ee/enums/ci/job_artifact.rb index 1039173d85d761..5cf93cf9066ebb 100644 --- a/ee/app/models/concerns/ee/enums/ci/job_artifact.rb +++ b/ee/app/models/concerns/ee/enums/ci/job_artifact.rb @@ -7,7 +7,7 @@ module JobArtifact extend ActiveSupport::Concern SECURITY_REPORT_FILE_TYPES = %w[sast secret_detection dependency_scanning container_scanning - cluster_image_scanning dast coverage_fuzzing api_fuzzing].freeze + cluster_image_scanning dast coverage_fuzzing api_fuzzing cyclonedx].freeze EE_REPORT_FILE_TYPES = { license_scanning: %w[license_scanning].freeze, diff --git a/ee/app/models/concerns/ee/enums/vulnerability.rb b/ee/app/models/concerns/ee/enums/vulnerability.rb index 8fbbe1fc88829e..a737d73810e03f 100644 --- a/ee/app/models/concerns/ee/enums/vulnerability.rb +++ b/ee/app/models/concerns/ee/enums/vulnerability.rb @@ -13,6 +13,7 @@ module Vulnerability api_fuzzing: 6, cluster_image_scanning: 7, container_scanning_for_registry: 8, + cyclonedx: 9, generic: 99 }.freeze diff --git a/ee/app/models/ee/ci/job_artifact.rb b/ee/app/models/ee/ci/job_artifact.rb index 23278f073b4ccb..5a11ab10504a9f 100644 --- a/ee/app/models/ee/ci/job_artifact.rb +++ b/ee/app/models/ee/ci/job_artifact.rb @@ -27,10 +27,12 @@ module Ci::JobArtifact EE_REPORT_FILE_TYPES = EE::Enums::Ci::JobArtifact.ee_report_file_types - scope :security_reports, ->(file_types: EE::Enums::Ci::JobArtifact.security_report_file_types) do + scope :security_reports, ->(file_types: EE::Enums::Ci::JobArtifact.security_report_file_types, including_cyclonedx: false) do requested_file_types = *file_types + allowed_file_types = requested_file_types & EE::Enums::Ci::JobArtifact.security_report_file_types + allowed_file_types |= ['cyclonedx'] if including_cyclonedx - with_file_types(requested_file_types & EE::Enums::Ci::JobArtifact.security_report_file_types) + with_file_types(allowed_file_types) end scope :with_files_stored_locally, -> { where(file_store: ::ObjectStorage::Store::LOCAL) } @@ -132,9 +134,20 @@ def security_report(validate: false) signatures_enabled = project.licensed_feature_available?(:vulnerability_finding_signatures) - report = ::Gitlab::Ci::Reports::Security::Report.new(file_type, job.pipeline, nil).tap do |report| - each_blob do |blob| - ::Gitlab::Ci::Parsers.fabricate!(file_type, blob, report, signatures_enabled: signatures_enabled, validate: validate).parse! + begin + if file_type == 'cyclonedx' + sbom_report = ::Gitlab::Ci::Reports::Sbom::Report.new.tap do |report| + each_blob do |blob| + ::Gitlab::Ci::Parsers.fabricate!(file_type, project: project).parse!(blob, report) + end + end + report = ::Gitlab::VulnerabilityScanning::SecurityReportBuilder.new(sbom_report: sbom_report, project: project, pipeline: job.pipeline).execute + else + report = ::Gitlab::Ci::Reports::Security::Report.new(file_type, job.pipeline, nil).tap do |report| + each_blob do |blob| + ::Gitlab::Ci::Parsers.fabricate!(file_type, blob, report, signatures_enabled: signatures_enabled, validate: validate).parse! + end + end end rescue StandardError report.add_error('ParsingError') diff --git a/ee/app/models/ee/ci/pipeline.rb b/ee/app/models/ee/ci/pipeline.rb index fdade0787214ba..d993c4236a7a56 100644 --- a/ee/app/models/ee/ci/pipeline.rb +++ b/ee/app/models/ee/ci/pipeline.rb @@ -51,7 +51,8 @@ module Pipeline requirements: %i[requirements], requirements_v2: %i[requirements], coverage_fuzzing: %i[coverage_fuzzing], - api_fuzzing: %i[api_fuzzing] + api_fuzzing: %i[api_fuzzing], + cyclonedx: %i[cyclonedx] }.freeze state_machine :status do diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index 7f7b802ccd4843..08f9f94e167b01 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -198,6 +198,7 @@ class Features container_scanning credentials_inventory custom_roles + cyclonedx dast dependency_scanning dora4_analytics diff --git a/ee/app/models/security/scan.rb b/ee/app/models/security/scan.rb index 351a2723058239..f3ea9d29be2774 100644 --- a/ee/app/models/security/scan.rb +++ b/ee/app/models/security/scan.rb @@ -26,7 +26,8 @@ class Scan < ::Gitlab::Database::SecApplicationRecord secret_detection: 5, coverage_fuzzing: 6, api_fuzzing: 7, - cluster_image_scanning: 8 + cluster_image_scanning: 8, + cyclonedx: 9 } declarative_enum Security::ScanStatusEnum diff --git a/ee/app/services/sbom/create_vulnerabilities_service.rb b/ee/app/services/sbom/create_vulnerabilities_service.rb index d60dda8a222d85..7d3d734bfef632 100644 --- a/ee/app/services/sbom/create_vulnerabilities_service.rb +++ b/ee/app/services/sbom/create_vulnerabilities_service.rb @@ -51,7 +51,8 @@ def execute source: sbom_report.source, pipeline: pipeline, project: project, - purl_type: affected_occurrence.purl.type) + purl_type: affected_occurrence.purl.type, + scanner: scanner) end create_vulnerabilities(finding_maps) @@ -119,5 +120,10 @@ def project pipeline.project end strong_memoize_attr :project + + def scanner + ::Gitlab::VulnerabilityScanning::SecurityScanner.fabricate + end + strong_memoize_attr :scanner end end diff --git a/ee/app/services/security/store_scans_service.rb b/ee/app/services/security/store_scans_service.rb index ab681ce299953a..3129ae129550b2 100644 --- a/ee/app/services/security/store_scans_service.rb +++ b/ee/app/services/security/store_scans_service.rb @@ -16,7 +16,9 @@ def execute # StoreGroupedScansService returns true only when it creates a `security_scans` record. # To avoid resource wastage we are skipping the reports ingestion when there are no new scans, but # we sync the rules as it might cause inconsistent state if we skip. - results = grouped_report_artifacts.map { |artifacts| StoreGroupedScansService.execute(artifacts) } + results = grouped_report_artifacts.map do |artifacts| + StoreGroupedScansService.execute(artifacts) + end sync_findings_to_approval_rules unless pipeline.default_branch? return unless results.any?(true) @@ -37,14 +39,15 @@ def already_purged? def grouped_report_artifacts pipeline.job_artifacts - .security_reports + .security_reports(including_cyclonedx: true) .group_by(&:file_type) .select { |file_type, _| parse_report_file?(file_type) } .values end def parse_report_file?(file_type) - project.feature_available?(Ci::Build::LICENSED_PARSER_FEATURES.fetch(file_type)) + feature = file_type == 'cyclonedx' ? :cyclonedx : Ci::Build::LICENSED_PARSER_FEATURES.fetch(file_type) + project.feature_available?(feature) end def schedule_store_reports_worker diff --git a/ee/app/services/security/vulnerability_scanning/build_finding_map_service.rb b/ee/app/services/security/vulnerability_scanning/build_finding_map_service.rb index 2d58ff16486d63..994f4b5e20361b 100644 --- a/ee/app/services/security/vulnerability_scanning/build_finding_map_service.rb +++ b/ee/app/services/security/vulnerability_scanning/build_finding_map_service.rb @@ -9,13 +9,14 @@ def self.execute(...) new(...).execute end - def initialize(advisory:, affected_component:, source:, pipeline:, project:, purl_type:) + def initialize(advisory:, affected_component:, source:, pipeline:, project:, purl_type:, scanner:) @advisory = advisory @affected_component = affected_component @source = source @pipeline = pipeline @project = project @purl_type = purl_type + @scanner = scanner end def execute @@ -29,18 +30,14 @@ def execute private - attr_reader :advisory, :affected_component, :source, :pipeline, :project, :purl_type - - def report_scanner - ::Gitlab::VulnerabilityScanning::SecurityScanner.fabricate - end + attr_reader :advisory, :affected_component, :source, :pipeline, :project, :purl_type, :scanner def finding builder = ::Gitlab::VulnerabilityScanning::FindingBuilder.for_purl_type!(purl_type) builder.new( project: project, pipeline: pipeline, - scanner: report_scanner, + scanner: scanner, sbom_source: source, advisory: advisory, affected_component: affected_component diff --git a/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb b/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb index 51c3a34d436ad9..a2a8ddd457a1fa 100644 --- a/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb +++ b/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb @@ -94,7 +94,8 @@ def bulk_vulnerability_ingestion(affected_package, advisory_data_object, occurre source: affected_component.source, pipeline: affected_component.pipeline, project: affected_component.project, - purl_type: affected_component.purl_type) + purl_type: affected_component.purl_type, + scanner: scanner) end.compact return if finding_maps.empty? @@ -135,6 +136,11 @@ def possibly_affected_projects_count def known_affected_projects_count @known_affected_projects.keys.size end + + def scanner + ::Gitlab::VulnerabilityScanning::SecurityScanner.fabricate + end + strong_memoize_attr :scanner end end end diff --git a/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb b/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb index 26c1811bdcdace..aec5f3e20fe402 100644 --- a/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb +++ b/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb @@ -4,57 +4,82 @@ module Gitlab module VulnerabilityScanning class SecurityReportBuilder include Gitlab::Utils::StrongMemoize + include Gitlab::VulnerabilityScanning::AdvisoryUtils - # We don't have a schema version set because there is no JSON to validate. - SECURITY_REPORT_VERSION = "0.0.0" - - attr_reader :report - - def initialize(report_type:, project:, pipeline:, sbom:) - @report_type = report_type + def initialize(sbom_report:, project:, pipeline:) + @sbom_report = sbom_report @project = project @pipeline = pipeline - @sbom = sbom - @report = ::Gitlab::Ci::Reports::Security::Report.new(report_type, pipeline, Time.zone.now) - report.version = SECURITY_REPORT_VERSION - report.add_scanner(scanner) end - # Add advisories affecting a component to the security report. - # - # @param advisories [Array<[Gitlab::VulnerabilityScanning::Advisory, - # Gitlab::VulnerabilityScanning::AffectedComponent]>] - def add_affections(affections) - affections.each do |advisory, affected_component| - add_affection(advisory, affected_component) + def execute + return unless sbom_report.valid? + return unless sbom_report.source.present? + + return if sbom_report.source.source_type != :dependency_scanning && Feature.disabled?( + :cvs_for_container_scanning, project) + + report = ::Gitlab::Ci::Reports::Security::Report.new('cyclonedx', pipeline, Time.zone.now) + report.add_scanner(scanner) + + sbom_report.components.each_slice(::Security::IngestionConstants::COMPONENTS_BATCH_SIZE) do |occurrence_batch| + affected_packages(occurrence_batch).each_batch do |affected_package_batch| + affected_package_batch.filter_map do |affected_package| + # We need to match every affected package to one occurrence + affected_occurrence = occurrence_batch.find do |occurrence| + next unless affected_package.package_name == occurrence.name + + affected_occurrence?(occurrence, sbom_report.source, affected_package) + end + + next unless affected_occurrence.present? + + advisory_data_object = Gitlab::VulnerabilityScanning::Advisory.from_affected_package( + affected_package: affected_package, advisory: affected_package.advisory) + + finding = ::Security::VulnerabilityScanning::BuildFindingMapService.execute( + advisory: advisory_data_object, + affected_component: affected_occurrence, + source: sbom_report.source, + pipeline: pipeline, + project: project, + purl_type: affected_occurrence.purl.type, + scanner: scanner) + + report.add_finding(finding.report_finding) + end + end end + + report end private - attr_reader :report_type, :project, :pipeline, :sbom + attr_reader :sbom_report, :project, :pipeline - def scanner - VulnerabilityScanning::SecurityScanner.fabricate + def affected_occurrence?(occurrence, source, affected_package) + advisory = affected_package.advisory + occurrence_is_affected?( + xid: advisory.advisory_xid, + purl_type: affected_package.purl_type, + range: affected_package.affected_range, + version: occurrence.version, + distro: affected_package.distro_version, + source: source, + project_id: pipeline.project_id, + source_xid: advisory.source_xid + ) end - strong_memoize_attr :scanner - - def add_affection(advisory, affected_component) - builder_class = Gitlab::VulnerabilityScanning::FindingBuilder.for_report_type(report_type) - return unless builder_class + def affected_packages(occurrence_batch) + ::PackageMetadata::AffectedPackage.for_occurrences(occurrence_batch).with_advisory + end - begin - builder = builder_class.new(project: project, pipeline: pipeline, sbom_source: sbom.source, scanner: scanner, - advisory: advisory, affected_component: affected_component) - finding = builder.finding - report.add_finding(finding) - finding.identifiers.each { |ident| report.add_identifier(ident) } - rescue DependencyScanning::FindingBuilder::MissingPropertiesError, - ContainerScanning::FindingBuilder::MissingPropertiesError => error - report.add_error('MissingPropertiesError', error.message) - end + def scanner + VulnerabilityScanning::SecurityScanner.fabricate end + strong_memoize_attr :scanner end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2c30242e4f166b..6e8f57160d789a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -64520,6 +64520,9 @@ msgid_plural "ciReport|Browser performance test metrics: %{strong_start}%{change msgstr[0] "" msgstr[1] "" +msgid "ciReport|CVS" +msgstr "" + msgid "ciReport|Checks" msgstr "" -- GitLab From 7fa0544b5fa9a8b1864249a6950c19b8778ae7e6 Mon Sep 17 00:00:00 2001 From: Oscar Tovar Date: Tue, 22 Oct 2024 22:09:03 -0400 Subject: [PATCH 2/5] Refactor security report builder --- .../security_report_builder.rb | 64 ++++++--- .../security_report_builder_spec.rb | 123 +++++++++--------- 2 files changed, 109 insertions(+), 78 deletions(-) diff --git a/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb b/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb index aec5f3e20fe402..0b0a0e6ab53a70 100644 --- a/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb +++ b/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb @@ -6,21 +6,19 @@ class SecurityReportBuilder include Gitlab::Utils::StrongMemoize include Gitlab::VulnerabilityScanning::AdvisoryUtils - def initialize(sbom_report:, project:, pipeline:) - @sbom_report = sbom_report + SECURITY_REPORT_VERSION = "0.0.0" + + def initialize(project:, pipeline:, sbom_report:) @project = project @pipeline = pipeline + @sbom_report = sbom_report + @report = ::Gitlab::Ci::Reports::Security::Report.new(source_type, pipeline, Time.zone.now) + report.version = SECURITY_REPORT_VERSION + report.add_scanner(scanner) end def execute - return unless sbom_report.valid? - return unless sbom_report.source.present? - - return if sbom_report.source.source_type != :dependency_scanning && Feature.disabled?( - :cvs_for_container_scanning, project) - - report = ::Gitlab::Ci::Reports::Security::Report.new('cyclonedx', pipeline, Time.zone.now) - report.add_scanner(scanner) + return unless should_execute? sbom_report.components.each_slice(::Security::IngestionConstants::COMPONENTS_BATCH_SIZE) do |occurrence_batch| affected_packages(occurrence_batch).each_batch do |affected_package_batch| @@ -37,26 +35,52 @@ def execute advisory_data_object = Gitlab::VulnerabilityScanning::Advisory.from_affected_package( affected_package: affected_package, advisory: affected_package.advisory) - finding = ::Security::VulnerabilityScanning::BuildFindingMapService.execute( - advisory: advisory_data_object, - affected_component: affected_occurrence, - source: sbom_report.source, - pipeline: pipeline, - project: project, - purl_type: affected_occurrence.purl.type, - scanner: scanner) - + finding = build_finding(advisory_data_object, affected_occurrence) report.add_finding(finding.report_finding) end end + rescue DependencyScanning::FindingBuilder::MissingPropertiesError, + ContainerScanning::FindingBuilder::MissingPropertiesError => error + report.add_error('MissingPropertiesError', error.message) end report end + strong_memoize_attr :execute # This is an expensive method to call, so we memoize it. private - attr_reader :sbom_report, :project, :pipeline + attr_reader :sbom_report, :project, :pipeline, :report + + def build_finding(advisory_data_object, affected_occurrence) + builder_class = Gitlab::VulnerabilityScanning::FindingBuilder.for_report_type(source_type) + builder_class.new( + project: project, + pipeline: pipeline, + sbom_source: sbom_report.source, + scanner: scanner, + advisory: advisory_data_object, + affected_component: affected_occurrence + ) + end + + def should_execute? + sbom_report_valid? && supported_source_type? + end + + def sbom_report_valid? + sbom_report.valid? && source_type.present? + end + + def supported_source_type? + return Feature.enabled?(:cvs_for_container_scanning, project) if source_type == :container_scanning + + source_type == :dependency_scanning + end + + def source_type + sbom_report&.source&.source_type + end def affected_occurrence?(occurrence, source, affected_package) advisory = affected_package.advisory diff --git a/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb b/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb index d1d3ab900c387b..5ef4fd69f572dc 100644 --- a/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb +++ b/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::VulnerabilityScanning::SecurityReportBuilder, feature_category: :software_composition_analysis do - let(:sbom) { build(:ci_reports_sbom_report, source: sbom_source) } + let(:sbom_report) { build(:ci_reports_sbom_report, source: sbom_source) } let_it_be(:ci_build) { build(:ci_build, pipeline: build(:ci_pipeline, user: build(:user))) } @@ -31,12 +31,11 @@ end subject(:builder) do - described_class.new(report_type: report_type, project: ci_build.project, pipeline: ci_build.pipeline, sbom: sbom) + described_class.new(project: ci_build.project, pipeline: ci_build.pipeline, sbom_report: sbom_report) end before do parser_class.parse!(json_report, expected, validate: true) - builder.add_affections(cves.map { |cve| [cve, affected_component] }) end describe "#report" do @@ -45,7 +44,7 @@ let(:report_type) { "sast" } it "does not add any findings" do - expect(builder.report.findings).to be_empty + expect(builder.execute.findings).to be_empty end end end @@ -60,60 +59,60 @@ version: "v0.0.0-20180419184637-5a16671f721f", purl_type: "golang") end - let_it_be(:cves) do + before do [ - build(:vs_advisory, - xid: "051e7fdd-4e0a-4dfd-ba52-083ee235a580", + create(:pm_advisory, + advisory_xid: "051e7fdd-4e0a-4dfd-ba52-083ee235a580", + source_xid: 'glad', title: "Allocation of File Descriptors or Handles Without Limits or Throttling", description: "Minio a Allocation of Memory Without Limits or Throttling vulnerability in write-to-RAM.", cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", - identifiers: [ - build(:pm_identifier, type: "cve", name: "CVE-2018-1000538", - url: "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000538", value: "CVE-2018-1000538") - ], - urls: ["https://github.com/minio/minio/pull/5957", "https://nvd.nist.gov/vuln/detail/CVE-2018-1000538"], - solution: "Unfortunately, there is no solution available yet." + urls: ["https://github.com/minio/minio/pull/5957", "https://nvd.nist.gov/vuln/detail/CVE-2018-1000538"] + # affected_range: ">=0.0.0", + # fixed_versions: [], + # solution: "Unfortunately, there is no solution available yet." ), - build(:vs_advisory, - xid: "216192fe-2efa-4c52-addd-4bf3522c2b69", + create(:pm_advisory, + advisory_xid: "216192fe-2efa-4c52-addd-4bf3522c2b69", + source_xid: 'glad', title: "Improper Authentication", description: "MinIO versions before has an authentication bypass issue in the MinIO admin API. " \ "Given an admin access key, it is possible to perform admin API operations, i.e., " \ "creating new service accounts for existing access keys without knowing the admin secret key.", cvss_v2: "AV:N/AC:L/Au:N/C:N/I:P/A:N", cvss_v3: "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N", - identifiers: [ - build(:pm_identifier, type: "cve", name: "CVE-2020-11012", - url: "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-11012", value: "CVE-2020-11012") - ], - urls: ["https://nvd.nist.gov/vuln/detail/CVE-2020-11012"], - solution: "Upgrade to version RELEASE.2020-04-23T00-58-49Z or above." + urls: ["https://nvd.nist.gov/vuln/detail/CVE-2020-11012"] + # affected_range: ">=0.0.0", + # fixed_versions: [], + # solution: "Upgrade to version RELEASE.2020-04-23T00-58-49Z or above." ) ] end + let_it_be(:affected_packages) + context "when components are vulnerable" do it "builds a valid report" do - expect(builder.report.errored?).to eq(false) - expect(builder.report.warnings?).to eq(false) + expect(builder.execute.errored?).to eq(false) + expect(builder.execute.warnings?).to eq(false) end it "adds correct findings" do convert_to_hash = ->(finding) { finding.to_hash.slice(*attributes) } - findings = builder.report.findings.map(&convert_to_hash) + findings = builder.execute.findings.map(&convert_to_hash) expected_findings = expected.findings.map(&convert_to_hash) expect(findings).to match_array(expected_findings) end it "adds correct identifiers" do - expect(builder.report.identifiers).to match_array(expected.identifiers) + expect(builder.execute.identifiers).to match_array(expected.identifiers) end it "does not produce or remove findings when compared against analyzer report" do comparer = Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(ci_build.project, expected, - builder.report) + builder.execute) expect(comparer.added).to be_empty expect(comparer.fixed).to be_empty end @@ -123,8 +122,8 @@ let(:sbom_source) { build(:ci_reports_sbom_source, data: {}) } it "adds an error to the report" do - expect(builder.report.errored?).to eq(true) - expect(builder.report.errors).to match_array([ + expect(builder.execute.errored?).to eq(true) + expect(builder.execute.errors).to match_array([ { type: "MissingPropertiesError", message: "Missing required gitlab:dependency_scanning CycloneDX properties" }, { type: "MissingPropertiesError", @@ -137,27 +136,33 @@ end context "for container scanning" do - let_it_be(:cves) do - [ - build(:vs_advisory, - xid: "df6969bdb23ce636df334f8f6d5fe631e58f75c4dc33ec0a4466d4af8e58c9d6", - title: "CVE-2017-18269 in registry.gitlab.com/gitlab-org/security-products/dast/webgoat-8.0@sha256:glibc", - description: "An SSE2-optimized memmove implementation for i386 in " \ - "sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S in the GNU C Library (aka glibc or " \ - "libc6) 2.21 through 2.27 does not correctly perform the overlapping memory check if the " \ - "source memory range spans the middle of the address space, resulting in corrupt data " \ - "being produced by the copy operation. This may disclose information to context-dependent " \ - "attackers, or result in a denial of service, or, possibly, code execution.", - cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", - cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", - identifiers: [ - build(:pm_identifier, type: "cve", name: "CVE-2017-18269", - url: "https://security-tracker.debian.org/tracker/CVE-2017-18269", value: "CVE-2017-18269") - ], - urls: ["https://security-tracker.debian.org/tracker/CVE-2017-18269"], - solution: "Upgrade glibc from 2.24-11+deb9u3 to 2.24-11+deb9u4" - ) - ] + before do + stub_feature_flags(cvs_for_container_scanning: true) + + advisory = create(:pm_advisory, + advisory_xid: "df6969bdb23ce636df334f8f6d5fe631e58f", + source_xid: 'trivy-db', + title: "CVE-2017-18269 in registry.gitlab.com/gitlab-org/security-products/dast/webgoat-8.0@sha256:glibc", + description: "An SSE2-optimized memmove implementation for i386 in " \ + "sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S in the GNU C Library (aka glibc or " \ + "libc6) 2.21 through 2.27 does not correctly perform the overlapping memory check if the " \ + "source memory range spans the middle of the address space, resulting in corrupt data " \ + "being produced by the copy operation. This may disclose information to context-dependent " \ + "attackers, or result in a denial of service, or, possibly, code execution.", + cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", + cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", + urls: ["https://security-tracker.debian.org/tracker/CVE-2017-18269"] + # solution: "Upgrade glibc from 2.24-11+deb9u3 to 2.24-11+deb9u4" + ) + + create(:pm_affected_package, + advisory: advisory, + purl_type: "deb", + distro_version: "debian 9", + package_name: "glibc", + solution: "Upgrade glibc from 2.24-11+deb9u3 to 2.224-11+deb9u4", + affected_range: ">2.24-11+deb9u2 <2.24-11+deb9u4", + fixed_versions: %w[2.24-11+deb9u4]) end let(:report_type) { "container_scanning" } @@ -172,43 +177,45 @@ let(:parser_class) { Gitlab::Ci::Parsers::Security::ContainerScanning } let(:report_path) { 'ee/spec/fixtures/security_reports/simple/gl-container-scanning-report.json' } + let(:sbom_report) { build(:ci_reports_sbom_report, source: sbom_source, components: [affected_component]) } + let_it_be(:affected_component) do - build(:vs_possibly_affected_component, :container_scanning, name: "glibc", + build(:ci_reports_sbom_component, :with_trivy_properties, name: "glibc", version: "2.24-11+deb9u3", purl_type: "deb") end context "when components are vulnerable" do it "builds a valid report" do - expect(builder.report.errored?).to eq(false) - expect(builder.report.warnings?).to eq(false) + expect(builder.execute.errored?).to eq(false) + expect(builder.execute.warnings?).to eq(false) end it "adds correct findings" do convert_to_hash = ->(finding) { finding.to_hash.slice(*attributes) } - findings = builder.report.findings.map(&convert_to_hash) + findings = builder.execute.findings.map(&convert_to_hash) expected_findings = expected.findings.map(&convert_to_hash) expect(findings).to match_array(expected_findings) end it "adds correct identifiers" do - expect(builder.report.identifiers).to match_array(expected.identifiers) + expect(builder.execute.identifiers).to match_array(expected.identifiers) end it "does not produce or remove findings when compared against analyzer report" do comparer = Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(ci_build.project, expected, - builder.report) + builder.execute) expect(comparer.added).to be_empty expect(comparer.fixed).to be_empty end end - context "when supplied cylonedx is incompatible" do + context "when supplied cyclonedx is incompatible" do let(:sbom_source) { build(:ci_reports_sbom_source, data: {}) } it "adds an error to the report" do - expect(builder.report.errored?).to eq(true) - expect(builder.report.errors).to match_array([ + expect(builder.execute.errored?).to eq(true) + expect(builder.execute.errors).to match_array([ { type: "MissingPropertiesError", message: "Missing required gitlab:container_scanning CycloneDX properties" } ]) -- GitLab From 4d05d6047947e071c5e8d88a60bdfac26f067bf6 Mon Sep 17 00:00:00 2001 From: Oscar Tovar Date: Wed, 23 Oct 2024 16:22:20 -0400 Subject: [PATCH 3/5] Fix security report specs after refactor * Create pm_advisory and pm_affected_package once before running specs * Return an error in report when a dependency scanning or container scanning sourced SBOM does not contain vulnerability scanning required properties. --- .../vulnerability_scanning/finding_builder.rb | 2 +- .../security_report_builder.rb | 107 +++++++---- .../security_report_builder_spec.rb | 174 ++++++++++-------- 3 files changed, 170 insertions(+), 113 deletions(-) diff --git a/ee/lib/gitlab/vulnerability_scanning/finding_builder.rb b/ee/lib/gitlab/vulnerability_scanning/finding_builder.rb index 71242abdddfb42..0ddabc7e5d227c 100644 --- a/ee/lib/gitlab/vulnerability_scanning/finding_builder.rb +++ b/ee/lib/gitlab/vulnerability_scanning/finding_builder.rb @@ -14,7 +14,7 @@ class FindingBuilder include Gitlab::Utils::StrongMemoize def self.for_report_type(report_type) - case report_type + case report_type.to_s when "dependency_scanning" ::Gitlab::VulnerabilityScanning::DependencyScanning::FindingBuilder when "container_scanning", "container_scanning_for_registry" diff --git a/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb b/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb index 0b0a0e6ab53a70..b4926fd0242521 100644 --- a/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb +++ b/ee/lib/gitlab/vulnerability_scanning/security_report_builder.rb @@ -20,29 +20,11 @@ def initialize(project:, pipeline:, sbom_report:) def execute return unless should_execute? - sbom_report.components.each_slice(::Security::IngestionConstants::COMPONENTS_BATCH_SIZE) do |occurrence_batch| - affected_packages(occurrence_batch).each_batch do |affected_package_batch| - affected_package_batch.filter_map do |affected_package| - # We need to match every affected package to one occurrence - affected_occurrence = occurrence_batch.find do |occurrence| - next unless affected_package.package_name == occurrence.name - - affected_occurrence?(occurrence, sbom_report.source, affected_package) - end + validate! - next unless affected_occurrence.present? + return report if report.errored? - advisory_data_object = Gitlab::VulnerabilityScanning::Advisory.from_affected_package( - affected_package: affected_package, advisory: affected_package.advisory) - - finding = build_finding(advisory_data_object, affected_occurrence) - report.add_finding(finding.report_finding) - end - end - rescue DependencyScanning::FindingBuilder::MissingPropertiesError, - ContainerScanning::FindingBuilder::MissingPropertiesError => error - report.add_error('MissingPropertiesError', error.message) - end + scan_sbom_report_occurrences report end @@ -52,17 +34,10 @@ def execute attr_reader :sbom_report, :project, :pipeline, :report - def build_finding(advisory_data_object, affected_occurrence) - builder_class = Gitlab::VulnerabilityScanning::FindingBuilder.for_report_type(source_type) - builder_class.new( - project: project, - pipeline: pipeline, - sbom_source: sbom_report.source, - scanner: scanner, - advisory: advisory_data_object, - affected_component: affected_occurrence - ) + def scanner + VulnerabilityScanning::SecurityScanner.fabricate end + strong_memoize_attr :scanner def should_execute? sbom_report_valid? && supported_source_type? @@ -82,6 +57,60 @@ def source_type sbom_report&.source&.source_type end + def validate! + return if supported_dependency_scanning_sbom? || supported_container_scanning_sbom? + + add_missing_properties_error + end + + def supported_dependency_scanning_sbom? + sbom_report&.source&.input_file_path.present? || sbom_report&.source&.source_file_path.present? + end + + def supported_container_scanning_sbom? + sbom_report&.source&.image_name.present? && sbom_report&.source&.image_tag.present? + end + + def add_missing_properties_error + report.add_error('MissingPropertiesError', missing_properties_error) + end + + def missing_properties_error + namespace = case source_type + when :dependency_scanning + 'gitlab:dependency_scanning' + when :container_scanning + 'gitlab:container_scanning' + end + + "Missing required #{namespace} CycloneDX properties" + end + + def scan_sbom_report_occurrences + sbom_report.components.each_slice(::Security::IngestionConstants::COMPONENTS_BATCH_SIZE) do |occurrence_batch| + affected_packages(occurrence_batch).each_batch do |affected_package_batch| + affected_package_batch.each do |affected_package| + occurrence_batch.each do |occurrence| + next unless affected_package.package_name == occurrence.name + + next unless affected_occurrence?(occurrence, sbom_report.source, affected_package) + + advisory_data_object = Gitlab::VulnerabilityScanning::Advisory.from_affected_package( + affected_package: affected_package, advisory: affected_package.advisory) + + finding = build_finding(advisory_data_object, occurrence) + report.add_finding(finding) + finding.identifiers.each { |ident| report.add_identifier(ident) } + end + end + end + end + end + + def affected_packages(occurrence_batch) + ::PackageMetadata::AffectedPackage.for_occurrences(occurrence_batch).with_advisory + end + def affected_occurrence?(occurrence, source, affected_package) advisory = affected_package.advisory occurrence_is_affected?( @@ -96,14 +125,18 @@ def affected_occurrence?(occurrence, source, affected_package) ) end - def affected_packages(occurrence_batch) - ::PackageMetadata::AffectedPackage.for_occurrences(occurrence_batch).with_advisory - end + def build_finding(advisory_data_object, affected_occurrence) + builder_class = Gitlab::VulnerabilityScanning::FindingBuilder.for_report_type(source_type) - def scanner - VulnerabilityScanning::SecurityScanner.fabricate + builder_class.new( + project: project, + pipeline: pipeline, + sbom_source: sbom_report.source, + scanner: scanner, + advisory: advisory_data_object, + affected_component: affected_occurrence + ).finding end - strong_memoize_attr :scanner end end end diff --git a/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb b/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb index 5ef4fd69f572dc..3e7b6fdde8dae6 100644 --- a/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb +++ b/ee/spec/lib/gitlab/vulnerability_scanning/security_report_builder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::VulnerabilityScanning::SecurityReportBuilder, feature_category: :software_composition_analysis do - let(:sbom_report) { build(:ci_reports_sbom_report, source: sbom_source) } + let(:sbom_report) { build(:ci_reports_sbom_report, source: sbom_source, components: [affected_component]) } let_it_be(:ci_build) { build(:ci_build, pipeline: build(:ci_pipeline, user: build(:user))) } @@ -34,6 +34,91 @@ described_class.new(project: ci_build.project, pipeline: ci_build.pipeline, sbom_report: sbom_report) end + before_all do + stub_feature_flags(cvs_for_container_scanning: true) + + create(:pm_affected_package, + advisory: create(:pm_advisory, + advisory_xid: "051e7fdd-4e0a-4dfd-ba52-083ee235a580", + source_xid: 'glad', + title: "Allocation of File Descriptors or Handles Without Limits or Throttling", + description: "Minio a Allocation of Memory Without Limits or Throttling vulnerability in write-to-RAM.", + cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", + cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", + urls: ["https://github.com/minio/minio/pull/5957", "https://nvd.nist.gov/vuln/detail/CVE-2018-1000538"], + identifiers: [ + create(:pm_identifier, + type: "cve", + name: "CVE-2018-1000538", + value: "CVE-2018-1000538", + url: "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000538") + ] + ), + purl_type: "golang", + package_name: "github.com/minio/minio", + solution: "", + affected_range: "<1.0.0", + fixed_versions: [] + ) + + create(:pm_affected_package, + advisory: create(:pm_advisory, + advisory_xid: "216192fe-2efa-4c52-addd-4bf3522c2b69", + source_xid: 'glad', + title: "Improper Authentication", + description: "MinIO versions before has an authentication bypass issue in the MinIO admin API. " \ + "Given an admin access key, it is possible to perform admin API operations, i.e., " \ + "creating new service accounts for existing access keys without knowing the admin secret key.", + cvss_v2: "AV:N/AC:L/Au:N/C:N/I:P/A:N", + cvss_v3: "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N", + urls: ["https://nvd.nist.gov/vuln/detail/CVE-2020-11012"], + identifiers: [ + create(:pm_identifier, + type: "cve", + name: "CVE-2020-11012", + value: "CVE-2020-11012", + url: "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-11012") + ] + ), + purl_type: "golang", + package_name: "github.com/minio/minio", + solution: "Upgrade to version RELEASE.2020-04-23T00-58-49Z or above.", + affected_range: "<1.0.0", + fixed_versions: %w[RELEASE.2020-04-23T00-58-49Z] + ) + + create(:pm_affected_package, + advisory: create(:pm_advisory, + advisory_xid: "df6969bdb23ce636df334f8f6d5fe631e58f", + source_xid: 'trivy-db', + title: "CVE-2017-18269 in registry.gitlab.com/gitlab-org/security-products/dast/webgoat-8.0@sha256:glibc", + description: "An SSE2-optimized memmove implementation for i386 in " \ + "sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S in the GNU C Library (aka glibc or " \ + "libc6) 2.21 through 2.27 does not correctly perform the overlapping memory check if the " \ + "source memory range spans the middle of the address space, resulting in corrupt data " \ + "being produced by the copy operation. This may disclose information to context-dependent " \ + "attackers, or result in a denial of service, or, possibly, code execution.", + cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", + cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", + urls: ["https://security-tracker.debian.org/tracker/CVE-2017-18269"], + identifiers: [ + create(:pm_identifier, + type: "cve", + name: "CVE-2017-18269", + value: "CVE-2017-18269", + url: "https://security-tracker.debian.org/tracker/CVE-2017-18269" + ) + ] + ), + purl_type: "deb", + distro_version: "debian 9", + package_name: "glibc", + solution: "Upgrade glibc from 2.24-11+deb9u3 to 2.24-11+deb9u4", + affected_range: "*", + fixed_versions: %w[2.24-11+deb9u4] + ) + end + before do parser_class.parse!(json_report, expected, validate: true) end @@ -42,9 +127,10 @@ shared_examples_for 'it handles unsupported report types' do context "when report type is not supported" do let(:report_type) { "sast" } + let(:sbom_source) { build(:ci_reports_sbom_source, type: :sast) } - it "does not add any findings" do - expect(builder.execute.findings).to be_empty + it "does not return a report" do + expect(builder.execute).to be_nil end end end @@ -52,46 +138,17 @@ context "for dependency scanning" do let(:report_type) { "dependency_scanning" } let(:parser_class) { Gitlab::Ci::Parsers::Security::DependencyScanning } - let(:sbom_source) { build(:ci_reports_sbom_source, input_file_path: "go.mod", source_file_path: "go.mod") } + let(:sbom_source) do + build(:ci_reports_sbom_source, type: :dependency_scanning, input_file_path: "go.mod", + source_file_path: "go.mod") + end + let(:report_path) { 'ee/spec/fixtures/security_reports/simple/gl-dependency-scanning-report.json' } let_it_be(:affected_component) do - build(:vs_possibly_affected_component, name: "github.com/minio/minio", + build(:ci_reports_sbom_component, name: "github.com/minio/minio", version: "v0.0.0-20180419184637-5a16671f721f", purl_type: "golang") end - before do - [ - create(:pm_advisory, - advisory_xid: "051e7fdd-4e0a-4dfd-ba52-083ee235a580", - source_xid: 'glad', - title: "Allocation of File Descriptors or Handles Without Limits or Throttling", - description: "Minio a Allocation of Memory Without Limits or Throttling vulnerability in write-to-RAM.", - cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", - cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", - urls: ["https://github.com/minio/minio/pull/5957", "https://nvd.nist.gov/vuln/detail/CVE-2018-1000538"] - # affected_range: ">=0.0.0", - # fixed_versions: [], - # solution: "Unfortunately, there is no solution available yet." - ), - create(:pm_advisory, - advisory_xid: "216192fe-2efa-4c52-addd-4bf3522c2b69", - source_xid: 'glad', - title: "Improper Authentication", - description: "MinIO versions before has an authentication bypass issue in the MinIO admin API. " \ - "Given an admin access key, it is possible to perform admin API operations, i.e., " \ - "creating new service accounts for existing access keys without knowing the admin secret key.", - cvss_v2: "AV:N/AC:L/Au:N/C:N/I:P/A:N", - cvss_v3: "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N", - urls: ["https://nvd.nist.gov/vuln/detail/CVE-2020-11012"] - # affected_range: ">=0.0.0", - # fixed_versions: [], - # solution: "Upgrade to version RELEASE.2020-04-23T00-58-49Z or above." - ) - ] - end - - let_it_be(:affected_packages) - context "when components are vulnerable" do it "builds a valid report" do expect(builder.execute.errored?).to eq(false) @@ -118,14 +175,12 @@ end end - context "when supplied cylonedx is incompatible" do - let(:sbom_source) { build(:ci_reports_sbom_source, data: {}) } + context "when cylonedx is missing required properties" do + let(:sbom_source) { build(:ci_reports_sbom_source, type: :dependency_scanning, data: {}) } it "adds an error to the report" do expect(builder.execute.errored?).to eq(true) expect(builder.execute.errors).to match_array([ - { type: "MissingPropertiesError", - message: "Missing required gitlab:dependency_scanning CycloneDX properties" }, { type: "MissingPropertiesError", message: "Missing required gitlab:dependency_scanning CycloneDX properties" } ]) @@ -136,35 +191,6 @@ end context "for container scanning" do - before do - stub_feature_flags(cvs_for_container_scanning: true) - - advisory = create(:pm_advisory, - advisory_xid: "df6969bdb23ce636df334f8f6d5fe631e58f", - source_xid: 'trivy-db', - title: "CVE-2017-18269 in registry.gitlab.com/gitlab-org/security-products/dast/webgoat-8.0@sha256:glibc", - description: "An SSE2-optimized memmove implementation for i386 in " \ - "sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S in the GNU C Library (aka glibc or " \ - "libc6) 2.21 through 2.27 does not correctly perform the overlapping memory check if the " \ - "source memory range spans the middle of the address space, resulting in corrupt data " \ - "being produced by the copy operation. This may disclose information to context-dependent " \ - "attackers, or result in a denial of service, or, possibly, code execution.", - cvss_v2: "AV:N/AC:L/Au:N/C:N/I:N/A:P", - cvss_v3: "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H", - urls: ["https://security-tracker.debian.org/tracker/CVE-2017-18269"] - # solution: "Upgrade glibc from 2.24-11+deb9u3 to 2.24-11+deb9u4" - ) - - create(:pm_affected_package, - advisory: advisory, - purl_type: "deb", - distro_version: "debian 9", - package_name: "glibc", - solution: "Upgrade glibc from 2.24-11+deb9u3 to 2.224-11+deb9u4", - affected_range: ">2.24-11+deb9u2 <2.24-11+deb9u4", - fixed_versions: %w[2.24-11+deb9u4]) - end - let(:report_type) { "container_scanning" } let(:sbom_source) do build(:ci_reports_sbom_source, :container_scanning, @@ -177,8 +203,6 @@ let(:parser_class) { Gitlab::Ci::Parsers::Security::ContainerScanning } let(:report_path) { 'ee/spec/fixtures/security_reports/simple/gl-container-scanning-report.json' } - let(:sbom_report) { build(:ci_reports_sbom_report, source: sbom_source, components: [affected_component]) } - let_it_be(:affected_component) do build(:ci_reports_sbom_component, :with_trivy_properties, name: "glibc", version: "2.24-11+deb9u3", purl_type: "deb") @@ -210,8 +234,8 @@ end end - context "when supplied cyclonedx is incompatible" do - let(:sbom_source) { build(:ci_reports_sbom_source, data: {}) } + context "when supplied cyclonedx is missing required properties" do + let(:sbom_source) { build(:ci_reports_sbom_source, type: :container_scanning, data: {}) } it "adds an error to the report" do expect(builder.execute.errored?).to eq(true) -- GitLab From de4b77a84afaaf35699d088fc90ce3a3204b27bc Mon Sep 17 00:00:00 2001 From: Oscar Tovar Date: Wed, 23 Oct 2024 16:31:30 -0400 Subject: [PATCH 4/5] Skip security report ingestion for empty reports Not all CycloneDX reports can produce a security report (we only support dependency scanning and container scanning w/ a feature flag enabled). When a CycloneDX is not supported, no security report is returned, and we must skip security ingestion in this scenario. --- ee/app/models/ee/ci/job_artifact.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ee/app/models/ee/ci/job_artifact.rb b/ee/app/models/ee/ci/job_artifact.rb index 5a11ab10504a9f..ad991cc82a0f19 100644 --- a/ee/app/models/ee/ci/job_artifact.rb +++ b/ee/app/models/ee/ci/job_artifact.rb @@ -142,6 +142,9 @@ def security_report(validate: false) end end report = ::Gitlab::VulnerabilityScanning::SecurityReportBuilder.new(sbom_report: sbom_report, project: project, pipeline: job.pipeline).execute + + # Not all CycloneDX files can be scanned to produce a security report. + next unless report else report = ::Gitlab::Ci::Reports::Security::Report.new(file_type, job.pipeline, nil).tap do |report| each_blob do |blob| -- GitLab From 3ec0f1bd0f80b5e0b6b0f6ae05aa76ab6edf44ab Mon Sep 17 00:00:00 2001 From: Oscar Tovar Date: Fri, 25 Oct 2024 20:45:05 -0400 Subject: [PATCH 5/5] Remove CVS report type coupling --- doc/api/graphql/reference/index.md | 1 + .../merge_request_security_report_generation_service.rb | 2 +- ee/app/services/security/store_scan_service.rb | 8 +++++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 116f696af24faa..38b8141f8223cc 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -37315,6 +37315,7 @@ Comparable security report type. | `API_FUZZING` | API Fuzzing report. | | `CONTAINER_SCANNING` | Container Scanning report. | | `COVERAGE_FUZZING` | Coverage Fuzzing report. | +| `CYCLONEDX` | Cyclonedx report. | | `DAST` | DAST report. | | `DEPENDENCY_SCANNING` | Dependency Scanning report. | | `SAST` | SAST report. | diff --git a/ee/app/services/security/merge_request_security_report_generation_service.rb b/ee/app/services/security/merge_request_security_report_generation_service.rb index a029ef2192ef38..0446f101f82ab6 100644 --- a/ee/app/services/security/merge_request_security_report_generation_service.rb +++ b/ee/app/services/security/merge_request_security_report_generation_service.rb @@ -6,7 +6,7 @@ class MergeRequestSecurityReportGenerationService DEFAULT_FINDING_STATE = 'detected' ALLOWED_REPORT_TYPES = %w[sast secret_detection container_scanning - dependency_scanning dast coverage_fuzzing api_fuzzing].freeze + dependency_scanning dast coverage_fuzzing api_fuzzing cyclonedx].freeze InvalidReportTypeError = Class.new(ArgumentError) diff --git a/ee/app/services/security/store_scan_service.rb b/ee/app/services/security/store_scan_service.rb index 7968ded1fe2368..c156d2e38a1054 100644 --- a/ee/app/services/security/store_scan_service.rb +++ b/ee/app/services/security/store_scan_service.rb @@ -52,7 +52,7 @@ def override_uuids? end def security_scan - @security_scan ||= Security::Scan.safe_find_or_create_by!(build: job, scan_type: artifact.file_type) do |scan| + @security_scan ||= Security::Scan.safe_find_or_create_by!(build: job, scan_type: scan_type) do |scan| scan.created_at = pipeline.created_at # to make sure retried jobs does not extend the retention period of security findings related to the pipeline. scan.processing_errors = security_report.errors.map(&:stringify_keys) if security_report.errored? scan.processing_warnings = security_report.warnings.map(&:stringify_keys) if security_report.warnings? @@ -123,5 +123,11 @@ def mark_scan_as_failed! security_scan.add_processing_error!(SCAN_INGESTION_ERROR) end + + def scan_type + artifact.file_type unless artifact.file_type == :cyclonedx + + security_report.type + end end end -- GitLab