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

[Rule Tuning] Update rules using NPC integration and non-ECS fields #3194

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

brokensound77
Copy link
Contributor

@brokensound77 brokensound77 commented Oct 16, 2023

Issues

related to elastic/dev/issues/2270

Summary

The NPC integration schema was updated to move non-ecs fields out of the root namespace. This had minimal impact, only requiring changes in 4 rules.

Rules needing to go from type -> network.protocol:

  • 11013227-0301-4a8c-b150-4db924484475 Abnormally Large DNS Response
  • cf53f532-9cc9-445a-9ae7-fced307ec53c Cobalt Strike Command and Control Beacon
  • 4a4e23cf-78a2-449c-bac3-701924c269d3 Possible FIN7 DGA Command and Control Behavior

Rule needing to go from status -> http.response.status_phrase:

  • 31295df3-277b-4c56-a1fb-84e31b4222a9 Inbound Connection to an Unsecure Elasticsearch Node

I also converted the Lucene rules to EQL or KQL for maintainability purposes.

Details

cf53f532-9cc9-445a-9ae7-fced307ec53c Cobalt Strike Command and Control Beacon

  • also converted to EQL

4a4e23cf-78a2-449c-bac3-701924c269d3 Possible FIN7 DGA Command and Control Behavior

  • also converted to EQL

31295df3-277b-4c56-a1fb-84e31b4222a9 Inbound Connection to an Unsecure Elasticsearch Node

  • also converted to KQL

Problems

As a reminder, Lucene rules are not parsed and queries validated, so some of this has existed, but is just now appearing due to the conversion to KQL

http.response.status_phrase

This field exists in the beats and integration custom schemas, but not ECS. The logic however, may be creating issues with how the extended schemas are validating and so may need revision.

Error at line:2,column:5
Unknown field
(event.dataset:network_traffic.http or (event.category:network_traffic and network.protocol:http)) and
    http.response.status_phrase:ok and destination.port:9200 and network.direction:inbound and
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Try adding event.module or event.dataset to specify beats module
stack: 8.12.0, beats: 8.10.3, ecs: 8.10.0

http.response.headers.content-type and http.request.headers.authorization

Header fields (http.response.headers.*) do not exist in ECS, but they do exist as an object under the integration and beats custom schemas, however, I do not think that they are being built properly when the schemas are downloaded an assembled. I assume this is because the type is object, which should imply * wildcard fields if not further defined. We need to explore that.

  • Under integration, the type is flattened
  • Under beats, the assembled schema is missing the field altogether, even though it exists in the packetbeat http module
image
Error at line:3,column:9
Unknown field
(event.dataset:network_traffic.http or (event.category:network_traffic and network.protocol:http)) and
    http.response.status_phrase:ok and destination.port:9200 and network.direction:inbound and
    not http.response.headers.content-type:"image/x-icon" and
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	Try adding event.module or event.dataset to specify integration module
	Will check against integrations ['network_traffic'] combined.
	package='network_traffic', integration='http', package_version='1.25.1', stack_version='8.10.0', ecs_version='8.10.0'

Ultimately, we may be able to (or need to) simplify the logic for the query, but we also need to solve the dynamic schema from object type as well

@brokensound77
Copy link
Contributor Author

After chatting more with @efd6, the header.* fields will also be updated to be more compatible as:

  • http.response.Headers.*
  • http.request.Headers.*

We can await that upstream change, pull in the integrations updates, and allow that to pass, however, we still need to look into whether schema of type object is built properly in beats and integrations

@brokensound77 brokensound77 added blocked Domain: Network Rule: Tuning tweaking or tuning an existing rule labels Oct 16, 2023
(event.dataset: network_traffic.http OR (event.category: network_traffic AND network.protocol: http)) AND
status:OK AND destination.port:9200 AND network.direction:inbound AND NOT http.response.headers.content-type:"image/x-icon" AND NOT
_exists_:http.request.headers.authorization
(event.dataset:network_traffic.http or (event.category:network_traffic and network.protocol:http)) and
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because our query validation for integrations occurs separately from the base query validation (ECS + beats diff), there is a conflict where a rule cannot satisfy both validations. This is the first rule with an integration-specific field (that I am aware of), which results in it

  • failing base query validation because the fields do not exist in ECS or beats
  • passing integration validation

As a test, even if we added the fields to non-ecs-schema (as a bad hack), the rule still fails because the ECS/beats field is not known to the integration

Integration only fields:

  • http.response.status_phrase
  • network_traffic.http.request.headers.authorization

Unknown to integration

  • http.request.headers.authorization
  • status

With the existing process, the only way to rectify this is to split the rule, however that is not sustainable. We need to review this for potential refactor.


attempted cross-compatible query, which fails based on the above

(event.dataset:network_traffic.http or (event.category:network_traffic and network.protocol:http)) and
    (status:ok or http.response.status_phrase:ok) and
    destination.port:9200 and network.direction:inbound and
    not http.response.mime_type:"image/x-icon" and
    not (network_traffic.http.request.headers.authorization:* or http.request.headers.authorization:*)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reference the description above for further details and coexisting bugs

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Justin for bringing this to our attention! I will have to dive a bit deeper, but I recall that we specifically allow it to validate ECS+beats OR integrations.

def validate(self, data: QueryRuleData, meta: RuleMeta) -> None:
"""Validate the query, called from the parent which contains [metadata] information."""
if meta.query_schema_validation is False or meta.maturity == "deprecated":
# syntax only, which is done via self.ast
return
if isinstance(data, QueryRuleData) and data.language != 'lucene':
packages_manifest = load_integrations_manifests()
package_integrations = TOMLRuleContents.get_packaged_integrations(data, meta, packages_manifest)
validation_checks = {"stack": None, "integrations": None}
# validate the query against fields within beats
validation_checks["stack"] = self.validate_stack_combos(data, meta)
if package_integrations:
# validate the query against related integration fields
validation_checks["integrations"] = self.validate_integration(data, meta, package_integrations)
if (validation_checks["stack"] and not package_integrations):
raise validation_checks["stack"]
if (validation_checks["stack"] and validation_checks["integrations"]):
raise ValueError(f"Error in both stack and integrations checks: {validation_checks}")

If you review this code, validation_checks holds the state of ECS+Beats OR Integration validation checks. If both fail, then yes the rule should be reviewed because neither the integration's schema nor ECS, Non-ECS or Beats have the fields or the query is incorrect.

@botelastic
Copy link

botelastic bot commented Feb 13, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@botelastic botelastic bot added the stale 60 days of inactivity label Feb 13, 2024
@w0rk3r
Copy link
Contributor

w0rk3r commented Feb 14, 2024

@brokensound77, is this one ready to proceed, as the related PRs/Issues seem to be resolved?

@botelastic botelastic bot removed the stale 60 days of inactivity label Feb 14, 2024
@w0rk3r w0rk3r added the backlog label Feb 14, 2024
@brokensound77
Copy link
Contributor Author

Due to this bug, this is blocked until #3556 is resolved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants