Demo feedback from NTIA Medical PoC

sei-vsarvepalli commented 3 years ago

From Meeting on July 21, 2021 for NTIA Medical PoC working group:

There were a few items identified by the meeting participants about SwiftBOM that can improve the PoC experience

When CPE is chosen as the input - ensure CPE data is preserved in the generated SBOM in various formats
Improve SPDX output by adding JSON output of SPDX specification
Present CVE data in CSAF or OSV format as appropriate to be appended to the output formats.

kestewart commented 3 years ago

Thanks Vijay. Yes, this matches my understanding. Only point to clarify is that if CPE is chosen input for SPDX, please store it as an External Repository identifier (https://spdx.github.io/spdx-spec/appendix-VI-external-repository-identifiers/) rather than as the SPDXRef-identifier.

sei-vsarvepalli commented 3 years ago

@kestewart

Thanks, the example I found looks like below. Let me know if you see something wrong or incorrect.

ExternalRef: SECURITY cpe23Type cpe:2.3:a:microsoft:sql_server:2005:sp4:express_advanced_services:*:*:*:*:*

and in JSON format

"externalRefs" : [ { "referenceCategory" : "SECURITY", "referenceLocator" : "cpe:2.3:a:microsoft:sql_server:2005:sp4:express_advanced_services:*:*:*:*:*", "referenceType" : "cpe23Type" } ]

sei-vsarvepalli commented 3 years ago

I am also adding @stevespringett to this conversation as the CPE example and the vulnerability example provided by Cyclonedx will be part of the feature request update for this issue.

Vijay

santosomar commented 3 years ago

Hi @sei-vsarvepalli ,

Amazing work on this! BTW, I used your tool to create some demo content from/for Cisco during the last plugfest and loved it! I really like your addition to CSAF/VEX data in the tool (https://democert.org/sbom/).

I just have one question. For the profile, you can certainly use generic_csaf. However, for this use case probably vex may be more appropriate? We added these profiles in CSAF and documented here

Thanks again!

sei-vsarvepalli commented 3 years ago

@santosomar thanks.

I will go with vex for what is being delivered from SwiftBOM. I think the VINCE output in CSAF format is probably more of the security_advisory flavor. The VINCE CSAF output https://democert.org/vince/ (use the VINCE API key to login) is not yet complete. I have not incorporated all of Thomas (@tschmidtb51) feedback as yet.

Vijay

santosomar commented 3 years ago

Thank you 🙏 sounds perfect! Thank you again!!

tschmidtb51 commented 3 years ago

Hi Vijay, thanks for adding CSAF.

I have several comments:

The CSAF document is not valid as the vex profile requires action statements for affected products. (So it fails test 6.1.27.10 Action Statement).
The /vulnerabilities[]/title should be shorter. I would use the first sentence and put the rest in a /vulnerabilities[]/note object. The CVSS Vector statement should be removed from the title / summary as it is present in the scores array.
The product_tree should also list the Infusion device.
The product_status should also state whether the Infusion device is affected (and a remediation => action statement) or not (and why => impact statement).
It would be nice to add the product_identification_helper with:
- sbom_urls if present
- cpe if present
- purl if present
- hashes if present
to help users to find the SBOM information.
Please add a patch_level to the product description as it is indicated in the title.

sei-vsarvepalli commented 3 years ago

Hello @tschmidtb51

Thanks again.

CSAF document includes now the vex profile required remediations action section with a patched product saying it is available.
The /vulnerabilities[]/title has been moved to /vulnerabilities[]/notes[0] with category summary . However the CVE description collected from https://github.com/olbat/nvdcve/blob/master/nvdcve/CVE-2019-2697.json includes the CVSS scores in the body of description JSON. We are just echoing it wysiwg fashion from the CVE JSON, not parsing and removing it.
The product_tree now has all the products. It looks like the product_group concept is not consistently available for easier grouping of multiple product_ids in CSAF spec, was this intentional for some reason?. For e.g., theknown_affected cannot have a group_ids but requires products listed individually. However remediations can accept the group_ids. Perhaps it is worth given the option of groups in CSAF to make this easier.
Infusion is "affected" - a remediation action is now stated in the live site. Is it possible in CSAF to also state that the product is affected because of inheritance/bundling of a vulnerable component and not directly due to a vulnerability. I am not sure how this can be done, if you have suggestions let me know and I will follow it.
The product_identification_helper is also added now using purl identifier. Preferably we can also add signature hashes after some adjustment to the current code.
Not sure if this requires another product to be added to the product_tree as the "patched" product or security fixed product.

See the example below:

SwiftBOM-CSAF-ACME-INFUSION-1-0-v5-2-5-json.txt

tschmidtb51 commented 3 years ago

@sei-vsarvepalli Thanks for addressing the feedback.

CSAF document includes now the vex profile required remediations action section with a patched product saying it is available.

Perfect.

The /vulnerabilities[]/title has been moved to /vulnerabilities[]/notes[0] with category summary. However the CVE description collected from https://github.com/olbat/nvdcve/blob/master/nvdcve/CVE-2019-2697.json includes the CVSS scores in the body of description JSON. We are just echoing it wysiwg fashion from the CVE JSON, not parsing and removing it.

Ok.

The product_tree now has all the products. It looks like the product_group concept is not consistently available for easier grouping of multiple product_ids in CSAF spec, was this intentional for some reason?. For e.g., theknown_affected cannot have a group_ids but requires products listed individually. However remediations can accept the group_ids. Perhaps it is worth given the option of groups in CSAF to make this easier.

This decision was intentionally (and carried over from CVRF). The reason for that is stated in CVRF:

There is a constraint in place to prevent a single product from being assigned two different (conflicting) Status elements within the scope of Vulnerability. Likewise, a Status child container cannot be tied to a Product Group due to the fact that a single product can be a member of more than one product group.

If you think we should add a sentence about that in the standard again, please open an issue at oasis-tcs/csaf. BTW: The constraint is in Test 6.1.6 Contradicting Product Status.

Infusion is "affected" - a remediation action is now stated in the live site. Is it possible in CSAF to also state that the product is affected because of inheritance/bundling of a vulnerable component and not directly due to a vulnerability. I am not sure how this can be done, if you have suggestions let me know and I will follow it.

I would separate the remediations. The current version just gives the information: "There is an update." However, you could do more and e.g. provide a link for each patched/latest version or let them know whether a restart is required and if that's the case what type of restart. By separating that per product you could also provide different information for each product. The easiest solution would be to have a separate remediation item for the Infusion only:

"remediations": [
        {
          "category": "vendor_fix",
          "product_ids": [
            "CSAFPID-a2282874-4c95-e481-e705-fad3f1e12da9",
            "CSAFPID-85616e8a-99ea-d77d-8e9d-f8a4c702b549",
            "CSAFPID-7ca38a78-fde9-44ba-d8e1-cb7204146303"
          ],
          "details": "Please update to the latest version of the software provided by the Vendor."
        },
        {
          "category": "vendor_fix",
          "details": "Infusion is affected because of inheritance/bundling of a vulnerable component and not directly due to a vulnerability. This was fixed by updating the dependencies in version xyz.",
          "product_ids": ["CSAFPID-c110ce80-d38d-559f-43eb-5532710ae684"]
        }
      ]

An alternative approach could be to state this sentence in a vulnerability note. I have a question though: Is it important whether a vulnerability was inherited or in the own code?

The product_identification_helper is also added now using purl identifier. Preferably we can also add signature hashes after some adjustment to the current code.

I'm not sure but it doesn't seem to work as intended as every PURL has supplier as type and the name and namespace is undefined. E.g. pkg:supplier/undefined/undefined@v4.7

Not sure if this requires another product to be added to the product_tree as the "patched" product or security fixed product.

Yes. You should add additional products in the product_tree for patched versions, e.g.:

   {
        "category": "vendor",
        "name": "ACME",
        "branches": [
          {
            "category": "product_name",
            "name": "INFUSION",
            "branches": [
              {
                "category": "product_version",
                "name": "1.1",
                "product": {
                  "product_id": "CSAFPID-0001",
                  "name": "ACME INFUSION 1.1",
                  "product_identification_helper": {
                    "purl": "pkg:supplier/ACME/INFUSION@1.1"
                  }
                }
              }
            ]
          }
        ]
      }

Which then has the product status fixed.

tschmidtb51 commented 3 years ago

I was just wondering: Is supplier a valid PURL type?

tschmidtb51 commented 3 years ago

According to Section 5.3 the JSON should be sorted alphabetically. It also supports the reader by minimizing forward references ;-)

sei-vsarvepalli commented 3 years ago

Hello @tschmidtb51

I think all these issues are fixed now with the latest version of the demo site and the JSON output shown here. There is one issue that remains is the fake PURL - url's we have.

I have a question though: Is it important whether a vulnerability was inherited or in the own code? This has comes up in our SBOM working groups. The consensus as I understand is that we are not 100% sure how this can be used or will be valuable or not. However, the only thing we can do - is if we have this information about component is either dependent or included on a vulnerable component, then it is worth stating that if the format for VeX supports it. Thats all.

If you think we should add a sentence about that in the standard again, please open an issue at oasis-tcs/csaf. BTW: The constraint is in Test 6.1.6 Contradicting Product Status.

This is understandable once you have stated it.

I'm not sure but it doesn't seem to work as intended as every PURL has supplier as type and the name and namespace is undefined. E.g. pkg:supplier/undefined/undefined@v4.7

I fixed this code that creates the fake SBOM PURL urls, but it is fundamentally broken! I can either not leave them in the sbom_urls as-is, as this is mostly for demo purposes OR entirely remove them as they are really not a valid identifier.

All other concerns should be completed, including the sorting of JSON doc. Seems like the JSON is also meant to be human readable and friendly as well.

SwiftBOM-CSAF-ACME-INFUSION-1-0-v5-2-6-json.txt

Vijay

tschmidtb51 commented 3 years ago

Hello @sei-vsarvepalli, thanks for addressing these issues so fast.

I think all these issues are fixed now with the latest version of the demo site and the JSON output shown here.

Perfect. :+1:

There is one issue that remains is the fake PURL - url's we have.

I would suggest to add a note on that:

"notes": [
      // ...
      {
        "category": "other",
        "text": "As this document was created for demo purposes only the PURL strings use the type `supplier`. This results that the PURL strings aren't valid according to the specification."
      }
    ]

I have a question though: Is it important whether a vulnerability was inherited or in the own code?

This has comes up in our SBOM working groups. The consensus as I understand is that we are not 100% sure how this can be used or will be valuable or not. However, the only thing we can do - is if we have this information about component is either dependent or included on a vulnerable component, then it is worth stating that if the format for VeX supports it. Thats all.

As you have added the information, I guess it is all clear now. Correct?

If you think we should add a sentence about that in the standard again, please open an issue at oasis-tcs/csaf.

Let me rephrase that (just in case somebody else comes across the thread: Please follow the process described at How to Provide Feedback during Public Review.

BTW: The constraint is in Test 6.1.6 Contradicting Product Status.

This is understandable once you have stated it.

I'm not sure but it doesn't seem to work as intended as every PURL has supplier as type and the name and namespace is undefined. E.g. pkg:supplier/undefined/undefined@v4.7

I fixed this code that creates the fake SBOM PURL urls, but it is fundamentally broken! I can either not leave them in the sbom_urls as-is, as this is mostly for demo purposes OR entirely remove them as they are really not a valid identifier.

As mentioned above, I suggest to add a note item and keep the purl.

All other concerns should be completed, including the sorting of JSON doc. Seems like the JSON is also meant to be human readable and friendly as well.

At least for small documents and examples that should be the case as it hopefully aid in helping the user to understand the concepts...

I have one more (minor) issue: It would be great if the fixed version could be added as a sibling to the affected one instead of having a separate complete branch (It isn't wrong but it reduces document size and makes it easier to read ;-)):

           {
            "category": "vendor",
            "name": "ACME",
            "branches": [
              {
                "category": "product_name",
                "name": "INFUSION",
                "branches": [
                  {
                    "category": "product_version",
                    "name": "1.0",
                    "product": {
                      "product_id": "CSAFPID-7e9090fb-d599-d26d-20a2-de479cb07ceb",
                      "name": "ACME INFUSION 1.0",
                      "product_identification_helper": {
                        "purl": "pkg:supplier/ACME/INFUSION@1.0"
                      }
                    }
                  },
                  {
                    "category": "product_version",
                    "name": "1.1",
                    "product": {
                      "product_id": "CSAFPID-FIXED-0001",
                      "name": "ACME INFUSION 1.1",
                      "product_identification_helper": {
                        "purl": "pkg:supplier/ACME/INFUSION@1.1"
                      }
                    }
                  }
                ]
              }
            ]
          }

sei-vsarvepalli commented 3 years ago

Should all be completed at least on the NTIA demo portion having a CSAF document that can be used in the VeX use case. Still lot to be done as you can imagine.

I feel more educated by CSAF now and it is designed It is so helpful to have a somewhat practical example to work through.

Vijay

SwiftBOM-CSAF-ACME-INFUSION-1-0-v5-2-7-json.txt

CERTCC / SBOM

Demo feedback from NTIA Medical PoC #10