CycloneDX/cyclonedx-go

backward/forward compatibility for Tools (cdx 1.5)

manifestori opened this issue · 11 comments

manifestori commented

Hi :)

I've been following the progress of cdx1.5 support in this library for quite some time now. I've noticed tools such as Trivy already used in production (trivy cli tool).

I wondered about your approach to maintaining compatibility (read/write) for Tools. It's the first time you've had to change the actual field type. Will you use an overriding mechanism, such as implementing UnmarshalJSON and convert old slices into the new model?

The decoding part is much more apparent as convertTool would probably be used for this purpose.

Can I work on this solution? I'd like to see a v1.5 compatible version of this library in the stable release.

nscuro commented

Hey @manifestori, appreciate you reaching out!

The tools change has given me a bit of a headache if I'm being honest. Here are my thoughts:

  • The original Tools was not removed in v1.5, but only deprecated. So even users generating BOMs following v1.5 of the spec should still be able to use it. This means we cannot simply convert legacy tools to tool components etc. when encoding.
  • If we want to stay backward-compatible, the current Metadata.Tools field must stay as it is, which means adding support for the new tool types would require a new field. For lack of a better name, let's call it ToolsNew for now.

What I think should happen:

  1. When encoding for a spec version lower than v1.5, tools specified as components or services will need to be converted to legacy tools (implies potential loss of information).
  2. When encoding, a custom MarshalJSON and MarshalXML for Metadata must check both Tools and ToolsNew. If both are set, that's an error. If only one of them is set, encode its content as tools.
  3. When decoding, a custom UnmarshalJSON and UnmarshalXML for Metadata must inspect the tools node, and populate Tools or ToolsNew accordingly.

I'd love to avoid having to maintain custom marshalling logic for Metadata , so definitely open for other suggestions if you have any! Also, if you'd like to help here, that'd be much appreciated for sure.

  • 👍1
  • 👀1
manifestori commented

We would never be able to approach this without custom logic for MarshalXXX and UnmarshalXXX as Tools cannot be a slice and not a slice at the same time :)

I liked you're approach for going with ToolsNew probably changing current tool to ToolsLegacy and making Tools as the existing spec format.

A different approach would be to change Tools to the new spec. converting older spec versions Tools into the new structure when encoding wouldn't be too hard; those "fake" components would have some fields missing (bom-ref, etc)
when decoding to older spec, we can just convert Tools.Components/Tools.Services back to the list of legacy Tools.

We just need to decide how to convert vendor. (author? publisher? group? supplier?)

nscuro commented

Apologies for the late reply @manifestori.

We just need to decide how to convert vendor. (author? publisher? group? supplier?)

Doesn't seem like vendor can be mapped with 100% accuracy to any of those fields. I think supplier comes the closest, the other fields have a different meaning (selling something [vendor, supplier] doesn't imply authorship or publishing of the sold item).

manifestori commented

Yeah I figured, but that's close enough.

So basically, adding ToolsLegacy for older specs, and use Tools for current spec.

I guess cdx spec won't change Tools again soon ;)

nscuro commented

Yeah that sounds like a good way forward!

I guess cdx spec won't change Tools again soon ;)

Sure hope so...

  • ❤️1
manifestori commented

Are you looking for community contributions or is it already on your roadmap @nscuro?

northdpole commented

hey @nscuro , +1 on the above, is this high on your priority list? otherwise happy to contribute the fix myself

  • 👍1
  • ❤️1
nscuro commented

Thanks @northdpole, appreciate the offer! It would indeed help immensely if we could get this contributed, as I am a bit burried with other stuff. However, if this is critical for you and potentially others, I may be able to re-prioritize.

northdpole commented

it's not critical at the moment as most tools I use support format definition, i'll try and get to this in the next 2 weeks

northdpole commented

sorry folks, I've ran out of time to do this, please don't wait for me

nscuro commented

So the way I've settled on is this:

  • The type of Metadata.Tools and Vulnerability.Tools changes from *[]Tool to *ToolsChoice
  • ToolsChoice holds both "legacy" *[]Tool, as well as the new *[]Component and *[]Service fields
  • The Tool type, as well as the ToolsChoice.Tools field are marked as deprecated
  • During encoding and decoding, it is asserted that only one of both options can be present
  • When encoding to lower spec versions, Components and Services are converted to legacy Tools
  • ❤️1