Merge branch 'add-rails-parser-for-new-cs-report-format' into 'master'

Handle new Container Scanning report format

Closes #33825

See merge request gitlab-org/gitlab!19123

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))
-
-            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"
+          def parse_report(json_data)
+            report = super
+
+            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/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,52 +4,52 @@ require 'spec_helper'
 
 describe Gitlab::Ci::Parsers::Security::ContainerScanning do
   let(:parser) { described_class.new }
+  let(:project) { artifact.project }
+  let(:pipeline) { artifact.job.pipeline }
+  let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline.sha) }
 
-  let(:clair_vulnerabilities) do
-    JSON.parse!(
-      File.read(
-        Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json')
-      )
-    )['vulnerabilities']
+  before do
+    artifact.each_blob do |blob|
+      parser.parse!(blob, report)
+    end
   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
-      artifact.each_blob do |blob|
-        parser.parse!(blob, report)
-      end
-    end
+    using RSpec::Parameterized::TableSyntax
 
-    it "parses all identifiers and occurrences for unapproved vulnerabilities" do
-      expect(report.occurrences.length).to eq(8)
-      expect(report.identifiers.length).to eq(8)
-      expect(report.scanners.length).to eq(1)
+    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
 
-    it 'generates expected location' do
-      location = report.occurrences.first.location
+    with_them do
+      let(:artifact) { create(:ee_ci_job_artifact, report_type) }
 
-      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',
-        operating_system: 'debian:9',
-        package_name: 'glibc',
-        package_version: '2.24-11+deb9u3'
-      )
-    end
+      it "parses all identifiers and occurrences for unapproved vulnerabilities" do
+        expect(report.occurrences.length).to eq(8)
+        expect(report.identifiers.length).to eq(8)
+        expect(report.scanners.length).to eq(1)
+      end
 
-    it "generates expected metadata_version" do
-      expect(report.occurrences.first.metadata_version).to eq('1.3')
-    end
+      it 'generates expected location' do
+        location = report.occurrences.first.location
 
-    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(location).to be_a(::Gitlab::Ci::Reports::Security::Locations::ContainerScanning)
+        expect(location).to have_attributes(
+          image: image,
+          operating_system: 'debian:9',
+          package_name: 'glibc',
+          package_version: '2.24-11+deb9u3'
+        )
+      end
+
+      it "generates expected metadata_version" do
+        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(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
-  let(:raw_report) do
-    JSON.parse!(
-      File.read(
-        Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json')
-      )
-    )
-  end
-
+describe Gitlab::Ci::Parsers::Security::Formatters::DeprecatedContainerScanning do
   let(:vulnerability) { raw_report['vulnerabilities'].first }
 
   describe '#format' do
-    it 'format ZAP vulnerability into the 1.3 format' do
+    let(:raw_report) do
+      JSON.parse!(
+        File.read(
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-container-scanning-report.json')
+        )
+      )
+    end
+
+    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)