feat: add make sbom / install-sbom / uninstall-sbom targets#258
feat: add make sbom / install-sbom / uninstall-sbom targets#258MarkAtwood wants to merge 5 commits into
Conversation
Adds CycloneDX + SPDX SBOM generation via wolfssl's gen-sbom script. Usage: make sbom WOLFSSL_DIR=/path/to/wolfssl wolfCLU is a binary (not .so); artifact hash uses --srcs from wolfssl_SOURCES. Version from CLUWOLFSSL_VERSION_STRING in version.h.
There was a problem hiding this comment.
Pull request overview
Adds SBOM (CycloneDX + SPDX) generation and installation/uninstallation targets to the autotools build, along with configure-time discovery of required tooling, to support compliance/evidence needs.
Changes:
- Adds
make sbom,make install-sbom,make uninstall-sbomtargets that generate SBOM artifacts and install/remove them under$(datadir)/doc/wolfclu/. - Introduces
configure.acchecks forpython3andpyspdxtoolsintended to support the SBOM generation flow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Makefile.am | Adds SBOM targets and wiring for generation + install/uninstall. |
| configure.ac | Adds configure-time tool detection for SBOM prerequisites. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
automake runs with -Werror; a second .PHONY (unconditional) overlapping the existing conditional .PHONY: manpages-gz triggered 'already defined' -> automake failed -> all CI configs red at autoreconf. Consolidate to one unconditional .PHONY covering manpages-gz + the sbom targets.
Signed-off-by: Sameeh Jubran <sameeh@wolfssl.com>
Signed-off-by: Sameeh Jubran <sameeh@wolfssl.com>
jackctj117
left a comment
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: review + review-securityOverall recommendation: COMMENT
Findings: 6 total — 6 posted, 0 skipped
6 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] [review+review-security] SBOM build-options capture misses wolfCLU's config.h and AM_CFLAGS feature defines —
scripts/sbom.am:129-131 - [Medium] [review] PR description (--srcs / no staging) does not match implementation (--lib on staged binary) —
scripts/sbom.am:106-183 - [Low] [review] AC_PATH_PROG([PYTHON3]) duplicates AM_PATH_PYTHON and requires an executable literally named 'python3' —
configure.ac:65 - [Low] [review] Reproducibility CI step excludes the .spdx tag-value output from the byte-identical check —
.github/workflows/sbom.yml:121-133 - [Low] [review+review-security] pull_request branch filter '*' won't match base branches containing a slash —
.github/workflows/sbom.yml:4-7 - [Info] [review-security] CI builds and executes code from an unmerged external PR ref by default —
.github/workflows/sbom.yml:40-59
Review generated by Skoll
| exit 1; \ | ||
| fi; \ | ||
| echo "SBOM: hashing $$sbom_art"; \ | ||
| $(CC) -dM -E $(DEFAULT_INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) \ |
There was a problem hiding this comment.
🟠 [Medium] SBOM build-options capture misses wolfCLU's config.h and AM_CFLAGS feature defines · Logic
The shared recipe captures the configured build macros with $(CC) -dM -E $(DEFAULT_INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(if $(wildcard $(abs_builddir)/config.h),-include $(abs_builddir)/config.h) -x c /dev/null, but for wolfCLU this captures almost none of the real configuration. (1) configure.ac declares AC_CONFIG_HEADERS([src/config.h]), so the generated header is src/config.h, NOT $(abs_builddir)/config.h; the $(wildcard $(abs_builddir)/config.h) therefore never matches and -include is dropped, so config.h is never force-included. (2) wolfCLU's actual feature toggles are added to AM_CFLAGS in configure.ac (e.g. -DHAVE_WOLFSSL_OPTIONS, -DWOLFCLU_NO_FILESYSTEM, -DNO_WOLFSSL_CRL_PRINT, -DNO_WOLFSSL_REQ_PRINT, -DNO_WC_ENCODE_OBJECT_ID, -DSIZEOF_LONG=8), but the capture command only passes AM_CPPFLAGS/CPPFLAGS, which for wolfCLU carry only include/visibility flags and no -D feature macros. The net effect is that the --options-h file passed to gen-sbom is essentially just compiler builtins, so the SBOM's build-configuration record does not reflect how wolfCLU was actually configured — undercutting the README claim that the SBOM "records the configured build options." make sbom still succeeds and produces a valid SBOM (the CI checks only assert format/name/purl/license/hashes/reproducibility, not the captured defines), so this is a completeness gap in the compliance evidence rather than a build break. Severity views differ across modes: the review mode rated this Medium and the review-security mode rated it Low; the stricter Medium is…
Fix: Point the config-header path at wolfCLU's actual header (e.g. $(if $(wildcard $(abs_builddir)/src/config.h),-include $(abs_builddir)/src/config.h) or make it overridable via an SBOM_CONFIG_H variable that wolfCLU sets to src/config.h), and include $(AM_CFLAGS) $(CFLAGS) in the $(CC) -dM -E command so the configured -D feature macros are captured. Alternatively, soften the README wording if capturing wolfCLU's build options is not intended.
| @set -e; \ | ||
| _defines=`mktemp $(abs_builddir)/_sbom_defines.XXXXXX`; \ | ||
| trap 'rm -rf $(abs_builddir)/_sbom_staging "$$_defines"' EXIT INT TERM HUP; \ | ||
| $(MAKE) install DESTDIR=$(abs_builddir)/_sbom_staging; \ |
There was a problem hiding this comment.
🟠 [Medium] PR description (--srcs / no staging) does not match implementation (--lib on staged binary) · Logic
The PR body states: "artifact hash uses --srcs $(wolfssl_SOURCES) (the compiled .c source list) rather than --lib; no staging step needed." The committed recipe does the opposite: it runs $(MAKE) install DESTDIR=$(abs_builddir)/_sbom_staging (a staging step), discovers the installed wolfssl binary, and passes --lib "$$sbom_art" (the binary) to gen-sbom. Hashing the installed binary (rather than the source list) makes the artifact hash sensitive to compiler/toolchain/build-path differences, which is worth confirming against the README's reproducibility claim ("repeated runs are byte-identical"). The CI reproducibility test passes only because both runs re-stage the same already-built binary; it would not guarantee cross-environment reproducibility of the hash. This is either a stale PR description or an implementation that diverged from intent.
Fix: Reconcile the PR description with the implementation. Confirm the intended artifact-hash source (installed binary via --lib vs source list via --srcs) and update whichever is wrong; if binary hashing is intended, soften the reproducibility wording to "reproducible for a given build".
| # Tools used by the SBOM targets (see scripts/sbom.am `make sbom`). GIT is used | ||
| # only to derive SOURCE_DATE_EPOCH for reproducible SBOM output; all three are | ||
| # optional and the target reports a clear error when a required one is missing. | ||
| AC_PATH_PROG([PYTHON3], [python3]) |
There was a problem hiding this comment.
🔵 [Low] AC_PATH_PROG([PYTHON3]) duplicates AM_PATH_PYTHON and requires an executable literally named 'python3' · Convention
configure.ac already runs AM_PATH_PYTHON([3.6],, [:]) which locates a Python >= 3.6 interpreter as $(PYTHON). The new AC_PATH_PROG([PYTHON3], [python3]) searches specifically for an executable named python3. On an environment where Python 3 is present only under a versioned/alternate name (e.g. python3.11, or python -> 3.x), PYTHON3 resolves empty and make sbom fails its test -n "$(PYTHON3)" guard even though a usable Python 3 ($(PYTHON)) was already found. Because the shared sbom.am hardcodes $(PYTHON3), this is a robustness nit rather than a bug (python3 is near-universal), but the two Python discovery paths are redundant.
Fix: Either fall back to $(PYTHON) when python3 isn't on PATH (e.g. AC_PATH_PROG([PYTHON3], [python3], [$PYTHON])), or add a short comment noting that make sbom intentionally requires a binary named python3.
| print('CDX ok:', m['name'], m['purl'], ids) | ||
| PY | ||
|
|
||
| - name: Reproducible across two runs (SOURCE_DATE_EPOCH) |
There was a problem hiding this comment.
🔵 [Low] Reproducibility CI step excludes the .spdx tag-value output from the byte-identical check · Test
The reproducibility test hashes and diffs only wolfclu-*.cdx.json and wolfclu-*.spdx.json, but make sbom also produces the tag-value wolfclu-*.spdx (via pyspdxtools conversion). The tag-value file is generated by a separate tool pass and could embed non-reproducible content (e.g. a creation timestamp) independent of the JSON outputs. Since reproducibility is a stated guarantee and this file is installed by install-sbom, it should be part of the byte-identical assertion.
Fix: Include wolfclu-*.spdx in both sha256sum invocations so the tag-value output's reproducibility is also verified.
| name: SBOM Test | ||
|
|
||
| on: | ||
| push: |
There was a problem hiding this comment.
🔵 [Low] pull_request branch filter '*' won't match base branches containing a slash · Logic
The push trigger targets [ 'master', 'main', 'release/**' ], but the pull_request trigger filters the base branch with [ '*' ]. In GitHub Actions branch filters, * matches only single-segment branch names (no / separator), so pull requests whose base is a release/** branch will not run the SBOM workflow, even though direct pushes to those branches do. This is a coverage inconsistency, not a security issue — SBOM regressions could merge into a release branch via PR without the check firing. Severity views differ across modes: review rated this Low/NIT and review-security rated it Info; the stricter Low is retained.
Fix: Use [ '**' ] (or explicitly list master, main, release/**) in the pull_request.branches filter so PRs into release branches also run the SBOM check, matching the intent of the push trigger's release/**.
| # default to the open PR head that carries it (wolfSSL/wolfssl#10343) so CI | ||
| # actually exercises `make sbom` instead of silently skipping. TODO: | ||
| # switch the fallback back to 'master' once #10343 merges. | ||
| - name: Checkout wolfssl (gen-sbom + library source) |
There was a problem hiding this comment.
⚪ [Info] CI builds and executes code from an unmerged external PR ref by default · Security
The workflow defaults the wolfssl checkout to refs/pull/10343/head (an open, unmerged pull request), then builds and installs it and executes its scripts/gen-sbom Python script during make sbom. Pulling and running code from a not-yet-merged PR is a mutable, pre-review supply-chain input. Risk is limited here because the ref is same-org (wolfSSL/wolfssl), the job uses permissions: contents: read, and it runs on a standard pull_request/push trigger (not pull_request_target), so no secrets are exposed and the token is read-only. The PR author has flagged this with a TODO to switch back to master once #10343 merges. Noting it so the temporary posture is tracked and reverted.
Fix: Track the TODO to switch the default ref back to master (or a pinned tag/commit SHA) once wolfSSL/wolfssl#10343 merges. Pinning to an immutable commit SHA rather than a mutable PR head would further reduce exposure in the interim.
Summary
make sbom,make install-sbom, andmake uninstall-sbomtargets to wolfCLU's autotools build for EU CRA compliance evidence (CycloneDX 1.6 + SPDX 2.3 output)AC_CHECK_PROGchecks forpython3andpyspdxtoolsinconfigure.acUsage
WOLFSSL_DIRmust point to a wolfssl checkout containingscripts/gen-sbom(branchfeat/sbom-embedded, ormasteronce wolfSSL/wolfssl#10343 merges).Notes
wolfssl), not a shared library — artifact hash uses--srcs $(wolfssl_SOURCES)(the compiled.csource list) rather than--lib; no staging step neededCLUWOLFSSL_VERSION_STRINGinwolfclu/version.h(the#define VERSION 0.3inclu_header_main.his stale and ignored)WOLFSSL_INCLUDEDIRdefaults to$(WOLFSSL_DIR)/include; wolfssl'soptions.his used for the config source since wolfCLU has no generated options header of its ownuninstall-hookdependency ensuresmake uninstallremoves SBOM files