Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CVSSv3 scores and severities #69

Merged
merged 8 commits into from
Jun 8, 2022
Merged

Conversation

dav3r
Copy link
Member

@dav3r dav3r commented May 5, 2022

🗣 Description

This PR adds support for CVSS v3 scores by importing them from the NVD. It also appropriately sets the severity of each CVE based on the v2 or v3 scoring system.

💭 Motivation and context

Since CVSS v3 has been out for quite a while now, it's about time that we support it.

This PR updates cyhy-nvdsync to import data for any CVE that has either v2 or v3 score. Previously, we only imported the v2 score.

If a v3 score is present in the NVD data, that score is imported into the DB. If no v3 score is present, the v2 score is imported. In either case, the version of the CVSS score is now stored in the DB along with the score itself.

Resolves:

This is part of the work for cisagov/cyhy-system#59.

Marking this PR as blocked until the CyHy team allows us to deploy it. Note that this PR should be deployed in conjunction with cisagov/cyhy-reports#76.

🧪 Testing

To test the updated cyhy-nvdsync code (1ad23ec and d5e722e), I ran it in my test environment and validated the following things:

  • The script ran without error and took the roughly same amount of time as it took before these code changes.
  • Every CVE that was imported received a cvss_version value (either "2.0", "3.0", or "3.1").
  • CVEs were getting assigned the correct severities, i.e. a CVE scored as a 9.0 in CVSS 2.0 got severity 3 (High), while a CVE scored as a 9.0 in CVSS 3.0 or 3.1 got severity 4 (Critical).
  • The same number of CVEs were imported both before (when it only imported CVSSv2 scores) and after these code changes. That isn't a 100% guarantee, but it's a good sign.

To test the updated ticket_manager.py code (2b81487 and 79d29f5), I deployed the code changes to my test environment and re-ran vulnerability scans for some hosts that previously had open tickets. I verified that their tickets were correctly updated with the new expected values for details.cvss_base_score, details.cvss_version, and details.severity. I also confirmed that no tickets were updated with unexpected or erroneous details.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

✅ Post-merge checklist

  • Coordinate deployment plan with CyHy team.
  • Deploy these changes to Production.

dav3r added 3 commits May 5, 2022 16:25
Previously, CVSSv2 severities were assumed.
If a v3 score exists, store it in our database.  If not, store the v2 score.
@dav3r dav3r added blocked This issue or pull request is awaiting the outcome of another issue or pull request improvement This issue or pull request will add new or improve existing functionality labels May 5, 2022
@dav3r dav3r self-assigned this May 5, 2022
@dav3r dav3r requested review from felddy, jsf9k and mcdonnnj as code owners May 5, 2022 21:22
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had one minor suggestions, which you can take or leave. This looks good to me!

bin/cyhy-nvdsync Show resolved Hide resolved
bin/cyhy-nvdsync Outdated Show resolved Hide resolved
dav3r and others added 2 commits May 6, 2022 14:12
@dav3r dav3r requested a review from jsf9k May 10, 2022 20:18
@dav3r dav3r marked this pull request as draft May 11, 2022 13:23
@dav3r dav3r marked this pull request as ready for review May 11, 2022 14:59
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looks great to me!

As of May 2022, some Nessus plugins report a severity that is inconsistent with their (non-NVD, non-CVE-based) CVSS v3 score.  To reduce confusion, we ensure that the severity is correct here.

For examples, see the following plugins: 34460, 104572, 107056, 140770, 156560, 156941, 156441
Copy link
Member

@felddy felddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Had a couple of thoughts about severity to score mapping.

cyhy/db/database.py Show resolved Hide resolved
cyhy/db/database.py Show resolved Hide resolved
Co-authored-by: Mark Feldhousen <felddy@users.noreply.github.com>
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty solid. I had a couple of suggestions that you can ignore if desired. I did have one question I would like answered though just in case I missed/misunderstood logic somewhere.

bin/cyhy-nvdsync Outdated Show resolved Hide resolved
cyhy/db/database.py Show resolved Hide resolved
cyhy/db/ticket_manager.py Show resolved Hide resolved
Co-authored-by: Nick <50747025+mcdonnnj@users.noreply.github.com>
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✔

@dav3r dav3r removed the blocked This issue or pull request is awaiting the outcome of another issue or pull request label Jun 8, 2022
@dav3r dav3r merged commit 073798f into develop Jun 8, 2022
@dav3r dav3r deleted the improvement/support-cvss-v3 branch June 8, 2022 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add new or improve existing functionality
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

4 participants