vulnsrc_rhel: one vulnerability by CVE #499

yebinama · 2017-12-19T09:52:01Z

Get one vulnerability by CVE_ID for RHEL instead of one by RHSA_ID so we can have NVD metadata added to the vulnerabilities.

Fixes #495

jzelinskie · 2017-12-20T01:04:09Z

ext/vulnsrc/rhel/rhel.go

-			vulnerabilities = append(vulnerabilities, vulnerability)
+
+			// One vulnerability by CVE
+			for _, reference := range definition.References {


Let's put this logic in the name function.

Also, let's try to do this and if there isn't a Source reference, we can failover to the RHSA value.

The same logic can be applied to links in the link function.

I think that the name function was convenient when there was only one vulnerability to deal with.

This for loop creates one vulnerability for each CVE referenced by the RHSA (ie: (len(references) - 1) vulnerabilities).

Maybe I could iterate on definition.References on line 188 and call name and link with the reference instead of the definition?

Something like:

for _, definition := range ov.Definitions { pkgs := toFeatures(definition.Criteria) if len(pkgs) > 0 { var affected []database.AffectedFeature for _, p := range pkgs { affected = append(affected, p) } for _, reference := range definition.References { if reference.Source == "CVE" { vulnerability := database.VulnerabilityWithAffected{ Vulnerability: database.Vulnerability{ Severity: severity(definition), Description: description(definition), Name: name(reference), Link: link(reference), }, Affected: affected, } vulnerabilities = append(vulnerabilities, vulnerability) } } } }

And then failover to a unique vulnerability with RHSA ID if there are no CVE at all.

That sounds reasonable.

jzelinskie · 2017-12-21T19:25:39Z

ext/vulnsrc/rhel/rhel.go

 			vulnerability := database.VulnerabilityWithAffected{
 				Vulnerability: database.Vulnerability{
-					Name:        name(definition),
-					Link:        link(definition),


Maybe we could leave this code the same, but rename the functions rhsaName(def) and rhsaLink(def).

I agree on that, we can default to RHSA name and link.

jzelinskie · 2017-12-21T19:26:52Z

ext/vulnsrc/rhel/rhel.go

 					Severity:    severity(definition),
 					Description: description(definition),
 				},
 			}
 			for _, p := range pkgs {
 				vulnerability.Affected = append(vulnerability.Affected, p)
 			}
-			vulnerabilities = append(vulnerabilities, vulnerability)


I don't see the reasoning behind removing this line and moving it anywhere else.

I have to move this line because of the new behavior.
I'll try to explain with an example.

These are the references for an RHSA:

> <reference source="RHSA" ref_id="RHSA-2015:1207-00" ref_url="https://rhn.redhat.com/errata/RHSA-2015-1207.html"/> > <reference source="CVE" ref_id="CVE-2015-2722" ref_url="https://access.redhat.com/security/cve/CVE-2015-2722"/> > <reference source="CVE" ref_id="CVE-2015-2724" ref_url="https://access.redhat.com/security/cve/CVE-2015-2724"/> > <reference source="CVE" ref_id="CVE-2015-2725" ref_url="https://access.redhat.com/security/cve/CVE-2015-2725"/>

The actual behavior is to create only one vulnerability and name it "RHSA-2015:1207" which references these 3 CVE.
If we want to name the vulnerabilities with the CVE ID, we now have to create the 3 following vulnerabilities:

CVE-2015-2722

CVE-2015-2724

CVE-2015-2725

That's why I iterate over definition.References and append a new vulnerability for each loop.

jzelinskie · 2017-12-21T19:31:15Z

ext/vulnsrc/rhel/rhel.go

-			vulnerabilities = append(vulnerabilities, vulnerability)
+
+			// Only RHSA is present
+			if len(definition.References) == 1 {


If we default to rhsaName and rhsaLink, we could make this code a little more simple by making it along the lines of

// Prefer the CVE name and link if available. if len(definition.References) > 1 { for _, ref := range definition.References { if ref.Source == "CVE" { vulnerability.Name = ref.ID vulnerability.Link = ref.URI } } }

Let's take the same example as above:

> <reference source="RHSA" ref_id="RHSA-2015:1207-00" ref_url="https://rhn.redhat.com/errata/RHSA-2015-1207.html"/> > <reference source="CVE" ref_id="CVE-2015-2722" ref_url="https://access.redhat.com/security/cve/CVE-2015-2722"/> > <reference source="CVE" ref_id="CVE-2015-2724" ref_url="https://access.redhat.com/security/cve/CVE-2015-2724"/> > <reference source="CVE" ref_id="CVE-2015-2725" ref_url="https://access.redhat.com/security/cve/CVE-2015-2725"/>

If we do that we will only create one vulnerability (CVE-2015-2725) and lose the data for the 2 others CVE.

yebinama · 2017-12-22T10:21:03Z

ext/vulnsrc/rhel/rhel.go

+				vulnerability.Link = definition.References[0].URI
+				vulnerabilities = append(vulnerabilities, vulnerability)
+			} else {
+				for _, reference := range definition.References[1:] {


We don't have to test if (ref.Source == "CVE") since all the references starting from the index 1 are CVE.
We create one vulnerability for each CVE.
As append copies the vulnerability, we can reuse the same object for each loop and only change the name and the link.

yebinama · 2017-12-22T10:22:08Z

ext/vulnsrc/rhel/rhel.go

+
+func rhsaLink(def definition) (link string) {
+	if len(def.References) > 0 {
+		link = def.References[0].URI


The RHSA is always the first reference.

yebinama · 2017-12-22T10:25:45Z

ext/vulnsrc/rhel/rhel.go

+
+			// Only RHSA is present
+			if len(definition.References) == 1 {
+				vulnerabilities = append(vulnerabilities, vulnerability)


Only one reference, it's the RHSA. We append the default vulnerability.

yebinama · 2017-12-22T10:27:02Z

Let's take 3 examples to cover all cases and explain the behavior

Example1:

>   <reference source="RHSA" ref_id="RHSA-2015:1207-00" ref_url="https://rhn.redhat.com/errata/RHSA-2015-1207.html"/>

We will only create one vulnerability:

RHSA-2015-1207

Example2:

>   <reference source="RHSA" ref_id="RHSA-2015:1207-00" ref_url="https://rhn.redhat.com/errata/RHSA-2015-1207.html"/>
>   <reference source="CVE" ref_id="CVE-2015-2722" ref_url="https://access.redhat.com/security/cve/CVE-2015-2722"/>

We'll create one vulnerability:

CVE-2015-2722

Example3:

>   <reference source="RHSA" ref_id="RHSA-2015:1207-00" ref_url="https://rhn.redhat.com/errata/RHSA-2015-1207.html"/>
>   <reference source="CVE" ref_id="CVE-2015-2722" ref_url="https://access.redhat.com/security/cve/CVE-2015-2722"/>
>   <reference source="CVE" ref_id="CVE-2015-2724" ref_url="https://access.redhat.com/security/cve/CVE-2015-2724"/>
>   <reference source="CVE" ref_id="CVE-2015-2725" ref_url="https://access.redhat.com/security/cve/CVE-2015-2725"/>

We'll create 3 vulnerabilities:

jzelinskie · 2018-07-06T01:13:14Z

This change looks good. Can you add a test for the Example 3 case? Thanks a bunch. I'm sorry for the delay and any confusion.

yebinama · 2018-07-24T09:07:40Z

Sure, I'll add a new test to cover all the cases.

yebinama · 2018-07-27T09:55:23Z

I've slightly changed the behavior.
The RHSA's severity is determined by the highest CVE's severity.
Now that we create one vulnerability by CVE, we can't use the RHSA's severity for CVE affected by a lower impact.
If no severity is defined, we use the default one (RHSA), else we use the specific one.

jzelinskie · 2018-09-10T17:13:51Z

Sorry about the delay. If you can rebase this change, I will merge it immediately.

Get one vulnerability by CVE_ID for RHEL instead of one by RHSA_ID so we can have NVD metadata added to the vulnerabilities. Fixes quay#495

If no CVE is present, create a vulnerability with rhsa ID

Code reorganisation

delete a useless line

Add test for multiple CVE

use the specific CVE's impact field instead of the RHSA's one

yebinama · 2018-09-14T22:06:43Z

Done :)

jzelinskie

LGTM! Thank you for your patience. I'm really glad to get this merged.

Get one vulnerability per CVE for Oracle instead of one per RHSA so we can have NVD metadata added to the vulnerabilities. Related: quay#495, quay#499.

Get one vulnerability per CVE for Oracle instead of one per ELSA so we can have NVD metadata added to the vulnerabilities. Related: quay#495, quay#499.

jzelinskie suggested changes Dec 20, 2017

View reviewed changes

jzelinskie suggested changes Dec 21, 2017

View reviewed changes

yebinama commented Dec 22, 2017

View reviewed changes

yebinama added 6 commits September 14, 2018 23:54

vulnsrc_rhel: one vulnerability by CVE

4ab98cf

Get one vulnerability by CVE_ID for RHEL instead of one by RHSA_ID so we can have NVD metadata added to the vulnerabilities. Fixes quay#495

vulnsrc_rhel: rhsa_ID by default

ac86a36

If no CVE is present, create a vulnerability with rhsa ID

vulnsrc_rhel: minor changes

4e4e98f

Code reorganisation

vulnsrc_rhel: minor changes

8b3338e

delete a useless line

vulnsrc_rhel: add test

a90db71

Add test for multiple CVE

vulnsrc_rhel: cve impact

c4ffa0c

use the specific CVE's impact field instead of the RHSA's one

yebinama force-pushed the rhel_CVEID branch from d169adf to c4ffa0c Compare September 14, 2018 22:03

jzelinskie approved these changes Sep 14, 2018

View reviewed changes

jzelinskie merged commit d0a3fe9 into quay:master Sep 14, 2018

glb added a commit to glb/clair that referenced this pull request Nov 2, 2018

vulnsrc_oracle: one vulnerability per CVE

03f7525

Get one vulnerability per CVE for Oracle instead of one per RHSA so we can have NVD metadata added to the vulnerabilities. Related: quay#495, quay#499.

glb mentioned this pull request Nov 2, 2018

vulnsrc_oracle: one vulnerability per CVE #656

Merged

glb added a commit to glb/clair that referenced this pull request Nov 2, 2018

vulnsrc_oracle: one vulnerability per CVE

3503ddb

Get one vulnerability per CVE for Oracle instead of one per ELSA so we can have NVD metadata added to the vulnerabilities. Related: quay#495, quay#499.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vulnsrc_rhel: one vulnerability by CVE #499

vulnsrc_rhel: one vulnerability by CVE #499

yebinama commented Dec 19, 2017

jzelinskie Dec 20, 2017

yebinama Dec 20, 2017

jzelinskie Dec 20, 2017

jzelinskie Dec 21, 2017

yebinama Dec 21, 2017

jzelinskie Dec 21, 2017

yebinama Dec 21, 2017 •

edited

jzelinskie Dec 21, 2017

yebinama Dec 21, 2017

yebinama Dec 22, 2017

yebinama Dec 22, 2017

yebinama Dec 22, 2017

yebinama commented Dec 22, 2017

jzelinskie commented Jul 6, 2018

yebinama commented Jul 24, 2018

yebinama commented Jul 27, 2018

jzelinskie commented Sep 10, 2018

yebinama commented Sep 14, 2018

jzelinskie left a comment

vulnsrc_rhel: one vulnerability by CVE #499

vulnsrc_rhel: one vulnerability by CVE #499

Conversation

yebinama commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yebinama Dec 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yebinama commented Dec 22, 2017

jzelinskie commented Jul 6, 2018

yebinama commented Jul 24, 2018

yebinama commented Jul 27, 2018

jzelinskie commented Sep 10, 2018

yebinama commented Sep 14, 2018

jzelinskie left a comment

Choose a reason for hiding this comment

yebinama Dec 21, 2017 •

edited