Handle new Container Scanning report format

Deprecate old `clair-scanner` container scanning report format and
handle the new report format based on the Security Products Common
Format

Add conditional to switch container scanning formatting behaviour
based on report type

Update container scanning fixtures to include deprecated `clair-scanner`
report and Security Products Common Format

changelogs/unreleased/add-rails-parser-for-new-cs-report-format.yml

+---
+title: Handle new Container Scanning report format
+merge_request: 19123
+author:
+type: changed

ee/lib/gitlab/ci/parsers/security/container_scanning.rb

@@ -9,32 +9,35 @@ module Gitlab
 
           DEPRECATED_REPORT_VERSION = "1.3".freeze
 
-          def parse!(json_data, report)
-            vulnerabilities = format_report(JSON.parse!(json_data))
+          def parse_report(json_data)
+            report = super
 
-            vulnerabilities.each do |vulnerability|
-              create_vulnerability(report, vulnerability, DEPRECATED_REPORT_VERSION)
-            end
-          rescue JSON::ParserError
-            raise SecurityReportParserError, 'JSON parsing failed'
-          rescue
-            raise SecurityReportParserError, "#{report.type} security report parsing failed"
+            return format_deprecated_report(report) if deprecated?(report)
+
+            report
           end
 
           private
 
-          # Transforms the Clair JSON report into the expected format
-          def format_report(data)
-            vulnerabilities = data['vulnerabilities']
+          # Transforms the clair-scanner JSON report into the expected format
+          # TODO: remove the following block when we no longer need to support legacy
+          # clair-scanner data. See https://gitlab.com/gitlab-org/gitlab/issues/35442
+          def format_deprecated_report(data)
             unapproved = data['unapproved']
-            formatter = Formatters::ContainerScanning.new(data['image'])
+            formatter = Formatters::DeprecatedContainerScanning.new(data['image'])
 
-            vulnerabilities.map do |vulnerability|
+            vulnerabilities = data['vulnerabilities'].map do |vulnerability|
               # We only report unapproved vulnerabilities
               next unless unapproved.include?(vulnerability['vulnerability'])
 
               formatter.format(vulnerability)
             end.compact
+
+            { "vulnerabilities" => vulnerabilities, "version" => DEPRECATED_REPORT_VERSION }
+          end
+
+          def deprecated?(data)
+            data['image']
           end
 
           def create_location(location_data)

ee/lib/gitlab/ci/parsers/security/formatters/container_scanning.rb → ee/lib/gitlab/ci/parsers/security/formatters/deprecated_container_scanning.rb

 # frozen_string_literal: true
 
+# TODO: remove this class when we no longer need to support legacy
+# clair-scanner data. See https://gitlab.com/gitlab-org/gitlab/issues/35442
 module Gitlab
   module Ci
     module Parsers
       module Security
         module Formatters
-          class ContainerScanning
+          class DeprecatedContainerScanning
             def initialize(image)
               @image = image
             end
 
             def format(vulnerability)
-              formatted_vulnerability = FormattedContainerScanningVulnerability.new(vulnerability)
+              formatted_vulnerability = DeprecatedFormattedContainerScanningVulnerability.new(vulnerability)
 
               {
                 'category' => 'container_scanning',

ee/lib/gitlab/ci/parsers/security/formatters/formatted_container_scanning_vulnerability.rb → ee/lib/gitlab/ci/parsers/security/formatters/deprecated_formatted_container_scanning_vulnerability.rb

 # frozen_string_literal: true
 
+# TODO: remove this class when we no longer need to support legacy
+# clair-scanner data. See https://gitlab.com/gitlab-org/gitlab/issues/35442
 module Gitlab
   module Ci
     module Parsers
       module Security
         module Formatters
-          class FormattedContainerScanningVulnerability
+          class DeprecatedFormattedContainerScanningVulnerability
             def initialize(vulnerability)
               @vulnerability = vulnerability
             end

ee/spec/factories/ci/builds.rb

@@ -72,6 +72,12 @@ FactoryBot.define do
       end
     end
 
+    trait :deprecated_container_scanning_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :deprecated_container_scanning_report, job: build)
+      end
+    end
+
     trait :dependency_scanning_feature_branch do
       after(:build) do |build|
         build.job_artifacts << create(:ee_ci_job_artifact, :dependency_scanning_feature_branch, job: build)

ee/spec/factories/ci/job_artifacts.rb

@@ -232,6 +232,16 @@ FactoryBot.define do
       end
     end
 
+    trait :deprecated_container_scanning_report do
+      file_format { :raw }
+      file_type { :container_scanning }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-container-scanning-report.json'), 'text/plain')
+      end
+    end
+
     trait :metrics do
       file_format { :gzip }
       file_type { :metrics }

ee/spec/finders/security/pipeline_vulnerabilities_finder_spec.rb

@@ -19,22 +19,22 @@ describe Security::PipelineVulnerabilitiesFinder do
     end
   end
 
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
+  let_it_be(:build_cs) { create(:ci_build, :success, name: 'cs_job', pipeline: pipeline, project: project) }
+
+  shared_examples_for 'a pipeline vulnerabilities finder' do
     describe '#execute' do
-    set(:project) { create(:project, :repository) }
-    set(:pipeline) { create(:ci_pipeline, :success, project: project) }
       let(:params) { {} }
 
-    set(:build_cs) { create(:ci_build, :success, name: 'cs_job', pipeline: pipeline, project: project) }
-    set(:build_dast) { create(:ci_build, :success, name: 'dast_job', pipeline: pipeline, project: project) }
-    set(:build_ds) { create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) }
-    set(:build_sast) { create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) }
+      let_it_be(:build_dast) { create(:ci_build, :success, name: 'dast_job', pipeline: pipeline, project: project) }
+      let_it_be(:build_ds) { create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) }
+      let_it_be(:build_sast) { create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) }
 
-    set(:artifact_cs) { create(:ee_ci_job_artifact, :container_scanning, job: build_cs, project: project) }
-    set(:artifact_dast) { create(:ee_ci_job_artifact, :dast, job: build_dast, project: project) }
-    set(:artifact_ds) { create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project) }
-    set(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_sast, project: project) }
+      let_it_be(:artifact_dast) { create(:ee_ci_job_artifact, :dast, job: build_dast, project: project) }
+      let_it_be(:artifact_ds) { create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project) }
+      let_it_be(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_sast, project: project) }
 
-    let(:cs_count) { read_fixture(artifact_cs)['unapproved'].count }
       let(:ds_count) { read_fixture(artifact_ds)['vulnerabilities'].count }
       let(:sast_count) { read_fixture(artifact_sast)['vulnerabilities'].count }
       let(:dast_count) do
@@ -67,7 +67,8 @@ describe Security::PipelineVulnerabilitiesFinder do
         let!(:unknown_low) { build(:vulnerabilities_occurrence, confidence: :low, severity: :unknown) }
 
         it 'orders by severity and confidence' do
-        allow_any_instance_of(described_class).to receive(:filter).and_return([
+          allow_next_instance_of(described_class) do |pipeline_vulnerabilities_finder|
+            allow(pipeline_vulnerabilities_finder).to receive(:filter).and_return([
                  unknown_low,
                  unknown_medium,
                  critical_high,
@@ -79,6 +80,7 @@ describe Security::PipelineVulnerabilitiesFinder do
             expect(subject).to eq([critical_high, critical_medium, high_high, unknown_high, unknown_medium, unknown_low])
           end
         end
+      end
 
       context 'by report type' do
         context 'when sast' do
@@ -307,4 +309,19 @@ describe Security::PipelineVulnerabilitiesFinder do
         JSON.parse(File.read(fixture.file.path))
       end
     end
+  end
+
+  context 'container_scanning' do
+    let_it_be(:artifact_cs) { create(:ee_ci_job_artifact, :container_scanning, job: build_cs, project: project) }
+    let(:cs_count) { read_fixture(artifact_cs)['vulnerabilities'].count }
+
+    it_behaves_like 'a pipeline vulnerabilities finder'
+  end
+
+  context 'deprecated container_scanning' do
+    let_it_be(:artifact_cs) { create(:ee_ci_job_artifact, :deprecated_container_scanning_report, job: build_cs, project: project) }
+    let(:cs_count) { read_fixture(artifact_cs)['unapproved'].count }
+
+    it_behaves_like 'a pipeline vulnerabilities finder'
+  end
 end

ee/spec/fixtures/security_reports/deprecated/gl-container-scanning-report.json

+{
+  "image": "registry.gitlab.com/groulot/container-scanning-test/master:5f21de6956aee99ddb68ae49498662d9872f50ff",
+  "unapproved": [
+    "CVE-2017-18269",
+    "CVE-2017-16997",
+    "CVE-2018-1000001",
+    "CVE-2016-10228",
+    "CVE-2018-18520",
+    "CVE-2010-4052",
+    "CVE-2018-16869",
+    "CVE-2018-18311"
+  ],
+  "vulnerabilities": [
+    {
+      "featurename": "glibc",
+      "featureversion": "2.24-11+deb9u3",
+      "vulnerability": "CVE-2017-18269",
+      "namespace": "debian:9",
+      "description": "SSE2-optimized memmove implementation problem.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2017-18269",
+      "severity": "Defcon1",
+      "fixedby": "2.24-11+deb9u4"
+    },
+    {
+      "featurename": "glibc",
+      "featureversion": "2.24-11+deb9u3",
+      "vulnerability": "CVE-2017-16997",
+      "namespace": "debian:9",
+      "description": "elf/dl-load.c in the GNU C Library (aka glibc or libc6) 2.19 through 2.26 mishandles RPATH and RUNPATH containing $ORIGIN for a privileged (setuid or AT_SECURE) program, which allows local users to gain privileges via a Trojan horse library in the current working directory, related to the fillin_rpath and decompose_rpath functions. This is associated with misinterpretion of an empty RPATH/RUNPATH token as the \"./\" directory. NOTE: this configuration of RPATH/RUNPATH for a privileged program is apparently very uncommon; most likely, no such program is shipped with any common Linux distribution.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2017-16997",
+      "severity": "Critical",
+      "fixedby": ""
+    },
+    {
+      "featurename": "glibc",
+      "featureversion": "2.24-11+deb9u3",
+      "vulnerability": "CVE-2018-1000001",
+      "namespace": "debian:9",
+      "description": "In glibc 2.26 and earlier there is confusion in the usage of getcwd() by realpath() which can be used to write before the destination buffer leading to a buffer underflow and potential code execution.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2018-1000001",
+      "severity": "High",
+      "fixedby": ""
+    },
+    {
+      "featurename": "glibc",
+      "featureversion": "2.24-11+deb9u3",
+      "vulnerability": "CVE-2016-10228",
+      "namespace": "debian:9",
+      "description": "The iconv program in the GNU C Library (aka glibc or libc6) 2.25 and earlier, when invoked with the -c option, enters an infinite loop when processing invalid multi-byte input sequences, leading to a denial of service.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2016-10228",
+      "severity": "Medium",
+      "fixedby": ""
+    },
+    {
+      "featurename": "elfutils",
+      "featureversion": "0.168-1",
+      "vulnerability": "CVE-2018-18520",
+      "namespace": "debian:9",
+      "description": "An Invalid Memory Address Dereference exists in the function elf_end in libelf in elfutils through v0.174. Although eu-size is intended to support ar files inside ar files, handle_ar in size.c closes the outer ar file before handling all inner entries. The vulnerability allows attackers to cause a denial of service (application crash) with a crafted ELF file.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2018-18520",
+      "severity": "Low",
+      "fixedby": ""
+    },
+    {
+      "featurename": "glibc",
+      "featureversion": "2.24-11+deb9u3",
+      "vulnerability": "CVE-2010-4052",
+      "namespace": "debian:9",
+      "description": "Stack consumption vulnerability in the regcomp implementation in the GNU C Library (aka glibc or libc6) through 2.11.3, and 2.12.x through 2.12.2, allows context-dependent attackers to cause a denial of service (resource exhaustion) via a regular expression containing adjacent repetition operators, as demonstrated by a {10,}{10,}{10,}{10,} sequence in the proftpd.gnu.c exploit for ProFTPD.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2010-4052",
+      "severity": "Negligible",
+      "fixedby": ""
+    },
+    {
+      "featurename": "nettle",
+      "featureversion": "3.3-1",
+      "vulnerability": "CVE-2018-16869",
+      "namespace": "debian:9",
+      "description": "A Bleichenbacher type side-channel based padding oracle attack was found in the way nettle handles endian conversion of RSA decrypted PKCS#1 v1.5 data. An attacker who is able to run a process on the same physical core as the victim process, could use this flaw extract plaintext or in some cases downgrade any TLS connections to a vulnerable server.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2018-16869",
+      "severity": "Unknown",
+      "fixedby": ""
+    },
+    {
+      "featurename": "perl",
+      "featureversion": "5.24.1-3+deb9u4",
+      "vulnerability": "CVE-2018-18311",
+      "namespace": "debian:9",
+      "description": "Perl before 5.26.3 and 5.28.x before 5.28.1 has a buffer overflow via a crafted regular expression that triggers invalid write operations.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2018-18311",
+      "severity": "Unknown",
+      "fixedby": "5.24.1-3+deb9u5"
+    },
+    {
+      "featurename": "foo",
+      "featureversion": "1.3",
+      "vulnerability": "CVE-2018-666",
+      "namespace": "debian:9",
+      "description": "Foo has a vulnerability nobody cares about and whitelist.",
+      "link": "https://security-tracker.debian.org/tracker/CVE-2018-666",
+      "severity": "Unknown",
+      "fixedby": "1.4"
+    }
+  ]
+}

ee/spec/lib/gitlab/ci/parsers/security/container_scanning_spec.rb

@@ -4,19 +4,8 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Parsers::Security::ContainerScanning do
   let(:parser) { described_class.new }
-
-  let(:clair_vulnerabilities) do
-    JSON.parse!(
-      File.read(
-        Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json')
-      )
-    )['vulnerabilities']
-  end
-
-  describe '#parse!' do
   let(:project) { artifact.project }
   let(:pipeline) { artifact.job.pipeline }
-    let(:artifact) { create(:ee_ci_job_artifact, :container_scanning) }
   let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline.sha) }
 
   before do
@@ -25,6 +14,17 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
     end
   end
 
+  describe '#parse!' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:report_type, :image, :version) do
+      :deprecated_container_scanning_report  | 'registry.gitlab.com/groulot/container-scanning-test/master:5f21de6956aee99ddb68ae49498662d9872f50ff'                                        | '1.3'
+      :container_scanning                    | 'registry.gitlab.com/gitlab-org/security-products/dast/webgoat-8.0@sha256:bc09fe2e0721dfaeee79364115aeedf2174cce0947b9ae5fe7c33312ee019a4e'  | '2.3'
+    end
+
+    with_them do
+      let(:artifact) { create(:ee_ci_job_artifact, report_type) }
+
       it "parses all identifiers and occurrences for unapproved vulnerabilities" do
         expect(report.occurrences.length).to eq(8)
         expect(report.identifiers.length).to eq(8)
@@ -36,7 +36,7 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
 
         expect(location).to be_a(::Gitlab::Ci::Reports::Security::Locations::ContainerScanning)
         expect(location).to have_attributes(
-        image: 'registry.gitlab.com/groulot/container-scanning-test/master:5f21de6956aee99ddb68ae49498662d9872f50ff',
+          image: image,
           operating_system: 'debian:9',
           package_name: 'glibc',
           package_version: '2.24-11+deb9u3'
@@ -44,12 +44,12 @@ describe Gitlab::Ci::Parsers::Security::ContainerScanning do
       end
 
       it "generates expected metadata_version" do
-      expect(report.occurrences.first.metadata_version).to eq('1.3')
+        expect(report.occurrences.first.metadata_version).to eq(version)
       end
 
       it "adds report image's name to raw_metadata" do
-      expect(JSON.parse(report.occurrences.first.raw_metadata).dig('location', 'image'))
-        .to eq('registry.gitlab.com/groulot/container-scanning-test/master:5f21de6956aee99ddb68ae49498662d9872f50ff')
+        expect(JSON.parse(report.occurrences.first.raw_metadata).dig('location', 'image')).to eq(image)
+      end
     end
   end
 end

ee/spec/lib/gitlab/ci/parsers/security/formatters/container_scanning_spec.rb → ee/spec/lib/gitlab/ci/parsers/security/formatters/deprecated_container_scanning_spec.rb

@@ -2,19 +2,19 @@
 
 require 'spec_helper'
 
-describe Gitlab::Ci::Parsers::Security::Formatters::ContainerScanning do
+describe Gitlab::Ci::Parsers::Security::Formatters::DeprecatedContainerScanning do
+  let(:vulnerability) { raw_report['vulnerabilities'].first }
+
+  describe '#format' do
     let(:raw_report) do
       JSON.parse!(
         File.read(
-        Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json')
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-container-scanning-report.json')
         )
       )
     end
 
-  let(:vulnerability) { raw_report['vulnerabilities'].first }
-
-  describe '#format' do
-    it 'format ZAP vulnerability into the 1.3 format' do
+    it 'formats the vulnerability into the 1.3 format' do
       formatter = described_class.new('image_name')
 
       expect(formatter.format(vulnerability)).to eq( {

ee/spec/lib/gitlab/ci/parsers/security/formatters/formatted_container_scanning_vulnerability_spec.rb → ee/spec/lib/gitlab/ci/parsers/security/formatters/deprecated_formatted_container_scanning_vulnerability_spec.rb

@@ -2,11 +2,11 @@
 
 require 'spec_helper'
 
-describe Gitlab::Ci::Parsers::Security::Formatters::FormattedContainerScanningVulnerability do
+describe Gitlab::Ci::Parsers::Security::Formatters::DeprecatedFormattedContainerScanningVulnerability do
   let(:raw_report) do
     JSON.parse!(
       File.read(
-        Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json')
+        Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-container-scanning-report.json')
       )
     )
   end

ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb

@@ -18,7 +18,7 @@ describe Ci::CompareContainerScanningReportsService do
       let!(:base_pipeline) { create(:ee_ci_pipeline) }
       let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report, project: project) }
 
-      it 'reports new licenses' do
+      it 'reports new, existing and fixed vulnerabilities' do
         expect(subject[:status]).to eq(:parsed)
         expect(subject[:data]['added'].count).to eq(8)
         expect(subject[:data]['existing'].count).to eq(0)