Someone else brought that up a while ago, had started something here https://github.com/falcosecurity/falco/pull/2670/files maybe let's discuss? I can see a bit of a better cleanup even under the hood than that wip PR.

@falcosecurity/falco-maintainers

the proposed changes make sense! There is no reason to duplicate the timestamp information in the output, or at least nothing I can think about at the moment!

Hey @Issif @incertum @Andreagit97 , i would like to work on this issue. Please guide me how can I start on this. Thanks!!

/assign

allow an opt-in to remove the timestamp

👍

@h4l0gen thank you for tackling this!
As you correctly highlighted here https://kubernetes.slack.com/archives/CMWH3EH32/p1705578359716419?thread_ts=1705577834.276719&cid=CMWH3EH32 the issue is this line

there is no need to change the tostring_withformat function, probably switching the formatter from OF_NORMAL to OF_JSON should be enough

diff --git a/userspace/engine/formats.cpp b/userspace/engine/formats.cpp
index 106a58b7..c6bd07c5 100644
--- a/userspace/engine/formats.cpp
+++ b/userspace/engine/formats.cpp
@@ -43,9 +43,6 @@ std::string falco_formats::format_event(gen_event *evt, const std::string &rule,
 
        formatter = m_falco_engine->create_formatter(source, format);
 
-       // Format the original output string, regardless of output format
-       formatter->tostring_withformat(evt, line, gen_event_formatter::OF_NORMAL);
-
        if(formatter->get_output_format() == gen_event_formatter::OF_JSON)
        {
                std::string json_line;
@@ -89,6 +86,7 @@ std::string falco_formats::format_event(gen_event *evt, const std::string &rule,
                if(m_json_include_output_property)
                {
                        // This is the filled-in output line.
+                       formatter->tostring_withformat(evt, line, gen_event_formatter::OF_JSON);
                        event["output"] = line;
                }
 
@@ -127,6 +125,11 @@ std::string falco_formats::format_event(gen_event *evt, const std::string &rule,
                full_line.append("}");
                line = full_line;
        }
+       else
+       {
+               // Obtain `line` in the normal format
+               formatter->tostring_withformat(evt, line, gen_event_formatter::OF_NORMAL);
+       }
 
        return line.c_str();
 }

BTW just to answer your question on slack:

if i can found the definition of gen_event or any other function which is generating output then i can modify its definition to solve issue but I am not able to locate it. Please assist me with this, and also confirm if this approach would be effective

The function tostring_withformat is defined in libsinsp and more precisely here https://github.com/falcosecurity/libs/blob/6d57bdec7d3f0ed898c24682c7ac417a5e3d6294/userspace/libsinsp/eventformatter.cpp#L250
Just for context: libsinsp is a library that Falco uses for many things (all the host capture + filter checks + other stuff)

BTW I propose to switch from

{
    "output": "14:37:27.505989596: Warning Detected ptrace PTRACE_ATTACH attempt (proc_pcmdline=%proc.pcmdline evt_type=%evt.type user=%user.name user_uid=%user.uid user_loginuid=%user.loginuid process=%proc.name proc_exepath=%proc.exepath parent=%proc.pname command=%proc.cmdline terminal=%proc.tty exe_flags=%evt.arg.flags %container.info)",
    "priority": "WARNING",
    "rule": "PTRACE attached to process",
    "time": "2023-12-20T14:37:27.505989596Z",
    "output_fields": {
        "container.info": "container.info",
        "evt.arg.flags": "evt.arg.flags",
        "evt.type": "evt.type",
        "proc.cmdline": "proc.cmdline",
        "proc.exepath": "proc.exepath",
        "proc.name": "proc.name",
        "proc.pcmdline": "proc.pcmdline",
        "proc.pname": "proc.pname",
        "proc.tty": "proc.tty",
        "user.loginuid": "user.loginuid",
        "user.name": "user.name",
        "user.uid": "user.uid"
    },
    "hostname": "host-7.local",
    "source": "syscalls",
    "tags": [
        "maturity_stable",
        "host",
        "container",
        "process",
        "mitre_privilege_escalation",
        "T1055.008"
    ]
}

to

{
    "output": "(proc_pcmdline=%proc.pcmdline evt_type=%evt.type user=%user.name user_uid=%user.uid user_loginuid=%user.loginuid process=%proc.name proc_exepath=%proc.exepath parent=%proc.pname command=%proc.cmdline terminal=%proc.tty exe_flags=%evt.arg.flags %container.info)",
    "priority": "WARNING",
    "rule": "PTRACE attached to process",
    "time": "2023-12-20T14:37:27.505989596Z",
    "output_fields": {
        "container.info": "container.info",
        "evt.arg.flags": "evt.arg.flags",
        "evt.type": "evt.type",
        "proc.cmdline": "proc.cmdline",
        "proc.exepath": "proc.exepath",
        "proc.name": "proc.name",
        "proc.pcmdline": "proc.pcmdline",
        "proc.pname": "proc.pname",
        "proc.tty": "proc.tty",
        "user.loginuid": "user.loginuid",
        "user.name": "user.name",
        "user.uid": "user.uid"
    },
    "hostname": "host-7.local",
    "source": "syscalls",
    "tags": [
        "maturity_stable",
        "host",
        "container",
        "process",
        "mitre_privilege_escalation",
        "T1055.008"
    ]
}

@Issif @falcosecurity/falco-maintainers
So removing from output the 3 fields: time, priority, rule -> remove (14:37:27.505989596: Warning Detected ptrace PTRACE_ATTACH attempt)

If we agree on this the proposed patch #2985 (comment) should be enough

It's good to me.

FYI, our example is not 100% accurate, the rule is not really part of the output for that specific rule, it's just a part of the configured output:

Detected ptrace PTRACE_ATTACH attempt (proc_pcmdline=%proc.pcmdline evt_type=%evt.type user=%user.name user_uid=%user.uid user_loginuid=%user.loginuid process=%proc.name proc_exepath=%proc.exepath parent=%proc.pname command=%proc.cmdline terminal=%proc.tty %container.info)

See: https://github.com/falcosecurity/rules/blob/main/rules/falco_rules.yaml#L1105,L1117

Oh you are right rule is PTRACE attached to process while the output is Detected ptrace PTRACE_ATTACH attempt! BTW yes i think that having the rule tag in the json object should be enough even if it could be different from the rule description in the output

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

/remove-lifecycle stale

Any updates on this? Is there any blocker? 🤔

The blocker is this #3037 (comment), we need to decide what to do:

• we can change the output of formatter->tostring_withformat(evt, line, sinsp_evt_formatter::OF_NORMAL); directly in sinsp, but not sure this is what we want since Falco is not the unique consumer of this method (@jasondellaluce)
• We can remove from the output the 3 fields: time, priority, rule (-> remove "14:37:27.505989596: Warning Detected ptrace PTRACE_ATTACH attempt") manually in Falco after the call to formatter->tostring_withformat(evt, line, sinsp_evt_formatter::OF_NORMAL);

I vote for option 2. This was my initial proposal after all. Correct me if I'm wrong, by the rule is not contained in the output, just the timestamp and the priority.

Yep we don't have the exact content of the rule tag but we have something very similar:

    "output": "14:37:27.505989596: Warning Detected ptrace PTRACE_ATTACH attempt ...",
    "rule": "PTRACE attached to process",

So Detected ptrace PTRACE_ATTACH attempt vs PTRACE attached to process.
We can also keep Detected ptrace PTRACE_ATTACH attempt in the output even if it's more or less a duplicate of the rule tag

This is a particular case, the output is set to display that message that's all. See https://github.com/falcosecurity/rules/blob/e65f2518b06b6e439b49451a0738115f74d224e0/rules/falco_rules.yaml#L1099,L1111

Hi @Andreagit97, @Issif, and @leogr. Since we've encountered such a significant change in Falco, I'll wait for your decision on this PR before making any further changes. While I don't have a strong opinion on this at the moment, though as @Issif mentioned, the second option looks good to me for now

I still have to double-check, but I believe the 2nd option is ok.

cc @falcosecurity/falco-maintainers wdyt?