unable to handle containers with huge metadata

[leogr]

Describe the bug

This is an early description of the problem. I'm investigating. More details will come.

libsinsp throws an exception when a container with a lot of metadata is noticed.

It seems that the JSON (containing the container metadata) is truncated, and the JSON parser is returning an empty error.

The bug seems to cause the problem reported in falcosecurity/falco#683

How to reproduce it

TODO

Expected behaviour

Screenshots

Invalid JSON encountered while parsing container info: {"container":{"Mounts":[],"cpu_period":100000,"cpu_quo...

...a lot of JSON in the middle...

...ent tools\"},{\"name\":\"e2fsprogs\",\"versioerror=

Note:

• the JSON seems to be truncated at some point
• error= (the error returned by the parser) is empty

Environment

N/A

Additional context

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[mikhailadvani]

/remove-lifecycle stale

[FedeDP]

Reporting same comment that i wrote on falco issue: falcosecurity/falco#683 (comment).

I am working on a possible fix.

[FedeDP]

We currently have the following limitation: it seems our protocol is not able to manage single arguments larger than 64kiB.
In this case, if the json is larger, it gets truncated and we are not able to parse it.

Given that we cannot change the protocol (otherwise we'd lose backward compatibility), here are a couple of ideas on how to fix it:

1. Always parse entire json but then adhere to 64kB limit for outputs.
   This is the simplest fix as it allows us to properly parse the json, but it comes with its bugs:

• when storing to a scap file we will only store 64kB, thus truncating the json; this means that we won't be able to replay the stored file because it will crash with same error outlined in this issue
• In stdout output, json will be truncated to 64kB

2. Split the json argument into multiple (up to 32) 64kB arguments.
   This is somewhat more tricky, but it allows us to support up to 2MiB. Yet, it comes with its set of issues:

• we still have a hard limit on json dimension, even if greater than before
• in event_table.c we need to repeat 32times the same {"json", PT_CHARBUF, PF_NA} argument (it's ok, it is just quite ugly)

3. Add a new ppm_event_flags that tells that the event size must be parsed as an uint32_t.
   While it is probably the cleanest solution, it would break reading a scap file from eg: wireshark.

Note however that i think we should enforce a proper json size limit anyway, because right now we are trusting external json source to give us a normal-sized json, as we are calling:

size_t totlen = sizeof(scap_evt) +  sizeof(uint16_t) + json.length() + 1;

ASSERT(evt->m_pevt_storage == nullptr);
evt->m_pevt_storage = new char[totlen];

potentially heap-allocating huge chunks of memory.

I guess that solution 2. together with a 2MiB limit on json dimension is our best candidate, yet for a simpler solution (maybe while thoroughly thinking about a backward compatible protocol change?) 1. can be enough.

/cc @gnosek

[FedeDP]

In the end, after a discussion with @leogr , it was decided to use a best-effort approach: if resulting json is too large to be handled, we first try to trim useless metadata (read: metadata that is not used as a filter), ie:

• "env"
• "port_mappings"
• "labels"

Eventually, if we still need to free space, "Mounts" is killed too.

This is the best possible approach as it does not require many changes yet it always preserves container main info, like id, image and whatever, only trimming fields when needed.

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

This issue should be fixed by #102
/close

[poiana]

@leogr: Closing this issue.

In response to this:

This issue should be fixed by #102
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.