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

Add network directions ingress and egress #945

Merged
merged 4 commits into from
Oct 2, 2020

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Aug 20, 2020

Adding these two values based on the discussion in #791.

I've only lightly modified the description to associate which values to use for host based, and which ones should be associated to perimeter based. Feedback welcome, if folks would like more adjustments to the description.

I think we should discuss whether this should be considered a breaking change. My opinion is that for folks that have dutifully been populating inbound/outbound, this will feel like a breaking change. On the other hand based on feedback so far, the usefulness of the field was hampered by the difficulty of using the same pair of values "depending on context". So I would lean towards getting this into ECS in the 1.x line, and not waiting for 2.0.

Closes #791

@webmat
Copy link
Contributor Author

webmat commented Aug 20, 2020

ping @dcode @leehinman @dainperkins

dcode
dcode previously approved these changes Aug 20, 2020
Copy link
Contributor

@dcode dcode left a comment

Choose a reason for hiding this comment

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

I like it. LGTM

@webmat
Copy link
Contributor Author

webmat commented Aug 25, 2020

ping @andrewkroh

@webmat
Copy link
Contributor Author

webmat commented Aug 25, 2020

I'd like to get your thoughts about implementation in Beats, @andrewkroh or @leehinman. Any concerns?

@andrewstucki
Copy link
Contributor

So, a couple of thoughts. For our current ECS implementation, it seems like we already have the semantics for defining traffic in internal ip space (internal) v. something involving external ips (external)--which really seem to be what the re-definition of inbound v. outbound are all about (basically a network boundary traversal == external).

The thing that appears to be missing from internal v. external is, interestingly enough, a sense of "directionality" (the irony isn't lost on me that this is network.direction we're talking about).

If you wanted to indicate that network coming into a host/network was from public ip space, it seems to me that the current definition of [inbound, external] would be functionally equivalent to and require re-categorization as [inbound, ingress]. Likewise [outbound, external] would turn into [outbound, egress].

So, overall I'm somewhat indifferent about/against this change for a couple of reasons:

  1. we already have the means for tagging network boundary traversals
  2. the directionality components of inbound/ingress and outbound/egress are basically always tied together
  3. in common usage, inbound and ingress (as well as outbound and egress) are often interchangeable (for example AWS Security Group Rules), so from an implementation, and even threat-hunting perspective, I would imagine this would get confusing.

That said, if we do see this as a substantive change, I'd want to consider deprecating internal and external because they seem to serve roughly the same purpose as the re-definition.

Alternatively, what about just shoring up the documentation and saying explicitly internal/external should be used for referring to network traversals while inbound/outbound are about host-level traffic direction? Or even just renaming inbound/outbound as ingress/egress? Would either of those two alternatives address the concern?

@ebeahan ebeahan added the ready Issues we'd like to address in the future. label Sep 8, 2020
@ebeahan
Copy link
Member

ebeahan commented Sep 17, 2020

Looking back at the issue posed in #791, I think both adding ingress and egress as recommended values and the guidance on the usage of ingress/egress vs. internal/external/inbound/outbound should help address the current ambiguity.

Is the use of unknown acceptable for either host-based or network-based situations?

We could provide more context around the expected use of internal, external, inbound, outbound, and unknown from the network perimeter's perspective. Based on my understanding, if you have some concept of a "home or "local" network defined, that info can be used to determine if your originator and responder are "local" or not.

originator local? responder local? direction
false false external
false true inbound
true false outbound
true true internal
undefined undefined unknown

@webmat
Copy link
Contributor Author

webmat commented Sep 24, 2020

@andrewstucki Let me take another stab at trying to clarify this. The current definition really doesn't expand on internal & external. But I think it's clear we'll need to expand more in the actual docs based on this.

Host monitoring perspective (sources could be EDR, Packetbeat on a host)

  • ingress: traffic originating from outside the host, to an address local to the host
  • egress: traffic originating from the host, towards a remote address

Perimeter monitoring perspective (sources could be firewall, router, Packetbeat on a network tap)

  • inbound: traffic originating from outside the network perimeter, towards a host on the network being monitored.
  • outbound: traffic originating from a host on the network being monitored, to the outside of the network perimeter.
  • internal: traffic between two hosts on subnets that are known to be inside the perimeter.
  • external: traffic between two hosts on external networks.
    • This would typically only be useful to folks monitoring backbone, ISPs, public VPN services, or other situations where one's own network is acting as a conduit between two untrusted networks.

Prior to this PR, "inbound" and "outbound" were meant to capture both kinds of incoming and outgoing traffic, and the distinction between the different situations was a hand-wavy "interpret this depending on the data source".

So splitting off ingress/egress out of inbound/outbound is meant to make it more straightforward to interpret either case, when writing detection rules and the like, without having to rely on another factor to determine "is this host-based or perimeter-based?" Now it's entirely captured in the values in network.direction.

Unknown: we should keep it. It can be useful on high traffic lossy taps that miss the SYN/ACK on some flows. It may also be useful for some event sources that don't give a clear indication of who's the initiator. For example some auditd events only refer to local/remote IPs.

@webmat
Copy link
Contributor Author

webmat commented Sep 24, 2020

Another note that's unrelated to the schema itself.

To reliably determine what's inside/outside the network perimeter, user configuration may be needed.

Heuristics based on standard private IP ranges may be fine as a default baseline a product can start from. However private IP addresses can be considered "outside" the perimeter in some situations, and public IP addresses can be considered "inside" the perimeter in some other situations.

Although I don't think the point I'm making here needs to be explicit in the network.direction field definition.

@webmat
Copy link
Contributor Author

webmat commented Sep 29, 2020

@elasticmachine, run elasticsearch-ci/docs

Mathieu Martin added 3 commits October 2, 2020 14:55
…ernal' values.

Also quoting the values, when they're mentioned in the description.
@webmat webmat force-pushed the moar-network-directions branch from 15bae6f to 0796cc3 Compare October 2, 2020 19:12
@webmat
Copy link
Contributor Author

webmat commented Oct 2, 2020

I rebased this on the most recent master branch for merging.

I've also added a paragraph to the description to clarify the meaning of "internal" and "external".

We can cover usage in more detail after #988 merges, and we work on the usage docs for the network field set.

This is ready for a final review.

ebeahan
ebeahan previously approved these changes Oct 2, 2020
Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

LGTM

@webmat webmat merged commit 1d32671 into elastic:master Oct 2, 2020
webmat pushed a commit to webmat/ecs that referenced this pull request Oct 2, 2020
dseeley added a commit to dseeley/ecs that referenced this pull request May 5, 2021
* bumping version for 1.x release branch (elastic#921)

* [1.x] add related.hosts (elastic#913) (elastic#924)

* [1.x][DOCS] Fixes SIEM links (elastic#936)

* [1.x] Consolidate field-details doc template (elastic#897) (elastic#946)

* Add http.[request|response].mime_type (elastic#944) (elastic#949)

* [1.x] Cut 1.6 Changelog (elastic#933) (elastic#952) (elastic#953)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add threat.technique.subtechnique (elastic#951) (elastic#956)

Co-authored-by: Ross Wolf <[email protected]>

* [1.x] Nest as for foreign reuse (elastic#960) (elastic#962)

* [1.x] Remove `expected_event_types` from protocol (elastic#964) (elastic#965)

* [1.x] Expand definitions of source and destination field sets (elastic#967) (elastic#973)

* [1.x] Introduce `--strict` flag (elastic#937) (elastic#975)

* [1.x] Add example value composite type checking (elastic#966) (elastic#976)

* Add example value composite type checking (elastic#966)
* generate csv artifact

* [1.x] Add event category configuration (elastic#963) (elastic#977)

* [1.x] Add normalizer multi-field capability (elastic#971) (elastic#978)

Co-authored-by: Eric Beahan <[email protected]>

Co-authored-by: Madison Caldwell <[email protected]>

* [1.x] Add mapping network event guidance doc (elastic#969) (elastic#983)

* [1.x] Removing unneeded link under `Additional Information` (elastic#984) (elastic#985)

* [1.x] Add discrete attribute to field details page headers (elastic#989) (elastic#990)

* [1.x] Uniformity across domain name breakdown fields (elastic#981) (elastic#994)

Co-authored-by: Mathieu Martin <[email protected]>

* Add --oss flag to the ECS generator script (elastic#991) (elastic#995)

* Add network directions ingress and egress (elastic#945) (elastic#997)

* Mention ECS Mapper in the main documentation (elastic#987) (elastic#1000)

Co-authored-by: Dan Roscigno <[email protected]>

* [1.x] Introduce experimental artifacts (elastic#993) (elastic#1001)

Co-authored-by: Mathieu Martin <[email protected]>

* Bump version to 1.8.0-dev in branch 1.x (elastic#1011)

* Cut 1.7 changelog (elastic#1010) (elastic#1012)

* [1.x] Clarify that file extension should exclude the dot. (elastic#1016) (elastic#1020)

* [1.x] Add usage docs section (elastic#988) (elastic#1024)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] feat: include alias path when generating template (elastic#877) (elastic#1035)

Co-authored-by: Richard Gomez <[email protected]>

* [1.x] Add support for `scaling_factor` in the generator (elastic#1042) (elastic#1055)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add fallback for constant_keyword (elastic#1046) (elastic#1056)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add wildcard type support to go code generator (elastic#1050) (elastic#1057)

* add wildcard type support

* also add version and constant_keyword

* changelog

* [1.x] New default make task that generates main and experimental artifacts. (elastic#1041) (elastic#1060)

Also changing the order of the 'generate' task: it now starts with the new generator, then runs the legacy scripts.

* [1.x] Change the index pattern in the sample template. (elastic#1048) (elastic#1068)

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "getting-started" (elastic#1073) (elastic#1079)

Co-authored-by: EamonnTP <[email protected]>

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "products-solutions" page (elastic#1074) (elastic#1083)

Co-authored-by: EamonnTP <[email protected]>

* [1.x] Add event.category session. (elastic#1049) (elastic#1093)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add event.category registry (elastic#1040) (elastic#1094)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Add --ref support for experimental artifacts (elastic#1063) (elastic#1101)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Remove experimental event.original definition (elastic#1053) (elastic#1104)

* [1.x] Add missing `process.thread.name` to experimental definitions (elastic#1103) (elastic#1106)

* [1.x] Remove index parameter for wildcard fields (elastic#1115) (elastic#1119)

* [1.x] Add dns.answer object into experimental schema (elastic#1118) (elastic#1121)

* [1.x] Clarify x509 definition guidance for network events with only one cert (elastic#1114) (elastic#1123)

* [1.x] Indicate when artifacts include experimental changes (elastic#1117) (elastic#1125)

* [1.x] Add os.type field, with list of allowed values (elastic#1111) (elastic#1130)

* [1.x] Add support for constant_keyword's 'value' parameter (elastic#1112) (elastic#1132)

* [1.x] Beta label support (elastic#1051) (elastic#1133)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Backport elastic#1134 and elastic#1135 (elastic#1136)

* Remove temporary ifeval in "getting started" page, add link to Metrics docs (elastic#1134)
* Remove temporary ifeval from products page, add link to Metrics (elastic#1135)

* Two small documentation backports (elastic#1149)

* Remove an incorrect `event.type` from the 'converting' page (elastic#1146)
* Mention Logstash support for ECS in the 'products' page (elastic#1147)

* [1.x] Reinforce the exclusion of the leading dot from url.extension (elastic#1151) (elastic#1152)

* [1.x] Make all fields linkable directly via an HTML ID (elastic#1148) (elastic#1154)

* [1.x] Tracing fields should be at the root (elastic#1165)

* Add notice to the tracing field set, about not nesting field names. (elastic#1162)
* Tracing fields should be at top level in Beats artifact (elastic#1164)

* [1.x] Usage of brackets for a URL containing IPv6 address (elastic#1131) (elastic#1168)

* [1.x] 6.x index template data type fallback (elastic#1171) (elastic#1172)

* [1.x] Apply RFC 0007 stage 3 changes - multi-user (elastic#1066) (elastic#1175)

Conflict: deleted file rfcs/text/0007-multiple-users.md as RFCs are not backported to version branches.

* [1.x] Handle `error.stack_trace` case for ES 6.x template (elastic#1176) (elastic#1177)

* [1.x] Add composable index templates artifacts (elastic#1156) (elastic#1179)

* [1.x] Move _meta section back inside mappings, in legacy templates. (elastic#1186) (elastic#1187)

Backports the following commits to 1.x:

* Move _meta section back inside mappings, in legacy templates. (elastic#1186) 

This fixes an issue introduced by elastic#1156, discovered in elastic#1180. Composable templates support `_meta` at the template's root, but legacy templates don't. So we're just putting it back inside the mappings for legacy templates.

This also fixes missing updates to the component template, after the introduction of wildcard in elastic#1098.

* [1.x] Apply the RFC 0005 stage 2 (host metrics) changes in the experimental artifacts (elastic#1159) (elastic#1184)

Co-authored-by: Mathieu Martin <[email protected]>

* [1.x] Stage 3 changes for wildcard RFC 0001 (elastic#1098) (elastic#1183)

* [1.x] Conditional handling in es_template.template_settings (elastic#1191) (elastic#1192)

* [1.x] Artifacts docs page (elastic#1189) (elastic#1195)

* [1.x] Remove beta warning label from categorization fields docs (elastic#1067) (elastic#1196)

* [1.x] Correct wording of `event.reference` description (elastic#1181) (elastic#1197)

* Bump version to 1.9.0-dev in branch 1.x (elastic#1198)

* [1.x] Cut 1.8 FF changelog.next.md elastic#1199 (elastic#1201)

* Merge custom and core multi_fields arrays (elastic#982) (elastic#1213)

Co-authored-by: Jonathan Buttner <[email protected]>

* [1.x] Stage 2 changes for RFC 0009 - data_stream fields (elastic#1215) (elastic#1222)

* [1.x] add http.request.id (elastic#1208) (elastic#1223)

Co-authored-by: Eric Beahan <[email protected]>
Co-authored-by: Gil Raphaelli <[email protected]>

* [1.x] add cloud.service.name (elastic#1204) (elastic#1224)

* add cloud.platform

* expand cloud.platform description

* move to cloud.service.name

Co-authored-by: Gil Raphaelli <[email protected]>

* [1.x] Add ssdeep hash (elastic#1169) (elastic#1227)

Co-authored-by: Andrew Stucki <[email protected]>

* [CI] Switch to GitHub actions (elastic#1236) (elastic#1245)

Co-authored-by: Eric Beahan <[email protected]>

Co-authored-by: Andrew Stucki <[email protected]>

* Revert wildcard adoption back to experimental stage (elastic#1235) (elastic#1243)

* Add scaled_float type to go generator (elastic#1250) (elastic#1251)

* add scaled_float

* changelog

* Add categorization fields usage docs (elastic#1242) (elastic#1257)

* add time_zone, postal_code, and continent_code (elastic#1229) (elastic#1258)

* Specify MAC address format (elastic#456) (elastic#1260)

Co-authored-by: Robin Schneider <[email protected]>

* finalize 1.8.0 changelog (elastic#1262) (elastic#1265)

* Add additional host fields (elastic#1248) (elastic#1267)

Co-authored-by: kaiyan-sheng <[email protected]>

* Stage 1 changes for RFC 0014 - extend pe fields (elastic#1256) (elastic#1270)

* Add 2 fields to code_signature (elastic#1269) (elastic#1272)

Co-authored-by: Yamin Tian <[email protected]>

* Stage 3 changes for RFC 0007 - remove beta attribute (elastic#1271) (elastic#1273)

* Stage 1 experimental changes for RFC 0008 - threat.indicator fields (elastic#1268) (elastic#1274)

* Stage 1 changes for RFC 0015 - add elf fieldset (elastic#1261) (elastic#1275)

* Cut 1.9 FF CHANGELOG.next.md (elastic#1277)

* lock go version in actions (elastic#1283) (elastic#1290)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts (elastic#1310) (elastic#1320)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts

* Bump pyyaml from 5.3b1 to 5.4 in /scripts (elastic#1318) (elastic#1325)

Co-authored-by: Eric Beahan <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Adjust terminology - change whitelist to allowlist (elastic#1315) (elastic#1331)

Co-authored-by: Dominic Page <[email protected]>

* Remove -dev label from 1.9 version (elastic#1329)

* remove -dev label from 1.9 version

* generate artifacts

* removing rules artifacts

* Cut 1.9 changelog (elastic#1328)

* move 1.9 changes to changelog

* add 1.9 release changes
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.

Improve semantics of network.direction
6 participants