feat: add make sbom / install-sbom / uninstall-sbom targets#410
feat: add make sbom / install-sbom / uninstall-sbom targets#410MarkAtwood wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SBOM generation and install/uninstall targets to the autotools build so wolfProvider can emit CycloneDX 1.6 and SPDX 2.3 artifacts (via wolfSSL’s gen-sbom script), plus configure-time discovery of required tooling.
Changes:
- Add
make sbom,make install-sbom, andmake uninstall-sbomtargets that stage-install the built artifacts and generate CycloneDX/SPDX outputs. - Add
AC_CHECK_PROGchecks forpython3andpyspdxtoolsinconfigure.ac.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Makefile.am | Adds SBOM generation and install/uninstall targets and related variables. |
| configure.ac | Adds checks for python3 and pyspdxtools availability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # --------------------------------------------------------------------------- | ||
| # SBOM generation (CycloneDX + SPDX) via wolfssl's gen-sbom script | ||
| # --------------------------------------------------------------------------- | ||
| WOLFSSL_DIR ?= $(WOLFSSL_INSTALL_DIR) |
| WOLFSSL_DIR ?= $(WOLFSSL_INSTALL_DIR) | ||
| WOLFSSL_INCLUDEDIR ?= $(WOLFSSL_DIR)/include | ||
| PRODUCT = wolfprov | ||
| VERSION = $(shell grep LIBWOLFPROV_VERSION_STRING $(srcdir)/include/wolfprovider/version.h 2>/dev/null | sed 's/.*"\(.*\)".*/\1/') |
| @if test -z "$(PYTHON3)"; then \ | ||
| echo "ERROR: python3 not found in PATH."; exit 1; fi | ||
| $(MAKE) install DESTDIR=$(builddir)/_sbom_stage | ||
| $(PYTHON3) $(GEN_SBOM) $(SBOM_OPTS) | ||
| rm -rf $(builddir)/_sbom_stage |
Adds CycloneDX + SPDX SBOM generation via wolfssl's gen-sbom script. Usage: make sbom WOLFSSL_DIR=/path/to/wolfssl wolfProvider has no generated options.h; uses installed wolfssl's options.h. WOLFSSL_DIR defaults to WOLFSSL_INSTALL_DIR. SONAME is libwolfprov.so.0.0.0 (libtool default, no version-info set).
a93e1f6 to
8db5499
Compare
Replace the inline SBOM recipe with the shared, product-agnostic scripts/sbom.am used across the wolfSSL autotools stack. wolfProvider just declares itself (a libwolfprov shared library that links both wolfSSL and OpenSSL) and includes the fragment, gaining reproducible output, license/version pinning, SPDX validation, dependency recording (wolfssl + openssl), and an install/uninstall hook. Switch configure to AC_PATH_PROG + AC_SUBST, expand the README, and add a SBOM CI workflow. Signed-off-by: Sameeh Jubran <sameeh.j@gmail.com>
aidangarske
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: REQUEST_CHANGES
Findings: 2 total — 2 posted, 0 skipped
Posted findings
- [High] SBOM version lookup breaks clean out-of-tree builds and overrides Automake VERSION —
Makefile.am:60-71 - [Medium] pyspdxtools probe is never enforced by the sbom target —
configure.ac:30, Makefile.am:80-83
Review generated by Skoll.
| # it is (a shared library that links both wolfSSL and OpenSSL) and includes it. | ||
| # WOLFSSL_DIR must point to a wolfssl source tree containing scripts/gen-sbom; | ||
| # it defaults to the --with-wolfssl location, which is an install prefix, so for | ||
| # `make sbom` pass a source tree: make sbom WOLFSSL_DIR=/path/to/wolfssl. |
There was a problem hiding this comment.
🟠 [High] SBOM version lookup breaks clean out-of-tree builds and overrides Automake VERSION
🚫 BLOCK bug
The PR defines VERSION by grepping $(srcdir)/include/wolfprovider/version.h, but this header is generated by configure.ac and is gitignored; a clean source tree only contains include/wolfprovider/version.h.in. In a normal VPATH/out-of-tree build, the generated header lives in the build tree, so this grep returns empty. Because VERSION is also Automake's standard package-version variable, this can produce SBOM paths like wolfprov-.spdx and can also affect other Automake rules that rely on $(VERSION). The PR introduces this by adding a new top-level VERSION = ... assignment for SBOM generation.
Recommendation: Replace the custom VERSION assignment with a separate SBOM variable based on $(PACKAGE_VERSION) or @VERSION@ before merge. For example: SBOM_VERSION =
| AC_LANG(C) | ||
|
|
||
| # 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 |
There was a problem hiding this comment.
🟡 [Medium] pyspdxtools probe is never enforced by the sbom target
💡 SUGGEST bug
The PR adds AC_CHECK_PROG([PYSPDXTOOLS], [pyspdxtools], [pyspdxtools]) and the README says pyspdxtools is required, but make sbom only checks $(PYTHON3) before invoking gen-sbom. If pyspdxtools is missing, configure still succeeds and the target proceeds until the generator fails later, likely with a less actionable error. This weakens the dependency check introduced by the PR.
Recommendation: Use the PYSPDXTOOLS configure result in the sbom recipe so the documented dependency is checked before running the generator, e.g. add an @if test -z "$(PYSPDXTOOLS)"; then echo "ERROR: pyspdxtools not found in PATH. Install with: pip install spdx-tools"; exit 1; fi guard.
|
Both Skoll findings are against code that isn't in this PR (verified against the current head):
The recipe was refactored into the shared |
Detailed verification — both Skoll findings are against code that isn't in this PRChecked two ways against the reviewed head Finding 1 — "VERSION grep at
|
Summary
make sbom,make install-sbom, andmake uninstall-sbomtargets to wolfProvider'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
wolfprovthroughout (matchesAC_INITandlib_LTLIBRARIES), notwolfprovideroptions.h; uses installed wolfssl'soptions.hvia$(WOLFSSL_INCLUDEDIR)/wolfssl/options.h.WOLFSSL_DIRdefaults to$(WOLFSSL_INSTALL_DIR)(set by--with-wolfssl=at configure time)libwolfprov.so.0.0.0(no-version-infoset) —--libpath uses this literal, not the package version1.1.1--with-wolfssl,WOLFSSL_INSTALL_DIRmay be malformed; passWOLFSSL_DIR=explicitly in that case