From 3a1a842dd56a7696f86465fb0ec00dc10781cb0a Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sat, 17 Jul 2021 21:42:55 +0200
Subject: [PATCH 01/20] Foundation for port forwarding in agent

---
 qubesagent/firewall.py | 54 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 952bcc1b0..f155f64a0 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -128,6 +128,25 @@ def read_rules(self, target):
         rules.append({'action': policy})
         return rules
 
+    def read_forward_rules(self, target):
+        """Read forward rules from QubesDB and return them as a list of dicts"""
+        """No policy here since they already are in the forward dict, use first/last flags"""
+        entries = self.qdb.multiread('/qubes-firewall-forward/')
+        assert isinstance(entries, dict)
+        # drop full path
+        entries = dict(((k.split('/')[3], v.decode())
+                        for k, v in entries.items()))
+        rules = []
+        for ruleno, rule in sorted(entries.items()):
+            if len(ruleno) != 4 or not ruleno.isdigit():
+                raise RuleParseError(
+                    'Unexpected non-rule found: {}={}'.format(ruleno, rule))
+            rule_dict = dict(elem.split('=') for elem in rule.split(' '))
+            if 'action' not in rule_dict:
+                raise RuleParseError('Rule \'{}\' lack action'.format(rule))
+            rules.append(rule_dict)
+        return rules
+
     def resolve_dns(self, fqdn, family):
         """
         Resolve the given FQDN via DNS.
@@ -178,8 +197,13 @@ def update_handled(self, addr):
         self.qdb.write('/qubes-firewall-handled/{}'.format(addr), str(cnt+1))
 
     def list_targets(self):
+        # here is 2 because we have /qubes-firewall/<ip>
         return set(t.split('/')[2] for t in self.qdb.list('/qubes-firewall/'))
 
+    def list_forward_targets(self):
+        # here is 3 because we have /qubes-firewall-forward/<appvm>/<ip>
+        return set(t.split('/')[3] for t in self.qdb.list('/qubes-firewall-forward/'))
+
     @staticmethod
     def is_ip6(addr):
         return addr.count(':') > 0
@@ -234,6 +258,31 @@ def handle_addr(self, addr):
 
         self.update_handled(addr)
 
+    def handle_forward_addr(self, addr):
+        # TODOTODOTODO
+        try:
+            rules = self.read_forward_rules(addr)
+            self.apply_rules(addr, rules)
+        except RuleParseError as e:
+            self.log_error(
+                'Failed to parse rules for {} ({}), blocking traffic'.format(
+                    addr, str(e)
+                ))
+            self.apply_rules(addr, [{'action': 'drop'}])
+        except RuleApplyError as e:
+            self.log_error(
+                'Failed to apply rules for {} ({}), blocking traffic'.format(
+                    addr, str(e))
+            )
+            # retry with fallback rules
+            try:
+                self.apply_rules(addr, [{'action': 'drop'}])
+            except RuleApplyError:
+                self.log_error(
+                    'Failed to block traffic for {}'.format(addr))
+
+        #self.update_handled(addr)
+
     @staticmethod
     def dns_addresses(family=None):
         with open('/etc/resolv.conf') as resolv:
@@ -252,11 +301,16 @@ def main(self):
         self.run_user_script()
         self.sd_notify('READY=1')
         self.qdb.watch('/qubes-firewall/')
+        self.qdb.watch('/qubes-firewall-forward/')
         self.qdb.watch('/connected-ips')
         self.qdb.watch('/connected-ips6')
         # initial load
         for source_addr in self.list_targets():
             self.handle_addr(source_addr)
+
+        for target_addr in self.list_forward_targets():
+            self.handle_forward_addr(target_addr)
+
         self.update_connected_ips(4)
         self.update_connected_ips(6)
         try:

From a4b0064bcd46d7b775edf83819f7e8d7cf2f09f0 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Tue, 27 Jul 2021 17:19:09 +0200
Subject: [PATCH 02/20] Started working on nft

---
 qubesagent/firewall.py | 186 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 177 insertions(+), 9 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index f155f64a0..b1907eb38 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -137,6 +137,9 @@ def read_forward_rules(self, target):
         entries = dict(((k.split('/')[3], v.decode())
                         for k, v in entries.items()))
         rules = []
+        if 'last' in entries:
+            entries.remove('last')
+            rules.append({'last': True})
         for ruleno, rule in sorted(entries.items()):
             if len(ruleno) != 4 or not ruleno.isdigit():
                 raise RuleParseError(
@@ -262,26 +265,20 @@ def handle_forward_addr(self, addr):
         # TODOTODOTODO
         try:
             rules = self.read_forward_rules(addr)
-            self.apply_rules(addr, rules)
+            self.apply_forward_rules(addr, rules)
+        
         except RuleParseError as e:
             self.log_error(
                 'Failed to parse rules for {} ({}), blocking traffic'.format(
                     addr, str(e)
                 ))
             self.apply_rules(addr, [{'action': 'drop'}])
+        
         except RuleApplyError as e:
             self.log_error(
                 'Failed to apply rules for {} ({}), blocking traffic'.format(
                     addr, str(e))
             )
-            # retry with fallback rules
-            try:
-                self.apply_rules(addr, [{'action': 'drop'}])
-            except RuleApplyError:
-                self.log_error(
-                    'Failed to block traffic for {}'.format(addr))
-
-        #self.update_handled(addr)
 
     @staticmethod
     def dns_addresses(family=None):
@@ -341,6 +338,9 @@ class IptablesWorker(FirewallWorker):
     supported_rule_opts = ['action', 'proto', 'dst4', 'dst6', 'dsthost',
                            'dstports', 'specialtarget', 'icmptype']
 
+    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'srcports',
+                           'dstports', 'forwardtype']
+
     def __init__(self):
         super(IptablesWorker, self).__init__()
         self.chains = {
@@ -500,6 +500,82 @@ def prepare_rules(self, chain, rules, family):
         iptables += 'COMMIT\n'
         return (iptables, ret_dns)
 
+def prepare_forward_rules(self, chain, rules, family, last=False):
+        """
+        Helper function to translate rules list into input for iptables-restore
+
+        :param chain: name of the chain to put rules into
+        :param rules: list of rules
+        :param family: address family (4 or 6)
+        :return: tuple: (input for iptables-restore, dict of DNS records resolved
+                        during execution)
+        :rtype: (str, dict)
+        """
+
+        iptables = "*filter\n"
+
+        fullmask = '/128' if family == 6 else '/32'
+
+        for rule in rules:
+            unsupported_opts = set(rule.keys()).difference(
+                set(self.supported_forward_rule_opts))
+            if unsupported_opts:
+                raise RuleParseError(
+                    'Unsupported forward rule option(s): {!s}'.format(unsupported_opts))
+            if 'src4' in rule and family == 6:
+                raise RuleParseError('IPv4 rule found for IPv6 address')
+            if 'src6' in rule and family == 4:
+                raise RuleParseError('src6 rule found for IPv4 address')
+
+            if 'proto' in rule:
+                protos = [rule['proto']]
+            else:
+                protos = None
+
+            if 'src4' in rule:
+                srchosts = [rule['src4']]
+            elif 'src6' in rule:
+                srchosts = [rule['src6']]
+            else:
+                srchosts = None
+
+            if 'dstports' in rule:
+                dstports = rule['dstports'].replace('-', ':')
+            else:
+                dstports = None
+
+            if 'srcports' in rule:
+                srcports = rule['srcports'].replace('-', ':')
+            else:
+                srcports = None
+
+            # make them iterable
+            if protos is None:
+                protos = [None]
+            if srchosts is None:
+                srchosts = [None]
+
+            if rule['action'] != 'forward':
+                raise RuleParseError(
+                    'Invalid rule action {}'.format(rule['action']))
+
+            # sorting here is only to ease writing tests
+            for proto in sorted(protos):
+                for dstport in sorted(dstports):
+                    ipt_rule = '-A {}'.format(chain)
+                    if dsthost is not None:
+                        ipt_rule += ' -d {}'.format(dsthost)
+                    if proto is not None:
+                        ipt_rule += ' -p {}'.format(proto)
+                    if dstports is not None:
+                        ipt_rule += ' --dport {}'.format(dstports)
+                    ipt_rule += ' -j {}\n'.format(action)
+                    iptables += ipt_rule
+    
+
+        iptables += 'COMMIT\n'
+        return (iptables, ret_dns)
+
     def apply_rules_family(self, source, rules, family):
         """
         Apply rules for given source address.
@@ -600,6 +676,9 @@ class NftablesWorker(FirewallWorker):
     supported_rule_opts = ['action', 'proto', 'dst4', 'dst6', 'dsthost',
                            'dstports', 'specialtarget', 'icmptype']
 
+    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'srcports',
+                           'dstports', 'forwardtype']
+
     def __init__(self):
         super(NftablesWorker, self).__init__()
         self.chains = {
@@ -801,6 +880,95 @@ def prepare_rules(self, chain, rules, family):
                 rules='\n   '.join(nft_rules)
             ), ret_dns)
 
+    def prepare_forward_rules(self, chain, rules, family):
+        """
+        Helper function to translate rules list into input for nft
+
+        :param chain: name of the chain to put rules into
+        :param rules: list of rules
+        :param family: address family (4 or 6)
+        :return: tuple: (input for nft, dict of DNS records resolved
+                        during execution)
+        :rtype: (str, dict)
+        """
+
+        assert family in (4, 6)
+        nft_rules = []
+        ip_match = 'ip6' if family == 6 else 'ip'
+
+        fullmask = '/128' if family == 6 else '/32'
+
+        for rule in rules:
+            unsupported_opts = set(rule.keys()).difference(
+                set(self.supported_forward_rule_opts))
+            if unsupported_opts:
+                raise RuleParseError(
+                    'Unsupported rule option(s): {!s}'.format(unsupported_opts))
+            if 'src4' in rule and family == 6:
+                raise RuleParseError('IPv4 rule found for IPv6 address')
+            if 'src6' in rule and family == 4:
+                raise RuleParseError('src6 rule found for IPv4 address')
+
+            nft_rule = ""
+            
+            if rule['action'] != 'forward':
+                raise RuleParseError(
+                    'Invalid rule action {}'.format(rule['action']))
+
+            if 'proto' in rule:
+                if family == 4:
+                    nft_rule += ' ip protocol {}'.format(rule['proto'])
+                elif family == 6:
+                    nft_rule += ' ip6 nexthdr {}'.format(rule['proto'])
+
+            if 'src4' in rule:
+                nft_rule += ' ip daddr {}'.format(rule['dst4'])
+            elif 'src6' in rule:
+                nft_rule += ' ip6 daddr {}'.format(rule['dst6'])
+            else:
+                raise RuleParseError(
+                    'Missing source address!')
+                
+
+            if 'dstports' in rule:
+                dstports = rule['dstports']
+                if len(set(dstports.split('-'))) == 1:
+                    dstports = dstports.split('-')[0]
+            else:
+                raise RuleParseError(
+                    'dstports needs to be defined!')
+
+            # now duplicate rules for tcp/udp if needed
+            # it isn't possible to specify "tcp dport xx || udp dport xx" in
+            # one rule
+            if dstports is not None:
+                if 'proto' not in rule:
+                    nft_rules.append(
+                        nft_rule + ' tcp dport {} {}'.format(
+                            dstports, action))
+                    nft_rules.append(
+                        nft_rule + ' udp dport {} {}'.format(
+                            dstports, action))
+                else:
+                    nft_rules.append(
+                        nft_rule + ' {} dport {} {}'.format(
+                            rule['proto'], dstports, action))
+            else:
+                nft_rules.append(nft_rule + ' ' + action)
+
+        return (
+            'flush chain {family} {table} {chain}\n'
+            'table {family} {table} {{\n'
+            '  chain {chain} {{\n'
+            '   {rules}\n'
+            '  }}\n'
+            '}}\n'.format(
+                family=('ip6' if family == 6 else 'ip'),
+                table='qubes-firewall',
+                chain=chain,
+                rules='\n   '.join(nft_rules)
+            ), ret_dns)
+
     def apply_rules_family(self, source, rules, family):
         """
         Apply rules for given source address.

From cc89c14296e8f57c9d036e6be727b9677d83dc82 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Fri, 6 Aug 2021 16:11:22 +0200
Subject: [PATCH 03/20] Generic additions, get_phys_interfaces()

---
 qubesagent/firewall.py | 47 +++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index b1907eb38..f2108e3e9 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -73,9 +73,18 @@ def cleanup(self):
         """Remove tables/chains - reverse work done by init"""
         raise NotImplementedError
 
-    def apply_rules(self, source_addr, rules):
-        """Apply rules in given source address"""
-        raise NotImplementedError
+    def apply_rules(self, source, rules):
+        if self.is_ip6(source):
+            self.apply_rules_family(source, rules, 6)
+        else:
+            self.apply_rules_family(source, rules, 4)
+
+    def apply_forward_rules(self, source, rules):
+        # what do we check here? 
+        if self.is_ip6(source):
+            self.apply_forward_rules_family(source, rules, 6)
+        else:
+            self.apply_forward_rules_family(source, rules, 4)
 
     def update_connected_ips(self, family):
         raise NotImplementedError
@@ -106,6 +115,14 @@ def run_user_script(self):
                 os.access(user_script_path, os.X_OK):
             subprocess.call([user_script_path])
 
+    def get_phys_interfaces():
+        phys = set()
+        with open('/proc/net/route') as f:
+            routes = f.readlines()[1:]
+        for route in routes:
+            phys.add(route.split('\t')[0])
+        return phys
+
     def read_rules(self, target):
         """Read rules from QubesDB and return them as a list of dicts"""
         entries = self.qdb.multiread('/qubes-firewall/{}/'.format(target))
@@ -137,9 +154,10 @@ def read_forward_rules(self, target):
         entries = dict(((k.split('/')[3], v.decode())
                         for k, v in entries.items()))
         rules = []
+        last =  False
         if 'last' in entries:
             entries.remove('last')
-            rules.append({'last': True})
+            last = True
         for ruleno, rule in sorted(entries.items()):
             if len(ruleno) != 4 or not ruleno.isdigit():
                 raise RuleParseError(
@@ -148,7 +166,7 @@ def read_forward_rules(self, target):
             if 'action' not in rule_dict:
                 raise RuleParseError('Rule \'{}\' lack action'.format(rule))
             rules.append(rule_dict)
-        return rules
+        return last, rules
 
     def resolve_dns(self, fqdn, family):
         """
@@ -262,10 +280,9 @@ def handle_addr(self, addr):
         self.update_handled(addr)
 
     def handle_forward_addr(self, addr):
-        # TODOTODOTODO
         try:
-            rules = self.read_forward_rules(addr)
-            self.apply_forward_rules(addr, rules)
+            last, rules = self.read_forward_rules(addr)
+            self.apply_forward_rules(addr, rules, last)
         
         except RuleParseError as e:
             self.log_error(
@@ -500,7 +517,7 @@ def prepare_rules(self, chain, rules, family):
         iptables += 'COMMIT\n'
         return (iptables, ret_dns)
 
-def prepare_forward_rules(self, chain, rules, family, last=False):
+def prepare_forward_rules(self, chain, rules, family):
         """
         Helper function to translate rules list into input for iptables-restore
 
@@ -604,12 +621,6 @@ def apply_rules_family(self, source, rules, family):
             raise RuleApplyError('\'iptables -F {}\' failed: {}'.format(
                 chain, e.output))
 
-    def apply_rules(self, source, rules):
-        if self.is_ip6(source):
-            self.apply_rules_family(source, rules, 6)
-        else:
-            self.apply_rules_family(source, rules, 4)
-
     def update_connected_ips(self, family):
         ips = self.get_connected_ips(family)
 
@@ -988,12 +999,6 @@ def apply_rules_family(self, source, rules, family):
         self.run_nft(nft)
         self.update_dns_info(source, dns)
 
-    def apply_rules(self, source, rules):
-        if self.is_ip6(source):
-            self.apply_rules_family(source, rules, 6)
-        else:
-            self.apply_rules_family(source, rules, 4)
-
     def init(self):
         nft_init = (
             'table {family} qubes-firewall {{\n'

From afabcd7a3d1b7ca28038ea2ab1afdaac9a9a4f45 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Mon, 9 Aug 2021 12:36:58 +0200
Subject: [PATCH 04/20] ip+name info in dict

---
 qubesagent/firewall.py | 43 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index f2108e3e9..17b8cea92 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -79,12 +79,12 @@ def apply_rules(self, source, rules):
         else:
             self.apply_rules_family(source, rules, 4)
 
-    def apply_forward_rules(self, source, rules):
+    def apply_forward_rules(self, target, rules, last):
         # what do we check here? 
-        if self.is_ip6(source):
-            self.apply_forward_rules_family(source, rules, 6)
+        if 'src6' in rules:
+            self.apply_forward_rules_family(target, rules, 6, last)
         else:
-            self.apply_forward_rules_family(source, rules, 4)
+            self.apply_forward_rules_family(target, rules, 4, last)
 
     def update_connected_ips(self, family):
         raise NotImplementedError
@@ -150,8 +150,8 @@ def read_forward_rules(self, target):
         """No policy here since they already are in the forward dict, use first/last flags"""
         entries = self.qdb.multiread('/qubes-firewall-forward/')
         assert isinstance(entries, dict)
-        # drop full path
-        entries = dict(((k.split('/')[3], v.decode())
+        # drop full path but add target ip info
+        entries = dict(((k.split('/')[2] + "/" + k.split('/')[3], v.decode())
                         for k, v in entries.items()))
         rules = []
         last =  False
@@ -517,7 +517,7 @@ def prepare_rules(self, chain, rules, family):
         iptables += 'COMMIT\n'
         return (iptables, ret_dns)
 
-def prepare_forward_rules(self, chain, rules, family):
+    def prepare_forward_rules(self, chain, rules, family, last):
         """
         Helper function to translate rules list into input for iptables-restore
 
@@ -621,6 +621,35 @@ def apply_rules_family(self, source, rules, family):
             raise RuleApplyError('\'iptables -F {}\' failed: {}'.format(
                 chain, e.output))
 
+    def apply_forward_rules_family(self, target, rules, family, last):
+        """
+        Apply forward rules for given target address.
+        Handle only rules for given address family (IPv4 or IPv6).
+
+        :param source: source address
+        :param rules: rules list
+        :param family: address family, either 4 or 6
+        :param last: whether this is the border vm or not
+        :return: None
+        """
+
+        chain = self.chain_for_addr(source)
+        if chain not in self.chains[family]:
+            self.create_chain(source, chain, family)
+
+        (iptables, dns) = self.prepare_rules(chain, rules, family)
+        try:
+            self.run_ipt(family, ['-F', chain])
+            p = self.run_ipt_restore(family, ['-n'])
+            (output, _) = p.communicate(iptables.encode())
+            if p.returncode != 0:
+                raise RuleApplyError(
+                    'iptables-restore failed: {}'.format(output))
+            self.update_dns_info(source, dns)
+        except subprocess.CalledProcessError as e:
+            raise RuleApplyError('\'iptables -F {}\' failed: {}'.format(
+                chain, e.output))
+
     def update_connected_ips(self, family):
         ips = self.get_connected_ips(family)
 

From 3d9efea79ac8a91a4240c5e47878da03a253ae80 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sat, 14 Aug 2021 11:18:57 +0200
Subject: [PATCH 05/20] firewall.py progress

---
 qubesagent/firewall.py | 51 ++++++++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 17b8cea92..544e0d440 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -81,7 +81,7 @@ def apply_rules(self, source, rules):
 
     def apply_forward_rules(self, target, rules, last):
         # what do we check here? 
-        if 'src6' in rules:
+        if self.is_ip6(target):
             self.apply_forward_rules_family(target, rules, 6, last)
         else:
             self.apply_forward_rules_family(target, rules, 4, last)
@@ -158,15 +158,24 @@ def read_forward_rules(self, target):
         if 'last' in entries:
             entries.remove('last')
             last = True
-        for ruleno, rule in sorted(entries.items()):
+        for rulename, rule in sorted(entries.items()):
+            ruleno = rulename.split('/')[1]
+            ruletarget = rulename.split('/')[0]
             if len(ruleno) != 4 or not ruleno.isdigit():
                 raise RuleParseError(
                     'Unexpected non-rule found: {}={}'.format(ruleno, rule))
             rule_dict = dict(elem.split('=') for elem in rule.split(' '))
             if 'action' not in rule_dict:
                 raise RuleParseError('Rule \'{}\' lack action'.format(rule))
+            if self.is_ip6(ruletarget):
+                rule_dict['dst6'] = ruletarget
+            else:
+                rule_dict['dst4'] = ruletarget
+            rule_dict['last'] = last
+            if ('dst4' in rule_dict and 'src6' in rule_dict) or ('dst6' in rule_dict and 'src4' in rule_dict):
+                raise RuleParseError('It is not possible to mix IPv4 and IPv6 Networking')
             rules.append(rule_dict)
-        return last, rules
+        return rules
 
     def resolve_dns(self, fqdn, family):
         """
@@ -281,8 +290,8 @@ def handle_addr(self, addr):
 
     def handle_forward_addr(self, addr):
         try:
-            last, rules = self.read_forward_rules(addr)
-            self.apply_forward_rules(addr, rules, last)
+            rules = self.read_forward_rules(addr)
+            self.apply_forward_rules(addr, rules)
         
         except RuleParseError as e:
             self.log_error(
@@ -355,8 +364,8 @@ class IptablesWorker(FirewallWorker):
     supported_rule_opts = ['action', 'proto', 'dst4', 'dst6', 'dsthost',
                            'dstports', 'specialtarget', 'icmptype']
 
-    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'srcports',
-                           'dstports', 'forwardtype']
+    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'dst4', 'dst6',
+                           'srcports', 'dstports', 'forwardtype']
 
     def __init__(self):
         super(IptablesWorker, self).__init__()
@@ -517,7 +526,7 @@ def prepare_rules(self, chain, rules, family):
         iptables += 'COMMIT\n'
         return (iptables, ret_dns)
 
-    def prepare_forward_rules(self, chain, rules, family, last):
+    def prepare_forward_rules(self, chain, rules, family):
         """
         Helper function to translate rules list into input for iptables-restore
 
@@ -540,9 +549,13 @@ def prepare_forward_rules(self, chain, rules, family, last):
                 raise RuleParseError(
                     'Unsupported forward rule option(s): {!s}'.format(unsupported_opts))
             if 'src4' in rule and family == 6:
-                raise RuleParseError('IPv4 rule found for IPv6 address')
+                raise RuleParseError('dst4 rule found for IPv6 address')
             if 'src6' in rule and family == 4:
                 raise RuleParseError('src6 rule found for IPv4 address')
+            if 'dst4' in rule and family == 6:
+                raise RuleParseError('dst4 rule found for IPv6 address')
+            if 'dst6' in rule and family == 4:
+                raise RuleParseError('dst6 rule found for IPv4 address')
 
             if 'proto' in rule:
                 protos = [rule['proto']]
@@ -556,11 +569,22 @@ def prepare_forward_rules(self, chain, rules, family, last):
             else:
                 srchosts = None
 
+            # dsthost here is added automatically in the previous functions
+            # it is always a /32
+            if 'dst4' in rule:
+                dsthost = [rule['dst4']]
+            elif 'dst6' in rule:
+                dsthost = [rule['dst6']]
+            else:
+                dsthost = None
+
             if 'dstports' in rule:
                 dstports = rule['dstports'].replace('-', ':')
             else:
                 dstports = None
 
+            # srcports cannot be a range
+            # a single port can be redirected to multiple ports, but not vice versa
             if 'srcports' in rule:
                 srcports = rule['srcports'].replace('-', ':')
             else:
@@ -621,7 +645,7 @@ def apply_rules_family(self, source, rules, family):
             raise RuleApplyError('\'iptables -F {}\' failed: {}'.format(
                 chain, e.output))
 
-    def apply_forward_rules_family(self, target, rules, family, last):
+    def apply_forward_rules_family(self, target, rules, family):
         """
         Apply forward rules for given target address.
         Handle only rules for given address family (IPv4 or IPv6).
@@ -637,7 +661,7 @@ def apply_forward_rules_family(self, target, rules, family, last):
         if chain not in self.chains[family]:
             self.create_chain(source, chain, family)
 
-        (iptables, dns) = self.prepare_rules(chain, rules, family)
+        iptables = self.prepare_forward_rules(chain, rules, family)
         try:
             self.run_ipt(family, ['-F', chain])
             p = self.run_ipt_restore(family, ['-n'])
@@ -645,7 +669,6 @@ def apply_forward_rules_family(self, target, rules, family, last):
             if p.returncode != 0:
                 raise RuleApplyError(
                     'iptables-restore failed: {}'.format(output))
-            self.update_dns_info(source, dns)
         except subprocess.CalledProcessError as e:
             raise RuleApplyError('\'iptables -F {}\' failed: {}'.format(
                 chain, e.output))
@@ -716,8 +739,8 @@ class NftablesWorker(FirewallWorker):
     supported_rule_opts = ['action', 'proto', 'dst4', 'dst6', 'dsthost',
                            'dstports', 'specialtarget', 'icmptype']
 
-    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'srcports',
-                           'dstports', 'forwardtype']
+    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'dst4'. 'dst6', 
+                            'srcports', 'dstports', 'forwardtype']
 
     def __init__(self):
         super(NftablesWorker, self).__init__()

From 39d4d7776a502171ce3b7ac9b69ef8174da6d6ba Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sat, 14 Aug 2021 14:30:13 +0200
Subject: [PATCH 06/20] Preliminary rules

---
 qubesagent/firewall.py | 87 ++++++++++++++++++++++++++++--------------
 1 file changed, 58 insertions(+), 29 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 544e0d440..47c4b0ec5 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -560,14 +560,14 @@ def prepare_forward_rules(self, chain, rules, family):
             if 'proto' in rule:
                 protos = [rule['proto']]
             else:
-                protos = None
+                protos = ['tcp', 'udp']
 
             if 'src4' in rule:
-                srchosts = [rule['src4']]
+                srchost = [rule['src4']]
             elif 'src6' in rule:
-                srchosts = [rule['src6']]
+                srchost = [rule['src6']]
             else:
-                srchosts = None
+                raise RuleParseError('src4/src6 is mandatory for forward rules')
 
             # dsthost here is added automatically in the previous functions
             # it is always a /32
@@ -578,44 +578,73 @@ def prepare_forward_rules(self, chain, rules, family):
             else:
                 dsthost = None
 
-            if 'dstports' in rule:
-                dstports = rule['dstports'].replace('-', ':')
-            else:
-                dstports = None
-
-            # srcports cannot be a range
-            # a single port can be redirected to multiple ports, but not vice versa
             if 'srcports' in rule:
                 srcports = rule['srcports'].replace('-', ':')
             else:
-                srcports = None
+                raise RuleParseError('srcports is mandatory for forward rules')
 
-            # make them iterable
-            if protos is None:
-                protos = [None]
-            if srchosts is None:
-                srchosts = [None]
+            # dstports cannot be a range
+            # a single port can be redirected to multiple ports, but not vice versa
+            if 'dstports' in rule:
+                if rule['dstports'].split('-')[0] !=  rule['dstports'].split('-')[1]:
+                    raise RuleParseError('dstports must be a single port')
+                dstport = rule['dstports'].split('-')[0]
+            else:
+                raise RuleParseError('dstports is mandatory for forward rules')
 
             if rule['action'] != 'forward':
                 raise RuleParseError(
                     'Invalid rule action {}'.format(rule['action']))
 
+            if rule['last']:
+                interfaces = self.get_phys_interfaces()
+                if len(intarfaces) < 1:
+                    raise RuleApplyError('There are no external interfaces available')
+                for iface in sorted(interfaces):
+                    for proto in sorted(protos):
+                        # first rule
+                        ipt_rule =  ' -t NAT'
+                        ipt_rule += ' -a PREROUTING'
+                        ipt_rule += ' -i {}'.format(iface)
+                        if proto is not None:
+                            ipt_rule += ' -p {}'.format(proto)
+                        if srcports is none None:
+                            ipt_rule += ' --dport {}'.format(srcports)
+                        ipt_rule += ' -j DNAT'
+                        if dsthost is not None:
+                            ipt_rule += ' --to-destination {}\n'.format(dsthost)
+                        iptables += ipt_rule
+
+                        # second rule
+                        ipt_rule =  ' -I FORWARD'
+                        ipt_rule += ' -i {}'.format(iface)
+                        if dsthost is not None:
+                            ipt_rule += ' -d {}'.format(dsthost)
+                        if proto is not None:
+                            ipt_rule += ' -p {}'.format(proto)
+                        if srcports is none None:
+                            ipt_rule += ' --dport {}'.format(srcports)
+                        ipt_rule += ' -m conntrack'
+                        ipt_rule += ' --cstate NEW'
+                        ipt_rule += ' -j ACCEPT\n'
+                        iptables += ipt_rule
+
+'''
             # sorting here is only to ease writing tests
             for proto in sorted(protos):
-                for dstport in sorted(dstports):
-                    ipt_rule = '-A {}'.format(chain)
-                    if dsthost is not None:
-                        ipt_rule += ' -d {}'.format(dsthost)
-                    if proto is not None:
-                        ipt_rule += ' -p {}'.format(proto)
-                    if dstports is not None:
-                        ipt_rule += ' --dport {}'.format(dstports)
-                    ipt_rule += ' -j {}\n'.format(action)
-                    iptables += ipt_rule
-    
+                ipt_rule = 
+                if dsthost is not None:
+                    ipt_rule += ' -d {}'.format(dsthost)
+                if proto is not None:
+                    ipt_rule += ' -p {}'.format(proto)
+                if dstports is not None:
+                    ipt_rule += ' --dport {}'.format(dstports)
+                ipt_rule += ' -j {}\n'.format(action)
+                iptables += ipt_rule
+    '''
 
         iptables += 'COMMIT\n'
-        return (iptables, ret_dns)
+        return iptables
 
     def apply_rules_family(self, source, rules, family):
         """

From 5d61eae1ed3f6453cbd261841a3a683c6e56a5b3 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sat, 14 Aug 2021 14:59:10 +0200
Subject: [PATCH 07/20] Fixed syntax errors

---
 qubesagent/firewall.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 47c4b0ec5..52203c9ff 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -159,8 +159,8 @@ def read_forward_rules(self, target):
             entries.remove('last')
             last = True
         for rulename, rule in sorted(entries.items()):
-            ruleno = rulename.split('/')[1]
-            ruletarget = rulename.split('/')[0]
+            ruleno = rulename.split('/')[2]
+            ruletarget = rulename.split('/')[1]
             if len(ruleno) != 4 or not ruleno.isdigit():
                 raise RuleParseError(
                     'Unexpected non-rule found: {}={}'.format(ruleno, rule))
@@ -608,7 +608,7 @@ def prepare_forward_rules(self, chain, rules, family):
                         ipt_rule += ' -i {}'.format(iface)
                         if proto is not None:
                             ipt_rule += ' -p {}'.format(proto)
-                        if srcports is none None:
+                        if srcports is None:
                             ipt_rule += ' --dport {}'.format(srcports)
                         ipt_rule += ' -j DNAT'
                         if dsthost is not None:
@@ -622,7 +622,7 @@ def prepare_forward_rules(self, chain, rules, family):
                             ipt_rule += ' -d {}'.format(dsthost)
                         if proto is not None:
                             ipt_rule += ' -p {}'.format(proto)
-                        if srcports is none None:
+                        if srcports is None:
                             ipt_rule += ' --dport {}'.format(srcports)
                         ipt_rule += ' -m conntrack'
                         ipt_rule += ' --cstate NEW'
@@ -641,8 +641,7 @@ def prepare_forward_rules(self, chain, rules, family):
                     ipt_rule += ' --dport {}'.format(dstports)
                 ipt_rule += ' -j {}\n'.format(action)
                 iptables += ipt_rule
-    '''
-
+'''
         iptables += 'COMMIT\n'
         return iptables
 
@@ -768,7 +767,7 @@ class NftablesWorker(FirewallWorker):
     supported_rule_opts = ['action', 'proto', 'dst4', 'dst6', 'dsthost',
                            'dstports', 'specialtarget', 'icmptype']
 
-    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'dst4'. 'dst6', 
+    supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'dst4', 'dst6', 
                             'srcports', 'dstports', 'forwardtype']
 
     def __init__(self):

From 3e944af2f52a5a0d312d50b4ec68f20051394fe4 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sat, 14 Aug 2021 18:23:53 +0200
Subject: [PATCH 08/20] working on nft

---
 qubesagent/firewall.py | 104 +++++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 46 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 52203c9ff..603b2cb39 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -79,12 +79,12 @@ def apply_rules(self, source, rules):
         else:
             self.apply_rules_family(source, rules, 4)
 
-    def apply_forward_rules(self, target, rules, last):
+    def apply_forward_rules(self, target, rules):
         # what do we check here? 
         if self.is_ip6(target):
-            self.apply_forward_rules_family(target, rules, 6, last)
+            self.apply_forward_rules_family(target, rules, 6)
         else:
-            self.apply_forward_rules_family(target, rules, 4, last)
+            self.apply_forward_rules_family(target, rules, 4)
 
     def update_connected_ips(self, family):
         raise NotImplementedError
@@ -151,16 +151,17 @@ def read_forward_rules(self, target):
         entries = self.qdb.multiread('/qubes-firewall-forward/')
         assert isinstance(entries, dict)
         # drop full path but add target ip info
-        entries = dict(((k.split('/')[2] + "/" + k.split('/')[3], v.decode())
+        entries = dict(((k.split('/')[3] + "/" + k.split('/')[4], v.decode())
                         for k, v in entries.items()))
         rules = []
         last =  False
-        if 'last' in entries:
-            entries.remove('last')
-            last = True
+
         for rulename, rule in sorted(entries.items()):
-            ruleno = rulename.split('/')[2]
-            ruletarget = rulename.split('/')[1]
+            ruleno = rulename.split('/')[1]
+            ruletarget = rulename.split('/')[0]
+            if ruleno == 'last':
+                last = True
+                continue
             if len(ruleno) != 4 or not ruleno.isdigit():
                 raise RuleParseError(
                     'Unexpected non-rule found: {}={}'.format(ruleno, rule))
@@ -365,7 +366,7 @@ class IptablesWorker(FirewallWorker):
                            'dstports', 'specialtarget', 'icmptype']
 
     supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'dst4', 'dst6',
-                           'srcports', 'dstports', 'forwardtype']
+                           'srcports', 'dstports', 'forwardtype', 'last']
 
     def __init__(self):
         super(IptablesWorker, self).__init__()
@@ -537,6 +538,7 @@ def prepare_forward_rules(self, chain, rules, family):
                         during execution)
         :rtype: (str, dict)
         """
+        assert family in (4, 6)
 
         iptables = "*filter\n"
 
@@ -629,19 +631,6 @@ def prepare_forward_rules(self, chain, rules, family):
                         ipt_rule += ' -j ACCEPT\n'
                         iptables += ipt_rule
 
-'''
-            # sorting here is only to ease writing tests
-            for proto in sorted(protos):
-                ipt_rule = 
-                if dsthost is not None:
-                    ipt_rule += ' -d {}'.format(dsthost)
-                if proto is not None:
-                    ipt_rule += ' -p {}'.format(proto)
-                if dstports is not None:
-                    ipt_rule += ' --dport {}'.format(dstports)
-                ipt_rule += ' -j {}\n'.format(action)
-                iptables += ipt_rule
-'''
         iptables += 'COMMIT\n'
         return iptables
 
@@ -768,7 +757,7 @@ class NftablesWorker(FirewallWorker):
                            'dstports', 'specialtarget', 'icmptype']
 
     supported_forward_rule_opts = ['action', 'proto', 'src4', 'src6', 'dst4', 'dst6', 
-                            'srcports', 'dstports', 'forwardtype']
+                            'srcports', 'dstports', 'forwardtype', 'last']
 
     def __init__(self):
         super(NftablesWorker, self).__init__()
@@ -971,7 +960,7 @@ def prepare_rules(self, chain, rules, family):
                 rules='\n   '.join(nft_rules)
             ), ret_dns)
 
-    def prepare_forward_rules(self, chain, rules, family):
+    def prepare_forward_rules(self, rules, family):
         """
         Helper function to translate rules list into input for nft
 
@@ -989,45 +978,55 @@ def prepare_forward_rules(self, chain, rules, family):
 
         fullmask = '/128' if family == 6 else '/32'
 
+        chain = 'forward'
+
         for rule in rules:
             unsupported_opts = set(rule.keys()).difference(
                 set(self.supported_forward_rule_opts))
             if unsupported_opts:
                 raise RuleParseError(
-                    'Unsupported rule option(s): {!s}'.format(unsupported_opts))
+                    'Unsupported forward rule option(s): {!s}'.format(unsupported_opts))
             if 'src4' in rule and family == 6:
-                raise RuleParseError('IPv4 rule found for IPv6 address')
+                raise RuleParseError('dst4 rule found for IPv6 address')
             if 'src6' in rule and family == 4:
                 raise RuleParseError('src6 rule found for IPv4 address')
-
+            if 'dst4' in rule and family == 6:
+                raise RuleParseError('dst4 rule found for IPv6 address')
+            if 'dst6' in rule and family == 4:
+                raise RuleParseError('dst6 rule found for IPv4 address')
             nft_rule = ""
             
-            if rule['action'] != 'forward':
-                raise RuleParseError(
-                    'Invalid rule action {}'.format(rule['action']))
-
             if 'proto' in rule:
                 if family == 4:
                     nft_rule += ' ip protocol {}'.format(rule['proto'])
                 elif family == 6:
                     nft_rule += ' ip6 nexthdr {}'.format(rule['proto'])
 
-            if 'src4' in rule:
+            if 'dst4' in rule:
                 nft_rule += ' ip daddr {}'.format(rule['dst4'])
-            elif 'src6' in rule:
+            elif 'dst6' in rule:
                 nft_rule += ' ip6 daddr {}'.format(rule['dst6'])
             else:
                 raise RuleParseError(
-                    'Missing source address!')
-                
+                    'Missing dst address!')
 
+            if 'srcports' in rule:
+                srcports = rule['srcports'].replace('-', ':')
+            else:
+                raise RuleParseError('srcports is mandatory for forward rules')
+
+            # dstports cannot be a range
+            # a single port can be redirected to multiple ports, but not vice versa
             if 'dstports' in rule:
-                dstports = rule['dstports']
-                if len(set(dstports.split('-'))) == 1:
-                    dstports = dstports.split('-')[0]
+                if rule['dstports'].split('-')[0] !=  rule['dstports'].split('-')[1]:
+                    raise RuleParseError('dstports must be a single port')
+                dstport = rule['dstports'].split('-')[0]
             else:
+                raise RuleParseError('dstports is mandatory for forward rules')
+
+            if rule['action'] != 'forward':
                 raise RuleParseError(
-                    'dstports needs to be defined!')
+                    'Invalid rule action {}'.format(rule['action']))
 
             # now duplicate rules for tcp/udp if needed
             # it isn't possible to specify "tcp dport xx || udp dport xx" in
@@ -1036,19 +1035,18 @@ def prepare_forward_rules(self, chain, rules, family):
                 if 'proto' not in rule:
                     nft_rules.append(
                         nft_rule + ' tcp dport {} {}'.format(
-                            dstports, action))
+                            dstport, action))
                     nft_rules.append(
                         nft_rule + ' udp dport {} {}'.format(
-                            dstports, action))
+                            dstport, action))
                 else:
                     nft_rules.append(
                         nft_rule + ' {} dport {} {}'.format(
-                            rule['proto'], dstports, action))
+                            rule['proto'], dstport, action))
             else:
                 nft_rules.append(nft_rule + ' ' + action)
 
-        return (
-            'flush chain {family} {table} {chain}\n'
+        '''return 'flush chain {family} {table} {chain}\n'
             'table {family} {table} {{\n'
             '  chain {chain} {{\n'
             '   {rules}\n'
@@ -1058,7 +1056,7 @@ def prepare_forward_rules(self, chain, rules, family):
                 table='qubes-firewall',
                 chain=chain,
                 rules='\n   '.join(nft_rules)
-            ), ret_dns)
+            )'''
 
     def apply_rules_family(self, source, rules, family):
         """
@@ -1079,6 +1077,20 @@ def apply_rules_family(self, source, rules, family):
         self.run_nft(nft)
         self.update_dns_info(source, dns)
 
+    def apply_forward_rules_family(self, target, rules, family):
+        """
+        Apply rules for given source address.
+        Handle only rules for given address family (IPv4 or IPv6).
+
+        :param source: source address
+        :param rules: rules list
+        :param family: address family, either 4 or 6
+        :return: None
+        """
+
+        nft = self.prepare_forward_rules(rules, family)
+        self.run_nft(nft)
+
     def init(self):
         nft_init = (
             'table {family} qubes-firewall {{\n'

From 2e574fa1c7c967fba728833aa3aaf772b8d19244 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sun, 15 Aug 2021 15:17:43 +0200
Subject: [PATCH 09/20] Added nft external rules support and chain creation

---
 qubesagent/firewall.py | 108 ++++++++++++++++++++++++++++-------------
 1 file changed, 75 insertions(+), 33 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 603b2cb39..c8ac06f3b 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -115,12 +115,13 @@ def run_user_script(self):
                 os.access(user_script_path, os.X_OK):
             subprocess.call([user_script_path])
 
-    def get_phys_interfaces():
+    def get_phys_interfaces(self):
         phys = set()
         with open('/proc/net/route') as f:
             routes = f.readlines()[1:]
         for route in routes:
-            phys.add(route.split('\t')[0])
+            if 'vif' not in route:
+                phys.add(route.split('\t')[0])
         return phys
 
     def read_rules(self, target):
@@ -154,14 +155,10 @@ def read_forward_rules(self, target):
         entries = dict(((k.split('/')[3] + "/" + k.split('/')[4], v.decode())
                         for k, v in entries.items()))
         rules = []
-        last =  False
 
         for rulename, rule in sorted(entries.items()):
             ruleno = rulename.split('/')[1]
             ruletarget = rulename.split('/')[0]
-            if ruleno == 'last':
-                last = True
-                continue
             if len(ruleno) != 4 or not ruleno.isdigit():
                 raise RuleParseError(
                     'Unexpected non-rule found: {}={}'.format(ruleno, rule))
@@ -172,7 +169,6 @@ def read_forward_rules(self, target):
                 rule_dict['dst6'] = ruletarget
             else:
                 rule_dict['dst4'] = ruletarget
-            rule_dict['last'] = last
             if ('dst4' in rule_dict and 'src6' in rule_dict) or ('dst6' in rule_dict and 'src4' in rule_dict):
                 raise RuleParseError('It is not possible to mix IPv4 and IPv6 Networking')
             rules.append(rule_dict)
@@ -808,6 +804,25 @@ def create_chain(self, addr, chain, family):
         self.run_nft(nft_input)
         self.chains[family].add(chain)
 
+    def create_forward_chain(self, family):
+        """
+        Create a forwarding chain using nat for forwarding rules
+        """
+        nft_input = (
+            'table {family} qubes-firewall-forward {{\n'
+            '  chain postrouting {{\n'
+            '    type nat hook postrouting priority srcnat; policy accept;\n'
+            '    masquerade;\n'
+            '  }}\n'
+            '  chain prerouting {{\n'
+            '    type nat hook prerouting priority dstnat; policy accept;\n'
+            '  }}\n'
+            '}}\n').format(
+                family=("ip6" if family == 6 else "ip")
+        )
+        self.run_nft(nft_input)
+        self.chains[family].add('qubes-firewall-forward')
+
     def update_connected_ips(self, family):
         family_name = ('ip6' if family == 6 else 'ip')
         table = 'qubes-firewall'
@@ -973,13 +988,15 @@ def prepare_forward_rules(self, rules, family):
         """
 
         assert family in (4, 6)
-        nft_rules = []
         ip_match = 'ip6' if family == 6 else 'ip'
 
         fullmask = '/128' if family == 6 else '/32'
 
         chain = 'forward'
 
+        forward_nft_rules = []
+        accept_nft_rules = []
+
         for rule in rules:
             unsupported_opts = set(rule.keys()).difference(
                 set(self.supported_forward_rule_opts))
@@ -997,21 +1014,31 @@ def prepare_forward_rules(self, rules, family):
             nft_rule = ""
             
             if 'proto' in rule:
-                if family == 4:
-                    nft_rule += ' ip protocol {}'.format(rule['proto'])
-                elif family == 6:
-                    nft_rule += ' ip6 nexthdr {}'.format(rule['proto'])
+                protos = [rule['proto']]
+            else:
+                protos = ['tcp', 'udp']
 
             if 'dst4' in rule:
-                nft_rule += ' ip daddr {}'.format(rule['dst4'])
+                dsthost = rule['dst4']
             elif 'dst6' in rule:
-                nft_rule += ' ip6 daddr {}'.format(rule['dst6'])
+                dsthost = rule['dst6']
+            else:
+                raise RuleParseError(
+                    'Missing dst address!')
+
+            if 'src4' in rule:
+                srchosts = rule['src4']
+            elif 'dst6' in rule:
+                srchosts = rule['src6']
             else:
                 raise RuleParseError(
                     'Missing dst address!')
 
             if 'srcports' in rule:
-                srcports = rule['srcports'].replace('-', ':')
+                if rule['srcports'].split('-')[0] ==  rule['srcports'].split('-')[1]:
+                    srcports = rule['srcports'].split('-')[0]
+                else:
+                    srcports = rule['srcports']
             else:
                 raise RuleParseError('srcports is mandatory for forward rules')
 
@@ -1031,32 +1058,44 @@ def prepare_forward_rules(self, rules, family):
             # now duplicate rules for tcp/udp if needed
             # it isn't possible to specify "tcp dport xx || udp dport xx" in
             # one rule
-            if dstports is not None:
-                if 'proto' not in rule:
-                    nft_rules.append(
-                        nft_rule + ' tcp dport {} {}'.format(
-                            dstport, action))
-                    nft_rules.append(
-                        nft_rule + ' udp dport {} {}'.format(
-                            dstport, action))
-                else:
-                    nft_rules.append(
-                        nft_rule + ' {} dport {} {}'.format(
-                            rule['proto'], dstport, action))
+
+            if 'last' in rule and rule['last']:
+                interfaces = self.get_phys_interfaces()
+                if len(interfaces) < 1:
+                    raise RuleApplyError('There are no external interfaces available')
+                for iface in sorted(interfaces):
+                    for proto in sorted(protos):
+                        forward_nft_rules.append('meta iifname "{iface}" {family} saddr {srchosts} {proto} dport {{ {srcports} }} dnat to {dsthost}:{dstport}'.format(iface=iface, family=ip_match, srchosts=srchosts, proto=proto, srcports=srcports, dsthost=dsthost, dstport=dstport))
+                        accept_nft_rules.append('meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept').format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
             else:
-                nft_rules.append(nft_rule + ' ' + action)
+                test = 1
 
-        '''return 'flush chain {family} {table} {chain}\n'
+        forward_rule = (
+            'flush chain {family} {table} {chain}\n'
             'table {family} {table} {{\n'
             '  chain {chain} {{\n'
             '   {rules}\n'
             '  }}\n'
             '}}\n'.format(
-                family=('ip6' if family == 6 else 'ip'),
+                family=ip_match,
+                table='qubes-firewall-forward',
+                chain='prerouting',
+                rules='\n   '.join(forward_nft_rules)
+            ))
+
+        accept_rule = (
+            'table {family} {table} {{\n'
+            '  chain {chain} {{\n'
+            '    {rules}\n'
+            '  }}\n'
+            '}}\n'.format(
+                family=ip_match,
                 table='qubes-firewall',
-                chain=chain,
-                rules='\n   '.join(nft_rules)
-            )'''
+                chain='forward',
+                rules='\n   '.join(accept_nft_rules)
+            ))
+        return forward_rule + accept_rule
+
 
     def apply_rules_family(self, source, rules, family):
         """
@@ -1088,6 +1127,9 @@ def apply_forward_rules_family(self, target, rules, family):
         :return: None
         """
 
+        if 'qubes-firewall-forward' not in self.chains[family]:
+            self.create_forward_chain(family)
+
         nft = self.prepare_forward_rules(rules, family)
         self.run_nft(nft)
 

From 0e564c721cce5ef19b940100c5e7c06a53c60a61 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sun, 15 Aug 2021 15:37:54 +0200
Subject: [PATCH 10/20] nft rules v2

---
 qubesagent/firewall.py | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index c8ac06f3b..795257535 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -1034,6 +1034,7 @@ def prepare_forward_rules(self, rules, family):
                 raise RuleParseError(
                     'Missing dst address!')
 
+            # if the range is zero nft complains, otherwise a range is ok
             if 'srcports' in rule:
                 if rule['srcports'].split('-')[0] ==  rule['srcports'].split('-')[1]:
                     srcports = rule['srcports'].split('-')[0]
@@ -1043,7 +1044,7 @@ def prepare_forward_rules(self, rules, family):
                 raise RuleParseError('srcports is mandatory for forward rules')
 
             # dstports cannot be a range
-            # a single port can be redirected to multiple ports, but not vice versa
+            # multiple ports can be redirected to a single port, but not vice versa
             if 'dstports' in rule:
                 if rule['dstports'].split('-')[0] !=  rule['dstports'].split('-')[1]:
                     raise RuleParseError('dstports must be a single port')
@@ -1055,20 +1056,21 @@ def prepare_forward_rules(self, rules, family):
                 raise RuleParseError(
                     'Invalid rule action {}'.format(rule['action']))
 
-            # now duplicate rules for tcp/udp if needed
-            # it isn't possible to specify "tcp dport xx || udp dport xx" in
-            # one rule
-
             if 'last' in rule and rule['last']:
+                # is this the outside facing qubes?
                 interfaces = self.get_phys_interfaces()
                 if len(interfaces) < 1:
                     raise RuleApplyError('There are no external interfaces available')
                 for iface in sorted(interfaces):
                     for proto in sorted(protos):
                         forward_nft_rules.append('meta iifname "{iface}" {family} saddr {srchosts} {proto} dport {{ {srcports} }} dnat to {dsthost}:{dstport}'.format(iface=iface, family=ip_match, srchosts=srchosts, proto=proto, srcports=srcports, dsthost=dsthost, dstport=dstport))
-                        accept_nft_rules.append('meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept').format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
+                        accept_nft_rules.append('meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
             else:
-                test = 1
+                # here should we limit srchost to the previous hop? if yes how do we gain knowledge of its ip?
+                # internal we always use the dstport for communication between qubes. Maybe it is worth randomizing at a later stage?
+                for proto in sorted(protos):
+                    forward_nft_rules.append('meta iifname "eth0" {family} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, proto=proto, srcports=srcports, dstport=dstport))
+                    accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
 
         forward_rule = (
             'flush chain {family} {table} {chain}\n'

From 192605bc0fdd289ab6592d7eed675f613d14a4e6 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sun, 15 Aug 2021 16:45:00 +0200
Subject: [PATCH 11/20] nft fixes

---
 qubesagent/firewall.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 795257535..7ec1b7733 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -124,6 +124,12 @@ def get_phys_interfaces(self):
                 phys.add(route.split('\t')[0])
         return phys
 
+    def get_ip(self):
+         return self.qdb.read('/qubes-ip').decode()
+
+    def get_gateway(self):
+         return self.qdb.read('/qubes-gateway').decode()
+
     def read_rules(self, target):
         """Read rules from QubesDB and return them as a list of dicts"""
         entries = self.qdb.multiread('/qubes-firewall/{}/'.format(target))
@@ -1066,10 +1072,10 @@ def prepare_forward_rules(self, rules, family):
                         forward_nft_rules.append('meta iifname "{iface}" {family} saddr {srchosts} {proto} dport {{ {srcports} }} dnat to {dsthost}:{dstport}'.format(iface=iface, family=ip_match, srchosts=srchosts, proto=proto, srcports=srcports, dsthost=dsthost, dstport=dstport))
                         accept_nft_rules.append('meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
             else:
-                # here should we limit srchost to the previous hop? if yes how do we gain knowledge of its ip?
+                # here should we limit srchost to the previous hop? if yes how do we gain knowledge of its ip? get_gateway and get_ip try to address this
                 # internal we always use the dstport for communication between qubes. Maybe it is worth randomizing at a later stage?
                 for proto in sorted(protos):
-                    forward_nft_rules.append('meta iifname "eth0" {family} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, proto=proto, srcports=srcports, dstport=dstport))
+                    forward_nft_rules.append('meta iifname "eth0" {family} saddr {gateway} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, gateway=self.get_gateway(), proto=proto, dsthost=dsthost, dstport=dstport))
                     accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
 
         forward_rule = (

From fb7276ad4fe29ce6233b94f19e7a9ba0d9785f61 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sun, 15 Aug 2021 17:03:41 +0200
Subject: [PATCH 12/20] fixed flush confusion

---
 qubesagent/firewall.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 7ec1b7733..20e7e9841 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -815,6 +815,8 @@ def create_forward_chain(self, family):
         Create a forwarding chain using nat for forwarding rules
         """
         nft_input = (
+            'flush chain {family} qubes-firewall-forward prerouting\n'
+            'flush chain {family} qubes-firewall-forward postrouting\n'
             'table {family} qubes-firewall-forward {{\n'
             '  chain postrouting {{\n'
             '    type nat hook postrouting priority srcnat; policy accept;\n'
@@ -847,6 +849,7 @@ def update_connected_ips(self, family):
                 family_name=family_name, addr=addr)
 
             nft_input += (
+                ''
                 'table {family_name} {table} {{\n'
                 '  chain prerouting {{\n'
                 '    {irule}'
@@ -1079,7 +1082,6 @@ def prepare_forward_rules(self, rules, family):
                     accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
 
         forward_rule = (
-            'flush chain {family} {table} {chain}\n'
             'table {family} {table} {{\n'
             '  chain {chain} {{\n'
             '   {rules}\n'

From fdff33312197179e7a73091a5c78b64b942c3301 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Sun, 15 Aug 2021 20:04:24 +0200
Subject: [PATCH 13/20] Add source address handling for internal rules

---
 qubesagent/firewall.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 20e7e9841..75f8cba82 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -1077,8 +1077,14 @@ def prepare_forward_rules(self, rules, family):
             else:
                 # here should we limit srchost to the previous hop? if yes how do we gain knowledge of its ip? get_gateway and get_ip try to address this
                 # internal we always use the dstport for communication between qubes. Maybe it is worth randomizing at a later stage?
+                if 'forwardtype' in rule and rule['forwardtype'] == 'internal':
+                    srchost = "{}/24".format(self.get_ip())
+                elif 'forwardtype' in rule and rule['forwardtype'] == 'external':
+                    srchost = self.get_gateway()
+                else:
+                    raise RuleParseError('Invalid forwardtype {}'.format(rule['forwardtype']))
                 for proto in sorted(protos):
-                    forward_nft_rules.append('meta iifname "eth0" {family} saddr {gateway} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, gateway=self.get_gateway(), proto=proto, dsthost=dsthost, dstport=dstport))
+                    forward_nft_rules.append('meta iifname "eth0" {family} saddr {srchost} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, srchost=srchost, proto=proto, dsthost=dsthost, dstport=dstport))
                     accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
 
         forward_rule = (

From a602e093cd4024859c8b7e91c119b99d30c96628 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Mon, 16 Aug 2021 22:51:10 +0200
Subject: [PATCH 14/20] Since srchost is always mandatory, it is better to use
 it even for internal fwd

---
 qubesagent/firewall.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 75f8cba82..1b2aa49ad 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -1077,12 +1077,6 @@ def prepare_forward_rules(self, rules, family):
             else:
                 # here should we limit srchost to the previous hop? if yes how do we gain knowledge of its ip? get_gateway and get_ip try to address this
                 # internal we always use the dstport for communication between qubes. Maybe it is worth randomizing at a later stage?
-                if 'forwardtype' in rule and rule['forwardtype'] == 'internal':
-                    srchost = "{}/24".format(self.get_ip())
-                elif 'forwardtype' in rule and rule['forwardtype'] == 'external':
-                    srchost = self.get_gateway()
-                else:
-                    raise RuleParseError('Invalid forwardtype {}'.format(rule['forwardtype']))
                 for proto in sorted(protos):
                     forward_nft_rules.append('meta iifname "eth0" {family} saddr {srchost} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, srchost=srchost, proto=proto, dsthost=dsthost, dstport=dstport))
                     accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))

From 21a71b7201721b3a33ecb2931e99d2fa856ec9d9 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Tue, 17 Aug 2021 19:03:35 +0200
Subject: [PATCH 15/20] removed masquerading in forward chain, fixed original
 srchost check

---
 qubesagent/firewall.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 1b2aa49ad..4bc4f28c0 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -820,7 +820,6 @@ def create_forward_chain(self, family):
             'table {family} qubes-firewall-forward {{\n'
             '  chain postrouting {{\n'
             '    type nat hook postrouting priority srcnat; policy accept;\n'
-            '    masquerade;\n'
             '  }}\n'
             '  chain prerouting {{\n'
             '    type nat hook prerouting priority dstnat; policy accept;\n'
@@ -1070,15 +1069,16 @@ def prepare_forward_rules(self, rules, family):
                 interfaces = self.get_phys_interfaces()
                 if len(interfaces) < 1:
                     raise RuleApplyError('There are no external interfaces available')
+                
                 for iface in sorted(interfaces):
                     for proto in sorted(protos):
                         forward_nft_rules.append('meta iifname "{iface}" {family} saddr {srchosts} {proto} dport {{ {srcports} }} dnat to {dsthost}:{dstport}'.format(iface=iface, family=ip_match, srchosts=srchosts, proto=proto, srcports=srcports, dsthost=dsthost, dstport=dstport))
                         accept_nft_rules.append('meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
             else:
-                # here should we limit srchost to the previous hop? if yes how do we gain knowledge of its ip? get_gateway and get_ip try to address this
                 # internal we always use the dstport for communication between qubes. Maybe it is worth randomizing at a later stage?
+                # since we removed masquerading we can retain the original srchost for filtering
                 for proto in sorted(protos):
-                    forward_nft_rules.append('meta iifname "eth0" {family} saddr {srchost} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, srchost=srchost, proto=proto, dsthost=dsthost, dstport=dstport))
+                    forward_nft_rules.append('meta iifname "eth0" {family} saddr {srchosts} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, srchosts=srchosts, proto=proto, dsthost=dsthost, dstport=dstport))
                     accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
 
         forward_rule = (

From 7119d2ee797b1e2d106419f66a4d18b273d8665f Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Thu, 19 Aug 2021 23:44:03 +0200
Subject: [PATCH 16/20] Updated watch mechanism for forwarding

---
 qubesagent/firewall.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 4bc4f28c0..9f938e011 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -334,8 +334,7 @@ def main(self):
         for source_addr in self.list_targets():
             self.handle_addr(source_addr)
 
-        for target_addr in self.list_forward_targets():
-            self.handle_forward_addr(target_addr)
+        self.load_forwarding()
 
         self.update_connected_ips(4)
         self.update_connected_ips(6)
@@ -353,16 +352,24 @@ def main(self):
                     source_addr = watch_path.split('/')[2]
                     self.handle_addr(source_addr)
 
+                if watch_path.startswith('/qubes-firewall-forward/'):
+                    self.load_forwarding()
+
         except OSError:  # EINTR
             # signal received, don't continue the loop
             pass
 
         self.cleanup()
 
+    def load_forwarding(self):
+        for target_addr in self.list_forward_targets():
+            self.handle_forward_addr(target_addr)
+
     def terminate(self):
         self.terminate_requested = True
 
 
+
 class IptablesWorker(FirewallWorker):
     supported_rule_opts = ['action', 'proto', 'dst4', 'dst6', 'dsthost',
                            'dstports', 'specialtarget', 'icmptype']
@@ -1170,6 +1177,8 @@ def cleanup(self):
         nft_cleanup = (
             'delete table ip qubes-firewall\n'
             'delete table ip6 qubes-firewall\n'
+            'delete table ip qubes-firewall-forward\n'
+            'delete table ip6 qubes-firewall-forward\n'
         )
         self.run_nft(nft_cleanup)
 

From adc4e17d7e3146dcb60344ed2ee54c3191c6a1e6 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Fri, 20 Aug 2021 01:03:26 +0200
Subject: [PATCH 17/20] Apply forward rules altogether, removed leftover code,
 add family detection

---
 qubesagent/firewall.py | 102 ++++++++++++++++++++++++-----------------
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 9f938e011..8ea128a1e 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -82,9 +82,9 @@ def apply_rules(self, source, rules):
     def apply_forward_rules(self, target, rules):
         # what do we check here? 
         if self.is_ip6(target):
-            self.apply_forward_rules_family(target, rules, 6)
+            self.apply_forward_rules_family(rules, 6)
         else:
-            self.apply_forward_rules_family(target, rules, 4)
+            self.apply_forward_rules_family(rules, 4)
 
     def update_connected_ips(self, family):
         raise NotImplementedError
@@ -152,7 +152,7 @@ def read_rules(self, target):
         rules.append({'action': policy})
         return rules
 
-    def read_forward_rules(self, target):
+    def read_forward_rules(self):
         """Read forward rules from QubesDB and return them as a list of dicts"""
         """No policy here since they already are in the forward dict, use first/last flags"""
         entries = self.qdb.multiread('/qubes-firewall-forward/')
@@ -291,24 +291,6 @@ def handle_addr(self, addr):
 
         self.update_handled(addr)
 
-    def handle_forward_addr(self, addr):
-        try:
-            rules = self.read_forward_rules(addr)
-            self.apply_forward_rules(addr, rules)
-        
-        except RuleParseError as e:
-            self.log_error(
-                'Failed to parse rules for {} ({}), blocking traffic'.format(
-                    addr, str(e)
-                ))
-            self.apply_rules(addr, [{'action': 'drop'}])
-        
-        except RuleApplyError as e:
-            self.log_error(
-                'Failed to apply rules for {} ({}), blocking traffic'.format(
-                    addr, str(e))
-            )
-
     @staticmethod
     def dns_addresses(family=None):
         with open('/etc/resolv.conf') as resolv:
@@ -360,10 +342,28 @@ def main(self):
             pass
 
         self.cleanup()
+        if 'qubes-firewall-forward' in self.chains['4'] or 'qubes-firewall-forward' in self.chains['6']:
+            self.forward_cleanup()
 
     def load_forwarding(self):
-        for target_addr in self.list_forward_targets():
-            self.handle_forward_addr(target_addr)
+        # the only reason for loading the forwarding rules this way is to get the ip and choose the correct family
+        # the address is discared aftern the correct family is chosen
+        try:
+            rules = self.read_forward_rules()
+            self.apply_forward_rules(rules)
+        
+        except RuleParseError as e:
+            self.log_error(
+                'Failed to parse rules for {} ({}), blocking traffic'.format(
+                    addr, str(e)
+                ))
+            self.apply_rules(addr, [{'action': 'drop'}])
+        
+        except RuleApplyError as e:
+            self.log_error(
+                'Failed to apply rules for {} ({}), blocking traffic'.format(
+                    addr, str(e))
+            )
 
     def terminate(self):
         self.terminate_requested = True
@@ -671,7 +671,7 @@ def apply_rules_family(self, source, rules, family):
             raise RuleApplyError('\'iptables -F {}\' failed: {}'.format(
                 chain, e.output))
 
-    def apply_forward_rules_family(self, target, rules, family):
+    def apply_forward_rules_family(self, rules, family):
         """
         Apply forward rules for given target address.
         Handle only rules for given address family (IPv4 or IPv6).
@@ -990,7 +990,7 @@ def prepare_rules(self, chain, rules, family):
                 rules='\n   '.join(nft_rules)
             ), ret_dns)
 
-    def prepare_forward_rules(self, rules, family):
+    def prepare_forward_rules(self, rules):
         """
         Helper function to translate rules list into input for nft
 
@@ -1002,11 +1002,6 @@ def prepare_forward_rules(self, rules, family):
         :rtype: (str, dict)
         """
 
-        assert family in (4, 6)
-        ip_match = 'ip6' if family == 6 else 'ip'
-
-        fullmask = '/128' if family == 6 else '/32'
-
         chain = 'forward'
 
         forward_nft_rules = []
@@ -1018,14 +1013,6 @@ def prepare_forward_rules(self, rules, family):
             if unsupported_opts:
                 raise RuleParseError(
                     'Unsupported forward rule option(s): {!s}'.format(unsupported_opts))
-            if 'src4' in rule and family == 6:
-                raise RuleParseError('dst4 rule found for IPv6 address')
-            if 'src6' in rule and family == 4:
-                raise RuleParseError('src6 rule found for IPv4 address')
-            if 'dst4' in rule and family == 6:
-                raise RuleParseError('dst4 rule found for IPv6 address')
-            if 'dst6' in rule and family == 4:
-                raise RuleParseError('dst6 rule found for IPv4 address')
             nft_rule = ""
             
             if 'proto' in rule:
@@ -1034,21 +1021,46 @@ def prepare_forward_rules(self, rules, family):
                 protos = ['tcp', 'udp']
 
             if 'dst4' in rule:
+                dstfamily = 4
                 dsthost = rule['dst4']
             elif 'dst6' in rule:
+                dstfamily = 6
                 dsthost = rule['dst6']
             else:
                 raise RuleParseError(
                     'Missing dst address!')
 
             if 'src4' in rule:
+                srcfamily = 4
                 srchosts = rule['src4']
+                family = 4
             elif 'dst6' in rule:
+                srcfamily = 6
                 srchosts = rule['src6']
             else:
                 raise RuleParseError(
                     'Missing dst address!')
 
+
+            if srcfamily != dstfamily:
+                raise RuleParseError(
+                    'Mixed src and dst ip version family')
+            family = dstfamily
+
+            if 'src4' in rule and family == 6:
+                raise RuleParseError('dst4 rule found for IPv6 address')
+            if 'src6' in rule and family == 4:
+                raise RuleParseError('src6 rule found for IPv4 address')
+            if 'dst4' in rule and family == 6:
+                raise RuleParseError('dst4 rule found for IPv6 address')
+            if 'dst6' in rule and family == 4:
+                raise RuleParseError('dst6 rule found for IPv4 address')
+
+            assert family in (4, 6)
+            ip_match = 'ip6' if family == 6 else 'ip'
+
+            fullmask = '/128' if family == 6 else '/32'
+
             # if the range is zero nft complains, otherwise a range is ok
             if 'srcports' in rule:
                 if rule['srcports'].split('-')[0] ==  rule['srcports'].split('-')[1]:
@@ -1133,7 +1145,7 @@ def apply_rules_family(self, source, rules, family):
         self.run_nft(nft)
         self.update_dns_info(source, dns)
 
-    def apply_forward_rules_family(self, target, rules, family):
+    def apply_forward_rules_family(self, rules):
         """
         Apply rules for given source address.
         Handle only rules for given address family (IPv4 or IPv6).
@@ -1144,10 +1156,13 @@ def apply_forward_rules_family(self, target, rules, family):
         :return: None
         """
 
-        if 'qubes-firewall-forward' not in self.chains[family]:
+        if 'qubes-firewall-forward' not in self.chains['4']:
+            self.create_forward_chain(family)
+
+        if 'qubes-firewall-forward' not in self.chains['6']:
             self.create_forward_chain(family)
 
-        nft = self.prepare_forward_rules(rules, family)
+        nft = self.prepare_forward_rules(rules)
         self.run_nft(nft)
 
     def init(self):
@@ -1177,6 +1192,11 @@ def cleanup(self):
         nft_cleanup = (
             'delete table ip qubes-firewall\n'
             'delete table ip6 qubes-firewall\n'
+        )
+        self.run_nft(nft_cleanup)
+
+    def forward_cleanup(self):
+        nft_cleanup = (
             'delete table ip qubes-firewall-forward\n'
             'delete table ip6 qubes-firewall-forward\n'
         )

From 6116c9e6a6a2dc1c3db17f61a2b74bc249764dd7 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Fri, 20 Aug 2021 01:06:34 +0200
Subject: [PATCH 18/20] Double check ip family mixups

---
 qubesagent/firewall.py | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 8ea128a1e..082ad1071 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -1023,9 +1023,15 @@ def prepare_forward_rules(self, rules):
             if 'dst4' in rule:
                 dstfamily = 4
                 dsthost = rule['dst4']
+                if self.is_ip6(srchosts):
+                    raise RuleParseError(
+                    'Ivalid value supplied as IPv4 address in dst4')
             elif 'dst6' in rule:
                 dstfamily = 6
                 dsthost = rule['dst6']
+                if not self.is_ip6(srchosts):
+                    raise RuleParseError(
+                    'Ivalid value supplied as IPv6 address in dst6')
             else:
                 raise RuleParseError(
                     'Missing dst address!')
@@ -1033,15 +1039,19 @@ def prepare_forward_rules(self, rules):
             if 'src4' in rule:
                 srcfamily = 4
                 srchosts = rule['src4']
-                family = 4
-            elif 'dst6' in rule:
+                if self.is_ip6(srchosts):
+                    raise RuleParseError(
+                    'Ivalid value supplied as IPv4 address in src4')
+            elif 'src6' in rule:
                 srcfamily = 6
                 srchosts = rule['src6']
+                if not self.is_ip6(srchosts):
+                    raise RuleParseError(
+                    'Ivalid value supplied as IPv6 address in src6')
             else:
                 raise RuleParseError(
                     'Missing dst address!')
 
-
             if srcfamily != dstfamily:
                 raise RuleParseError(
                     'Mixed src and dst ip version family')

From f855af7f7fa68f4c0569566eed3e13718472669d Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Fri, 20 Aug 2021 01:52:38 +0200
Subject: [PATCH 19/20] Working point

---
 qubesagent/firewall.py | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 082ad1071..839c88ee6 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -79,13 +79,6 @@ def apply_rules(self, source, rules):
         else:
             self.apply_rules_family(source, rules, 4)
 
-    def apply_forward_rules(self, target, rules):
-        # what do we check here? 
-        if self.is_ip6(target):
-            self.apply_forward_rules_family(rules, 6)
-        else:
-            self.apply_forward_rules_family(rules, 4)
-
     def update_connected_ips(self, family):
         raise NotImplementedError
 
@@ -156,6 +149,10 @@ def read_forward_rules(self):
         """Read forward rules from QubesDB and return them as a list of dicts"""
         """No policy here since they already are in the forward dict, use first/last flags"""
         entries = self.qdb.multiread('/qubes-firewall-forward/')
+        # filter out base empty entry
+        for key in list(entries.keys()):
+            if len(key.split('/')) < 5:
+                del entries[key]
         assert isinstance(entries, dict)
         # drop full path but add target ip info
         entries = dict(((k.split('/')[3] + "/" + k.split('/')[4], v.decode())
@@ -342,27 +339,22 @@ def main(self):
             pass
 
         self.cleanup()
-        if 'qubes-firewall-forward' in self.chains['4'] or 'qubes-firewall-forward' in self.chains['6']:
+        if 'qubes-firewall-forward' in self.chains[4] or 'qubes-firewall-forward' in self.chains[6]:
             self.forward_cleanup()
 
     def load_forwarding(self):
-        # the only reason for loading the forwarding rules this way is to get the ip and choose the correct family
-        # the address is discared aftern the correct family is chosen
         try:
             rules = self.read_forward_rules()
             self.apply_forward_rules(rules)
         
         except RuleParseError as e:
             self.log_error(
-                'Failed to parse rules for {} ({}), blocking traffic'.format(
-                    addr, str(e)
-                ))
-            self.apply_rules(addr, [{'action': 'drop'}])
+                'Failed to parse forwarding rule ({})'.format(str(e))
+                )
         
         except RuleApplyError as e:
             self.log_error(
-                'Failed to apply rules for {} ({}), blocking traffic'.format(
-                    addr, str(e))
+                'Failed to apply forwarding rule ({})'.format(str(e))
             )
 
     def terminate(self):
@@ -822,8 +814,6 @@ def create_forward_chain(self, family):
         Create a forwarding chain using nat for forwarding rules
         """
         nft_input = (
-            'flush chain {family} qubes-firewall-forward prerouting\n'
-            'flush chain {family} qubes-firewall-forward postrouting\n'
             'table {family} qubes-firewall-forward {{\n'
             '  chain postrouting {{\n'
             '    type nat hook postrouting priority srcnat; policy accept;\n'
@@ -1023,13 +1013,13 @@ def prepare_forward_rules(self, rules):
             if 'dst4' in rule:
                 dstfamily = 4
                 dsthost = rule['dst4']
-                if self.is_ip6(srchosts):
+                if self.is_ip6(dsthost):
                     raise RuleParseError(
                     'Ivalid value supplied as IPv4 address in dst4')
             elif 'dst6' in rule:
                 dstfamily = 6
                 dsthost = rule['dst6']
-                if not self.is_ip6(srchosts):
+                if not self.is_ip6(dsthost):
                     raise RuleParseError(
                     'Ivalid value supplied as IPv6 address in dst6')
             else:
@@ -1155,7 +1145,7 @@ def apply_rules_family(self, source, rules, family):
         self.run_nft(nft)
         self.update_dns_info(source, dns)
 
-    def apply_forward_rules_family(self, rules):
+    def apply_forward_rules(self, rules):
         """
         Apply rules for given source address.
         Handle only rules for given address family (IPv4 or IPv6).
@@ -1166,11 +1156,11 @@ def apply_forward_rules_family(self, rules):
         :return: None
         """
 
-        if 'qubes-firewall-forward' not in self.chains['4']:
-            self.create_forward_chain(family)
+        if 'qubes-firewall-forward' not in self.chains[4]:
+            self.create_forward_chain(4)
 
-        if 'qubes-firewall-forward' not in self.chains['6']:
-            self.create_forward_chain(family)
+        if 'qubes-firewall-forward' not in self.chains[6]:
+            self.create_forward_chain(6)
 
         nft = self.prepare_forward_rules(rules)
         self.run_nft(nft)

From 7a719628d7fb06f6785675fadf83dd45cd50b633 Mon Sep 17 00:00:00 2001
From: Giulio <giulio@gmx.com>
Date: Fri, 20 Aug 2021 02:42:12 +0200
Subject: [PATCH 20/20] Code ok, nft rules still in progress

---
 qubesagent/firewall.py | 47 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 839c88ee6..d5e945297 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -997,6 +997,9 @@ def prepare_forward_rules(self, rules):
         forward_nft_rules = []
         accept_nft_rules = []
 
+        forward_nft_rules_6 = []
+        accept_nft_rules_6 = []
+
         for rule in rules:
             unsupported_opts = set(rule.keys()).difference(
                 set(self.supported_forward_rule_opts))
@@ -1091,14 +1094,21 @@ def prepare_forward_rules(self, rules):
                 
                 for iface in sorted(interfaces):
                     for proto in sorted(protos):
-                        forward_nft_rules.append('meta iifname "{iface}" {family} saddr {srchosts} {proto} dport {{ {srcports} }} dnat to {dsthost}:{dstport}'.format(iface=iface, family=ip_match, srchosts=srchosts, proto=proto, srcports=srcports, dsthost=dsthost, dstport=dstport))
-                        accept_nft_rules.append('meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
+                        forward_entry = 'meta iifname "{iface}" {family} saddr {srchosts} {proto} dport {{ {srcports} }} dnat to {dsthost}:{dstport}'.format(iface=iface, family=ip_match, srchosts=srchosts, proto=proto, srcports=srcports, dsthost=dsthost, dstport=dstport)
+                        accept_entry = 'meta iifname "{iface}" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(iface=iface, family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport)
             else:
                 # internal we always use the dstport for communication between qubes. Maybe it is worth randomizing at a later stage?
                 # since we removed masquerading we can retain the original srchost for filtering
                 for proto in sorted(protos):
-                    forward_nft_rules.append('meta iifname "eth0" {family} saddr {srchosts} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, srchosts=srchosts, proto=proto, dsthost=dsthost, dstport=dstport))
-                    accept_nft_rules.append('meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport))
+                    forward_entry = 'meta iifname "eth0" {family} saddr {srchosts} {proto} dport {{ {dstport} }} dnat to {dsthost}:{dstport}'.format(family=ip_match, srchosts=srchosts, proto=proto, dsthost=dsthost, dstport=dstport)
+                    accept_entry = 'meta iifname "eth0" {family} daddr {dsthost} {proto} dport {dstport} ct state new counter accept'.format(family=ip_match, proto=proto, dsthost=dsthost, dstport=dstport)
+
+            if family == 4:
+                forward_nft_rules.append(forward_entry)
+                accept_nft_rules.append(accept_entry)
+            elif family == 6:
+                forward_nft_rules_6.append(forward_entry)
+                accept_nft_rules_6.append(accept_entry)
 
         forward_rule = (
             'table {family} {table} {{\n'
@@ -1106,7 +1116,7 @@ def prepare_forward_rules(self, rules):
             '   {rules}\n'
             '  }}\n'
             '}}\n'.format(
-                family=ip_match,
+                family='ip',
                 table='qubes-firewall-forward',
                 chain='prerouting',
                 rules='\n   '.join(forward_nft_rules)
@@ -1118,11 +1128,36 @@ def prepare_forward_rules(self, rules):
             '    {rules}\n'
             '  }}\n'
             '}}\n'.format(
-                family=ip_match,
+                family='ip',
                 table='qubes-firewall',
                 chain='forward',
                 rules='\n   '.join(accept_nft_rules)
             ))
+
+        forward_rule += (
+            'table {family} {table} {{\n'
+            '  chain {chain} {{\n'
+            '   {rules}\n'
+            '  }}\n'
+            '}}\n'.format(
+                family='ip6',
+                table='qubes-firewall-forward',
+                chain='prerouting',
+                rules='\n   '.join(forward_nft_rules_6)
+            ))
+
+        accept_rule += (
+            'table {family} {table} {{\n'
+            '  chain {chain} {{\n'
+            '    {rules}\n'
+            '  }}\n'
+            '}}\n'.format(
+                family='ip6',
+                table='qubes-firewall',
+                chain='forward',
+                rules='\n   '.join(accept_nft_rules_6)
+            ))
+
         return forward_rule + accept_rule