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 driver loader enhancements #2165

Merged
merged 2 commits into from
Aug 24, 2022
Merged

Falco driver loader enhancements #2165

merged 2 commits into from
Aug 24, 2022

Conversation

IanRobertson-wpe
Copy link
Contributor

@IanRobertson-wpe IanRobertson-wpe commented Aug 16, 2022 •
edited by jasondellaluce
Loading

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

This PR introduces changes to the falco-driver-loader script.

to support multiple download URLs in the DRIVERS_REPO environment variable, and to store downloaded drivers in a folder that mirrors the download directory.

The DRIVERS_REPO environment variable can now be used to specify multiple download URLs by separating individual URLs by a comma. URLs will be attempted in order from first to last as specified. Once a successful download has been performed (per a successful result code from curl), any remaining URLs will not be attempted. This will allow, for example, users to specify their own custom driver repo URL without having to duplicate those that are already available in the standard downloads.falco.org repo.

Example:

DRIVERS_REPO="https://myrepo.mycompany.com/mydrivers,https://download.falco.org/driver"

When storing downloaded drivers, falco-driver-loader previously stored these under /root/.falco/. However, falco-driver-loader could load a mismatched DRIVER_VERSION kernel module or eBPF probe as there was no identifying information relating to the locally-stored driver. These drivers will now be stored under /root/.falco/${DRIVER_VERSION}/${ARCH}/, which exactly mirrors the download URL that is constructed. Thus, there is parity between the locally-stored download location and the download path. While preventing mismatched driver versions with Falco, this also provides the advantage of allowing users to pre-load drivers by placing these kernel modules and eBPF probes directly on disk through a distribution mechanism other than DRIVERS_REPO.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new(falco-driver-loader): `DRIVERS_REPO` now supports the use of multiple download URLs (comma separated)

@IanRobertson-wpe
Identify DRIVER_VERSION and ARCH by storing in their proper directories
14328c6 
Signed-off-by: Ian Robertson <ian.robertson@wpengine.com>
@IanRobertson-wpe
Support multiple URLs for DRIVERS_REPO environment variable (comma se…
dd8239a 
…parated)

Signed-off-by: Ian Robertson <ian.robertson@wpengine.com>
@poiana
Copy link
Contributor

poiana commented Aug 16, 2022

Welcome @IanRobertson-wpe! It looks like this is your first PR to falcosecurity/falco 🎉

@poiana poiana added the size/M label Aug 16, 2022
@poiana poiana requested review from Kaizhe and mfdii August 16, 2022 20:37
@leogr
Copy link
Member

leogr commented Aug 17, 2022

cc @FedeDP

@leogr
Copy link
Member

leogr commented Aug 17, 2022

/milestone 0.33.0

@poiana poiana added this to the 0.33.0 milestone Aug 17, 2022
FedeDP
FedeDP reviewed Aug 22, 2022
View reviewed changes
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.

All in all, i really like the changes; i just left a minor comment.
Thank you for this patch :)

scripts/falco-driver-loader
IFS=", " read -r -a urls <<< "${DRIVERS_REPO}"
for url in "${urls[@]}"; do
load_bpf_probe_download $url
if [ $? -eq 0 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is missing from the kmod section above (after load_kernel_module_download $url); is it ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the load_kernel_module_download function will immediately exit when it gets a good download/install, so I didn't need to do the same check on the errorlevel. If that function is not successful (if we need to try another url) it will return here and the loop can proceed. This is different from the probe wherein I must check the errorlevel to determine success, as that function does not directly exit and always returns.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, thank you!

@FedeDP
Copy link
Contributor

FedeDP commented Aug 22, 2022

Changes LGTM!

@FedeDP
Copy link
Contributor

FedeDP commented Aug 24, 2022

Was able to test this:

  • passing multiple drivers repos
# DRIVERS_REPO=https://download.falco.org/driver,https://download.falco.org/driverss ./falco-driver-loader.sh --download
* Running falco-driver-loader for: falco version=0.32.1, driver version=2.0.0+driver, arch=x86_64, kernel release=5.19.3-arch1-1, kernel version=1
* Running falco-driver-loader with: driver=module, compile=no, download=yes

================ Cleaning phase ================

* 1. Check if kernel module 'falco' is still loaded:
- OK! There is no 'falco' module loaded.

* 2. Check all versions of kernel module 'falco' in dkms:
- OK! There are no 'falco' module versions in dkms.

[SUCCESS] Cleaning phase correctly terminated.

================ Cleaning phase ================

* Looking for a falco module locally (kernel 5.19.3-arch1-1)
* Filename 'falco_arch_5.19.3-arch1-1_1.ko' is composed of:
- driver name: falco
- target identifier: arch
- kernel release: 5.19.3-arch1-1
- kernel version: 1
* Trying to download a prebuilt falco module from https://download.falco.org/driver/2.0.0%2Bdriver/x86_64/falco_arch_5.19.3-arch1-1_1.ko
curl: (22) The requested URL returned error: 404
Unable to find a prebuilt falco module
* Trying to download a prebuilt falco module from https://download.falco.org/driverss/2.0.0%2Bdriver/x86_64/falco_arch_5.19.3-arch1-1_1.ko
curl: (22) The requested URL returned error: 404
Unable to find a prebuilt falco module
* Trying to load a system falco module, if present
Consider compiling your own falco driver and loading it or getting in touch with the Falco community
  • using default one:
./falco-driver-loader.sh --download
* Running falco-driver-loader for: falco version=0.32.1, driver version=2.0.0+driver, arch=x86_64, kernel release=5.19.3-arch1-1, kernel version=1
* Running falco-driver-loader with: driver=module, compile=no, download=yes

================ Cleaning phase ================

* 1. Check if kernel module 'falco' is still loaded:
- OK! There is no 'falco' module loaded.

* 2. Check all versions of kernel module 'falco' in dkms:
- OK! There are no 'falco' module versions in dkms.

[SUCCESS] Cleaning phase correctly terminated.

================ Cleaning phase ================

* Looking for a falco module locally (kernel 5.19.3-arch1-1)
* Filename 'falco_arch_5.19.3-arch1-1_1.ko' is composed of:
- driver name: falco
- target identifier: arch
- kernel release: 5.19.3-arch1-1
- kernel version: 1
* Trying to download a prebuilt falco module from https://download.falco.org/driver/2.0.0%2Bdriver/x86_64/falco_arch_5.19.3-arch1-1_1.ko
curl: (22) The requested URL returned error: 404
Unable to find a prebuilt falco module
* Trying to load a system falco module, if present
Consider compiling your own falco driver and loading it or getting in touch with the Falco community

FedeDP
FedeDP approved these changes Aug 24, 2022
View reviewed changes
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 poiana added the lgtm label Aug 24, 2022
@poiana
Copy link
Contributor

poiana commented Aug 24, 2022

LGTM label has been added.

Git tree hash: 448daff7d6380c66a2a71727b35a1f3cc463b50c

@poiana
Copy link
Contributor

poiana commented Aug 24, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, IanRobertson-wpe

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

leogr
leogr approved these changes Aug 24, 2022
View reviewed changes
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

Thank you!

Really nice 👍

@poiana poiana merged commit 8872f25 into falcosecurity:master Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FedeDP FedeDP FedeDP approved these changes

@leogr leogr leogr approved these changes

@Kaizhe Kaizhe Awaiting requested review from Kaizhe

@mfdii mfdii Awaiting requested review from mfdii

Assignees

@leogr leogr

@FedeDP FedeDP

Labels
approved area/engine dco-signoff: yes kind/feature lgtm release-note size/M
Projects
None yet
Milestone
0.33.0
Development

Successfully merging this pull request may close these issues.
4 participants
@IanRobertson-wpe @poiana @leogr @FedeDP