search

CycloneDX / specification

OWASP CycloneDX is a full-stack Bill of Materials (BOM) standard that provides advanced supply chain capabilities for cyber risk reduction. SBOM, SaaSBOM, HBOM, AI/ML-BOM, CBOM, OBOM, MBOM, VDR, and VEX
https://cyclonedx.org/
Apache License 2.0
362 stars 57 forks source link

Promote and enhance vulnerability schema #38

Closed garethr closed 2 years ago

garethr commented 4 years ago

Thanks for creating the JSON Schema variant of the vulnerability extension. This prompted me to take a run at describing some real world data using the format. Here's the full example https://gist.github.com/garethr/7dcc9d6ef4e7cc497e018dc279c00123 (bias warning, I work for Snyk, so I used Snyk in the comparison. I don't think any of the following is Snyk specific though, more about the general complexity of the domain and general usage of the output.)

Here are my observations from that experiment. Happy to discuss these here in one long thread or break out elsewhere if easier. Posting partly to discover the best way to discuss and appetite for addressing issues agreed after discussion.

  1. No fields are required. This makes the life of consumers harder, should there be a minimum set of fields? (id, description?)
  2. The score is always base/impact/exploitability, and always 0-10, so only compatible with CVSS
  3. A vulnerability refers to a BOM ref, which I think means if multiple components have the same vulnerability then it needs to be included multiple times rather than via reference?
  4. No provision for multiple vulnerability identifiers for the same vulnerability, ie. CVE-2009-5155 is also SNYK-DEBIAN9-GLIBC-338103
  5. Although method on ratings can be set to "Other", there is no way of specifying what method/source the information is coming from. So you could have multiple other sources, but no way for the user to trace those back to that source based on data
  6. In the case of multiple ratings with the same method, but different scores/vectors, what are the heuristics? Is this valid? Should the spec prohibit this?
  7. Additional properties are allowed as per the JSON Schema. Is this purposeful? What is the expected parser behaviour for attributes outside the schema? Should there be a formal extension mechanism?
  8. The description is a string. Are there are rules here, ie. SARIF has plain text and markdown properties explicitly, rather than leaving up to the producer
  9. Are advisories intended to always be URLs, or can they be arbitrary strings? The schema says the latter, although all the examples are the former. This would impact someone writing a parser.

ie. Should advisories be defined as:

{
    "type": "string",
    "format": "uri",
    "pattern": "^(https?|http?)://"
}

Hopefully the above is useful feedback, rather than being seen as too much criticism. Some might be me being new to the spec and missing things.

I'd be happy to help, and to discuss further if useful.

coderpatros commented 4 years ago

Thanks for the feedback @garethr, it is very much appreciated!

stevespringett commented 4 years ago

Thanks for the critique @garethr. Very much appreciated. CycloneDX extensions are designed to be iterated on, much faster than traditional standards process. So conceivably, we could use your critique when drafting v1.1 or 2.0 of the vulnerability extension.

Some of the above bullets above are intentional, others are shortcomings, so there's a lot of possibility for improvement.

  1. No fields are required. This makes the life of consumers harder, should there be a minimum set of fields? (id, description?)

There are some sources of vulnerability intelligence that have very minimal data. Friends of PHP being a prime example of this. For example: https://github.com/FriendsOfPHP/security-advisories/blob/master/adodb/adodb-php/2018-03-06.yaml. Requiring an ID or description would prevent this vulnerability from being represented in a BOM.

  1. The score is always base/impact/exploitability, and always 0-10, so only compatible with CVSS

True. CVSS is less than ideal but also (mostly) universally used. Does Snyk use something different or is there a different spec you'd like to see here?

  1. A vulnerability refers to a BOM ref, which I think means if multiple components have the same vulnerability then it needs to be included multiple times rather than via reference?

That is correct and by design. A one-to-one relation between vuln and component means that the vulnerability can contain scoring specific to the component that is affected. For example, if a vuln has a CVE score defined, a human auditor can then apply an OWASP (or some other) score that provides additional risk-based analysis for a specific occurrence of a vulnerability. If multiple components could be affected by the same vulnerability, then all implementations of CycloneDX would need to do field-level comparison to ensure per-instance decisions were not made prior to using a reference. This is a lot of additional work implementations would need to do.

However, with that said, I think there's room for improvement here and welcome suggesitons.

  1. No provision for multiple vulnerability identifiers for the same vulnerability, ie. CVE-2009-5155 is also SNYK-DEBIAN9-GLIBC-338103

That is correct and by design. A vulnerability is currently defined as having a single source. So the source for CVE-2009-5155 would be the NVD, whereas the source for SNYK-DEBIAN9-GLIBC-338103 would be Snyk.

What would be interesting is to expand this definition so that a vulnerability could have multiple ID (and thus multiple sources). That would be a really great enhancement IMO.

  1. Although method on ratings can be set to "Other", there is no way of specifying what method/source the information is coming from. So you could have multiple other sources, but no way for the user to trace those back to that source based on data

I agree. This needs improvement.

  1. In the case of multiple ratings with the same method, but different scores/vectors, what are the heuristics? Is this valid? Should the spec prohibit this?

No, VulnDB and Sonatype are classic examples of this. Each service will provide the NVD score but in many cases will also provide their own CVSS score. What could be added is identity on who actually performed the score and then limit based on that. So prohibiting multiple scores from the NVD for example.

  1. Additional properties are allowed as per the JSON Schema. Is this purposeful? What is the expected parser behaviour for attributes outside the schema? Should there be a formal extension mechanism?

Yes its purposeful. XML has namespaces and CycloneDX allows (in many cases) arbitrary elements anywhere in the spec as long as they're in a different namespace. The CycloneDX JSON schema itself allows arbitrary properties anywhere as well. The proposal for the vuln extension in JSON is to be consistent with everything else. We find that many organizations use this capability to add their own flavor to CycloneDX without having to violate the spec.

  1. The description is a string. Are there are rules here, ie. SARIF has plain text and markdown properties explicitly, rather than leaving up to the producer.

I wish there were rules. The NVD for example is all strings. NPM Advisories can either be strings or markdown and they don't tell you. It's up to the consumer of their APIs to identify it prior to use. I think there's room for improvement here and welcome suggestions.

  1. Are advisories intended to always be URLs, or can they be arbitrary strings? The schema says the latter, although all the examples are the former. This would impact someone writing a parser.

Currently strings as you pointed out, but I think the spec needs to be more prescriptive in what gets put there. So yes, I think extending the advisories to include perhaps a title, description, and url instead of a single description field would be ideal.

Are there any blockers you see that would prevent Snyk from implementing CycloneDX with or without the vulnerability extension?

Are there any improvements you'd like to see made to the vulnerability extension prior to adoption?

Again, thanks for the feedback. Looking forward to iterating on this with you.

coderpatros commented 4 years ago
  1. The score is always base/impact/exploitability, and always 0-10, so only compatible with CVSS

    Does Snyk use something different or is there a different spec you'd like to see here?

Some flexibility and generic approach here might be good for supporting different scoring methods. But that could make automation harder for consumers. Maybe a decent approach is to have vulnerability scoring extensions? Then we could get really specific with different scoring methods, especially industry/vendor specific ones, without adding complexity to the base spec.

  1. A vulnerability refers to a BOM ref, which I think means if multiple components have the same vulnerability then it needs to be included multiple times rather than via reference?

    That is correct and by design.

I think, in the context of an SBOM, a vulnerability needs to be thought of as component vulnerability information.

  1. No provision for multiple vulnerability identifiers for the same vulnerability, ie. CVE-2009-5155 is also SNYK-DEBIAN9-GLIBC-338103

    What would be interesting is to expand this definition so that a vulnerability could have multiple ID (and thus multiple sources). That would be a really great enhancement IMO.

I sort of disagree on this. I think component vulnerability information here should have a clear identifiable single source. But support multiple reference IDs for cross referencing.

  1. Although method on ratings can be set to "Other", there is no way of specifying what method/source the information is coming from. So you could have multiple other sources, but no way for the user to trace those back to that source based on data

And perhaps just as problematic, it makes automation hard and won't scale. Perhaps this should be deprecated and replaced with vulnerability scoring extensions?

  1. Are advisories intended to always be URLs, or can they be arbitrary strings? The schema says the latter, although all the examples are the former. This would impact someone writing a parser.

    Currently strings as you pointed out, but I think the spec needs to be more prescriptive in what gets put there. So yes, I think extending the advisories to include perhaps a title, description, and url instead of a single description field would be ideal.

I think if we are going to include description/content it should use the attachedTextType which allows specifying a content type.

But I think advisories should probably just be an externalReference for efficiency and consistency.

garethr commented 4 years ago

Thanks for the responses.

That is correct and by design. A one-to-one relation between vuln and component means that the vulnerability can contain scoring specific to the component that is affected. For example, if a vuln has a CVE score defined, a human auditor can then apply an OWASP (or some other) score that provides additional risk-based analysis for a specific occurrence of a vulnerability. If multiple components could be affected by the same vulnerability, then all implementations of CycloneDX would need to do field-level comparison to ensure per-instance decisions were not made prior to using a reference. This is a lot of additional work implementations would need to do.

My concern here was mainly about the potential for duplication and extra parsing, and larger SBOM file sizes. Scores being different for different vulnerability instances is useful, but I'd wager 99% of the time the advisories, descriptions and source information would be identical. In some ecosystems this duplication can be ~5x or more. SARIF for instance has rules (1) and results (n) which support this use case without duplication.

That is correct and by design. A vulnerability is currently defined as having a single source. So the source for CVE-2009-5155 would be the NVD, whereas the source for SNYK-DEBIAN9-GLIBC-338103 would be Snyk. What would be interesting is to expand this definition so that a vulnerability could have multiple ID (and thus multiple sources). That would be a really great enhancement IMO.

I sort of disagree on this. I think component vulnerability information here should have a clear identifiable single source. But support multiple reference IDs for cross referencing.

My concern here is that as a producer you either need to choose to include only one ID (thereby limiting the information available to the consumer) or include several entries. This would further exacerbate the issue above with regards duplication information. Worse, any counts of instances of vulnerabilities would be incorrect.

Having a canonical ID makes sense, but I think having a field for additional Identifiers would be useful.

What could be added is identity on who actually performed the score and then limit based on that. So prohibiting multiple scores from the NVD for example.

Agreed. I think this solves for both 5 & 6. Method is parallel to who provides the score. eg.

"ratings": [
      {
        "score": {
          "base": 9,
          "impact": 5.9,
          "exploitability": 3.0 
        },
        "severity": "Medium",
        "source": "NVD",
        "method": "CVSSv3",
        "vector": "CVSS:3.0/AV:L/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
      },
      {
        "severity": "None",
        "source": "Debian Security Team",
        "method": "Other"
      },
      {
        "severity": "Low",
        "Source": "Snyk",
        "method": "Other"
      }
    ],

But I think advisories should probably just be an externalReference for efficiency and consistency.

+1

coderpatros commented 3 years ago

I think we also need to create some guidance on how to represent vulnerability information, and in particular recommendations.

A component might have a vulnerability that is resolved in the next version of that component. But at the assembled software top level it is resolved by upgrading to the next version of that software. Or perhaps that vulnerability is only exploitable under certain conditions, or not at all.

coderpatros commented 3 years ago

A suggestion. Maybe we should differentiate between raw vulnerability information and vulnerability assessment information?

They are, in my opinion, quite different things. One has no context whatsoever about the software in question. Where as the other can have much more value when triaging an issue and remediation.

garethr commented 3 years ago

@coderpatros I think that distinction is useful here. The SARIF rules/results is a similar idea:

It allows you to answer both:

You can also reason about things like:

At the very small end it's a bit more complex, but I think in real-world cases it's invariably going to be a better fit.

talz commented 3 years ago

I'll add, that IMHO, a real problem that might arise from a real world use of this, by a reader of the report – regards how to select which version to upgrade to, affecting only part of the vulnerabilities. (For example, I want to fix only those with CVSS score higher than 8.0, and I prefer to find closest version)

So enforcing a schema for version specification in rules might also work well. Something along the lines of CPE and/or schema.

Another idea I feel could be relevant regards separating raw vulnerability information and vulnerability assessment information.

This is something we currently implement in our product, and find very informative.

We currently focus on the following questions:

stevespringett commented 3 years ago

I really want to avoid concluding whether or not something is/is not exploitable in the spec. CycloneDX strives to be "facts first" so any solution that aligns with that ideal is great.

@garethr do you have ideas for proposals to support the good ideas you've pointed out in https://github.com/CycloneDX/specification/issues/38#issuecomment-716155173?

stevespringett commented 3 years ago

@talz CycloneDX supports vulnerability remediation, including backports, patches, commits, etc. This is part of a components pedigree. Refer to https://cyclonedx.org/use-cases/#pedigree

Is the vulnerability reachable?

This is an area with a high probability of false negatives and a topic of the NTIA VEX subgroup. False negatives are common when there's a lack of rules that can accurately describe what's possible, and is especially problematic in non-standard (unknown libraries, frameworks, and design patterns) and ployglot environments. This is extremely difficult to communicate accurately.

Is the vulnerable code in the library even called by the user of the library?

Under normal operation, this may be able to be determined depending on the capabilities of the dataflow engine, rules, monoglot vs polyglot, but ignores pivot points within a vulnerable application and entry points outside the scope of what's being analyzed. This is extremely difficult to communicate accurately.

Both of the above questions also are limited in scope as they do not take into consideration the full stack. SBOMs are typically provided with distributables such as end-user software, IoT devices, medical devices, automobile assemblies, and consumer electronics, all of which have (in many cases) multiple software stacks and each stack comprising of hardware, firmware, possibly an OS layer, applications and libraries.

The reachability of vulnerable code is most accurate through a combination of automated and manual analysis that will eventually end with a weighted analysis decision, along with all the evidence (and confidence of each piece of evidence) which led to the analysis decision.

IMO, I think this is valuable but its outside of the scope of CycloneDX Core and is currently not planned for the Vulnerability extension.

CycloneDX supports remediation (natively via component pedigree) and disclosure (via the vulnerability extension). There are currently no plans to support the opinion of auditors and/or tools to determine the exploitability of vulnerabilities or the effectiveness of patches or mitigating controls.

The NTIA VEX subgroup asked the Core team to present during one of their recent meetings. Attached is the presentation from that. I think it may highlight what the project does with regard to vulnerability information and what it does not.

CycloneDX VEX.pdf

That said, any organization or industry can create schema extensions that are applicable to their use case. This allow others to use the CycloneDX Core specification and built on top of it to meet their needs.

stevespringett commented 3 years ago

@talz Can you elaborate on this?

So enforcing a schema for version specification in rules might also work well. Something along the lines of CPE and/or schema.

I think I understand what you mean, but if you could elaborate, that would be great.

talz commented 3 years ago

Hey, in broad strokes - I'm talking about a enforcing a versioning schema, that'll allow a user to have control on comparing versions of different BOM entries.

That is, breaking down version strings to their respective components: CPE uses something along the lines of <version>:<update>:<edition>:<language>

I think one can also break version part itself to something more canonical: <major>:<minor>:<patch>

garethr commented 3 years ago

Versioning is a nightmare at scale. The number of different versioning schemes is huge, and lots of them aren't distinguishable from just reading the version string.

I think recommending, or providing guidance on how to use purl is worthwhile https://github.com/package-url/purl-spec (also from @stevespringett). Which solves your formatting problem you describe. But purl right punts on specifying the version in any rigid way

A version is a plain and opaque string. Some package types use versioning conventions such as semver for NPMs or nevra conventions for RPMS. A type may define a procedure to compare and sort versions, but there is no reliable and uniform way to do such comparison consistently.

That makes version comparison a downstream problem unfortunately, or you limit the utility of the spec. It's not enough to say "CycloneDX versions should be semver" either. Because you can have versions that look like semver that aren't semver, so you can validate the output in a meaningful way that would mean a consumer could assume semver comparisons.

garethr commented 3 years ago

@garethr do you have ideas for proposals to support the good ideas you've pointed out in #38 (comment)?

@stevespringett when I get a moment I'm going to try and take the conversations in this thread and post a super-early proposal that we can kick around. Timing might not be for a week or two, in case someone else starts off.

garethr commented 3 years ago

Finally found a moment. Draft PR in https://github.com/CycloneDX/specification/pull/44. Still WIP but wanted to start gathering feedback.

garethr commented 3 years ago

I've created a gist to test some of these ideas out as well https://gist.github.com/garethr/b069d9bf84ca80635cd506e74c8e2247

The main observation at the moment is the overlap between the concepts of sources and ratings, when both become arrays. I think merging these might be useful. ie. in many (but not all) cases sources of vulnerabilities (NVD, Debian, security vendors) are also likely sources of ratings. Thoughts?

So from something like:

"sources": [
  {
    "name": "NVD",
    "url": "https://nvd.nist.gov/vuln/detail/CVE-2010-0928"
  }
],
"ratings": [
  {
    "score": {
      "base": 9.8,
      "impact": 5.9,
      "exploitability": 3.0 
   },
   "severity": "Medium",
   "source": "NVD",
   "method": "CVSSv3",
   "vector": "CVSS:3.0/AV:L/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
  }
]

To something like:

"sources": [
  {
    "score": {
      "base": 9.8,
      "impact": 5.9,
      "exploitability": 3.0 
   },
   "severity": "Medium",
   "source": "NVD",
   "url": "https://nvd.nist.gov/vuln/detail/CVE-2010-0928",
   "method": "CVSSv3",
   "vector": "CVSS:3.0/AV:L/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N"
  }
]
coderpatros commented 3 years ago

I like how you've tied the score to the source in that second example.

ddillard commented 3 years ago

A few additional comments:

garethr commented 3 years ago

@ddillard For multiple scores, and for specifying the source of the score, the proposed changes add that capabilities. https://github.com/CycloneDX/specification/pull/44

Adding a comment field to the rating makes sense to me. What sort of things would you envisage adding in there?

ddillard commented 3 years ago

@garethr An explanation of why the scoring was done the way it was. Red Hat does this occasionally in their security advisories. For example https://access.redhat.com/security/cve/CVE-2020-15862, you'll see a box with the title "Why did Red Hat score this CVE differently?" Right above that they highlight the differences in the access vector between the default scoring and their scoring, but that explanation helps provide additional information and context. That's all I'm looking for.

stevespringett commented 3 years ago

The existing vulnerability extension has undergone real-world tests and has proven to provide value for audit use cases, machine-to-machine transport of BOM+vuln info, as well as output from various SCA and container tools used in production environments.

Based on what has been learned over the past two years, I propose to bring the vulnerability extension into the core spec along with several improvements that will provide richer data. By doing so, vulnerability info will be available across all serialization formats.

stevespringett commented 3 years ago

@garethr I'm going to use the standalone JSON schema extension you created earlier as a starting point and start moving it into the core spec along with some changes. From there, I plan to create a draft PR that we can discuss and once we get buy-in, we can port it to the XML and protobuf schemas. At that point, it would go through our documented standardization process just like all the other tickets we're working on for v1.4.

stevespringett commented 2 years ago

Closing - as this is being released in CycloneDX v1.4 next week.