Policy API Integration #499
Conversation
|
Tagging @mdueltgen @jkaufman-mitre for awareness, this PR contains baseline changes. No substantive changes to the policies, just changes along the line of changing the policies section headers to "Policies" for consistency. |
There was a problem hiding this comment.
I've barely scratched the surface of the testing that needs to be done, but here's a first round of feedback.
Two types of feedback:
- Change now
- Change soon but after this PR.
I've left individual comments on changes that should be made before this PR is merged.
I'm listing the changes that can wait here in this comment. In the spirit of keeping the scope of this PR from expanding any further, these changes can wait but should be tracked as issues before this PR is merged.
Simplify the rego code
Controls that have policy API support should exclusively use the policy API. No need to maintain the old reports code. Would greatly simplify the code. (I know you note this in the description but we still need to track it as an issue).
Rework the baseline version logic.
This code assumes that all controls will have the same version number. This is not a safe assumption. Pre-version 1.0, this will always be the case. But after the 1.0 release, if a control needs to be updated, only the version number of that control will be updated. For reference, see the Exchange Online baseline. Note that not all controls have the same version number (e.g., MS.EXO.1.1v1 and MS.EXO.2.2v2).
Update the "prerequisites" field of the rego tests
As noted in a comment in the reporter, "If Prerequisites is not defined, assume the test just depends on the reports API." This is no longer a safe assumption.
Update the report details message for controls relying on log events
Clearly indicate the controls that can only be checked via the log events. These checks give imperfect visibility which should be highlighted to the users. For example, if the log events for the top level OU is show compliance, ScubaGoggles will currently report "Requirement met for all OUs and groups," but in reality we cannot know that with full confidence.
This has been the plan all along. I approached this conservatively and preserved the log event code to keep the testing working.
The way I've implemented the "version suffix" changes doesn't preclude the possibility of having different suffixes for certain baseline policies. When we do end up having different suffixes on the baselines, it will not require much additional coding. It must be implemented such that there is only one source of the baseline version, most likely the markdown file where the baseline is defined. The correct suffix for the policies can be obtained from the source and driven across the system through the code instead of hard-coding the suffix in multiple places. |
getopa: tolerate missing "v" in specified version
There was a problem hiding this comment.
In the Testing section, it would help to have instructions on how to update any dependencies (automatically by running specific scripts/commands) or download any required packages.
Since this PR covers major updates, assuming running the tool now requires a different command (or atleast different flags), listing the instructions out in the Testing section would help.
… (see 11/6 change) commoncontrols 4.1: use "friendly" value in non-compliance message
scuba_argument_parser: fix bug in converting argument value data types drive rego: fix 6.1 for subOUs/groups policy_api: add ability to dump Google's raw policy response installation & OPA instructions: add more detail about setup downloading OPA
There was a problem hiding this comment.
Great job addressing my feedback. I still have a few concerns, but I don't see them as big enough to block the policy API, so I propose we open follow-up issues for those and merge this as it is now.
My remaining concerns:
- The Windows version of the smoke test is failing. It passes when run locally and the Mac version passes, so a recommend disabling the Windows workflow for now and revisiting it later.
- The installation process. I am no longer concerned by the so-called "conflicting" config files I brought up earlier, but still share some of David and Sheetal's concerns. Also I definitely see your changes here as a step in the right direction. I agree with David that there should be a .scubagoggles folder in the home directory, where the .scubagoggles initial config file lives. I also recommend that be the default location for the OPA executable. I also agree with Sheetal that prompts presented by the setup process are confusing and that some of the documentation needs tweaking.
- The issue with Takeout for services without individual admin control described in my other comment.
Again, I'm ok with these final things being addressed later (as long as we open issues). Thanks for addressing all my other feedback!
ScubaGoggles with Google's Policy API Implementation
This is the complete integration of the version of ScubaGoggles containing the
implementation of Googles's Policy API.
implemented by the Policy API.
scubagws tenant).
and RedHat RHEL 8 linux.
Changes Introduced with this Version
Google Policy API
The Google Policy API is used for all baselines that can be assessed by the data
returned by the API. Google returns all settings available for the top-level
orgunit, and then only the settings that differ from the top-level orgunit for
sub-orgunits and groups.
The Policy API data is available to the rego code in the input data in the
"policies" section. The settings are grouped by orgunit or orgunit/group name.
There is no distinction between orgunits and groups.
The Policy API is queried using Google's
AuthorizedSessionclass becauseaccess to the API is not yet available using the Google API client, which is
used for the other APIs queried by ScubaGoggles (reports, directory, groups).
There have been no issues using this method to access the Policy API.
Because of the way Rego handles undefined variables, basically returning
undefinedwhen a variable is not defined, the Python code fetching the PolicyAPI data must verify that all expected settings are present in the top-level
orgunit's data. It's expected that missing settings will be rare, but they
can't be ignored because missing settings will lead to incorrect results.
ScubaGoggles doesn't have to abort on any missing settings, so currently a
warning is issued for any missing settings.
As with the log events, the Rego code does the work to determine whether the
requirement has been met for each baseline covered by the Policy API. In
implementing the Policy API in Rego, the log event code has been kept along
with the new Policy API code for debugging/testing during the development of
the Policy API implementation. This code will be removed in a subsequent
release.
ScubaGoggles Package Distribution & Installation
This version of ScubaGoggles is able to be packaged using the standard Python
packaging software, using the
setuptoolsback-end. Being able to installScubaGoggles and its dependencies as a package using
pipmakes theinstallation much easier for users and provides a separation between the
distributed code and the user's data (credentials and reports).
Unfortunately, the current configuration in the repository does not conform to a
structure that can be packaged. This has led to subtle issues, such as circular
import errors when running ScubaGoggles, and the incorrect placement of
ScubaGoggles-related directories in the top-level
site-packagesdirectory inthe Python environment.
For this version, directories that are included in the package have been
moved below the
scubagogglesdirectory:baselinesdocsregosample-config-filessample-reportTestingutilsThe current use of
setup.py, with package configuration included in the file,is deprecated by
setuptools. This version now usespyproject.tomlforpackage configuration.
The package is built using the
scubagoggles/utils/build.shscript. Itproduces a "wheel" file (
.whl) that is installable usingpip.In addition to installing ScubaGoggles, users have to specify:
credentials.json).For this version, users will run
scubagoggles setupto provide thoselocations. Unless the user explicitly changes a location, the setup is a
one-time step, even after software updates.
What are the advantages of making ScubaGoggles an installable package:
dependent packages separately.
scubagogglescommand will always work and there won't be a need to havethem try
Python scuba.py.environment. User's will not have to relocate files, such as the
credentials.jsonor OPA executable.Versioning
There are two forms of the version number used for ScubaGoggles: the software
version number (e.g., "0.3.0"), and what I call the "version suffix" that
proliferates throughout the files referencing baseline identifiers. There are
multiple places in the code that define the version number. Whenever there is a
version upgrade, the change touches nearly every file in the repository.
For this version, the "official" version number is defined in only one place
(
scubagoggles/__init__.py). All other references to the version number arederived from this value. The version number is managed using the
scubagoggles versioncommand.The Python code must use the
Versionclass when referencing the versionnumber. For the Rego code, the version suffix used in the baseline identifiers
is accessible in the input written by the Python code.
Baseline identifiers in the Rego code are defined once in a variable for each
baseline, and all references to the identifier are to use this variable
(including the test Rego code). For example:
The
utils.PolicyIdWithSuffix()function handles the addition of the versionsuffix at the end of the identifier. There must be no reference to the
version suffix in any comments in the Rego code (in fact, there should be
no occurrences of the hard-coded version suffix in any Rego file). It
is also not necessary to reference a specific version suffix in the user
documentation.
The "drift rules" and baseline Markdown files will continue to have the
version suffix included in the baseline version. The version suffixes in
these files will be modified as part of the version upgrade process.
When a version upgrade occurs, these are the only files that are modified
in the process:
README.mddrift-rules/GWS Drift Monitoring Rules - Calendar.csvdrift-rules/GWS Drift Monitoring Rules - Chat.csvdrift-rules/GWS Drift Monitoring Rules - Classroom.csvdrift-rules/GWS Drift Monitoring Rules - Common Controls as of 11-14-23.csvdrift-rules/GWS Drift Monitoring Rules - Drive and Docs.csvdrift-rules/GWS Drift Monitoring Rules - Gmail.csvdrift-rules/GWS Drift Monitoring Rules - Groups.csvdrift-rules/GWS Drift Monitoring Rules - Meet.csvdrift-rules/GWS Drift Monitoring Rules - Sites.csvscubagoggles/__init__.pyscubagoggles/baselines/calendar.mdscubagoggles/baselines/chat.mdscubagoggles/baselines/classroom.mdscubagoggles/baselines/commoncontrols.mdscubagoggles/baselines/drive.mdscubagoggles/baselines/gmail.mdscubagoggles/baselines/groups.mdscubagoggles/baselines/meet.mdscubagoggles/baselines/sites.mdPandas Dependency Dropped
The pandas module was included as a requirement because it was used to create
an HTML table for the reports. Pandas is a "beast" of a module, and also
requires another "beast" in numpy. The table creation is easy in this case,
and it allows us to drop a module that really isn't being used for its
intended purpose (data analysis). Having pandas dropped as a requirement also
lessens the potential issues users will have when installing ScubaGoggles.
No "sudo" for Running OPA
There was some issue with OPA in the past that apparently required the use of
"sudo" (privilege escalation) to run it on macOS and perhaps linux. This
issue seems to be fixed, so the ability to use "sudo" for running OPA has
been removed. OPA is basically a language interpreter like Python - they are
user mode programs that should never require privilege escalation. If this
happens again, the onus is on OPA to fix the issue, rather than trying to
work-around it.
Python Logging Facility Enabled
The Python logging facility has been enabled, so logging may be inserted into
the code where appropriate using
log.{debug, info, warning, error, critical}()function calls. The current default level that is displayed to the user is
warning(meaning thatwarninglevel messages and above will be displayed andnot lower level (
debug, info)), but this can be altered on the command lineusing the
--logoption.Downloading OPA
With users installing ScubaGoggles as a Python package, they can't easily
run the
download_opa.pyscript. To continue to provide users the abilityto download OPA, the
scubagoggles getopacommand performs the same task, butalso ensures the executable has the correct permissions to run on macOS and
linux.
Ability to Purge Output Directories
This might be more useful for developers running ScubaGoggles often for testing,
but there's now the ability via
scubagoggles purgeto delete older versionsof the reports output directories.
Closes #498
Closes #449
Closes #360
Closes #430
Closes #412
Closes #305
Closes #465
Closes #83
Follow-up issues
🧪 Testing
These changes have been continuously tested during the implementation of the Policy API settings released by Google.
As mentioned in the description, all automated tests (PYLint, regal, Rego tests) pass.
✅ Pre-approval checklist
✅ Pre-merge Checklist
Squash and mergebutton.✅ Post-merge Checklist