-
Notifications
You must be signed in to change notification settings - Fork 431
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
Clarify the semantics of network.direction
#212
Conversation
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.
The descriptions LGTM.
I'm a little hesitant to add another enum value. Prior to internal
I think would have have used unknown
for something like traffic on a loopback device. I'm wondering what the implications of the new state will be on analysis tools. Is there some new analysis that can be enabled by this new state? If not then I'd probably say that the three existing states are sufficient.
Is it common for network monitoring tools to say the direction is "unknown" when both sides are private / non-routable addresses, though? I feel like we're losing information that's readily available. I do think the presence of "internal" traffic is important for some kinds of analysis. For example a database server should have 100% internal traffic. If you see some inbound or outbound traffic, you would want to investigate right away. I agree that the value for a host-based monitoring solution to tag local traffic as "internal" probably has little use, but do these tools even report local network activity? |
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.
I am +1 for this change.
Suggest that we have five recommended values: external
internal
inbound
outbound
unknown
because some monitoring systems will provide us with independent indicators whether the source/dest or client/server was local, so we have four possible states if they're populated, and the fifth state in case they're not.
For example, when looking at the Zeek conn.log, we could apply logic as follows:
If Site::local_nets does not exist - network.direction: "unknown"
else {local_orig, local_resp}:
00 - network.direction: "external"
01 - network.direction: "inbound"
10 - network.direction: "outbound"
11 - network.direction: "internal"
We could add a clarifying note to the definition that says: If the network event is "remote," use "external" as the value of this field. If the network event is "local," use "internal" in this field.
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.
I leave the 👍 or 👎 to @MikePaquette / @robgil / @andrewkroh
I don't really like any of these fields. Specifically because they are very limited in their description and use. Some samples of where ambiguity rears its head. internal / externalInternal is very loosely defined. On a network you may have multiple customers such as subsidiaries, remote offices, and so on. Are these internal? No, not really. Most folks would consider them External is the same thing. Does it mean inbound / outboundI've also always felt these were ambiguous without the ProposalThis proposes a revamp of the naming to account for clearer network boundaries when describing For example, we could do something like this.
or
This gives the flexibility to define security zones by naming the networks. If a firewall loosely defines something as We can further abstract it in to the concept of zones.
or
Analyst ImpactWith the above proposal, the analyst can easily see if security boundaries have been violated. If logs come in with something like the following and are unexpected, its a clear red-flag.
Assume the database should not be accessible from untrusted networks |
Actually I agree with @MikePaquette that if we're to add "internal" to the list, we should also add "external", bringing the total list to 5:
I really like @robgil's proposal, and I agree that a single field named |
…ence. Felt out of place, now that we have internal and external as well.
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.
LGTM
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.
LGTM for beta/1.0
@andrewkroh Instead of removing "internal", we added "external". Are you good with it anyway? Are you seeing a problem with the semantics here? |
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.
I'm good with the additional enum values.
I think when these semantics are followed, it will be possible to reliably
do inbound/outbound traffic analysis for both host level and perimeter-level
network activity.
Since a given solution typically does one of the two kinds of monitoring,
analysts should not mix both kinds of monitoring anyway, when doing host-based
analysis vs perimeter-based.
The clarification I'm introducing makes me realise we should likely add another
expected value: "internal". So I've added it as a separate commit. Let me know
if you think this part should be reverted.