search

aquasecurity / vuln-list-update

Apache License 2.0
175 stars 99 forks source link

fix(mariner): use `advisory_id` for definition file names #271

Closed DmitriyLewen closed 6 months ago

DmitriyLewen commented 9 months ago

Description

There are times when CBL-Mariner uses multiple definitions for a single CVE. e.g:

<definition class="vulnerability" id="oval:com.microsoft.cbl-mariner:def:31880" version="1">
      <metadata>
        <title>CVE-2023-5678 affecting package openssl for versions less than 1.1.1k-28</title>
        <affected family="unix">
          <platform>CBL-Mariner</platform>
        </affected>
        <reference ref_id="CVE-2023-5678" ref_url="https://nvd.nist.gov/vuln/detail/CVE-2023-5678" source="CVE"/>
        <patchable>true</patchable>
        <advisory_id>31880-1</advisory_id>
        <severity>Medium</severity>
        <description>CVE-2023-5678 affecting package openssl for versions less than 1.1.1k-28. A patched version of the package is available.</description>
      </metadata>
      <criteria operator="AND">
        <criterion comment="Package openssl is earlier than 1.1.1k-28, affected by CVE-2023-5678" test_ref="oval:com.microsoft.cbl-mariner:tst:31880000"/>
      </criteria>
    </definition>
    <definition class="vulnerability" id="oval:com.microsoft.cbl-mariner:def:31872" version="1">
      <metadata>
        <title>CVE-2023-5678 affecting package edk2 for versions less than 20230301gitf80f052277c8-38</title>
        <affected family="unix">
          <platform>CBL-Mariner</platform>
        </affected>
        <reference ref_id="CVE-2023-5678" ref_url="https://nvd.nist.gov/vuln/detail/CVE-2023-5678" source="CVE"/>
        <patchable>true</patchable>
        <advisory_id>31872-1</advisory_id>
        <severity>Medium</severity>
        <description>CVE-2023-5678 affecting package edk2 for versions less than 20230301gitf80f052277c8-38. A patched version of the package is available.</description>
      </metadata>
      <criteria operator="AND">
        <criterion comment="Package edk2 is earlier than 20230301gitf80f052277c8-38, affected by CVE-2023-5678" test_ref="oval:com.microsoft.cbl-mariner:tst:31872000"/>
      </criteria>
    </definition>

To avoid overwriting - use advisory_id field for file names. See https://github.com/aquasecurity/vuln-list-update/pull/271#issuecomment-2111678641

Related Issues

knqyf263 commented 9 months ago

I'm checking with the Aqua commercial team.

eric-desrochers commented 8 months ago

Any ETA for this PR to be merged in Trivy ?

eric-desrochers commented 8 months ago

@knqyf263 any development based on your check with Aqua commercial team ?

knqyf263 commented 8 months ago

I'll remind them

eric-desrochers commented 7 months ago

I'll remind them

@knqyf263 any update ? This is impacting AKS Image Cleaner in its ability to retire unsecure images for which CVE are found.

knqyf263 commented 7 months ago

I reminded them again, but they seem to be busy. If you're in a hurry, you can update your data as described below. https://github.com/aquasecurity/trivy-db/issues/379#issuecomment-1941447343

knqyf263 commented 6 months ago

@DmitriyLewen Can't we save two files named the advisory id instead of CVE-ID, like /definitions/2023/31872-1.json?

DmitriyLewen commented 6 months ago

hm... i didn't think about this.

I see 2 points:

knqyf263 commented 6 months ago

But advisory_id is custom field - https://github.com/OVALProject/Language/blob/7fa7bba7b48f09decb732d00b2be032a487ff9fc/schemas/oval-definitions-schema.xsd#L242-L276. Therefore, we cannot be 100% sure about this field.

id and version are required. I think we can construct an advisory id from them. https://github.com/OVALProject/Language/blob/7fa7bba7b48f09decb732d00b2be032a487ff9fc/schemas/oval-definitions-schema.xsd#L237-L238

    <definition class="vulnerability" id="oval:com.microsoft.cbl-mariner:def:31880" version="1">

Working with these files will be more difficult. advisory_id means nothing to user. Therefore, to find file for CVE, you need to use search. It's not a big problem, but it's still 1 extra action.

It's no big deal. We already use custom advisory IDs for others. https://github.com/aquasecurity/vuln-list/tree/main/ghsa/go/github.com/aws/aws-sdk-go https://github.com/aquasecurity/vuln-list/tree/main/alma/9/2024

DmitriyLewen commented 6 months ago

hm... looks like your are right. IIUC in some cases mariner uses only latest from version:

    <definition class="vulnerability" id="oval:com.microsoft.cbl-mariner:def:27423" version="2000000001">
...
        <advisory_id>27423-1</advisory_id>

But it doesn't have much impact.

knqyf263 commented 6 months ago

IIUC in some cases mariner uses only latest from version:

What do you mean? 2000000001 is the latest?

DmitriyLewen commented 6 months ago

hm... i missed word 😞 I meant mariner uses only latest number from version version="2000000001" -> 1 version="2000000002" -> 2 `version="2000000000" -> without version

knqyf263 commented 6 months ago

Got it, you mean they use the last digit. Even 27423-2000000001 is not bad since we just need a unique identifier.

DmitriyLewen commented 6 months ago

only cbl-1.0 uses version 2000... cbl-2.0 uses single digit as version (1,2,3, etc.). I think we can reproduce their logic.

knqyf263 commented 6 months ago

If we go with this approach, what changes do we need in trivy-db compared to the current PR?

DmitriyLewen commented 6 months ago

we will only need to update names of test files to match the names in vuln-list.

DmitriyLewen commented 6 months ago

@knqyf263 I updated this PR. Can you take a look?

knqyf263 commented 6 months ago

Even after we merge this PR, will trivy-db not fail?

DmitriyLewen commented 6 months ago

right. I used mariner dir with new file names:

➜  make db-build
./trivy-db build --cache-dir cache --update-interval 6h
2024/05/15 15:51:45 Updating vulnerability database...
2024/05/15 15:51:45 Updating cbl-mariner data...
2024/05/15 15:51:45     Parsing cache/vuln-list/mariner/1.0
2024/05/15 15:51:45     Parsing cache/vuln-list/mariner/2.0

image

knqyf263 commented 6 months ago

Thanks for confirming. I'll merge this PR, then.

knqyf263 commented 6 months ago

Triggered the workflow https://github.com/aquasecurity/vuln-list-update/actions/runs/9095013101