[pve-devel] [PATCH firewall] use only the top bit for our accept marks

[Wolfgang Bumiller]

This way we can let the remaining 31 bits be used by the
user.
Note that the routing decision has already been made when
these tables are being traversed, so the fwmark will not be
usable for routing rules (ip-rule(8)), but the mark can
still be used for other tasks such as traffic control (tc)
which happens on the outgoing interface.
---
Do we also want a `-mark` option on rules and/or a 'MARK' action?

 src/PVE/Firewall.pm          | 23 +++++++++++++++++------
 src/PVE/FirewallSimulator.pm | 12 ++++++++----
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index bda5ff9..052c8dc 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -515,10 +515,13 @@ my $pve_fw_macro_descr;
 my $pve_fw_macro_ipversion = {};
 my $pve_fw_preferred_macro_names = {};
 
+my $FWACCEPTMARK_ON  = "0x80000000/0x80000000";
+my $FWACCEPTMARK_OFF = "0x00000000/0x80000000";
+
 my $pve_std_chains = {};
 $pve_std_chains->{4} = {
     'PVEFW-SET-ACCEPT-MARK' => [
-	"-j MARK --set-mark 1",
+	"-j MARK --set-mark $FWACCEPTMARK_ON",
     ],
     'PVEFW-DropBroadcast' => [
 	# same as shorewall 'Broadcast'
@@ -605,7 +608,7 @@ $pve_std_chains->{4} = {
 
 $pve_std_chains->{6} = {
     'PVEFW-SET-ACCEPT-MARK' => [
-        "-j MARK --set-mark 1",
+        "-j MARK --set-mark $FWACCEPTMARK_ON",
     ],
     'PVEFW-DropBroadcast' => [
         # same as shorewall 'Broadcast'
@@ -1980,7 +1983,7 @@ sub ruleset_create_vm_chain {
 	if ($ipfilter_ipset) {
 	    ruleset_addrule($ruleset, $chain, "-m set ! --match-set $ipfilter_ipset src -j DROP");
 	}
-	ruleset_addrule($ruleset, $chain, "-j MARK --set-mark 0"); # clear mark
+	ruleset_addrule($ruleset, $chain, "-j MARK --set-mark $FWACCEPTMARK_OFF"); # clear mark
     }
 
     my $accept_action = $direction eq 'OUT' ? '-g PVEFW-SET-ACCEPT-MARK' : "-j $accept";
@@ -2004,7 +2007,7 @@ sub ruleset_add_group_rule {
 	ruleset_addrule($ruleset, $chain, "-j $group_chain");
     }
 
-    ruleset_addrule($ruleset, $chain, "-m mark --mark 1 -j $action");
+    ruleset_addrule($ruleset, $chain, "-m mark --mark $FWACCEPTMARK_ON -j $action");
 }
 
 sub ruleset_generate_vm_rules {
@@ -2259,7 +2262,7 @@ sub generate_group_rules {
     my $chain = "GROUP-${group}-IN";
 
     ruleset_create_chain($ruleset, $chain);
-    ruleset_addrule($ruleset, $chain, "-j MARK --set-mark 0"); # clear mark
+    ruleset_addrule($ruleset, $chain, "-j MARK --set-mark $FWACCEPTMARK_OFF"); # clear mark
 
     foreach my $rule (@$rules) {
 	next if $rule->{type} ne 'in';
@@ -2272,7 +2275,7 @@ sub generate_group_rules {
     $chain = "GROUP-${group}-OUT";
 
     ruleset_create_chain($ruleset, $chain);
-    ruleset_addrule($ruleset, $chain, "-j MARK --set-mark 0"); # clear mark
+    ruleset_addrule($ruleset, $chain, "-j MARK --set-mark $FWACCEPTMARK_OFF"); # clear mark
 
     foreach my $rule (@$rules) {
 	next if $rule->{type} ne 'out';
@@ -2291,6 +2294,14 @@ for (my $i = 0; $i < $MAX_NETS; $i++)  {
     $valid_netdev_names->{"net$i"} = 1;
 }
 
+sub get_mark_values {
+    my ($value, $mask) = @_;
+    $value = hex($value) if $value =~ /^0x/;
+    $mask = hex($mask) if defined($mask) && $mask =~ /^0x/;
+    $mask = 0xffffffff if !defined($mask);
+    return ($value, $mask);
+}
+
 sub parse_fw_rule {
     my ($prefix, $line, $cluster_conf, $fw_conf, $rule_env, $verbose) = @_;
 
diff --git a/src/PVE/FirewallSimulator.pm b/src/PVE/FirewallSimulator.pm
index 2e9bc38..c924b41 100644
--- a/src/PVE/FirewallSimulator.pm
+++ b/src/PVE/FirewallSimulator.pm
@@ -13,6 +13,8 @@ my $mark;
 my $trace;
 my $debug = 0;
 
+my $NUMBER_RE = qr/0x[0-9a-fA-F]+|\d+/;
+
 sub debug {
     my $new_value = shift;
 
@@ -211,15 +213,17 @@ sub rule_match {
 	    next;
 	}
 
-	if ($rule =~ s/^-m mark --mark (\d+)\s*//) {
-	    return undef if !defined($mark) || $mark != $1;
+	if ($rule =~ s@^-m mark --mark ($NUMBER_RE)(?:/($NUMBER_RE))?\s*@@) {
+	    my ($value, $mask) = PVE::Firewall::get_mark_values($1, $2);
+	    return undef if !defined($mark) || ($mark & $mask) != $value;
 	    next;
 	}
 
 	# final actions
 
-	if ($rule =~ s/^-j MARK --set-mark (\d+)\s*$//) {
-	    $mark = $1;
+	if ($rule =~ s@^-j MARK --set-mark ($NUMBER_RE)(?:/($NUMBER_RE))?\s*$@@) {
+	    my ($value, $mask) = PVE::Firewall::get_mark_values($1, $2);
+	    $mark = ($mark & ~$mask) | $value;
 	    return undef;
 	}
 
-- 
2.1.4