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

feat: add cvssV4 support #6756

Merged
merged 1 commit into from
Jul 1, 2024
Merged

feat: add cvssV4 support #6756

merged 1 commit into from
Jul 1, 2024

Conversation

jeremylong
Copy link
Owner

Fixes Issue #6746

Description of Change

Add support for CvssV4.

Have test cases been added to cover the new functionality?

no

@boring-cyborg boring-cyborg bot added ant changes to ant cli changes to the cli core changes to core maven changes to the maven plugin tests test cases utils changes to utils labels Jun 30, 2024
@jeremylong
Copy link
Owner Author

@chadlwilson and @aikebah - currently this adds support for getting CvssV4 into the database. The report and a few other things still need to be added. Unfortunately, I won't be able to get to this until tomorrow morning (US eastern).

@chadlwilson
Copy link
Contributor

chadlwilson commented Jun 30, 2024

With all due respect to you as the benevolent dictator and project lead (😛) I am still failing to see why this is urgent and why effort is going into supporting CVSSv4, rather than first simply releasing a 9.2.1 or 9.3.0 version of ODC that at least works without CVSSv4 - which the PR at jeremylong/Open-Vulnerability-Project#165 was sufficient to support on its own, and could have been released with low risk and no breaking changes.

There's no rush for CVSSv4 support is there?

By contrast, there is (arguably) a rush to fix every ODC installation in the world which is currently broken and has no NVD support at all, due to the parsing leniency for metrics (to make CVSSv4 introduction backward compatible) being mis-applied from the earlier change in jeremylong/Open-Vulnerability-Project#158?

@aikebah
Copy link
Collaborator

aikebah commented Jun 30, 2024

@jeremylong tend to agree with @chadlwilson that getting any 'working with NVD API' release out is more important than getting CVSSv4 reported on. By tomorrow morning US time you'll have a swarm of users from Asia and Europe screaming for a fix as their builds are broken.

@chadlwilson
Copy link
Contributor

chadlwilson commented Jun 30, 2024

After thinking about this a little more, I wonder if perhaps the concern about making a change to fix sync while ignoring CVSSv4 metrics is that people's databases or caches would contain versions of the NVD API responses missing data (but with a lastModifiedTime that assumes they include any CVSSv4 additions), and would later need a cache invalidation approach to re-download that data when CVSSv4 support is added, likely involving mass invalidation/re-process to avoid people having incomplete data for certain entries forever that might be hard to track down. ("why doesn't this reported issue show CVSSv4 metrics?")

Such a mass invalidation sounds like it might have happened for the breaking H2 upgrade anyway, I guess, which might make it good time to add CVSSv4 support and mitigate the concern above.

The concern hasn't been expressed that way but it probably would lead to the outcome after CVSSv4 support is added of not knowing which entries need updating to fill in CVSSv4 data with either a forced mass invalidation, or users to do it themselves (depending on their caching strategy).

@jeremylong
Copy link
Owner Author

@chadlwilson - correct, the concern is about incomplete databases. We have several users who utilize a centralized database that won't create a new database when the h2 upgrade happens. I'm okay pushing the release without the reporting - I'll make that happen now.

@jeremylong jeremylong merged commit ad0d16a into main Jul 1, 2024
6 checks passed
@jeremylong jeremylong deleted the scratch/CvssV4 branch July 1, 2024 10:24
@chadlwilson
Copy link
Contributor

I hate to say I told you so.... but there appear a number of bugs in this (including #6761 and #6746 (comment) ), and now the NVD API is down/under load as well (wouldn't be surprised if that's due to all the constantly retrying ODC clients on old versions), and the thundering herd of users has arrived at GitHub. :-(

@jeremylong
Copy link
Owner Author

I know at least one person used the new version successfully - I just accidentally left debug code in: #6746 (comment)

Then the API went down from load.

@jeremylong
Copy link
Owner Author

@chadlwilson really appreciate your assistance with this. Other than the NVD API being down, likely due to load, everything else seems kind of minor and had quick fixes (typo in the postgres SQL and left over debug code).

@michaelmejaeger
Copy link

michaelmejaeger commented Jul 2, 2024

@jeremylong, we are using a centralized database and I wonder now, if I can use version 10 of the Maven plugin for updating the database while having clients which are still using version 9. Does the upgrade imply that all clients also have to be updated in order to be able to work with the new DB schema?

@jeremylong
Copy link
Owner Author

Yes - that should work.

@jeremylong
Copy link
Owner Author

But you will need to update the database schema

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ant changes to ant cli changes to the cli core changes to core maven changes to the maven plugin tests test cases utils changes to utils
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants