Create SUSE OVAL importer

[johnmhoran]

Implements #1079

[johnmhoran]

@pombredanne @TG1999 The suse_oval importer is ready for your review.

All 10 GH checks passed -- 1 test failed when I ran make test locally but that was due to the new release of Black -- see #1126 and #1127.

[johnmhoran]

@pombredanne @TG1999 Following up on today's jit.si meeting, this PR has been open for a while and awaits your feedback.

This PR is also related to open PR #1127, both of which involve the use of Black and the effect of running make valid Back in early 2023 with make valid, Black changed the line spacing in more than 40 files to enforce a then-new single-line rule -- not a result we want to have to attend to. I have stopped using make valid in my VulnerableCode work since then for that reason, awaiting a reply to the question posed here and in #1127.

[pombredanne]

@johnmhoran Thanks!

1. We need to trim down the test file sizes. They are too big. We should avoid compressing too as binaries do not work too well with Git in general and a development workflow in particular.
2. We run Black using a pinned version now, so the formatting issue should not exist anymore. See Run latest release of Black #1127 (comment)

[pombredanne]

You may want to merge the latest main in your branch too.

[johnmhoran]

Thanks. This has also been awaiting a reply since February -- will attend to when time permits.

[TG1999]

@johnmhoran please rebase your PR

[johnmhoran]

@pombredanne I just updated main and see that while the requirements.txt setting for Black is black==22.3.0, the setup.cfg setting is black>=22.3.0 -- I take it that I should change the latter to black==22.3.0 as well, right?

[johnmhoran]

@TG1999 Thank you for making the change to black in setup.cfg. I've updated main, checked out the branch, updated the branch, merged main (no conflicts), and am about to change list(set(matching_tests)) to sorted(set(matching_tests)) as @pombredanne suggested in our earlier jit.si -- and will run make test to be sure all is good.

[johnmhoran]

@TG1999 In oval_parser.py as @pombredanne suggested on line 94 I changed return list(set(matching_tests)) to return sorted(set(matching_tests)), but when I reran ran make test (did so just before the change and 332 passed, 0 failed), this time 1 test failed.

============================================================================================ short test summary info ============================================================================================
FAILED vulnerabilities/tests/test_data_source.py::test__collect_pkgs - TypeError: '<' not supported between instances of 'OvalTest' and 'OvalTest'
====================================================================== 1 failed, 331 passed, 1 skipped, 1 deselected, 9 warnings in 52.74s ======================================================================
make: *** [Makefile:116: test] Error 1

(venv) Tue Dec 12, 2023 12:34 PM  /home/jmh/dev/nexb/vulnerablecode jmh (1079-create-suse-oval-importer)
$

I haven't worked on this code since January or so, so it may take a while to refresh my recollection on how this all worked....

[johnmhoran]

@TG1999 I see that the test that failed is a test @ziadhany wrote 14 months ago in /vulnerablecode/vulnerabilities/tests/test_data_source.py -- def test__collect_pkgs(). I hate to admit it but I have no idea how to interpret or fix this cryptic failure.

(venv) Tue Dec 12, 2023 12:50 PM  /home/jmh/dev/nexb/vulnerablecode jmh (1079-create-suse-oval-importer)
$ pytest -vvs vulnerabilities/tests/test_data_source.py::test__collect_pkgs
============================================================================================== test session starts ==============================================================================================
platform linux -- Python 3.8.10, pytest-7.1.1, pluggy-1.0.0 -- /home/jmh/dev/nexb/vulnerablecode/venv/bin/python
cachedir: .pytest_cache
django: settings: vulnerablecode.settings (from ini)
rootdir: /home/jmh/dev/nexb/vulnerablecode, configfile: pyproject.toml
plugins: django-4.5.2
collected 1 item

vulnerabilities/tests/test_data_source.py::test__collect_pkgs FAILED

=================================================================================================== FAILURES ====================================================================================================
______________________________________________________________________________________________ test__collect_pkgs _______________________________________________________________________________________________

    def test__collect_pkgs():

        xmls = load_oval_data()

        expected_suse_pkgs = {"cacti-spine", "apache2-mod_perl", "cacti", "apache2-mod_perl-devel"}
        expected_ubuntu_pkgs = {"potrace", "tor"}

        translations = {"less than": "<"}

        found_suse_pkgs = MockOvalImporter()._collect_pkgs(
>           OvalParser(translations, xmls["suse"]).get_data()
        )

vulnerabilities/tests/test_data_source.py:86:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
vulnerabilities/oval_parser.py:39: in get_data
    matching_tests = self.get_tests_of_definition(definition)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <vulnerabilities.oval_parser.OvalParser object at 0x7f7b3368f7c0>, definition = <vulnerabilities.lib_oval.OvalDefinition object at 0x7f7b33832220>

    def get_tests_of_definition(self, definition: OvalDefinition) -> List[OvalTest]:
        """
        returns a list of all valid tests of the passed OvalDefinition
        """

        criteria_refs = []

        for child in definition.element.iter():
            if "test_ref" in child.attrib:
                criteria_refs.append(child.get("test_ref"))

        matching_tests = []
        for ref in criteria_refs:
            oval_test = self.oval_document.getElementByID(ref)
            # All matches will be `rpminfo_test` elements inside the `tests` element.
            # Test for len == 2 because this IDs a pair of nested `object` and `state` elements.
            if len(oval_test.element) == 2:
                _, state = self.get_object_state_of_test(oval_test)
                valid_test = True
                for child in state.element:
                    if child.get("operation") not in self.translations:
                        valid_test = False
                        continue
                    elif (
                        child.get("operation") in self.translations
                        # "debian_evr_string" is used in both Debian and Ubuntu test XML files; SUSE OVAL uses "evr_string".
                        # See also https://github.com/OVALProject/Language/blob/master/docs/oval-common-schema.md
                        and child.get("datatype") in ["evr_string", "debian_evr_string"]
                    ):
                        matching_tests.append(self.oval_document.getElementByID(ref))

        # return list(set(matching_tests))
>       return sorted(set(matching_tests))
E       TypeError: '<' not supported between instances of 'OvalTest' and 'OvalTest'

vulnerabilities/oval_parser.py:95: TypeError
=============================================================================================== warnings summary ================================================================================================
venv/lib/python3.8/site-packages/django/conf/__init__.py:240
  /home/jmh/dev/nexb/vulnerablecode/venv/lib/python3.8/site-packages/django/conf/__init__.py:240: RemovedInDjango50Warning: The USE_L10N setting is deprecated. Starting with Django 5.0, localized formatting of data will always be enabled. For example Django will display numbers and dates using the format of the current locale.
    warnings.warn(USE_L10N_DEPRECATED_MSG, RemovedInDjango50Warning)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================================================ short test summary info ============================================================================================
FAILED vulnerabilities/tests/test_data_source.py::test__collect_pkgs - TypeError: '<' not supported between instances of 'OvalTest' and 'OvalTest'
========================================================================================= 1 failed, 1 warning in 0.69s ==========================================================================================

(venv) Tue Dec 12, 2023 12:51 PM  /home/jmh/dev/nexb/vulnerablecode jmh (1079-create-suse-oval-importer)
$

[johnmhoran]

@TG1999 Is it possible that the change @pombredanne asked me to make is not a change we actually want? Or perhaps I misinterpreted his suggestion? The change is inside def get_tests_of_definition(self, definition: OvalDefinition) -> List[OvalTest] in oval_parser.py, which @sbs2001 wrote 4 years ago and I updated early this year.

        # return list(set(matching_tests))
        return sorted(set(matching_tests))

In any event, I'm unable so far to figure out what this means, how my change led to the failure, or how to fix:

TypeError: '<' not supported between instances of 'OvalTest' and 'OvalTest'

[johnmhoran]

@pombredanne @TG1999 Been digging into the wonderful world of OVAL a bit, refreshing my recollection etc. I'm wondering if the OvalTest instances mentioned in the failure message above are the <definition> elements in an OVAL .xml file, for example, our suse_oval_data.xml test file.

If so, perhaps the error message is simply informing us that one cannot carry out a < comparison of two such entities? And thus changing list(set(matching_tests)) to sorted(set(matching_tests)) asks Python to execute a comparison that cannot be executed given the nature of the objects being compared?

FWIW, the failing test test__collect_pkgs() expected result (in this test file) is expected_suse_pkgs = {"cacti-spine", "apache2-mod_perl", "cacti", "apache2-mod_perl-devel"}, and each of those four values can be found nested in the <objects> element at the bottom of that suse_oval_data.xml test file:

<objects>
        <rpminfo_object id="oval:org.opensuse.security:obj:2009031246" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux">
            <name>openSUSE-release</name>
        </rpminfo_object>

        <rpminfo_object id="oval:org.opensuse.security:obj:2009031297" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux">
            <name>cacti</name>
        </rpminfo_object>

        <rpminfo_object id="oval:org.opensuse.security:obj:2009037882" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux">
            <name>cacti-spine</name>
        </rpminfo_object>

        <rpminfo_object id="oval:org.opensuse.security:obj:2009040947" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux">
            <name>apache2-mod_perl</name>
        </rpminfo_object>

        <rpminfo_object id="oval:org.opensuse.security:obj:2009041273" version="1" xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux">
            <name>apache2-mod_perl-devel</name>
        </rpminfo_object>
    </objects>

[johnmhoran]

@pombredanne @TG1999 TL;DR imho the failing test can and should be resolved by reversing the change I made yesterday from return list(set(matching_tests)) to return sorted(set(matching_tests)) -- we're attempting to sort OvalTest objects, which by their nature cannot be sorted. What do you think?

[johnmhoran]

I've just committed and pushed and you can see the failing test error here, for example: https://github.com/nexB/vulnerablecode/actions/runs/7199601614/job/19611624377?pr=1085

[johnmhoran]

Looks like there's also one commit with a missing signoff:

Commit sha: 52e2ec1, Author: Tushar Goel, Committer: GitHub; The sign-off is missing.

[TG1999]

@johnmhoran we have to add comparators to the class to allow sorting of OVAL tests

[johnmhoran]

@TG1999 Thanks for your comment -- let's discuss this via a huddle. I've reviewed a number of related files but it is not clear how the concept of versions is involved in these OvalTest() objects or how to implement your suggestion of adding comparators to enable sorting of OvalTest() objects.

The Oval ecosystem bears no recognizable similarity to the VersionConstraint() objects -- with their comparator attributes -- that I worked on roughly 12 months ago in apache_tomcat.py, apache_httpd.py, test_apache_tomcat.py and test_apache_httpd.py -- all of which included the concept of version ranges (not present with the OvalTest() class). The OvalTest() class, in contrast, comprises this simple bit of code created 4 years ago in lib_oval.py:

class OvalTest(OvalElement):
    def __init__(self, element):
        # self.element = element.getElement()
        self.element = element

    def getType(self):
        return OvalElement.TEST

There is no mention of OvalTest() versions there or, as far as I can understand, anywhere else in lib_oval.py. (That file does refer to schema versions, but these appear unrelated to the version references in the .xml test data <test> sections, discussed briefly below.)

If I look at the test that failed when I changed the return in the OvalParser class' get_tests_of_definition() function from return list(set(matching_tests)) to return sorted(set(matching_tests)), that test -- test__collect_pkgs() in test_data_source.py -- defines a variable translations = {"less than": "<"} , but it's not clear to me how that variable is used, or what OvalTest versions are being sorted and causing the failure. Sample use in test__collect_pkgs():

found_suse_pkgs = MockOvalImporter()._collect_pkgs(
        OvalParser(translations, xmls["suse"]).get_data()
    )

There are three Oval-related test .xml data files (debian_oval_data.xml, suse_oval_data,xml and ubuntu_oval_data.xml), and each <tests> section includes nested sections each of which themselves includes version="1". But what these refer to, and whether those are the versions involved in the now-failing test, is elusive.

When time permits, let's discuss how the version-related bits of the OvalTest() structure and testing work together.

[johnmhoran]

@TG1999 I've modified the __lt__ method of the OvalElement class (inherited inter alia by the OvalTest class) to handle the sorting-by-version we need (including converting strings to ints for correct sorting), and have added a test as well. Running make test, no tests fail: 333 passed, 1 skipped, 1 deselected, 9 warnings in 51.41s

In addition to any other comments you or @pombredanne might have, the only remaining item is that one of your commits is missing your signoff -- https://github.com/nexB/vulnerablecode/pull/1085/checks?check_run_id=19931480766. I'll leave that for you to fix.

[johnmhoran]

@TG1999 I saw that you did some more merges into main since I updated my PR on Saturday, and this created merge conflicts which GH alerted me to. I have just finished doing a git pull of main, merging main into my local branch of this PR, and resolving merge conflicts in importer.py and test_data_source.py. These looked straightforward to me.

However, after committing but before pushing, I ran make test to be sure all was OK, and I got a huge number of error messages that I cannot decipher. The error output begins like this:

(venv) Tue Dec 26, 2023 03:58 PM  /home/jmh/dev/nexb/vulnerablecode jmh (1079-create-suse-oval-importer)
$ make test
-> Run the test suite
. venv/bin/activate; python3 -m pytest -vvs -m "not webtest"
============================================================================================== test session starts ==============================================================================================
platform linux -- Python 3.8.10, pytest-7.1.1, pluggy-1.0.0 -- /home/jmh/dev/nexb/vulnerablecode/venv/bin/python3
cachedir: .pytest_cache
django: settings: vulnerablecode.settings (from ini)
rootdir: /home/jmh/dev/nexb/vulnerablecode, configfile: pyproject.toml
plugins: django-4.5.2
collected 239 items / 54 errors / 1 deselected / 238 selected

==================================================================================================== ERRORS =====================================================================================================
_______________________________________________________________________________ ERROR collecting vulnerabilities/import_runner.py _______________________________________________________________________________
vulnerabilities/import_runner.py:23: in <module>
    from vulnerabilities.improvers.default import DefaultImporter
venv/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:168: in exec_module
    exec(co, module.__dict__)
vulnerabilities/improvers/__init__.py:10: in <module>
    from vulnerabilities.improvers import valid_versions
venv/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:168: in exec_module
    exec(co, module.__dict__)
vulnerabilities/improvers/valid_versions.py:20: in <module>
    from fetchcode import package_versions
E   ImportError: cannot import name 'package_versions' from 'fetchcode' (/home/jmh/dev/nexb/vulnerablecode/venv/lib/python3.8/site-packages/fetchcode/__init__.py)
_______________________________________________________________________________ ERROR collecting vulnerabilities/import_runner.py _______________________________________________________________________________
ImportError while importing test module '/home/jmh/dev/nexb/vulnerablecode/vulnerabilities/import_runner.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
venv/lib/python3.8/site-packages/_pytest/python.py:608: in _importtestmodule
    mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
venv/lib/python3.8/site-packages/_pytest/pathlib.py:533: in import_path

. . .

[johnmhoran]

@TG1999 I am going to try to push my merge commit so you can see the current merged code. At this point I have no idea what the underlying cause is.

[johnmhoran]

Just 1 test failure on the GH side: test_suse_oval_importer_CVE_2008_5679 (inside test_suse_oval.py), a test a wrote 11 months ago when I initially opened this ancient PR. I'll give this a look locally.

[johnmhoran]

@TG1999 I fixed 1 test locally (add the url field to an expected JSON test file, per one of your recent changes), ran make dev (that's what I was missing before), and no more errors or failed tests:

 333 passed, 1 skipped, 1 deselected, 12 warnings in 70.96s (0:01:10)

I've committed and pushed, and all GH checks passed except for an earlier commit of yours which is missing the signoff. https://github.com/nexB/vulnerablecode/pull/1085/checks?check_run_id=19969783903

[pombredanne]

LGTM, let's merge this!