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 tcp flags to aws/vpcflow fileset #23157

Merged
merged 5 commits into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Add Google Workspace module and mark Gsuite module as deprecated {pull}22950[22950]
- Mark m365 defender, defender atp, okta and google workspace modules as GA {pull}23113[23113]
- Added support for first_event context in filebeat httpjson input {pull}23437[23437]
- Add parsing of tcp flags to AWS vpcflow fileset {issue}228020[22820] {pull}23157[23157]
- Added `alternative_host` option to google pubsub input {pull}23215[23215]

*Heartbeat*
Expand Down
10 changes: 10 additions & 0 deletions filebeat/docs/fields.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -2250,6 +2250,16 @@ type: keyword
The bitmask value for the following TCP flags: 2=SYN,18=SYN-ACK,1=FIN,4=RST


type: keyword

--

*`aws.vpcflow.tcp_flags_array`*::
+
--
List of TCP flags: 'fin, syn, rst, psh, ack, urg'


type: keyword

--
Expand Down
2 changes: 1 addition & 1 deletion x-pack/filebeat/module/aws/fields.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions x-pack/filebeat/module/aws/vpcflow/_meta/fields.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@
type: keyword
description: >
The bitmask value for the following TCP flags: 2=SYN,18=SYN-ACK,1=FIN,4=RST
- name: tcp_flags_array
type: keyword
description: >
List of TCP flags: 'fin, syn, rst, psh, ack, urg'
- name: type
type: keyword
description: >
Expand Down
33 changes: 33 additions & 0 deletions x-pack/filebeat/module/aws/vpcflow/ingest/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,39 @@ processors:
field: event.kind
value: event

- script:
lang: painless
ignore_failure: true
source: |
if (ctx?.aws?.vpcflow?.tcp_flags == null)
return;

if (ctx?.aws?.vpcflow?.tcp_flags_array == null) {
ArrayList al = new ArrayList();
ctx.aws.vpcflow.put("tcp_flags_array", al);
}

def flags = Integer.parseUnsignedInt(ctx.aws.vpcflow.tcp_flags);

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

Choose a reason for hiding this comment

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

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

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 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"]

Copy link
Member

@andrewkroh andrewkroh Dec 21, 2020

Choose a reason for hiding this comment

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

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.)

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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"

Copy link
Member

Choose a reason for hiding this comment

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

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

ctx.aws.vpcflow.tcp_flags_array.add('syn');
}
if ((flags & 0x04) != 0) {
ctx.aws.vpcflow.tcp_flags_array.add('rst');
}
if ((flags & 0x08) != 0) {
ctx.aws.vpcflow.tcp_flags_array.add('psh');
}
if ((flags & 0x10) != 0) {
ctx.aws.vpcflow.tcp_flags_array.add('ack');
}
if ((flags & 0x20) != 0) {
ctx.aws.vpcflow.tcp_flags_array.add('urg');
}

on_failure:
- set:
field: "error.message"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
"aws.vpcflow.pkt_srcaddr": "10.20.33.164",
"aws.vpcflow.subnet_id": "subnet-22222222bbbbbbbbb",
"aws.vpcflow.tcp_flags": "3",
"aws.vpcflow.tcp_flags_array": [
"fin",
"syn"
],
"aws.vpcflow.type": "IPv4",
"aws.vpcflow.version": "3",
"aws.vpcflow.vpc_id": "vpc-abcdefab012345678",
Expand Down
2 changes: 2 additions & 0 deletions x-pack/filebeat/module/aws/vpcflow/test/tcp-flag-sequence.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
version vpc-id subnet-id instance-id interface-id account-id type srcaddr dstaddr srcport dstport pkt-srcaddr pkt-dstaddr protocol bytes packets start end action tcp-flags log-status
3 vpc-abcdefab012345678 subnet-aaaaaaaa012345678 i-01234567890123456 eni-1235b8ca123456789 123456789010 IPv4 52.213.180.42 10.0.0.62 43416 5001 52.213.180.42 10.0.0.62 6 568 8 1566848875 1566848933 ACCEPT 2 OK
3 vpc-abcdefab012345678 subnet-aaaaaaaa012345678 i-01234567890123456 eni-1235b8ca123456789 123456789010 IPv4 52.213.180.42 10.0.0.62 43638 5001 52.213.180.42 10.0.0.62 6 1260 17 1566933133 1566933193 ACCEPT 3 OK
3 vpc-abcdefab012345678 subnet-aaaaaaaa012345678 i-01234567890123456 eni-1235b8ca123456789 123456789010 IPv4 10.0.0.62 52.213.180.42 5001 43638 10.0.0.62 52.213.180.42 6 967 14 1566933133 1566933193 ACCEPT 19 OK
Loading