Commit a4273cc0 authored by Tiger Watson's avatar Tiger Watson

Merge branch '343279-ingest-evidence' into 'master'

Add finding evidence ingestion

See merge request gitlab-org/gitlab!75287
parents e08bbf6a 855b613c
......@@ -47,13 +47,14 @@ module Security
report_finding = report_finding_for(security_finding)
return Vulnerabilities::Finding.new unless report_finding
finding_data = report_finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures, :flags)
finding_data = report_finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures, :flags, :evidence)
identifiers = report_finding.identifiers.map do |identifier|
Vulnerabilities::Identifier.new(identifier.to_hash)
end
signatures = report_finding.signatures.map do |signature|
Vulnerabilities::FindingSignature.new(signature.to_hash)
end
evidence = Vulnerabilities::Finding::Evidence.new(data: report_finding.evidence.data) if report_finding.evidence
Vulnerabilities::Finding.new(finding_data).tap do |finding|
finding.location_fingerprint = report_finding.location.fingerprint
......@@ -61,6 +62,7 @@ module Security
finding.project = project
finding.sha = pipeline.sha
finding.scanner = security_finding.scanner
finding.finding_evidence = evidence
if calculate_false_positive?
finding.vulnerability_flags = report_finding.flags.map do |flag|
......
......@@ -82,7 +82,7 @@ module Security
def normalize_report_findings(report_findings, vulnerabilities)
report_findings.map do |report_finding|
finding_hash = report_finding.to_hash
.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures, :flags)
.except(:compare_key, :identifiers, :location, :scanner, :links, :signatures, :flags, :evidence)
finding = Vulnerabilities::Finding.new(finding_hash)
# assigning Vulnerabilities to Findings to enable the computed state
......@@ -100,6 +100,7 @@ module Security
finding.signatures = report_finding.signatures.map do |signature|
Vulnerabilities::FindingSignature.new(signature.to_hash)
end
finding.finding_evidence = Vulnerabilities::Finding::Evidence.new(data: report_finding.evidence.data) if report_finding.evidence
if calculate_false_positive?
finding.vulnerability_flags = report_finding.flags.map do |flag|
......
......@@ -293,6 +293,8 @@ module Vulnerabilities
def evidence
evidence_data = finding_evidence.present? && Feature.enabled?(:read_from_vulnerability_finding_evidence) ? finding_evidence.data : metadata.dig('evidence')
return if evidence_data.nil?
{
summary: evidence_data&.dig('summary'),
request: build_evidence_request(evidence_data&.dig('request')),
......
......@@ -40,11 +40,11 @@ class Vulnerabilities::FindingEntity < Grape::Entity
expose :location
expose :remediations
expose :solution
expose(:evidence) { |model, _| model.evidence[:summary] }
expose(:request, using: Vulnerabilities::RequestEntity) { |model, _| model.evidence[:request] }
expose(:response, using: Vulnerabilities::ResponseEntity) { |model, _| model.evidence[:response] }
expose(:evidence_source) { |model, _| model.evidence[:source] }
expose(:supporting_messages) { |model, _| model.evidence[:supporting_messages]}
expose(:evidence) { |model, _| model.evidence&.dig(:summary) }
expose(:request, using: Vulnerabilities::RequestEntity) { |model, _| model.evidence&.dig(:request) }
expose(:response, using: Vulnerabilities::ResponseEntity) { |model, _| model.evidence&.dig(:response) }
expose(:evidence_source) { |model, _| model.evidence&.dig(:source) }
expose(:supporting_messages) { |model, _| model.evidence&.dig(:supporting_messages)}
expose(:assets) { |model, _| model.assets }
end
......
......@@ -17,6 +17,7 @@ module Security
delegate :scan, to: :security_finding, private: true
delegate :project, to: :scan, private: true
delegate :project_fingerprint, to: :report_finding, private: true
delegate :evidence, to: :report_finding
def initialize(security_finding, report_finding)
@security_finding = security_finding
......
......@@ -16,6 +16,7 @@ module Security
IngestFindingIdentifiers
IngestFindingLinks
IngestFindingSignatures
IngestFindingEvidence
IngestVulnerabilityFlags
IngestIssueLinks
IngestVulnerabilityStatistics
......
# frozen_string_literal: true
module Security
module Ingestion
module Tasks
# Creates new `Vulnerabilities::Finding::Evidence` records.
class IngestFindingEvidence < AbstractTask
include BulkInsertableTask
self.model = Vulnerabilities::Finding::Evidence
self.unique_by = :vulnerability_occurrence_id
private
def attributes
finding_maps.select(&:evidence).map do |finding_map|
{
vulnerability_occurrence_id: finding_map.finding_id,
data: finding_map.evidence.data
}
end
end
end
end
end
end
......@@ -522,6 +522,16 @@ RSpec.describe Security::PipelineVulnerabilitiesFinder do
end
end
context 'when evidence is not provided in the report findings' do
let!(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_sast, project: project) }
it 'does not set the evidences for findings' do
evidences = subject.findings.select(&:sast?).map(&:evidence)
expect(evidences.compact).to be_empty
end
end
def read_fixture(fixture)
Gitlab::Json.parse(File.read(fixture.file.path))
end
......
......@@ -162,7 +162,7 @@
"reason_phrase": "OK",
"status_code": 200
},
"summary": ""
"summary": "Evidence summary"
},
"id": "47308329-7297-4f8f-b98e-1cc33f26fad4",
"identifiers": [
......@@ -186,9 +186,9 @@
],
"location": {
"hostname": "http://pancakes",
"method": "",
"method": "GET",
"param": "",
"path": ""
"path": "/WebGoat/plugins/bootstrap/css/pancakes.css"
},
"message": "Content Security Policy (CSP) Header Not Set",
"scanner": {
......
......@@ -14,17 +14,20 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Dast do
where(:report_format,
:occurrence_count,
:identifier_count,
:evidence_count,
:scanner_count,
:scanned_resources_count,
:last_occurrence_hostname,
:last_occurrence_method_name,
:last_occurrence_path,
:last_occurrence_severity,
:last_occurrence_confidence) do
:dast | 24 | 15 | 1 | 6 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low'
:dast_multiple_sites | 25 | 15 | 1 | 0 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low'
:dast_deprecated_no_spider | 2 | 3 | 1 | 0 | 'http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io' | 'GET' | '/' | 'low' | 'medium'
:dast_deprecated_no_common_fields | 24 | 15 | 1 | 0 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low'
:last_occurrence_confidence,
:last_occurrence_evidence_summary) do
:dast | 24 | 15 | 1 | 1 | 6 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low' | nil
:dast_multiple_sites | 25 | 15 | 1 | 1 | 0 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low' | nil
:dast_deprecated_no_spider | 2 | 3 | 1 | 1 | 0 | 'http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io' | 'GET' | '/' | 'low' | 'medium' | nil
:dast_deprecated_no_common_fields | 24 | 15 | 1 | 1 | 0 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low' | nil
:dast_14_0_2 | 1 | 2 | 1 | 1 | 3 | 'http://pancakes' | 'GET' | '/WebGoat/plugins/bootstrap/css/pancakes.css' | 'medium' | 'high' | "Evidence summary"
end
with_them do
......@@ -51,6 +54,11 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Dast do
)
end
it 'generates expected evidence' do
evidence = report.findings.last.evidence
expect(evidence&.data&.dig('summary')).to eq(last_occurrence_evidence_summary)
end
describe 'occurrence properties' do
where(:attribute, :value) do
:report_type | 'dast'
......
......@@ -13,6 +13,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
let_it_be(:link) { build(:ci_reports_security_link) }
let_it_be(:scanner) { build(:ci_reports_security_scanner) }
let_it_be(:location) { build(:ci_reports_security_locations_sast) }
let_it_be(:evidence) { build(:ci_reports_security_evidence)}
let_it_be(:remediation) { build(:ci_reports_security_remediation) }
let(:flag_1) { build(:ci_reports_security_flag) }
......@@ -27,6 +28,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
flags: [flag_1, flag_2],
remediations: [remediation],
location: location,
evidence: evidence,
metadata_version: 'sast:1.0',
name: 'Cipher with no integrity',
original_data: {},
......@@ -69,6 +71,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
flags: [flag_1, flag_2],
remediations: [remediation],
location: location,
evidence: evidence,
metadata_version: 'sast:1.0',
name: 'Cipher with no integrity',
raw_metadata: '{}',
......@@ -135,6 +138,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
links: occurrence.links,
flags: occurrence.flags,
location: occurrence.location,
evidence: occurrence.evidence,
metadata_version: occurrence.metadata_version,
name: occurrence.name,
project_fingerprint: occurrence.project_fingerprint,
......
......@@ -885,14 +885,7 @@ RSpec.describe Vulnerabilities::Finding do
context 'has no evidence summary when evidence is present, summary is not' do
let(:finding) { create(:vulnerabilities_finding, raw_metadata: { evidence: {} }) }
it do
is_expected.to match a_hash_including(
summary: nil,
source: nil,
supporting_messages: [],
request: nil,
response: nil)
end
it { is_expected.to be_nil }
end
end
......
......@@ -29,6 +29,7 @@ RSpec.describe Security::Ingestion::IngestReportSliceService do
expect(Security::Ingestion::Tasks::IngestFindingIdentifiers).to have_received(:execute).ordered.with(pipeline, finding_maps)
expect(Security::Ingestion::Tasks::IngestFindingLinks).to have_received(:execute).ordered.with(pipeline, finding_maps)
expect(Security::Ingestion::Tasks::IngestFindingSignatures).to have_received(:execute).ordered.with(pipeline, finding_maps)
expect(Security::Ingestion::Tasks::IngestFindingEvidence).to have_received(:execute).ordered.with(pipeline, finding_maps)
expect(Security::Ingestion::Tasks::IngestVulnerabilityFlags).to have_received(:execute).ordered.with(pipeline, finding_maps)
expect(Security::Ingestion::Tasks::IngestIssueLinks).to have_received(:execute).ordered.with(pipeline, finding_maps)
expect(Security::Ingestion::Tasks::IngestVulnerabilityStatistics).to have_received(:execute).ordered.with(pipeline, finding_maps)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Security::Ingestion::Tasks::IngestFindingEvidence do
describe '#execute' do
let(:pipeline) { create(:ci_pipeline) }
let(:finding_evidence) { create(:ci_reports_security_evidence) }
let(:finding_1) { create(:vulnerabilities_finding) }
let(:finding_2) { create(:vulnerabilities_finding) }
let(:report_finding_1) { create(:ci_reports_security_finding, evidence: finding_evidence) }
let(:report_finding_2) { create(:ci_reports_security_finding, evidence: finding_evidence) }
let(:finding_map_1) { create(:finding_map, finding: finding_1, report_finding: report_finding_1) }
let(:finding_map_2) { create(:finding_map, finding: finding_2, report_finding: report_finding_2) }
let(:service_object) { described_class.new(pipeline, [finding_map_1, finding_map_2]) }
subject(:ingest_finding_evidence) { service_object.execute }
it 'creates finding evidence for the new records' do
expect { ingest_finding_evidence }.to change { Vulnerabilities::Finding::Evidence.count }.by(2)
end
end
end
......@@ -99,6 +99,7 @@ module Gitlab
flags = create_flags(data['flags'])
links = create_links(data['links'])
location = create_location(data['location'] || {})
evidence = create_evidence(data['evidence'])
signatures = create_signatures(tracking_data(data))
if @vulnerability_finding_signatures_enabled && !signatures.empty?
......@@ -117,6 +118,7 @@ module Gitlab
name: finding_name(data, identifiers, location),
compare_key: data['cve'] || '',
location: location,
evidence: evidence,
severity: parse_severity_level(data['severity']),
confidence: parse_confidence_level(data['confidence']),
scanner: create_scanner(data['scanner']),
......@@ -253,6 +255,12 @@ module Gitlab
raise NotImplementedError
end
def create_evidence(evidence_data)
return unless evidence_data.is_a?(Hash)
::Gitlab::Ci::Reports::Security::Evidence.new(data: evidence_data)
end
def finding_name(data, identifiers, location)
return data['message'] if data['message'].present?
return data['name'] if data['name'].present?
......
# frozen_string_literal: true
module Gitlab
module Ci
module Reports
module Security
class Evidence
attr_reader :data
def initialize(data:)
@data = data
end
end
end
end
end
end
......@@ -13,6 +13,7 @@ module Gitlab
attr_reader :flags
attr_reader :links
attr_reader :location
attr_reader :evidence
attr_reader :metadata_version
attr_reader :name
attr_reader :old_location
......@@ -33,13 +34,14 @@ module Gitlab
alias_method :cve, :compare_key
def initialize(compare_key:, identifiers:, flags: [], links: [], remediations: [], location:, metadata_version:, name:, original_data:, report_type:, scanner:, scan:, uuid:, confidence: nil, severity: nil, details: {}, signatures: [], project_id: nil, vulnerability_finding_signatures_enabled: false) # rubocop:disable Metrics/ParameterLists
def initialize(compare_key:, identifiers:, flags: [], links: [], remediations: [], location:, evidence:, metadata_version:, name:, original_data:, report_type:, scanner:, scan:, uuid:, confidence: nil, severity: nil, details: {}, signatures: [], project_id: nil, vulnerability_finding_signatures_enabled: false) # rubocop:disable Metrics/ParameterLists
@compare_key = compare_key
@confidence = confidence
@identifiers = identifiers
@flags = flags
@links = links
@location = location
@evidence = evidence
@metadata_version = metadata_version
@name = name
@original_data = original_data
......@@ -65,6 +67,7 @@ module Gitlab
flags
links
location
evidence
metadata_version
name
project_fingerprint
......
# frozen_string_literal: true
FactoryBot.define do
factory :ci_reports_security_evidence, class: '::Gitlab::Ci::Reports::Security::Evidence' do
data do
{
summary: 'Credit card detected',
request: {
headers: [{ name: 'Accept', value: '*/*' }],
method: 'GET',
url: 'http://goat:8080/WebGoat/logout',
body: nil
},
response: {
headers: [{ name: 'Content-Length', value: '0' }],
reason_phrase: 'OK',
status_code: 200,
body: nil
},
source: {
id: 'assert:Response Body Analysis',
name: 'Response Body Analysis',
url: 'htpp://hostname/documentation'
},
supporting_messages: [
{
name: 'Origional',
request: {
headers: [{ name: 'Accept', value: '*/*' }],
method: 'GET',
url: 'http://goat:8080/WebGoat/logout',
body: ''
}
},
{
name: 'Recorded',
request: {
headers: [{ name: 'Accept', value: '*/*' }],
method: 'GET',
url: 'http://goat:8080/WebGoat/logout',
body: ''
},
response: {
headers: [{ name: 'Content-Length', value: '0' }],
reason_phrase: 'OK',
status_code: 200,
body: ''
}
}
]
}
end
skip_create
initialize_with do
::Gitlab::Ci::Reports::Security::Evidence.new(**attributes)
end
end
end
......@@ -6,6 +6,7 @@ FactoryBot.define do
confidence { :medium }
identifiers { Array.new(1) { association(:ci_reports_security_identifier) } }
location factory: :ci_reports_security_locations_sast
evidence factory: :ci_reports_security_evidence
metadata_version { 'sast:1.0' }
name { 'Cipher with no integrity' }
report_type { :sast }
......@@ -25,7 +26,53 @@ FactoryBot.define do
name: "Cipher does not check for integrity first?",
url: "https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first"
}
]
],
evidence: {
summary: 'Credit card detected',
request: {
headers: [{ name: 'Accept', value: '*/*' }],
method: 'GET',
url: 'http://goat:8080/WebGoat/logout',
body: nil
},
response: {
headers: [{ name: 'Content-Length', value: '0' }],
reason_phrase: 'OK',
status_code: 200,
body: nil
},
source: {
id: 'assert:Response Body Analysis',
name: 'Response Body Analysis',
url: 'htpp://hostname/documentation'
},
supporting_messages: [
{
name: 'Origional',
request: {
headers: [{ name: 'Accept', value: '*/*' }],
method: 'GET',
url: 'http://goat:8080/WebGoat/logout',
body: ''
}
},
{
name: 'Recorded',
request: {
headers: [{ name: 'Accept', value: '*/*' }],
method: 'GET',
url: 'http://goat:8080/WebGoat/logout',
body: ''
},
response: {
headers: [{ name: 'Content-Length', value: '0' }],
reason_phrase: 'OK',
status_code: 200,
body: ''
}
}
]
}
}.deep_stringify_keys
end
scanner factory: :ci_reports_security_scanner
......
......@@ -12,6 +12,76 @@
"id": "gemnasium",
"name": "Gemnasium"
},
"evidence": {
"source": {
"id": "assert:CORS - Bad 'Origin' value",
"name": "CORS - Bad 'Origin' value"
},
"summary": "The Origin header was changed to an invalid value of http://peachapisecurity.com and the response contained an Access-Control-Allow-Origin header which included this invalid Origin, indicating that the CORS configuration on the server is overly permissive.\n\n\n",
"request": {
"headers": [
{
"name": "Host",
"value": "127.0.0.1:7777"
}
],
"method": "GET",
"url": "http://127.0.0.1:7777/api/users",
"body": ""
},
"response": {
"headers": [
{
"name": "Server",
"value": "TwistedWeb/20.3.0"
}
],
"reason_phrase": "OK",
"status_code": 200,
"body": "[{\"user_id\":1,\"user\":\"admin\",\"first\":\"Joe\",\"last\":\"Smith\",\"password\":\"Password!\"}]"
},
"supporting_messages": [
{
"name": "Origional",
"request": {
"headers": [
{
"name": "Host",
"value": "127.0.0.1:7777"
}
],
"method": "GET",
"url": "http://127.0.0.1:7777/api/users",
"body": ""
}
},
{
"name": "Recorded",
"request": {
"headers": [
{
"name": "Host",
"value": "127.0.0.1:7777"
}
],
"method": "GET",
"url": "http://127.0.0.1:7777/api/users",
"body": ""
},
"response": {
"headers": [
{
"name": "Server",
"value": "TwistedWeb/20.3.0"
}
],
"reason_phrase": "OK",
"status_code": 200,
"body": "[{\"user_id\":1,\"user\":\"admin\",\"first\":\"Joe\",\"last\":\"Smith\",\"password\":\"Password!\"}]"
}
}
]
},
"location": {},
"identifiers": [
{
......@@ -57,6 +127,76 @@
"id": "gemnasium",
"name": "Gemnasium"
},
"evidence": {
"source": {
"id": "assert:CORS - Bad 'Origin' value",
"name": "CORS - Bad 'Origin' value"
},
"summary": "The Origin header was changed to an invalid value of http://peachapisecurity.com and the response contained an Access-Control-Allow-Origin header which included this invalid Origin, indicating that the CORS configuration on the server is overly permissive.\n\n\n",
"request": {
"headers": [
{
"name": "Host",
"value": "127.0.0.1:7777"
}
],
"method": "GET",
"url": "http://127.0.0.1:7777/api/users",
"body": ""
},
"response": {
"headers": [
{
"name": "Server",
"value": "TwistedWeb/20.3.0"
}
],
"reason_phrase": "OK",
"status_code": 200,
"body": "[{\"user_id\":1,\"user\":\"admin\",\"first\":\"Joe\",\"last\":\"Smith\",\"password\":\"Password!\"}]"
},
"supporting_messages": [
{
"name": "Origional",
"request": {
"headers": [
{
"name": "Host",
"value": "127.0.0.1:7777"
}
],
"method": "GET",
"url": "http://127.0.0.1:7777/api/users",
"body": ""
}
},
{
"name": "Recorded",
"request": {
"headers": [
{
"name": "Host",
"value": "127.0.0.1:7777"
}
],
"method": "GET",
"url": "http://127.0.0.1:7777/api/users",
"body": ""
},
"response": {
"headers": [
{
"name": "Server",
"value": "TwistedWeb/20.3.0"
}
],
"reason_phrase": "OK",
"status_code": 200,
"body": "[{\"user_id\":1,\"user\":\"admin\",\"first\":\"Joe\",\"last\":\"Smith\",\"password\":\"Password!\"}]"
}
}
]
},
"location": {},
"identifiers": [
{
......
......@@ -342,6 +342,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
end
end
describe 'parsing evidence' do
it 'returns evidence object for each finding', :aggregate_failures do
evidences = report.findings.map(&:evidence)
expect(evidences.first.data).not_to be_empty
expect(evidences.first.data["summary"]).to match(/The Origin header was changed/)
expect(evidences.size).to eq(3)
expect(evidences.compact.size).to eq(2)
expect(evidences.first).to be_a(::Gitlab::Ci::Reports::Security::Evidence)
end
end
describe 'setting the uuid' do
let(:finding_uuids) { report.findings.map(&:uuid) }
let(:uuid_1) do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment