[pve-devel] [PATCH] firewall: resources: accept invalid ct state by default

[pve-devel] [PATCH] firewall: resources: accept invalid ct state by default

[Hannes Laimer]

We only add a `block-conntrack-invalid` jump to the in chain, if
the `nf_conntrack_allow_invalid` option is not set in the config. But we
already drop connections with an invalid ct state by default. So we have
to either allow connections with an invalid ct state by default, or explicitly
allow them when checking for the option and keeping them blocked by default.
I chose to change the 'default' as it has the same result but is
simpler a change.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
relevant code[1]
```
        if self.config.host().block_invalid_conntrack() {
            log::debug!("set block_invalid_conntrack");

            commands.push(Add::rule(AddRule::from_statement(
                chain_in,
                Statement::jump("block-conntrack-invalid"),
            )));
        }
```
I am not sure whether adding a jump to `block-conntrack-invalid` would
also make sense for the out chain.

[1] https://git.proxmox.com/?p=proxmox-firewall.git;a=blob;f=proxmox-firewall/src/firewall.rs;h=941aa2008b4e3a22d9b37a0f8bc39c3643eb97c8;hb=bea3e651b47f2a9113b93e45db2904ddfd2fe174#l394

 proxmox-firewall/resources/proxmox-firewall.nft | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/proxmox-firewall/resources/proxmox-firewall.nft b/proxmox-firewall/resources/proxmox-firewall.nft
index f42255c..be9a930 100644
--- a/proxmox-firewall/resources/proxmox-firewall.nft
+++ b/proxmox-firewall/resources/proxmox-firewall.nft
@@ -316,7 +316,7 @@ table bridge proxmox-firewall-guests {
     }
 
     chain pre-vm-out {
-        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : accept }
     }
 
     chain vm-out {
@@ -326,7 +326,7 @@ table bridge proxmox-firewall-guests {
     }
 
     chain pre-vm-in {
-        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
+        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : accept }
         meta protocol arp accept
     }
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH] firewall: resources: accept invalid ct state by default

[Stefan Hanreich]

On 11/15/24 13:33, Hannes Laimer wrote:
> We only add a `block-conntrack-invalid` jump to the in chain, if
> the `nf_conntrack_allow_invalid` option is not set in the config. But we
> already drop connections with an invalid ct state by default. So we have
> to either allow connections with an invalid ct state by default, or explicitly
> allow them when checking for the option and keeping them blocked by default.
> I chose to change the 'default' as it has the same result but is
> simpler a change.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> relevant code[1]
> ```
>         if self.config.host().block_invalid_conntrack() {
>             log::debug!("set block_invalid_conntrack");
> 
>             commands.push(Add::rule(AddRule::from_statement(
>                 chain_in,
>                 Statement::jump("block-conntrack-invalid"),
>             )));
>         }
> ```

This setting is for toggling invalid traffic on host level and works
because in the host default-in chain we have:

  ct state established,related accept

and then insert in the option-in chain

  jump block-conntrack-invalid

depending on the `nf_conntrack_allow_invalid` setting.


For VMs this isn't configurable, we always drop conntrack invalid traffic:
>  
>      chain pre-vm-out {
> -        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
> +        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : accept }
>      }
>  
>      chain vm-out {
> @@ -326,7 +326,7 @@ table bridge proxmox-firewall-guests {
>      }
>  
>      chain pre-vm-in {
> -        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : drop }
> +        meta protocol != arp ct state vmap { established : accept, related : accept, invalid : accept }
>          meta protocol arp accept
>      }
>  

This change has a problem though: It would *always* allow traffic with
ct state invalid for VMs.

I see two ways of solving this problem:

* We introduce a knob at VM level that lets you decide whether to drop
ct invalid traffic or not. (Invalid traffic would then still be
evaluated by the firewall rules if it's allowed in principle, as is the
case on host-level)

* We apply the host-level setting to VMs as well.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH] firewall: resources: accept invalid ct state by default

[Stefan Hanreich]

On 11/15/24 14:13, Stefan Hanreich wrote:
> I see two ways of solving this problem:
> 
> * We introduce a knob at VM level that lets you decide whether to drop
> ct invalid traffic or not. (Invalid traffic would then still be
> evaluated by the firewall rules if it's allowed in principle, as is the
> case on host-level)
> 
> * We apply the host-level setting to VMs as well.

The old firewall does it like this - so maybe we should do it here as well:

* drop invalid traffic in PVEFW-HOST-IN (= INPUT chain) irregardless
  of the setting
* drop invalid traffic on PVEFW-FORWARD (= FORWARD chain) if
  allow_invalid is 0 (= default)

It's important not to accept it immediately, because then the rest of
the ruleset still gets evaluated, mitigating the blast radius of this
setting considerably.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel