From 5ab0ad9fab30b62c229ea3f5c49c3cc1d09a8480 Mon Sep 17 00:00:00 2001 From: Sameeh Jubran Date: Mon, 13 Jul 2026 15:18:47 +0300 Subject: [PATCH] sbom: address advisory/SBOM review findings - gen-advisory: honour explicit per-version status when defaultStatus is "affected", so unaffected/fixed releases are no longer marked vulnerable - gen-advisory: fail loudly when a CVE record has no non-empty English description (CSAF/CycloneDX note text is required, minLength 1) - gen-advisory: note that --cve-id fetches from the CVE Services API - bomsh_verify: scope the object-store shape check to sha1, matching the sha1 gitoid hashing (drop the unreachable sha256-length branch) - Makefile.am: fail `make bomsh` early when python3/pyspdxtools are absent; quote $(ENABLED_LIBZ)/$(ENABLED_LIBOQS); consolidate clean-local so the omnibor/ and advisories/out/ build dirs are removed on clean - tests: cover the defaultStatus fix, the _bucket_for unknown-state hard-fail, and a csaf_validate.mjs runner self-test wired into CI Signed-off-by: Sameeh Jubran --- .github/workflows/advisory.yml | 7 ++++ Makefile.am | 31 +++++++++++++- doc/include.am | 10 ++--- scripts/bomsh_verify.py | 15 +++++-- scripts/gen-advisory | 26 ++++++++++-- scripts/test_csaf_validate.mjs | 77 ++++++++++++++++++++++++++++++++++ scripts/test_gen_advisory.py | 48 +++++++++++++++++++++ 7 files changed, 199 insertions(+), 15 deletions(-) create mode 100644 scripts/test_csaf_validate.mjs diff --git a/.github/workflows/advisory.yml b/.github/workflows/advisory.yml index 8d0981e9b9..b763e1ca81 100644 --- a/.github/workflows/advisory.yml +++ b/.github/workflows/advisory.yml @@ -205,6 +205,13 @@ jobs: --advisory-id wolfSSL-SA-5.9.1 \ --csaf-out /tmp/adv/wolfSSL-SA-5.9.1.csaf.json + - name: csaf_validate.mjs runner self-test (exit-code contract) + # Pins the gate's own logic (summarize + 0/1/2 exit codes) so a + # csaf-validator-lib bump that changed the result shape cannot silently + # turn the conformance gate into a no-op that still exits 0. Passes a + # known-good generated document to also assert the exit-0 path. + run: node scripts/test_csaf_validate.mjs /tmp/adv/CVE-2026-5501.csaf.json + - name: CSAF strict schema + mandatory tests run: node scripts/csaf_validate.mjs /tmp/adv/*.csaf.json diff --git a/Makefile.am b/Makefile.am index acfd531805..5010f771b0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -351,6 +351,18 @@ wolfssl-test-data.stamp: DISTCLEANFILES += wolfssl-test-data.stamp +# Local clean: doc build artefacts plus the generated SBOM/advisory/OmniBOR +# outputs. Consolidated here (automake allows a single clean-local) so the +# OmniBOR graph (omnibor/) and advisory documents (advisories/out/) are cleaned +# alongside the other top-level build products rather than from doc/include.am. +clean-local: + -rm -rf doc/build/ + -rm -rf doc/html/ + -rm -f doc/refman.pdf + -rm -f doc/doxygen_warnings + -rm -rf $(BOMSH_OMNIBORDIR) + -rm -rf $(ADVISORY_OUT_DIR) + # Remove the symlinks created for out-of-tree builds. Gated on # srcdir != builddir so an in-tree build never touches the real source files. distclean-local: @@ -566,8 +578,8 @@ sbom: $(if $(SBOM_DOCUMENT_NAMESPACE),--document-namespace '$(SBOM_DOCUMENT_NAMESPACE)') \ --options-h $(abs_builddir)/wolfssl/options.h \ --lib "$$sbom_lib" \ - --dep-libz $(ENABLED_LIBZ) \ - --dep-liboqs $(ENABLED_LIBOQS) \ + --dep-libz "$(ENABLED_LIBZ)" \ + --dep-liboqs "$(ENABLED_LIBOQS)" \ $(foreach dv,$(SBOM_DEP_VERSIONS),--dep-version '$(dv)') \ --cdx-out $(abs_builddir)/$(SBOM_CDX) \ --spdx-out $(abs_builddir)/$(SBOM_SPDX); \ @@ -704,6 +716,21 @@ bomsh: echo ""; \ exit 1; \ fi + @if test -z "$(PYTHON3)"; then \ + echo ""; \ + echo "ERROR: 'python3' not found in PATH. Cannot generate SBOM."; \ + echo " (make bomsh re-runs make sbom after the traced build.)"; \ + echo ""; \ + exit 1; \ + fi + @if test -z "$(PYSPDXTOOLS)"; then \ + echo ""; \ + echo "ERROR: 'pyspdxtools' not found in PATH. Cannot validate SBOM."; \ + echo " Install: pip install spdx-tools"; \ + echo " (make bomsh re-runs make sbom after the traced build.)"; \ + echo ""; \ + exit 1; \ + fi $(MAKE) clean @printf 'raw_logfile=%s\n' '$(BOMSH_RAWLOG_BASE)' > '$(BOMSH_CONF)' $(BOMTRACE3) -c '$(BOMSH_CONF)' $(MAKE) diff --git a/doc/include.am b/doc/include.am index 473a5b25c3..0d63432f2f 100644 --- a/doc/include.am +++ b/doc/include.am @@ -19,9 +19,7 @@ dox-html: dox: dox-html dox-pdf -clean-local: - -rm -rf doc/build/ - -rm -rf doc/html/ - -rm -f doc/refman.pdf - -rm -f doc/doxygen_warnings - -rm -rf $(BOMSH_OMNIBORDIR) +# NOTE: `clean-local` (doc build artefacts + the OmniBOR/advisory outputs) is +# defined in the top-level Makefile.am. automake permits a single clean-local, +# and the OmniBOR/advisory products belong with the other top-level build +# outputs, so all local-clean removals are consolidated there. diff --git a/scripts/bomsh_verify.py b/scripts/bomsh_verify.py index 30e41b8973..5ca096209b 100644 --- a/scripts/bomsh_verify.py +++ b/scripts/bomsh_verify.py @@ -111,15 +111,22 @@ def _looks_like_blob_path(parts): """True iff `parts` is the canonical `/` shape Git uses for content-addressed blob fanout: exactly two components, the first of which is a 2-char lowercase-hex prefix and the second of - which is the remaining lowercase-hex of a sha1 digest (38 chars) - or sha256 digest (62 chars). Anything else (`info/`, `pack/...`, - deeper nesting) is housekeeping and must NOT be gitoid-checked.""" + which is the remaining 38 lowercase-hex chars of a sha1 digest. + Anything else (`info/`, `pack/...`, deeper nesting) is housekeeping + and must NOT be gitoid-checked. + + sha1-only on purpose: check_object_store_integrity() hashes with + gitoid_sha1() and load_spdx_gitoids() rejects non-sha1 locators, so + the whole verifier is sha1-scoped. Admitting a 62-char (sha256) + digest here would only let the integrity check compare it against an + sha1 hash and always report it corrupt; a real bomsh switch to + sha256 must update the hashing (and this length) in lockstep.""" if len(parts) != 2: return False aa, rest = parts if len(aa) != 2 or not all(c in _HEX_CHARS for c in aa): return False - if len(rest) not in (38, 62): + if len(rest) != 38: return False return all(c in _HEX_CHARS for c in rest) diff --git a/scripts/gen-advisory b/scripts/gen-advisory index 39c242284a..90a71b1127 100755 --- a/scripts/gen-advisory +++ b/scripts/gen-advisory @@ -200,6 +200,9 @@ def load_cve_record(path=None, cve_id=None): return json.load(f) except (OSError, json.JSONDecodeError) as e: sys.exit(f"ERROR: cannot read CVE record {path!r}: {e}") + # Fetched from the CVE Services API (cveawg.mitre.org), the machine- + # readable endpoint MITRE serves the CVE 5.x JSON records from; cve.org is + # the human-facing catalogue for the same data. url = f'https://cveawg.mitre.org/api/cve/{cve_id}' try: with urllib.request.urlopen(url, timeout=30) as r: @@ -261,6 +264,15 @@ def parse_record(record): if d.get('lang', '').lower().startswith('en'): description = d.get('value', '') break + # CSAF 2.0 requires /vulnerabilities[]/notes[]/text and the document + # summary note to be non-empty (schema minLength 1), and gen_csaf emits + # this description verbatim into both. A record with no (non-empty) + # English description would therefore produce a document the strict CSAF + # gate rejects; fail loudly here rather than writing garbage, per this + # tool's fail-rather-than-emit-garbage contract. + if not description.strip(): + sys.exit(f"ERROR: CVE record {cve_id} has no non-empty English " + f"description (needed for the CSAF/CycloneDX note text)") # CSAF 2.0 carries a single `cwe` {id, name}; take the primary one. The # name is resolved from the official CWE catalogue (CWE_NAMES) so it is the @@ -353,7 +365,14 @@ def product_model(adv, ov): product = a['product'] affected_ranges = [] for v in a['versions']: - if v.get('status') == 'affected' or a['default_status'] == 'affected': + # Fall back to defaultStatus only when the entry carries no explicit + # status. Using `or a['default_status'] == 'affected'` here would + # force EVERY entry into the vulnerable bucket when defaultStatus is + # "affected" -- including entries explicitly marked + # status="unaffected" (the CVE-5.x affected-by-default with + # unaffected/fixed exceptions pattern) -- emitting a fixed release + # as a known_affected range, the opposite of the truth. + if v.get('status', a['default_status']) == 'affected': affected_ranges.append({ 'label': _range_label(v), 'cpe': cpe_for(product, '*'), @@ -811,8 +830,9 @@ def main(): help='Path to a CVE JSON 5.x record (repeatable). Overrides ' 'the default --records-dir scan.') p.add_argument('--cve-id', action='append', default=[], - help='Fetch a record from cve.org by id (repeatable). ' - 'Overrides the default --records-dir scan.') + help='Fetch a record by id from the CVE Services API ' + '(cveawg.mitre.org) (repeatable). Overrides the ' + 'default --records-dir scan.') p.add_argument('--records-dir', default=str(DEFAULT_RECORDS_DIR), help='Directory of CVE JSON 5.x records scanned when no ' f'--cve-record/--cve-id is given (default: ' diff --git a/scripts/test_csaf_validate.mjs b/scripts/test_csaf_validate.mjs new file mode 100644 index 0000000000..5470fcf4c4 --- /dev/null +++ b/scripts/test_csaf_validate.mjs @@ -0,0 +1,77 @@ +// Runner self-test for scripts/csaf_validate.mjs. +// +// csaf_validate.mjs is otherwise only exercised end-to-end in +// .github/workflows/advisory.yml against generated documents, so its own +// logic -- summarize() treating `isValid === false || errors.length > 0` as a +// failure, and the exit-code contract (0 all-pass, 1 any-invalid, 2 usage) -- +// has no direct coverage. A regression there (e.g. mis-reading the validator +// result shape after a @secvisogram/csaf-validator-lib bump) would silently +// turn the gate into a no-op that still exits 0. This test pins the contract +// so the gate cannot degrade unnoticed. +// +// Usage: node scripts/test_csaf_validate.mjs [] +// +// The optional argument is a document that passes the gate (e.g. one produced +// by gen-advisory earlier in the CI job); when given it enables the exit-0 +// assertion. Without it, only the usage(2) and invalid(1) contracts run, so +// the test still works offline without generating a document first. +// +// Requires @secvisogram/csaf-validator-lib to be installed (same dependency +// csaf_validate.mjs imports); the advisory.yml csaf-conformance job installs it. + +import { spawnSync } from 'node:child_process' +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' + +const HERE = dirname(fileURLToPath(import.meta.url)) +const RUNNER = join(HERE, 'csaf_validate.mjs') + +function run(args) { + return spawnSync(process.execPath, [RUNNER, ...args], { encoding: 'utf8' }) +} + +let failures = 0 +function check(name, ok) { + if (ok) { + console.log(`ok - ${name}`) + } else { + failures++ + console.error(`FAIL - ${name}`) + } +} + +// 1) usage: no arguments -> exit 2. +check('no args exits 2 (usage)', run([]).status === 2) + +const tmp = mkdtempSync(join(tmpdir(), 'csaf-selftest-')) +try { + // 2) invalid: parseable JSON but non-conformant CSAF -> exit 1. Missing the + // required tracking/publisher/vulnerabilities fields, so the strict 2.0 + // schema test (part of the gate) must reject it. This exercises the + // summarize()/gate path rather than the JSON parse-error path. + const badDoc = join(tmp, 'bad.csaf.json') + writeFileSync(badDoc, JSON.stringify({ + document: { category: 'csaf_security_advisory', csaf_version: '2.0' }, + })) + check('non-conformant document exits 1', run([badDoc]).status === 1) + + // 3) unparseable input -> non-zero (read/parse-failure path). + const junk = join(tmp, 'junk.csaf.json') + writeFileSync(junk, '{ not valid json') + check('unparseable document exits non-zero', run([junk]).status !== 0) + + // 4) valid: a document that passes the gate -> exit 0 (only when provided). + const validDoc = process.argv[2] + if (validDoc) { + check(`valid document exits 0 (${validDoc})`, run([validDoc]).status === 0) + } else { + console.log('skip - valid-document exit-0 check (no valid doc path given)') + } +} finally { + rmSync(tmp, { recursive: true, force: true }) +} + +console.log(failures === 0 ? 'PASS' : `FAILED (${failures})`) +process.exit(failures === 0 ? 0 : 1) diff --git a/scripts/test_gen_advisory.py b/scripts/test_gen_advisory.py index 9a9019fe94..4f329a043b 100644 --- a/scripts/test_gen_advisory.py +++ b/scripts/test_gen_advisory.py @@ -282,6 +282,54 @@ class TestProductModel(unittest.TestCase): # not-affected FIPS with no fix => no_fix_planned, not none_available. self.assertEqual(fips['remediation_category'], 'no_fix_planned') + def test_default_status_affected_honours_explicit_unaffected(self): + # CVE-5.x "affected-by-default with unaffected/fixed exceptions": an + # entry explicitly marked status="unaffected" must NOT be emitted as a + # vulnerable range even when defaultStatus is "affected". Guards the + # `v.get('status', default_status)` fallback (an earlier `or` form + # wrongly marked the fixed release as known_affected). + adv = {'affected': [{ + 'vendor': 'wolfSSL', 'product': 'wolfSSL', + 'default_status': 'affected', + 'versions': [ + {'status': 'affected', 'version': '0', 'lessThan': '5.9.1'}, + {'status': 'unaffected', 'version': '5.9.1'}, + ], + }]} + prods = ga.product_model(adv, {'state': 'exploitable'}) + labels = [r['label'] for r in prods[0]['affected_ranges']] + self.assertEqual(len(labels), 1) + self.assertEqual(labels, ['< 5.9.1']) + self.assertNotIn('5.9.1', labels) + + def test_default_status_affected_covers_unspecified_entries(self): + # An entry with no explicit status DOES fall back to defaultStatus. + adv = {'affected': [{ + 'vendor': 'wolfSSL', 'product': 'wolfSSL', + 'default_status': 'affected', + 'versions': [{'version': '5.8.0'}], + }]} + prods = ga.product_model(adv, {'state': 'exploitable'}) + self.assertEqual(len(prods[0]['affected_ranges']), 1) + + +class TestBucketFor(unittest.TestCase): + def test_known_states_map_to_expected_buckets(self): + self.assertEqual(ga._bucket_for('exploitable'), 'known_affected') + self.assertEqual(ga._bucket_for('not_affected'), 'known_not_affected') + self.assertEqual(ga._bucket_for('in_triage'), 'under_investigation') + + def test_unknown_state_hard_fails(self): + # A deliberately fail-loud sys.exit rather than silently defaulting an + # unrecognized determination to the worst case (known_affected). + with self.assertRaises(SystemExit): + ga._bucket_for('definitely_not_a_real_state') + + def test_product_model_propagates_unknown_state_failure(self): + adv = _adv('CVE-2026-5501.json') + with self.assertRaises(SystemExit): + ga.product_model(adv, {'state': 'definitely_not_a_real_state'}) + class TestHedgeNote(unittest.TestCase): def test_renders_defines_and_default_off(self):