DelusionalOptimist / dropit

Network firewall using XDP
MIT License
1 stars 1 forks source link

feat: Implement bandwidth limit on egress using tc #2

Open Revolyssup opened 1 year ago

Revolyssup commented 1 year ago

Proposal: TC(Traffic Control) supports multiple types of filters that can be attached to different classes inside q-disks(RTM). Since neither libbpfgo nor ebpf-go libraries support tc functionality directly due to multiple reasons listed here

We can use a separate library like go-tc to attach ebpf filters on an interface(ingress/egress) to implement bandwidth limiting on any given interface.

If the proposal sounds good, then I would like to create a draft PR on this where we can carry forward the discussion.


Some references

  1. https://liuhangbin.netlify.app/post/ebpf-and-xdp/
Revolyssup commented 1 year ago

@DelusionalOptimist Please suggest if you have any better approach.

DelusionalOptimist commented 1 year ago

@Revolyssup Have we considered the TC hooking capabilities that libbpfgo provides? Are they not sufficient for the use case? Here is an example - https://github.com/aquasecurity/libbpfgo/tree/main/selftest/tc

Revolyssup commented 1 year ago

@DelusionalOptimist LGTM, I might have missed this. Lets go with this.

DelusionalOptimist commented 1 year ago

Let's go! :rocket:

Revolyssup commented 1 year ago

@DelusionalOptimist Just to confirm few things, here is the proposal:

DelusionalOptimist commented 1 year ago

@Revolyssup based on your proposed configuration, I can picture it somewhat like below. Let me know if I'm right.

rules:
  - id: "restrict webserver"
    ingress:
      - id: "only allow port 443"
        sourceIP: '*'
        destinationPort: 443
        <...>
        # future
        action: allow
        priority: -20
    egress:
      - id: "block access to github.com"
        destIP: '20.207.73.82'
        destinationPort: '*'
        <...>
        # future
        priority: 20
  - id: "restrict firefox"
    egress:
      - id: "block access to ads.google.com"
        destIP: '142.250.194.206'
        destinationPort: '*'
        <...>
        # future
        priority: -20

This sounds good and also pretty close to Kubernetes' NetworkPolicy. I think this way we can use each individual "rule block" to granularly define process level networking.

A couple of things worth mentioning:

  1. Structuring the rules internally keeping in mind limitations of BPF Maps and how they can be iterated.
  2. For reference - https://github.com/xdp-project/xdp-tutorial/tree/master/advanced01-xdp-tc-interact

We can have sibling field action where more complicated packet manipulation actions could be defined, later referenced by ID/name in rules. This last one could be a no-no depending if we want dropit to do just one thing(act as a firewall dropping packets) or if we want to later on extend it to go beyond plain packet dropping.

I think we can extend further to basic things like redirecting packet based on some information and so on but let's think about it later.

Revolyssup commented 1 year ago

@DelusionalOptimist In first iteration, will we need the interaction between xdp and tc or can they work independently on ingress and egress respectively?

DelusionalOptimist commented 1 year ago

@Revolyssup My bad. I meant to cite that resource for future reference. I don't think we would need any interaction b/w the two for our current use case.

Revolyssup commented 1 year ago

@DelusionalOptimist What if we kept it simple and just add a type field to existing rules ? Assuming functionality added might have a different approach for ingress and egress but will be the same. In cases when something can not be implemented by either one, we can produce warn logs indicating that. For eg: [warn] xyz capability not supported on ingress by sanitising/checking the configuration provided with the features that can be supported at config load time.

Let me know if this categorization makes sense to you?

Revolyssup commented 1 year ago

I think I should create a draft PR from here and carry discussion there so we have something concrete to make comments on.

Revolyssup commented 1 year ago

@DelusionalOptimist Do you suggest we have separate maps for ingress and egress filter_rules? To avoid contention as the rules are evaluated on each entry, we would be evaluating ingress rules on egress packets.

DelusionalOptimist commented 1 year ago

What if we kept it simple and just add a type field to existing rules ? Assuming functionality added might have a different approach for ingress and egress but will be the same. In cases when something can not be implemented by either one, we can produce warn logs indicating that. For eg: [warn] xyz capability not supported on ingress by sanitising/checking the configuration provided with the features that can be supported at config load time.

Quite frankly that was what I was thinking initially but I thought you wanted to keep it close to Network Policy. I'm fine either ways.