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

Remove optional leveraged-authorization prop on implemented-requirement #1729

Conversation

aj-stein-nist
Copy link
Contributor

@aj-stein-nist aj-stein-nist commented Mar 30, 2023

Committer Notes

Per #1552 analysis, the original analysis and provenance for this is unclear and the name indicates its intended use overlaps with more accurate mechanisms available for the implemented requirement. I cannot find a way to write clearer documentation better that cannot explain it, so I opt to remove it.

Closes #1552.

All Submissions:

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

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 all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.

Per #1552 analysis, the original analysis and provenance for this is unclear and the name indicates its intended use overlaps with more accurate mechanisms available for the implemented requirement. I cannot find a way to write clearer documentation better that cannot explain it, so I opt to remove it.
@aj-stein-nist
Copy link
Contributor Author

@Compton-NIST not sure you had thoughts on this but I know this touches on CRM and leveraged authorization work. I am not sure you have a perspective on this, but I was unable to find any rationale to keep it after spelunking.

@aj-stein-nist aj-stein-nist marked this pull request as ready for review March 30, 2023 12:50
Copy link
Contributor

@wendellpiez wendellpiez left a comment

Choose a reason for hiding this comment

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

LGTM. Question: does something like this warrant an ADR?

@aj-stein-nist
Copy link
Contributor Author

LGTM. Question: does something like this warrant an ADR?

I think for large impactful changes that architecture-level impact (the A in ADR), that would make a lot of sense. For this change, it is very minor.

But not specifically to the content of the PR, I think we could and should clarify how we will float such changes, small-medium, and large into pre-releases and publish them for feedback to keep us honest. I think Chris or other team members brought it up to me recently, and I heavily agree we need to communicate and act on that, just maybe not in the context of this change in this PR, but it is related. Does that make sense?

Copy link

@GaryGapinski GaryGapinski left a comment

Choose a reason for hiding this comment

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

Looks like FedRAMP never used this prop. Control implementation to leveraged authorization is via an intra-document component.

@Compton-US
Copy link
Contributor

@Compton-NIST not sure you had thoughts on this but I know this touches on CRM and leveraged authorization work. I am not sure you have a perspective on this, but I was unable to find any rationale to keep it after spelunking.

@aj-stein-nist Not anything that crosses my path that I'm aware of.

@iMichaela
Copy link
Contributor

@aj-stein-nist , @Compton-NIST - I am suggesting discussing it with the community as well, at minimum with the GRC tool vendors that implemented already SSP and leveraged authorizations (@Telos-sa, @vmangat, @gregelin, @ancatri, etc..). @GaryGapinski indicated FedRAMP has not used it.

@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Mar 30, 2023

@aj-stein-nist , @Compton-NIST - I am suggesting discussing it with the community as well, at minimum with the GRC tool vendors that implemented already SSP and leveraged authorizations (@Telos-sa, @vmangat, @gregelin, @ancatri, etc..). @GaryGapinski indicated FedRAMP has not used it.

One of the users you listed above was the original reporter of this issue, they reported it to you and I five months ago in November 2022 and existed as-is two years prior to that. (I don't mention them unless they ask.) They can continue to use the property, whether or not they want to confirm how they use it here. We are not generating schemas or add-on constraints that require its usage, nor recommend or require mandatory values. The original request was to ask "what should go there? It is unclear" so if anything, with this change we need not obligate others to interpret how to use it and imply it has an important use. We cannot provide one.

I am more than willing to take any feedback to the contrary prior to a release and adapt though. Let's see what others say.

aj-stein-nist added a commit to aj-stein-nist/OSCAL that referenced this pull request Mar 30, 2023
@iMichaela
Copy link
Contributor

One of the users you listed above was the original reporter of this issue, they reported it to you and I five months ago in November 2022 and existed as-is two years prior to that.

@aj-stein-nist - I am not opposed to the removal, and I am aware of the question and the timeframe when it was posted, including our tardiness in responding - hence my comment. Since the PR is removing an allowed value for the property which can be locally (re)defined if needed (as you correctly stated), I am convinced now we are not causing any issue to the mentioned implementer (or others). You can disregard my comment. Thank you.

@wendellpiez
Copy link
Contributor

Note: removal of a definition of a constraint over a prop does not remove the prop or make it invalid.

Do we need to clarify for users the actual change to how their documents validate? The change is that unenumerated prop values that would earlier have been reported as errors (actually only warnings in this case since allow-other was still 'yes'), are no longer reported, but simply allowed to be. (Please correct me if this is not the case.)

@aj-stein-nist
Copy link
Contributor Author

Note: removal of a definition of a constraint over a prop does not remove the prop or make it invalid.

Totally agree. Everything you wrote was correct and I was trying to express the same thing. Seems like we are all in alignment.

@aj-stein-nist aj-stein-nist merged commit 7d9a11c into develop Mar 31, 2023
@GaryGapinski
Copy link

GaryGapinski commented Mar 31, 2023

[Edited to target provided-uuid rather than uuid]
[Edited to question parentage.]
Esprit de l'escalier: after looking at the related #1552 it appears that FedRAMP is not employing/exploiting system-security-plan/control-implementation/implemented-requirement/statement/by-component/inherited which means there are multiple approaches to inheritance specification. Less is more, as said Mies van der Rohe. Using the strongly named (adjective, past tense) element inherited has semantic strength but weak reference: it cites a provided-uuid which is "Provided UUID: A machine-oriented identifier reference to an inherited control implementation that a leveraging system is inheriting from a leveraged system." but hunting that down might be subject to ambiguity (of location).

If the provided-uuid of inherited targets a control implementation (implemented-requirement) then the target of provided-uuid should perhaps be something more than implemented-requirement.

@aj-stein-nist
Copy link
Contributor Author

[Edited to target provided-uuid rather than uuid] [Edited to question parentage.] Esprit de l'escalier: after looking at the related #1552 it appears that FedRAMP is not employing/exploiting system-security-plan/control-implementation/implemented-requirement/statement/by-component/inherited which means there are multiple approaches to inheritance specification. Less is more, as said Mies van der Rohe. Using the strongly named (adjective, past tense) element inherited has semantic strength but weak reference: it cites a provided-uuid which is "Provided UUID: A machine-oriented identifier reference to an inherited control implementation that a leveraging system is inheriting from a leveraged system." but hunting that down might be subject to ambiguity (of location).

If the provided-uuid of inherited targets a control implementation (implemented-requirement) then the target of provided-uuid should perhaps be something more than implemented-requirement.

But to be clear that is follow-on work, right? I just was concerned you posted this here because it is on the PR we merged, but I read it twice and could not determine if it meant there was value to the prop. The points are worth considering, but it does not seem related to the prop in the scope of the PR. I just wanted to check we understand you correctly.

@GaryGapinski
Copy link

I will make a separate issue. The main thing I wanted to highlight is more than one way to specify inheritance is one more than desirable.

@wendellpiez
Copy link
Contributor

Point is well made @GaryGapinski, even if you understate the problem. (Who says the limit of UUID-space is the cosmos, this one or any other?) Of course, we are only moving it (the problem) and trying to offer a breadcrumb (a data point). Like everything here, what is shown in the instance is naught-but-a-show, i.e. a representation. Up to the players what to do with it.

👍 on more than one way is undesirable; on pruning unused features; on sketching and testing recommendation(s).

Work on the constraints layer within the Metaschema context includes mechanisms both to define indexes for lookup (across defines sets of documents) and to constrain values (such as UUIDs) to resolve to items in such indexes, as we are well aware that such relations scream out (in a technical sense) for handling and validation outside of application semantics.

@gregelin
Copy link

gregelin commented Apr 1, 2023

**@aj-stein-nist Thanks for pegging me on this topic.

I support removing a property on OSCAL SSP to indicate leveraging authorization because it's problematic for properties of a relationship between two entities -- like 2 SSPs or 2 statements in a leveraged SSP and leveraging SSP -- to be assigned to one entity or the other.

A relationship is its own entity. That's why linking tables exist in a normalized relational database: to store/manage the attributes of the relationship distinct from the entities in the relationship.

The discussion seems to have started because a property was placed on the leveraging SSP to indicate a relationship...and now there's a reasonable question from an OSCAL user to have information defined about that relationship.

But such a relationship between two controls statements (or SSPs) can be complex, conditional, and context-sensitive. Simple expressions of that relationship won't exist, only exploding complexity. Assume OSCAL came up with a "simple" 3 states to tie together the leveraged SSP statement and the leveraging SSP statement. What about parameters? What about Customer Responsibilities on the Leveraged SSP? Even putting both of those aside, what if the leveraged SSP is leveraging another SSP itself?

Summing up, the road to unmanageable complexity is paved with conveniency properties.

@vmangat
Copy link
Contributor

vmangat commented Apr 3, 2023

We are leaning towards FedRAMP's approach of defining component definition assemblies for leveraged packages, rather than linking ImplementedRequirements to an entire leveraged SSP. We would support removal of this prop.

aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Jun 29, 2023
…istgov#1729)

Per usnistgov#1552 analysis, the original analysis and provenance for this is unclear and the name indicates its intended use overlaps with more accurate mechanisms available for the implemented requirement. I cannot find a way to write clearer documentation better that cannot explain it, so I opt to remove it.
aj-stein-nist added a commit to aj-stein-nist/OSCAL-forked that referenced this pull request Jul 10, 2023
…istgov#1729)

Per usnistgov#1552 analysis, the original analysis and provenance for this is unclear and the name indicates its intended use overlaps with more accurate mechanisms available for the implemented requirement. I cannot find a way to write clearer documentation better that cannot explain it, so I opt to remove it.
@aj-stein-nist aj-stein-nist added this to the v1.1.0 milestone Jul 27, 2023
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.

Documentation for implemented-requirement prop leveraged-authorization in SSP is unclear
7 participants