On Sat, Aug 14, 2021 at 06:33:18PM +0200, Giulio wrote:

> Hello,
> Sorry for the late reply.
> While everything cli related is almost ready, I am having some issues on
> the actual implementation of the iptables/nft rules. I see that in the
> current state, it seems like Qubes is using both as also stated in [1].

Yes, there are two backends, depending on nft availability. This is
mostly because older distros (Debian 8...) did not have nft at all. 

> However, in the core-agent-linux source code, if the 'nft' binary is
> available  that is the only one that gets invoked. Furthermore, there
> are differences on the iptables backend depending on templates as
> reported in [2].

Yes, and since basically all the distributions have nft now, iptables
backend may be soon removed. I don't think we have any case where
iptables backend is used in practice in Qubes 4.1. RPM package has
strict dependency on nft, and Debian package has it as Suggests only,
but it is in practice installed.

> I am a bit stuck in understanding which rule to put where in order to
> have consistency across templates and between iptables/nft, also because
> if I blindly implement the rules suggested in [1] they will not actually
> work since most of the time iptables is not invoked at all.

Indeed adding rules to the IptablesWorker class will make no effect if
nft is in use.
Theoretically, iptables rules and nft rules can coexist, but we should
really focus on nft with new features.

> I also checked [3], however it is very similar to the instructions in
> [1] which leads to the same problems.
> Are we able to write working nft forwarding rules without invoking
> iptables at all? If yes, could you heml me determine which ones?

As for the nft syntax, I think iptables-translate tool can help you
(part of the iptables-nft package).
See https://wiki.nftables.org/wiki-nftables/index.php/Moving_from_iptables_to_nftables

> You can see in [4] how I organized the forwarding mechanism. All the
> necessary information, as well as ipv4/ipv6 support should already be in
> the 'prepare_forward_rules' function meaning that only the actual
> building syntax is left.

Yes, this layout looks ok.

> For simplicity you can look at the other changes at [5] and at [6].

This too looks fine (although I haven't don't detailed review).

In both cases, the code will need some tests of course.

> [1] - https://www.qubes-os.org/doc/firewall/
> [2] - https://github.com/QubesOS/qubes-issues/issues/5031
> [3] - https://gist.github.com/fepitre/941d7161ae1150d90e15f778027e3248
> [4] -
> https://github.com/lsd-cat/qubes-core-agent-linux/compare/f24ca2c..3e944af
> [5] - https://github.com/lsd-cat/qubes-core-admin/compare/6a1570b..6e3e250
> [6] -
> https://github.com/lsd-cat/qubes-core-admin-client/compare/1a2ce72..a17853b

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab