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

Feature: Adding version enforcement #280

Merged
merged 19 commits into from
Sep 17, 2024
Merged

Conversation

cmcginley-splunk
Copy link
Collaborator

@cmcginley-splunk cmcginley-splunk commented Sep 10, 2024

Context

  • Detections now have versions
  • Prior to publishing content, we want to enforce that if a detection changes, its version gets bumped appropriately

Code changes

  • SplunkApp extended to be able to download an app directly from Splunkbase
  • inspect action extended to perform checks against a current build's detections against a prior build's detections
    • new previous_build config option; if unspecified, latest release from Splunkbase is downloaded to use for detection comparison as previous build
  • DetectionStanza class added to model an individual stanza representing a detection in savedsearches.conf
  • SavedsearchesConf class added to model and parse a savedsearches.conf file
    • Note that currently this class does not model the full file, but only the ESCU detections as the rest is not relevant to us at this time
  • Validations performed:
    • No detections were deleted between builds
    • No detection IDs changed
    • No versions were decremented
    • If a detection changed between releases (based on stanza hash), its version was bumped appropriately
  • Other changes include some formatting and typing cleanup

Testing

  • Tested internally against the v4.39.1 release, with a few local manual edits to force the generation of errors
AppInspect found [error,failure,manual_check,warning] that MAY cause a failure during AppInspect API:
 - 9 warnings
Details:
 - warning [check_app_configuration_file: check_custom_conf_replication]
 - warning [check_app_configuration_file: check_for_updates_disabled]
 - warning [check_app_configuration_file: check_for_valid_package_id]
 - warning [check_bias_language: check_for_bias_language]
 - warning [check_malware: check_hostnames_and_ips]
 - warning [check_meta_files: check_kos_are_accessible]
 - warning [check_packaging_standards: check_valid_version_number]
 - warning [check_packaging_standards: check_version_is_valid_semver]
 - warning [check_slim_ssai: check_custom_confs]
Downloading latest ESCU build from Splunkbase to serve as previous build during validation...
Latest release downloaded from Splunkbase to: downloads/escu_latest.tgz
Detection Metadata Enforcement:
	❌ ESCU - Email Attachments With Lots Of Spaces - Rule
		🔸 Detection version in current build should be bumped to at least 4.
	❌ ESCU - O365 Compliance Content Search Started - Rule
		🔸 Detection from previous build not found in current build.
	❌ ESCU - Detect API activity from users without MFA - Rule
		🔸 Detection version (1) in current build is less than version (2) in previous build.
	❌ ESCU - Zscaler Virus Download threat blocked - Rule
		🔸 Detection ID aa19e627-d448-4a31-85cd-82068dec5692 in current build does not match ID aa19e627-d448-4a31-85cd-82068dec5691 in previous build.
		🔸 Detection version (1) in current build is less than version (2) in previous build.
		🔸 Detection version in current build should be bumped to at least 3.
Verbose error logging is DISABLED.
Please use the --verbose command line argument if you need more context for your error or file a bug report.
Validation errors when comparing detection stanzas in current and previous build: (6 sub-exceptions)

TODO

  • Create PR to external security_content adding downloads/ to .gitignore and adding --enable-metadata-validation to CLI invocation OR contentctl.yml in CI/CD
  • Create MR to internal security_content adding downloads/ to .gitignore and adding --enable-metadata-validation to CLI invocation OR contentctl.yml in CI/CD

contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/objects/constants.py Show resolved Hide resolved
contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/objects/config.py Show resolved Hide resolved
contentctl/objects/constants.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
contentctl/objects/config.py Show resolved Hide resolved
contentctl/objects/detection_stanza.py Show resolved Hide resolved
contentctl/objects/savedsearches_conf.py Show resolved Hide resolved
@cmcginley-splunk cmcginley-splunk marked this pull request as ready for review September 10, 2024 18:34
@cmcginley-splunk cmcginley-splunk added enhancement New feature or request and removed Draft labels Sep 10, 2024
@cmcginley-splunk cmcginley-splunk requested review from ljstella and pyth0n1c and removed request for ljstella September 10, 2024 18:43
Copy link
Contributor

@pyth0n1c pyth0n1c left a comment

Choose a reason for hiding this comment

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

Feel free to close out any comments as resolved if you feel that's appropriate.
If you agree that suggestions should be future enhancements, then let me know and I will create the issue(s).

README.md Outdated Show resolved Hide resolved
contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/objects/config.py Show resolved Hide resolved
contentctl/objects/config.py Show resolved Hide resolved
contentctl/objects/constants.py Outdated Show resolved Hide resolved
contentctl/objects/constants.py Outdated Show resolved Hide resolved
contentctl/objects/detection_stanza.py Outdated Show resolved Hide resolved
contentctl/objects/detection_stanza.py Outdated Show resolved Hide resolved
contentctl/objects/savedsearches_conf.py Outdated Show resolved Hide resolved
Copy link
Contributor

@pyth0n1c pyth0n1c left a comment

Choose a reason for hiding this comment

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

Most of the comments are resolved, but still one or two more open.
I think these will be a very small change.

contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/objects/config.py Show resolved Hide resolved
contentctl/actions/inspect.py Show resolved Hide resolved
contentctl/objects/config.py Show resolved Hide resolved
contentctl/objects/savedsearches_conf.py Outdated Show resolved Hide resolved
contentctl/objects/constants.py Outdated Show resolved Hide resolved
Copy link
Contributor

@pyth0n1c pyth0n1c left a comment

Choose a reason for hiding this comment

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

We had some extended discussions in the comments, but I wanted to add the functional testing I performed on this build. I have tested the following which worked as anticipated:

  1. Checking for changes with the newest version of security_content against the latest version of ESCU in Splunkbase. All anticipated changes were found.
  2. Check for changes against two local builds of security_content. The first with security_content exactly as it appears in main and the second with the following changes:
    a. No changes.
    b. One detection updated
    c. several detections updated
    d. The name/filename of a detection changes (this gives a different error, as it should)
    e. Detection has the same name, but a different UUID. This also gives a different error, as it should.
    f. Detection removed from newer version of a package. This also throws an error as it should.
  3. Created a NEW app using contentctl init with some content. Built it, then made some changes (as you see above) and re-inspected. It picked up all changes between this new local app and the older local app correctly as well.

Great job by @cmcginley-splunk , I approve. Thank you!

@pyth0n1c pyth0n1c merged commit bf6fe08 into main Sep 17, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants