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

CRL_1700 ES Macros #278

Merged
merged 10 commits into from
Dec 16, 2019
Merged

CRL_1700 ES Macros #278

merged 10 commits into from
Dec 16, 2019

Conversation

josehelps
Copy link
Collaborator

@josehelps josehelps commented Dec 11, 2019

  • introduced security_content_ctime replacing the ctime macro across all content
  • introduced security_content_summariesonly replacing the summariesonly macro across all content
  • removed runstory macro definition
  • removed comment macro for empty definitions
  • updated macro jinja template for output empty definitions
  • updated searches/files to use standard naming conventions vs camel case

@auto-assign auto-assign bot requested a review from P4T12ICK December 11, 2019 20:17
Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

baselines/discover_dns_records was not updated correctly

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/dns_evilginx_subdomains.yml needs to be fixed

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/dns_record_changed.yml needs to be updated

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/dyn_dns_web_traffic.yml

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/no_win_updates_in_timeframe.yml

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

investigations/first_occurrence_mac_address.yml

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/ec2_instance_started_with_previously_unseen_ami.yml

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/large_icmp_outbound.yml

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/cloud_compute_instance_created_by_previously_unseen_user.yml

Copy link
Contributor

@trogdorsey trogdorsey left a comment

Choose a reason for hiding this comment

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

detections/new_aws_console_login_by_user.yml

@patel-bhavin
Copy link
Contributor

updated as per dorsey's feedback, fixed some other files too. updated searches for naming consistency as well, while i was at it

patel-bhavin
patel-bhavin previously approved these changes Dec 13, 2019
@patel-bhavin
Copy link
Contributor

@trogdorsey : Can you perhaps review this? want to make sure i didnt miss anything

@P4T12ICK
Copy link
Collaborator

@patel-bhavin @trogdorsey : Should the macro security_content_ctime not be independent from ctime (not use ctime)? Otherwise, ES needs to be installed to run our searches. In my opinion we need to put the content of ctime in the macro and not a reference to ctime.

@P4T12ICK
Copy link
Collaborator

P4T12ICK commented Dec 16, 2019

@patel-bhavin I found an issue with the comment macro (which is used for empty pre and post filter). If you want to run the search in any app besides search, the searches will fail because the comment macro is shared only inside the search app. We should ship with a config which makes the comment macro global accessible.

@josehelps
Copy link
Collaborator Author

@patel-bhavin @trogdorsey : Should the macro security_content_ctime not be independent from ctime (not use ctime)? Otherwise, ES needs to be installed to run our searches. In my opinion we need to put the content of ctime in the macro and not a reference to ctime.

ctime is offered by core as a command: https://docs.splunk.com/Documentation/Splunk/latest/SearchReference/Convert#Convert_functions

@patel-bhavin
Copy link
Contributor

@patel-bhavin I found an issue with the comment macro (which is used for empty pre and post filter). If you want to run the search in any app besides search, the searches will fail because the comment macro is shared only inside the search app. We should ship with a config which makes the comment macro global accessible.

@P4T12ICK : removed the comment macro all together after chatting with @d1vious

@patel-bhavin patel-bhavin merged commit b59991e into develop Dec 16, 2019
@delete-merged-branch delete-merged-branch bot deleted the CRL-1700_ES_macros branch December 16, 2019 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants