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 support for recording multiple CPEs in cyclonedx #819

Closed
wants to merge 1 commit into from

Conversation

sambhav
Copy link
Contributor

@sambhav sambhav commented Feb 13, 2022

Fixes #818

For reasoning and logic behind the CPE serialization decision please take a look at the issue. We still store our most specific CPE in the cyclonedx CPE field but for all the other CPEs, we store them as external references.

Comment on lines +112 to +114
assert.Equal(t, CPEURI(c2), test.CPEUrl)
assert.Equal(t, CPEURI(c1), test.CPEUrl)
assert.Equal(t, must(NewCPE(CPEURI(c1))), test.WFN)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ensures that we are also making our URL generation also go through the same rigorous CPE encoding/decoding test that our CPE strings go through.

Comment on lines +104 to +113
output.Vendor = sanitize(c.Vendor)
output.Product = sanitize(c.Product)
output.Language = sanitize(c.Language)
output.Version = sanitize(c.Version)
output.TargetSW = sanitize(c.TargetSW)
output.Part = sanitize(c.Part)
output.Edition = sanitize(c.Edition)
output.Other = sanitize(c.Other)
output.SWEdition = sanitize(c.SWEdition)
output.TargetHW = sanitize(c.TargetHW)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mimics our custom CPEString logic above to handle special characters in the CPE.

@sambhav sambhav mentioned this pull request Feb 13, 2022
@sambhav
Copy link
Contributor Author

sambhav commented Feb 13, 2022

cc: @coderpatros & @stevespringett to validate if this seems like a reasonable usage of the cyclonedx spec. to get around accurate CPE determination limitation that scanning tools face. Detailed reasoning at #818

@stevespringett
Copy link

I think using properties is more appropriate. External references in CycloneDX are typically URIs to other resources. This varies a lot from SPDX which applies the external reference name to mean potential alternative identifiers.

We have a formal property taxonomy as well. https://github.com/CycloneDX/cyclonedx-property-taxonomy

We could register anchore as a top-level namespace if desired?

@sambhav
Copy link
Contributor Author

sambhav commented Feb 14, 2022

NOTE: The PR should first be reviewed by syft maintainers before any further action is taken -> CycloneDX/cyclonedx-property-taxonomy#8

Thanks for the feedback, I have opened a PR to register a syft namespace since we are also using cyclonedx properties for storing other fields.

cc: @luhring, @wagoodman, @spiffcs, @kzantow please take a look. My reasoning for choosing syft over anchore was in case there was ever some movement in the future on ossf/tac#70. The choice of a namespace like syft ensures that the namespace is easily identifiable and closely associated to syft while still being impervious to such changes.

@sambhav
Copy link
Contributor Author

sambhav commented Feb 14, 2022

Separately it looks like there is an spdx taxonomy in work as well which contains the CPE field at CycloneDX/cyclonedx-property-taxonomy#7 which might also be suitable for storing this and other SPDX fields. This might also be useful with reference to #563, #723 and #737

@kzantow
Copy link
Contributor

kzantow commented Mar 1, 2022

@samj1912 since #849 has been merged, which outputs syft: namespace'd CPEs, I'm going to close this one; please reopen if you disagree!

@kzantow kzantow closed this Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for multiple CPEs in CycloneDX
3 participants