tracee: add tini tracee docker image #883

josedonizetti · 2021-08-05T20:54:50Z

Fixes #736. If adding a dependency is not a problem, tini is a very common solution to handle signals properly on containers.

The image size doesn't change:

tracee:with_tini         446MB
tracee:without_tini      446MB

Testing:

# first shell
# build trace image
docker build  -t tracee:with_tini .
# run tracee
docker run -v /boot/config-$(uname -r):/boot/config-$(uname -r) --name tracee --rm --privileged -it tracee:with_ini

# second shell
docker kill --signal=SIGINT tracee
# or
docker kill --signal=SIGTERM tracee

josedonizetti · 2021-08-05T20:55:16Z

@simar7 Let me know what you think please.

simar7 · 2021-08-06T00:27:35Z

@josedonizetti we've already added an exec directive to launch so I don't think this is necessary as signals are handled appropriately. https://github.com/aquasecurity/tracee/blob/main/entrypoint.sh#L9

Did I miss something?

josedonizetti · 2021-08-06T02:19:28Z

@simar7 well, maybe I am missing something, but testing here in two different computers (ubuntu 18.04, and 20.04) with aquasec/tracee:latest ( i have delete the image to have docker pull latest) and SIGINT is only handled if I hit CTRL+C, if I send a SIGINT or SIGTERM via docker kill or kill the container doesn't gracefully exit.

itaysk · 2021-08-06T12:35:33Z

Only the "trace" subcommand is using exec, and therefore handled appropriately. the "regular" tracee cannot leverage exec since it's not a single command (<- currently. we do have plans to make it a single process).

Here's my test result for the "regular" tracee

method	explanation	did it quit
CTRL+C (in the terminal when running with `-it`)	send INT to foreground group, i.e to bash, tracee-ebpf, tracee-rules individually	yes
kill -INT (to the container's root PID)	send SIGINT to bash	no
kill -TERM (to the container's root PID)	send SIGTERM to bash)	yes (actually I don't understand why)
docker stop	send SIGTERM to the container's root PID	yes

itaysk · 2021-08-06T12:42:50Z

anyway, I think it's fine to add tini to the container to get more predictable experience. @simar7 do you have another idea to fix the issue?
When we merge the processes we can reconsider if we need it.

josedonizetti · 2021-08-06T13:58:04Z

@itaysk if you prefer I can investigate a little further if we can accomplish the same without tini. About your tests results, at least for me docker stop is also not working.

Docker will first attempt a SIGTERM, but if the container doesn't respect it, it forces a SIGKILL.

Eg:

$ docker run --name tracee --rm --privileged -it tracee:with_tini
Loaded signature(s):  [TRC-1 TRC-2 TRC-3 TRC-4 TRC-5 TRC-6 TRC-7]
$ echo $?
143 # using tini, SIGTERM was respected

$ docker run --name tracee --rm --privileged -it aquasec/tracee:latest
Loaded signature(s):  [TRC-1 TRC-2 TRC-3 TRC-4 TRC-5 TRC-6 TRC-7]
$ echo $?
137 # without tini, a SIGKILL was forced here

simar7 · 2021-08-06T22:09:25Z

anyway, I think it's fine to add tini to the container to get more predictable experience. @simar7 do you have another idea to fix the issue?
When we merge the processes we can reconsider if we need it.

Docker 1.13 and above should have the signal handling functionally if we pass in --init flag. See below for example:

vagrant@ubuntu-groovy:~/linux$ docker run -it --name  tracee --rm --privileged -v /lib/modules/:/lib/modules/:ro -v /usr/src:/usr/src:ro -v /tmp/tracee:/tmp/tracee aquasec/tracee:0.5.2
Loaded signature(s):  [TRC-1 TRC-2 TRC-3 TRC-4 TRC-5 TRC-6 TRC-7]

vagrant@ubuntu-groovy:~/linux$ echo $?
137

Now with --init flag:

vagrant@ubuntu-groovy:~/linux$ docker run --init -it --name  tracee --rm --privileged -v /lib/modules/:/lib/modules/:ro -v /usr/src:/usr/src:ro -v /tmp/tracee:/tmp/tracee aquasec/tracee:0.5.2
Loaded signature(s):  [TRC-1 TRC-2 TRC-3 TRC-4 TRC-5 TRC-6 TRC-7]

vagrant@ubuntu-groovy:~/linux$ echo $?
143

Ref: https://docs.docker.com/engine/reference/commandline/run/

We could include tini instead of adding this to our docs. I just err on the side of including less dependencies if we can.

Thoughts?

josedonizetti · 2021-08-06T23:17:46Z

@simar7 Yes, --init is a good option, specially because it is tini that docker uses to support it. But a few considerations.

1 - I wonder if this isn't too specific for docker only, it seems podman supports --init too, but I don't think cri-o and containerd do.
2 - For anyone running tracee as a k8s dameonset, will also not handle SIGTERM, and k8s forcing a SIGKILL during a rolling update.
3 - It seems tracee is expecting external behaviour to guarantee an internal feature (handling signals, and gracefully exiting)

wdyt?

simar7

Fair enough - sounds like there are enough reasons to have it then to not. I'll let @itaysk give it another read but lgtm

itaysk

Not sure if we should removed the exec from trace sub command

tracee: add tini tracee docker image

d839835

simar7 requested review from simar7 and itaysk August 6, 2021 23:21

simar7 approved these changes Aug 6, 2021

View reviewed changes

itaysk approved these changes Aug 7, 2021

View reviewed changes

itaysk merged commit 26a9eb2 into aquasecurity:main Aug 7, 2021

josedonizetti deleted the add-tini branch August 7, 2021 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tracee: add tini tracee docker image #883

tracee: add tini tracee docker image #883

tracee: add tini tracee docker image #883

tracee: add tini tracee docker image #883

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment