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

Hdf2ckl severity #5866

Merged
merged 81 commits into from
Jul 12, 2024
Merged

Hdf2ckl severity #5866

merged 81 commits into from
Jul 12, 2024

Conversation

kemley76
Copy link
Contributor

Fix to #5842.

kemley76 added 2 commits May 31, 2024 09:46
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
@kemley76 kemley76 requested a review from georgedias May 31, 2024 14:14
Signed-off-by: kemley76 <kemley@mitre.org>
@ejaronne
Copy link
Contributor

ejaronne commented Jun 3, 2024

image
The hdf2ckl export works properly, however, the Details tab should be:

Severity:
(tag.severity value)
image

Impact:
(Impact value)
image

Copy link
Contributor

@georgedias georgedias left a comment

Choose a reason for hiding this comment

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

Please see my inline comments

kemley76 added 2 commits June 5, 2024 11:56
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
@kemley76
Copy link
Contributor Author

kemley76 commented Jun 5, 2024

I have made more changes regarding this issue. This includes:

  • adding a severity tag upon importing a Checklist file
  • using a control's severity tag to compute its severity rather than defaulting to use only the impact value
  • compute the amount of "impact bubbles" in the results view with the impact rather than the severity
  • be able to sort the controls by impact when clicking the impact column header
  • be able to show falsy values in the details (like when there is impact of 0)

Done in previous commit, but not listed out

  • use severity tag (if available) when exporting form HDF to CKL

This shows the sorting and bubble usage. The bubbles indicate impact, the text indicates severity.
Screenshot 2024-06-05 at 11 48 52 AM

kemley76 added 3 commits June 5, 2024 12:19
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
Copy link
Contributor

@Amndeep7 Amndeep7 left a comment

Choose a reason for hiding this comment

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

Currently the behavior is severity if it's there, otherwise fallback to impact.

What happens if there is a security override coming from a ckl? What should the behavior be then? I think the behavior should be the override if it's there, then the severity, then fallback to impact.

What should we do about the severity override justification? Should that be shown in the details tab?

Questions for @ejaronne

kemley76 added 4 commits June 14, 2024 10:16
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
@kemley76
Copy link
Contributor Author

The original issue was caused by two things:

  1. When a control has a severity tag, Heimdall didn't care and computed the severity it displays with the impact, which can differ from this tag
  2. When exporting from hdf to checklist, the severity in the resulting checklist was computed with the hdf's impact and disregarded any severity tags.
    These have been addressed

Additional fixes that were addressed that may be beyond the scope of the original issue, but still needed to be addressed:

Compute a control's severity based on

  1. Any severity override (checklist vuln tag or hdf control tag)
  2. Any severity tag (checklist tag or hdf control tag)
  3. Impact

Display severity override information in results table
Screenshot 2024-06-14 at 4 31 46 PM
Display severity override information in details view
Screenshot 2024-06-14 at 4 32 37 PM

It was also noticed that these changes affected the center "Severity Counts" ring graph. Before the "severities" in the graph were computed directly from the impact. Now, if there is a severity tag or severity override, that will be shown in the graph instead. When importing checklists, it is now impossible to see severity: none in this graph since severity: none does not exist in the checklist world.

kemley76 added 4 commits June 17, 2024 09:54
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
@kemley76
Copy link
Contributor Author

One last behavioral change was made here:
Upon exporting form hdf to checklist, severity tags none and critical now map to low and high respectively.
This has the side effect that exporting from hdf to ckl and then back to hdf could result in different severity levels if they were originally none or critical.

kemley76 added 2 commits June 17, 2024 14:03
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
@kemley76
Copy link
Contributor Author

The impact and severity columns have now been separated and are being displayed with color coded chips.
Results View Changes

kemley76 added 3 commits June 17, 2024 15:55
Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: kemley76 <kemley@mitre.org>
…verity

Signed-off-by: kemley76 <kemley@mitre.org>
kemley76 and others added 5 commits July 8, 2024 21:22
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
kemley76 and others added 2 commits July 9, 2024 15:43
Co-authored-by: Amndeep Singh Mann <amann@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
kemley76 added 5 commits July 10, 2024 13:00
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
@kemley76 kemley76 added the ready-to-merge Used by mergify to identify if a PR is ready to merge into master. label Jul 10, 2024
@Amndeep7 Amndeep7 added ready-to-merge Used by mergify to identify if a PR is ready to merge into master. and removed ready-to-merge Used by mergify to identify if a PR is ready to merge into master. labels Jul 11, 2024
Copy link
Contributor

mergify bot commented Jul 11, 2024

This pull request has a conflict. Could you fix it @kemley76?

kemley76 and others added 2 commits July 11, 2024 14:05
Copy link

Copy link
Contributor

@georgedias georgedias left a comment

Choose a reason for hiding this comment

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

ready to merge

@georgedias georgedias merged commit 93b33ea into master Jul 12, 2024
17 of 19 checks passed
@mergify mergify bot deleted the hdf2ckl-severity branch July 12, 2024 17:37
aaronlippold pushed a commit that referenced this pull request Nov 20, 2024
* use severity tag in hdf2ckl mapping

Signed-off-by: kemley76 <kemley@mitre.org>

* use default values in severity check

Signed-off-by: kemley76 <kemley@mitre.org>

* update hdf2ckl test

Signed-off-by: kemley76 <kemley@mitre.org>

* fix inconsistencies with how severity is computed and displayed

Signed-off-by: kemley76 <kemley@mitre.org>

* linting

Signed-off-by: kemley76 <kemley@mitre.org>

* add clarifying comments for severity computation

Signed-off-by: kemley76 <kemley@mitre.org>

* update ckl2hdf tests

* remove unecessary lowercase conversion

Signed-off-by: kemley76 <kemley@mitre.org>

* show severityoverride and severity justification in details panel

Signed-off-by: kemley76 <kemley@mitre.org>

* severity override info displayed in results table

Signed-off-by: kemley76 <kemley@mitre.org>

* format results view impact column to show severity as well

Signed-off-by: kemley76 <kemley@mitre.org>

* linting

Signed-off-by: kemley76 <kemley@mitre.org>

* added severity and severity overrides to hdf2ckl and ckl2hdf

Signed-off-by: kemley76 <kemley@mitre.org>

* ensure severity low and critical get mapped properly in hdf2ckl

Signed-off-by: kemley76 <kemley@mitre.org>

* fix fallbacks in ControlRowHeader for showing severity override

Signed-off-by: kemley76 <kemley@mitre.org>

* linting

Signed-off-by: kemley76 <kemley@mitre.org>

* split impact and severity into two columns

Signed-off-by: kemley76 <kemley@mitre.org>

* linting

Signed-off-by: kemley76 <kemley@mitre.org>

* add information labels on severity and impact table headers

Signed-off-by: kemley76 <kemley@mitre.org>

* linting

Signed-off-by: kemley76 <kemley@mitre.org>

* add visual spacing between delta and severity level for overridden severity

Signed-off-by: kemley76 <kemley@mitre.org>

* update impact ranges for results table header tooltip

Signed-off-by: kemley76 <kemley@mitre.org>

* removed transparancy from v-tooltip backgrounds

Signed-off-by: Kaden Emley <kemley@mitre.org>

* refactor checklist mapper to use result type when parsing Json

Signed-off-by: Kaden Emley <kemley@mitre.org>

* use severity form Third_Party_Tools section if present upon ckl2hdf

Signed-off-by: Kaden Emley <kemley@mitre.org>

* ensure that impact is computed using computed severity upon ckl2hdf

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add data to ckl thirdPartyTools to ensure hdf's severity and impact are preserved

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add severityoverride tag to control when impact and severity differ

Signed-off-by: Kaden Emley <kemley@mitre.org>

* recombine severity into impact column and indicate if they differ

Signed-off-by: Kaden Emley <kemley@mitre.org>

* linting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add ability to filter controls by the presence of specific tags

Signed-off-by: Kaden Emley <kemley@mitre.org>

* create InfoCardRow component to alert user to any severity overrides

Signed-off-by: Kaden Emley <kemley@mitre.org>

* bring back severity column

Signed-off-by: Kaden Emley <kemley@mitre.org>

* linting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* remove impact column, only showing severity

Signed-off-by: Kaden Emley <kemley@mitre.org>

* revert changes to include severityoverride when severity and impact differ

Signed-off-by: Kaden Emley <kemley@mitre.org>

* ensure hdf to ckl to hdf doesn't add extra metadata

Signed-off-by: Kaden Emley <kemley@mitre.org>

* update hdf2ckl test

Signed-off-by: Kaden Emley <kemley@mitre.org>

* linting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* remove extra code leftover from removed impact column

Signed-off-by: Kaden Emley <kemley@mitre.org>

* removed ts specific code tested in frontend test that caused error

Signed-off-by: Kaden Emley <kemley@mitre.org>

* linting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* updated ckl2hdf tests to consider third party tools

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add checklist with overrides file to sample files

Signed-off-by: Kaden Emley <kemley@mitre.org>

* expanded checklist override test to include non-overridden vuln severities

Signed-off-by: Kaden Emley <kemley@mitre.org>

* added frontend test to ensure severity overrides can be filtered properly

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add cypress test to ensure severity override lables appear

Signed-off-by: Kaden Emley <kemley@mitre.org>

* clean up vue logic for severity override display

Signed-off-by: Kaden Emley <kemley@mitre.org>

* account for non-lowercase severity tags

Signed-off-by: Kaden Emley <kemley@mitre.org>

* remove unneeded code bits

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix sample loading in cypress test

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix hdf2checklist third party tools computation

Signed-off-by: Kaden Emley <kemley@mitre.org>

* update control search help menu with tag filter

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fixed issue with critical severity being lost in hdf to ckl to hdf

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix logic and complexity of hdf2ckl addHdfSpecificData

Signed-off-by: Kaden Emley <kemley@mitre.org>

* linting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* accounted for possiblity of nil severity tag when doing hdf2ckl

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add severity name constants in inspecJs as utility

Signed-off-by: Kaden Emley <kemley@mitre.org>

* added test util for version replacement for ckl and xccdf reverse testing

Signed-off-by: Kaden Emley <kemley@mitre.org>

* add parseJson to util file with better return type

Signed-off-by: Kaden Emley <kemley@mitre.org>

* relocate ckl2hdf helper function

Signed-off-by: Kaden Emley <kemley@mitre.org>

* refactor hdf2ckl computeImpact to use standard util function

Signed-off-by: Kaden Emley <kemley@mitre.org>

* remove redundant 'active-class' in results table's chips

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix weird autoformating instances in vue

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix comment typo

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix messed up test in checklist reverse mapper

Signed-off-by: Kaden Emley <kemley@mitre.org>

* fix typo

Co-authored-by: Amndeep Singh Mann <amann@mitre.org>

* refactored to remove unecessary type casting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* use more representative type for JSON parse output

Signed-off-by: Kaden Emley <kemley@mitre.org>

* simplify ckl mapper helper function

Signed-off-by: Kaden Emley <kemley@mitre.org>

* linting

Signed-off-by: Kaden Emley <kemley@mitre.org>

* remove unused imports

Signed-off-by: Kaden Emley <kemley@mitre.org>

* export inspecJS function for converting impact into severity

Signed-off-by: Kaden Emley <kemley@mitre.org>

* restart CI

---------

Signed-off-by: kemley76 <kemley@mitre.org>
Signed-off-by: Kaden Emley <kemley@mitre.org>
Co-authored-by: Amndeep Singh Mann <amann@mitre.org>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Used by mergify to identify if a PR is ready to merge into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants