conan-io / conan-extensions

Some extra Conan commands for different purposes, like artifactory tasks, conan-center-index, etc
MIT License
27 stars 26 forks source link

[bug] SBOM generation with sbom:cyclonedx using "--requires" argument produces broken sbom #138

Open markvmk opened 3 months ago

markvmk commented 3 months ago

I am trying to generate SBOM for conan packages that are used in a project using the sbom:cyclonedx conan extension as specified in the docs.

In my case it is necessary to use the "--requires" option to provide a reference to the recipe for which I want to create SBOM. When doing so the generated SBOM looks broken where an unknown component is introduced in the SBOM output. If the SBOM is generated by passing a path to the conan recipe of the project then the SBOM looks fine.

The same can be seen in the output provided in the README file for the extension.

The problem seems to be that the extension tries to set the component in metadata using the dependency graph root, which in case when "--requires" is used is always "cli" (I don't know what exactly that means). Since "cli" is not valid reference or package I always get UNKNOWN component in the produced SBOM.

memsharded commented 3 months ago

Thanks for reporting this @markvmk

Maybe @hedtke who contributed this extension can provide more feedback about this. It seems you are right, and the code should differentiate both cases, when using a conanfile as a root, and when using directly --requires without such a root, but I'll wait for @hedtke feedback first. Thanks!

hedtke commented 2 months ago

Hi. Yes, this was done more or less on purpose. When called only with the requires argument there still was a name required for the root node. At least with the dependencytrack version I used back then, omitting one common parent node was not an option.

Within the graph, this UNKNOWN node has the required packages as children and thus the SBOM can be generated.

I am not sure how to fix that. We could try to branch on this case, and omit this when no file is used. We would need to check with multiple SBOM clients whether they accept that.

memsharded commented 2 months ago

I am not sure if I understand completely.

The --requires approach generates a "Virtual" conanfile on the fly in memory, and then add those arguments as requires.

The <path> approach loads that file conanfile from disk and its defined requires will be the dependencies.

In both cases it seems the virtual conanfile root or the <path> conanfile root could be treated in the same way?

Or maybe the approach works only with the --requires if exactly 1 --requires is provided, but it will fail if provided more than one, because that --requires is being used as the root of the SBOM? Does the SBOM require to have exactly one named "root"?

hedtke commented 2 months ago

I think, at the moment, I treat both cases in the same way, thus one time using the virtual conanfile as root which generates an entity called UNKNOWN in the SBOM (the root). I was not aware that this creates problems

memsharded commented 2 months ago

I think we need to understand better.

The SBOM is identical in both cases for dependencies, just the root is different. In one case with --requires the SBOM is:

"metadata": {
    "component": {
      "author": "Conan",
      "bom-ref": "BomRef.23799299611816604.711995208657742",
      "name": "UNKNOWN.1847826091984",
      "type": "library"
    },
    "timestamp": "2024-07-03T12:17:39.433832+00:00",
    "tools": [
      {

In the other case with <path> it is:

"metadata": {
    "component": {
      "author": "<Put your name here> <And your email here>",
      "bom-ref": "pkg:conan/mypkg@0.1",
      "description": "<Description of mypkg package here>",
      "licenses": [
        {
          "license": {
            "name": "<Put the package license here>"
          }
        }
      ],
      "name": "mypkg",
      "purl": "pkg:conan/mypkg@0.1",
      "type": "library",
      "version": "0.1"
    },

Note in the second case the "bom-ref": "pkg:conan/mypkg@0.1", is not really an uploade package or anything like that. It is just a local name that might not exist anywhere else. So in this regard it is not much different than the first place. For example if instead of using a conanfile.py with a name we use a conanfile.txt that won't define a name, we will get back:

"metadata": {
    "component": {
      "author": "Conan",
      "bom-ref": "BomRef.42409095884590187.8917758652747707",
      "name": "UNKNOWN.2876611447008",
      "type": "library"
    },

Maybe it should be possible to just completely drop the "root" node?

markvmk commented 2 months ago

In my case, when I use --requires, the package given as requires is listed in the components array of the generated SBOM. In this case the component in the metadata is populated with that UNKNOWN component.

When I use a path to the recipe, the package that is described in the recipe is not listed in the components array, but instead it is set as component in the metadata section of the SBOM.

Ideally, in both cases the produced output would be the same (I don't know if that is possible at the moment).

Maybe removing the root node (cli) in case of --requires so that the next node becomes root would solve the issue but I don't think that would work in case of multiple --requires.

Another problem that I see also at the moment is that if you generate the SBOM for the same package with --requires 2 times one after another you get different name of the metadata component. In the past as it seems the name used to be always only "UNKNOWN" but now it has random ID added in the name. Additionally, if you generate SBOM for many packages and then try to merge these using a SBOM merge tool (e.g. cyclonedx-cli), then a lot of UNKNOWN components are inserted into the final merged SBOM, so in my opinion the UNKNOWN component should never exist in the output.

memsharded commented 2 months ago

When I use a path to the recipe, the package that is described in the recipe is not listed in the components array, but instead it is set as component in the metadata section of the SBOM.

No, I am afraid there is some misunderstanding here. When given a <path> that is not a package at all. That is just a consumer recipe, that is not even in the cache, not uploaded to anywhere. It cannot be a component because it is not a Conan package, it might not have artifacts built at all. Exactly the same as if it is using a conanfile.txt without name and version. The fact that a local conanfile.py can have a name and version and list it in the SBOM, doesn't make it a component.

If you want to list that as a component you need to first conan create <path> to make it a package, then do a conan sbom:cyclonedc --requires=<ref> to make it a component of the SBOM.

markvmk commented 2 months ago

Maybe I was not very specific above, but probably there is misunderstanding too.

I will take the elfutils project as example. The first option is to use --requires so I generate the SBOM as following: conan sbom:cyclonedx --format 1.4_json --requires elfutils/0.190

The second option is to use path to the recipe. I have local checkout of the conan-center-index and assuming that the terminal is navigated into the recipe dir for elfutils (conan-center-index/recipes/elfutils/all) I try to generate the SBOM as following: conan sbom:cyclonedx --format 1.4_json --version 0.190 .

In the first variant the component that is included in metadata is UNKNOWN, and that is the one that creates problem when merging multiple SBOMs because it ends up in the components list in the merged SBOM. elfutils in this case is added in the components list of the SBOM.

In the second variant I think the SBOM looks fine. The component in metadata is elfutils, and the components list does not contain elfutils in this case.

I think (I might be wrong) that it would be also fine if in case of --requires the component in metadata remains empty because if I understand the CycloneDX SBOM spec correctly the metadata and/or component in metadata section are optional.

memsharded commented 2 months ago

In the second variant I think the SBOM looks fine. The component in metadata is elfutils, and the components list does not contain elfutils in this case.

No, this is not really correct. In the second case, elfutils is not a package. It might not even exist as a package and it will be missing detailed information as the recipe-revision, the package-id or the package-revision. It might look-like a package, but it is not. You might be even be able to remove the name and version from the conanfile and still get its dependencies, as elfutils is nothing but a local name, not a real existing component/package name.

There are 2 different dependency graphs here. In the first case with --requires, elfutils is actually a dependency, there is a package with that name and version that is a dependency. In the second case with <path>, elfutils package doesn't exist.

My suggestion above to @hedtke was actually to remove the "root" component inside metadata, leaving the components to represent the dependencies, the actual packages that makes sense for an SBOM to represent. I admit that I don't have enough knowledge about SBOMs in this regard.

memsharded commented 2 months ago

Maybe we can summon @jkowalleck, and ask for clarification about this item:

Many thanks!

jkowalleck commented 2 months ago

re: https://github.com/conan-io/conan-extensions/issues/138#issuecomment-2206646166

I've seen your ping. Need to wrap my head around this, and this might take a while.

Meanwhile, I would encourage you to join the CycloneDX slack workspace (invite) and ask your questions in there (we even have a conan channel), in hope that somebody done it already and can tell how they solved it. Or ask via one of the other comm methods.