Skip to content

[FR] Upgrade eql to 1.0.1, align KQL with lark 1.x, and fix NTLM relay field comparison - #6611

Open
Mikaayenson wants to merge 7 commits into
mainfrom
fix/ntlm-relay-eql-length-compare
Open

[FR] Upgrade eql to 1.0.1, align KQL with lark 1.x, and fix NTLM relay field comparison#6611
Mikaayenson wants to merge 7 commits into
mainfrom
fix/ntlm-relay-eql-length-compare

Conversation

@Mikaayenson

@Mikaayenson Mikaayenson commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes widespread rule execution failures.
  • Root cause: [Rule Tuning] Potential Computer Account NTLM Relay Activity #6431 added length(host.name) == length(user.name) - 1, which SIEM EQL rejects (Comparisons against fields are not currently supported). local (pyeql) validation did not catch.
  • Replaces that check with equivalent logic: mutual startswith~ for exact short-hostname match, or startswith~(host.name, concat(account, ".")) for FQDN — preserves the FP reduction intent without field arithmetic in ==.
  • Pulls in logic from from eql upstream

Replace length(user.name) - 1 equality checks that fail rule execution
on multiple clusters with mutual startswith~/concat hostname matching.
Copilot AI review requested due to automatic review settings August 6, 2026 13:21
@botelastic botelastic Bot added Domain: Endpoint OS: Windows windows related rules labels Aug 6, 2026
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 6, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

@Mikaayenson Mikaayenson changed the title [Bug] Fix EQL field comparison in Potential Computer Account NTLM Relay Activity [Rule Tuning] Fix EQL field comparison in Potential Computer Account NTLM Relay Activity Aug 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes an Elasticsearch EQL execution failure in the Potential Computer Account NTLM Relay Activity rule by removing unsupported field-to-field arithmetic comparison and replacing it with equivalent hostname ↔ machine-account matching logic.

Changes:

  • Updates the rule query to avoid length(host.name) == length(user.name) - 1 (unsupported field comparison) while preserving the intended “exact short-hostname or FQDN boundary” match behavior.
  • Updates the rule updated_date to reflect the tuning date.

Comment thread rules/windows/credential_access_dollar_account_relay.toml Outdated
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 6, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

@Mikaayenson Mikaayenson self-assigned this Aug 6, 2026
@Mikaayenson Mikaayenson added Rule: Tuning tweaking or tuning an existing rule currentrelease labels Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Rule: Tuning - Guidelines

These guidelines serve as a reminder set of considerations when tuning an existing rule.

Documentation and Context

  • Detailed description of the suggested changes.
  • Provide example JSON data or screenshots.
  • Provide evidence of reducing benign events mistakenly identified as threats (False Positives).
  • Provide evidence of enhancing detection of true threats that were previously missed (False Negatives).
  • Provide evidence of optimizing resource consumption and execution time of detection rules (Performance).
  • Provide evidence of specific environment factors influencing customized rule tuning (Contextual Tuning).
  • Provide evidence of improvements made by modifying sensitivity by changing alert triggering thresholds (Threshold Adjustments).
  • Provide evidence of refining rules to better detect deviations from typical behavior (Behavioral Tuning).
  • Provide evidence of improvements of adjusting rules based on time-based patterns (Temporal Tuning).
  • Provide reasoning of adjusting priority or severity levels of alerts (Severity Tuning).
  • Provide evidence of improving quality integrity of our data used by detection rules (Data Quality).
  • Ensure the tuning includes necessary updates to the release documentation and versioning.

Rule Metadata Checks

  • updated_date matches the date of tuning PR merged.
  • min_stack_version should support the widest stack versions.
  • name and description should be descriptive and not include typos.
  • query should be inclusive, not overly exclusive. Review to ensure the original intent of the rule is maintained.

Testing and Validation

  • Validate that the tuned rule's performance is satisfactory and does not negatively impact the stack.
  • Ensure that the tuned rule has a low false positive rate.

@Mikaayenson Mikaayenson changed the title [Rule Tuning] Fix EQL field comparison in Potential Computer Account NTLM Relay Activity [Rule Tuning] Potential Computer Account NTLM Relay Activity (EQL field comparison) Aug 6, 2026

@eric-forte-elastic eric-forte-elastic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Manual review, LGTM 👍

Pick up ES field-comparison validation in py-eql so CI rejects
unsupported SIEM EQL field-vs-field queries.
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 7, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

Bump lib/kql to 0.1.15 and pin eql 1.0.1. Switch the kql dependency to
a local file path so PR installs no longer pull main's older eql pin.
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 7, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 7, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

eql 1.0.1 pulls lark>=1.3.1, which moved Tree positions to meta.
Add KvTree line/column properties (matching eql) and pin lark>=1.3.1
so KQL schema errors raise KqlParseError instead of AttributeError.
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 7, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

@Mikaayenson Mikaayenson changed the title [Rule Tuning] Potential Computer Account NTLM Relay Activity (EQL field comparison) [FR] Upgrade eql to 1.0.1, align KQL with lark 1.x, and fix NTLM relay field comparison Aug 7, 2026
@Mikaayenson Mikaayenson removed the patch label Aug 7, 2026
Matches the minor label: eql 1.0.1 upgrade and KQL/lark 1.x alignment.
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Aug 7, 2026

Copy link
Copy Markdown

⛔️ Test failed

Results
  • ❌ Potential Computer Account NTLM Relay Activity (eql)
    • coverage_issue: no_rta
    • stack_validation_failed: no_rta

Comment thread pyproject.toml
"semver==3.0.4",
"PyGithub==2.9.1",
"detection-rules-kql @ git+https://github.com/elastic/detection-rules.git#subdirectory=lib/kql",
"detection-rules-kql @ file:./lib/kql",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We may want to change the file path here (maybe switch to a setup.py approach?) since I think it makes it incompatible with uv:

Image

@eric-forte-elastic eric-forte-elastic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Logically I think this is good to go, just may want to update the way we are handling the dependency change for uv support.. Otherwise LGTM 👍

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants