feat: add sbom Makefile target#414
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a make sbom target to generate CycloneDX and SPDX SBOM artifacts for wolfHSM using wolfSSL’s scripts/gen-sbom.
Changes:
- Introduces an
sbomMakefile target with checks for version parsing andWOLFSSL_DIR. - Gathers sources from
src/*.cand passes compiler defines + metadata intogen-sbom. - Emits versioned
wolfhsm-<version>.cdx.jsonandwolfhsm-<version>.spdx.jsonoutputs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Running make sbom WOLFSSL_DIR=../wolfssl against the latest wolfSSL master I get the following error:
ERROR: ../wolfssl/scripts/gen-sbom not found.
Use a wolfSSL tree that includes SBOM support.
make: *** [Makefile:73: sbom] Error 1
Did this not merge into wolfSSL master yet? If so it shouldn't merge into wolfHSM.
jackctj117
left a comment
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: review + review-securityOverall recommendation: COMMENT
Findings: 5 total — 5 posted, 0 skipped
5 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] [review] Default WOLFSSL_DIR is off by one level for the top-level Makefile —
Makefile:62 - [Medium] [review+review-security] gen-sbom --options-h receives only compiler builtin macros, not wolfHSM/wolfSSL config —
Makefile:87-99 - [Low] [review+review-security] VERSION shell-out runs on every make invocation, not just sbom —
Makefile:63 - [Low] [review+review-security] Temp file uses hardcoded /tmp prefix, ignoring $TMPDIR —
Makefile:87 - [Low] [review+review-security] README manual invocation diverges from the Makefile sbom target —
README.md:37-44
Review generated by Skoll
|
@bigbrett Correct — gen-sbom hasn't merged to wolfssl master. It's in wolfssl PR wolfSSL/wolfssl#10343, which is still open, so no wolfssl release or master checkout has scripts/gen-sbom yet. The target can't create that file; what it can do is fail clearly. I updated the error to name the dependency and point you at setting WOLFSSL_DIR to a tree that has the script, and the README says the same. So the wolfHSM side is ready to use the moment #10343 lands; it just no-ops with a clear message until then. I did not vendor a copy of gen-sbom into wolfHSM. If you'd rather hold this PR until #10343 merges, flag it and I'll mark it blocked. |
4cef60f to
d03b224
Compare
|
Force-pushed a cleanup: this branch was cut from a stale shallow base, which made ~7,800 lines of already-merged main content (armv8m-tz, hwkeystore, test files) appear as PR additions. Rebased onto current main; the PR now contains only the SBOM change: +133 in Makefile and README. Verified on the rebased head: @bigbrett @dgarske ready for re-review; your earlier comments on the unrelated files are moot. |
|
Resolved the July 10 review threads: every fix discussed in them is carried in the rebuilt branch (d03b224 contains the cumulative Makefile/README state, verified end-to-end with make sbom). The conversation history references pre-rebuild commit ids; the content is all here. |
|
direction is right, but the target should reuse gen-sbom the way the rest of the family does Thanks Mark. The overall approach — a dedicated make sbom target that calls wolfSSL's gen-sbom — is the correct call for wolfHSM, since wolfHSM is a hand-written Makefile and can't include scripts/sbom.am like the autotools repos (wolfMQTT, wolfProvider, wolfCLU, wolfEngine, wolfssh, wolfscep). We already have a precedent for a non-automake reimplementation: wolfTPM's CMake sbom target, which was explicitly brought to parity with the canonical scripts/sbom.am. That's the bar this PR should hit. Right now #414 diverges from the canonical generator in a few ways, which is what the review threads are circling: It rolls its own source hash (SHA-256 fingerprint of src/*.c) instead of using gen-sbom's own input. gen-sbom already supports --lib (hash the built library from a staged install) and --srcs/--srcs-file (OmniBOR gitoid for embedded/source builds). Using a private hash means the wolfHSM SBOM checksum isn't comparable to the other products and doesn't describe the shipped artifact. Please pick --lib or --srcs deliberately and note which in the PR. --options-h only receives compiler builtins, not wolfHSM/wolfSSL config macros (jackctj117's finding). The point of --options-h is to record the configured feature macros, so this needs the real CC -dM -E dump with wolfHSM's actual CPPFLAGS/CFLAGS + config header. Missing parity items the fragment/CMake target have: install-sbom / uninstall-sbom, SOURCE_DATE_EPOCH for reproducible output, and the SPDX validation / tag-value step. Sequencing: as bigbrett and dgarske noted, gen-sbom is still on wolfSSL PR #10343 and hasn't merged to master. The clear-error no-op behavior is fine, but this PR shouldn't merge before #10343 lands — let's mark it blocked-on that. Net: keep the bespoke-Makefile approach, but rework it to parity with wolfTPM's target (drive everything through gen-sbom, capture real macros, add the install/reproducibility bits), and gate the merge on #10343. Happy to pair on the diff. |
- Validate SPDX with pyspdxtools and emit tag-value wolfhsm-<ver>.spdx; SBOM_VALIDATE=no skips both - Honor SOURCE_DATE_EPOCH, defaulting to last git commit time, so repeated runs are byte-identical - Add opt-in install-sbom / uninstall-sbom under $(PREFIX)/share/doc/wolfhsm - List wolfssl as a dependency component with version read from WOLFSSL_DIR; SBOM_DEP_WOLFSSL=no for WOLFHSM_CFG_NO_CRYPTO builds
|
@sameehj parity items are in as of 073f4eb:
On the artifact hash: the July 14 rebuild already dropped the private fingerprint — hashing goes through Config macros: also in since the rebuild — the recipe preprocesses Sequencing: agreed. Marked blocked on wolfSSL/wolfssl#10343 at the top of the description — @bigbrett that also answers your question: gen-sbom is still on that PR, this one must not merge before it. |
| - [wolfHSM API Reference](https://www.wolfssl.com/documentation/manuals/wolfhsm/appendix01.html) | ||
| - [wolfHSM Examples](https://github.com/wolfSSL/wolfHSM/tree/main/examples) | ||
|
|
||
| ## SBOM / EU CRA Compliance |
There was a problem hiding this comment.
Could we pls move this to a new markdown document like SBOM.md or EU-CRA.md?, I'd like to keep the top level README as lean as possible. You can add an sbom generation bullet to the bulleted list above pointing to that doc if you want
Important
Blocked on wolfSSL/wolfssl#10343 —
scripts/gen-sbomhas not merged to wolfssl master yet. Do not merge this PR before #10343 lands. Until then the target fails with a clear error pointing at the missing script.Summary
Adds
make sbom,make install-sbomandmake uninstall-sbomtargets that produce CycloneDX and SPDX SBOM files for wolfHSM, at parity with the canonicalscripts/sbom.amfragment and wolfTPM's CMake target.ChangeLog.md(e.g.,# wolfHSM Release v1.4.0)src/*.c(sorted for reproducible output)gen-sbom --srcs(OmniBOR gitoid over the source list).--srcswas chosen over--libdeliberately: wolfHSM ships as source into embedded builds, so there is no canonical installed library to hashwolfhsm/wh_settings.hwith the same defines/include path as the test build (WOLFHSM_CFG_DIRselects the config; point it at your port'swolfhsm_cfg.h/user_settings.h)WOLFSSL_DIR;SBOM_DEP_WOLFSSL=noforWOLFHSM_CFG_NO_CRYPTObuildspyspdxtoolsand converts it to tag-value;SBOM_VALIDATE=noskips bothSOURCE_DATE_EPOCH(defaults to last git commit time) — repeated runs are byte-identicalinstall-sbomplaces files under$(PREFIX)/share/doc/wolfhsm(opt-in; plainmake/make installnever touches SBOM files)Usage
Outputs:
wolfhsm-<version>.cdx.json,wolfhsm-<version>.spdx.json,wolfhsm-<version>.spdx(tag-value)Requirements
WOLFSSL_DIRmust point to a wolfssl source tree containingscripts/gen-sbom(feat: SBOM generation and OmniBOR build provenance (CRA compliance) wolfssl#10343, pending a future wolfssl release)python3, pluspip install spdx-toolsunlessSBOM_VALIDATE=noTest plan
make sbom WOLFSSL_DIR=...— CDX/SPDX/tag-value produced, correct version and source countWOLFHSM_CFG_*/wolfSSL options with the default test config)SBOM_DEP_WOLFSSL=noSBOM_VALIDATE=noskips validation/tag-value; missingpyspdxtoolsfails with a clear error otherwiseinstall-sbom/uninstall-sbomround-trip underDESTDIRmake -n cleandoes not shell out (GNU make 3.81 parse regression check)