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

Propose renaming observation type from finding -> implementation-issue #2105

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

aj-stein-gsa
Copy link
Contributor

Committer Notes

If accepted, closes #2104.

All Submissions:

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

(For reviewers: The wiki has guidance on code review and overall issue review for completeness.)

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • ~Have you written new tests for your core changes, as applicable?
  • ~Have you included examples of how to use your new feature(s)?
  • Have you updated the OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the OSCAL-Pages and OSCAL_Reference repositories. Documentation will update once PR merged and documentation pipeline initiated.

@aj-stein-gsa aj-stein-gsa requested a review from a team as a code owner February 13, 2025 15:07
Act on feedback from community member @egyptiankarim in the issue for more descriptive and constructive enum values.

Source: usnistgov#2104 (comment)
@aj-stein-gsa aj-stein-gsa changed the title Propose renaming observation type from finding -> tool-finding Propose renaming observation type from finding -> implementation-issue Feb 19, 2025
Copy link
Contributor

@iMichaela iMichaela left a comment

Choose a reason for hiding this comment

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

I believe we need to brainstorm a little more here.
What we have today:
ssp-statement-issue = A difference between the SSP implementation statement, and actual implementation, is clear and defines an issue (a problem that requires attention in the SSP.

What is proposed:
documentation-issue = An observation of possible risk from a difference between the documentation for a system and its implementation. <- is not the documentation-finding suggested and based on the description, it is not a good replacement for the ssp-statement-issue.
implementation-issue = An observation of possible risk from the system's implementation from security scanning tools, penetration testing, and other means. <- It was proposed to be called implementation-finding not implementation-issue (see line #904). But I am confused a bit over the scenario : a scanning tools finds something and it different from the implementation documented in the SSP. So how is it different from the documentation-issue? Does the documentation-issue imply the process of finding the discrepancy between the SSP and the implemented system was manual and did not involve a tool?
Clarifying those aspects will help us fix the PR and update it.

Per @iMichaela's feedback, adjust the deprecation messages to document
the correct replacement enumeration values in favor of the deprecated
ones as described in this comment.

usnistgov#2105 (review)
@aj-stein-gsa
Copy link
Contributor Author

I believe we need to brainstorm a little more here. What we have today: ssp-statement-issue = A difference between the SSP implementation statement, and actual implementation, is clear and defines an issue (a problem that requires attention in the SSP.

I updated the description but disagree on the naming, but I do not want to slow this review down further. Can you propose an alternative @iMichaela? I understand your opposition. I graciously took a recommendation from a community member and you seem to oppose it, feel free to suggest in the PR something else. That, or I can close the PRs and issues as WONTFIX.

But I am confused a bit over the scenario : a scanning tools finds something and it different from the implementation documented in the SSP. So how is it different from the documentation-issue? Does the documentation-issue imply the process of finding the discrepancy between the SSP and the implemented system was manual and did not involve a tool? Clarifying those aspects will help us fix the PR and update it.

If you observe a problem with documentation, in a SSP statement or another form of documentation in SSP or other referenced document for that matter, you would document Observation 1. If you find a problem with the implementation with a scanning tool, you would document that in Observation 2 and link them. When we do FedRAMP reviews, this is what we do. Do you do it differently? If you want, you could cross-reference them by link, but even aside from OSCAL, I am not sure why both would be documented as the exact same problem. I could see why they are related.

@iMichaela
Copy link
Contributor

iMichaela commented Feb 27, 2025

If you observe a problem with documentation, in a SSP statement or another form of documentation in SSP or other referenced document for that matter, you would document Observation 1. If you find a problem with the implementation with a scanning tool, you would document that in Observation 2 and link them. When we do FedRAMP reviews, this is what we do. Do you do it differently? If you want, you could cross-reference them by link, but even aside from OSCAL, I am not sure why both would be documented as the exact same problem. I could see why they are related.

@aj-stein-gsa

  1. Are you trying to replace the sup-implementation-issue with documentation-issue to allow for it to be used outside an SSP?
  2. categorizing a finding as issue implies an immediate need to be addressed /resolved. If FedRAMP demand everything that it flags to be considered an issue and be addressed, it doe snot mean that the world uses it in the same way, and therefore documentation-finding appears to be more appropriate. Maybe the observation should have an optional flag which would indicate the finding must be addressed, or there is no continuation of the assessment process.
  3. Same with the concept of implementation-finding. Authorizing official might assess the risk and decide not to address it. Then FedRAMP and other certification bodies could use the flag to indicate it is not to the AO to decide.

On a more general note, if a tool finds a discrepancy between the information in the SSP and the system, how would the tool decide the problem is with the OSCAL SSP artifact and not with the system implementation? Why is this an implementation-finding and not assessment-finding. More over, if a control is assessed manually, and there is a finding what would that be?
If you study the RMF rev2 (see below), during the implementation step, task I-2 calls for updates of the implementation information and implementation-finding could describe such case.

image

Why not simply deprecate finding in favor of discovery which would be more intuitive. And if there is a need to generalize a finding in the context of a document, then we could deprecate ** ssp-statement-issue and replace it with documentation. After all it is a type of observation.

For FedRFAMP and other entities that would like to further impose how to respond to an observation, and the type documentation will indicate where.

I am still suggesting an optional boolean flag for the observation , something like enforceable which would indicate the fact that such discovery must be addressed.

Thoughts ?

@aj-stein-gsa
Copy link
Contributor Author

  1. Are you trying to replace the sup-implementation-issue with documentation-issue to allow for it to be used outside an SSP?

If you look at the original issue, I was interpreting a comment from another community member with this request. I thought it was sensible, and in an attempt to use naming that appeals to community developers, I opted for adding it as that contributor is the only one other than me to show interest in this topic. That is how I interpreted it, but it was not my original idea and we can ask them.

  1. categorizing a finding as issue implies an immediate need to be addressed /resolved. If FedRAMP demand everything that it flags to be considered an issue and be addressed, it doe snot mean that the world uses it in the same way, and therefore documentation-finding appears to be more appropriate. Maybe the observation should have an optional flag which would indicate the finding must be addressed, or there is no continuation of the assessment process.

Just to be clear about the material change in this PR, you are aware I am trying to alter that very flag for the type of observation, that's the only change that has been proposed in the reporting issue and change? I am not changing finding assembly or anything else. I would recommends we look at the target of the allowed value, which is why I didn't think this detail should be repeated again.

  1. Same with the concept of implementation-finding. Authorizing official might assess the risk and decide not to address it. Then FedRAMP and other certification bodies could use the flag to indicate it is not to the AO to decide.

This perspective also presumes a formal authorization, accreditation, audit, or review process with a formal authorizing official. Is this enumeration only for RMF and RMF-related processes like FedRAMP?

And to repeat, such a flag with different values is what we have been discussing in this change the whole time.

On a more general note, if a tool finds a discrepancy between the information in the SSP and the system, how would the tool decide the problem is with the OSCAL SSP artifact and not with the system implementation? Why is this an implementation-finding and not assessment-finding. More over, if a control is assessed manually, and there is a finding what would that be?

A tool can either identify an issue of concern with a systems documentation, implementation, or both. Are these all not kinds of issues found during assessment actions (in the formal RMF sense or more generally)? I would think the tools know the origin of the issue from the nature of the subject of assessment. I think operators of an environment and assessor should be able to make that call and use these models to express them, yes? If you want a specific example we can get into that but one or more will not be exhaustive or lead to more debate.

If you study the RMF rev2 (see below), during the implementation step, task I-2 calls for updates of the implementation information and implementation-finding could describe such case.

image

OK I am not sure how you want this detail to inform review. SP 800-37 is an adaptable framework and can change from organization to organization, and it is only one such framework. I have asked about it only in this context you imply in responses to my questions and comments conformance to your interpretation of it may be a requirement for the model as-is and my proposed changes to it. Can you confirm that is in fact the case?

Why not simply deprecate finding in favor of discovery which would be more intuitive. And if there is a need to generalize a finding in the context of a document, then we could deprecate ** ssp-statement-issue * and replace it with ** documentation. After all it is a type of observation. For FedRFAMP and other entities that would like to further impose how to respond to an observation, and the type documentation will indicate where.

I suppose that is intuitive to you but I am one of two community developers who have presented what is more intuitive to us by consensus agreement. Discovery is ok but to me would cause confusion for repeatable observing something know to be detected in previous observations, but that seems besides the point.

For a minor change I have spent a lot of time coordinating this small change that would appeal to two community developers and that you have made clear you will not accept, so I will just change it to discovery and move it along.

I am still suggesting an optional boolean flag for the observation , something like enforceable with would indicate the importance of addressing it.

Is that not what the use of risk, finding, or their combined usage with related-observations for? How can an observation be enforceable?

One of the reasons I wanted to not have an observation of type finding relates to this choice. Observations can originate findings but the "enforceable action" of them is really determined by the finding given the risk as-is and the mitigating factors for that risk. The AR and POAM models actually support that through shared model definition and conceptually that seems to be in line with a general view of risk and findings with respect to risk, vulnerabilities, and threats as I recall from instruction and practice in the field (but I'll have to look for citation).

Thoughts ?

Given the above analysis as to why I wanted to surgically finding from this enumeration, I propose this separate topic we track and discuss further in another issue outside of this PR as it is important but different from the scope of this proposed change. Do you agree?

@iMichaela
Copy link
Contributor

I agree it is too much dialog between us for something that I do not disagree addressing as long as the way it is addressed makes sense for others in the community. You indicated ^^at FedRAMP we mark such findings with document-issues and implementation-issues^^, but when I asked what are you trying to accomplish with the first deprecation, you pointed to the community member that posted a brief comment.

While I agree finding enumeration value is confusing due to existing field, I don't think the PR's suggestions are reflecting the best solution. You asked me what do I propose and I spent time explaining what I propose and why I propose it, to just have you dismissing it without any solid argument.

To sum, I argue that replacing the enumeration value finding with an abc-issue value is changing its core meaning as oppose to replacing it with an equivalent word in English.
#While at NIST, you were frustrated and against changes not endorsed by the community (more than one member) . Should we wait then for the community to review the PR or do you have a better suggestion than discovery' for the replacement of the enumeration value finding`?

@aj-stein-gsa
Copy link
Contributor Author

aj-stein-gsa commented Feb 27, 2025

Should we wait then for the community to review the PR or do you have a better suggestion than discovery' for the replacement of the enumeration value finding`?

I will update this PR with discovery and return the SSP statement issue value to its original form and be done with it.

After that I think it will be my last new PR to this repository for a while, I find this form of PR review draining.

@iMichaela
Copy link
Contributor

Thank you. It is up to you if you want or not to consider or receive opinions that differ from yours. I cannot control your willingness to collaborate or how you accept other opinions. I am not stuck on "discovery". You can use "result" or another synonym to "finding". Change in the original meaning through deprecation that is theoretically a break of the backwards compatibility for people that used the respective value especially when there is no substantiated concern like in the case of the "ssp-statement-issue" is what I called out and requested thorough analysis, a solid reason for the change to be documented, and broader consensus.
I am planning a patch release next week with the latest PRs

1 similar comment
@iMichaela
Copy link
Contributor

Thank you. It is up to you if you want or not to consider or receive opinions that differ from yours. I cannot control your willingness to collaborate or how you accept other opinions. I am not stuck on "discovery". You can use "result" or another synonym to "finding". Change in the original meaning through deprecation that is theoretically a break of the backwards compatibility for people that used the respective value especially when there is no substantiated concern like in the case of the "ssp-statement-issue" is what I called out and requested thorough analysis, a solid reason for the change to be documented, and broader consensus.
I am planning a patch release next week with the latest PRs

@aj-stein-gsa
Copy link
Contributor Author

aj-stein-gsa commented Feb 27, 2025

I have changed to the values to match your desired changes in #2105 (comment), @iMichaela. To complete the checklist, I need you to run the CI/CD to complete testing. Other than that, I do not have much to ask or answer. Feel free to reject the PR and I will tell the community member who asked for help (separate of the issue, not of egyptiankarim) that it was not accepted.

Thanks for your engagement thus far, I leave it to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants