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

`nftables` support for firewalling rules #6223

Open
stgraber opened this issue Sep 19, 2019 · 8 comments

Comments

@stgraber
Copy link
Member

commented Sep 19, 2019

As more and more Linux distributions switch to nft, we need to cleanup our existing logic and add an nft alternative implementation.

The todo will look something like:

  • Centralize all our iptables, ip6tables and ebtables calls into one struct in a separate package, implementing a firewall interface.
  • Updating all existing code to load that struct (effectively asking for a firewall implementation backed by iptables)
  • Implement the same interface for nftables
  • Add startup logic to query both iptables and nftables, if we find existing rules in either, we will prefer the backend currently in use on the system, if neither contain rules, we use nft, if both of them do, we also use nft

This should handle all cases we care about in a clean way. Note that we will NOT support using the iptables compat wrappers as we've repeatedly found issues with those, especially around external modules/extensions which aren't properly supported. A native implementation is going to be much more reliable.

@stgraber stgraber added this to the later milestone Sep 19, 2019
@stgraber stgraber changed the title Introduce `iptables` package and implement `nftables` alternative `nftables` support for firewalling rules Sep 19, 2019
@keyallis

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2019

Myself and my partner from the UT Virtualization class would like to take this issue as part of our class project

@piluke

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2019

Hi, I'm the partner, could I also be assigned?

@stgraber

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2019

Thanks, assigned it to you. Unlike the image expiry issue, this one will be a bit trickier and will likely be done through multiple distinct pull requests to make things easier to review.

  1. The first stage is to look at all our current uses of the iptables, ip6tables and ebtables commands in the LXD code, understand what each call does and design a list of functions that will perform these actions in a generic way.

  2. All those function signatures will then be attached to a Go interface we'll call firewall and a first implementation of that interface will be done called xtables (generic name covering iptables, ip6tables and ebtables).

  3. The LXD State struct will grow an additional Firewall member which will hold a pointer to the currently used firewall backend. Startup logic will be added to the LXD daemon to set that pointer on startup.

  4. All existing calls to ebtables/iptables/ip6tables will be converted to using the relevant State.Firewall.function() call

  5. A second implementation of the firewall interface called nftables will be introduced, directly driving nftables and implementing all the same functionality as the existing xtables.

  6. Detection logic will be added to the daemon startup to decide whether to use xtables or nftables, setting State.Firewall to whichever backend is chosen (and logging an appropriate startup message).

I would expect each of those steps (except for the first) to lead to its own separate pull request, making it easier to review and get things into LXD rather than having one huge code dump at the end.

@stgraber

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2019

@tomponline and myself will be the main contact points for this one.

@stgraber

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2019

@tomponline does the above plan sound good to you?

@tomponline

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

@stgraber yep sounds good to me.

Worth reiterating that the interface and the implementations should be in their own package outside of main.

Do you have any initial thoughts on what the logic for step 6 would be?

@stgraber

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2019

@tomponline what I described in the issue's description:

Add startup logic to query both iptables and nftables, if we find existing rules in either, we will prefer the backend currently in use on the system, if neither contain rules, we use nft, if both of them do, we also use nft.
@tomponline

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

@stgraber ah OK thanks.

@stgraber stgraber modified the milestones: later, soon Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.