Skip to content

Commit

Permalink
Dont sanitize strings filters (#625)
Browse files Browse the repository at this point in the history 
* Switch back to erase/remove_if for sanitization.

It turns out that with -O3 (release flags), using erase/remove_if with
an operator struct is slightly faster than the inline version that was
here, as the compiler can inline the operators. It isn't faster with -O2
and is much slower without optimization.

Optimize for the release build by adding the operators back.

* Whitespace diffs.

Making whitespace changes in separate commit.

* Make sanitize_string cfgable and off for filters.

Modify sinsp_filter_check::extract to take an optional argument
sanitize_strings, defaulting to true. It controls whether the
filtercheck should sanitize any string arguments when extracting the
value from the event.

The only place where it is set to false is in
sinsp_filter_check::compare(). This means that when comparing events
against a filter expression, any string values are not sanitized. They
will still be sanitized when the events are displayed.

This significantly speeds up users like falco that have complicated
filter expressions that run against lots of events.
  • Loading branch information
@mstemm
mstemm committed Jul 18, 2016
1 parent 66f7b1b commit abf11d8
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 86 deletions.
3 changes: 2 additions & 1 deletion userspace/libsinsp/filter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,8 @@ bool sinsp_filter_check::flt_compare(cmpop op, ppm_param_type type, void* operan
bool sinsp_filter_check::compare(sinsp_evt *evt)
{
uint32_t evt_val_len=0;
uint8_t* extracted_val = extract(evt, &evt_val_len);
bool sanitize_strings = false;
uint8_t* extracted_val = extract(evt, &evt_val_len, sanitize_strings);

if(extracted_val == NULL)
{
Expand Down
97 changes: 60 additions & 37 deletions userspace/libsinsp/filterchecks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ sinsp_filter_check* sinsp_filter_check_fd::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_fd();
}

bool sinsp_filter_check_fd::extract_fdname_from_creator(sinsp_evt *evt, OUT uint32_t* len)
bool sinsp_filter_check_fd::extract_fdname_from_creator(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
const char* resolved_argstr;
uint16_t etype = evt->get_type();
Expand Down Expand Up @@ -247,7 +247,10 @@ bool sinsp_filter_check_fd::extract_fdname_from_creator(sinsp_evt *evt, OUT uint
namelen);

m_tstr = fullpath;
sanitize_string(m_tstr);
if(sanitize_strings)
{
sanitize_string(m_tstr);
}

return true;
}
Expand All @@ -257,7 +260,7 @@ bool sinsp_filter_check_fd::extract_fdname_from_creator(sinsp_evt *evt, OUT uint
}
}

uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
//
// Even is there's no fd, we still try to extract a name from exit events that create
Expand All @@ -268,7 +271,7 @@ uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_
{
case TYPE_FDNAME:
{
if(extract_fdname_from_creator(evt, len) == true)
if(extract_fdname_from_creator(evt, len, sanitize_strings) == true)
{
*len = m_tstr.size();
return (uint8_t*)m_tstr.c_str();
Expand All @@ -280,7 +283,7 @@ uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_
}
case TYPE_CONTAINERNAME:
{
if(extract_fdname_from_creator(evt, len) == true)
if(extract_fdname_from_creator(evt, len, sanitize_strings) == true)
{
m_tstr = m_tinfo->m_container_id + ':' + m_tstr;
*len = m_tstr.size();
Expand All @@ -293,9 +296,12 @@ uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_
}
case TYPE_DIRECTORY:
{
if(extract_fdname_from_creator(evt, len) == true)
if(extract_fdname_from_creator(evt, len, sanitize_strings) == true)
{
sanitize_string(m_tstr);
if(sanitize_strings)
{
sanitize_string(m_tstr);
}

size_t pos = m_tstr.rfind('/');
if(pos != string::npos)
Expand All @@ -320,9 +326,12 @@ uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_
}
case TYPE_CONTAINERDIRECTORY:
{
if(extract_fdname_from_creator(evt, len) == true)
if(extract_fdname_from_creator(evt, len, sanitize_strings) == true)
{
sanitize_string(m_tstr);
if(sanitize_strings)
{
sanitize_string(m_tstr);
}

size_t pos = m_tstr.rfind('/');
if(pos != string::npos)
Expand Down Expand Up @@ -354,9 +363,12 @@ uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_
return NULL;
}

if(extract_fdname_from_creator(evt, len) == true)
if(extract_fdname_from_creator(evt, len, sanitize_strings) == true)
{
sanitize_string(m_tstr);
if(sanitize_strings)
{
sanitize_string(m_tstr);
}

size_t pos = m_tstr.rfind('/');
if(pos != string::npos)
Expand Down Expand Up @@ -428,7 +440,7 @@ uint8_t* sinsp_filter_check_fd::extract_from_null_fd(sinsp_evt *evt, OUT uint32_
}
}

uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
ASSERT(evt);

Expand All @@ -451,7 +463,7 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
case TYPE_CONTAINERNAME:
if(m_fdinfo == NULL)
{
return extract_from_null_fd(evt, len);
return extract_from_null_fd(evt, len, sanitize_strings);
}

if(evt->get_type() == PPME_SOCKET_CONNECT_X)
Expand All @@ -464,7 +476,7 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)

if(retval < 0)
{
return extract_from_null_fd(evt, len);
return extract_from_null_fd(evt, len, sanitize_strings);
}
}

Expand All @@ -478,8 +490,11 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
m_tstr = m_fdinfo->m_name;
}

sanitize_string(m_tstr);
*len = m_tstr.size();
if(sanitize_strings)
{
sanitize_string(m_tstr);
*len = m_tstr.size();
}

return (uint8_t*)m_tstr.c_str();
case TYPE_FDTYPE:
Expand All @@ -494,7 +509,7 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
{
if(m_fdinfo == NULL)
{
return extract_from_null_fd(evt, len);
return extract_from_null_fd(evt, len, sanitize_strings);
}

if(!(m_fdinfo->is_file() || m_fdinfo->is_directory()))
Expand All @@ -503,7 +518,10 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
}

m_tstr = m_fdinfo->m_name;
sanitize_string(m_tstr);
if(sanitize_strings)
{
sanitize_string(m_tstr);
}

if(m_fdinfo->is_file())
{
Expand Down Expand Up @@ -533,7 +551,7 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
{
if(m_fdinfo == NULL)
{
return extract_from_null_fd(evt, len);
return extract_from_null_fd(evt, len, sanitize_strings);
}

if(!m_fdinfo->is_file())
Expand All @@ -542,7 +560,10 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
}

m_tstr = m_fdinfo->m_name;
sanitize_string(m_tstr);
if(sanitize_strings)
{
sanitize_string(m_tstr);
}

size_t pos = m_tstr.rfind('/');
if(pos != string::npos)
Expand All @@ -563,7 +584,7 @@ uint8_t* sinsp_filter_check_fd::extract(sinsp_evt *evt, OUT uint32_t* len)
case TYPE_FDTYPECHAR:
if(m_fdinfo == NULL)
{
return extract_from_null_fd(evt, len);
return extract_from_null_fd(evt, len, sanitize_strings);
}

m_tcstr[0] = m_fdinfo->get_typechar();
Expand Down Expand Up @@ -1250,7 +1271,8 @@ bool sinsp_filter_check_fd::compare(sinsp_evt *evt)
// Standard extract-based fields
//
uint32_t len;
uint8_t* extracted_val = extract(evt, &len);
bool sanitize_strings = false;
uint8_t* extracted_val = extract(evt, &len, sanitize_strings);

if(extracted_val == NULL)
{
Expand Down Expand Up @@ -1545,7 +1567,7 @@ uint8_t* sinsp_filter_check_thread::extract_thread_cpu(sinsp_evt *evt, sinsp_thr
return NULL;
}

uint8_t* sinsp_filter_check_thread::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_thread::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
sinsp_threadinfo* tinfo = evt->get_thread_info();

Expand Down Expand Up @@ -2812,7 +2834,7 @@ uint8_t* sinsp_filter_check_event::extract_error_count(sinsp_evt *evt, OUT uint3
return NULL;
}

uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_event::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
switch(m_field_id)
{
Expand Down Expand Up @@ -3845,7 +3867,8 @@ bool sinsp_filter_check_event::compare(sinsp_evt *evt)
if(m_field_id == TYPE_ARGRAW)
{
uint32_t len;
uint8_t* extracted_val = extract(evt, &len);
bool sanitize_strings = false;
uint8_t* extracted_val = extract(evt, &len, sanitize_strings);

if(extracted_val == NULL)
{
Expand Down Expand Up @@ -3910,7 +3933,7 @@ sinsp_filter_check* sinsp_filter_check_user::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_user();
}

uint8_t* sinsp_filter_check_user::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_user::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
sinsp_threadinfo* tinfo = evt->get_thread_info();
scap_userinfo* uinfo;
Expand Down Expand Up @@ -3982,7 +4005,7 @@ sinsp_filter_check* sinsp_filter_check_group::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_group();
}

uint8_t* sinsp_filter_check_group::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_group::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
sinsp_threadinfo* tinfo = evt->get_thread_info();

Expand Down Expand Up @@ -4349,7 +4372,7 @@ uint8_t* sinsp_filter_check_tracer::extract_arg(sinsp_partial_tracer* pae)
return (uint8_t*)res;
}

uint8_t* sinsp_filter_check_tracer::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_tracer::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
sinsp_tracerparser* eparser;
sinsp_threadinfo* tinfo = evt->get_thread_info();
Expand Down Expand Up @@ -4818,7 +4841,7 @@ sinsp_filter_check* sinsp_filter_check_evtin::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_evtin();
}

uint8_t* sinsp_filter_check_evtin::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_evtin::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
return NULL;
}
Expand Down Expand Up @@ -5223,7 +5246,7 @@ int32_t rawstring_check::parse_field_name(const char* str, bool alloc_state)
return -1;
}

uint8_t* rawstring_check::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* rawstring_check::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
*len = m_text_len;
return (uint8_t*)m_text.c_str();
Expand Down Expand Up @@ -5265,7 +5288,7 @@ int32_t sinsp_filter_check_syslog::parse_field_name(const char* str, bool alloc_
return res;
}

uint8_t* sinsp_filter_check_syslog::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_syslog::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
ASSERT(m_decoder != NULL);
if(!m_decoder->is_data_valid())
Expand Down Expand Up @@ -5316,7 +5339,7 @@ sinsp_filter_check* sinsp_filter_check_container::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_container();
}

uint8_t* sinsp_filter_check_container::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_container::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
sinsp_threadinfo* tinfo = evt->get_thread_info();
if(tinfo == NULL)
Expand Down Expand Up @@ -5456,7 +5479,7 @@ int32_t sinsp_filter_check_reference::parse_field_name(const char* str, bool all
return -1;
}

uint8_t* sinsp_filter_check_reference::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_reference::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
*len = m_len;
return m_val;
Expand Down Expand Up @@ -5783,7 +5806,7 @@ sinsp_filter_check* sinsp_filter_check_utils::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_utils();
}

uint8_t* sinsp_filter_check_utils::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_utils::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
switch(m_field_id)
{
Expand Down Expand Up @@ -5824,7 +5847,7 @@ sinsp_filter_check* sinsp_filter_check_fdlist::allocate_new()
return (sinsp_filter_check*) new sinsp_filter_check_fdlist();
}

uint8_t* sinsp_filter_check_fdlist::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_fdlist::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
ASSERT(evt);
sinsp_evt_param *parinfo;
Expand Down Expand Up @@ -6199,7 +6222,7 @@ bool sinsp_filter_check_k8s::find_label(const k8s_pair_list& labels, const strin
return false;
}

uint8_t* sinsp_filter_check_k8s::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_k8s::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
if(m_inspector->m_k8s_client == NULL)
{
Expand Down Expand Up @@ -6614,7 +6637,7 @@ bool sinsp_filter_check_mesos::find_label(const mesos_pair_list& labels, const s
return false;
}

uint8_t* sinsp_filter_check_mesos::extract(sinsp_evt *evt, OUT uint32_t* len)
uint8_t* sinsp_filter_check_mesos::extract(sinsp_evt *evt, OUT uint32_t* len, bool sanitize_strings)
{
if(!m_inspector || !m_inspector->m_mesos_client)
{
Expand Down
Loading

0 comments on commit abf11d8

Please sign in to comment.