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(falco_service): falco service needs to write under /sys/module/falco #2238

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

Andreagit97
Copy link
Member

@Andreagit97 Andreagit97 commented Oct 10, 2022

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area build

What this PR does / why we need it:

Trying to run the latest Falco deb/rpm package there is an issue:

Since this PR #2214, Falco service needs to write under /sys/module/falco/parameters/g_buffer_bytes_dim the variable buffer dimension. The problem is that right now the path /sys/module is mounted read-only so Falco will fail with Errno 30 in the attempt to write /sys/module/falco/parameters/g_buffer_bytes_dim:

Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Falco version: 0.32.1-241+251b4d5 (x86_64)
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Falco version: 0.32.1-241+251b4d5 (x86_64)
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Falco initialized with configuration file: /etc/falco/falco.yaml
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Falco initialized with configuration file: /etc/falco/falco.yaml
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Loading rules from file /etc/falco/falco_rules.yaml
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Loading rules from file /etc/falco/falco_rules.yaml
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Loading rules from file /etc/falco/falco_rules.local.yaml
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Loading rules from file /etc/falco/falco_rules.local.yaml
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: The chosen syscall buffer dimension is: 8388608 bytes (8 MBs)
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: The chosen syscall buffer dimension is: 8388608 bytes (8 MBs)
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Starting health webserver with threadiness 6, listening on port 8765
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Starting health webserver with threadiness 6, listening on port 8765
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Enabled event sources: syscall
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Opening capture with Kernel module
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Enabled event sources: syscall
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Opening capture with Kernel module
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Trying to inject the Kernel module and opening the capture again...
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Fri Oct  7 23:41:26 2022: Trying to inject the Kernel module and opening the capture again...
Oct 07 23:41:26 ubuntu2204.localdomain falco[34605]: Error: unable to open '/sys/module/falco/parameters/g_buffer_bytes_dim': Errno 30. Please ensure the kernel module is already loaded.

Considering the falco service config:

[Unit]
Description=Falco: Container Native Runtime Security
Documentation=https://falco.org/docs/

[Service]
Type=simple
User=root
ExecStartPre=/sbin/modprobe falco
ExecStart=/usr/bin/falco --pidfile=/var/run/falco.pid
ExecStopPost=/sbin/rmmod falco
UMask=0077
TimeoutSec=30
RestartSec=15s
Restart=on-failure
PrivateTmp=true
NoNewPrivileges=yes
ProtectHome=read-only
ProtectSystem=full
ProtectKernelTunables=true
RestrictRealtime=true
RestrictAddressFamilies=~AF_PACKET

[Install]
WantedBy=multi-user.target

there are some solutions to this problem:

  1. The most aggressive one is to put ProtectKernelTunables to false
  2. [PROPOSED ONE] The cleanest one should be to inject the kmod in a separate unit and call the falco unit only when the path /sys/module/falco is already there, in this way the falco unit could set ReadWritePaths to only /sys/module/falco
  3. The third solution sets ReadWritePaths to /sys/module because the falco subfolder is not yet created at startup-time and we cannot mkdir this directory into another unit because we cannot create folders under sys/module only the kernel can do that.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix(falco_service): falco service needs to write under /sys/module/falco

…alco`

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@Andreagit97
Copy link
Member Author

@leogr @FedeDP @happy-dude WDYT?

@Andreagit97
Copy link
Member Author

PS: I forgot to mention that systemd fail at all when something in a ReadWritePath directive doesn't exist 👇
systemd/systemd#10972 (comment)

@Andreagit97 Andreagit97 added this to the 0.33.0 milestone Oct 10, 2022
@jasondellaluce jasondellaluce mentioned this pull request Oct 10, 2022
96 tasks
@poiana poiana added size/S and removed size/XS labels Oct 10, 2022
@Andreagit97
Copy link
Member Author

Andreagit97 commented Oct 10, 2022

The first commit propose the solution

The third solution sets ReadWritePaths to /sys/module because the falco subfolder is not yet created at startup-time and we cannot mkdir this directory into another unit because we cannot create folders under sys/module only the kernel can do that.

The last commit propose the solution that should be better 🤔

[PROPOSED ONE] The cleanest one should be to inject the kmod in a separate unit and call the falco unit only when the path /sys/module/falco is already there, in this way the falco unit could set ReadWritePaths to only /sys/module/falco

Signed-off-by: Andrea Terzolo <andrea.terzolo@polito.it>
@happy-dude
Copy link
Contributor

Does PermissionsStartOnly=true allow writing into /sys/module or is still mounted as read only?

While I don't love having multiple service units, I do appreciate the clarity that comes from splitting the modprobe tasks.
(I personally modify the service unit to remove the modprobe commands because I want to start the service in BPF mode.)

@FedeDP
Copy link
Contributor

FedeDP commented Oct 10, 2022

Hi @happy-dude !
I think PermissionsStartOnly=true is only useful for Pre/Post Exec, right?
In this case, it won't help because the write to /sys/module is being done by Falco itself (https://github.com/falcosecurity/libs/blob/master/userspace/libscap/engine/kmod/scap_kmod.c#L79).

Btw i am working on an improvements over the current falco systemd unit, to support bpf too. I will hopefully remember to tag you when the PR it's open :D

@FedeDP
Copy link
Contributor

FedeDP commented Oct 10, 2022

The changes LGTM. I'll wait some more feedback!

@jasondellaluce
Copy link
Contributor

/milestone 0.33.0

@FedeDP
Copy link
Contributor

FedeDP commented Oct 11, 2022

See also #2242 .
I think this is safer for the imminent release though :)

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Oct 12, 2022

LGTM label has been added.

Git tree hash: 43d9aa61422d96b75c563125b6151806a877ed1d

Copy link
Contributor

@jasondellaluce jasondellaluce left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Oct 12, 2022

[APPROVALNOTIFIER] This PR is APPROVED

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

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

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

@poiana poiana merged commit 7da3041 into master Oct 12, 2022
@poiana poiana deleted the fix_kmod_deb branch October 12, 2022 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants