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

examples: Add trace network with container filter #2599

Conversation

danielpacak
Copy link
Contributor

@danielpacak danielpacak commented Mar 9, 2024

I've shared an example setup I'm using to test network tracer with container filters. Dockerfile, deployment descriptor and install/uninstall scripts need to be adjusted to fit the project structure, but I wanted to share network.go and collect feedback first, before this example might be accepted as a representative example.

Resolves: #2593

Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

I left some comments on network.go (so you can change your implementation too).

Some broader comments before continue reviewing:

  1. If you want to polish this PR to merge it, that's fine for me, I'll review it, but please consider we're in the process of moving to image-based gadgets and implementing a golang API for them, once that's done, we'll deprecate and remove the built-in gadgets (API being used by this example).

  2. We're trying to create a much easier to consume API for the gadgets. Thinks like setting the operators, connecting the socket enricher to the tracer and so on should be handled automatically by us. We'll love to have feedback from users of our API, if you're willing I'll ping you once we have something to test.

  3. Are you interested in enriching the IP addresses to K8s svc / pod names? You can see how it's done in https://github.com/inspektor-gadget/inspektor-gadget/blob/main/pkg/gadget-collection/gadgets/trace/network/gadget.go#L123-L139.

@alban do you have any comments on this example?

examples/gadgets/withfilter/trace/network/go.mod Outdated Show resolved Hide resolved
examples/gadgets/withfilter/trace/network/network.go Outdated Show resolved Hide resolved
examples/gadgets/withfilter/trace/network/network.go Outdated Show resolved Hide resolved
@alban
Copy link
Member

alban commented Mar 11, 2024

@alban do you have any comments on this example?

I agree with your comments. Hopefully the new Golang API will make it easier to consume IG packages. Meanwhile, it is good to have this example. And once the new Golang API is ready, the example code in network.go should be a lot less lines of code.

@danielpacak danielpacak force-pushed the examples/network-tracer-with-container-filter branch 3 times, most recently from e17aee6 to 8ca0b57 Compare March 11, 2024 20:57
@danielpacak danielpacak marked this pull request as ready for review March 11, 2024 20:59
@danielpacak danielpacak force-pushed the examples/network-tracer-with-container-filter branch 2 times, most recently from 18f87ea to a00b9a9 Compare March 11, 2024 21:01
@danielpacak
Copy link
Contributor Author

Thanks for the heads up @mauriciovasquezbernal about breaking changes in the internal APIs. I'd love to see and evaluate new APIs that encapsulate nitty gritty details of container and Kubernetes enrichment.

We've been evaluating IG for its efficient container enrichment APIs and to collect golden signals for security monitoring (exec, network, open, capabilities, etc.). We'd also want to deploy it with least privilege principle (on recent Linux kernels). I'm happy to provide more feedback as your shape up the APIs.

@eiffel-fl
Copy link
Member

Hi!

We'd also want to deploy it with least privilege principle (on recent Linux kernels).

Can you please share more insights on this?

Best regards.

@danielpacak
Copy link
Contributor Author

Hi!

We'd also want to deploy it with least privilege principle (on recent Linux kernels).

Can you please share more insights on this?

Best regards.

Loading eBPF programs and getting container metadata requires elevated privileges. That's why people usually run such code as privileged pod controlled by a DaemonSet. Our requirement in security domain is to avoid running as privileged as much as possible and configure SecurityContext with just enough permissions and Linux capabilities so the tracers can work. Otherwise the tracer becomes a threat vector on its own and we'd not be able to deploy it in many environments, such as FedRAMP, AWS Bottlecket, etc.

That's why you'll see in the corresponding PR a deployment descriptor that I use to test if it's feasible to deploy IG in a non-privileged container.

@eiffel-fl
Copy link
Member

Hi!

We'd also want to deploy it with least privilege principle (on recent Linux kernels).

Can you please share more insights on this?
Best regards.

Loading eBPF programs and getting container metadata requires elevated privileges. That's why people usually run such code as privileged pod controlled by a DaemonSet. Our requirement in security domain is to avoid running as privileged as much as possible and configure SecurityContext with just enough permissions and Linux capabilities so the tracers can work. Otherwise the tracer becomes a threat vector on its own and we'd not be able to deploy it in many environments, such as FedRAMP, AWS Bottlecket, etc.

I totally share your point of view!
Nonetheless, Inspektor Gadget is no more run as privileged since #348.
We nonetheless still need to use CAP_SYS_ADMIN and other "important" capabilities, they are all documented here:

That's why you'll see in the corresponding PR a deployment descriptor that I use to test if it's feasible to deploy IG in a non-privileged container.

Your deploy.yaml seems not that different from ours:
https://github.com/inspektor-gadget/inspektor-gadget/blob/main/pkg/resources/manifests/deploy.yaml
The only differences I see with regard to security context are:

  1. You use super_t and we use spc_t for SELinux, but I am not sure if this leads to pratical difference as my SELinux knowledge is not that broad.
  2. You set privileged to false, but this is the default value.
  3. You set runAsUser and we do not.

Do you see any ways we could reduce the privileges we are actually using?
Your feedback would be appreciated on this question.

@danielpacak danielpacak force-pushed the examples/network-tracer-with-container-filter branch 2 times, most recently from 6af9c8f to 97aab78 Compare March 14, 2024 10:08
Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

Besides two minor nits LGTM. I'll merge once those nits are fixed. Thanks for the contribution!

@danielpacak danielpacak force-pushed the examples/network-tracer-with-container-filter branch 2 times, most recently from 4cdadb7 to bd85c12 Compare March 19, 2024 21:40
Resolves: inspektor-gadget#2593

Signed-off-by: Daniel Pacak <1322923+danielpacak@users.noreply.github.com>
@danielpacak danielpacak force-pushed the examples/network-tracer-with-container-filter branch from bd85c12 to bb6b116 Compare March 19, 2024 22:07
@danielpacak
Copy link
Contributor Author

Hi!

We'd also want to deploy it with least privilege principle (on recent Linux kernels).

Can you please share more insights on this?
Best regards.

Loading eBPF programs and getting container metadata requires elevated privileges. That's why people usually run such code as privileged pod controlled by a DaemonSet. Our requirement in security domain is to avoid running as privileged as much as possible and configure SecurityContext with just enough permissions and Linux capabilities so the tracers can work. Otherwise the tracer becomes a threat vector on its own and we'd not be able to deploy it in many environments, such as FedRAMP, AWS Bottlecket, etc.

I totally share your point of view! Nonetheless, Inspektor Gadget is no more run as privileged since #348. We nonetheless still need to use CAP_SYS_ADMIN and other "important" capabilities, they are all documented here:

That's why you'll see in the corresponding PR a deployment descriptor that I use to test if it's feasible to deploy IG in a non-privileged container.

Your deploy.yaml seems not that different from ours: https://github.com/inspektor-gadget/inspektor-gadget/blob/main/pkg/resources/manifests/deploy.yaml The only differences I see with regard to security context are:

1. You use `super_t` and we use `spc_t` for SELinux, but I am not sure if this leads to pratical difference as my SELinux knowledge is not that broad.

2. You set `privileged` to false, but this is the [default value](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#securitycontext-v1-core).

3. You set `runAsUser` and we do not.

Do you see any ways we could reduce the privileges we are actually using? Your feedback would be appreciated on this question.

I'm on a constant mission to run an agent with least privileges. I used super_t instead of spc_t because it was required to run on EKS that runs on Bottlerocket nodes. Anyways, for this example I believe we have a representative securityContext. If we find a way to further narrow down privileges we can patch it in subsequent pull requests.

@eiffel-fl
Copy link
Member

/* SNIP */
Do you see any ways we could reduce the privileges we are actually using? Your feedback would be appreciated on this question.

I'm on a constant mission to run an agent with least privileges. I used super_t instead of spc_t because it was required to run on EKS that runs on Bottlerocket nodes. Anyways, for this example I believe we have a representative securityContext. If we find a way to further narrow down privileges we can patch it in subsequent pull requests.

Same for us, reducing Inspektor Gadget privileges is something I take very seriously.
I will take a deeper look at super_t vs spc_t! I am nonetheless not sure to understand as we also run our CI on EKS:
#2481
Maybe EKS comes with different nodes and we are not using the bottlerocket ones?
In all cases, your contributions are welcomed 😄!

@mauriciovasquezbernal
Copy link
Member

Thanks a lot for your contribution!

@mauriciovasquezbernal mauriciovasquezbernal merged commit 79b1149 into inspektor-gadget:main Mar 20, 2024
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add trace network example (with container filter)
4 participants