[feature] Improve CDX parsing

[pxp928]

Is your feature request related to a problem? Please describe.
Currently, the CDX parser appends all dependencies to the top level package which may be inaccurate:

https://github.com/guacsec/guac/blob/main/pkg/ingestor/parser/cyclonedx/parser_cyclonedx.go#L243-L262

Instead, this needs to be done based on the relationships defined by the CDX SBOM.

Describe the solution you'd like

This is not based on the relationship so that can be inaccurate (can capture both direct and in-direct).
This needs to be done by utilizing *c.cdxBom.Dependencies 

see CycloneDX/specification#33

Describe alternatives you've considered
None

[Yaxhveer]

Hey @pxp928, I would like to work on this. Just wanted to ask here how we use the direct dependencies that we get from *c.cdxBom.Dependencies?

[pxp928]

Hey @Yaxhveer!

So c.cdxBom.Dependencies is implemented https://github.com/guacsec/guac/blob/main/pkg/ingestor/parser/cyclonedx/parser_cyclonedx.go#L285-L309 but we are not following the logic set forth by CycloneDX/specification#33 (comment). We want to be able to check if this dependency is direct, in-direct (transitive) or both. It can be both a direct and in-direct dependency in which case there will be two assembler.IsDependencyIngest created for the direct one and the in-direct one. Currently, we are marking them all as unknown: https://github.com/guacsec/guac/blob/main/pkg/ingestor/parser/common/helpers.go#L53

It should be unknown by default, but if you can determine it via the logic provided, it should be specified as Direct or In-direct.

Let me know if you have other questions! Looking forward to the contribution :)

[Yaxhveer]

Hey @pxp928, just a simple doubt here the dependency type is between top level and foundPkg or between foundPkg and DepPkg (in func GetIsDep())

guac/pkg/ingestor/parser/common/helpers.go

Lines 49 to 56 in 4dc7c94

	Pkg: foundNode,
	DepPkg: rpackNode,
	DepPkgMatchFlag: GetMatchFlagsFromPkgInput(rpackNode),
	IsDependency: &model.IsDependencyInputSpec{
	DependencyType: model.DependencyTypeUnknown,
	Justification: justification,
	VersionRange: *rpackNode.Version,
	},

[pxp928]

Based off this CycloneDX/specification#33 (comment) it can be both. So if c.cdxBom.Dependencies is defined (not null), we should not do this: https://github.com/guacsec/guac/blob/main/pkg/ingestor/parser/cyclonedx/parser_cyclonedx.go#L260 but instead use the relationships defined to create both the top level and foundPkg (direct) and between foundPkg and DepPkg (in-direct) in the function:

guac/pkg/ingestor/parser/cyclonedx/parser_cyclonedx.go

Lines 285 to 309 in 4dc7c94

	for _, deps := range *c.cdxBom.Dependencies {
	currPkg, found := c.packagePackages[deps.Ref]
	if !found {
	continue
	}
	if reflect.DeepEqual(currPkg, toplevel) {
	continue
	}
	if deps.Dependencies != nil {
	for _, depPkg := range *deps.Dependencies {
	if depPkg, exist := c.packagePackages[depPkg]; exist {
	for _, packNode := range currPkg {
	p, err := common.GetIsDep(packNode, depPkg, []*model.PkgInputSpec{}, "CDX BOM Dependency")
	if err != nil {
	logger.Errorf("error generating CycloneDX edge %v", err)
	continue
	}
	if p != nil {
	preds.IsDependency = append(preds.IsDependency, *p)
	}
	}
	}
	}
	}
	}

In that case you will have to remove this lines:

if reflect.DeepEqual(currPkg, toplevel) {
	continue
}

as we dont want to skip.

If c.cdxBom.Dependencies is null then keep doing https://github.com/guacsec/guac/blob/main/pkg/ingestor/parser/cyclonedx/parser_cyclonedx.go#L260 like it is currently.

[Yaxhveer]

Thanks just wanted to confirm that.