Merge ~sahnaseredini/ubuntu-cve-tracker:amir-dev-oval-pkgcve-imp into ubuntu-cve-tracker:master

[Amir Naseredini]

addressing the issues with regards to SEC-3844 and SEC-3846

this update should fix the unexpected results cause from adding `--expand` as
well as having some performance improvements

[David Fernandez Gonzalez (litios)]

Hey Amir, thanks for approaching this! I found an issue:

'--oval-releases esm-apps/bionic esm-infra/bionic bionic --expand' only generates esm-apps because of 'out_releases = set(out_releases) - parent_releases'. If any parent releases are part of the selected output releases, those will be removed.

If expand is provided, whatever releases the user provided in the CLI should be generated, so you could ignore the 'out_releases = set(out_releases) - parent_releases' part completely.

review: Needs Fixing

[Amir Naseredini (sahnaseredini)]

Thanks a lot David. This new commit should fix the issue.

[David Fernandez Gonzalez (litios)]

Thanks for the fix! But it has an issue, `out_releases = releases` in line 23 will include the parents since line 21 is `releases = set(releases + list(parent_releases))`.

You can get rid of that entire assign since you are already doing that in line 9.

[Amir Naseredini (sahnaseredini)]

Thanks a lot for your review, David. As discussed on MM(https://chat.canonical.com/canonical/pl/fds3bdieefbh9c6x9uic7zxuty), we can ignore this case and consider the current form as what is intended.

[David Fernandez Gonzalez (litios)]

Thanks!

review: Approve

[Eduardo Barretto (ebarretto)]

since you moved the expand logic into generate-oval itself. Do we still need to pass it to oval_lib?

[Amir Naseredini (sahnaseredini)]

I believe we still do, since we still need to decide what should be the output file name.

[Eduardo Barretto (ebarretto)]

we discussed this offline and my suggestion won't work with our proposed scenarios for this PR, therefore we should pass expand to oval_lib.

lgtm, thanks!

review: Approve

[Amir Naseredini (sahnaseredini)]

Thanks a lot for the review Eduardo.

diff --git a/scripts/generate-oval b/scripts/generate-oval
index c555304..cc44ee7 100755
--- a/scripts/generate-oval
+++ b/scripts/generate-oval
@@ -105,16 +105,23 @@ def main():
             if needs_oval(r):
                 releases.append(r)
         
+    out_releases = releases
+
     # for each release we need to get its parent to also
     # load their cache data in order to generate a complete
     # oval for such release
     parent_releases = set()
+
     for release in releases:
         while(release_parent(release)):
             release = release_parent(release)
             parent_releases.add(release)
 
     releases = set(releases + list(parent_releases))
+    if args.expand:
+        out_releases = releases
+    else:
+        out_releases = set(out_releases) - parent_releases
 
     if args.type == 'usn':
             generate_oval_usn(args.output_dir, args.usn_number, releases,
@@ -126,9 +133,9 @@ def main():
             cache.update({release: get_package_cache(args.pkg_cache_dir, release)})
 
         if args.type == 'pkg':
-            generate_oval_package(releases, args.output_dir, args.cves, cache, cve_cache, args.oci, args.no_progress, args.packages, args.fixed_only, args.oci_output_dir, args.expand)
+            generate_oval_package(out_releases, args.output_dir, args.cves, cache, cve_cache, args.oci, args.no_progress, args.packages, args.fixed_only, args.oci_output_dir, args.expand)
         elif args.type == 'cve':
-            generate_oval_cve(releases, args.output_dir, args.cves, cache, cve_cache, args.oci, args.no_progress, args.packages, args.fixed_only, args.oci_output_dir, args.expand)
+            generate_oval_cve(out_releases, args.output_dir, args.cves, cache, cve_cache, args.oci, args.no_progress, args.packages, args.fixed_only, args.oci_output_dir, args.expand)
 
 
 def warn(message):
@@ -225,18 +232,6 @@ def generate_oval_usn(outdir, usn, usn_releases, cves, usn_db_dir, no_progress, 
 
     return True
 
-def filter_releases_for_output(releases, ov):
-    output_releases = []
-    for release in releases:
-        ov._init_ids(release)
-        if ov._eligible_for_output(release):
-            if ov.expand == False and 'esm' in ov.release:
-                output_releases.append(ov.release_codename)
-            else:
-                output_releases.append(release)
-    return output_releases
-
-
 def generate_oval_package(releases, outdir, cves, pkg_cache, cve_cache, oci, no_progress, packages, fixed_only, ocioutdir, expand):
     if not no_progress:
         print('[*] Initiating OVAL Generation for PKG')
@@ -254,10 +249,8 @@ def generate_oval_package(releases, outdir, cves, pkg_cache, cve_cache, oci, no_
         expand=expand
     )
 
-    output_releases = filter_releases_for_output(releases, ov)
-
     if not no_progress:
-        print(f'[*] Generating OVAL PKG for packages in releases {", ".join(output_releases)}')
+        print(f'[*] Generating OVAL PKG for packages in releases {", ".join(releases)}')
 
     ov.generate_oval()
 
@@ -267,7 +260,7 @@ def generate_oval_package(releases, outdir, cves, pkg_cache, cve_cache, oci, no_
         ov.generate_oval()
 
     if not no_progress:
-        print(f'[X] Done generating OVAL PKG for packages in releases {", ".join(output_releases)}')
+        print(f'[X] Done generating OVAL PKG for packages in releases {", ".join(releases)}')
 
 def generate_oval_cve(releases, outdir, cves, pkg_cache, cve_cache, oci, no_progress, packages, fixed_only, ocioutdir, expand):
     if not no_progress:
@@ -286,10 +279,8 @@ def generate_oval_cve(releases, outdir, cves, pkg_cache, cve_cache, oci, no_prog
         expand=expand
     )
 
-    output_releases = filter_releases_for_output(releases, ov)
-
     if not no_progress:
-        print(f'[*] Generating OVAL CVE for packages in releases {", ".join(output_releases)}')
+        print(f'[*] Generating OVAL CVE for packages in releases {", ".join(releases)}')
 
     ov.generate_oval()
 
@@ -299,7 +290,7 @@ def generate_oval_cve(releases, outdir, cves, pkg_cache, cve_cache, oci, no_prog
         ov.generate_oval()
 
     if not no_progress:
-        print(f'[X] Done generating OVAL CVE for packages in releases {", ".join(output_releases)}')
+        print(f'[X] Done generating OVAL CVE for packages in releases {", ".join(releases)}')
 
 if __name__ == '__main__':
     main()
diff --git a/scripts/oval_lib.py b/scripts/oval_lib.py
index f51dbb1..954634a 100644
--- a/scripts/oval_lib.py
+++ b/scripts/oval_lib.py
@@ -829,14 +829,6 @@ class OvalGenerator:
             with open(os.path.join(self.output_dir, self.output_filepath), 'w') as file:
                 file.write(xmlstr)
 
-    def _eligible_for_output(self, release):
-        if self.expand == False:
-            if self.release_codename == release and 'LTS' in cve_lib.release_name(release):
-                return False
-            elif 'esm-infra' in release:
-                return False
-        return True
-
     # Object generators
     def _generate_criteria(self) -> etree.Element:
         criteria = etree.Element("criteria")
@@ -1523,8 +1515,7 @@ class OvalGeneratorPkg(OvalGenerator):
                 else:
                     self._populate_pkg(all_pkgs[pkg], root_element)
 
-            if self._eligible_for_output(release):
-                self._write_oval_xml(xml_tree, root_element)
+            self._write_oval_xml(xml_tree, root_element)
 
 class OvalGeneratorCVE(OvalGenerator):
     def __init__(self, releases, cve_paths, packages, progress, pkg_cache, fixed_only=True, cve_cache=None, outdir='./', oval_format='dpkg', expand=False) -> None:
@@ -1773,8 +1764,7 @@ class OvalGeneratorCVE(OvalGenerator):
                 self._set_definition_id(cve_id=all_cves[cve].number)
                 self._generate_elements_from_cve(all_cves[cve], accepted_releases, root_element, running_kernel_id, pkg_cache, fixed_versions)
 
-            if self._eligible_for_output(release):
-                self._write_oval_xml(xml_tree, root_element)
+            self._write_oval_xml(xml_tree, root_element)
 
 class OvalGeneratorUSNs(OvalGenerator):
     def __init__(self, releases, cve_paths, packages, progress, pkg_cache, usn_database, fixed_only=True, cve_cache=None, outdir='./', oval_format='dpkg') -> None: