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

Add convert CCI list workflow #6336

Open
wants to merge 54 commits into
base: master
Choose a base branch
from
Open

Conversation

jtquach1
Copy link
Contributor

@jtquach1 jtquach1 commented Oct 28, 2024

This PR adds on improvements and code cleanup to the NIST to CCI and CCI to NIST mappings that are to be used in #3315, so NIST and CCI tags are displayed at the highest Revision. All of the X->HDF mappers are updated to reflect the NIST<->CCI mapping changes. This PR should also resolve this issue: #6359, where the reported behavior on master has it so when CCI tags are mapped to NIST tags, if a CCI tag has no NIST tag mapping, the libs/hdf-converters/src/mappings/CciNistMapping.ts's
CciNistTwoWayMapper.findMatchingCciIdsByNistControl method tries to find that NIST tag's parent and sibling NIST tags which is incorrect. That private method is used by the public CciNistTwoWayMapper.cciFilter method, which is used in libs/hdf-converters/src/ckl-mapper/checklist-jsonix-converter.ts (CKL->jsonix, where jsonix is used for jsonix->HDF).

image

List of changes/improvements:

  • Added GitHub Action that pulls U_CCI_List.xml from public.cyber.mil and produce three git-versioned files called U_CCI_List.cci.json (NIST tags->CCI tags), U_CCI_List.defs.json (NIST tag->NIST definitions), U_CCI_List.nist.json (CCI tags->NIST tags) using a script called cciListXml2json (more details below)
  • Renamed and refactored the xml2json script into a script called cciListXml2json (to reflect what it's doing) which produces 3 JSON files that contain these mappings: NIST->CCI, NIST tag->NIST definition, CCI->NIST. Also used node's argparse library and updated documentation to reflect the script's usage.
  • Updated corresponding documentation for cciListXml2json.
  • Removed apps/frontend/src/utilities/cci_util.ts, libs/hdf-converters/src/mappings/CciNistMappingItem.ts, libs/hdf-converters/src/utils/CCI_List.ts since they are redundant with the below files
  • Heavily modified libs/hdf-converters/src/mappings/NistCciMappingData.ts, libs/hdf-converters/src/mappings/CciNistMappingData.ts, libs/hdf-converters/src/mappings/CciNistMapping.ts since they originally had redundant functionality with the xml2json/cciListXml2json script and mapped CCI tags to any/all NIST tags regardless of their Revision. The referenced PR would refer to the "third" NIST tag in an array of NIST tags that a CCI tag maps to, and without indication of what Revision that NIST tag comes from, but that third NIST tag may not always exist.
  • Moved uppercase-named constants from libs/hdf-converters/src/utils/global.ts to libs/hdf-converters/src/mappings/CciNistMappingData.ts
  • Manually added two NIST control families in libs/inspecjs/src/nist.ts and libs/inspecjs/src/raw_nist.ts, otherwise the parse_nist function will not recognize the more recent NIST tags (since 2022) in the automatically-pulled NIST tags from U_CCI_List.xml
  • Fixed typos in comments and names of generated HDF files in *.spec.ts tests
  • Updated XCCDF results mapper (OpenSCAP & SCC)->HDF, depending on whether the input SCAP or SCC XML file already contains CCI tags and no NIST tags, already contains CCI tags and NIST tags, doesn't already contain CCI tags but contains NIST tags, and neither contains CCI tags nor NIST tags.

@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from 3e8e7a0 to aa9fb80 Compare October 29, 2024 14:26
@jtquach1 jtquach1 marked this pull request as ready for review October 29, 2024 14:29
@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from 1d5d4ff to f892789 Compare October 29, 2024 17:23
@jtquach1 jtquach1 changed the title Add convert cci list workflow Add convert CCI list workflow Oct 29, 2024
.github/workflows/convert-cci-list.yml Outdated Show resolved Hide resolved
.github/workflows/convert-cci-list.yml Outdated Show resolved Hide resolved
.github/workflows/convert-cci-list.yml Outdated Show resolved Hide resolved
.github/workflows/convert-cci-list.yml Outdated Show resolved Hide resolved
.github/workflows/convert-cci-list.yml Outdated Show resolved Hide resolved
libs/hdf-converters/data/converters/cciListXml2json.ts Outdated Show resolved Hide resolved
libs/hdf-converters/data/converters/cciListXml2json.ts Outdated Show resolved Hide resolved
libs/hdf-converters/data/converters/cciListXml2json.ts Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

why did just this sample file for this mapper get changed and not any of the other samples for this mapper or any other mapper at all?

Copy link
Contributor Author

@jtquach1 jtquach1 Oct 30, 2024

Choose a reason for hiding this comment

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

In the HDF Converters tests GitHub Action, heimdall2/libs/hdf-converters/test/mappers/forward/xccdf_mapper.spec.ts was purely failing for the SCAP ubuntu 1804 test, but none of the others. The failed test comprised of a bunch of key/value pairs like "ident": undefined and the like, and upon me looking into where those values come from, "ident" those key/value pairs don't show up in the resulting JSON since JSON.stringify removes pairs with undefined values. Anyhow, that was the diff I saw on GitHub Actions.

Upon locally running the same test file, I saw some other test files (for different mappers) fail due to async file loading, even though those respective tests didn't seem to fail on GitHub Actions. The XCCDF test file strangely didn't have the same failing error as the GitHub Actions one. But upon regenerating the relevant "expected" HDF of that particular ubuntu 1804 test, I did a git diff and saw that some of the NIST tags changed for existing CCIs. Just as a shot in the dark, I reckoned to commit that, and it looked like this particular test finally turned green. (Perhaps that is not "the" solution though.)

TLDR: Local HDF Converters tests didn't seem to have consistent results with the GitHub Actions' ones. Maybe witchcraft?

@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from 229d078 to e350181 Compare October 30, 2024 17:56
@Amndeep7
Copy link
Contributor

libs/hdf-converters/src/mappings/NistCciMappingData.ts

Current state:

Defines some default CCI values for a select set of NIST tags.
Used in libs/hdf-converters/src/utils/global.ts in two places:

  1. to define a constant based off of another constant.
  2. to create a function (getCCIsForNISTTags) that will be used as a transformer in all the mappers to convert NIST tags if they're provided into CCIs.

Desired state:

Eugene is ideally working on doing a refresh of this data.
I want this file turned into just a raw json blob similar to what the ccilistxml2json script does.
Then I want it to be imported into and exposed as a constant from CciNistMapping.ts similar to how I described I wanted you to expose the data from the script.

libs/hdf-converters/src/utils/global.ts

Current state:

Amongst other things, it defines some constants related to NIST/CCIs and the getCCIsForNISTTags function.

Desired state:

Relevant constants and that function are moved over to libs/hdf-converters/src/mappings/CciNistMapping.ts.
All mappers are updated to point to the new place.
If Eugene gets the update in time, then you might need to regen some more samples.

libs/hdf-converters/src/mappings/CciNistMappingData.ts

Current state:

Currently exposes an object called 'data' that contains the CCI/Nist mapping.

Desired state:

As already described in the peer review, I want you to turn this into two separate files that each contain a raw json blob (i.e. no 'export const data = {' stuff necessary). The first file contains the object mapping CCI to latest NIST rev. The second file contains CCI to its description.
These files are then exposed from CCiNistMapping.ts.

libs/hdf-converters/src/utils/CCI_List.ts

Current state:

It is used in CciNistMapping.ts to help define the two way nist/cci mapper.

Desired state:

Deleted

libs/hdf-converters/src/mappings/CciNistMappingItem.ts

Current state:

Used to define a cci/nist mapping for use in the array form of the data which imo is pretty dumb.

Desired state:

Deleted

libs/hdf-converters/src/mappings/CciNistMapping.ts

Current state:

Defines several types that define the JSON object generated by the xml parser run against CCI_LIST.
Two classes:

  1. CciNistMapping
    Converts the CciNistMappingData object into an array in the constructor.
    Has a function called 'nistFilter' that maps from the provided array of CCIs and converts them into an array of NIST tags with the two interesting things being that it a) returns some default value if no nist tags are found and b) it has an option (set to true by default) to basically run the equivant of a unique pass across the returned data.
  2. CciNistTwoWayMapper
    Converts CCI_LIST into a json object which will have the types defined above.
    Has a function called 'nistFilter' that does a bunch of complicated shit and uses like private functions and stuff but essnetially it boils down to the exact same thing that the other class was doing in a manner that is still inefficient but not as completely dumb as the other implementation.
    Has a function called 'cciFilter' that tries to find all CCIs that have the same NIST tag.

Desired state:

Those constants defined in global are now moved here, and we've defined more constants here that expose the raw json blobs.
Since CCI_LIST is being deleted, those types are no longer required and can be deleted as well.
When getCCIsForNISTTags is moved here, rename it something like defaultNIST2CCI or limitedNIST2CCI, use inspecjs (libs/inspecjs/src/nist.ts) to extract out the tag instead of the handrolled regex that was added there, and then do it as map/reduce (map incoming nist tag strings to properly formatted nist tag top level control (AC-1, not AC-1 (b) blah), map that value to the CCI array using whatever you name the constant that contains the data for our handcrafted mapping, flatten the array of arrays, unique it, return).
I do not think we need those classes any more. They do not take any parameters for their constructors, which in fact are used purely to do some data pre-processing before exposing some functions. If instead we just define constants based off of processing other constants (and maybe a private function or two), then we can expose some raw functions instead of having to have a meaningless wrapper class around them.
CciNistMapping can be deleted.
CciNistTwoWayMapper can be simplified into two exposed functions (might need some private funcs but probably not):

  1. "CCI2NIST"
    Basically it'll look exactly like the nistFilter func but simpler cause instead of doing that forloop and manually having to assess which is the latest rev nist tag, we can just do a map against our prior work and updates to that mapping. I think that the return statement where we potentially return the default nist tags is buggy because ?? only returns the righthand side on null/undefined, but we want it to return that side when matches is the falsy value of an empty array so the operator we actually need there is ||. Sonarqube complains about this everytime and it's wrong about the suggestion to use ?? like 75% of the time.
  2. "NIST2CCI"
    Add a new constant that'll be an object that will be a 'trie'-esque data structure built off of inversing the ccinistmpapingdata data. ccinistmappingdata is { cci-1234: 'ac-1', cci-4321: 'ac-1', cci-2314: 'ac-1 a' }. strict inverse (that won't actually do what we want properly but for demonstration): { ac-1: cci-1234, ac-1: cci-4321, 'ac-1 a': cci-2314 }. We have problems where a) this isn't actually possible to do in js and b) we want adding the specifiers to restrict the ccis that are returned. Consequently, we can do something like this: { ac-1: { a: { ccis: [cci-2314] }, ccis: [cci-1234, cci-4321, cci-2314] } }.
    Use inspecjs to convert the incoming nist tags into their normalized forms, and then use that to go into the trie-esque data structure by going into it as far as the amount of specifiers that we have (so ac-1 a (2) is 3 levels), if there's nothing there then keep backing up a specifier (ac-1 a (2) has nothing, but ac-1 a has cci-2314) until you actually get results or you've backed yourself out of the structure entirely in which case just return the default ccis.

You'll then need to update the mappers and other locations as appropriate.


Future work

Update libs/inspecjs/src/raw_nist.ts to ensure that our NIST tags are all up to date. Maybe find out a way to automate this process.

Review the rest of what's going on in this mappings directory to see if we can simplify implementations / reduce redundancies like we're doing now with the nist/cci stuff.

@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch 2 times, most recently from 11d47a7 to e420048 Compare November 5, 2024 18:49
@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from 3e4980f to 078c006 Compare November 8, 2024 18:23
@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from a2d8389 to 4e5c4c4 Compare November 15, 2024 20:48

export const NIST_TO_CCI: Record<string, string[]> = nistToCciData;

export const HANDCRAFTED_DEFAULT_NIST_TO_CCI = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

difference between this HANDCRAFTED_DEFAULT_NIST_TO_CCI and old data is that I removed NIST->CCI mappings in data that were otherwise already existing in NIST_TO_CCI

@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from 4e5c4c4 to 0795b23 Compare November 18, 2024 20:31
@aaronlippold aaronlippold requested a review from Copilot November 20, 2024 06:03

Choose a reason for hiding this comment

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

Copilot reviewed 164 out of 178 changed files in this pull request and generated no suggestions.

Files not reviewed (14)
  • libs/hdf-converters/package.json: Language not supported
  • libs/hdf-converters/sample_jsons/anchore_grype_mapper/amazon-grype-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/anchore_grype_mapper/amazon-grype-withraw.json: Language not supported
  • libs/hdf-converters/sample_jsons/anchore_grype_mapper/anchore-grype-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/anchore_grype_mapper/anchore-grype-withraw.json: Language not supported
  • libs/hdf-converters/sample_jsons/anchore_grype_mapper/tensorflow-grype-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/anchore_grype_mapper/tensorflow-grype-withraw.json: Language not supported
  • libs/hdf-converters/sample_jsons/asff_mapper/asff-aws_foundational_security_best_practices_v1.0.0-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/asff_mapper/asff-cis_aws-foundations_benchmark_v1.2.0-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/asff_mapper/prowler-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/asff_mapper/trivy-image_golang-1.12-alpine-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/burpsuite_mapper/burpsuite-hdf-withraw.json: Language not supported
  • libs/hdf-converters/sample_jsons/burpsuite_mapper/burpsuite-hdf.json: Language not supported
  • libs/hdf-converters/sample_jsons/checklist_mapper/converted-RHEL8V1R3.ckl: Language not supported
Comments skipped due to low confidence (1)

libs/hdf-converters/data/converters/cciListXml2json.ts:64

  • The error message is unclear. It should specify the names of the output files.
console.error('You must provide the path to the input and three output files.');
@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch 4 times, most recently from 31f77ee to 544bee8 Compare November 29, 2024 15:02
@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from 814bc66 to 813ea65 Compare November 29, 2024 21:30
Signed-off-by: Joyce Quach <[email protected]>
…SON file and check in that file

Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
… static analysis tags if there are already existing found NIST tags and/or mapped CCI->NIST tags

Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
…lt NIST and CCI tags discussion

Signed-off-by: Joyce Quach <[email protected]>
…ONIX is an empty string representing the serialized CCI tags

Signed-off-by: Joyce Quach <[email protected]>
Signed-off-by: Joyce Quach <[email protected]>
@jtquach1 jtquach1 force-pushed the add-convert-cci-list-workflow branch from e22db52 to c7c5f81 Compare December 2, 2024 16:47
Copy link

sonarqubecloud bot commented Dec 2, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
9 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

3 participants