Schema 1.4 support about cyclonedx-rust-cargo HOT 14 CLOSED

Yes, thank you @amy-keibler . I follow your reasoning. I'm heading now into this direction. Meanwhile I completed the comparison of the 1.3 to the 1.4 specification. I'll start with the vulnerabilities as this is the biggest change with new objects, references and stuff.

I hope I'm able to send a PR for review soon. It won't be the full 1.4 implementation but enough to discuss the next steps and required refactoring.

from cyclonedx-rust-cargo.

Thanks for your question! Schema support for 1.4 is planned after #68. Right now, we've been working through documenting the existing code to make it easier to people to use the library for their own projects. Additionally, I have been hoping to get some time to experiment with creating a macro crate for all of the XML code before I start working on 1.4, to reduce some boilerplate. All of the high-level Rust XML crates I've found make assumptions that don't match our use-case (the most common one being the need to fail to parse a document if an element is missing, rather than using Default to populate a value). I will probably bump that till after 1.4 support at this point.

from cyclonedx-rust-cargo.

I'm looking forward to schema support for 1.4, especially because it supports vulnerabilities in the SBOM. I'm also willing to contribute the support for the model "vulnerabilities" but I'm not sure if this is already in progress.

Let me know if I may help with a PR.

from cyclonedx-rust-cargo.

I went through the code of cyclonedx-bom and found specs and tests prepared to support another version. However, regarding the models I'm not sure how to add another version. For example in models::bom there are fn such as output_as_json_v1_3() which could easily complemented by output_as_json_v1_4(). But in this modul there is also "impl Default for Bom" without a version identifier.

So, what is the way to go? Will we see a Bom_1_3 and a Bom_1_4 in models::bom or is it preferred to split the models as well?

from cyclonedx-rust-cargo.

@tokcum thanks for offering to help with this effort. I have been rather busy at the moment, so sorry for the slower than usual response times.

So, what is the way to go? Will we see a Bom_1_3 and a Bom_1_4 in models::bom or is it preferred to split the models as well?

The intent is to keep the models::* as a single representative model for CycloneDX schemas, rather than splitting them by version. When the schema versions differ in complicated ways, the design I had in mind was for the models::* structs/enums/etc. to match the most recent schema and then have a best-effort translation backwards to the previous schemas (the example I had in mind were to error if there was an enum variant that wasn't valid in an older schema when outputting that version). Does that answer your question?

from cyclonedx-rust-cargo.

Does that answer your question?

Yes, thank you @amy-keibler. I'll head into this direction in the next days. Let's see how it this works out for me. :)

When I looked at the tests, I came across the .snap files. From the test code I figure you use cargo insta to generate those and run the tests. I assume we stick with this approach for 1.4 as well. Correct?

from cyclonedx-rust-cargo.

Thank you!

When I looked at the tests, I came across the .snap files. From the test code I figure you use cargo insta to generate those and run the tests. I assume we stick with this approach for 1.4 as well. Correct?

That's correct. We also have some schema conformance tests under tests/data/1.3/ (sourced from the specification repository that we'll want to replicate for 1.4, but I can take care of that after the initial effort.

from cyclonedx-rust-cargo.

@amy-keibler I started to compare the specification of 1.3 to 1.4 and would like to clarify with an example if I got you correctly.

In 1.4 there is one new element at the Bom Level: Vulnerabilities. You said we should keep models::* generic across versions while following a best-effort approach to translate to former versions of the spec.

So we have to add a field to the Bom in models::bom to support 1.4. However, this field should only be allowed in 1.4. So for 1.3 we have to make sure we do not accept and throw an error when serializing / deserializing a 1.3 Bom. Is this what you had in mind?

pub struct Bom {
    pub version: u32,
    pub serial_number: Option<UrnUuid>,
    // [...] all fields identical in 1.3 and 1.4
    pub vulnerabilities: Option<Vulnerabilities>, // new in 1.4
}

Of course his is a very simple example. There might be more complicated differences along the way. I just started to compare the specs ... anyway I want to avoid to head into the wrong direction. As soon as I get the first things right I'll become more confident.

Thanks for your guidance.

from cyclonedx-rust-cargo.

This is my actual directory structure.

.
├── src
│   ├── external_models
│   ├── models
│   └── specs
│       ├── v1_3
│       │   └── snapshots
│       └── v1_4
│           └── snapshots
└── tests
    ├── data
    │   ├── 1.3
    │   │   └── not_yet_supported
    │   └── 1.4
    │       └── not_yet_supported
    └── snapshots
        ├── 1.3
        └── 1.4

And after adding Vulnerabilities to models::bom::Bom and spec v1_4, I'm now thinking about what to do with spec v1_3. I've the impression that it would be good not to alter the v1_3 spec.

error[E0063]: missing field `vulnerabilities` in initializer of `models::bom::Bom`
  --> cyclonedx-bom/src/specs/v1_3/bom.rs:82:9
   |
82 |         Self {
   |         ^^^^ missing `vulnerabilities`

error[E0063]: missing field `vulnerabilities` in initializer of `models::bom::Bom`
   --> cyclonedx-bom/src/specs/v1_3/bom.rs:385:9
    |
385 |         models::bom::Bom {
    |         ^^^^^^^^^^^^^^^^ missing `vulnerabilities`

from cyclonedx-rust-cargo.

Good question. So my thought is that the code under v1_3 should not be changed, except where it creates the models::<x> value. For the vulnerabilities field, my thought is that it would be vulnerabilites: Option<Vulnerabilities> on the models::bom::Bom, so the errors above would be updated to have vulnerabilities: None in the deserialization code and not output anything when serializing for that field.

For serializing, we could consider trace logging that some data was not output when you are serializing to an older version of the specification (if there were vulnerabilities present, for example). That could be done as a follow-up ticket and isn't necessarily something that would hold up merging an initial version of 1.4

Does that answer your questions?

from cyclonedx-rust-cargo.

Thank you @amy-keibler. Hmm, I'm not sure, if I understand correctly: this results in having "Vulnerabilities" in v1_3 spec although v1_3 doesn't support vulnerabilities?

When we take into consideration that there might be more subtle diffs in the specs as well as some more versions of the spec in future, isn't introducing new features to lower spec versions confusing and negatively affecting maintainability? Anytime there is a new spec some or all old specs will have to be adopted as well.

Not sure about this point. Maybe I got you wrong or miss something.

Meanwhile I created an enum in models::bom::Bom to support both specs without changing the specs as a PoC. However, I've the impression that this approach might be the opposite of what you had in mind?!

This approach should allow us to implement spec v1_4, while leaving spec v1_3 untouched.

pub enum Bom {
    BomV1_3(BomV1_3),
    BomV1_4(BomV1_4),
}

I managed to get all tests running except the doc test in lib.rs. I would like to take the opportunity to discuss the interface to the user when taking the approach described above.

let bom = Bom {
     serial_number: Some(
         UrnUuid::new("urn:uuid:3e671687-395b-41f5-a30f-a58921a69b79".to_string())
             .expect("Failed to create UrnUuid"),
     ),
     metadata: Some(Metadata {
         tools: Some(Tools(vec![Tool {
             name: Some(NormalizedString::new("my_tool")),
             ..Tool::default()
         }])),
         ..Metadata::default()
     }),
     ..Bom::default()
};

This won't work anymore as Bom is now an enum. Maybe we can trick this in the prelude where we set the latest spec version to be exported as Bom? This might even be configurable when adding the lib to Cargo.toml, e. g. default_version = 1.4. This are just initial thoughts. Let me know what you think. Thanks.

P. S. No worries, if you dislike this, just tell me. I can stand it. :)

from cyclonedx-rust-cargo.

Hmm, I'm not sure, if I understand correctly: this results in having "Vulnerabilities" in v1_3 spec although v1_3 doesn't support vulnerabilities?

So the changes in the v1_3 spec would be limited to where it interacts with the models equivalent. For Bom, this would be

• Adding vulnerabilites: None to the Self declaration in the impl From<Bom> for models::bom::Bom trait implementation
• Adding to vulnerabilites: None to the corresponding_internal_model test data function

This is slightly awkward in that the files technically change, but there is no impact on what JSON/XML input is accepted or rejected. I chose this approach over having an enum in models, because the user experience is better for a person making use of the cyclonedx-bom library. I should probably create a diagram and design docs now that GitHub supports mermaid diagrams, but the short version is that the user experience I wanted to provide looked a bit like:

stateDiagram-v2
    deserialization: BOM of a specific format (e.g. v3)
    note right of deserialization
      Deserialized from JSON or XML
      into a model specific to the specification
      version (not accessible by library users)
    end note
    [*] --> deserialization: serde's Deserialize / FromXmlDocument
    internalModel: Internal model of the BOM
    note right of internalModel
      Version-independent representation
      of the CycloneDX BOM (intended
      to be used by library users to create
      or modify BOM documents)
    end note
    serialization: BOM of a specific format (e.g. v4)
    note left of serialization
      Serialized to JSON or XML from a
      model specific to the specification
      version (not accessible by library users)
    end note
    deserialization --> internalModel: From
    internalModel --> serialization: From
    serialization --> [*]: serde's Serialize / ToXml

My intent of having the models be a single set of types that the user can modify directly. This enables a workflow where a tool pulls in documents conforming to old versions of the specification, augments them with vulnerability information, and then outputs the latest specification version with all of the original data + what was added. Having the user manage the conversion between different versions felt more cumbersome than pushing that responsibility to the outer edges in the code that is pub(crate) visibility.

Does that answer your question?

from cyclonedx-rust-cargo.

Hey @amy-keibler, while working on the support for vulnerabilities in a bom according to the spec, I found that the rating uses a float to represent the score. In the JSON spec its just called "number". In the XML spec it's a xs:decimal, see https://cyclonedx.org/docs/1.4/xml/#type_ratingType. In fact vulnerability scores are often numbers like 9.8.

The thing is, if I use a float (f32), it's not possible to derive Eq anymore because Eq is not implemented for f32. This affects the VulnerabilityRating and propagates via Vulnerability to the Bom.

Could we just dismiss the derive Eq for all those structs? I'm not sure about the implications of such a change. Please let me know your thoughts. Thanks.

P. S. This is the last obstacle to get the vulnerability support compiled. As soon as this is clarified, I'll start with making the tests work.

from cyclonedx-rust-cargo.

v1.4 support has shipped in cyclonedx-bom = "0.5.0"

from cyclonedx-rust-cargo.