sysdig assertion failure when evt.res can't be resolved #598

mstemm · 2016-05-23T06:29:19Z

In one of the docker-compose files I've been using for falco testing, the result from a call to clone is -513 (which shouldn't happen from my reading of clone(), but it's there):

2215327 15:13:37.217888690 0 docker (531) > clone
2215328 15:13:37.219435808 0 docker (531) < clone res=-513 exe= args= tid=531(docker) pid=17116(docker) ptid=1(systemd) cwd=/ fdlimit=1048576 pgft_maj=0 pgft_min=28599 vm_size=893864 vm_rss=64092 vm_swap=4 comm=docker cgroups=cpuset=/system.slice.cpu=/system.slice.cpuacct=/system.slice.blkio=/system.sl... flags=0 uid=0 gid=0 vtid=531(docker) vpid=17116(docker)

If I run sysdig on this trace file with a filter sysdig -r ../../falco/test/traces-negative/docker-compose.scap evt.res = EACCESS in debug mode, I get an assertion failure:

sysdig -r ../../falco/test/traces-negative/docker-compose.scap evt.res = EACCESS
sysdig: /mnt/sf_mstemm/work/src/sysdig/userspace/libsinsp/filterchecks.cpp:3287: virtual uint8_t* sinsp_filter_check_event::extract(sinsp_evt*, uint32_t*): Assertion `resolved_argstr != __null && resolved_argstr[0] != 0' failed.
Aborted (core dumped)

Here's the relevant code:

ASSERT(pi->m_len == sizeof(int64_t));

int64_t res = *(int64_t*)pi->m_val;
if(res >= 0)
{
    *len = sizeof("SUCCESS");
    return (uint8_t*)"SUCCESS";
}
else
{
    argstr = evt->get_param_value_str("res", &resolved_argstr);
    ASSERT(resolved_argstr != NULL && resolved_argstr[0] != 0);

    if(resolved_argstr != NULL && resolved_argstr[0] != 0)
    {
       return (uint8_t*)resolved_argstr;
    }
    else if(argstr != NULL)
    {
       return (uint8_t*)argstr;
    }
}

The raw value of res cast to an int64_t is -12, btw.

It seems like the assertion should be removed as the if handles that case.

The text was updated successfully, but these errors were encountered:

This rule is exposing a bug in sysdig in debug mode, draios/sysdig#598. I'll disable it for now just so I can get the testing half stable, and decide what to do before merging the PR.

Do another round of rule cleanups now that we have a larger set of positive and negative trace files to work with. Outside of this commit, there are now trace files for all the positive rules, a docker-compose startup and teardown, and some trace files from the sysdig cloud staging environment. Also add a script that runs sysdig with a filter that removes all the syscalls not handled by falco as well as a few other high-volume, low-information syscalls. This script was used to create the staging environment trace files. Notable rule changes: - The direction for write_binary_dir/write_etc needs to be exit instead of enter, as the bin_dir clause works on the file descriptor returned by the open/openat call. - Add login as a trusted binary that can read sensitive files (occurs for direct console logins). - sshd can read sensitive files well after startup, so exclude it from the set of binaries that can trigger read_sensitive_file_trusted_after_startup. - limit run_shell_untrusted to non-containers. - Disable the ssh_error_syslog rule for now. With the current restriction on system calls (no read/write/sendto/recvfrom/etc), you won't see the ssh error messages. Nevertheless, add a string to look for to indicate ssh errors and add systemd's true location for the syslog device. - Sshd attemps to setuid even when it's not running as root, so exclude it from the set of binaries to monitor for now. - Let programs that are direct decendants of systemd spawn user management tasks for now. - Temporarily disable the EACCESS rule. This rule is exposing a bug in sysdig in debug mode, draios/sysdig#598. The rule is also pretty noisy so I'll keep it disabled until the sysdig bug is fixed. - The etc_dir and bin_dir macros both have the problem that they match pathnames with /etc/, /bin/, etc in the middle of the path, as sysdig doesn't have a "begins with" comparison. Add notes for that. - Change spawn_process to spawned_process to indicate that it's for the exit side of the execve. Also use it in a few places that were looking for the same conditions without any macro. - Get rid of adduser_binaries and fold any programs not already present into shadowutils_binaries. - Add new groups sysdigcloud_binaries and sysdigcloud_binaries_parent and add them as exceptions for write_etc/write_binary_dir. - Add yum as a package management binary and add it as an exception to write_etc/write_binary_dir. - Change how db_program_spawned_process works. Since all of the useful information is on the exit side of the event, you can't really add a condition based on the process being new. Isntead, have the rule check for a non-database-related program being spawned by a database-related program. - Allow dragent to run shells. - Add sendmail, sendmail-msp as a program that attempts to setuid. - Some of the *_binaries macros that were based on dpkg -L accidentally contained directories in addition to end files. Trim those. - Add systemd-logind as a login_binary. - Add unix_chkpwd as a shadowutils_binary. - Add parentheses around any macros that group items using or. I found this necessary when the macro is used in the middle of a list of and conditions. - Break out system_binaries into a new subset user_mgmt_binaries containing login_, passwd_, and shadowutils_ binaries. That way you don't have to pull in all of system_binaries when looking for sensisitive files or user management activity. - Rename fs-bash to fbash, thinking ahead to its more likely name.

gianlucaborello · 2016-06-02T18:43:10Z

We put many of those assertions even for conditions that get actually handled within the code, just to point out that something is "likely" wrong when we run in debug. In this case, the assert might be good since -513 is weird. Are you able to replicate it live? If so, what does strace show?

mstemm · 2016-06-03T03:37:37Z

Ok that reasoning makes sense. I'll go back and see if I can reproduce why the clone result was -513.

mstemm · 2016-06-27T22:19:39Z

I've tried running the docker-compose test in a loop for 10 or so iterations with a live sysdig, and wasn't able to reproduce the problem. And you gave a good reason why we want to keep the debug asserts, so I'll close this.

Do another round of rule cleanups now that we have a larger set of positive and negative trace files to work with. Outside of this commit, there are now trace files for all the positive rules, a docker-compose startup and teardown, and some trace files from the sysdig cloud staging environment. Also add a script that runs sysdig with a filter that removes all the syscalls not handled by falco as well as a few other high-volume, low-information syscalls. This script was used to create the staging environment trace files. Notable rule changes: - The direction for write_binary_dir/write_etc needs to be exit instead of enter, as the bin_dir clause works on the file descriptor returned by the open/openat call. - Add login as a trusted binary that can read sensitive files (occurs for direct console logins). - sshd can read sensitive files well after startup, so exclude it from the set of binaries that can trigger read_sensitive_file_trusted_after_startup. - limit run_shell_untrusted to non-containers. - Disable the ssh_error_syslog rule for now. With the current restriction on system calls (no read/write/sendto/recvfrom/etc), you won't see the ssh error messages. Nevertheless, add a string to look for to indicate ssh errors and add systemd's true location for the syslog device. - Sshd attemps to setuid even when it's not running as root, so exclude it from the set of binaries to monitor for now. - Let programs that are direct decendants of systemd spawn user management tasks for now. - Temporarily disable the EACCESS rule. This rule is exposing a bug in sysdig in debug mode, draios/sysdig#598. The rule is also pretty noisy so I'll keep it disabled until the sysdig bug is fixed. - The etc_dir and bin_dir macros both have the problem that they match pathnames with /etc/, /bin/, etc in the middle of the path, as sysdig doesn't have a "begins with" comparison. Add notes for that. - Change spawn_process to spawned_process to indicate that it's for the exit side of the execve. Also use it in a few places that were looking for the same conditions without any macro. - Get rid of adduser_binaries and fold any programs not already present into shadowutils_binaries. - Add new groups sysdigcloud_binaries and sysdigcloud_binaries_parent and add them as exceptions for write_etc/write_binary_dir. - Add yum as a package management binary and add it as an exception to write_etc/write_binary_dir. - Change how db_program_spawned_process works. Since all of the useful information is on the exit side of the event, you can't really add a condition based on the process being new. Isntead, have the rule check for a non-database-related program being spawned by a database-related program. - Allow dragent to run shells. - Add sendmail, sendmail-msp as a program that attempts to setuid. - Some of the *_binaries macros that were based on dpkg -L accidentally contained directories in addition to end files. Trim those. - Add systemd-logind as a login_binary. - Add unix_chkpwd as a shadowutils_binary. - Add parentheses around any macros that group items using or. I found this necessary when the macro is used in the middle of a list of and conditions. - Break out system_binaries into a new subset user_mgmt_binaries containing login_, passwd_, and shadowutils_ binaries. That way you don't have to pull in all of system_binaries when looking for sensisitive files or user management activity. - Rename fs-bash to fbash, thinking ahead to its more likely name.

mstemm mentioned this issue May 23, 2016

Add correctness tests falcosecurity/falco#83

Merged

mstemm closed this as completed Jun 27, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sysdig assertion failure when evt.res can't be resolved #598

sysdig assertion failure when evt.res can't be resolved #598

mstemm commented May 23, 2016

gianlucaborello commented Jun 2, 2016

mstemm commented Jun 3, 2016

mstemm commented Jun 27, 2016

sysdig assertion failure when evt.res can't be resolved #598

sysdig assertion failure when evt.res can't be resolved #598

Comments

mstemm commented May 23, 2016

gianlucaborello commented Jun 2, 2016

mstemm commented Jun 3, 2016

mstemm commented Jun 27, 2016