add tcp flags to aws/vpcflow fileset #23157

leehinman · 2020-12-15T21:46:03Z

What does this PR do?

Converts the existing aws.vpcflow.tcp_flags bit mask into an array of
tcp flags and stores them in aws.vpcflow.tcp_flags_array.

Why is it important?

makes it easier to query if a particular flag is set

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
~~- [ ] I have made corresponding change to the default configuration files~~
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

TESTING_FILEBEAT_MODULES=aws \
TESTING_FILEBEAT_FILESETS=vpcflow \
mage -v pythonIntegTest

Related issues

Closes [filebeat][aws][vpcflow] Parse aws vpcflow tcp_flags #22820

elasticmachine · 2020-12-15T21:47:51Z

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

elasticmachine · 2020-12-15T22:38:17Z

💚 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

Build Cause: Pull request #23157 updated
- Start Time: 2021-01-13T21:10:24.059+0000
Duration: 48 min 37 sec
Commit: b005a83

Test stats 🧪

Test	Results
Failed	0
Passed	5129
Skipped	574
Total	5703

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	5129
Skipped	574
Total	5703

ynirk · 2020-12-16T10:26:41Z

@leehinman AWS has a weird way of dealing ACK:

In the Flow Logs, the handshake is shown as two lines, with tcp-flags values of 2 (SYN), 18 (SYN + ACK).  
ACK is reported only when it is accompanied with SYN (otherwise it would be too much noise for you to filter out).

So maybe we should have a syn-ack flag instead of ack when you have syn + ack

leehinman · 2020-12-16T15:02:21Z

@leehinman AWS has a weird way of dealing ACK:
In the Flow Logs, the handshake is shown as two lines, with tcp-flags values of 2 (SYN), 18 (SYN + ACK).  
ACK is reported only when it is accompanied with SYN (otherwise it would be too much noise for you to filter out).
So maybe we should have a syn-ack flag instead of ack when you have syn + ack

If bitmask is 18 you should get

 "aws.vpcflow.tcp_flags_array": [
            "syn",
            "ack"
        ]

Is that OK? The code will add to the array for every bit that is set. I think what Amazon is saying is that they don't bother to report when only 'ack' is set.

ynirk · 2020-12-16T17:25:49Z

sounds good

andrewkroh

This should good after the syn-ack support. I looked this over and also consulted these AWS docs to see some examples.

andrewkroh · 2020-12-21T19:27:57Z

x-pack/filebeat/module/aws/vpcflow/ingest/pipeline.yml

+        if ((flags & 0x01) != 0) {
+          ctx.aws.vpcflow.tcp_flags_array.add('fin');
+        }
+        if ((flags & 0x02) != 0) {


I think that if you check for 0x12 first (syn-ack) and then subtract that value out this will be good.

I think that if you check for 0x12 first (syn-ack) and then subtract that value out this will be good.

This could be grumpy old networking guy talking but there is no SYN-ACK flag, you just have the syn and the ack flags set. So the array that gets populated should contain 'syn' and 'ack'.

/g/k/h/t/n/n/ /r/c/s/s/y/i/ /u/a/p/r/s/f/ 0 1 0 0 1 0 (0x12 or 18)

Which would give you:

"aws.vpcflow.tcp_flags_array": ["syn", "ack"]

Or we could go with a keyword instead of array where we cat the flags together separated by "-", that would give:

"aws.vpcflow.tcp_flags_array": "syn-ack"

But I have trouble mixing the two approaches. For example if all the fields were set would you expect:

"aws.vpcflow.tcp_flags_array": ["fin", "syn", "rst", "psh", "ack", "urg"] or "aws.vpcflow.tcp_flags_array": ["fin", "syn-ack", "rst", "psh", "urg"]

I realize that there is no syn-ack flag and that this is a bitmask. 😄

The reason for the unique handling is that AWS treats ACK special. It only sets the ACK bit when it sees it as part of a SYN,ACK response. If, for example, it sees a plain ACK or PSH,ACK packet in a flow it won't report the ACK bit. So rather than having a plain ACK in the list I was thinking having the SYN-ACK in there to highlight the meaning, but we could just as well report ["SYN", "ACK"] and have consumers understand the meaning of ACK in this case.

In my experience this is distinct from how Netflow reports tcp flags. It uses a simple OR based aggregation over the flow. This makes it nearly impossible to determine which side initiated a flow or terminated it. (BTW there's a issue #12858 to do this same thing for netflow.)

From a query perspective I don't think it matters too much. You can query flags:SYN and not flags.ACK or flags:SYN and flags:ACK.

Yeah, I see that AWS will never report a naked "ACK". 😃

Looks like AWS will also OR across the aggregation interval (19 for fin, syn & ack), added example of that.

I was also thinking the code could be useful for other data sets like liblogparser.js, which also makes me lean towards just turning the existing flags into names.

That being said, I can live with "syn-ack"

I'm good with separate keywords for each bit given that there is no loss of functionality from a query perspective.

- new field aws.vpcflow.tcp_flags_array Closes elastic#22820

andrewkroh · 2021-01-13T03:31:56Z

@leehinman Is this one ready to merge? When we get an ECS field declared we can come back and update separately (if that's what you're waiting on).

run tests

* add tcp flags to aws/vpcflow fileset - new field aws.vpcflow.tcp_flags_array Closes elastic#22820 Co-authored-by: Andrew Kroh <[email protected]> (cherry picked from commit 8beb815)

* add tcp flags to aws/vpcflow fileset - new field aws.vpcflow.tcp_flags_array Closes #22820 Co-authored-by: Andrew Kroh <[email protected]> (cherry picked from commit 8beb815)

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 15, 2020

leehinman added enhancement Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. Team:Security-External Integrations labels Dec 15, 2020

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Dec 15, 2020

leehinman force-pushed the 22820_vpcflow_tcp_flags branch from f47df01 to 05a1429 Compare December 15, 2020 21:48

leehinman mentioned this pull request Dec 15, 2020

[filebeat][aws][vpcflow] Parse aws vpcflow tcp_flags #22820

Closed

andrewkroh reviewed Dec 21, 2020

View reviewed changes

andrewkroh approved these changes Dec 21, 2020

View reviewed changes

leehinman added 2 commits December 21, 2020 17:04

add tcp flags to aws/vpcflow fileset

7f5fb77

- new field aws.vpcflow.tcp_flags_array Closes elastic#22820

Add additional examples

ae54f40

leehinman force-pushed the 22820_vpcflow_tcp_flags branch from 11e98ca to ae54f40 Compare December 21, 2020 23:05

andrewkroh and others added 3 commits January 12, 2021 22:32

Merge branch 'master' into 22820_vpcflow_tcp_flags

0cb990b

Merge branch 'master' into 22820_vpcflow_tcp_flags

ae5eff5

update fields.asciidoc

b005a83

leehinman merged commit 8beb815 into elastic:master Jan 13, 2021

leehinman deleted the 22820_vpcflow_tcp_flags branch January 13, 2021 22:13

leehinman mentioned this pull request Jan 13, 2021

Cherry-pick #23157 to 7.x: add tcp flags to aws/vpcflow fileset #23494

Merged

5 tasks

leehinman added v7.12.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add tcp flags to aws/vpcflow fileset #23157

add tcp flags to aws/vpcflow fileset #23157

leehinman commented Dec 15, 2020

elasticmachine commented Dec 15, 2020

elasticmachine commented Dec 15, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Test stats 🧪

ynirk commented Dec 16, 2020

leehinman commented Dec 16, 2020

ynirk commented Dec 16, 2020

andrewkroh left a comment

andrewkroh Dec 21, 2020

leehinman Dec 21, 2020

andrewkroh Dec 21, 2020 •

edited

Loading

andrewkroh Dec 21, 2020

leehinman Dec 21, 2020

andrewkroh Dec 21, 2020

andrewkroh commented Jan 13, 2021 •

edited

Loading

add tcp flags to aws/vpcflow fileset #23157

add tcp flags to aws/vpcflow fileset #23157

Conversation

leehinman commented Dec 15, 2020

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

elasticmachine commented Dec 15, 2020

elasticmachine commented Dec 15, 2020 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

Test stats 🧪

ynirk commented Dec 16, 2020

leehinman commented Dec 16, 2020

ynirk commented Dec 16, 2020

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh Dec 21, 2020

Choose a reason for hiding this comment

leehinman Dec 21, 2020

Choose a reason for hiding this comment

andrewkroh Dec 21, 2020 • edited Loading

Choose a reason for hiding this comment

andrewkroh Dec 21, 2020

Choose a reason for hiding this comment

leehinman Dec 21, 2020

Choose a reason for hiding this comment

andrewkroh Dec 21, 2020

Choose a reason for hiding this comment

andrewkroh commented Jan 13, 2021 • edited Loading

elasticmachine commented Dec 15, 2020 •

edited by jenkins-beats-ci bot

Loading

andrewkroh Dec 21, 2020 •

edited

Loading

andrewkroh commented Jan 13, 2021 •

edited

Loading