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.

Sign up for GitHub

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

Improve xccdf_results_mapper when converting XCCDF Results to HDF Results #4255

Merged
merged 28 commits into from
Jul 13, 2023
Merged

Improve xccdf_results_mapper when converting XCCDF Results to HDF Results #4255

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
403987d
Improve xccdf_results_mapper when converting XCCDF->HDF
candrews Apr 12, 2023
88cd988
Merge branch 'master' into xccdf_results_mapper-improvements
aaronlippold Apr 13, 2023
25b361c
Minor changes requested by code review
candrews May 9, 2023
e8bc220
Use the "RegExp.exec()" method instead
candrews May 9, 2023
263cf60
Set impact to 0 for 'notapplicable' and 'informational' results
candrews May 9, 2023
c2640f5
Don't handle every array item within each array item
candrews May 11, 2023
51f047f
"version" should use "version.text" (not just "version")
candrews May 11, 2023
2f63274
For version, prefer version over id
candrews May 22, 2023
6992f6d
For version, remove unnecessary comment
candrews May 22, 2023
9276f02
Remove unnecessary String conversion
candrews May 22, 2023
71fad37
Add tsdoc to getRulesInGroup
candrews May 22, 2023
4b01555
Merge branch 'master' into xccdf_results_mapper-improvements
Amndeep7 May 23, 2023
f216a6f
removed 'id' as a potential path for 'version'. the complianceascode…
Amndeep7 May 23, 2023
747bdc8
linting
Amndeep7 May 23, 2023
2f618b2
Use triple equals for string comparson
candrews May 23, 2023
5a9cf54
Various fixes
candrews Jun 17, 2023
aad9481
Correct "refs" to comply with schema
candrews Jun 21, 2023
68ab499
Only include description if it has a label
candrews Jun 21, 2023
587282d
make the nist family part of the regexes only match against valid nis…
Amndeep7 Jun 23, 2023
d637e70
Use `as unknown as ControlDescription` instead of `as any`
candrews Jun 23, 2023
8df5b2d
Run lint on src/nist.ts
candrews Jun 23, 2023
cd8b2ee
Regenerate samples
candrews Jun 23, 2023
fc89563
Use concise character class syntax '\d' instead of '[0-9]'.
candrews Jun 23, 2023
ebdaade
the treemap expects a canonized form of the nist controls that are no…
Amndeep7 Jul 13, 2023
d855250
get rid of dupe nist tags - even if there were dupes in the original …
Amndeep7 Jul 13, 2023
e926155
could simplify the default_partial_config implementation and also ran…
Amndeep7 Jul 13, 2023
9e93622
sonarqube
Amndeep7 Jul 13, 2023
cd2aaa6
Merge branch 'master' into xccdf_results_mapper-improvements
Amndeep7 Jul 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184,689 changes: 118,797 additions & 65,892 deletions ...ers/sample_jsons/xccdf_results_mapper/xccdf-openscap-ComplianceAsCode-ubuntu1804-hdf.json
View file
Open in desktop

Large diffs are not rendered by default.

7,673 changes: 4,719 additions & 2,954 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-openscap-rhel7-hdf-withraw.json
View file
Open in desktop

Large diffs are not rendered by default.

7,685 changes: 4,725 additions & 2,960 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-openscap-rhel7-hdf.json
View file
Open in desktop

Large diffs are not rendered by default.

9,540 changes: 6,010 additions & 3,530 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-openscap-rhel8-hdf-withraw.json
View file
Open in desktop

Large diffs are not rendered by default.

9,480 changes: 5,980 additions & 3,500 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-openscap-rhel8-hdf.json
View file
Open in desktop

Large diffs are not rendered by default.

6,446 changes: 3,974 additions & 2,472 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-scc-rhel7-hdf-withraw.json
View file
Open in desktop

Large diffs are not rendered by default.

6,434 changes: 3,968 additions & 2,466 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-scc-rhel7-hdf.json
View file
Open in desktop

Large diffs are not rendered by default.

7,978 changes: 4,981 additions & 2,997 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-scc-rhel8-hdf-withraw.json
View file
Open in desktop

Large diffs are not rendered by default.

7,938 changes: 4,961 additions & 2,977 deletions libs/hdf-converters/sample_jsons/xccdf_results_mapper/xccdf-scc-rhel8-hdf.json
View file
Open in desktop

Large diffs are not rendered by default.

105 changes: 92 additions & 13 deletions libs/hdf-converters/src/xccdf-results-mapper.ts
View file
Open in desktop
Amndeep7 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {ExecJSON} from 'inspecjs';
import {ExecJSON, is_control, parse_nist} from 'inspecjs';
import _ from 'lodash';
import {version as HeimdallToolsVersion} from '../package.json';
import {
Expand All @@ -15,6 +15,15 @@ import {
DEFAULT_STATIC_CODE_ANALYSIS_NIST_TAGS
} from './utils/global';

const nistCanonConfig = {
add_spaces: true,
allow_letters: true,
max_specifiers: 5,
pad_zeros: true,
add_periods: false,
add_parens: true
};

const IMPACT_MAPPING: Map<string, number> = new Map([
['critical', 0.9],
['high', 0.7],
Expand Down Expand Up @@ -151,11 +160,18 @@ function extractCci(input: IIdent | IIdent[]): string[] {
}

function nistTag(input: IIdent | IIdent[]): string[] {
Amndeep7 marked this conversation as resolved.
Show resolved Hide resolved
const identifiers: string[] = extractCci(input);
return CCI_NIST_MAPPING.nistFilter(
identifiers,
extractCci(input),
DEFAULT_STATIC_CODE_ANALYSIS_NIST_TAGS,
false
).concat(
asArray(input)
.filter((x) => !!x)
.map((x) => x.text)
.map(parse_nist)
.filter((x) => !!x)
.filter(is_control)
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like your sample is not updated properly still? or this function is borked lol

xccdf-scc-rhel7-hdf.json

image

seems like the nist tag array is including the stig control ids, which it shouldn't do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is_control(parse_nist('SV-86515')) return true... so as far as inspectjs is concerned, 'SV-86515' is a nist control and therefore should be included.

Am I misunderstanding nist.js? How should I determine if an item should be included or excluded from this list?

Copy link
Contributor

Choose a reason for hiding this comment

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

this feels like a failure on inspecjs' part in not removing things that are clearly wrong. parse_nist should throw errors/explicitly fail when they are not nist controls/control families. it not doing so confuses the type system by mistakenly typing that string. is_control more or less is only used to narrow down types and doesn't do much actual validation.

anyways as shown there and in 'raw_nist.ts', nist controls look like 'two_letter_family-small_number possibly followed by letters and numbers in parentheses' whereas stig ids are 'S?V-5__or_6_digit_number'.

the issue that arose here is that it thinks 'sv' is a control family and then processes the numbers after it as like 'control "big number" from within that control family' but doesn't take into account the reality of the situation where 'sv' is not a valid control family nor is there a control with an index as high as '86515' in existence.

looking briefly, it seems like the regexes that parse_nist uses are vastly broader than the actually allowed set of inputs. i will experiment to see if it's possible to restrict it to allowed control families.

.map((x) => x.canonize(nistCanonConfig))
);
}

Expand Down Expand Up @@ -336,13 +352,27 @@ export class XCCDFResultsMapper extends BaseConverter {
) as string
},
rule_id: {path: 'id'},
check: {path: 'check'},
check: {
path: 'check',
transformer: (
data: Record<string, unknown> | Record<string, unknown>[]
) => JSON.stringify(data, null, 2)
},
fix_id: {path: 'fix.id'},
fixtext_fixref: {
path: ['fixtext.fixref.text', 'fixtext.fixref']
path: ['fixtext.fixref.text', 'fixtext.fixref'],
transformer: (text: string) => text || undefined
},
ident: {
path: 'ident',
transformer: (text: string) => text || undefined
},
reference: {
path: 'reference',
transformer: (data: Record<string, unknown>) => ({
references: data
})
},
ident: {path: 'ident'},
reference: {path: 'reference'},
selected: {path: 'selected'},
weight: {path: 'weight'},
profiles: [
Expand All @@ -365,7 +395,23 @@ export class XCCDFResultsMapper extends BaseConverter {
path: ['ruleResult']
},
value: {
path: ['values']
path: ['values'],
transformer: (
values: Record<string, unknown> | Record<string, unknown>[]
) =>
asArray(values).map((value) => ({
title: _.get(value, 'title'),
description:
_.get(value, 'description.text') ||
_.get(value, 'description'),
warning:
_.get(value, 'warning.text') || _.get(value, 'warning'),
value: _.get(value, 'value'),
Id: _.get(value, 'Id'),
id: _.get(value, 'id'),
type: _.get(value, 'type'),
interactive: _.get(value, 'interactive')
}))
},
transformer: (data: Record<string, unknown>) => ({
...conditionallyProvideAttribute(
Expand All @@ -375,7 +421,19 @@ export class XCCDFResultsMapper extends BaseConverter {
)
})
},
refs: [],
refs: [
Amndeep7 marked this conversation as resolved.
Show resolved Hide resolved
{
path: 'reference',
ref: {
path: 'text',
transformer: (text: string) => text || undefined
},
url: {
path: 'href',
transformer: (text: string) => text || undefined
}
}
],
source_location: {},
title: {path: ['title.text', 'title']},
id: {path: ['id']},
Expand All @@ -392,16 +450,34 @@ export class XCCDFResultsMapper extends BaseConverter {
{
data: {
path: ['check.check-content-ref.name'],
Amndeep7 marked this conversation as resolved.
Show resolved Hide resolved
transformer: parseHtml
transformer: (text: string | string[]) =>
asArray(text).join('\n') || undefined
},
label: 'check'
},
{
data: {
path: ['fixtext', 'fix'],
transformer: parseHtml
path: ['fixtext.text', 'fixtext', 'fix.text', 'fix'],
transformer: (text: string | string[]) =>
asArray(text).map(parseHtml).join('\n') || undefined
Amndeep7 marked this conversation as resolved.
Show resolved Hide resolved
},
label: 'fix'
},
{
data: {
path: ['rationale.text', 'rationale'],
transformer: (text: string | string[]) =>
asArray(text).map(parseHtml).join('\n') || undefined
},
label: 'rationale'
},
{
data: {
path: ['warning'],
transformer: (text: string | string[]) =>
asArray(text).map(parseHtml).join('\n') || undefined
},
label: 'warning'
}
],
impact: {
Expand All @@ -412,7 +488,10 @@ export class XCCDFResultsMapper extends BaseConverter {
>;
if (ruleResult) {
const result = _.get(ruleResult, 'result') as string;
if (result === 'notapplicable' || result === 'informational') {
if (
result === 'notapplicable' ||
result === 'informational'
) {
return 0;
}
}
Expand Down