new(driver,sinsp): Add euid to execve/execveat exit events #391

gnosek · 2022-06-10T12:44:31Z

We can't prevent losing setuid events completely and the uid
is pretty important for some execve-related rules, so explicitly
pass the uid in execve/at exit events

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-udig

/area libscap

/area libsinsp

/area tests

/area proposals

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

poiana · 2022-09-18T21:47:14Z

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

Andreagit97 · 2022-09-19T11:32:24Z

/remove-lifecycle stale

FedeDP · 2022-12-02T13:49:03Z

/milestone 0.11.0

gnosek · 2022-12-05T19:03:14Z

Marking as wip since we need to handle modern_bpf as well

driver/bpf/fillers.h

driver/ppm_fillers.c

Andreagit97 · 2022-12-08T13:26:02Z

driver/bpf/fillers.h

@@ -2855,17 +2855,12 @@ FILLER(execve_family_flags, true)

 	/* Parameter 26: exe_file mtime (last modification time, epoch value in nanoseconds) (type: PT_ABSTIME) */
 	time = _READ(inode->i_mtime);
-	return bpf_val_to_ring_type(data, bpf_epoch_ns_from_time(time), PT_ABSTIME);
+	ret = bpf_val_to_ring_type(data, bpf_epoch_ns_from_time(time), PT_ABSTIME);


sorry my fault is res not ret

gnosek · 2023-02-07T06:44:30Z

@incertum, @Andreagit97: I rebased the PR, care to take a look?

could we double-check this and cleanup if we don't support <2.6.20?

Looks like we still have <2.6.20 checks so I'd rather remove them all in a separate PR. I don't know if we actually run on <2.6.20 any more but I'd rather not break it on purpose in one place.

Signed-off-by: Grzegorz Nosek <[email protected]>

We can't prevent losing setuid events completely and the uid is pretty important for some execve-related rules, so explicitly pass the uid in execve/at exit events Signed-off-by: Grzegorz Nosek <[email protected]> Co-authored-by: Angelo Puglisi <[email protected]> Co-authored-by: Andrea Terzolo <[email protected]>

gnosek · 2023-02-07T06:46:02Z

/hold

gnosek · 2023-02-07T06:47:11Z

Hmm just noticed that the PT_ABSTIME patch that went in in #789 was slightly different so we still have a commit here:

commit 841bd9fd4fb5850a913a38c53b5b0dd4f1e5f8e1
Author: Grzegorz Nosek <[email protected]>
Date:   Wed Dec 7 19:11:27 2022 +0100

    fix(sinsp): format PT_ABSTIME values

    Signed-off-by: Grzegorz Nosek <[email protected]>

diff --git a/userspace/libsinsp/event.cpp b/userspace/libsinsp/event.cpp
index 4dd490e92..69ed1110b 100644
--- a/userspace/libsinsp/event.cpp
+++ b/userspace/libsinsp/event.cpp
@@ -1425,7 +1425,6 @@ Json::Value sinsp_evt::get_param_as_json(uint32_t id, OUT const char** resolved_
                        break;
                }
        case PT_DYN:
-               ASSERT(false);
                snprintf(&m_paramstr_storage[0],
                         m_paramstr_storage.size(),
                         "INVALID DYNAMIC PARAMETER");

Do we want this? @Andreagit97 @incertum (probably with a better commit message if so)

incertum · 2023-02-07T07:27:46Z

@gnosek re above question we probably don't need this commit in this PR, ty.

gnosek · 2023-02-07T07:29:38Z

Thanks @incertum, dropped the patch.

/unhold

incertum

/approve

poiana · 2023-02-07T08:08:55Z

LGTM label has been added.

Git tree hash: faa1e94d78b3537617e504bce3adc17083f54aa9

FedeDP

/approve

FedeDP · 2023-02-07T08:13:01Z

/hold

Andreagit97

/approve

poiana · 2023-02-07T08:18:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, gnosek, hbrueckner, incertum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Andreagit97,FedeDP,gnosek,incertum]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Andreagit97 · 2023-02-07T08:18:35Z

/unhold

poiana added release-note-none kind/feature New feature or request dco-signoff: yes area/driver-kmod area/driver-bpf area/libsinsp size/M labels Jun 10, 2022

poiana requested review from Andreagit97 and fntlnz June 10, 2022 12:45

poiana added the approved label Jun 10, 2022

gnosek changed the title ~~Add euid to execve/execveat exit events~~ wip: Add euid to execve/execveat exit events Jun 10, 2022

poiana added the do-not-merge/work-in-progress label Jun 10, 2022

gnosek changed the title ~~wip: Add euid to execve/execveat exit events~~ Add euid to execve/execveat exit events Jun 20, 2022

poiana removed the do-not-merge/work-in-progress label Jun 20, 2022

poiana added the lifecycle/stale label Sep 18, 2022

poiana removed the lifecycle/stale label Sep 19, 2022

poiana added this to the 0.11.0 milestone Dec 2, 2022

gnosek force-pushed the execve-uid branch from f308559 to fcf4192 Compare December 5, 2022 19:02

gnosek changed the title ~~Add euid to execve/execveat exit events~~ wip: Add euid to execve/execveat exit events Dec 5, 2022

poiana added the do-not-merge/work-in-progress label Dec 5, 2022

gnosek force-pushed the execve-uid branch 4 times, most recently from 0fd5fe5 to 0f9a83d Compare December 7, 2022 19:14

Andreagit97 reviewed Dec 8, 2022

View reviewed changes

driver/bpf/fillers.h Outdated Show resolved Hide resolved

driver/bpf/fillers.h Outdated Show resolved Hide resolved

driver/bpf/fillers.h Show resolved Hide resolved

driver/ppm_fillers.c Outdated Show resolved Hide resolved

driver/ppm_fillers.c Show resolved Hide resolved

Andreagit97 reviewed Dec 8, 2022

View reviewed changes

gnosek dismissed Andreagit97’s stale review via 2d1c50c February 7, 2023 06:41

gnosek force-pushed the execve-uid branch from 2896e6a to 2d1c50c Compare February 7, 2023 06:41

poiana removed the lgtm label Feb 7, 2023

poiana requested review from Andreagit97 and hbrueckner February 7, 2023 06:42

gnosek and others added 2 commits February 7, 2023 07:45

new(tests): log the received event on tests mismatch

3e4f77f

Signed-off-by: Grzegorz Nosek <[email protected]>

poiana added the do-not-merge/hold label Feb 7, 2023

gnosek force-pushed the execve-uid branch from 2d1c50c to b6687b2 Compare February 7, 2023 07:28

poiana removed the do-not-merge/hold label Feb 7, 2023

incertum approved these changes Feb 7, 2023

View reviewed changes

poiana assigned incertum Feb 7, 2023

poiana added the lgtm label Feb 7, 2023

FedeDP approved these changes Feb 7, 2023

View reviewed changes

poiana assigned FedeDP Feb 7, 2023

poiana added the do-not-merge/hold label Feb 7, 2023

Andreagit97 approved these changes Feb 7, 2023

View reviewed changes

poiana removed the do-not-merge/hold label Feb 7, 2023

poiana merged commit dae8bc2 into falcosecurity:master Feb 7, 2023

gnosek deleted the execve-uid branch February 7, 2023 11:34

greyhame-s mentioned this pull request Feb 10, 2023

Oss merge libs 0.10.1 #874

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(driver,sinsp): Add euid to execve/execveat exit events #391

new(driver,sinsp): Add euid to execve/execveat exit events #391

gnosek commented Jun 10, 2022 •

edited

Loading

poiana commented Sep 18, 2022

Andreagit97 commented Sep 19, 2022

FedeDP commented Dec 2, 2022

gnosek commented Dec 5, 2022

Andreagit97 Dec 8, 2022

gnosek commented Feb 7, 2023

gnosek commented Feb 7, 2023

gnosek commented Feb 7, 2023

incertum commented Feb 7, 2023

gnosek commented Feb 7, 2023

incertum left a comment

poiana commented Feb 7, 2023

FedeDP left a comment

FedeDP commented Feb 7, 2023

Andreagit97 left a comment

poiana commented Feb 7, 2023

Andreagit97 commented Feb 7, 2023

new(driver,sinsp): Add euid to execve/execveat exit events #391

new(driver,sinsp): Add euid to execve/execveat exit events #391

Conversation

gnosek commented Jun 10, 2022 • edited Loading

poiana commented Sep 18, 2022

Andreagit97 commented Sep 19, 2022

FedeDP commented Dec 2, 2022

gnosek commented Dec 5, 2022

Andreagit97 Dec 8, 2022

Choose a reason for hiding this comment

gnosek commented Feb 7, 2023

gnosek commented Feb 7, 2023

gnosek commented Feb 7, 2023

incertum commented Feb 7, 2023

gnosek commented Feb 7, 2023

incertum left a comment

Choose a reason for hiding this comment

poiana commented Feb 7, 2023

FedeDP left a comment

Choose a reason for hiding this comment

FedeDP commented Feb 7, 2023

Andreagit97 left a comment

Choose a reason for hiding this comment

poiana commented Feb 7, 2023

Andreagit97 commented Feb 7, 2023

gnosek commented Jun 10, 2022 •

edited

Loading