Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mount): reintroduce root path requirement #4328

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

NDStrahilevitz
Copy link
Collaborator

@NDStrahilevitz NDStrahilevitz commented Sep 25, 2024

1. Explain what the PR does

"Replace me with make check-pr output"

2. Explain how to test it

  1. Deploy tracee to an EKS env
  2. sh into it with kubectl exec -it <pod_name> -- /bin/sh
  3. cat /proc/self/mountinfo
  4. Confirm that an appropriate cgroupfs mount exists for cpuset or cgroup2
  5. Importantly, if it is known that the deployed tracee instance has cgroupfs mounted via k8s or docker, there should be no additional mount for the relevant cgroup version (cgroup v2 may still be mounted by tracee in cgroupv1 envs)
  6. ls that cgroupfs folder and confirm that kubepods.slice exists and other pods are visible
  7. Optionally: check the debug logs at tracee's startup for info on the mount object and confirm it aligns with expected result per the environment.

3. Other comments

Resolve #4311

@NDStrahilevitz NDStrahilevitz changed the title [DRAFT] fix(mount): apply to shortest mount root [DRAFT] fix(mount): reintroduce root path requirement Sep 25, 2024
@NDStrahilevitz NDStrahilevitz changed the title [DRAFT] fix(mount): reintroduce root path requirement fix(mount): reintroduce root path requirement Oct 9, 2024
@NDStrahilevitz NDStrahilevitz added candidate/v0.22.3 Candidate to be cherry-picked or backported into v0.22.3 release milestone/v0.23.0 labels Oct 9, 2024
@NDStrahilevitz NDStrahilevitz marked this pull request as ready for review October 9, 2024 11:21
SearchForHostMountpoint now strictly requires its results to be mounted
on the "/" root folder again. This is to prevent selecting an already
OS managed mount which is non root located (for example in TAS and EKS).

Previous inode and path selection code is left to accomodate those
changes.
@yanivagman
Copy link
Collaborator

@NDStrahilevitz can you please confirm this fix was tested both on EKS and TAS environments?

@NDStrahilevitz
Copy link
Collaborator Author

Just confirmed it works in TAS as well as EKS. GTG @yanivagman

Copy link
Collaborator

@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NDStrahilevitz
Copy link
Collaborator Author

/fast-forward

Copy link

Triggered from #4328 (comment) by @​NDStrahilevitz.

Trying to fast forward main (eb04644) to fix_eks_mount (033d01f).

Target branch (main):

commit eb046449fdf135fad069ae9307482665f7dd07e8 (HEAD -> main, origin/main, origin/HEAD)
Author: Shoham Yosef Bitton <[email protected]>
Date:   Wed Oct 30 13:52:23 2024 +0200

    changed process filter to scope filters (#4371)

Pull request (fix_eks_mount):

commit 033d01f7d4102cd07617b1ad324a9737c13d632f (pull_request/fix_eks_mount)
Author: Nadav Strahilevitz <[email protected]>
Date:   Wed Sep 25 13:45:10 2024 +0000

    fix(mount): reintroduce root path requirement
    
    SearchForHostMountpoint now strictly requires its results to be mounted
    on the "/" root folder again. This is to prevent selecting an already
    OS managed mount which is non root located (for example in TAS and EKS).
    
    Previous inode and path selection code is left to accomodate those
    changes.

Can't fast forward main (eb04644) to fix_eks_mount (033d01f). main (eb04644) is not a direct ancestor of fix_eks_mount (033d01f). Branches appear to have diverged at 47451f5:

* eb046449fdf135fad069ae9307482665f7dd07e8 changed process filter to scope filters (#4371)
* 3f60a6fae7f0bcca2ecc5e36f3366839dbe8f6e7 fix(time): ticks is uint64
* 165759623e9417c5eda089941485ed61f5f01b07 chore,fix(proc): improve /proc stat parsing
* c93e3295be5edafc87989860ec1cd241fed2c07f chore(proc): add benchmark for /proc stat parsing
* 3bff1c9d74755bb9c7dcba5f96dd95e5a19f7929 chore(proc): improve /proc status parsing
* 89e458e7c6ce8d8a77d74e7d3bdb5e108c01b481 chore(proc): add benchmark for /proc status parsing
* f53003797eeeb2966751df3305ae586718b9011a fix(events): check if init finished in hidden kernel module (#4367)
* a23b9f634b117b8ec82def7257c641c0a3665ac5 feat(events): change log level in hooked_syscall (#4366)
* ab6344fe82ce2d0f631ccea606f851864f09dc31 feat(ebpf): restrict set_fs_pwd to (f)chdir syscall (#4359)
* 2099f423d29fd1a3afd1b69d37b973bdfdefb8c0 fix: print err when parseArgument() fails (#4355)
* 7eafda0b2ece5adb5d9ee18962a7f296ebecdded fix(core): ctx_id* types of io_* events typo
* 2a3f0dd4c6501ce9e8be316aba22d076db2965e6 fix(parse): correct types sizes
* 30b33a4db248cdef77cfdc7886fa58ec5b657138 fix(ebpf): register processor conditionally
* cf1562d0a54fa61b4fbbff9518ad03cc4a954d75 fix(ebpf): try to assert only when it is not nil
* a869f8030eb72064752f234841145e66343d249d fix(events): use umode_t instead of mode_t
* 21f573140158009c7a049772c0046bdc860408c7 feat(events): add chmod_common event
| * 033d01f7d4102cd07617b1ad324a9737c13d632f fix(mount): reintroduce root path requirement
|/  
* 47451f5627649da50141565052eee6914ac8ea9e feat(cmd): disable arg parsing by default

commit 47451f5627649da50141565052eee6914ac8ea9e
Author: Nadav Strahilevitz <[email protected]>
Date:   Wed May 29 12:14:53 2024 +0000

    feat(cmd): disable arg parsing by default

Rebase locally, and then force push to fix_eks_mount.

@NDStrahilevitz NDStrahilevitz merged commit 03208e5 into aquasecurity:main Oct 31, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate/v0.22.3 Candidate to be cherry-picked or backported into v0.22.3 release milestone/v0.23.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cgroupfs mounting doesn't work on EKS
2 participants