-
Notifications
You must be signed in to change notification settings - Fork 529
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
base: main
Are you sure you want to change the base?
[Rule Tuning] Update rules using NPC integration and non-ECS fields #3194
Conversation
After chatting more with @efd6, the
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 |
(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 |
There was a problem hiding this comment.
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:*)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
detection-rules/detection_rules/rule_validators.py
Lines 48 to 70 in a6c5cfc
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.
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. |
@brokensound77, is this one ready to proceed, as the related PRs/Issues seem to be resolved? |
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 Responsecf53f532-9cc9-445a-9ae7-fced307ec53c
Cobalt Strike Command and Control Beacon4a4e23cf-78a2-449c-bac3-701924c269d3
Possible FIN7 DGA Command and Control BehaviorRule needing to go from
status
->http.response.status_phrase
:31295df3-277b-4c56-a1fb-84e31b4222a9
Inbound Connection to an Unsecure Elasticsearch NodeI also converted the Lucene rules to EQL or KQL for maintainability purposes.
Details
cf53f532-9cc9-445a-9ae7-fced307ec53c
Cobalt Strike Command and Control Beacon4a4e23cf-78a2-449c-bac3-701924c269d3
Possible FIN7 DGA Command and Control Behavior31295df3-277b-4c56-a1fb-84e31b4222a9
Inbound Connection to an Unsecure Elasticsearch NodeProblems
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.
http.response.headers.content-type
andhttp.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 isobject
, which should imply*
wildcard fields if not further defined. We need to explore that.flattened
packetbeat http
moduleUltimately, 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