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

Ssprod 23814 add addl event init method #1052

Closed

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Apr 17, 2023

Bring over a change from upstream (#1050) that I needed for some local tests.

Andreagit97 and others added 30 commits December 15, 2022 17:11
This change increases the number of retries to retrieve container
information from CRI API from 3 to 5, as several failures were observed
with the maximum number of attempts set to 3.

Signed-off-by: Iacopo Rozzo <[email protected]>
Co-authored-by: Andrea Terzolo <[email protected]>
Signed-off-by: Melissa Kilby <[email protected]>
This change makes sure that 5 maximum retries to retrieve container information
are used with CRI only. It puts back the number of retries to 3 for all the
other container runtimes.
It also adjusts the maximum time to complete all their attempts to take into
account the increased of retries.

Signed-off-by: Iacopo Rozzo <[email protected]>
Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Hendrik Brueckner <[email protected]>
Co-authored-by: Mauro Ezequiel Moltrasio <[email protected]>
Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Hendrik Brueckner <[email protected]>
Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Hendrik Brueckner <[email protected]>
FedeDP and others added 19 commits March 16, 2023 12:13
Add a way to skip PPM_SC_NA_X codes while populating syscall_info_table.

Signed-off-by: Federico Di Pierro <[email protected]>
… user and group container lookup (#142)

Partial port of falcosecurity#803 (e2e test update skipped)
When node is using NIS / nss_compat for user management, /etc/passwd 
entries can refer to NIS groups or users, which causes parser to return null 
pointers instead of c-strings.

This change adds checks against those.

---------

Signed-off-by: Wiktor Gołgowski <[email protected]>
**What type of PR is this?**

/kind feature

**Any specific area of the project related to this PR?**

/area libsinsp

**Does this PR require a change in the driver versions?**

**What this PR does / why we need it**:

Change usage of the library TBB so that it can cope with a newer version of it.
More specifically it removes the usage of `tbb::tbb_hash` (nno longer available in newer versions of TBB) in favour of a hash function composed from `std::hash`.

**Which issue(s) this PR fixes**:

**Special notes for your reviewer**:

The change affects code that is only present in `draios/agent-libs` and **not** in `falcosecurity/libs`.

**Does this PR introduce a user-facing change?**:

```release-note
NONE
```
* Add method to retrieve argid from thread filterchecks

Thread filterchecks can have a numeric arg for some fields like
proc.aname/proc.apid. This allows returning an argid for a filtercheck
object. It's -1 if the field doesn't support or doesn't have an arg.

This can be used in unit tests to print/compare filtercheck objects.

Signed-off-by: Mark Stemm <[email protected]>

* Also save (pointers) to filtercheck values in order

Currently, a filtercheck saves the raw values in m_val_storages and
a pointer + parsed len in m_val_storages_members. Because
m_val_storages_members is an unordered_set, the original order is
lost. This makes it difficult to print out a filter expression and
compare it to the original input string, as the order of checks like
"field in (val1, val2, val3, ...)" are lost.

To help retain this order, add a vector m_vals that saves the pointer
+ length, but in a vector instead of in an ordered_set.

Signed-off-by: Mark Stemm <[email protected]>

* Add a const base_expr_visitor

Following on the changes in
falcosecurity/libs#837, add a const variant of
base_expr_visitor. This allows definining subclasses that want to
traverse an ast read-only without implementng all the methods.

Signed-off-by: Mark Stemm <[email protected]>

---------

Signed-off-by: Mark Stemm <[email protected]>
tid != vtid is not necessary (though sufficient) to check if
a thread is in a child pid namespace. This leads to pidns_start_ts
being wrong occasionally (when a thread happens to have tid == vtid
by chance, even if it's in a child pidns).

Signed-off-by: Grzegorz Nosek <[email protected]>
We can significantly simplify the logic of checking the pidns start time
(during the initial /proc scan), based on the following observations:
* the task's start time is (apparently) accessible as simply the timestamp (any timestamp) on its /proc/
* for every task, its root filesystem is accessible via /proc/<pid>/root
* the first task in every pidns has pid==1 (as seen from the pidns)

Putting these together, it seems it's enough to stat("/proc/<pid>/root/proc/1")
and pick whichever you want out of [acm]time.

Ref: falcosecurity/libs#860 (comment)

Signed-off-by: Grzegorz Nosek <[email protected]>
…reation time

See falcosecurity/libs#932 for more context

Change occurrences of `/proc/1` to `/proc/1/cmdline` in
* userspace/libscap/linux/scap_procs.c
* userspace/libscap/scap.c

Previous:
```c
snprintf(proc_dir, sizeof(proc_dir), "%s/proc/1/", scap_get_host_root());
```

This PR:
```c
snprintf(proc_cmdline, sizeof(proc_cmdline), "%s/proc/1/cmdline", scap_get_host_root());
```

Co-authored-by: Grzegorz Nosek <[email protected]>
Co-authored-by: Melissa Kilby <[email protected]>
Signed-off-by: Stanley Chan <[email protected]>
Get boot time from btime value in /proc/stat

ref: falcosecurity/libs#932

/proc/uptime and btime in /proc/stat are fed by the same kernel sources.

Multiple ways to get boot time:
* btime in /proc/stat
* calculation via clock_gettime(CLOCK_REALTIME - CLOCK_BOOTTIME)
* calculation via time(NULL) - sysinfo().uptime

Maintainers preferred btime in /proc/stat because:
* value does not depend on calculation using current timestamp
* btime is "static" and doesn't change once set
* btime is available in kernels from 2008
* CLOCK_BOOTTIME is available in kernels from 2011 (2.6.38)

By scraping btime from /proc/stat, it is both the heaviest and most likely to succeed

Co-authored-by: Grzegorz Nosek <[email protected]>
Co-authored-by: Melissa Kilby <[email protected]>
Signed-off-by: Stanley Chan <[email protected]>
Co-authored-by: Grzegorz Nosek <[email protected]>
Co-authored-by: Melissa Kilby <[email protected]>
Signed-off-by: Stanley Chan <[email protected]>
Co-authored-by: Angelo Puglisi <[email protected]>
Signed-off-by: Angelo Puglisi <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
The nodriver engine is still the only user of limited /proc scan
but we no longer check handle->m_mode. Instead we have a dedicated
flag. This lets us have the nodriver engine with a full /proc scan
if we want.

Signed-off-by: Grzegorz Nosek <[email protected]>
The patch is somewhat weird because it introduces an option
for the nodriver engine which is used only by the main libscap
code (the /proc scan does not live in the engine). Still, we're
(logically) configuring the nodriver engine, I believe the flag
belongs in the engine config.

Signed-off-by: Grzegorz Nosek <[email protected]>
This new flag means that we're *not* going to get any events
from the engine and are using it just for the secondary
effects of scap_open (mostly getting the process table).

This is needed to safely open a new inspector while another
one exists. Otherwise we'd overwrite the gVisor socket
with a new one (which would become inactive the moment we close
the second inspector), breaking all future gVisor connections.

Co-Authored-By: Angelo Puglisi <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
In dba2e32 we switched the way we
determine the boot time from CLOCK_BOOTTIME based to /proc/stat based.

The new way is more compatible (including compatibility with ancient
kernels) but it only has a full second accuracy (the fractional part
is lost).

Unfortunately, we need the extra precision in BPF engines since we only
get timestamps since boot from the kernel. Without the subsecond part,
all events get their timestamps shifted by up to a second to the past
(the exact value depends on the fractional part of the second the machine
booted).

Since BPF engines do not need compatibility with prehistoric kernels
(they don't support eBPF anyway), switch them to use CLOCK_BOOTTIME
to get the boot time.

Signed-off-by: Grzegorz Nosek <[email protected]>
Some tests rely on creating fake events without all of the overhead of
an inspector, etc. To support new tests that rely on
m_errorcode (generally maps to the res field of events), add a new
initializer that passes in a scap + ppm header and errorcode, and just
directly sets m_errorcode.

Signed-off-by: Mark Stemm <[email protected]>
@poiana
Copy link
Contributor

poiana commented Apr 17, 2023

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@poiana
Copy link
Contributor

poiana commented Apr 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mstemm

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 Apr 17, 2023

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@mstemm mstemm closed this Apr 17, 2023
@mstemm mstemm deleted the SSPROD-23814-add-addl-event-init-method branch April 17, 2023 16:24
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.