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

Check information type categorization CIA impacts have a selected field #955

Open
14 tasks
aj-stein-gsa opened this issue Dec 4, 2024 · 5 comments · Fixed by #965
Open
14 tasks

Check information type categorization CIA impacts have a selected field #955

aj-stein-gsa opened this issue Dec 4, 2024 · 5 comments · Fixed by #965

Comments

@aj-stein-gsa
Copy link
Contributor

aj-stein-gsa commented Dec 4, 2024

Constraint Task

We had previously coded constraint checks in #689 to ensure base and selected CIA impact fields in system-characterstics/system-information/categorization are equivalent and otherwise provide a justification, but this can be avoided by not providing a selected value and just base. We need to ensure that the schema optional selected is provided.

Tests still passed if removing selected from the example valid SSP below, when it should in fact trigger because selected is not present and the equivalence check should fail.

<confidentiality-impact>
<base>fips-199-high</base>
<selected>fips-199-high</selected>
<!-- adjustment-justification removed to ensure cia-impact-has-adjustment-justification passes when base and selected have the same impact level -->
</confidentiality-impact>
.

We should have a separate check to be sure.

Intended Outcome

Check optional selected field is set before checking base and selected equivalence.

Syntax Type

This is required core OSCAL syntax.

Allowed Values

There are no relevant allowed values.

Metapath(s) to Content

<!-- context -->
/system-secuirty-plan/system-characteristics/system-information

<!-- target -->
information-type[./categorization[@system='https://doi.org/10.6028/NIST.SP.800-60v2r1']]


<!-- constraint test -->
count(./confidentiality-impact/selected) = 1
count(./integrity-impact/selected) = 1
count(./availability-impact/selected) = 1

Purpose of the OSCAL Content

Ensure base and selected are present to determine equivalence or checked for justification text.

Dependencies

N/A

Acceptance Criteria

  • All OSCAL adoption content affected by the change in this issue have been updated in accordance with the Documentation Standards.
    • Explanation is present and accurate
    • sample content is present and accurate
    • Metapath is present, accurate, and does not throw a syntax exception using oscal-cli metaschema metapath eval -e "expression".
  • All constraints associated with the review task have been created
  • The appropriate example OSCAL file is updated with content that demonstrates the FedRAMP-compliant OSCAL presentation.
  • The constraint conforms to the FedRAMP Constraint Style Guide.
    • All automated and manual review items that identify non-conformance are addressed; or technical leads (David Waltermire; AJ Stein) have approved the PR and “override” the style guide requirement.
  • Known good test content is created for unit testing.
  • Known bad test content is created for unit testing.
  • Unit testing is configured to run both known good and known bad test content examples.
  • Passing and failing unit tests, and corresponding test vectors in the form of known valid and invalid OSCAL test files, are created or updated for each constraint.
  • A Pull Request (PR) is submitted that fully addresses the goals section of the User Story in the issue.
  • This issue is referenced in the PR.

Other information

This check is part of the #814.

@aj-stein-gsa aj-stein-gsa moved this from 🆕 New to 🔖 Ready in FedRAMP Automation Dec 4, 2024
@Gabeblis Gabeblis self-assigned this Dec 5, 2024
@Gabeblis Gabeblis moved this from 🔖 Ready to 🏗 In progress in FedRAMP Automation Dec 5, 2024
@Gabeblis Gabeblis linked a pull request Dec 5, 2024 that will close this issue
6 tasks
@Gabeblis Gabeblis moved this from 🏗 In progress to 👀 In review in FedRAMP Automation Dec 5, 2024
@Gabeblis
Copy link
Contributor

Gabeblis commented Dec 5, 2024

@aj-stein-gsa I don't believe we need to add the constraints in this issue. The following constraint already exists:

<expect id="cia-impact-has-selected" target="system-information/information-type/(confidentiality-impact | integrity-impact | availability-impact)" test="selected" level="ERROR">
    <formal-name>Cia Impact Has Selected</formal-name>
    <prop namespace="https://docs.oasis-open.org/sarif/sarif/v2.1.0" name="help-url" value="https://automate.fedramp.gov/documentation/ssp/4-ssp-template-to-oscal-mapping/#system-information-and-information-types"/>
    <message>A FedRAMP SSP information type confidentiality, integrity, or availability impact MUST specify the selected impact.</message>
</expect>

Should I just tweak this one to test count(selected) = 1?

@aj-stein-gsa
Copy link
Contributor Author

I guess that was sloppy on my part, apologies. Does that pattern work elsewhere? I would have made it something like the following:

<expect id="cia-impact-has-selected" target="system-information/information-type/(confidentiality-impact | integrity-impact | availability-impact)" test="not(exists(selected))" level="ERROR">
    <formal-name>Cia Impact Has Selected</formal-name>
    <prop namespace="https://docs.oasis-open.org/sarif/sarif/v2.1.0" name="help-url" value="https://automate.fedramp.gov/documentation/ssp/4-ssp-template-to-oscal-mapping/#system-information-and-information-types"/>
    <message>A FedRAMP SSP information type confidentiality, integrity, or availability impact MUST specify the selected impact.</message>
</expect>

I just noticed when reviewing the epic if I took a valid SSP and removed selected it was still all valid. I am guessing that maybe how test works given an empty test evaluation could result to ... true, or at least not evaluate to false and not fire?

@Gabeblis
Copy link
Contributor

Gabeblis commented Dec 5, 2024

I just ran the tests on ssp-all-VALID and it throws the error when I remove the selected. I'm not sure what's different about how you're testing it. The current implementation is valid, but I am more than happy to change it to something like exists(selected).

@aj-stein-gsa
Copy link
Contributor Author

I just ran the tests on ssp-all-VALID and it throws the error when I remove the selected. I'm not sure what's different about how you're testing it. The current implementation is valid, but I am more than happy to change it to something like exists(selected).

Le sigh, I will double check but I will have to try again, but I guess I cannot trust myself to run these things and do issue refinement anymore. 🤦

@aj-stein-gsa
Copy link
Contributor Author

OK well then, yay for me. It would seem I ran npm run constraint ... blah and presumed the error was ... the error. I am guess it was unrelated.

@Gabeblis Gabeblis moved this from 👀 In review to 🚢 Ready to Ship in FedRAMP Automation Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🚢 Ready to Ship
Development

Successfully merging a pull request may close this issue.

2 participants