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

Base interface, VLAN, and zone updates for observer traffic #752

Merged
merged 8 commits into from Mar 4, 2020
Merged

Base interface, VLAN, and zone updates for observer traffic #752

merged 8 commits into from Mar 4, 2020

Conversation

dainperkins
Copy link
Contributor

Addding core observer, network, vlan, zone fields for tracking connection flows from e.g. netflow, firewalls (ingress, egress, etc.)

Addding core observer, network fields for tracking connection flows from e.g. netflow, firewalls (ingress, egress, etc.)
Updated change & pr #
@dainperkins
Copy link
Contributor Author

somehow the inner / outer vlan fields got lost in the shuffle from multiple PRs, I'll try and get that updated tomorrow

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Thanks @dainperkins

I notice that interface.id and vlan.id are both core and the rest is extended. What was the thinking there?

Got a few additional comments below. But we should be good to go soon, this is in line with what we discussed :-)

Comment on lines 29 to 30
description: >
Optional interface name as reported by the system. Could be short (eth0), long (gigabitethernet0/2),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm all for flexibility, but I wonder if users will want to track more than of those at the same time? E.g. the computer-generated one ("eth0") as well as the customized one ("dmz").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typically I'ver seen an observer use 1 or the other, but that begs the question of mixing interface name (gigabitethernet0/1) with logical name (outside)

Maybe we should add alias in the first go round? (for logical interface custom names in e.g. firewalls)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added alias - we can pick another name, but I think we would be better off NOT mixing actual interface names (eth0, gigabitethernet0/1, etc) with aliases (outside, inside, dmz, etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also - dropped everything to extended

schemas/observer.yml Show resolved Hide resolved
schemas/observer.yml Outdated Show resolved Hide resolved
schemas/observer.yml Outdated Show resolved Hide resolved
schemas/observer.yml Outdated Show resolved Hide resolved
schemas/interface.yml Outdated Show resolved Hide resolved
CHANGELOG.next.md Outdated Show resolved Hide resolved
address @webmat items, added interface.aslias, fixed network nesting for vlan id/name (network.vlan.., network.inner/outer.vlan...)  fixed examples
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Noticed a few small things that need adjusting.

I also think we should leverage the vlan.* description to tell users when to use which fields. See my comment for details.

CHANGELOG.next.md Show resolved Hide resolved
schemas/observer.yml Outdated Show resolved Hide resolved
- name: egress.zone
level: extended
type: keyword
short: Observer Egress Zone
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
short: Observer Egress Zone
short: Observer egress zone

schemas/vlan.yml Outdated
Comment on lines 5 to 7
description: >
The VLAN fields describe information about ingress and egress vlans reported
by an observer.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the vlan description to give some guidance on when to use network.vlan.* vs network.inner|outer.vlan.* e.g. when populating inner/outer, should the user also populate network.vlan.*? If so, with which value?

Same thing with observer.ingress|egress.vlan.* vs network*.vlan.*. Should they be populated at the same time? If not, when to pick one vs the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good ideas - I'll put something in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to update vlan.yml to be a little more prescriptive, both in the specific vlan fields, as well as utilizing them for network state (vlan of traffic, inner, outer, etc.

Updates pushed

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the adjustments so far. The observer.* vs network.* vlan fields usage is clearer now.

I'm not sure we've clarified whether network.vlan.* needs to be populated, when network.inner|outer.vlan.* are populated. When we populate inner/outer VLAN info, should we also populate network.vlan.*? If so, with what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well thats a bugger now that you have me thinking about it... maybe we can come to a quick decision on the call

schemas/interface.yml Show resolved Hide resolved
schemas/network.yml Outdated Show resolved Hide resolved
Updated descriptions to better define nesting and utilization of vlan etc.
removed outer.vlan in favor of "network.vlan" representing the native external vlan, with inner.vlan to represent an additional nested vlan for q-in-q
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Looking good.

I'll apply the two small fixes I'm commenting here myself, them merge.

Thanks Dain!

reported by an observer (e.g. firewall, router, load balancer) in the context of the
observer handling a network connection. In the case of a single observer interface
(e.g. network sensor on a span port) only the observer.ingress information should be
completed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
completed.
populated.

schemas/vlan.yml Outdated

Observer.ingress and observer.egress VLAN values are used to record observer specific
information when observer events contain discrete ingress and egress VLAN information,
typically provided by firewalls, routera, or load balancers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
typically provided by firewalls, routera, or load balancers.
typically provided by firewalls, routers, or load balancers.

@webmat webmat merged commit 42d211f into elastic:master Mar 4, 2020
dcode pushed a commit to dcode/ecs that referenced this pull request Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants