Skip to content
This repository has been archived by the owner on Aug 20, 2021. It is now read-only.

Profile processing for catalog code generation #12

Open
anweiss opened this issue Dec 10, 2018 · 17 comments
Open

Profile processing for catalog code generation #12

anweiss opened this issue Dec 10, 2018 · 17 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@anweiss
Copy link
Contributor

anweiss commented Dec 10, 2018

Support processing of profiles for parameter insertions and add, merge, modify, alter and remove to generate catalogs with profile data.

@anweiss anweiss added the enhancement New feature or request label Dec 10, 2018
@anweiss anweiss added this to the OSCAL 1.0 milestone Dec 10, 2018
@anweiss
Copy link
Contributor Author

anweiss commented Dec 10, 2018

Some of this is addressed by #11.

@anweiss
Copy link
Contributor Author

anweiss commented Jan 11, 2019

@minhaj10p let's start digging into this one. That way we can begin to identify some of the challenges/gaps in nested profiles and figure out ways to deal with them.

@asadullah-yousuf-10p
Copy link
Contributor

@anweiss @minhaj10p do we want to fill in parameters in generated control structs? I think we would want to do that for implementation as well.

@anweiss
Copy link
Contributor Author

anweiss commented Jan 15, 2019

@asadullah-yousuf-10p yes, that would be ideal.

@anweiss
Copy link
Contributor Author

anweiss commented Jan 16, 2019

@asadullah-yousuf-10p let me know your thoughts on this. I know this is not a complete solution, but I think it's one of the flows that we should be able to address.

I'll use the FedRAMP High profile as an example, and I'll use the phrase "root profile" to refer to the profile in context, which in this example is FedRAMP High. I'll also refer to the add, merge, modify, alter and remove elements as "manipulation" elements in this example.

  1. Only the controls/subcontrols include'ed in the root profile need to be selected. If the import'ed profiles also import other profiles, disregard any of the nested controls/subcontrols that are not referenced by the root profile, since they are not in the context of the root profile.
  2. The "manipulation" elements in the root profile and import'ed profiles should be processed, but only for the controls/subcontrols that have been identified per "Step 1" in this sequence. One should recursively search the import chain only for the controls/subcontrols that are referenced by the root profile. The highest-level profile's "manipulation" elements always override any "manipulation" elements when recursively searching the import chain.

@minhaj10p
Copy link
Contributor

minhaj10p commented Jan 17, 2019

Hey @anweiss, I was able to play around with profile processing and here are some insights and personal opinions.

  • We need to make sure that a particular import of a profile caters only one catalog. Another import is required to cater controls from another catalog. If not, we can have two different kinds of controls listed down under a single import section in a profile which makes things very difficult to parse.

  • The modification sections are outside of the import (import identifies the catalog in the light of above point) which makes it difficult to identify what control/sub-controls belong to which catalog.
    ---- One way to do it is we pick up the first control in the import section, and find it in the modification tags so we know which catalog does this modification control/subcontrol belongs to, and then continue to process it. (taking this approach for starters)

  • The parts listed in catalogs has ids but the Fedramp profiles do not. Should we process against class attribute or are we going to be pushing some changes to the fedramp parts so we can add more ids

  • The parameters listed in set-param of a profile are upfront which makes it difficult to figure their relative catalog out or their position. My suggestion is we should make the parameters in the profile fall under a control hierarchy so that it will be easier to track it up against a catalog.

#52
This PR is based on above mentioned assumptions and is subjected to change. It adds the feature to identify the catalog in the import chain/tree + add attribute in alter section of the profile.

@minhaj10p
Copy link
Contributor

minhaj10p commented Jan 17, 2019

@anweiss It would be highly appreciated if you could document what we discussed as part of this issue regarding the modification attributes. Thanks

@anweiss
Copy link
Contributor Author

anweiss commented Jan 17, 2019

@minhaj10p sure thing

@anweiss
Copy link
Contributor Author

anweiss commented Jan 17, 2019

For now, let's exclude the scenario where a profile may import multiple catalogs and just assume that the same catalog is being import'ed.

With that being said, here's a summary of the "manipulation" parsing discussion that we had:

  • "manipulation" elements in profiles should always reference control/subcontrol id's that have been include'd in a profile. If they reference other id's that aren't included, than this is an error in the profile
  • for as long as the same control/subcontrol id is included in an import chain, only the highest-level "manipulation" elements should be parsed ... for example:
    • if AC-1 is in both the root profile and all import'ed profiles in a chain, only parse the first set of "manipulation" elements that you find in the chain
    • if AC-1 is in the root profile, and the root profile also has "manipulation" elements for AC-1, only parse those "manipulation" elements
    • if AC-1 is in both the root profile and the import'ed profiles, but the root profile has no "manipulation" elements, then iterate through the import'ed profiles in the chain until you find the first set of "manipulation" elements, if any exist in the chain for AC-1

@anweiss
Copy link
Contributor Author

anweiss commented Jan 17, 2019

@minhaj10p I opened usnistgov/OSCAL#292 as well to address your last bullet point re. moving set-param's inside alter's

@minhaj10p
Copy link
Contributor

minhaj10p commented Jan 21, 2019

@anweiss

#58 addresses the bug for the import paths i.e #57
#52 addresses alter and add manipulation attributes + parsing import chains of profile and catalog.
Would you be able to elaborate the processing for merge remove and set-param attributes of the profile regarding catalog generation so I can progress on this opened issue. Thanks

@anweiss anweiss changed the title Profile processing for catalog generation Profile processing for catalog code generation Jan 21, 2019
@anweiss
Copy link
Contributor Author

anweiss commented Jan 21, 2019

Thanks @minhaj10p. Once we get #52 reviewed by a couple of other folks, we'll get it merged, along with #58.

@anweiss
Copy link
Contributor Author

anweiss commented Jan 21, 2019

@minhaj10p @asadullah-yousuf-10p I'm also curious whether or not we could try splitting up the generated code across multiple global variables and files, rather than one massive slice. Thinking we could try incorporating some sort of "separator" logic that splits the data by "group" or some other element.

@minhaj10p
Copy link
Contributor

@anweiss
According to my understanding, If a control/subcontrol exist in the include section of a profile and there is no alter attribute in said profile, I look for it in the entire import tree. If i don't find the particular alter in the import tree, it returns an error. The code in oscalkit at this point is returning errors.

If the assumption isn't correct, there can be a lot of i/o operations for each alter attribute with hundreds of goroutines spawning as there can be hundreds of alter attribute if we try to do it concurrently. I tried accommodating that change but it seems like that requires a different approach to find alter in the import tree.
Let me know if my assumption is correct or if we need to devise a better way to fetch alter from the import tree.

I can create a PR for this commit if we need to have a new strategy to parse alter's
minhaj10p@c0498d6

cc: @farhan-khan30 @asadullah-yousuf-10p

@anweiss
Copy link
Contributor Author

anweiss commented Jan 27, 2019

@minhaj10p your understanding is correct. If there is no alter in the import chain for the control/subcontrol in context, you can simply return nil rather than erroring out.

@minhaj10p
Copy link
Contributor

@anweiss I have created a PR #69 as the current implementation uses timeout strategy while traversing the import chain. if the goroutines don't find the respective alter after a certain time it returns an error; It is unrealistic to wait for every alter in the profile for seconds.

I'll have to replace it with the usual tree traversal

@minhaj10p
Copy link
Contributor

@anweiss PR #71 is addressing the alter fetching from import tree
Please have a look when you get time. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants