Merge ~ebarretto/ubuntu-cve-tracker:new-pkg-cache into ubuntu-cve-tracker:master

[Eduardo Barretto]

This PR moves the OVAL generation for CVE and PKG based OVALs from the old cache mechanism to the new.
The new cache mechanism fixes a few of the issues we currently have, such as:
- long time to generate OVAL data
- situations where source package produces binaries with different versions, e.g. libreoffice.

It also fixes an epoch issue in the PKG-based OVAL and tries to simplify some of the PKG-based OVAL generation.

[David Fernandez Gonzalez (litios)]

LGTM, thanks!

All the tests seem to generate the "same output" and tests pass.
A great increase in generation speed!

review: Approve

diff --git a/scripts/generate-oval b/scripts/generate-oval
index 110d48e..6a80945 100755
--- a/scripts/generate-oval
+++ b/scripts/generate-oval
@@ -31,15 +31,11 @@ import json
 import os
 import re
 import sys
-#from launchpadlib.launchpad import Launchpad
 
 import apt_pkg
-from cve_lib import (kernel_srcs, product_series, load_cve, PRODUCT_UBUNTU, all_releases, eol_releases, devel_release, release_parent, release_name, release_ppa, release_progenitor, needs_oval)
+from cve_lib import (kernel_srcs, product_series, load_cve, PRODUCT_UBUNTU, all_releases, eol_releases, devel_release, release_parent, release_name, release_progenitor, needs_oval)
 from kernel_lib import meta_kernels
 import oval_lib
-import functools
-import lpl_common
-import tempfile
 
 # cope with apt_pkg api changes.
 if 'init_system' in dir(apt_pkg):
@@ -57,18 +53,15 @@ for r in set(all_releases).difference(set(eol_releases)).difference(set([devel_r
 
 default_cves_to_process = ['active/CVE-*', 'retired/CVE-*']
 
-packages_to_ignore = ("-dev", "-doc", "-dbg", "-dbgsym", "-udeb", "-locale-")
-
 debug_level = 0
 
 package_cache = None
 
-cve_cache = {}
-
 def main():
     """ parse command line options and iterate through files to be processed
     """
     global debug_level
+    global supported_releases
 
     # parse command line options
     parser = argparse.ArgumentParser(description='Generate CVE OVAL from '
@@ -90,10 +83,8 @@ def main():
                         '(default is ./)')
     parser.add_argument('--no-progress', action='store_true',
                         help='do not show progress meter')
-    parser.add_argument('--pkg-cache', action='store', default="pkg_cache.json",
+    parser.add_argument('--pkg-cache-dir', action='store', default='./',
                         help='cache location for binary packages')
-    parser.add_argument('--force-cache-reload', action='store_true',
-                        help='force reload of cache file')
     parser.add_argument('-d', '--debug', action='count', default=0,
                         help="report debugging information")
     parser.add_argument('--usn-oval', action='store_true',
@@ -149,25 +140,27 @@ def main():
 
         return
 
-    cve_cache = {}
-    cache = PackageCache(args.pkg_cache, args.force_cache_reload)
+    releases = [args.oval_release] if args.oval_release else supported_releases
+    supported_releases = releases
+    cache = {}
+    for release in releases:
+        cve_cache = {}
+        cache.update({release: get_package_cache(args.pkg_cache_dir, release)})
 
     if args.pkg_oval:
-        releases = [args.oval_release] if args.oval_release else supported_releases
-        for release in releases:
-            if args.oci:
-                generate_oval_package(release, outdir, args.cve_prefix_dir, cache, cve_cache, args.oci, args.no_progress, args.packages, pathnames, args.fixed_only, ociprefix, ocioutdir)
-            else:
-                generate_oval_package(release, outdir, args.cve_prefix_dir, cache, cve_cache, args.oci, args.no_progress, args.packages, pathnames, args.fixed_only)
+        if args.oci:
+            generate_oval_package(outdir, args.cve_prefix_dir, cache, cve_cache, args.oci, args.no_progress, args.packages, pathnames, args.fixed_only, ociprefix, ocioutdir)
+        else:
+            generate_oval_package(outdir, args.cve_prefix_dir, cache, cve_cache, args.oci, args.no_progress, args.packages, pathnames, args.fixed_only)
         return
 
     if args.oci:
-        generate_oval_cve(args.output_dir, args.cve_prefix_dir, cache, args.oci,
-                        args.no_progress, args.packages, pathnames,  ociprefix, ocioutdir)
+        generate_oval_cve(outdir, args.cve_prefix_dir, cache, args.oci,
+                          args.no_progress, args.packages, pathnames,  ociprefix, ocioutdir)
     else:
-        generate_oval_cve(args.output_dir, args.cve_prefix_dir, cache, args.oci,
-                        args.no_progress, args.packages, pathnames)
-
+        generate_oval_cve(outdir, args.cve_prefix_dir, cache, args.oci,
+                          args.no_progress, args.packages, pathnames)
+    return
 
 
 # given a status generated by parse_package_status(), duplicate it for a
@@ -182,10 +175,8 @@ def duplicate_package_status(release, package, original_status, cache, override_
     elif 'fix-version' in original_status:
         copied_status['fix-version'] = original_status['fix-version']
 
-    if 'fix-version' in copied_status and copied_status['fix-version'].isdigit():
-        copied_status['bin-pkgs'] = cache.get_binarypkgs(package, release, version=copied_status['fix-version'])
-    else:
-        copied_status['bin-pkgs'] = cache.get_binarypkgs(package, release)
+    if 'bin-pkgs' in original_status:
+        copied_status['bin-pkgs'] = original_status['bin-pkgs']
 
     return copied_status
 
@@ -355,158 +346,16 @@ def prepend_usn_to_id(usn_database, usn_id):
     if re.search(r'^[0-9]+-[0-9]$', usn_id):
         usn_database[usn_id]['id'] = 'USN-' + usn_id
 
-# Class to contain the binary package cache
-class PackageCache():
-
-    cachefile = None
-    cache_updates = 0
-    releases = dict()
-    unpublished_sources = dict()
-    packages_to_ignore = ("-dev", "-doc", "-dbg", "-dbgsym", "-udeb", "-locale-")
-
-    def __init__(self, cachefile='data_file.json', force_reload=False):
-        self.cachefile = cachefile
-        self.force_reload = force_reload
-
-        # open the local cache if it exists
-        if os.path.exists(self.cachefile):
-            debug('Opening and reading cache file %s' % self.cachefile)
-            with open(self.cachefile, "r") as read_file:
-                self.pkgcache = json.load(read_file)
-        else:
-            self.pkgcache = dict()
-            self.force_reload = True
-
-        # Get launchpad handlers...
-        debug('Setting up launchpad connection...')
-        self.lp = lpl_common.connect(version='devel')
-        #lp = Launchpad.login_anonymously("generate-oval", "production", version='devel')
-        self.ubuntu = self.lp.distributions['ubuntu']
-        self.archive = self.ubuntu.main_archive
-
-        # if debugging is enabled, flush cache every update
-        self.cache_write_frequency = 1 if debug_level > 0 else 100
-
-    def write_cache(self):
-        debug('Writing cache file %s' % self.cachefile)
-        if self.cachefile:
-            # create as separate file and try to do atomic rename so
-            # multiple writers don't corrupt cache.
-            with tempfile.NamedTemporaryFile(mode='w', prefix='oval_pkg_cache-', suffix='.new', dir=os.path.dirname(self.cachefile), delete=False) as write_file:
-                new_cachefile = write_file.name
-                json.dump(self.pkgcache, write_file, indent=2)
-
-            os.rename(new_cachefile, self.cachefile)
-
-    def _has_no_published_source(self, package, release):
-        return (package in self.unpublished_sources
-                and release in self.unpublished_sources[package])
-
-    def _add_no_published_source(self, package, release):
-        if package not in self.unpublished_sources:
-            self.unpublished_sources[package] = [release]
-        else:
-            self.unpublished_sources[package].append(release)
-
-    # lookup source package in launchpad, get latest version
-    def _lookup_latest_source_package(self, source_name, release):
-
-        # cache lp release info
-        if release not in self.releases:
-            # we can have nested parent releases
-            parent = release_parent(release)
-            while release_parent(parent):
-                parent = release_parent(parent)
-            if parent:
-                self.releases[release] = self.ubuntu.getSeries(name_or_version=parent)
-            else:
-                self.releases[release] = self.ubuntu.getSeries(name_or_version=product_series(release)[1])
-
-        ppa = release_ppa(release)
-        if ppa:
-            archive, group, ppa_full_name = lpl_common.get_archive(ppa, self.lp, False, distribution=self.ubuntu)
-            sources = archive.getPublishedSources(exact_match=True, source_name=source_name, distro_series=self.releases[release], status='Published')
-            if len(sources) == 0:
-                # if release is subproject and no version of package was released to it
-                # then copy source from parent release
-                sources = self.archive.getPublishedSources(exact_match=True, source_name=source_name, distro_series=self.releases[release], status='Published')
-        else:
-            sources = self.archive.getPublishedSources(exact_match=True, source_name=source_name, distro_series=self.releases[release], status='Published')
-
-        # some kernels get statuses even when not published in the
-        # archive yet
-        if len(sources) == 0:
-            self._add_no_published_source(source_name, release)
-            return None
-
-        # in python3, filter returns an iterable object, so wrap in list()
-        sources = list(filter(lambda x: x.pocket in ['Release', 'Security', 'Updates'], sources))
-        # some packages are only in proposed, even for non-devel releases
-        if len(sources) == 0:
-            self._add_no_published_source(source_name, release)
-            return None
-
-        source = sorted(sources,
-                        key=functools.cmp_to_key(lambda x, y: apt_pkg.version_compare(x.source_package_version, y.source_package_version)),
-                        reverse=True)[0]
-        debug('Launchpad returned %s %s' % (source.source_package_name, source.source_package_version))
-        return source
-
-    def get_binarypkgs(self, pname, release, version=None):
-        """ return a list of binary packages from the source package """
-
-        # first check local cache
-        #
-        # if the version in the cve tracker is newer than the
-        # version in the cache, we should refresh the cache
-        # if there's no version in the tracker, return the cached entry
-        if pname in self.pkgcache:
-            if (release in self.pkgcache[pname]['Releases'] and
-                (not version or
-                 apt_pkg.version_compare(version, self.pkgcache[pname]['Releases'][release].get('source_version', 0)) <= 0)):
-                return self.pkgcache[pname]['Releases'][release]['binaries']
-
-        debug('Cache miss: %s %s %s' % (pname, release, version))
-
-        # skip lookup if unpublished_sources is empty and
-        # force_reload is False
-        if not self.unpublished_sources and not self.force_reload:
-            if pname not in self.pkgcache or release not in self.pkgcache[pname]['Releases']:
-                return None
-
-        # skip lookup if we've already done a lookup and found no
-        # published source for that release
-        if self._has_no_published_source(pname, release):
-            return None
-
-        # query launchpad if not in local cache
-        source = self._lookup_latest_source_package(pname, release)
-        if not source:
-            return None
-
-        binaries = source.getPublishedBinaries()
-        binlist = []
-        for i in binaries:
-            # skip if we already saw this package
-            if (i.binary_package_name in binlist):
-                continue
-            # for kernel we only want linux images
-            if pname.startswith('linux') and not (i.binary_package_name).startswith('linux-image-'):
-                continue
-            # skip ignored packages, with exception of golang*-dev pkgs
-            if (i.binary_package_name).startswith(('golang-go')) or not any(s in i.binary_package_name for s in self.packages_to_ignore):
-                binlist.append(i.binary_package_name)
-
-        # save current pkgcache to local cache
-        if pname not in self.pkgcache:
-            self.pkgcache[pname] = {'Releases': {}}
-        self.pkgcache[pname]['Releases'][release] = {'binaries': binlist, 'source_version': source.source_package_version}
-
-        self.cache_updates += 1
-        if self.cache_updates % self.cache_write_frequency == 0:
-            self.write_cache()
+# loads cve package cache <release>-pkg-cache.json based given path to it.
+# To get the cache proceed as: $UCT/scripts/fetch-db <release>-pkg-cache.json pkg-cache
+def get_package_cache(pkg_cache_dir, release):
+    data = {}
+    for filename in glob.glob(os.path.join(pkg_cache_dir, release.replace('/', '_') + '-pkg-cache.json')):
+        debug(f"Opening and reading cache file {filename}")
+        with open(filename, 'r') as f:
+            data = json.load(f)
 
-        return binlist
+    return data
 
 # loads usn database.json based given a path to it.
 # To get the database proceed as: $UCT/scripts/fetch-db database.json.bz2
@@ -581,31 +430,31 @@ def generate_oval_usn(outdir, usn, usn_release, cve_dir, usn_db_dir, ociprefix=N
 
     return True
 
-def generate_oval_package(release, outdir, cve_prefix_dir, pkg_cache, cve_cache, oci, no_progress, packages, pathnames, fixed_only, ociprefix='', ocioutdir=None):
-    if not no_progress:
-        print(f'[*] Generating OVAL for packages in release {release}')
-    ov = oval_lib.OvalGeneratorPkg(release, release_name(release), pathnames, packages, not no_progress,pkg_cache=pkg_cache, fixed_only=fixed_only, cve_cache=cve_cache, oval_format='oci' if oci else 'dpkg', outdir=outdir, cve_prefix_dir=cve_prefix_dir, prefix=ociprefix)
-    ov.generate_oval()
-
-    if oci:
-        ov.oval_format = 'dpkg'
+def generate_oval_package(outdir, cve_prefix_dir, pkg_cache, cve_cache, oci, no_progress, packages, pathnames, fixed_only, ociprefix='', ocioutdir=None):
+    for release in supported_releases:
+        if not no_progress:
+            print(f'[*] Generating OVAL for packages in release {release}')
+        ov = oval_lib.OvalGeneratorPkg(release, release_name(release), pathnames, packages, not no_progress, pkg_cache=pkg_cache, fixed_only=fixed_only, cve_cache=cve_cache, oval_format='oci' if oci else 'dpkg', outdir=outdir, cve_prefix_dir=cve_prefix_dir, prefix=ociprefix)
         ov.generate_oval()
 
-    pkg_cache.write_cache()
-    if not no_progress:
-        print(f'[X] Done generating OVAL for packages in release {release}')
+        if oci:
+            ov.oval_format = 'dpkg'
+            ov.generate_oval()
+
+        if not no_progress:
+            print(f'[X] Done generating OVAL for packages in release {release}')
 
 def generate_oval_cve(outdir, cve_prefix_dir, cache, oci, no_progress, packages, pathnames, ociprefix=None, ocioutdir=None):
     ovals = dict()
-    for i in supported_releases:
+    for release in supported_releases:
         # we can have nested parent releases
-        parent = release_progenitor(i)
-        index = '{0}_dpkg'.format(i)
-        ovals[index] = oval_lib.OvalGeneratorCVE(i, release_name(i), parent, warn, outdir, prefix='', oval_format='dpkg')
+        parent = release_progenitor(release)
+        index = '{0}_dpkg'.format(release)
+        ovals[index] = oval_lib.OvalGeneratorCVE(release, release_name(release), parent, warn, outdir, prefix='', oval_format='dpkg')
         ovals[index].add_release_applicability_definition()
         if oci:
-            index = '{0}_oci'.format(i)
-            ovals[index] = oval_lib.OvalGeneratorCVE(i, release_name(i), parent, warn, ocioutdir, prefix=ociprefix, oval_format='oci')
+            index = '{0}_oci'.format(release)
+            ovals[index] = oval_lib.OvalGeneratorCVE(release, release_name(release), parent, warn, ocioutdir, prefix=ociprefix, oval_format='oci')
             ovals[index].add_release_applicability_definition()
 
     # loop through all CVE data files
@@ -636,6 +485,5 @@ def generate_oval_cve(outdir, cve_prefix_dir, cache, oci, no_progress, packages,
     for i in ovals:
         ovals[i].write_to_file()
 
-    cache.write_cache()
 if __name__ == '__main__':
     main()
diff --git a/scripts/oval_lib.py b/scripts/oval_lib.py
index 20f452e..26908f9 100644
--- a/scripts/oval_lib.py
+++ b/scripts/oval_lib.py
@@ -19,6 +19,7 @@
 from __future__ import unicode_literals
 
 from datetime import datetime, timezone
+import apt_pkg
 import io
 import os
 import random
@@ -142,6 +143,49 @@ def generate_cve_tag(cve):
     cve_ref += '>{0}</cve>'.format(cve['Candidate'])
     return cve_ref
 
+def get_latest_version(versions):
+    latest = None
+    for version in versions:
+        if not latest:
+            latest = version
+            continue
+        elif apt_pkg.version_compare(version, latest) > 0:
+            latest = version
+
+    return latest
+
+def get_binarypkgs(cache, source_name, release, version=None):
+    """ return a list of binary packages from the source package version """
+    packages_to_ignore = ("-dev", "-doc", "-dbg", "-dbgsym", "-udeb", "-locale-")
+    version_map = collections.defaultdict(list)
+    rel = release
+    cache_version = version
+
+    if source_name not in cache[release]:
+        # if a source package does not exist in such a release
+        # return None
+        return (None, None)
+    elif version and version not in cache[rel][source_name]:
+        # if version is not in release, then fetch latest source version
+        cache_version = get_latest_version(list(cache[rel][source_name].keys()))
+    elif not version:
+        # if no version is provided, get latest source version
+        version = get_latest_version(list(cache[rel][source_name].keys()))
+        cache_version = version
+    else:
+        cache_version = version
+
+    for binary, bin_data in cache[rel][source_name][cache_version]['binaries'].items():
+        # for kernel we only want linux images
+        if source_name.startswith('linux') and not binary.startswith('linux-image-'):
+            continue
+        # skip ignored packages, with exception of golang*-dev pkgs
+        if binary.startswith(('golang-go')) or \
+                not any(s in binary for s in packages_to_ignore):
+            version_map[bin_data['version']].append(binary)
+
+    return (version, version_map)
+
 class OvalGenerator:
     supported_oval_elements = ('definition', 'test', 'object', 'state', 'variable')
     generator_version = '1.1'
@@ -340,13 +384,13 @@ class OvalGenerator:
         return family_state, state
 
 class CVEPkgRelEntry:
-    def __init__(self, pkg, release, cve, status, note) -> None:
+    def __init__(self, pkg, release, cve, status, note, cache) -> None:
         self.pkg = pkg
         self.cve = cve
         self.orig_status = status
         self.orig_note = note
         self.release = release
-        cve_info = CVEPkgRelEntry.parse_package_status(self.release, pkg.name, status, note, cve.number, None)
+        cve_info = CVEPkgRelEntry.parse_package_status(self.release, pkg.name, status, note, cve.number, cache)
 
         self.note = cve_info['note']
         self.status = cve_info['status']
@@ -369,16 +413,13 @@ class CVEPkgRelEntry:
         code = status_text.lower()
         detail = note.strip('()') if note else None
         status = {}
-        fix_version = ""
+        fix_version = None
 
         if detail and detail[0].isdigit() and len(detail.split(' ')) == 1:
             fix_version = detail
 
         if cache and code != 'dne':
-            if fix_version and code in ['released', 'not-affected']:
-                status['bin-pkgs'] = cache.get_binarypkgs(package, release, version=fix_version)
-            else:
-                status['bin-pkgs'] = cache.get_binarypkgs(package, release)
+            status['source-version'], status['bin-pkgs'] = get_binarypkgs(cache, package, release, version=fix_version)
 
         note_end = " (note: '{0}').".format(detail) if detail else '.'
         if code == 'dne':
@@ -536,7 +577,7 @@ class OvalGeneratorPkg(OvalGenerator):
         for cve in package.cves:
             if self.fixed_only and cve.pkg_rel_entries[package.name].status != 'fixed':
                 continue
-            cve_obj = self._generate_cve_object(cve)
+            cve_obj = self._generate_cve_tag(cve)
             advisory.append(cve_obj)
 
         rights.text = f"Copyright (C) {datetime.now().year} Canonical Ltd."
@@ -573,6 +614,19 @@ class OvalGeneratorPkg(OvalGenerator):
 
         return criteria
 
+    def _generate_subcriteria(self, operator) -> etree.Element:
+        return etree.Element("criteria", attrib={
+            "operator": operator
+        })
+
+    def _generate_criterion_element(self, comment, id) -> etree.Element:
+        criterion = etree.Element("criterion", attrib={
+            "test_ref": f"{self.ns}:tst:{id}",
+            "comment": comment
+        })
+
+        return criterion
+
     # Element generators
     def _generate_reference(self, package) -> etree.Element:
         reference = etree.Element("reference", attrib={
@@ -583,7 +637,7 @@ class OvalGeneratorPkg(OvalGenerator):
 
         return reference
 
-    def _generate_definition_object(self, package) -> None:
+    def _generate_definition_element(self, package) -> None:
         id = f"{self.ns}:def:{self.definition_id}"
         definition = etree.Element("definition")
         definition.set("class", "vulnerability")
@@ -594,9 +648,10 @@ class OvalGeneratorPkg(OvalGenerator):
         criteria = self._generate_criteria()
         definition.append(metadata)
         definition.append(criteria)
+
         return definition
 
-    def _generate_cve_object(self, cve: CVE) -> etree.Element:
+    def _generate_cve_tag(self, cve: CVE) -> etree.Element:
         cve_tag = etree.Element("cve",
             attrib={
                 'href' : f"https://ubuntu.com/security/{cve.number}",
@@ -612,7 +667,8 @@ class OvalGeneratorPkg(OvalGenerator):
                 cve_tag.set('usns', ','.join(cve.usns))
 
         return cve_tag
-    def _generate_var_object(self, comment, id, binaries) -> etree.Element:
+
+    def _generate_var_element(self, comment, id, binaries) -> etree.Element:
         var = etree.Element("constant_variable",
             attrib={
                 'id' : f"{self.ns}:var:{id}",
@@ -627,7 +683,7 @@ class OvalGeneratorPkg(OvalGenerator):
 
         return var
 
-    def _generate_object_object(self, comment, id, var_id) -> etree.Element:
+    def _generate_object_element(self, comment, id, var_id) -> etree.Element:
         if self.oval_format == 'dpkg':
             object = etree.Element("linux-def:dpkginfo_object",
                 attrib={
@@ -662,6 +718,7 @@ class OvalGeneratorPkg(OvalGenerator):
             path.text = '.'
             filename.text = 'manifest'
             instance.text = '1'
+
         return object
 
     def _generate_test_element(self, comment, id, create_state, type, obj_id = None, state_id=None) -> etree.Element:
@@ -694,7 +751,10 @@ class OvalGeneratorPkg(OvalGenerator):
 
         return test
 
-    def _generate_state_object(self, comment, id, version) -> None:
+    def _generate_state_element(self, comment, id, version) -> None:
+        if version.find(':') == -1:
+            version = f"0:{version}"
+
         if self.oval_format == 'dpkg':
             object = etree.Element("linux-def:dpkginfo_state",
                 attrib={
@@ -708,7 +768,7 @@ class OvalGeneratorPkg(OvalGenerator):
                 "operation": "less than"
             })
 
-            version_check.text = f"0:{version}"
+            version_check.text = f"{version}"
         elif self.oval_format == 'oci':
             object = etree.Element("ind-def:textfilecontent54_state",
                 attrib={
@@ -722,20 +782,12 @@ class OvalGeneratorPkg(OvalGenerator):
                 "operation": "less than"
             })
 
-            version_check.text = f"0:{version}"
+            version_check.text = f"{version}"
         else:
             ValueError(f"Format not {self.oval_format} not supported")
 
         return object
 
-    def _generate_criterion_element(self, comment, id) -> etree.Element:
-        criterion = etree.Element("criterion", attrib={
-            "test_ref": f"{self.ns}:tst:{id}",
-            "comment": comment
-        })
-
-        return criterion
-
     # Running kernel element generators
     def _add_running_kernel_checks(self, root_element):
         objects = root_element.find("objects")
@@ -813,11 +865,6 @@ class OvalGeneratorPkg(OvalGenerator):
         return test
 
     # Kernel elements generators
-    def _generate_criteria_kernel(self, operator) -> etree.Element:
-        return etree.Element("criteria", attrib={
-            "operator": operator
-        })
-
     def _generate_kernel_version_object_element(self, id, var_id) -> etree.Element:
         object = etree.Element("ind-def:variable_object",
             attrib={
@@ -926,64 +973,45 @@ class OvalGeneratorPkg(OvalGenerator):
         criterion = self._generate_criterion_element(criterion_note, id)
         self._add_to_criteria(definition, criterion, depth)
 
-    def _generate_vulnerable_elements(self, package, obj_id=None):
+    def _generate_elements(self, package, binaries, pkg_rel_entry, obj_id=None):
+        create_state = False
+        state = None
+        var = None
+        obj = None
         binary_keyword = 'binaries' if len(package.binaries) > 1 else 'binary'
-        test_note = f"Does the '{package.name}' package exist?"
         object_note = f"The '{package.name}' package {binary_keyword}"
+        test_note = ""
 
-        test = self._generate_test_element(test_note, self.definition_id, False, 'pkg', obj_id=obj_id)
-
-        if not obj_id:
-            object = self._generate_object_object(object_note, self.definition_id, self.definition_id)
-
-            bins = package.binaries
+        if self.oval_format == 'oci':
             if is_kernel_binaries(package.binaries):
                 regex = process_kernel_binaries(package.binaries, 'oci')
-                bins = [f'{regex}']
-
-            binaries = []
-            if self.oval_format == 'oci':
+                binaries = [f'^{regex}(?::\w+|)\s+(.*)$']
+            else:
                 variable_values = '(?::\w+|)\s+(.*)$'
-                for binary in bins:
+
+                binaries = []
+                for binary in package.binaries:
                     binaries.append(f'^{binary}{variable_values}')
-            else:
-                binaries = bins
 
-            var = self._generate_var_object(object_note, self.definition_id, binaries)
-        else:
-            object = None
-            var = None
-        return test, object, var
+        if pkg_rel_entry.status == 'vulnerable':
+            test_note = f"Does the '{package.name}' package exist?"
+        elif pkg_rel_entry.status == 'fixed':
+            test_note = f"Does the '{package.name}' package exist and is the version less than '{pkg_rel_entry.fixed_version}'?"
+            state_note = f"The package version is less than '{pkg_rel_entry.fixed_version}'"
 
-    def _generate_fixed_elements(self, package, pkg_rel_entry, obj_id=None):
-        binary_keyword = 'binaries' if len(package.binaries) > 1 else 'binary'
-        test_note = f"Does the '{package.name}' package exist and is the version less than '{pkg_rel_entry.fixed_version}'?"
-        object_note = f"The '{package.name}' package {binary_keyword}"
-        state_note = f"The package version is less than '{pkg_rel_entry.fixed_version}'"
+            state = self._generate_state_element(state_note, self.definition_id, pkg_rel_entry.fixed_version)
+            create_state = True
 
-        test = self._generate_test_element(test_note, self.definition_id, True, 'pkg', obj_id=obj_id)
-        if not obj_id:
-            object = self._generate_object_object(object_note, self.definition_id, self.definition_id)
+        if not obj_id or create_state:
+            obj_id = None
 
-            binaries = package.binaries
-            if self.oval_format == 'oci':
-                if is_kernel_binaries(package.binaries):
-                    regex = process_kernel_binaries(package.binaries, 'oci')
-                    binaries = [f'^{regex}(?::\w+|)\s+(.*)$']
-                else:
-                    variable_values = '(?::\w+|)\s+(.*)$'
+            var = self._generate_var_element(object_note, self.definition_id, binaries)
 
-                    binaries = []
-                    for binary in package.binaries:
-                        binaries.append(f'^{binary}{variable_values}')
+            obj = self._generate_object_element(object_note, self.definition_id, self.definition_id)
 
-            var = self._generate_var_object(object_note, self.definition_id, binaries)
-        else:
-            object = None
-            var = None
-        state = self._generate_state_object(state_note, self.definition_id, pkg_rel_entry.fixed_version)
+        test = self._generate_test_element(test_note, self.definition_id, create_state, 'pkg', obj_id=obj_id)
 
-        return test, object, var, state
+        return test, obj, var, state
 
     def _populate_pkg(self, package, root_element):
         tests = root_element.find("tests")
@@ -993,7 +1021,7 @@ class OvalGeneratorPkg(OvalGenerator):
 
         # Add package definition
         definitions = root_element.find("definitions")
-        definition_element = self._generate_definition_object(package)
+        definition_element = self._generate_definition_element(package)
 
         # Control/cache variables
         one_time_added_id = None
@@ -1001,44 +1029,41 @@ class OvalGeneratorPkg(OvalGenerator):
         binaries_id = None
         cve_added = False
 
+        #criteria = None
+        #if len(package.binaries) > 1:
+        #    criteria = self._generate_subcriteria('AND')
+
         for cve in package.cves:
             pkg_rel_entry = cve.pkg_rel_entries[package.name]
-            if pkg_rel_entry.status == 'vulnerable':
-                cve_added = True
-                if one_time_added_id:
+            for key in sorted(list(package.binaries)):
+                binaries = package.binaries[key]
+                if pkg_rel_entry.fixed_version:
+                    if pkg_rel_entry.fixed_version in fixed_versions:
+                        self._add_criterion(fixed_versions[pkg_rel_entry.fixed_version], pkg_rel_entry, cve, definition_element)
+                        continue
+                    else:
+                        self._add_criterion(self.definition_id, pkg_rel_entry, cve, definition_element)
+                        fixed_versions[pkg_rel_entry.fixed_version] = self.definition_id
+                elif one_time_added_id:
                     self._add_criterion(one_time_added_id, pkg_rel_entry, cve, definition_element)
+                    continue
                 else:
                     self._add_criterion(self.definition_id, pkg_rel_entry, cve, definition_element)
-
-                    test, object, var = self._generate_vulnerable_elements(package, binaries_id)
-                    tests.append(test)
-
-                    if not binaries_id:
-                        objects.append(object)
-                        variables.append(var)
-                        binaries_id = self.definition_id
-
                     one_time_added_id = self.definition_id
-                    self._increase_id(is_definition=False)
-            elif pkg_rel_entry.status == 'fixed':
-                cve_added = True
 
-                if pkg_rel_entry.fixed_version in fixed_versions:
-                    self._add_criterion(fixed_versions[pkg_rel_entry.fixed_version], pkg_rel_entry, cve, definition_element)
-                else:
-                    self._add_criterion(self.definition_id, pkg_rel_entry, cve, definition_element)
+                test, obj, var, state = self._generate_elements(package, binaries, pkg_rel_entry, binaries_id)
 
-                    test, object, var, state = self._generate_fixed_elements(package, pkg_rel_entry, binaries_id)
-                    tests.append(test)
+                if state:
                     states.append(state)
 
-                    if not binaries_id:
-                        objects.append(object)
-                        variables.append(var)
-                        binaries_id = self.definition_id
+                if obj and var:
+                    binaries_id = self.definition_id
+                    variables.append(var)
+                    objects.append(obj)
 
-                    fixed_versions[pkg_rel_entry.fixed_version] = self.definition_id
-                    self._increase_id(is_definition=False)
+                tests.append(test)
+                self._increase_id(is_definition=False)
+                cve_added = True
 
         if cve_added:
             definitions.append(definition_element)
@@ -1048,7 +1073,7 @@ class OvalGeneratorPkg(OvalGenerator):
     def _populate_kernel_pkg(self, package, root_element, running_kernel_id):
         # Add package definition
         definitions = root_element.find("definitions")
-        definition_element = self._generate_definition_object(package)
+        definition_element = self._generate_definition_element(package)
 
         # Control/cache variables
         fixed_versions = {}
@@ -1056,7 +1081,7 @@ class OvalGeneratorPkg(OvalGenerator):
 
         # Generate one-time elements
         kernel_criterion = self._generate_kernel_package_elements(package, root_element, running_kernel_id)
-        criteria = self._generate_criteria_kernel('OR')
+        criteria = self._generate_subcriteria('OR')
 
         self._add_to_criteria(definition_element, kernel_criterion, operator='AND')
         self._add_to_criteria(definition_element, criteria, operator='AND')
@@ -1075,16 +1100,13 @@ class OvalGeneratorPkg(OvalGenerator):
 
     def _add_new_package(self, package_name, cve, release, cve_data, packages) -> None:
         if package_name not in packages:
-            binaries = self.pkg_cache.get_binarypkgs(package_name, release)
-            version = ''
-            if binaries:
-                version = self.pkg_cache.pkgcache[package_name]['Releases'][release]['source_version']
+            version, binaries = get_binarypkgs(self.pkg_cache, package_name, release)
 
             pkg_obj = Package(package_name, release, binaries, version)
             packages[package_name] = pkg_obj
 
         pkg_obj = packages[package_name]
-        cve_pkg_entry = CVEPkgRelEntry(pkg_obj, release, cve, cve_data['pkgs'][package_name][release][0], cve_data['pkgs'][package_name][release][1])
+        cve_pkg_entry = CVEPkgRelEntry(pkg_obj, release, cve, cve_data['pkgs'][package_name][release][0], cve_data['pkgs'][package_name][release][1], self.pkg_cache)
 
         if cve_pkg_entry.status != 'fixed' and self.fixed_only:
             return
@@ -1223,14 +1245,13 @@ class OvalGeneratorCVE:
         test_refs = []
         packages = cve['packages']
         for package in sorted(packages.keys()):
-            releases = packages[package]['Releases']
-            for release in sorted(releases.keys()):
-                if release == self.release:
-                    release_status = releases[release]
-                    if 'bin-pkgs' in release_status and release_status['bin-pkgs']:
+            if self.release in packages[package]['Releases']:
+                release_status = packages[package]['Releases'][self.release]
+                if 'bin-pkgs' in release_status and release_status['bin-pkgs']:
+                    for key in sorted(list(release_status['bin-pkgs'])):
                         pkg = {
                             'name': package,
-                            'binaries': release_status['bin-pkgs'],
+                            'binaries': release_status['bin-pkgs'][key],
                             'status': release_status['status'],
                             'note': release_status['note'],
                             'fix-version': release_status['fix-version'] if 'fix-version' in release_status else '',
@@ -1251,9 +1272,11 @@ class OvalGeneratorCVE:
                             if 'parent' in release_status:
                                 product_description = cve_lib.get_subproject_description(release_status['parent'])
                             else:
-                                product_description = cve_lib.get_subproject_description(release)
+                                product_description = cve_lib.get_subproject_description(self.release)
                             instruction = prepare_instructions(instruction, header['Candidate'], product_description, pkg)
 
+
+
         # if no packages for this release, then we're done
         if not len(test_refs):
             return False