From 8cc7ff3c4508fe343df1f827086a9c9d358fdd37 Mon Sep 17 00:00:00 2001 From: Alden Hilton <106177711+adhilto@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:48:45 -0700 Subject: [PATCH] Add config file support for control omission (#431) * Update Config.md with section on omitting policies * Create omit_policies.yaml * Update Config.md with example config specific to scubagoggles * Update Config.md minor grammatical changes * Update Config.md correct link * Update Config.md correct minor typo * Add config file support for omitting policies * Correct some linter errors * Correct more linter errors * Move pylint disable comment * Update Config.md, clarify usage of single config file * Update omit_policies.yaml, comment about usage of anchors * Warn for unexpected control ids * must appease the linter * Remove commented out code Co-authored-by: David Bui <105074908+buidav@users.noreply.github.com> * Break out omitpolicy validation into separate function. * Add final newline to file for the linter * Add special handling for rules * Remove duplicate word Co-authored-by: mitchelbaker-cisa <149098823+mitchelbaker-cisa@users.noreply.github.com> * Create warn wrapper function to clean tqdm output --------- Co-authored-by: David Bui <105074908+buidav@users.noreply.github.com> Co-authored-by: mitchelbaker-cisa <149098823+mitchelbaker-cisa@users.noreply.github.com> --- docs/usage/Config.md | 19 +- sample-config-files/omit_policies.yaml | 22 ++ scubagoggles/orchestrator.py | 23 +- scubagoggles/reporter/reporter.py | 267 +++++++++++++----- scubagoggles/reporter/scripts/main.js | 3 + .../reporter/styles/FrontPageStyle.css | 2 +- scubagoggles/scuba_argument_parser.py | 53 +++- 7 files changed, 316 insertions(+), 73 deletions(-) create mode 100644 sample-config-files/omit_policies.yaml diff --git a/docs/usage/Config.md b/docs/usage/Config.md index 143eba01..44b87fa1 100644 --- a/docs/usage/Config.md +++ b/docs/usage/Config.md @@ -6,12 +6,12 @@ All ScubaGoggles [parameters](/docs/usage/Parameters.md) can be placed into a co > If a parameter is specified both on the command-line and in a configuration file, the command-line parameter has precedence over the config file. ## Sample Configuration Files -[Sample config files](/sample-config-files) are available in the repo and are discussed below. +[Sample config files](/sample-config-files) are available in the repo and are discussed below. When executing ScubaGoggles, only a single config file can be read in; we recommend looking through the following examples and constructing a config file that best suits your use case. ### Basic Usage The [basic use](/sample-config-files/basic_config.yaml) example config file specifies the `outpath`, `baselines`, and `quiet` parameters. -ScubaGoggles can be invokes with this config file: +ScubaGoggles can be invoked with this config file: ``` scubagoggles gws --config basic_config.yaml ``` @@ -21,6 +21,19 @@ It can also be invoked while overriding the `baselines` parameter. scubagoggles gws --config basic_config.yaml -b gmail chat ``` +### Omit Policies + +In some cases, it may be appropriate to omit specific policies from ScubaGoggles evaluation. For example: +- When a policy is implemented by a third-party service that ScubaGoggles does not audit. +- When a policy is not applicable to your organization (e.g., policy GWS.GMAIL.4.3v0.3, which is only applicable to federal, executive branch, departments and agencies). + +The `omitpolicy` top-level key, shown in this [example ScubaGoggles configuration file](/sample-config-files/omit_policies.yaml), allows the user to specify the policies that should be omitted from the ScubaGoggles report. Omitted policies will show up as "Omitted" in the HTML report and will be colored gray. Omitting policies must only be done if the omissions are approved within an organization's security risk management process. **Exercise care when omitting policies because this can inadvertently introduce blind spots when assessing your system.** + +For each omitted policy, the config file allows you to indicate the following: +- `rationale`: The reason the policy should be omitted from the report. This value will be displayed in the "Details" column of the report. ScubaGoggles will output a warning if no rationale is provided. +- `expiration`: Optional. A date after which the policy should no longer be omitted from the report. The expected format is yyyy-mm-dd. + + ## Navigation - Continue to [Usage: Examples](/docs/usage/Examples.md) -- Return to [Documentation Home](/README.md) \ No newline at end of file +- Return to [Documentation Home](/README.md) diff --git a/sample-config-files/omit_policies.yaml b/sample-config-files/omit_policies.yaml new file mode 100644 index 00000000..8004fb70 --- /dev/null +++ b/sample-config-files/omit_policies.yaml @@ -0,0 +1,22 @@ +# YAML configuration demonstrating omitting policies from ScubaGoggles evaluation. +# Any omitted policies should be carefully considered and documented as part of an +# organization's cybersecurity risk management program process and practices. + +baselines: [gmail, chat, drive] + +omitpolicy: + GWS.GMAIL.3.1v0.3: + rationale: "Known false positive; our SPF policy currently cannot to be retrieved via ScubaGoggles due to a split + horizon setup but is available publicly." + expiration: "2023-12-31" + GWS.CHAT.5.1v0.3: + rationale: &DLPRationale "The DLP capability required by the baselines is implemented by third party product, [x], + which ScubaGoggles does not have the ability to check." + GWS.DRIVEDOCS.7.1v0.3: + rationale: *DLPRationale + +# The "&" character used in the above example defines an anchor, which saves a value +# for future reference. This value can then be retrieved with the "*" character. See +# https://yaml.org/spec/1.2.2/#692-node-anchors for more details. In this case, the +# anchor allows you to configure multiple omissions that share the same rationale +# without repeating yourself. diff --git a/scubagoggles/orchestrator.py b/scubagoggles/orchestrator.py index 9c0ae31c..b5b68931 100644 --- a/scubagoggles/orchestrator.py +++ b/scubagoggles/orchestrator.py @@ -155,9 +155,10 @@ def _generate_summary(cls, stats: dict) -> str: n_fail = stats["Failures"] n_manual = stats["Manual"] n_error = stats["Errors"] + n_omit = stats['Omit'] pass_summary = (f"
{n_success}" - f" {cls._pluralize('test', 'tests', n_success)} passed
") + f" {cls._pluralize('pass', 'passes', n_success)}") # The warnings, failures, and manuals are only shown if they are # greater than zero. Reserve the space for them here. They will @@ -166,21 +167,26 @@ def _generate_summary(cls, stats: dict) -> str: failure_summary = "
" manual_summary = "
" error_summary = "
" + omit_summary = "
" if n_warn > 0: warning_summary = (f"
{n_warn}" f" {cls._pluralize('warning', 'warnings', n_warn)}
") if n_fail > 0: failure_summary = (f"
{n_fail}" - f" {cls._pluralize('test', 'tests', n_fail)} failed
") + f" {cls._pluralize('failure', 'failures', n_fail)}") if n_manual > 0: manual_summary = (f"
{n_manual} manual" - f" {cls._pluralize('check', 'checks', n_manual)} needed
") + f" {cls._pluralize('check', 'checks', n_manual)}") if n_error > 0: error_summary = (f"
{n_error}" f" {cls._pluralize('error', 'errors', n_error)}
") + if n_omit > 0: + omit_summary = (f"
{n_omit}" + " omitted
") - return f"{pass_summary}{warning_summary}{failure_summary}{manual_summary}{error_summary}" + return f"{pass_summary}{warning_summary}{failure_summary}" \ + f"{manual_summary}{omit_summary}{error_summary}" def _run_reporter(self): """ @@ -245,6 +251,11 @@ def _run_reporter(self): tenant_info = json.load(file)['tenant_info'] tenant_domain = tenant_info['domain'] + # Determine if any controls were omitted in the config file + omissions = {} + if 'omitpolicy' in args and args.omitpolicy is not None: + omissions = args.omitpolicy + # Create the individual report files out_jsonfile = args.outjsonfilename summary = {} @@ -284,7 +295,9 @@ def _run_reporter(self): prod_to_fullname, baseline_policies[product], successful_calls, - unsuccessful_calls) + unsuccessful_calls, + omissions, + products_bar) stats_and_data[product] = \ reporter.rego_json_to_ind_reports(test_results_data, out_folder) diff --git a/scubagoggles/reporter/reporter.py b/scubagoggles/reporter/reporter.py index 63a775b6..d7bd8872 100644 --- a/scubagoggles/reporter/reporter.py +++ b/scubagoggles/reporter/reporter.py @@ -12,6 +12,8 @@ from scubagoggles.scuba_constants import API_LINKS +# Nine instance attributes is reasonable in this case. +# pylint: disable-next=too-many-instance-attributes class Reporter: """The Reporter class generates the HTML files containing the conformance @@ -27,7 +29,9 @@ def __init__(self, prod_to_fullname: dict, product_policies: list, successful_calls: set, - unsuccessful_calls: set): + unsuccessful_calls: set, + omissions: dict, + progress_bar = None): """Reporter class initialization @@ -39,6 +43,11 @@ def __init__(self, read from the baseline markdown :param successful_calls: set with the set of successful calls :param unsuccessful_calls: set with the set of unsuccessful calls + :param omissions: dict with the omissions specified in the config + file (empty dict if none omitted) + :param progress_bar: Optional TQDM instance. If provided, the + progress bar will be cleared before any warnings are printed + while generating the report, for cleaner output. """ self._product = product @@ -48,6 +57,11 @@ def __init__(self, self._successful_calls = successful_calls self._unsuccessful_calls = unsuccessful_calls self._full_name = prod_to_fullname[product] + self._omissions = { + # Lowercase all the keys for case-insensitive comparisons + key.lower(): value for key, value in omissions.items() + } + self.progress_bar = progress_bar @staticmethod def _get_test_result(requirement_met: bool, criticality: str, no_such_events: bool) -> str: @@ -132,6 +146,80 @@ def build_front_page_html(fragments: list, tenant_info: dict) -> str: html = html.replace('{{TENANT_DETAILS}}', meta_data) return html + def _is_control_omitted(self, control_id : str) -> bool: + ''' + Determine if the supplied control was marked for omission in the + config file and if the expiration date has passed, if applicable. + :param control_id: the control ID, e.g., GWS.GMAIL.1.1v1. Case- + insensitive. + ''' + # Lowercase for case-insensitive comparison + control_id = control_id.lower() + if control_id in self._omissions: + # The config indicates the control should be omitted + if self._omissions[control_id] is None: + # If a user doesn't provide either a rationale or expiration + # date, the control ID will be in the omissions dict but it + # will map to None. + return True + if 'expiration' not in self._omissions[control_id]: + return True + # An expiration date for the omission expiration was + # provided. Evaluate the date to see if the control should + # still be omitted. + raw_date = self._omissions[control_id]['expiration'] + if raw_date is None or raw_date == "": + # If the expiration date is left blank or an empty string, + # omit the policy + return True + try: + expiration_date = datetime.strptime(raw_date, '%Y-%m-%d') + except ValueError: + # Malformed date, don't omit the policy + warning = f"Config file indicates omitting {control_id}, " \ + f"but the provided expiration date, {raw_date}, is " \ + "malformed. The expected format is yyyy-mm-dd. Control" \ + " will not be omitted." + self._warn(warning, RuntimeWarning) + return False + now = datetime.now() + if expiration_date > now: + # The expiration date is in the future, omit the policy + return True + # The expiration date is passed, don't omit the policy + warning = f"Config file indicates omitting {control_id}, but " \ + f"the provided expiration date, {raw_date}, has passed. " \ + "Control will not be omitted." + self._warn(warning, RuntimeWarning) + return False + + def _get_omission_rationale(self, control_id : str) -> str: + ''' + Return the rationale indicated in the config file for the indicated + control, if provided. If not, return a string warning the user that + no rationale was provided. + :param control_id: the control ID, e.g., GWS.GMAIL.1.1v1. Case- + insensitive. + ''' + # Lowercase for case-insensitive comparison + control_id = control_id.lower() + if control_id not in self._omissions: + raise RuntimeError(f"{control_id} not omitted in config file, " \ + "cannot fetch rationale") + # If any of the following conditions is true, no rationale was + # provided + no_rationale = \ + (self._omissions[control_id] is None) or \ + ('rationale' not in self._omissions[control_id]) or \ + (self._omissions[control_id]['rationale'] is None) or \ + (self._omissions[control_id]['rationale'] == "") + if no_rationale: + warning = f"Config file indicates omitting {control_id}, but " \ + "no rationale provided." + self._warn(warning, RuntimeWarning) + return "Rationale not provided." + return self._omissions[control_id]['rationale'] + def _build_report_html(self, fragments: list) -> str: ''' Adds data into HTML Template and formats the page accordingly @@ -252,6 +340,43 @@ def _get_summary_category(result: str) -> str: return "Passes" raise ValueError(f"Unexpected result, {result}", RuntimeWarning) + def _handle_rules_omission(self, control_id : str, tests : list): + '''Process the test results for the rules report if the rules control + was omitted. + + :control_id: The control ID for the rules control. + :tests: A list of test result dictionaries. + ''' + table_data = [] + for test in tests: + if 'Not-Implemented' in test['Criticality']: + # The easiest way to identify the common controls "rules" + # results that belong to the Common Controls report is they're + # marked as Not-Implemented. This if excludes them from the + # rules report. + continue + rationale = self._get_omission_rationale(control_id) + table_data.append({ + 'Control ID': control_id, + 'Rule Name': test['Requirement'], + 'Result': 'Omitted', + 'Criticality': test['Criticality'], + 'Rule Description': f'N/A; test omitted by user. {rationale}' + }) + return table_data + + def _warn(self, *args, **kwargs): + ''' + Wrapper for the warnings.warn function, that clears and refreshes the + progress bar if one has been provided, to keep the output clean. + Accepts all the arguments the warnings.warn function accepts. + ''' + if self.progress_bar is not None: + self.progress_bar.clear() + warnings.warn(*args, **kwargs) + if self.progress_bar is not None: + self.progress_bar.refresh() + def rego_json_to_ind_reports(self, test_results: list, out_path: str) -> list: ''' Transforms the Rego JSON output into individual HTML and JSON reports @@ -275,14 +400,12 @@ def rego_json_to_ind_reports(self, test_results: list, out_path: str) -> list: "Errors": 0, "Failures": 0, "Warnings": 0, - # While ScubaGoggles doesn't currently have the capability to omit controls, - # this key is needed here to maintain parity with ScubaGear. "Omit": 0 } for baseline_group in self._product_policies: table_data = [] - results_data ={} + results_data = {} for control in baseline_group['Controls']: tests = [test for test in test_results if test['PolicyId'] == control['Id']] if len(tests) == 0: @@ -295,66 +418,84 @@ def rego_json_to_ind_reports(self, test_results: list, out_path: str) -> list: 'Result': "Error - Test results missing", 'Criticality': "-", 'Details': f'Report issue on {issues_link}'}) - warnings.warn(f"No test results found for Control Id {control['Id']}", + self._warn(f"No test results found for Control Id {control['Id']}", RuntimeWarning) - else: - for test in tests: - failed_prereqs = self._get_failed_prereqs(test) - if len(failed_prereqs) > 0: - report_stats["Errors"] += 1 - failed_details = self._get_failed_details(failed_prereqs) - table_data.append({ - 'Control ID': control['Id'], - 'Requirement': control['Value'], - 'Result': "Error", - 'Criticality': test['Criticality'], - 'Details': failed_details}) - else: - result = self._get_test_result(test['RequirementMet'], - test['Criticality'], - test['NoSuchEvent']) - - details = test['ReportDetails'] - - if result == "No events found": - warning_icon = "\ - " - details = warning_icon + " " + test['ReportDetails'] - # As rules doesn't have its own baseline, Rules and Common Controls - # need to be handled specially - if product_capitalized == "Rules": - if 'Not-Implemented' in test['Criticality']: - # The easiest way to identify the GWS.COMMONCONTROLS.13.1v1 - # results that belong to the Common Controls report is they're - # marked as Not-Implemented. This if excludes them from the - # rules report. - continue - report_stats[self._get_summary_category(result)] += 1 - table_data.append({ - 'Control ID': control['Id'], - 'Rule Name': test['Requirement'], - 'Result': result, - 'Criticality': test['Criticality'], - 'Rule Description': test['ReportDetails']}) - elif product_capitalized == "Commoncontrols" \ - and baseline_group['GroupName'] == 'System-defined Rules' \ - and 'Not-Implemented' not in test['Criticality']: - # The easiest way to identify the System-defined Rules - # results that belong to the Common Controls report is they're - # marked as Not-Implemented. This if excludes the full results - # from the Common Controls report. - continue - else: - report_stats[self._get_summary_category(result)] += 1 - table_data.append({ - 'Control ID': control['Id'], - 'Requirement': control['Value'], - 'Result': result, - 'Criticality': test['Criticality'], - 'Details': details}) + continue + if self._is_control_omitted(control['Id']): + # Handle the case where the control was omitted + if product_capitalized == "Rules": + # Rules is a special case + rules_data = self._handle_rules_omission(control['Id'], tests) + table_data.extend(rules_data) + report_stats['Omit'] += len(rules_data) + continue + report_stats['Omit'] += 1 + rationale = self._get_omission_rationale(control['Id']) + table_data.append({ + 'Control ID': control['Id'], + 'Requirement': control['Value'], + 'Result': "Omitted", + 'Criticality': tests[0]['Criticality'], + 'Details': f'Test omitted by user. {rationale}' + }) + continue + for test in tests: + failed_prereqs = self._get_failed_prereqs(test) + if len(failed_prereqs) > 0: + report_stats["Errors"] += 1 + failed_details = self._get_failed_details(failed_prereqs) + table_data.append({ + 'Control ID': control['Id'], + 'Requirement': control['Value'], + 'Result': "Error", + 'Criticality': test['Criticality'], + 'Details': failed_details}) + continue + result = self._get_test_result(test['RequirementMet'], + test['Criticality'], + test['NoSuchEvent']) + + details = test['ReportDetails'] + + if result == "No events found": + warning_icon = "\ + " + details = warning_icon + " " + test['ReportDetails'] + # As rules doesn't have its own baseline, Rules and Common Controls + # need to be handled specially + if product_capitalized == "Rules": + if 'Not-Implemented' in test['Criticality']: + # The easiest way to identify the GWS.COMMONCONTROLS.13.1v1 + # results that belong to the Common Controls report is they're + # marked as Not-Implemented. This if excludes them from the + # rules report. + continue + report_stats[self._get_summary_category(result)] += 1 + table_data.append({ + 'Control ID': control['Id'], + 'Rule Name': test['Requirement'], + 'Result': result, + 'Criticality': test['Criticality'], + 'Rule Description': test['ReportDetails']}) + elif product_capitalized == "Commoncontrols" \ + and baseline_group['GroupName'] == 'System-defined Rules' \ + and 'Not-Implemented' not in test['Criticality']: + # The easiest way to identify the System-defined Rules + # results that belong to the Common Controls report is they're + # marked as Not-Implemented. This if excludes the full results + # from the Common Controls report. + continue + else: + report_stats[self._get_summary_category(result)] += 1 + table_data.append({ + 'Control ID': control['Id'], + 'Requirement': control['Value'], + 'Result': result, + 'Criticality': test['Criticality'], + 'Details': details}) markdown_group_name = "-".join(baseline_group['GroupName'].split()) md_basename = "commoncontrols" if self._product == "rules" else self._product group_reference_url = f'{github_url}/blob/v{tool_version}/baselines/'\ diff --git a/scubagoggles/reporter/scripts/main.js b/scubagoggles/reporter/scripts/main.js index b48881f8..2706e99c 100644 --- a/scubagoggles/reporter/scripts/main.js +++ b/scubagoggles/reporter/scripts/main.js @@ -24,6 +24,9 @@ const colorRows = () => { else if (rows[i].children[statusCol].innerHTML === "Pass") { rows[i].style.background = "var(--test-pass)"; } + else if (rows[i].children[statusCol].innerHTML === "Omitted") { + rows[i].style.background = "var(--test-other)"; + } else if (rows[i].children[criticalityCol].innerHTML.includes("Not-Implemented")) { rows[i].style.background = "var(--test-other)"; } diff --git a/scubagoggles/reporter/styles/FrontPageStyle.css b/scubagoggles/reporter/styles/FrontPageStyle.css index eea6f88b..c1f2daf4 100644 --- a/scubagoggles/reporter/styles/FrontPageStyle.css +++ b/scubagoggles/reporter/styles/FrontPageStyle.css @@ -14,7 +14,7 @@ footer { display: inline-block; padding: 5px; border-radius: 5px; - min-width: 140px; + min-width: 100px; text-align: center; } diff --git a/scubagoggles/scuba_argument_parser.py b/scubagoggles/scuba_argument_parser.py index 2bf89a51..8bb7732d 100644 --- a/scubagoggles/scuba_argument_parser.py +++ b/scubagoggles/scuba_argument_parser.py @@ -2,8 +2,13 @@ Class for parsing the config file and command-line arguments. """ +import warnings import argparse +from pathlib import Path + import yaml +from scubagoggles.orchestrator import Orchestrator +from scubagoggles.reporter.md_parser import read_baseline_docs class ScubaArgumentParser: """ @@ -58,7 +63,11 @@ def parse_args_with_config(self) -> argparse.Namespace: if param in cli_args: continue vars(args)[param] = config[param] - # Return the args (argparse.Namespace) as a dictionary + + # Check for logical errors in the resulting configuration + self.validate_config(args) + + # Return the args (argparse.Namespace) return args @classmethod @@ -84,3 +93,45 @@ def _get_explicit_cli_args(cls, args : argparse.Namespace) -> dict: aux_parser.add_argument(*dests) cli_args, _ = aux_parser.parse_known_args() return cli_args + + @staticmethod + def validate_config(args : argparse.Namespace) -> None: + """ + Check for an logical errors in the advanced ScubaGoggles configuration + options. NOTE: "omitpolicy" is the only such option for now; more to + come. + """ + if 'omitpolicy' in args: + ScubaArgumentParser.validate_omissions(args) + + @staticmethod + def validate_omissions(args : argparse.Namespace) -> None: + """ + Warn for any control IDs configured for omission that aren't in the + set of IDs covered by the baselines specificied in --baselines. + """ + products = Orchestrator.gws_products()['prod_to_fullname'] + prod_to_fullname = { + key: products[key] + for key in args.baselines + if key in products + } + + # Parse the baselines to determine the set of valid control IDs + path = Path(args.documentpath).resolve() + baseline_policies = read_baseline_docs(path, prod_to_fullname) + control_ids = set() + for product_baseline in baseline_policies.values(): + for group in product_baseline: + for control in group['Controls']: + control_ids.add(control['Id'].lower()) + + # Warn for any unexpected IDs + for control_id in args.omitpolicy: + if control_id.lower() not in control_ids: + warnings.warn("Config file indicates omitting " \ + f"{control_id}, but {control_id} is not one of the " \ + "controls encompassed by the baselines indicated " \ + "by the baselines parameter. Control " \ + + "will not be omitted.")