-
Notifications
You must be signed in to change notification settings - Fork 32
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
Implement python dataclass_json objects to represent decision points #328
Conversation
s/option/value/ s/label/name/ add namespace
Squashed commits: [83a77f3] objects instead of classes [5a59166] objects instead of classes [7b3713a] objects instead of classes [073ca1f] objects instead of classes [566d353] objects instead of classes [f4d8c4d] objects instead of classes [4c88f78] objects instead of classes [ab2ae3e] objects instead of classes [f79af19] objects instead of classes [70409e4] objects instead of classes [9e8dbb2] objects instead of classes [4681eb0] objects instead of classes [9d9f7e7] objects instead of classes [8a01597] objects instead of classes [73d60ef] objects instead of classes [f893a71] objects instead of classes [40003b0] objects instead of classes [7bf8692] objects instead of classes [bbec973] objects instead of classes [838fbf4] refactor imports [c81545c] copy objects instead of new classes [0d4d8ee] add default version 0.0.0 [bd0b9d9] add cvss v1, v2, v3 [700d049] rename classes, clean up mixins [10884aa] add namespaced mixin class [5a83e0c] add s/option/value/ [42f2110] add docstrings
Also add leading underscores to decision point value constants to discourage importing outside of the module they live in.
Waiting on @sei-vsarvepalli for JSON format of JSON schema. |
|
||
@dataclass_json | ||
@dataclass(kw_only=True) | ||
class _Keyed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mixin class was added so that we can remove the key from the base class that DecisionPointGroup inherits from.
|
||
@dataclass_json | ||
@dataclass(kw_only=True) | ||
class SsvcDecisionPointValue(_Base, _Keyed): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SsvcDecisionPointValues now inherit from _Keyed so they have a key.
|
||
@dataclass_json | ||
@dataclass(kw_only=True) | ||
class SsvcDecisionPoint(_Base, _Keyed, _Versioned, _Namespaced): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SsvcDecisionPoints now inherit from _Keyed so they have a key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For JSON Schema creation and automation. I am using the extraneous field
method with a default metadata and an extension schemaprops
all defined in base.py as part of the setup. The _mixin.py also has the similar setup to help automate the creation of JSON schema doc
As dataclasses_jsonschema is a bit bloated for this application, our own method is created in doc_helpers/tools.py. I cannot see if we are using GitHub actions to create and automate all the files, so for now, I have just hand created then. The workflow can be updated so the docs/reference/schema/ is entirely automated and is not part of any manual creation. As we are unlikely to change this schema too often it is a bit more than we need to do at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed primarily the code for src/ssvc to use Python3.10 dataclasses and creation of JSON schema. The JavaScript libraries and demo calculator is not updated as yet.
"key": "N" | ||
}, | ||
{ | ||
"name": "PoC", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a separate issue -- Should we re-name this to "Public PoC" to emphasize it's not just a PoC used to validate that the vulnerability is a real vulnerability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}, | ||
{ | ||
"name": "PoC", | ||
"description": "One of the following cases is true: (1) private evidence of exploitation is attested but not shared; (2) widespread hearsay attests to exploitation; (3) typical public PoC in places such as Metasploit or ExploitDB; or (4) the vulnerability has a well-known method of exploitation.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually think (1) and (2) are operationalizable?
CISA's SSVC version of State of Exploitation removes them https://www.cisa.gov/sites/default/files/publications/cisa-ssvc-guide%20508c.pdf
Maybe we should move for consistency between instances of the decision point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"namespace": "ssvc", | ||
"version": "2.0.0", | ||
"name": "Mission Impact", | ||
"description": "Impact on Mission Essential Functions of the Organization", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar question to my comment on Automatable, can we embed a link to the presidential policy directive here? https://www.cisa.gov/sites/default/files/2023-01/ppd-21-critical-infrastructure-and-resilience-508_0.pdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to have to bump a version in an SSVC object when a URL changes on a site we don't control. The description that goes into the json should be fairly minimalist. The documentation surrounding it can have whatever details we want to add.
If the changes to "Minor" look ok, we should propagate to the other examples. This table was a table within a table, so the conversion is not as simple. Also I think operator resiliency has proven too complicated to translate out of the FAA context.
I closed this PR (not deliberately, but as a side effect of deleting the branch to restore what I had before others made changes to it). I was not expecting commits to a branch in my repo and I had other unpushed work that was dependent on the branch as it was. The changes made by @sei-vsarvepalli and @j--- are available in another branch now https://github.com/ahouseholder/SSVC/tree/feature/update_json_schema It's too disruptive to my work in progress to have commits directly to the branch I'm actively working on. If you want to clone the update_json_schema branch and do a PR against my pyjson branch, let's do it that way? |
Opened a few issues linked above to capture a few side paths from the conversation in this PR from before it closed. |
Starting this as a draft so others can look/comment
Blocked by Update SSVC JSON schema to reflect versioned decision points and groups #321
resolves Create python package to facilitate json round-tripping #325
resolves Add Critical Software decision point #319
resolves Model CVSS v1, v2, v3 vector elements as decision points #330