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

Create an #assembly_oscal-ssp_impact SSP model #1129

Closed
3 tasks
guyzyl opened this issue Feb 8, 2022 · 12 comments · Fixed by #1171
Closed
3 tasks

Create an #assembly_oscal-ssp_impact SSP model #1129

guyzyl opened this issue Feb 8, 2022 · 12 comments · Fixed by #1171

Comments

@guyzyl
Copy link
Contributor

guyzyl commented Feb 8, 2022

User Story:

Currently, inside the #assembly_oscal-ssp_system-information model (within the SSP schema) there are 3 fields which implement the same object. The fields are:

  • confidentiality-impact
  • integrity-impact
  • availability-impact

In order to avoid duplication, I suggest creating a new #assembly_oscal-ssp_impact model within the SSP schema to define the impact component
The model will be used by the 3 fields mentioned, and will deprecate the repeated implementation within the fields.

Since the fields themselves remain the same (and just the way they are defined in the schema is changed), this proposed change is backwards compatible and can be implemented in OSCAL v1.x.y.

Goals:

Reduce duplication in the SSP schema by creating a new #assembly_oscal-ssp_impact model.

Acceptance Criteria

  • All OSCAL website and readme documentation affected by the changes in this issue have been updated. Changes to the OSCAL website can be made in the docs/content directory of your branch.
  • A Pull Request (PR) is submitted that fully addresses the goals of this User Story. This issue is referenced in the PR.
  • The CI-CD build process runs without any reported errors on the PR. This can be confirmed by reviewing that all checks have passed in the PR.
@guyzyl
Copy link
Contributor Author

guyzyl commented Feb 8, 2022

In case this gets accepted, I'd be more then happy to open a PR to fix it.

@aj-stein-nist
Copy link
Contributor

tl;dr: this is by design. I will explain why below.

Ok, so this is a little bit of inside baseball. How familiar you are with the whole Risk Management Framework?

The idea here is part of Step 1 and Step 2, preparation and categorization. For systems designed and deployed in the US federal government, you are supposed to follow, as part of RMF, in this second step a process where you itemize your information-types (you probably saw this adjacent field), look at their C-I-A impact levels, and those bubble up into the "average score" that explains is your system FIPS-199 Low, Moderate, or High Impact. That is why those are there. So, this is very much on purpose.

Here is the document explaining that from the official perspective.

Now, do we want to minimize that for use outside of RMF or disambiguate it somehow? I definitely defer that to the NIST OSCAL developer team, and the community, and in that particular order.

@guyzyl
Copy link
Contributor Author

guyzyl commented Feb 8, 2022

The link to the document is unreachable (SSL cipher mismatch), but that's really an unrelated issue.

I understand that the fields need to be filled out in different steps of the RMF, but that still doesn't mean their objects need to be defined in-field.
The content of the fields can be defined by the same object schema (i.e. #assembly_oscal-ssp_impact), and the fields themselves will still be independent of each other but of the same data structure, and be filled out in the proper RMF steps.

In case I totally misunderstood you comment, feel free to ignore mine 😅

@aj-stein-nist
Copy link
Contributor

Ah ok, I see, so you are suggesting we deduplicate code? This is how it is implemented now that generates the XML Schema and JSON Schema (which I assume you reference from the notation you use).

From what I can tell, the impact wrappers are unique, but they wrap the same field structures and reuse them (which is why they are called <field/> and not <define-field/>, but we get the ability to change the formal name and doc string for each separately the way it is. For a small refactor, I do not understand what is gained but am I understanding what you are recommending?

You might want to review the bit that is not exposed in the source Metaschema code that is in the <constraint/> around the use of fips-199-low, fips-199-moderate, and fips-199-high and how that plays into reducing duplication of values and why it was done that way, but I think I am (personally, the more veteran devs on the NIST team probably less so than me) lead me to miss what the benefit would be here. Am I still off the mark?

@guyzyl
Copy link
Contributor Author

guyzyl commented Feb 9, 2022

tl;dr - The proposed change improves the usability of OSCAL via code without changing the schema behavior (or breaking compatibility).

From what I can tell, the impact wrappers are unique, but they wrap the same field structures and reuse them (which is why they are called and not , but we get the ability to change the formal name and doc string for each separately the way it is. For a small refactor, I do not understand what is gained but am I understanding what you are recommending?

You understood my suggestion correctly, and you're right that from a schema perspective there's no benefit here.

The reasoning behind this proposed change (which explains the benefit I see) is that I'm currently working on creating a 1-to-1 implementation of the OSCAL schemas in Python (as I have previously stated on Gitter).
Each component/field in the schemas is directly defined as a Python dataclass which represents the correct data structure and its types.

Since currently there's no #assembly_oscal-ssp_impact, 3 exactly the same but separate code objects (dataclasses in this case) need to be defined to represent the data structure for each of the fields mentioned in the ticket.

The increased amount of object types involved (in the code) increases its complexity, and decreases the ease of implementation of more advance features based on those fields/types.
This is since there's a certain implementation in the schemas that can be optimized without changing functionality, but will bring improved code usability for OSCAL implementations.

It is of course possible to deviate from the original jsonschema and define a single data model in the code implementation, but I believe such deviations are not the correct path to go for OSCAL which relies solely on its data structures.

@aj-stein-nist aj-stein-nist added Model Engineering An issue to be discussed during the bi-weekly Model Engineering Meeting User Story and removed User Story labels Feb 11, 2022
@guyzyl
Copy link
Contributor Author

guyzyl commented Feb 21, 2022

@aj-stein-nist where does this issue stand ATM?

@aj-stein-nist
Copy link
Contributor

Sorry, I wanted to catch up with @david-waltermire-nist and the development team to discuss if there are any objections to moving forward with it. I was unable to last week. I will try and sync up with on this issue this week.

@guyzyl
Copy link
Contributor Author

guyzyl commented Feb 22, 2022

@aj-stein-nist no worries, thanks for the update.

@david-waltermire david-waltermire added this to Needs triage in Issue Triage Feb 25, 2022
@david-waltermire david-waltermire moved this from Needs triage to Discussion Needed in Issue Triage Feb 25, 2022
@david-waltermire
Copy link
Contributor

@guyzyl Please make the Metaschema changes to support this and submit a PR.

@david-waltermire david-waltermire moved this from Discussion Needed to Assigned to Developer in Issue Triage Mar 8, 2022
@guyzyl
Copy link
Contributor Author

guyzyl commented Mar 13, 2022

@david-waltermire-nist I started implementing this and hit a small hurdle.
From my understanding of the Metaschema format, it's not currently possible to use an assembly and override the name field (the name must match the ref name).
This means that I can't create a new <define-assembly name="impact">, and then reference it under different assemblys with different names.
Did I understand correctly? Do you know how I can overcome this?

@david-waltermire
Copy link
Contributor

You can use the use-name element in the assembly.

Here is an example.

@guyzyl
Copy link
Contributor Author

guyzyl commented Mar 13, 2022

Thanks!

guyzyl added a commit to guyzyl/OSCAL that referenced this issue Mar 13, 2022
@david-waltermire david-waltermire added this to the OSCAL 1.0.2 milestone Mar 15, 2022
@david-waltermire david-waltermire moved this from Assigned to Developer to Allocated to Milestone in Issue Triage Mar 15, 2022
@david-waltermire david-waltermire linked a pull request Mar 17, 2022 that will close this issue
4 tasks
@david-waltermire david-waltermire removed the Model Engineering An issue to be discussed during the bi-weekly Model Engineering Meeting label Apr 1, 2022
david-waltermire pushed a commit to guyzyl/OSCAL that referenced this issue May 17, 2022
@david-waltermire david-waltermire self-assigned this Jul 6, 2022
david-waltermire pushed a commit to guyzyl/OSCAL that referenced this issue Aug 25, 2022
david-waltermire added a commit that referenced this issue Aug 25, 2022
* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves #1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
Issue Triage automation moved this from Allocated to Milestone to Done Aug 26, 2022
@oscalbuilder oscalbuilder removed this from Done in Issue Triage Aug 26, 2022
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Oct 6, 2022
…gov#1171)

* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves usnistgov#1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
aj-stein-nist pushed a commit that referenced this issue Oct 18, 2022
* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves #1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
david-waltermire added a commit that referenced this issue Oct 31, 2022
* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves #1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jan 10, 2023
…gov#1171)

* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves usnistgov#1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Feb 6, 2023
…gov#1171)

* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves usnistgov#1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jun 29, 2023
…gov#1171)

* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves usnistgov#1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
aj-stein-nist pushed a commit to aj-stein-nist/OSCAL-forked that referenced this issue Jul 10, 2023
…gov#1171)

* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves usnistgov#1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
aj-stein-nist pushed a commit to galtm/OSCAL that referenced this issue Sep 28, 2023
…gov#1171)

* Create`<define-assembly name="impact">`
* Restored C/I/A formal names and descriptions.

Resolves usnistgov#1129.

Co-authored-by: David Waltermire <david.waltermire@nist.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants