search

falcosecurity / plugins

Falco plugins registry
Apache License 2.0
83 stars 75 forks source link

empty string with double quotes in aws_cloudtrail event `""` #56

Closed pabloopez closed 2 years ago

pabloopez commented 2 years ago

Describe the bug

In an aws_cloudtrail event, the value "" is not interpreted as an empty string, but literally as two double quotes.

How to reproduce it

Run one of the latest builds of Falco 0.30 (for example this one), set config file to:

plugins:
  - name: cloudtrail
    library_path: libcloudtrail.so
    init_config: ""
    open_params: "/root/cloudtrail.json"
  - name: json
    library_path: libjson.so
    init_config: ""
load_plugins: [cloudtrail,json]

and create a log file /root/cloudtrail.json with the content of this gist.

The last event in the file should trigger the rule Delete Bucket Public Access Block, but it does not. If I modify the rule condition to make it more general and just be triggered by the eventName being PutBucketPublicAccessBlock and include in the output the next fields:

    field_1=%json.value[/requestParameters/publicAccessBlock]
    field_2=%json.value[/requestParameters/PublicAccessBlockConfiguration/RestrictPublicBuckets]
    field_3=%json.value[/requestParameters/PublicAccessBlockConfiguration/BlockPublicPolicy]
    field_4=%json.value[/requestParameters/PublicAccessBlockConfiguration/BlockPublicAcls]
    field_5=%json.value[/requestParameters/PublicAccessBlockConfiguration/IgnorePublicAcls]

The alert I get contains:

23:54:15.000000000: Critical A pulic access block for a bucket has been deleted (requesting user=developer, requesting IP=24.7.51.241, AWS region=eu-central-1, bucket=fak3-name-066997976161), field_1="" field_2=true field_3=true field_4=false field_5=true

Expected behaviour

Expected behaviour when I use the original rule: an alert is displayed when I use the logs file above for the rule: Delete Bucket Public Access Block. I don't think it is a problem of the rule syntax (including here @seishinsha in case it is), but how an empty value returned by AWS is interpreted.

Screenshots

Non required, check the code_blocks above please.

Environment

Additional context

I can provide a VM with the same environment I am using to test this.

seishinsha commented 2 years ago

Just wanted to confirm that the original Delete Bucket Public Access Block rule (here) has been validated with the Cloud Connector against the same json event (here) and it is effectively fired. Which leads me to believe that the problem might be in the parser implementation.

Screenshots

image

mstemm commented 2 years ago

Part of the problem was generally a bug in this PR (https://github.com/falcosecurity/libs/pull/74), combined with how plugin filterchecks advertise their compatible event types (https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/plugin.cpp#L85).

In #74 we pushed down some code from falco that determines the sinsp event types that are applicable for a given filter. The set of event types starts with "all events", and as the filter condition is parsed, including logical operators like "and", "or", "not", etc, the set of event types is changed, honoring the logical operators.

For example, for a condition proc.name=nginx, the filter is applicable for all event types, as the condition does not include any evt.type field. For a more complicated condition like evt.type=openat and proc.name=nginx, the first field restricts the event types to openat, which is logical anded against all event types from proc.name, resulting in only the event type openat.

With the introduction of plugins, there's also a need to segregate plugin-related filterchecks from non-plugin-related filterchecks by event source, but that's handled at a higher level, using a notion of filter factories and formatter factories (https://github.com/falcosecurity/libs/pull/77).

The bug is that plugin filtercheck fields like ct.name, json.value were mistakenly being restricted to only the plugin event PPME_PLUGINEVENT_E. This was being mistakenly inverted when conditions had a "not" operator, with the result being that they did not run on any event types at all.

The fix is to treat plugin fields as working with all event types, just like almost all other fields like proc.name, etc. are. This allows the logical operators to combine event type sets properly.

mstemm commented 2 years ago

The other half of the problem is that the double-quote "" part of json.value[/requestParameters/publicAccessBlock]="" and needs to be escaped, like this:

json.value[/requestParameters/publicAccessBlock]='""' and

When I fix the above two things, falco generates alerts for the rule Delete Bucket Public Access Block and the last event in the linked gist.

pabloopez commented 2 years ago

Thank you for the detailed explanation @mstemm and for the time to fix it!

One question about the second comment:

The other half of the problem is that the double-quote "" part of json.value[/requestParameters/publicAccessBlock]="" and needs to be escaped, like this: json.value[/requestParameters/publicAccessBlock]='""' and

Does it mean that in the rules "" will always need to be escaped? When I found this behaviour, the first thing I did was go to the default set of Falco rules to see if there was a function/operator like is_empty but I found instead that the way it is done is like this: field = "".

I won't discuss if this is good or bad, but it would be nice to keep this syntax consistent between plugin and former fields. Or maybe the quotes in this example are part of what is being parsed in the json. Then it makes sense.

Thanks again for your help!

mstemm commented 2 years ago

Thank you for the detailed explanation @mstemm and for the time to fix it!

One question about the second comment:

The other half of the problem is that the double-quote "" part of json.value[/requestParameters/publicAccessBlock]="" and needs to be escaped, like this: json.value[/requestParameters/publicAccessBlock]='""' and

Does it mean that in the rules "" will always need to be escaped? When I found this behaviour, the first thing I did was go to the default set of Falco rules to see if there was a function/operator like is_empty but I found instead that the way it is done is like this: field = "".

I won't discuss if this is good or bad, but it would be nice to keep this syntax consistent between plugin and former fields. Or maybe the quotes in this example are part of what is being parsed in the json. Then it makes sense.

Thanks again for your help!

Yes it does, but we already know how to escape literal double quotes in rules files, see https://falco.org/docs/rules/#escaping-special-characters.

leogr commented 2 years ago

Yes it does, but we already know how to escape literal double quotes in rules files, see https://falco.org/docs/rules/#escaping-special-characters.

No, strings will not need to be always escaped. Strings need to be escaped only when containing special characters. However it's not the problem described in this issue.

The real problem issue was in the extractor function of the json plugin, all strings were returned enclosed by a double-quote. We fixed it by #59

Now json.value[/requestParameters/publicAccessBlock]="" works as expected.