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

validation of cyclonedx sboms went south since 4.11 #3759

Closed
2 tasks done
MangoIV opened this issue May 27, 2024 · 12 comments
Closed
2 tasks done

validation of cyclonedx sboms went south since 4.11 #3759

MangoIV opened this issue May 27, 2024 · 12 comments
Labels
wontfix This will not be worked on

Comments

@MangoIV
Copy link
Contributor

MangoIV commented May 27, 2024

Current Behavior

After also encountering the bug wrt the incorrect deserialization of the version field and upgrading to 4.11.1, we now have a couple more errors in an sbom that was correct before upgrading to 4.11 and is shown correct by sbomqs, there's a lot more, but I just wanted to demonstrate with one of the errors:

  1. $.components[0].licenses: should be valid to one and only one schema, but 0 are valid
   λ cat sbom.json | jq '.components.[0].licenses'
[
  {
    "license": {
      "id": "MIT",
      "name": "MIT License"
    }
  }
]

(from https://cyclonedx.org/docs/1.5/json/)
image

sbomqs output:

λ sbomqs score sbom.json
SBOM Quality Score:7.3	components:521	sbom.json
+-----------------------+--------------------------------+-----------+--------------------------------+
|       CATEGORY        |            FEATURE             |   SCORE   |              DESC              |
+-----------------------+--------------------------------+-----------+--------------------------------+
| NTIA-minimum-elements | comp_with_name                 | 10.0/10.0 | 521/521 have names             |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_supplier             | 0.0/10.0  | 0/521 have supplier names      |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_uniq_ids             | 10.0/10.0 | 521/521 have unique ID's       |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_version              | 9.7/10.0  | 503/521 have versions          |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_authors                   | 0.0/10.0  | doc has 0 authors              |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_creation_timestamp        | 10.0/10.0 | doc has creation timestamp     |
|                       |                                |           | 2024-05-27 14:02:23.402773459  |
|                       |                                |           | UTC                            |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_dependencies              | 10.0/10.0 | doc has 21011 relationships    |
+-----------------------+--------------------------------+-----------+--------------------------------+
| Quality               | comp_valid_licenses            | 9.3/10.0  | 487/521 components with valid  |
|                       |                                |           | license                        |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_any_vuln_lookup_id   | 10.0/10.0 | 520/521 components have any    |
|                       |                                |           | lookup id                      |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_deprecated_licenses  | 10.0/10.0 | 2/521 components have          |
|                       |                                |           | deprecated licenses            |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_multi_vuln_lookup_id | 0.0/10.0  | 0/521 components have multiple |
|                       |                                |           | lookup id                      |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_primary_purpose      | 9.8/10.0  | 508/521 components have        |
|                       |                                |           | primary purpose specified      |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_restrictive_licenses | 10.0/10.0 | 0/521 components have          |
|                       |                                |           | restricted licenses            |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_with_creator_and_version  | 0.0/10.0  | 0/0 tools have creator and     |
|                       |                                |           | version                        |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_with_primary_component    | 10.0/10.0 | primary component found        |
+-----------------------+--------------------------------+-----------+--------------------------------+
| Semantic              | comp_with_checksums            | 0.0/10.0  | 0/521 have checksums           |
+                       +--------------------------------+-----------+--------------------------------+
|                       | comp_with_licenses             | 9.3/10.0  | 487/521 have licenses          |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_required_fields           | 9.9/10.0  | Doc Fields:true Pkg            |
|                       |                                |           | Fields:false                   |
+-----------------------+--------------------------------+-----------+--------------------------------+
| Sharing               | sbom_sharable                  | 0.0/10.0  | doc has a sharable license     |
|                       |                                |           | free 0 :: of 0                 |
+-----------------------+--------------------------------+-----------+--------------------------------+
| Structural            | sbom_parsable                  | 10.0/10.0 | provided sbom is parsable      |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_spec                      | 10.0/10.0 | provided sbom is in a          |
|                       |                                |           | supported sbom format of       |
|                       |                                |           | spdx,cyclonedx                 |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_spec_file_format          | 10.0/10.0 | provided sbom should be in     |
|                       |                                |           | supported file format for      |
|                       |                                |           | spec: json and version:        |
|                       |                                |           | json,xml                       |
+                       +--------------------------------+-----------+--------------------------------+
|                       | sbom_spec_version              | 10.0/10.0 | provided sbom should be        |
|                       |                                |           | in supported spec version      |
|                       |                                |           | for spec:1.5 and versions:     |
|                       |                                |           | 1.0,1.1,1.2,1.3,1.4,1.5        |
+-----------------------+--------------------------------+-----------+--------------------------------+

Steps to Reproduce

The sbom in question is attached

Uploading sbom.json…

Expected Behavior

The upload of the sbom continues to work

Dependency-Track Version

4.11.1

Dependency-Track Distribution

Container Image

Database Server

PostgreSQL

Database Server Version

No response

Browser

N/A

Checklist

@MangoIV MangoIV added defect Something isn't working in triage labels May 27, 2024
@nscuro
Copy link
Member

nscuro commented May 27, 2024

[...] we now have a couple more errors in an sbom that was correct before upgrading to 4.11 [...]

DT versions prior to v4.11 did not validate against the CycloneDX schema at all, and that caused issues because "obvious" errors would cause the asynchronous BOM processing to fail, leaving the client that uploaded the BOM in the dark.

We now validate against the schema per default, although this can be disabled in the settings as mentioned multiple times in the changelog.

It is indeed possible for a BOM to be parseable, but not valid. Parsers are not good at identifying semantic issues, they may ignore unknown properties, and so on. A BOM being parseable is thus not a good indicator of whether it conforms to the specification.

So I'd argue BOM validation didn't "go south", it's doing its job. The provided BOM is invalid according to the schema, so the tool that generated it needs fixing. The location of the schema you linked says a license can provide either id, or name, but in the BOM it has both.

[...] is shown correct by sbomqs [...]

To my knowledge, sbomqs doesn't validate against the JSON or XML schemas either. If it did, it should fail for the provided BOM.

@nscuro nscuro added wontfix This will not be worked on and removed defect Something isn't working in triage labels May 27, 2024
@MangoIV
Copy link
Contributor Author

MangoIV commented May 27, 2024

afaict it tells "at least one of the id or the name is required", what comes below is optional, I don't know how this contradicts with my json.

@MangoIV
Copy link
Contributor Author

MangoIV commented May 27, 2024

the validation indeed found a bug in the generation, btw, so that's nice, I don't know if that particular one is one though...

@nscuro
Copy link
Member

nscuro commented May 27, 2024

Admittedly, the error message could be clearer, but that is sadly outside of our control since we're using a standard JSON schema validator.

The JSON schema documentation itself is less vague:

image

(The schema for id and name is a oneOf, hence XOR)

@MangoIV
Copy link
Contributor Author

MangoIV commented May 27, 2024

that's not my point though, even after reading the raw schema, I still find it confusing, that required isn't only changing the behavior for requiring the field but also whether it may be included. But I guess we have to trust that the schema validation is implemented correctly? :D

@MangoIV
Copy link
Contributor Author

MangoIV commented May 27, 2024

image
it's also weird that in the schema itself it explicitly talks about required...

@MangoIV
Copy link
Contributor Author

MangoIV commented May 27, 2024

whatever, I will just remove one field, I guess it's good enough without. thank you.

@MangoIV MangoIV closed this as completed May 27, 2024
@nscuro
Copy link
Member

nscuro commented May 27, 2024

But I guess we have to trust that the schema validation is implemented correctly?

Sure, you can always challenge the implementation if you have doubts. Feel free to get the schema(s) and use whatever other validator you can find to compare results: https://github.com/CycloneDX/specification/tree/master/schema

I still find it confusing, that required isn't only changing the behavior for requiring the field but also whether it may be included.

The key here is the XOR behavior of oneOf. If you provide both id and name, the license is valid against both schemas. That would be fine for anyOf (OR), but not oneOf (XOR).

They have an example here as well: https://json-schema.org/understanding-json-schema/reference/combining#oneOf

@MangoIV
Copy link
Contributor Author

MangoIV commented May 27, 2024

yea, seeing it from a purely boolean algebra perspective, it is clear why it goes wrong, it's just weird that it does interact with "required" in the way it does, but I guess that's just the way it is

@riteshnoronha
Copy link

@nscuro @MangoIV yes sbomqs currently does not validate against schema. Its something that should be added as a scoring criteria. interlynk-io/sbomqs#248

@ChristoWolf
Copy link

Hi!

I also had issues with uploading SBOM XMLs produced by CycloneDX, the .NET tool v3.0.6 in particular because Schema validation failed.

After updating the tool to v3.0.7, everything seems to be working again.

Copy link
Contributor

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants