obsrvbl-oss / flowlogs-reader

Command line tool and Python library for working with AWS VPC Flow Logs
Apache License 2.0
136 stars 25 forks source link

Support VPC Flow Logs v6 fields for TGW #59

Closed bbayles closed 1 year ago

bbayles commented 1 year ago

This PR is an alternative to https://github.com/obsrvbl/flowlogs-reader/pull/58.

I've added support for each of the version 6 fields shown here. This should cause flowlogs_reader to be able to interpret logs with Transit Gateway records.

BThacker commented 1 year ago

I think this will help as well.

The fields are mixmatched across the board.

Here is an example of one of the VPCFlowLogs s3 files, that shows how they are mixmatched when a Transit gateway logs are injected in them. The fields not aligning is an issue for much more than just account_id and the default fields for TGW seem to be much wider. No idea why AWS seems to allow the logs to contain both.

version account-id interface-id srcaddr dstaddr srcport dstport protocol packets bytes start end action log-status
2 999999999999 eni-99999999999999999 10.10.10.10 10.10.10.11 65532 389 17 1 84 9999999999 9999999999 ACCEPT OK
6 TransitGateway 999999999999 tgw-99999999999999999 tgw-attach-99999999999999999 999999999999 999999999999 - vpc-99999999999999999 - subnet-99999999999999999 - eni-99999999999999999 usw2-az4 usw2-az3 tgw-attach-99999999999999999 10.10.10.10 10.10.10.11 9769 123456 6 7 1234 9999999999 8888888888 OK IPv4 0 0 0 0 - us-west-2 ingress - -
bbayles commented 1 year ago

Maybe there are two different things writing logs to the same location? And they have different formats configured?

I've added a check_column_count keyword to filter out rows with the wrong number of columns.

BThacker commented 1 year ago

Maybe there are two different things writing logs to the same location? And they have different formats configured?

I've added a check_column_count keyword to filter out rows with the wrong number of columns.

That is what seems to happen. If you turn on TGW logging and point it to the same S3 bucket, it will combine the logs and you end up with this result.

BThacker commented 1 year ago

@bbayles after some more discussion internally we've decided to drop the desire to check the column count and loop over each record twice. It turns out this is an edge case created specifically for one scenario and not something that can commonly happen by sending flow logs and transit gateway logs to S3. However we would still like to keep your changes for the future handling of TGW records. Can you remove the check column count and just leave the extra slots? If so we can approve and roll this up.

Our new direction for handling this specific case will just be to skip over the record if it valueerrors and continuing parsing the rest of the file. See https://github.com/obsrvbl/flowlogs-reader/tree/try_catch_valueerror

bbayles commented 1 year ago

I've reverted the last two commits - thanks.