network_traffic: copy non-ECS fields to ECS-compliant locations for detection rules #8221

efd6 · 2023-10-16T23:55:15Z

Proposed commit message

Currently, detection rules depend on three non-ECS-compliant fields:

type (all except flows)
http.request.headers.authorization (http)
http.response.headers.content-type (http)

With the exception of flows and icmp, type is already set in network.protocol as well. So make the following copies:

type field into network.protocol for icmp
http.request.headers.authorization to network_traffic.http.request.headers.authorization for http
http.response.headers.content-type to http.response.mime_type for http

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

Note query about use of network_traffic.* with http datastream. The issue here is that the field under network_traffic would be http which is an ECS name, and I have seen some issues where use of ECS names is claimed to require ECS conformance even when in a namespaced location. I have doubts about the merits of this position in the long term given that is essentially prevents the use of perfectly cromulent labels.
For the new field (assuming it's acceptable), do we want to make the subfields explicitly or dynamically mapped rather than having them as a flattened field.

How to test this PR locally

Related issues

For Move non-ECS fields in Network Packet Capture datastream fields out of root namespace #8185

Screenshots

efd6 · 2023-10-16T23:58:19Z

packages/network_traffic/data_stream/http/elasticsearch/ingest_pipeline/default.yml

+# Detection Rules compatibility
+- set:
+    tag: set_compatibility_request_authorization
+    field: network_traffic.http.request.headers.authorization


I considered using http.request.Headers.authorization instead, but I think a good path (depending on approval of this location) would be to reflect what is currently http.* (without the ECS enrichments) into network_traffic.http.* (and equivalently all <datastream>.* into network_traffic.<datastream>.* in the other datastreams).

I think that network_traffic.<datastream>.* is a good safe pattern to follow.

There has been a long standing lack of guidance from ECS on the headers field (elastic/ecs#232). With us moving in the direction of open-telemetry I expect us to adapt to their naming conventions at some point. So I figured I should mention that the OTEL definition is singular http.{request,response}.header.<key> (with key lowercased). https://opentelemetry.io/docs/specs/semconv/attributes-registry/http/

elasticmachine · 2023-10-17T01:03:40Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-11-13T21:56:32.891+0000
Duration: 68 min 13 sec

Test stats 🧪

Test	Results
Failed	0
Passed	154
Skipped	0
Total	154

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2023-10-17T01:03:52Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`1/1`)	💚
Files	100.0% (`2/2`)	💚
Classes	100.0% (`2/2`)	💚
Methods	98.276% (`57/58`)	❕
Lines	88.356% (`129/146`)	❕
Conditionals	100.0% (`0/0`)	💚

elasticmachine · 2023-10-17T02:39:28Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

kcreddy

LGTM. Maybe someone with better understanding of detection rules setup could also review it. Thanks!

kcreddy · 2023-11-01T16:52:48Z

packages/network_traffic/changelog.yml

@@ -1,4 +1,15 @@
 # newer versions go on top
+- version: "1.26.0"
+  changes:
+    - description: Copy `type` field to `network.protocol` in ICPM datastream.


Suggested change

- description: Copy `type` field to `network.protocol` in ICPM datastream.

- description: Copy `type` field to `network.protocol` in ICMP datastream.

Currently, detection rules depend on three non-ECS-compliant fields: - type (all except flows) - http.request.headers.authorization (http) - http.response.headers.content-type (http) With the exception of flows and icmp, type is already set in network.protocol as well. So make the following copies: - type field into network.protocol for icmp - http.request.headers.authorization to network_traffic.http.request.headers.authorization for http - http.response.headers.content-type to http.response.mime_type for http

brokensound77

Changes look good from our end 👍 . We will make adjustments accordingly in elastic/detection-rules#3194

Thank you

andrewkroh · 2023-11-23T17:19:59Z

packages/network_traffic/data_stream/http/elasticsearch/ingest_pipeline/default.yml

+# Detection Rules compatibility
+- set:
+    tag: set_compatibility_request_authorization
+    field: network_traffic.http.request.headers.authorization


I think that network_traffic.<datastream>.* is a good safe pattern to follow.

There has been a long standing lack of guidance from ECS on the headers field (elastic/ecs#232). With us moving in the direction of open-telemetry I expect us to adapt to their naming conventions at some point. So I figured I should mention that the OTEL definition is singular http.{request,response}.header.<key> (with key lowercased). https://opentelemetry.io/docs/specs/semconv/attributes-registry/http/

elasticmachine · 2023-11-27T21:57:49Z

Package network_traffic - 1.27.0 containing this change is available at https://epr.elastic.co/search?package=network_traffic

elasticmachine · 2023-11-27T21:57:50Z

Package network_traffic - 1.27.0 containing this change is available at https://epr.elastic.co/search?package=network_traffic

elasticmachine · 2023-11-27T21:57:51Z

Package network_traffic - 1.27.0 containing this change is available at https://epr.elastic.co/search?package=network_traffic

efd6 added enhancement New feature or request Team:Security-External Integrations Integration:Network Packet Capture labels Oct 16, 2023

efd6 self-assigned this Oct 16, 2023

efd6 commented Oct 16, 2023

View reviewed changes

efd6 mentioned this pull request Oct 16, 2023

Move non-ECS fields in Network Packet Capture datastream fields out of root namespace #8185

Open

6 tasks

efd6 force-pushed the 8185-stage-0 branch from 40282fd to f4c056e Compare October 17, 2023 00:06

efd6 marked this pull request as ready for review October 17, 2023 02:39

efd6 requested a review from a team as a code owner October 17, 2023 02:39

efd6 requested review from andrewkroh, ebeahan and brokensound77 October 17, 2023 02:40

kcreddy approved these changes Nov 1, 2023

View reviewed changes

efd6 added 2 commits November 14, 2023 08:25

address pr comment

6653b33

efd6 force-pushed the 8185-stage-0 branch from 4d87411 to 6653b33 Compare November 13, 2023 21:56

brokensound77 mentioned this pull request Nov 20, 2023

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

Open

brokensound77 approved these changes Nov 22, 2023

View reviewed changes

andrewkroh approved these changes Nov 23, 2023

View reviewed changes

efd6 merged commit 8450394 into elastic:main Nov 27, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

network_traffic: copy non-ECS fields to ECS-compliant locations for detection rules #8221

network_traffic: copy non-ECS fields to ECS-compliant locations for detection rules #8221

efd6 commented Oct 16, 2023 •

edited

Loading

efd6 Oct 16, 2023

andrewkroh Nov 23, 2023

elasticmachine commented Oct 17, 2023 •

edited

Loading

Build stats

Test stats 🧪

elasticmachine commented Oct 17, 2023 •

edited

Loading

elasticmachine commented Oct 17, 2023

kcreddy left a comment

kcreddy Nov 1, 2023

brokensound77 left a comment

andrewkroh Nov 23, 2023

elasticmachine commented Nov 27, 2023

elasticmachine commented Nov 27, 2023

elasticmachine commented Nov 27, 2023

	- description: Copy `type` field to `network.protocol` in ICPM datastream.
	- description: Copy `type` field to `network.protocol` in ICMP datastream.

network_traffic: copy non-ECS fields to ECS-compliant locations for detection rules #8221

network_traffic: copy non-ECS fields to ECS-compliant locations for detection rules #8221

Conversation

efd6 commented Oct 16, 2023 • edited Loading

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

Related issues

Screenshots

efd6 Oct 16, 2023

Choose a reason for hiding this comment

andrewkroh Nov 23, 2023

Choose a reason for hiding this comment

elasticmachine commented Oct 17, 2023 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

elasticmachine commented Oct 17, 2023 • edited Loading

🌐 Coverage report

elasticmachine commented Oct 17, 2023

kcreddy left a comment

Choose a reason for hiding this comment

kcreddy Nov 1, 2023

Choose a reason for hiding this comment

brokensound77 left a comment

Choose a reason for hiding this comment

andrewkroh Nov 23, 2023

Choose a reason for hiding this comment

elasticmachine commented Nov 27, 2023

elasticmachine commented Nov 27, 2023

elasticmachine commented Nov 27, 2023

efd6 commented Oct 16, 2023 •

edited

Loading

elasticmachine commented Oct 17, 2023 •

edited

Loading

elasticmachine commented Oct 17, 2023 •

edited

Loading