Merge ~evancaville/ubuntu-cve-tracker:oval/fix-kernel-pkg-ids into ubuntu-cve-tracker:master

[Evan Caville]

When generating oval data for kernel packages (`_populate_kernel_pkg`), the `_add_test_ref_to_cve_tag` would always add the `self.defintion_id` as the id. This meant every <cve> tag had a `test_ref` that didn't always link to a oval test. Now the code checks whether there is an existing id from `fixed_versions` that would be assigned to the cve. If not, then it can assign the id as `self.definition_id`.

Another issue was handling None values for `pkg_rel_entry.fixed_version`. When adding kernel elements (`_add_kernel_elements`) that had a None value, it would proceed as normal for the first cve. But it would also add an entry to the `fixed_versions` dict with a key of None. This meant that for every CVE after, with a pkg that had None for its `pkg_rel_entry.fixed_version`, the logic for dealing with a fixed_version (`if version in fixed_versions:`) would be triggered. To avoid this, a None check is added before updating the `fixed_version` dict.

[Eduardo Barretto (ebarretto)]

This PR tries to solve the following issue:
on jammy pkg oval search for 22040105109660 and you won't find any item besides a <cve> entry referencing it.

[Evan Caville (evancaville)]

It also tries to solve the following issue:
on jammy pkg oval search for 22040000100010 and you'll see it be used incorrectly in <criterion> entries referencing it.

[Eduardo Barretto (ebarretto)]

a few notes:
1. It seems both ids we mentioned are not showing up anymore in current oval. Can you find any other case like those?
2. I think the proposed PR is actually creating a big amount of data, that all points to the same variable id.
if you diff a current oval with an oval generated with your patch, you will see that previous entries that all had the same criterion id because they all had the same status, now they each have their own ids, which create more tests with different ids, that creates more objects with different ids that at the end point to all the same variable. We should avoid it, as we try to re-use ids where possible to avoid increasing the oval file size.

review: Needs Fixing

[Evan Caville (evancaville)]

using ./scripts/generate-oval --pkg-oval -d --oval-release=jammy --packages linux --output-dir <output_dir> --pkg-cache-dir <cache> to test:

1. 22040000107620 (CVE-2023-3359) should pop up in the current oval as a <cve> tag and <criterion> tag with two different id's, and will now match when run with this patch

2. I have reviewed and yeah, my None check in `_add_kernel_elements` was not needed. It is now removed and things look better.

Will continue to test

[Eduardo Barretto (ebarretto)]

Just some brain dump here on my investigation:

The oval file from Feb 5th, had id 22040000107620 in the CVE tag for CVE-2024-0641, but that id didn't point to anything else in that OVAL file.
That CVE was touched on February 7th, changing the status from `pending (<version>)` to `released (<version>)`. I tried to generate an oval with the status back to `pending (<version>)` but still could not reproduce the issue.

On February 7th, David realized that because of all the infrastructure issues we had the days before, OVAL generation had been stuck for 2 days.

Therefore I think this issue might be a one-off error during generation caused by many different issues that might have happened at the same time. Probably a combination of cache + uct + server that led to this issue.

I will move the status of this PR to Work in Progress as we try to reproduce this in some way.

[David Fernandez Gonzalez (litios)]

Evan's comment is right here:

"When generating oval data for kernel packages (`_populate_kernel_pkg`), the `_add_test_ref_to_cve_tag` would always add the `self.defintion_id` as the id. This meant every <cve> tag had a `test_ref` that didn't always link to an oval test. Now the code checks whether there is an existing id from `fixed_versions` that would be assigned to the cve. If not, then it can assign the id as `self.definition_id`."

The race condition will always be based on the current status of the CVEs. An example:

1. Being version A the fixed version, the first CVE with a fixed status with version A will create the test with self.definition_id, making the cve tag point to the right test.

2. The second fixed CVE with an status with version A will reuse the test from 1. but it will still link the cve tag to self.definition_id, making it point to nowhere.

Each time a CVE is changed, version A may be listed, altering the order of which one is picked first, therefore not making it sure that the failing cve tag today will be the same as tomorrow, but there will be a wrong one for sure if there was one.

Evan fix should be enough to tackle this issue.

[Eduardo Barretto (ebarretto)]

Evan listed some other instances of the same issue and testing with this PR does fix it.
Thanks!

review: Approve

diff --git a/scripts/oval_lib.py b/scripts/oval_lib.py
index b30a987..d5d0c52 100644
--- a/scripts/oval_lib.py
+++ b/scripts/oval_lib.py
@@ -1448,7 +1448,11 @@ class OvalGeneratorPkg(OvalGenerator):
             if pkg_rel_entry.is_not_applicable(): continue
             cve_added = True
 
-            self._add_test_ref_to_cve_tag(self.definition_id, cve, definition_element)
+            # Determine if CVE is linked to existing test_ref
+            test_ref_id =  self.definition_id 
+            if fixed_versions.get(pkg_rel_entry.fixed_version, False):
+                test_ref_id = fixed_versions[pkg_rel_entry.fixed_version]
+            self._add_test_ref_to_cve_tag(test_ref_id, cve, definition_element)
 
             kernel_version_criterion = self._add_kernel_elements(cve, package, pkg_rel_entry.fixed_version, pkg_rel_entry, root_element, running_kernel_id, fixed_versions)
             self._add_to_criteria(definition_element, kernel_version_criterion, depth=3)