From 763469fd3ee2ee7a88a7fdb67ebbbeaa5f223cc4 Mon Sep 17 00:00:00 2001 From: Melissa Kilby Date: Sat, 24 Feb 2024 21:45:16 +0000 Subject: [PATCH] fix(cri): properly handle state assignments in new fast-track CRI container sync lookups Co-authored-by: Roberto Scolaro Signed-off-by: Melissa Kilby --- userspace/libsinsp/container_engine/cri.cpp | 4 ++++ userspace/libsinsp/parsers.cpp | 16 +++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/userspace/libsinsp/container_engine/cri.cpp b/userspace/libsinsp/container_engine/cri.cpp index 1ecc283afc8..0a80afb6e7d 100644 --- a/userspace/libsinsp/container_engine/cri.cpp +++ b/userspace/libsinsp/container_engine/cri.cpp @@ -245,6 +245,9 @@ bool cri::resolve(sinsp_threadinfo *tinfo, bool query_os_for_missing_info) container_id.c_str()); // `lookup_sync` function directly invokes the container engine specific parser `parse` done = m_async_source->lookup_sync(key, result); + // note: The container image is the most crucial field from a security incident response perspective. + // We aim to raise the bar for successful container lookups. Conversely, pod sandboxes do not include + // a container image in the API response. if(!result.m_image.empty() || result.is_pod_sandbox()) { /* @@ -267,6 +270,7 @@ bool cri::resolve(sinsp_threadinfo *tinfo, bool query_os_for_missing_info) * issues noted by adopters, such as the absence of container images in syscall events even when * disabling async lookups. */ + result.set_lookup_status(sinsp_container_lookup::state::STARTED); cache->replace_container(std::make_shared(result)); } } diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index 4170a7eda4e..bb57f194758 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -5221,11 +5221,6 @@ void sinsp_parser::parse_container_json_evt(sinsp_evt *evt) } } - if(!container_info->is_successful()) - { - SINSP_DEBUG("Filtering container event for failed lookup of %s (but calling callbacks anyway)", container_info->m_id.c_str()); - evt->set_filtered_out(true); - } evt->set_tinfo_ref(container_info->get_tinfo(m_inspector)); evt->set_tinfo(evt->get_tinfo_ref().get()); container_info->set_lookup_status(static_cast(lookup_state.asUInt())); @@ -5237,6 +5232,9 @@ void sinsp_parser::parse_container_json_evt(sinsp_evt *evt) break; default: { + // note: The container image is the most crucial field from a security incident response perspective. + // We aim to raise the bar for successful container lookups. Conversely, pod sandboxes do not include + // a container image in the API response. if (!container_info->m_image.empty() || container_info->is_pod_sandbox()) { // Marked as SUCCESSFUL if and only if the container image field present or @@ -5248,6 +5246,14 @@ void sinsp_parser::parse_container_json_evt(sinsp_evt *evt) } } } + + // note: Important, only set event filtering flag after all final state updates are done + if(!container_info->is_successful()) + { + SINSP_DEBUG("Filtering container event for failed lookup of %s (but calling callbacks anyway)", container_info->m_id.c_str()); + evt->set_filtered_out(true); + } + // `add_container` also triggers the callbacks, therefore always continue even if the // lookup_status was set to FAILED m_inspector->m_container_manager.add_container(container_info, evt->get_thread_info(true));