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.

Sign up for GitHub

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

Jump to bottom

falco: when leastPrivileged is true, set the apparmor profile to … #769

Merged
merged 4 commits into from
Oct 30, 2024
Merged

falco: when leastPrivileged is true, set the apparmor profile to … #769

merged 4 commits into from
Oct 30, 2024
+17 −2

Conversation

doublez13
Copy link
Contributor

@doublez13 doublez13 commented Oct 24, 2024 •
edited
Loading

What type of PR is this?

/kind bug
/kind chart-release

What this PR does / why we need it:
It appears that when setting leastPrivileged: true, apparmor does not not allow falco to ptrace, which appears to leave the container fields null.

Oct 24 09:52:57 hostname kernel: audit: type=1400 audit(1729785177.339:404624): apparmor="DENIED" operation="ptrace" profile="cri-containerd.apparmor.d" pid=2389102 comm="falco" requested_mask="read" denied_mask="read" peer="unconfined"

If leastPrivileged: true, set the apparmor profile to unconfined.

@leogr
This just a request for comments, as I'm not sure if this if the best way to solve the issue.
Or perhaps there should be an optional field in the helm file that allows specifying a apparmor profile (custom or unconfined.)

Which issue(s) this PR fixes:
falcosecurity/falco#3345

Checklist

  • Chart Version bumped
  • Variables are documented in the README.md
  • CHANGELOG.md updated

@poiana poiana added do-not-merge/work-in-progress kind/bug Something isn't working dco-signoff: yes kind/chart-release Add this label when the chart version has been bumped labels Oct 24, 2024
@poiana
Copy link
Contributor

poiana commented Oct 24, 2024

Welcome @doublez13! It looks like this is your first PR to falcosecurity/charts 🎉

@poiana poiana requested review from bencer and leogr October 24, 2024 20:54
@poiana poiana added the size/S label Oct 24, 2024
@leogr leogr mentioned this pull request Oct 28, 2024
Open
@leogr
Copy link
Member

leogr commented Oct 28, 2024

This just a request for comments, as I'm not sure if this if the best way to solve the issue.
Or perhaps there should be an optional field in the helm file that allows specifying a apparmor profile (custom or unconfined.)

Hey @doublez13

Thank you for this. I haven't dug into it, but it seems to be the correct approach. I'll do some tests.
cc @falcosecurity/charts-maintainers

@leogr
Copy link
Member

leogr commented Oct 28, 2024

@doublez13

also, can you bump the chart version? so the test will run 🙏

@leogr
Copy link
Member

leogr commented Oct 29, 2024

Hey @doublez13

I'm ok with this fix, so we can go ahead.

To merge this PR, we just need to:

  • rebase
  • bump the version again
  • run make falco-docs (and commit changes)
  • remove WIP from PR's title.

Let me know if you can do that; otherwise, I will do it for you.

Thank you

@doublez13 doublez13 changed the title WIP falco: when leastPrivileged is true, set the apparmor profile to … falco: when leastPrivileged is true, set the apparmor profile to … Oct 29, 2024
@doublez13
Copy link
Contributor Author

I'm away from my computer for awhile (just phone). You're welcome to rebase and merge, or I can do it later.

@leogr
Copy link
Member

leogr commented Oct 30, 2024

I'm away from my computer for awhile (just phone). You're welcome to rebase and merge, or I can do it later.

I'm rebasing right now.

doublez13 and others added 4 commits October 30, 2024 11:27
@doublez13 @leogr
RFC falco: when leastPrivileged is true, set the apparmor profile to …
c96a27e 
…unconfined

It appears that when setting leastPrivileged: true, apparmor does not not allow falco to ptrace, which appears to leave the container fields null. If leastPrivileged: true, set the apparmor profile to unconfined.

Oct 24 09:52:57 hostname kernel: audit: type=1400 audit(1729785177.339:404624): apparmor="DENIED" operation="ptrace" profile="cri-containerd.apparmor.d" pid=2389102 comm="falco" requested_mask="read" denied_mask="read" peer="unconfined"


Signed-off-by: doublez13 <[email protected]>
@doublez13 @leogr
Falco: Bump chart to 4.12.0
cca8b65 
Signed-off-by: doublez13 <[email protected]>
@doublez13 @leogr
Update the changelog to document unconfined apparmor
8d809fb 
Signed-off-by: doublez13 <[email protected]>
@leogr
docs(charts/falco): update README.md
5d7c35b 
Signed-off-by: Leonardo Grasso <[email protected]>
@poiana poiana added the lgtm label Oct 30, 2024
@poiana
Copy link
Contributor

poiana commented Oct 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: doublez13, 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:

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

@poiana
Copy link
Contributor

poiana commented Oct 30, 2024

LGTM label has been added.

Git tree hash: cc294e786fb73dc8a2b6eb8368e8f7a1eafe810b

@poiana poiana merged commit 6018dfb into falcosecurity:master Oct 30, 2024
6 checks passed
mkilchhofer added a commit to mkilchhofer/k8s-gitops-services that referenced this pull request Nov 1, 2024
@mkilchhofer
fix(kyverno-policies): Exclude Falco from apparmor restrictions
6788bea 
Related PRs:
- #404
- falcosecurity/charts#769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leogr leogr leogr approved these changes

@bencer bencer Awaiting requested review from bencer

Assignees

@leogr leogr

Labels
approved dco-signoff: yes kind/bug Something isn't working kind/chart-release Add this label when the chart version has been bumped lgtm size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@doublez13 @poiana @leogr