new(driver): resolve executable path symlink #1300

Andreagit97 · 2023-08-17T12:32:33Z

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libsinsp

/area tests

Does this PR require a change in the driver versions?

/version driver-SCHEMA-version-minor

What this PR does / why we need it:

This PR allows us to resolve executable file paths directly kernel side. We add a new parameter in PPME_SYSCALL_EXECVE_19_X and PPME_SYSCALL_EXECVEAT_X events called trusted_exepath, which contains the fully resolved executable path.
In the kernel module implementation, we can use the d_path helper so the resolution is quite simple, while in BPF drivers we need to implement this helper by hand :/ More in detail this brings to the table some limitations:

the number of path components is limited to MAX_NUM_COMPONENTS.
we cannot use locks so we can face race conditions during the path reconstruction.
reconstructed path could be slightly different from the one returned by d_path. See pseudo_filesystem prefixes or the " (deleted)" suffix in the execveX_success_memfd test

Please note: I used the same new bpf utility bpf_d_path_approx also in open_by_handle_at syscall and I removed the previous one (bpf_get_path)which was partial respect to the new one. Now open_by_handle_atX_success_mp test is passing on all drivers!

I've added the support to write this new field trusted_exepath on a scap_file.

I've added some new filterchecks:

proc.trusted_exepath
proc.is_exe_symlink
proc.trusted_pexepath
proc.trusted_aexepath

Which issue(s) this PR fixes:

Fixes #1111

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new(driver): resolve executable path symlink

github-actions · 2023-08-17T12:33:36Z

Please double check driver/API_VERSION file. See versioning.

/hold

Andreagit97 · 2023-08-17T12:33:27Z

driver/bpf/filler_helpers.h

+#define MAX_NUM_COMPONENTS 48
+#else
+#define MAX_NUM_COMPONENTS 24


These limitations shouldn't be too restrictive

driver/bpf/fillers.h

Andreagit97 · 2023-08-17T12:34:38Z

driver/bpf/fillers.h

+		struct file *f = bpf_fget(retval);
+		if(f != NULL)
+		{
+			char* filepath = bpf_d_path_approx(data, &(f->f_path));


since we are here, use the new helper to have a better detection, see the corresponding test that now is passing!

driver/ppm_fillers.c

Andreagit97 · 2023-08-17T12:36:54Z

test/drivers/test_suites/generic_tracepoints_suite/sched_process_exec.cpp

+	if(evt_test->is_kmod_engine())
+	{
+		evt_test->assert_charbuf_param(28, "/memfd:malware (deleted)");
+	}
+	else
+	{
+		/* In BPF drivers we have no the correct result but we can reconstruct part of it */
+		evt_test->assert_charbuf_param(28, "memfd:malware");
+	}


Unfortunately, the final outputs are different but we cannot handle this complexity in the bpf drivers, btw the result seems acceptable

Mentioned it before, we aren't very worried about this edge case for threat detection.

Andreagit97 · 2023-08-17T12:37:34Z

test/drivers/test_suites/syscall_exit_suite/open_by_handle_at_x.cpp

-	if(evt_test->is_bpf_engine())
-	{
-		GTEST_SKIP() << "[OPEN_BY_HANDLE_AT_X]: old BPF probe is not able to collect the mount path" << std::endl;
-	}


we have improved the detection!

Andreagit97 · 2023-08-17T12:38:00Z

userspace/libscap/engine/savefile/scap_savefile.c

@@ -708,6 +709,32 @@ static int32_t scap_read_proclist(scap_reader_t* r, uint32_t block_length, uint3
 			tinfo.exe_from_memfd = (exe_from_memfd != 0);
 		}

+		// trusted_exepath
+		if(sub_len && (subreadsize + sizeof(uint16_t)) <= sub_len)


Please double-check @FedeDP

One day we may actually have tests for scap files .... [hint] :)

Andreagit97 · 2023-08-17T12:40:13Z

userspace/libsinsp/filterchecks.cpp

@@ -2382,6 +2385,7 @@ const filtercheck_field_info sinsp_filter_check_thread_fields[] =
 	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_writable", "Process Executable Is Writable", "'true' if this process' executable file is writable by the same user that spawned the process."},
 	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_upper_layer", "Process Executable Is In Upper Layer", "'true' if this process' executable file is in upper layer in overlayfs. This field value can only be trusted if the underlying kernel version is greater or equal than 3.18.0, since overlayfs was introduced at that time."},
 	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_from_memfd", "Process Executable Is Stored In Memfd", "'true' if the executable file of the current process is an anonymous file created using memfd_create() and is being executed by referencing its file descriptor (fd). This type of file exists only in memory and not on disk. Relevant to detect malicious in-memory code injection. Requires kernel version greater or equal to 3.17.0."},
+	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_symlink", "Executable file is a symlink", "'true' if the executable file of the current process is a symlink. This could generate false positive for 2 reasons. (1) our userspace exepath reconstruction is best effort so maybe we have a slightly different exepath from the one obtained from the kernel, but we don't have a symlink. (2) eBPF technology has some complexity limits so mainly in older kernels (< 5.2) we could obtain a partial exepath, but even if it differs from the userspace one, we don't necessarily have a symlink."},


not sure we can do better than this :/ the only valid idea I had was to compare the "userspace computed exepath"(exepath) with the one obtained by the kernel (trusted_exepath)... open to suggestions :)

This is ok, thanks for providing all these details, adopters will appreciate it!

Andreagit97 · 2023-08-17T15:38:05Z

Kernel testing is green :) https://github.com/falcosecurity/libs/actions/runs/5892819702 see matrix_x86 and matrix_arm64 artifacts

incertum

Thank you @Andreagit97 excellent work! As you see mainly only concerned about the UX. Implementation looks good and thanks for improving the tests as well!

driver/bpf/fillers.h

incertum · 2023-08-17T22:34:09Z

driver/modern_bpf/helpers/store/auxmap_store_params.h

+ * This brings some limitations:
+ * 1. the number of path components is limited to MAX_NUM_COMPONENTS
+ * 2. we cannot use locks so we can face race conditions during the path reconstruction.
+ * 3. reconstructed path could be slightly different from the one returned by `d_path`.


re 3 we probably aren't worried about it for threat detection use cases as exepath specifically is super critical when talking about actual files on disk.

driver/modern_bpf/helpers/store/auxmap_store_params.h

incertum · 2023-08-17T22:40:59Z

test/drivers/test_suites/generic_tracepoints_suite/sched_process_exec.cpp

+	if(evt_test->is_kmod_engine())
+	{
+		evt_test->assert_charbuf_param(28, "/memfd:malware (deleted)");
+	}
+	else
+	{
+		/* In BPF drivers we have no the correct result but we can reconstruct part of it */
+		evt_test->assert_charbuf_param(28, "memfd:malware");
+	}


Mentioned it before, we aren't very worried about this edge case for threat detection.

incertum · 2023-08-17T22:42:30Z

userspace/libscap/engine/savefile/scap_savefile.c

@@ -708,6 +709,32 @@ static int32_t scap_read_proclist(scap_reader_t* r, uint32_t block_length, uint3
 			tinfo.exe_from_memfd = (exe_from_memfd != 0);
 		}

+		// trusted_exepath
+		if(sub_len && (subreadsize + sizeof(uint16_t)) <= sub_len)


One day we may actually have tests for scap files .... [hint] :)

incertum · 2023-08-17T22:43:34Z

userspace/libsinsp/filterchecks.cpp

@@ -2382,6 +2385,7 @@ const filtercheck_field_info sinsp_filter_check_thread_fields[] =
 	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_writable", "Process Executable Is Writable", "'true' if this process' executable file is writable by the same user that spawned the process."},
 	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_upper_layer", "Process Executable Is In Upper Layer", "'true' if this process' executable file is in upper layer in overlayfs. This field value can only be trusted if the underlying kernel version is greater or equal than 3.18.0, since overlayfs was introduced at that time."},
 	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_from_memfd", "Process Executable Is Stored In Memfd", "'true' if the executable file of the current process is an anonymous file created using memfd_create() and is being executed by referencing its file descriptor (fd). This type of file exists only in memory and not on disk. Relevant to detect malicious in-memory code injection. Requires kernel version greater or equal to 3.17.0."},
+	{PT_BOOL, EPF_NONE, PF_NA, "proc.is_exe_symlink", "Executable file is a symlink", "'true' if the executable file of the current process is a symlink. This could generate false positive for 2 reasons. (1) our userspace exepath reconstruction is best effort so maybe we have a slightly different exepath from the one obtained from the kernel, but we don't have a symlink. (2) eBPF technology has some complexity limits so mainly in older kernels (< 5.2) we could obtain a partial exepath, but even if it differs from the userspace one, we don't necessarily have a symlink."},


This is ok, thanks for providing all these details, adopters will appreciate it!

incertum · 2023-08-17T23:14:07Z

userspace/libsinsp/filterchecks.cpp

@@ -2348,6 +2348,9 @@ const filtercheck_field_info sinsp_filter_check_thread_fields[] =
 	{PT_CHARBUF, EPF_NONE, PF_NA, "proc.exepath", "Process Executable Path", "The full executable path of the process (truncated after 1024 bytes). This field is collected only if the executable lives on the filesystem. This field is collected from the syscalls args or, as a fallback, extracted resolving the path of /proc/<pid>/exe."},
 	{PT_CHARBUF, EPF_NONE, PF_NA, "proc.pexepath", "Parent Process Executable Path", "The proc.exepath (full executable path) of the parent process."},
 	{PT_CHARBUF, EPF_NONE, PF_NA, "proc.aexepath", "Ancestor Executable Path", "The proc.exepath (full executable path) for a specific process ancestor. You can access different levels of ancestors by using indices. For example, proc.aexepath[1] retrieves the proc.exepath of the parent process, proc.aexepath[2] retrieves the proc.exepath of the grandparent process, and so on. The current process's proc.exepath line can be obtained using proc.aexepath[0]. When used without any arguments, proc.aexepath is applicable only in filters and matches any of the process ancestors. For instance, you can use `proc.aexepath endswith java` to match any process ancestor whose path ends with the term `java`."},
+	{PT_FSPATH, EPF_NONE, PF_NA, "proc.trusted_exepath", "Process Executable Path resolved by the kernel", "The full executable path of the process. This field is collected directly from the kernel while 'proc.exepath' is reconstructed in userspace."},


Let's start another UX discussion:

@Andreagit97 I am now wearing the adopter, Data Science, Big Data and Operations hats:

The current changes of exposing the trustworthy exepath via a new field may come at on exorbitant cost, so high adopters may even choose to just ignore it and not benefit from the changes. Internally the new name trusted_exepath is perfect though.

This is because the existing exposed field proc.exepath has been around for such a long time, folks have defined their downstream ETL schemas based on it or even defined their incident response runbooks based on the existing field naming convention. It could also be that most adopters today fully trust the existing proc.exepath and thought it was always the true / trustworthy executable path and therefore wouldn't even think about checking the updated proc fields in this regard. In addition, our rules style guide requires proc.exepath as critical field. This means we need to change the style guide and all upstream rules output fields, because the trusted path is the one that should be the default one you look at.

Therefore, I would propose to assign the trusted_exepath to the existing proc.exepath. This will improve the overall data quality as well, because today if we fail to log the cwd the proc.exepath was null anyways. A current flaw on top of the symlink issues. Furthermore, the true executable path is the one that matters the most for threat detection. In summary, this approach would allow for a more seamless transition from my perspective.

If we were to agree on this, we likely have a few options for the "other" new fields. I admit it will be tricky to find a good name, maybe proc.exepath_symlink and only populate if proc.is_exe_symlink is true, knowing about all of the limitations of course and restating them?

What do you all think @falcosecurity/core-maintainers ?

If we have diverging opinions I would kindly ask to involve several adopters and ask for their opinion as well, because the adopters are the ones who use Falco in production.

Again my feedback as being an adopter: It would cost me significant engineering time to cutover to a new field in this context, not to mention the educational outreach I would need to do, e.g. talk to security analysts who triage alerts, updating documentation explaining why now a new field name and how come etc. Something of course I would love to avoid.

Therefore, I would propose to assign the trusted_exepath to the existing proc.exepath.

I'm fine with this, if we choose this way we will have a smooth transition for adopters 👍 Tagging our rule expert @darryk10 just to have a second opinion on this

If we were to agree on this, we likely have a few options for the "other" new fields. I admit it will be tricky to find a good name, maybe proc.exepath_symlink and only populate if proc.is_exe_symlink is true, knowing about all of the limitations of course and restating them?

Yeah it could be an idea, I like it, the actual exepath will become proc.exepath_symlink because it reports the symlink path if we detect a symlink, of course, it could be a false positive as we said but this is in someway acceptable

I think assigning trusted_exepath with proc.exepath would be a good trade-off. This might break some exceptions or conditions for existent users if applied on symlink but I agree it improves the data quality.

It could also be that most adopters today fully trust the existing proc.exepath and thought it was always the true / trustworthy executable path and therefore wouldn't even think about checking the updated proc fields in this regard

I strongly agree with this, and that's why I always thought to overwrite the value of proc.exepath with the correct, resolved one once we had this implemented in kernel, since as @incertum was saying, people expect it to be true and trustworthy. It makes no sense for me to keep a field that it's not reliable. Another advantage of this is that we can totally delete the logic for exepath reconstruction with cwd in userspace, since there's simply no need for it anymore.
So with this in mind, we only have a problem: checking how proc.exepath is used in the rules condition. If for instance people have some rules that are meant to trigger on exepath and that is a default symlink the OS is creating for us (think about insmod, it is always a symlink), we are going to break those rules. We can maybe plan how to spread this info very well so that we transition in the best possible way, but seems non-trivial. To force this, we probably need to work on a semantic versioning for engine version and bumping the major so that we can force user to understand why the old rules are not accepted anymore..
I took a look at our rules and fortunately, we are not using proc.exepath in any condition, but we can't predict how it was used by others.

There's a point I don't understand though: what's the value in detection knowing that proc.is_exe_symlink is true? how can we use it in rules? I think once we have the full, resolved exepath, we just need that and nothing more, but I'm open to heat other opinions

There's a point I don't understand though: what's the value in detection knowing that proc.is_exe_symlink is true? how can we use it in rules? I think once we have the full, resolved exepath, we just need that and nothing more, but I'm open to heat other opinions

That's a good point! the idea of saying to the user "ei this is a symlink" sounds good but keeping into account that we could have false positive and that probably its security value is not so high we can also think to remove it, I put it without thinking too much about its security value if we are able to cleanup some useless filterchecks even better!

@incertum please don't misunderstand me 😅 I had shared my perspective to ensure we weren't making an oversight. But, thanks to your comment, I've changed my mind. Still, I provided all the details below, hoping they can be helpful to the design of this feature.

TL;DR

I genuinely believed that switching the proc.exepath to the new value was incorrect, as it represents something entirely different 👼 However, the above comment highlighted a detail I had entirely overlooked, which has significantly changed my viewpoint. I'm now on board with replacing the current proc.exepath behavior.

Full details and replies

Adopters will still face challenges anyway. The data from the new implementation has a different meaning (think the insmod vs. kmod case). Overwriting proc.exepath would immediately invalidate all existing rules around the world and make things more confusing in the long run.

Is this assessment speculative or rooted in data/feedback? Particularly, does the assertion of increased confusion stem from diverse input from adopters, or from personal input?

My assessment was based on these assumptions:

A file's full path (i.e., the absolute path) doesn't necessarily match the real path (the absolute path after resolving all symlinks).

The current description of proc.exepath represents its semantics.

Altering the behavior of proc.exepath might break existing rules (consider proc.exepath = /usr/bin/insmod).

Although these assumptions remain valid, I overlooked a crucial detail:

libs/userspace/libsinsp/filterchecks.cpp

Line 2348 in ae071c0

{PT_CHARBUF, EPF_NONE, PF_NA, "proc.exepath", "Process Executable Path", "The full executable path of the process (truncated after 1024 bytes). This field is collected only if the executable lives on the filesystem. This field is collected from the syscalls args or, as a fallback, extracted resolving the path of /proc/<pid>/exe."},

as a fallback, extracted by resolving the path of /proc//exe

The old implementation fetches proc.exepath sometimes from /proc/../exe (which provides the real path of the executable) while, at other times, it derives data from syscall arguments and uses the current working directory to reconstruct the path. This could result in either the binary's absolute path or the symlink.

Consequently:

All scenarios I thought might be compromised are already flawed. 🤦‍♂️

We don't have a reliable and efficient method to obtain the absolute path of the executable symlink.

This means searching for both /usr/bin/kmod AND /usr/bin/insmod in the same condition is unreliable (contrary to my initial understanding).

This let me completely change my mind. You can find my latest proposal below.

For instance, doesn't the Linux kernel also adapt to revised interpretations as superior iterations emerge? Aren't such enhancements inherent in all software products that steadily refine and improve? Do our documented SLOs or SLAs (in case we even have them?) prohibit semantic modifications when they notably benefit the project?

To clarify: Nothing prevents us from changing semantics if it enhances the project. I merely expressed concerns about the nature of the proposed change, fearing it might not be beneficial. Now, I'm happy to admit I was wrong 😅

Take, for instance, this comment from an adopter: #705 (comment), highlighting that the Linux kernel's ground truth with readlink consistently provides the resolved on-disk path.

I always agreed with the adopter's viewpoint regarding what to accomplish. Nevertheless, opinions diverged on how to achieve this. I think that distinguishing the what from the how is critical when evaluating feedback. Do you agree?

Finally, it seems that the majority of both upstream and probably custom rules rely on proc.name. For instance, until the recent release, we hadn't even included support for recursively tracing the parent process lineage wrt exepath. To validate my assumption, it would be beneficial to have confirmation from other adopters.

👍

The examples you've provided would be ideal to include in the next release notes.

Given the significant changes in the next release, offering more than release notes might be worthwhile. We should consider creating a dedicated page on falco.org that documents these changes' implications and suggests a potential migration path. wdyt?
Anyway, this should be a separate discussion, we should open a different GitHub issue for that

As I originally proposed, it would be a good idea to include more adopters who use Falco in production. This would help us gather useful information and a diverse range of opinions to guide our discussion.

To make a decision, we could follow a process similar to how we decide on maintenance matters. We could reach out to at least 10 (or even more) adopters and ask for their input. Then, we would go with the option that most of them agree on.

While I agree, I also feel that preliminary discussions (like this one) remain essential. This allows us to furnish all necessary contextual information to the adopters, ensuring we solicit their input appropriately.

Latest proposal

1. Resolve executable path symlink for the current fields

I'm now in favor of applying this for:
- proc.exepath
- proc.pexepath
- proc.aexepath

Additionally, it's essential to revise the descriptions of these fields.

It might also be beneficial to ensure the resolution logic aligns with what's described here: https://man7.org/linux/man-pages/man7/path_resolution.7.html

I would not include other changes in this PR.

New possible symlink related fields

Regarding new possible fields as indicated by Andrea:

proc.exepath_symlink

proc.pexepath_symlink

proc.aexepath_symlink

proc.is_exe_symlink

Considering the prevailing uncertainties, I suggest initiating a separate discussion to address any concerns, possibly involving adopter feedback as well.

Quoting Melissa

Yes from my perspective either way fine, adding them or not. We could also choose to not add them now and consider adding the symlink related filterchecks later on? Most important would be to not block this PR.

I propose to exclude these fields from the current PR and shift the discussion to an independent thread.

Thank you @leogr for your additional thoughts and comments, very much appreciated 🙏, that's why we have discussions 🙃 .

👍 re final decision for now you posted above.

By the way this discussion sparked an idea. Perhaps we could add a dedicated page on the website to set expectations for the nature of changes between releases and also expose a more concise overview checklist for each release that would complement the traditional release note. I'll see if I have some time today to get a PR up and we would shift the discussion to that PR. Because we will come back to this very same discussion once we address the fd.name and we have many related items, where a change would make Falco better and more robust, but at the same price, that is, subtle changes in semantics or behavior.

Uhm the plan sounds good to me! Any other ideas or we can proceed?

i've updated the PR :)

incertum · 2023-08-17T23:20:25Z

@alok-aggarwal it took us a long time, but the PR you likely are interested in is this one :)
Old reference: #705

driver/bpf/filler_helpers.h

Andreagit97 · 2023-08-18T10:16:29Z

REMINDER FOR MYSELF
we need to remember that we can replace the exepath only in live captures otherwise we will break all old scap-files

leogr · 2023-08-23T10:35:47Z

/milestone 0.13.0

leogr · 2023-08-24T15:22:22Z

REMINDER FOR MYSELF we need to remember that we can replace the exepath only in live captures otherwise we will break all old scap-files

~~This is another good reason not to replace existing paths~~
Update: I've changed my mind

Signed-off-by: Andrea Terzolo <[email protected]>

* proc.trusted_exepath * proc.is_exe_symlink Signed-off-by: Andrea Terzolo <[email protected]>

Signed-off-by: Andrea Terzolo <[email protected]>

falcosecurity@f3c311c Signed-off-by: Andrea Terzolo <[email protected]>

Signed-off-by: Andrea Terzolo <[email protected]>

Signed-off-by: Andrea Terzolo <[email protected]> Co-authored-by: Lorenzo Susini <[email protected]>

since we don't have anymore a new sinsp field `trusted_exepath` we can remove the support for write/read it from a scap-file Signed-off-by: Andrea Terzolo <[email protected]>

Signed-off-by: Andrea Terzolo <[email protected]>

Andreagit97 · 2023-08-30T12:12:13Z

driver/ppm_fillers.c

+		if(trusted_exepath != NULL)
+		{
+			char deleted_suffix[] = " (deleted)";
+			int diff_len = strlen(trusted_exepath) - strlen(deleted_suffix);
+			if(diff_len > 0 &&
+				(strncmp(&trusted_exepath[diff_len], deleted_suffix, sizeof(deleted_suffix)) == 0))
+			{					
+				trusted_exepath[diff_len] = '\0';
+			}
+		}


@FedeDP please take a look if here something is wrong we are dead 💀

Algorithm looks good to me!

FedeDP · 2023-08-30T13:11:55Z

driver/ppm_fillers.c

+		{
+			char deleted_suffix[] = " (deleted)";
+			int diff_len = strlen(trusted_exepath) - strlen(deleted_suffix);
+			if(diff_len > 0 &&


I assume we cannot receive a string (deleted) here? Otherwise we would need to use
`if (diff_len >= 0 &&

if we receive a string that is exactly (deleted) I think that we should do nothing, so diff_len > 0 should be ok 🤔

FedeDP · 2023-08-30T13:13:18Z

driver/ppm_fillers.c

+		if(trusted_exepath != NULL)
+		{
+			char deleted_suffix[] = " (deleted)";
+			int diff_len = strlen(trusted_exepath) - strlen(deleted_suffix);
+			if(diff_len > 0 &&
+				(strncmp(&trusted_exepath[diff_len], deleted_suffix, sizeof(deleted_suffix)) == 0))
+			{					
+				trusted_exepath[diff_len] = '\0';
+			}
+		}


Algorithm looks good to me!

FedeDP

/approve

poiana · 2023-08-31T07:56:36Z

LGTM label has been added.

Git tree hash: c820da341f9b967741a806c0cc28be765288426d

jasondellaluce · 2023-08-31T09:16:06Z

Great work Andre! +1 for this approach.

leogr

🥳

poiana · 2023-08-31T11:14:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, leogr

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,leogr]

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

Andreagit97 added this to the next-driver milestone Aug 17, 2023

poiana added release-note kind/feature New feature or request dco-signoff: yes area/driver-kmod area/driver-bpf approved area/driver-modern-bpf area/libsinsp area/tests labels Aug 17, 2023

poiana requested review from hbrueckner and leogr August 17, 2023 12:32

poiana added the size/XXL label Aug 17, 2023

poiana added the do-not-merge/hold label Aug 17, 2023

Andreagit97 commented Aug 17, 2023

View reviewed changes

incertum reviewed Aug 17, 2023

View reviewed changes

driver/bpf/filler_helpers.h Show resolved Hide resolved

leogr mentioned this pull request Aug 21, 2023

Falco 0.36. Rules Messaging falcosecurity/rules#132

Closed

Andreagit97 mentioned this pull request Aug 22, 2023

Discuss about fundamental solution of detecting symlink file based bypass method falcosecurity/falco#2203

Open

poiana modified the milestones: next-driver, 0.13.0 Aug 23, 2023

Andreagit97 added 3 commits August 30, 2023 09:19

new: support executable file path in old bpf probe

935694a

Signed-off-by: Andrea Terzolo <[email protected]>

new: support executable file path in kernel module

f234f45

Signed-off-by: Andrea Terzolo <[email protected]>

new: support executable file path in modern bpf

dde3669

Signed-off-by: Andrea Terzolo <[email protected]>

Andreagit97 and others added 11 commits August 30, 2023 09:19

tests: add some tests to check executable path

1ed7346

Signed-off-by: Andrea Terzolo <[email protected]>

new(sinsp): support a new field m_trusted_exepath

aa1758e

Signed-off-by: Andrea Terzolo <[email protected]>

new: add some new filterchecks

c5f4a2d

* proc.trusted_exepath * proc.is_exe_symlink Signed-off-by: Andrea Terzolo <[email protected]>

new: support new field m_trusted_exepath in scap-files

24b985a

Signed-off-by: Andrea Terzolo <[email protected]>

new: introduce trusted_pexepath and trusted_aexepath fields

c090e34

Signed-off-by: Andrea Terzolo <[email protected]>

test: adapt deafult tree test to this commit f3c311c

9a7c59a

falcosecurity@f3c311c Signed-off-by: Andrea Terzolo <[email protected]>

cleanup(sinsp): remove a check become useless after this commit f3c311c

c5fe7c2

Signed-off-by: Andrea Terzolo <[email protected]>

doc: add a reference to the symlink issue

6a257ba

Signed-off-by: Andrea Terzolo <[email protected]> Co-authored-by: Lorenzo Susini <[email protected]>

cleanup: remove scap-file support

1a61cba

since we don't have anymore a new sinsp field `trusted_exepath` we can remove the support for write/read it from a scap-file Signed-off-by: Andrea Terzolo <[email protected]>

update: use exepath field to contain the new trusted_exepath

9c87daf

Signed-off-by: Andrea Terzolo <[email protected]>

chore: bump SCHEMA_VERSION

d48fa38

Signed-off-by: Andrea Terzolo <[email protected]>

Andreagit97 force-pushed the symlink_new branch from 7c7dc17 to d48fa38 Compare August 30, 2023 09:58

update: remove the (deleted) suffix in kmod

59134a9

Signed-off-by: Andrea Terzolo <[email protected]>

Andreagit97 commented Aug 30, 2023

View reviewed changes

FedeDP reviewed Aug 30, 2023

View reviewed changes

FedeDP approved these changes Aug 31, 2023

View reviewed changes

poiana assigned FedeDP Aug 31, 2023

poiana added the lgtm label Aug 31, 2023

leogr approved these changes Aug 31, 2023

View reviewed changes

poiana assigned leogr Aug 31, 2023

Andreagit97 removed the do-not-merge/hold label Aug 31, 2023

poiana merged commit ca3271e into falcosecurity:master Aug 31, 2023
30 checks passed

Molter73 mentioned this pull request Oct 25, 2023

Facing issue while trying to build ebpf on power Rhel 8 stackrox/collector#1391

Open

new(driver): resolve executable path symlink #1300

new(driver): resolve executable path symlink #1300

Conversation

Andreagit97 commented Aug 17, 2023

github-actions bot commented Aug 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Andreagit97 commented Aug 17, 2023

incertum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loresuso Aug 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TL;DR

Full details and replies

Latest proposal

1. Resolve executable path symlink for the current fields

New possible symlink related fields

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

incertum commented Aug 17, 2023

Andreagit97 commented Aug 18, 2023

leogr commented Aug 23, 2023

leogr commented Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Aug 31, 2023

jasondellaluce commented Aug 31, 2023

leogr left a comment

Choose a reason for hiding this comment

poiana commented Aug 31, 2023

loresuso Aug 18, 2023 •

edited

Loading

leogr commented Aug 24, 2023 •

edited

Loading