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

added nat.ip and nat.port to source and destination fields #491

Merged
merged 3 commits into from
Jul 4, 2019
Merged

added nat.ip and nat.port to source and destination fields #491

merged 3 commits into from
Jul 4, 2019

Conversation

dainperkins
Copy link
Contributor

To better support Port/Address translation in flow/fw type network events added source.nat.ip, source.nat.port, destination.nat.ip, destination.nat.port.

If, in the case of e.g. 1:1 nat, the observer doesn't report nat ports, the original port should be copied to the nat.port field for ease of searching all fields of the connection (e.g. searching nat ips/ports for threat hunting etc)

To - do:
If there is telemetry provided across devices implementing NAT (e.g. internally load balanced source nat sessions being reported across multiple devices before and after S/NAT) we may want to also include:

  • nat.session_id
    • copy native session id at NAT gateway to stitch together pre/post NAT sessions

Updated url host
@MikePaquette MikePaquette self-requested a review June 28, 2019 15:43
@webmat webmat self-requested a review June 28, 2019 15:45
@webmat
Copy link
Contributor

webmat commented Jun 28, 2019

@dainperkins Thanks for the PR.

At this time, the network pairs are source/destination as well as client/server. We keep them in sync. Could you add the fields to client/server too, please?

Now let's clarify the descriptions a tiny bit. The nat.ip and nat.port are meant to be the private values, correct? Could you state this more explicitly in the description?

After these adjustments, please run make to generate all the other files, and commit all of the generated files as well. The build will fail if generated files are not updated and committed according to your changes.

@dainperkins
Copy link
Contributor Author

@webmat
I specifically avoided specifying public/private as theres plenty of cases where all IPs will be private or public (nat and natural), but will better scope the field definitions and copy to client/server

@webmat
Copy link
Contributor

webmat commented Jun 28, 2019

Ah you're right. Perhaps use wording such as "the internal side of..."

modified definitions and copied source/dest to client/server
Copy link
Contributor

@MikePaquette MikePaquette left a comment

Choose a reason for hiding this comment

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

LGTM

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.

LGTM

Thanks @dainperkins

@webmat
Copy link
Contributor

webmat commented Jul 2, 2019

@dainperkins One last little thing and then we merge.

Could you add the following entry to the file CHANGELOG.next.md, please?

* Added `.nat.ip` and `.nat.port` to `source`, `destination`, `client` and `server`. #491 

@dainperkins
Copy link
Contributor Author

added, & made (and pushed to my nat-ips fork - do I have to pull again?)

@webmat
Copy link
Contributor

webmat commented Jul 4, 2019

Great! Thanks @dainperkins

@webmat webmat merged commit ccba36b into elastic:master Jul 4, 2019
@neu5ron
Copy link

neu5ron commented Jul 5, 2019

awesome work yall!
this is great!

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.

4 participants