Add some additional property fileds to cycloneDX output.

[pbalogh-sa]

Signed-off-by: Peter Balogh [email protected]

Related anchore/grype#481

This PR adds some additional properties to the CycloneDX output. After converting the generated CyclonDX format to Syft JSON format with these properties, the Grype using the converted json can find the vulnerabilities.

[wagoodman]

@pbalogh-sa stellar PR --thanks for opening! The use of properties here helps a TON in the sense of unlocking the use of using CycloneDX formatted documents as input in grype (e.g. the source/origin properties you added).

I think the only thing that looks out of place is the distro information which has been attached to the BomDescriptor section. This section is meant to describe the tool and context that created the SBOM. The distro itself is from the result of cataloging and not truly meant for this BOM Descriptor section.

I'm not certain that there is a good spot for this information in a top-level section in CycloneDX (if there is, shout out! I could have easily missed something here). But, it may be that the easiest path forward is to encode the distro information directly on packages which this information matters for. For instance, capturing distro name and version for rpm, deb and apk as properties on the package (but not capture this information for non-distro packages, such as python, java, etc).

Let me know what you think about this approach @pbalogh-sa --happy to chat it through more.

[pbalogh-sa]

@wagoodman I wanted to convert cyclonedx format to syft json, and as I see in the syft json the distro information is in top-level, and not in package level. So This was the reason why I put it into the bomDescriptor section. The distro is only crucial in the case when an image was the input. Putting the distro information at the package level makes sense. For example, if we analyze a source directory and after it, we analyze the docker image we built using that source code we will be able to merge two BOM into one because all information will be in the package level.

[wagoodman]

Using the BOM Descriptor is very convenient, but technically not the reason for the section, which is why I can't recommend heading further down that path (maybe keep it in our back pocket as a workaround incase other solutions don't work as well as we hope?).

I think the only two downsides (so far) to adding distro information to each package is:

1. the SBOM gets larger due to redundant information --this is more of an annoyance than a problem I suspect.
2. what if there are zero OS packages found? An odd case, but with the new --exclude option it's possible. In this case we wouldn't have any distro information in the SBOM. This is bad, but possibly not a show stopper for this path (still notable).

...we will be able to merge two BOM into one because all information will be in the package level

That's true! There is a benefit there I didn't think of.

When we get to format conversion, I think this will complicate the case where we are trying to get information transformed back into the syft-native model, but I think we can solve that one later (so shouldn't be a restriction here).

@pbalogh-sa are you OK with the per-package approach? or want to keep looking for alternate solutions? (there could be another option in the spec for us)

[spiffcs]

@pbalogh-sa! Thanks for the excellent PR. @wagoodmans comments get to the core of the specification questions I might have here.

I added some small comments regarding the tests to just be sure the correct values are being redacted.

Thanks again!

[pbalogh-sa]

are you OK with the per-package approach? or want to keep looking for alternate solutions? (there could be another option in the spec for us)

@wagoodman Currently distro is in the level of syft.SBOM.Artifacts struct. So it would be hard change now I think.

[wagoodman]

@pbalogh-sa We recently got in a feature that may help here: #754

Now we're encoding distro information onto each package within the pURL. In the near future the pURL will be used downstream in grype for vulnerability matching (enabling OS-level matching as we do today).

Does this change fit your needs?

[pbalogh-sa]

@wagoodman Thanks, along with #710 can fit my needs. I will check it.

[pbalogh-sa]

@wagoodman In the case of Java artifactID and groupID are missing.

I found the reason:

syft/internal/formats/common/cyclonedxhelpers/properties.go

Lines 28 to 44 in 1350d6c

	func getCycloneDXProperties(m interface{}) *[]cyclonedx.Property {
	props := []cyclonedx.Property{}
	structValue := reflect.ValueOf(m)
	// we can only handle top level structs as interfaces for now
	if structValue.Kind() != reflect.Struct {
	return &props
	}
	structType := structValue.Type()
	for i := 0; i < structValue.NumField(); i++ {
	if name, value := getCycloneDXPropertyName(structType.Field(i)), getCycloneDXPropertyValue(structValue.Field(i)); name != "" && value != "" {
	props = append(props, cyclonedx.Property{
	Name: name,
	Value: value,
	})
	}
	}
	return &props

Maybe we need to check lower level structs for properties.
I've opened a PR to address it. #758

[wagoodman]

@pbalogh-sa With #758 merged, I think the remaining items on this PR are:

• remove the distro object from being encoded into the BomDescriptor
• rebase to pull in the latest changes

Let me know if there is anything I can do to help (or if I got the above list wrong)!

[pbalogh-sa]

implemented by: #710 and #758