Repository: LLNL/Surfactant

Issue:
Enable plugins to hint at important metadata fields
Currently what the interesting bits of metadata that give a good indication of what an SBOM field such as version should be is hard-coded in [`surfactant/cmd/generate.py`](https://github.com/LLNL/Surfactant/blob/main/surfactant/cmd/generate.py#L52-L75).

This can probably be done by adding a [hookspec](https://github.com/LLNL/Surfactant/blob/main/surfactant/plugin/hookspecs.py) that allows a plugin to return hints about what metadata fields can be mapped to specific pieces of key SBOM information. Implementations of the hook for PE and OLE/MSI file metadata should be added. Alternative ideas for how to approach this can be considered.

Discussion:

Solution patch:
diff --git a/surfactant/cmd/generate.py b/surfactant/cmd/generate.py
index be37201..75c7776 100644
--- a/surfactant/cmd/generate.py
+++ b/surfactant/cmd/generate.py
@@ -53,6 +53,7 @@ def get_software_entry(
             sw_entry.containerPath = [re.sub("^" + root_path, container_uuid + "/", filepath)]
     sw_entry.recordedInstitution = user_institution_name
     sw_children: List[Software] = []
+    sw_field_hints: List[Tuple[str, Any, int]] = []
 
     # for unsupported file types, details are just empty; this is the case for archive files (e.g. zip, tar, iso)
     # as well as intel hex or motorola s-rec files
@@ -64,12 +65,13 @@ def get_software_entry(
             filetype=filetype,
             context=context,
             children=sw_children,
+            software_field_hints=sw_field_hints,
             omit_unrecognized_types=omit_unrecognized_types,
         )
         if not skip_extraction
         else []
     )
-    # add metadata extracted from the file, and set SBOM fields if metadata has relevant info
+    # add metadata extracted from the file
     for file_details in extracted_info_results:
         # None as details doesn't add any useful info...
         if file_details is None:
@@ -80,38 +82,36 @@ def get_software_entry(
             sw_entry.metadata = []
         sw_entry.metadata.append(file_details)
 
-        # before checking for keys, make sure the file details object is a dictionary
-        if not isinstance(file_details, Dict):
-            continue
-
-        # common case is Windows PE file has these details under FileInfo, otherwise fallback default value is fine
-        if "FileInfo" in file_details:
-            fi = file_details["FileInfo"]
-            if "ProductName" in fi:
-                sw_entry.name = fi["ProductName"]
-            if "FileVersion" in fi:
-                sw_entry.version = fi["FileVersion"]
-            if "CompanyName" in fi:
-                sw_entry.vendor = [fi["CompanyName"]]
-            if "FileDescription" in fi:
-                sw_entry.description = fi["FileDescription"]
-            if "Comments" in fi:
-                sw_entry.comments = fi["Comments"]
-
-        # less common: OLE file metadata that might be relevant
-        if filetype == "OLE":
-            logger.trace("-----------OLE--------------")
-            if "subject" in file_details["ole"]:
-                sw_entry.name = file_details["ole"]["subject"]
-            if "revision_number" in file_details["ole"]:
-                sw_entry.version = file_details["ole"]["revision_number"]
-            if "author" in file_details["ole"]:
-                # ensure the vendor list has been created
-                if sw_entry.vendor is None:
-                    sw_entry.vendor = []
-                sw_entry.vendor.append(file_details["ole"]["author"])
-            if "comments" in file_details["ole"]:
-                sw_entry.comments = file_details["ole"]["comments"]
+    # set SBOM fields based on sw_field_hints
+    field_confidence: Dict[str, Tuple[Any, int]] = {}
+    for field, value, confidence in sw_field_hints:
+        # special case since vendor can list multiple values
+        if field == "vendor":
+            if field not in field_confidence:
+                field_confidence[field] = ([], 0)
+            field_confidence[field][0].append(value)
+        # otherwise, find the value for each field with the highest confidence
+        elif field not in field_confidence or confidence > field_confidence[field][1]:
+            field_confidence[field] = (value, confidence)
+
+    # set any fields that haven't been set yet (user/previously set fields take precedence)
+    for field, (value, _) in field_confidence.items():
+        if field == "name" and not sw_entry.name:
+            sw_entry.name = value
+        elif field == "version" and not sw_entry.version:
+            sw_entry.version = value
+        elif field == "vendor":
+            # make sure the vendor field is initialized
+            if sw_entry.vendor is None:
+                sw_entry.vendor = []
+            # add any new vendors detected to the list
+            for vendor in value:
+                if vendor not in sw_entry.vendor:
+                    sw_entry.vendor.append(value)
+        elif field == "description" and not sw_entry.description:
+            sw_entry.description = value
+        elif field == "comments" and not sw_entry.comments:
+            sw_entry.comments = value
     return (sw_entry, sw_children)
 
 
diff --git a/surfactant/infoextractors/ole_file.py b/surfactant/infoextractors/ole_file.py
index 3989a90..1cac154 100755
--- a/surfactant/infoextractors/ole_file.py
+++ b/surfactant/infoextractors/ole_file.py
@@ -2,7 +2,7 @@
 # See the top-level LICENSE file for details.
 #
 # SPDX-License-Identifier: MIT
-from typing import Any, Dict
+from typing import Any, Dict, List, Tuple
 
 import olefile
 
@@ -15,10 +15,26 @@ def supports_file(filetype) -> bool:
 
 
 @surfactant.plugin.hookimpl
-def extract_file_info(sbom: SBOM, software: Software, filename: str, filetype: str) -> object:
+def extract_file_info(
+    sbom: SBOM,
+    software: Software,
+    filename: str,
+    filetype: str,
+    software_field_hints: List[Tuple[str, object, int]],
+) -> object:
     if not supports_file(filetype):
         return None
-    return extract_ole_info(filename)
+    ole_info = extract_ole_info(filename)
+    if ole_info and "ole" in ole_info:
+        if "subject" in ole_info["ole"]:
+            software_field_hints.append(("name", ole_info["ole"]["subject"], 80))
+        if "revision_number" in ole_info["ole"]:
+            software_field_hints.append(("version", ole_info["ole"]["revision_number"], 80))
+        if "author" in ole_info["ole"]:
+            software_field_hints.append(("vendor", ole_info["ole"]["author"], 80))
+        if "comments" in ole_info["ole"]:
+            software_field_hints.append(("comments", ole_info["ole"]["comments"], 80))
+    return ole_info
 
 
 def extract_ole_info(filename: str) -> object:
diff --git a/surfactant/infoextractors/pe_file.py b/surfactant/infoextractors/pe_file.py
index bbe5ed9..b5ef0b6 100755
--- a/surfactant/infoextractors/pe_file.py
+++ b/surfactant/infoextractors/pe_file.py
@@ -10,7 +10,7 @@
 
 import pathlib
 import re
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Tuple
 
 import defusedxml.ElementTree
 import dnfile
@@ -25,10 +25,30 @@ def supports_file(filetype) -> bool:
 
 
 @surfactant.plugin.hookimpl
-def extract_file_info(sbom: SBOM, software: Software, filename: str, filetype: str) -> object:
+def extract_file_info(
+    sbom: SBOM,
+    software: Software,
+    filename: str,
+    filetype: str,
+    software_field_hints: List[Tuple[str, object, int]],
+) -> object:
     if not supports_file(filetype):
         return None
-    return extract_pe_info(filename)
+    pe_info = extract_pe_info(filename)
+    if pe_info:
+        if "FileInfo" in pe_info:
+            fi = pe_info["FileInfo"]
+            if "ProductName" in fi:
+                software_field_hints.append(("name", fi["ProductName"], 80))
+            if "FileVersion" in fi:
+                software_field_hints.append(("version", fi["FileVersion"], 80))
+            if "CompanyName" in fi:
+                software_field_hints.append(("vendor", fi["CompanyName"], 80))
+            if "FileDescription" in fi:
+                software_field_hints.append(("description", fi["FileDescription"], 80))
+            if "Comments" in fi:
+                software_field_hints.append(("comments", fi["Comments"], 80))
+    return pe_info
 
 
 # https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#machine-types
diff --git a/surfactant/infoextractors/uimage_file.py b/surfactant/infoextractors/uimage_file.py
index 419e7b9..d90a0ea 100644
--- a/surfactant/infoextractors/uimage_file.py
+++ b/surfactant/infoextractors/uimage_file.py
@@ -8,6 +8,7 @@
 # https://github.com/u-boot/u-boot/blob/master/boot/image.c
 
 import struct
+from typing import List, Tuple
 
 from loguru import logger
 
@@ -222,10 +223,19 @@ def supports_file(filetype) -> bool:
 
 
 @surfactant.plugin.hookimpl
-def extract_file_info(sbom: SBOM, software: Software, filename: str, filetype: str) -> object:
+def extract_file_info(
+    sbom: SBOM,
+    software: Software,
+    filename: str,
+    filetype: str,
+    software_field_hints: List[Tuple[str, object, int]],
+) -> object:
     if not supports_file(filetype):
         return None
     try:
-        return {"uimage_header": _parse_uimage_header(filename)}
+        uimage_header = _parse_uimage_header(filename)
+        if "name" in uimage_header:
+            software_field_hints.append(("name", uimage_header["name"], 40))
+        return {"uimage_header": uimage_header}
     except ValueError as e:
         return None
diff --git a/surfactant/plugin/hookspecs.py b/surfactant/plugin/hookspecs.py
index d8c81df..19e5cb2 100644
--- a/surfactant/plugin/hookspecs.py
+++ b/surfactant/plugin/hookspecs.py
@@ -4,7 +4,7 @@
 # SPDX-License-Identifier: MIT
 
 from queue import Queue
-from typing import List, Optional
+from typing import List, Optional, Tuple
 
 from pluggy import HookspecMarker
 
@@ -37,6 +37,7 @@ def extract_file_info(
     filetype: str,
     context: "Queue[ContextEntry]",
     children: List[Software],
+    software_field_hints: List[Tuple[str, object, int]],
     omit_unrecognized_types: bool,
 ) -> object:
     """Extracts information from the given file to add to the given software entry. Return an
@@ -49,9 +50,18 @@ def extract_file_info(
         software (Software): The software entry the gathered information will be added to.
         filename (str): The full path to the file to extract information from.
         filetype (str): File type information based on magic bytes.
-        context (Queue[ContextEntry]): Modifiable queue of entries from input config file. Existing plugins should still work without adding this parameter.
-        children (List[Software]): List of additional software entries to include in the SBOM. Plugins can add additional entries, though if the plugin extracts files to a temporary directory, the context argument should be used to have Surfactant process the files instead.
-        omit_unrecognized_types (bool): Whether files with types that are not recognized by Surfactant should be left out of the SBOM. When a plugin is adding additional context entries to the queue, it should typically default to propagating this value to the new context entries that it creates.
+        context (Queue[ContextEntry]): Modifiable queue of entries from input config file. Existing plugins should
+            still work without adding this parameter.
+        children (List[Software]): List of additional software entries to include in the SBOM. Plugins can add
+            additional entries, though if the plugin extracts files to a temporary directory, the context argument
+            should be used to have Surfactant process the files instead.
+        software_field_hints (List[tuple[str, str]]): List of tuples containing the name of a software entry field,
+            a suggested value for it, and a confidence level. Plugins can use this information to suggest values for
+            software entry fields by adding entries to this list. The one with the highest confidence level for a
+            field will be selected.
+        omit_unrecognized_types (bool): Whether files with types that are not recognized by Surfactant should be
+            left out of the SBOM. When a plugin is adding additional context entries to the queue, it should typically
+            default to propagating this value to the new context entries that it creates.
 
     Returns:
         object: An object to be added to the metadata field for the software entry. May be `None` to add no metadata.


Test patch:
diff --git a/tests/file_types/test_uimage_files.py b/tests/file_types/test_uimage_files.py
index 7bf4676..5e2fcf2 100644
--- a/tests/file_types/test_uimage_files.py
+++ b/tests/file_types/test_uimage_files.py
@@ -37,7 +37,9 @@ def test_uimage_files():
         file_path = os.path.join(data_dir, file_name)
         file_type = identify_file_type(file_path)
         assert file_type == "UIMAGE"
-        file_info = extract_file_info(SBOM(), Software(), file_path, file_type)
+        sw_field_hints = []
+        file_info = extract_file_info(SBOM(), Software(), file_path, file_type, sw_field_hints)
+        assert sw_field_hints == [("name", "Test uImage", 40)]
         assert "uimage_header" in file_info
         uimage_header = file_info["uimage_header"]
         for key, expected_value in expected_values.items():
@@ -52,5 +54,6 @@ def test_bad_uimage_file():
     file_path = os.path.join(base_path, "..", "data", "uimage_files", "bad1.img")
     file_type = identify_file_type(file_path)
     assert file_type == "UIMAGE"
-    file_info = extract_file_info(SBOM(), Software(), file_path, file_type)
+    sw_field_hints = []
+    file_info = extract_file_info(SBOM(), Software(), file_path, file_type, sw_field_hints)
     assert file_info is None