Skip to content

Proposed fix for 106 json format#107

Merged
sei-vsarvepalli merged 3 commits intoCERTCC:mainfrom
sei-vsarvepalli:issue106-JSON-format
Feb 19, 2021
Merged

Proposed fix for 106 json format#107
sei-vsarvepalli merged 3 commits intoCERTCC:mainfrom
sei-vsarvepalli:issue106-JSON-format

Conversation

@sei-vsarvepalli
Copy link
Contributor

This should address a bulk of the issues that were discussed with @ahouseholder in #106 . I will keep marking the ones that are addressed and capture them in the follow-up comments.

This was linked to issues Feb 16, 2021
@sei-vsarvepalli sei-vsarvepalli self-assigned this Feb 17, 2021
Copy link
Collaborator

@j--- j--- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good.
@sei-vsarvepalli , I'd like to talk about opening a new issue to make sure we document the way that the schema can be used with the CSV to make a "computed" entry that abides the correct style and forces entry elements to match the available decision point options. Another option would be to produce schema similar to examples/Provision-Coordinator for the recommended Deployer and Supplier trees in the v2 doc.

},
{
"label": "Virulence",
"key": "V",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In SSVC v2 "Virulence" is being replaced by "Automatable" with options yes or no. I think that's out of scope for this issue, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few ways to make the CSV also have a computed form. One of them is a add a boolean column called "active" or "available" with yes/no. In CSV, each row represent a possible path in the tree and if the "available" column is "no" this option has been negated or disabled. I think this may get a bit messy to organize in CSV.

The other way is ...
We can generate CSV from JSON and have it eliminate the irrelevant/unavailable rows OR mark them such (like the above added column). The computed JSON with full provision tree can be the "system of record" for a vulnerability at a time, capturing more information about all possible paths in the tree and the ones that were "triaged" in some way.

I am open to both. My only concern is the analyst ability to "create" computed JSON's or CSV's. It is going to be difficult without a tool to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: just create the issue and move these comments there. (Well, that probably means copy them and then do a resolve conversation here.)

@j---
Copy link
Collaborator

j--- commented Feb 18, 2021

@ahouseholder , can you deconflict the open (unchecked) items in #106 with #88, and get us one list of what we need to address? It's not clear to me whether this PR should close 106, because every open item in #106 is covered under #88, or not.

@ahouseholder ahouseholder changed the title Issue106 json format Proposed fix for 106 json format Feb 18, 2021
@ahouseholder
Copy link
Contributor

With the separate creation of #108, #109, and #110, I think this might fix #88.

@ahouseholder ahouseholder linked an issue Feb 18, 2021 that may be closed by this pull request
4 tasks
@ahouseholder ahouseholder mentioned this pull request Feb 18, 2021
6 tasks
Copy link
Contributor

@ahouseholder ahouseholder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an inconsistency in vector notation -- SSVC/v:2/R:C/... appears in one place but SSVCv2/... in others. I think this is just a change toward the former that wasn't propagated to the other json files.

Once that's fixed, then this PR will also fix #110 when merged.

"role": "Coordinator",
"id": "VU#290915",
"version": "2.0",
"computed": "SSVCv2/E:P/V:R/T:P/M:H/D:A/1607626684/",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still like to see version be separate from "SSVC", which I take as a namespace rather than a version, but that's also covered in #110 so I'm ok with making that be a separate task if the decision is to defer it here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, indication of which tree the vector addresses, also in #110. A separate pull request later for that is fine too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SSVC/2/... makes sense, that is, i'm in favor of separating the name space and the version.

"decision_tree": {
"decision_points": [
{
"label": "Exploitation",
Copy link
Contributor

@ahouseholder ahouseholder Feb 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should individual decision points also have "descriptions" the way their options do? That might be a separate issue if so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(If so, please cut a new issue for that so we can merge this and move on.)

},
"computed": {
"description": "Computed score short representation such as SSVCv2/Ps:Nm/T:T/U:E/1605040000/ for a vulnerability with no or minor Public Safety Impact, total Technical Impact, and efficient Utility, which was evaluated on Nov 10, 2020.",
"description": "Computed score short representation such as SSVC/v:2/R:C/E:[P,A]/V:R/T:P/M:H/D:[T,R,A,E]/1607626684/ for a vulnerability with no or minor Public Safety Impact, total Technical Impact, and efficient Utility, which was evaluated on Nov 10, 2020.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you have version and role split out here, but not in https://github.com/CERTCC/SSVC/pull/107/files#diff-59989ecd9b70fdbb329640c6da907e5fcc247ba37bc20eaeac267f325e7f70e0 so this might just be a cleanup for consistency before merging this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes look right. One thing I want to be clear is v 2.01 mentioned is about the JSON file, it is NOT a new version of SSVC. Sorry I should picked a different number scheme perhaps. Hope that makes sense.

It will be worth in next two weeks or so for three of us to meet and sort of clarify what our thoughts about things to cover and close in the next few weeks.

@ahouseholder ahouseholder linked an issue Feb 18, 2021 that may be closed by this pull request
@sei-vsarvepalli sei-vsarvepalli merged commit 293c080 into CERTCC:main Feb 19, 2021
@sei-vsarvepalli sei-vsarvepalli deleted the issue106-JSON-format branch February 19, 2021 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants