Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a SBOM file in CycloneDX format #9777

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

hughsie
Copy link

@hughsie hughsie commented Nov 15, 2024

Hi,

My name is Richard Hughes and I'm a developer at Red Hat. I'm the maintainer of fwupd and LVFS, and am trying to improve software supply chain security by encouraging OEMs, ODMs and IBVs to ship Software Bill of Materials with each firmware binary blob (SBOMs).

I'm working alongside lots of other companies proactively trying to do the right thing. The reason I've opened this pull request is because your project is either used in the build process of a firmware we care about (e.g. EDK II, or coreboot) or is built into the firmware binary itself. Although my personal focus is on firmware, the SBOM file is in CycloneDX format (one of the most popular industry standards) which makes it also useful when building containers or OS images too.

I would like to contribute this template SBOM file into your project that gets included into source control with substituted values that get populated automatically. I'm not super familiar with your project, and so I've done my best populating the project values -- but please point out any that are incorrect and I'll fix them up. I've also put the sbom.cdx.json file in what I feel is the right place, but please say if you want me to put it somewhere different or name it a different thing; the directory and sbom prefix are unimportant.

I've written a bit more about this proposal here https://blogs.gnome.org/hughsie/2024/11/14/firmware-sboms-for-open-source-projects/ and there's also lot more information about firmware SBOMs here: https://lvfs.readthedocs.io/en/latest/sbom.html – many thanks for your time.

Improve supply chain security by including a SBOM file with substituted values.

This will be used to construct a composite platform SBOM.

@hughsie
Copy link
Author

hughsie commented Nov 22, 2024

I've updated the PR to match the existing CPE name I found on nvd.nist.gov, I hope that's okay.

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review size-xs Estimated task size: extra small (a few hours at most) component-platform Portability layer and build scripts and removed needs-design-approval size-s Estimated task size: small (~2d) labels Nov 25, 2024
@gilles-peskine-arm
Copy link
Contributor

Having discussed this internally, we're happy to include an SBOM file. We don't particularly intend to maintain it, but I don't expect this will require a lot of maintenance.

The Mbed TLS build currently pulls from two repositories (https://github.com/Mbed-TLS/mbedtls and https://github.com/Mbed-TLS/mbedtls-framework), and soon we will split out a third project (https://github.com/Mbed-TLS/TF-PSA-Crypto). Pulling a specific commit of mbedtls also dictates which commit is used in the other repositories. Do the other repositories need to be mentioned in the SBOM file?

@hughsie
Copy link
Author

hughsie commented Nov 25, 2024

Having discussed this internally, we're happy to include an SBOM file.

Great, thanks!

We don't particularly intend to maintain it, but I don't expect this will require a lot of maintenance.

Agree, with the exception of renaming the project, changing the licence or source control location -- all which seem most unlikely.

Pulling a specific commit of mbedtls also dictates which commit is used in the other repositories. Do the other repositories need to be mentioned in the SBOM file?

Ideally we'd have a sbom.cdx.json in each project; that way we record the explicit commit and the auto-populated list of authors are set up correctly. Would you like me to submit PRs for both other projects?

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've looked enough to approve once my concerns are addressed.

sbom.cdx.json Outdated Show resolved Hide resolved
sbom.cdx.json Outdated
"metadata": {
"authors": [
{
"name": "@VCS_SBOM_AUTHORS@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who is supposed to substitute these keywords? With what tools?

In particular, needing an extra tool in the environment where we prepare release tarballs would require some work on our part.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good question, but no extra work required on your part. Those tokens are populated automatically using the uswid tool at build time -- and also whatever non-free build tools the firmware teams use. The tokens themselves are only standardized in uswid at the moment, but will soon move to the Firmware Embedded SBOM Specification where they'll be concreted into place. We've got quite a lot of support from other projects now -- e.g. see https://docs.google.com/spreadsheets/d/1gKEWLxdLubOfgS1cqqY2umEX0ohYEoEy-B1i59HNVqg/edit?usp=sharing -- and so I think the @TOKENS@ are here to stay now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no extra work required on your part

Currently you can get our releases either via a git tag or via a tarball. How could the CVS_xxx tokens get populated from a release tarball?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question; I think all the firmware is currently built using git tags rather than tarballs, but I do appreciate the tarball case. In most cases for other projects the sbom isn't actually in the list of files that gets included in the release tarball (as like you said, the values are not auto-populated) -- but if you do want it in the tarball we can do something like https://github.com/fwupd/fwupd/blob/main/contrib/meson.build#L15 -- I'd need a bit of a hand with the CMake stuff if so please.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a usable SBOM in the tarball would be nice. If you don't need it, though, it can wait until someone needs it. At the moment, we don't have bandwidth to investigate, and our build and release scripts are changing heavily, so if we can avoid changing those scripts for now, all the better.

sbom.cdx.json Outdated
{
"type": "library",
"bom-ref": "pkg:github/Mbed-TLS/mbedtls@@VCS_TAG@",
"cpe": "cpe:2.3:a:mbed:mbedtls:@VCS_TAG@:*:*:*:*:*:*:*",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with CPE and I have not validated this string, but it looks suspicious. Mbed TLS is no longer part of the “Mbed” organization, it's now part of the TrustedFirmware organization.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly why I wanted this file to live upstream, I missed this completely! On one hand using the cpe:2.3:a:mbed makes sense as it's what other CVEs have been using, but on the other hand using TrustedFirmware. According to MITRE, the CPE vendor can be trustedfirmware -- e.g. from https://nvd.nist.gov/products/cpe/detail/2AF395D6-6367-4EFF-A0D0-C0CB6CA99E3E

I'm happy to modify the SBOM file if you want to use the trustedfirmware CPE vendor going forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to trustedfirmware. mbed was correct until 2020, but since then it's trustedfirmware.

Do you know if there's a process we should follow to get the NIST database updated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to trustedfirmware

Done; thanks.

Do you know if there's a process we should follow to get the NIST database updated?

I think the big picture idea is this PR; in that the upstream project says "this is what I want to use" and then the various CNAs dedupe the old ones like cpe:2.3:a:arm:mbed_tls. At the moment we don't have a very good way of saying "cpe:2.3:a:trustedfirmware:mbed_tls" is better than "cpe:2.3:a:arm:mbed_tls" and when https://docs.google.com/spreadsheets/d/1gKEWLxdLubOfgS1cqqY2umEX0ohYEoEy-B1i59HNVqg/edit?usp=sharing is all green we can start tidying up the CPE register. If it helps it's also something that irritates me, as I want to write a simple VEX rule for "is this system affected" and when there are variants of CPE you have to make the match frustratingly fuzzy.

sbom.cdx.json Outdated Show resolved Hide resolved
sbom.cdx.json Outdated
@@ -0,0 +1,43 @@
{
"bomFormat": "CycloneDX",
"specVersion": "1.6",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self — full specification at https://cyclonedx.org/docs/1.6/json/

sbom.cdx.json Outdated
"url": "https://github.com/Mbed-TLS/mbedtls"
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a dependency on mbedtls-framework? That repository contains build scripts (including source code generators). The dependency is implemented as a git submodule (framework).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need an extra sbom.cdx.json in there; I'm happy to do that too of course, but make get this one perfect and then the other one becomes a formality. The uswid CLI actually adds a dependency on subprojects automatically, but I can make it explicit too if you'd rather.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The uswid CLI actually adds a dependency on subprojects automatically

Good to know, thanks. So I think this works out:

  • For Git consumers, they'll get the dependency automatically.
  • For tarball consumers, they'll get a tarball with both sbom.ddx.json files. There won't be a dependency but the whole thing comes as a single tarball so it can't be dissociated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Git consumers, they'll get the dependency automatically.

Exactly.

For tarball consumers, they'll get a tarball with both sbom.ddx.json files

Aha. So if we do want to ship them in the tarball we should add the dep section manually then; uswid will only auto-add the dep if it's not already present. Which way around makes sense, mbedtls requires mbedtls-framework I assume? Or do they depend on each other the "other way round" too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mbedtls-framework (the framework submodule) does not provide a useful product on its own. It's used as part of the build of mbedtls. The framework is mostly test code, but it also contains a few build scripts, so a tainted framework would taint the library build.

@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Nov 25, 2024
Improve supply chain security by including a SBOM file with substituted values.

This will be used to construct a composite platform SBOM.

Signed-off-by: Richard Hughes <[email protected]>
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing this. Looks good to me, as an expert on Mbed TLS but not on SBOM files.

As discussed, the Mbed TLS build uses files from the framework directory, which is a Git submodule that points to https://github.com/Mbed-TLS/mbedtls-framework . If I understand correctly, the submodule should also have an SBOM file, can you please submit one?

The first release containing this file will be Mbed TLS 4.0. We will also make more release to the Mbed TLS 3.6.x long-time support branch. If you'd like it to have an SBOM file, please make a pull request targeting the mbedtls-3.6 branch. There's also an LTS branch for Mbed TLS 2.28.x, but in all likelihood, it will only have one more release until end-of-life, so I don't think an SBOM file would be useful there.

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review priority-medium Medium priority - this can be reviewed as time permits and removed needs-work labels Dec 3, 2024
@hughsie
Copy link
Author

hughsie commented Dec 3, 2024

If I understand correctly, the submodule should also have an SBOM file, can you please submit one?

Can do! Mbed-TLS/mbedtls-framework#88

If you'd like it to have an SBOM file, please make a pull request targeting the mbedtls-3.6 branch

I'll ask the firmware people and find out what version they're currently using. Thanks for the heads up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-platform Portability layer and build scripts enhancement needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: In Development
Development

Successfully merging this pull request may close these issues.

3 participants