usnistgov / OSCAL

Open Security Controls Assessment Language (OSCAL)
https://pages.nist.gov/OSCAL/
Other
664 stars 180 forks source link

Profile Schema: "control-id" is currently optional #1053

Closed stephenbanghart closed 2 years ago

stephenbanghart commented 2 years ago

"control-id" (https://pages.nist.gov/OSCAL/reference/latest/profile/xml-reference/#/profile/modify/alter/@control-id) is currently optional.

As the containing statement is inherently non-operational without this value, it should either be changed to required, or explained in the profile specification.

I'd like to hear from @butler54 as well as the NIST team (pinging @david-waltermire-nist @wendellpiez so they see this) on their thoughts between these two options.

wendellpiez commented 2 years ago

The only rationale that occurs to me for control-id being optional is in view of possible extension of the semantics of alter to address other elements (besides controls). However that is a hypothetical, arguably unlikely (given the purpose of profiles), and up to us in any case.

To preserve some wiggle room however and to avoid backward-incompatible thrashing in the XSD/JSON Schema, the requirement could be expressed as a cardinality constraint rather than as a part of the definition. (I.e. min-occurs=0 but add a constraint to require it). Depending on whether and how easily constraints can be layered (in or out), that offers more flexibility. (I am fine with either.)

The profile resolution spec IMO should state that the flag is expected and there is an error condition (either reportable or just GIGO -- at least, optionally reportable as a warning?) in profile resolution, if an alter does not resolve to a single target (irrespective of nominal validity issues in upstream).

This is similar to if I have clashing IDs and no map to resolve the clash (which can happen even if everything is schema-valid), except I am pointing to nothing instead of more than one thing. Error.

So my vote is to address this in the profile spec, irrespective of changes to schemas or new constraints.

JustKuzya commented 2 years ago

Hi, Wendell.

I also have an control ID question: when profile is resolved and there are modifications to the control ID, should not the ID be changed if the modifications are not only additions?

I get that by looking at the profile one can see changes separately, control references separately. But once resolved, though the UUID would be different, the references to the father/mother catalogs are not mandatory any more. So one can end up with a few catalogs where AC-1 has the ID, but essentially not an AC-1 anymore. ☹ Maybe something generic like AC-1-MODIFIED or AC-1-ALTERED

Possibly, I’m missing something. The thought occurred to me a while ago, but as I dug into catalog schema I did not find any mandatory enforcement of name changing like that.

DC

From: Wendell Piez @.> Sent: Monday, November 15, 2021 11:04 AM To: usnistgov/OSCAL @.> Cc: Subscribed @.***> Subject: Re: [usnistgov/OSCAL] Profile Schema: "control-id" is currently optional (Issue #1053)

The only rationale that occurs to me for control-id being optional is in view of possible extension of the semantics of alter to address other elements (besides controls). However that is a hypothetical, arguably unlikely (given the purpose of profiles), and up to us in any case.

To preserve some wiggle room however and to avoid backward-incompatible thrashing in the XSD/JSON Schema, the requirement could be expressed as a cardinality constraint rather than as a part of the definition. (I.e. min-occurs=0 but add a constraint to require it). Depending on whether and how easily constraints can be layered (in or out), that offers more flexibility. (I am fine with either.)

The profile resolution spec IMO should state that the flag is expected and there is an error condition (either reportable or just GIGO -- at least, optionally reportable as a warning?) in profile resolution, if an alter does not resolve to a single target (irrespective of nominal validity issues in upstream).

This is similar to if I have clashing IDs and no map to resolve the clash (which can happen even if everything is schema-valid), except I am pointing to nothing instead of more than one thing. Error.

So my vote is definitely to address in the profile spec, irrespective of changes to schemas or new constraints.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/usnistgov/OSCAL/issues/1053#issuecomment-969059518, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACS6IVRN3SXCCS3QXZOG223UMEVPVANCNFSM5IB7OJRQ. Triage notifications on the go with GitHub Mobile for iOShttps://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Cdmitry.cousin%40nist.gov%7C7bb27ca993e840385d2f08d9a85192a7%7C2ab5d82fd8fa4797a93e054655c61dec%7C1%7C0%7C637725890576613372%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ufjESS%2FCHucECbFeIXtcf%2FtqjrQC5dOUepfb7UMz57U%3D&reserved=0 or Androidhttps://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Cdmitry.cousin%40nist.gov%7C7bb27ca993e840385d2f08d9a85192a7%7C2ab5d82fd8fa4797a93e054655c61dec%7C1%7C0%7C637725890576623326%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=uwXR9%2FjNywySQiMvLfm%2FKs5CnxWfgM1SCaUkw1gDjcY%3D&reserved=0.

wendellpiez commented 2 years ago

@JustKuzya,

That is correct, there is zero guarantee that a control with id ac-1 in a resolved profile would have any similarity to (any) ac-1 in an upstream catalog. alter can do just about anything while 'nominal control identity' implicitly remains with the ID value.

Indeed with the new mapping feature there is no guarantee that ac-1 was even identified as ac-1 and not something else.

Users of profiles should understand this! Additionally, it is an opportunity for a profile resolver to supplement resolution with warnings about modifications judged (by whatever set of rules) to be out of bounds. (For example, maybe you have a rule to say that no new ID given in a map can be the same as any old ID in the resolved source, to detect 'creativity' with this. Etc.) Like traceability, that is an option a resolver could offer over and above "bare" resolution.

HTH - interesting question - @stephenbanghart and @david-waltermire-nist may also have perspective.

butler54 commented 2 years ago

So my thoughts - I think it should be required but I'm happy to leave this to v2. But my recommendation is the text in the documentation is updated to say that it is expected to error without it.

My thoughts on the scope of alter: There is so much flexibility already I see that as being unlikely to be exercised. The most likely functionality (IMO) that will be used is adding and removing either 1) Props 2) 'Guidance' like sections (e.g. corporate specific overlays). These are easily accommodated in the current functionality.