Commit 855b613c authored by Jonathan Schafer's avatar Jonathan Schafer Committed by Tiger Watson

Add ingestion for Finding Evidence

This will add evidence ingestion for both the existing service
and the new ingestion service.

Changelog: added
EE: true
parent 7d9d2aca
......@@ -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