Make nat and nft rules consistent; improve rule ordering.

First, check if TTL indicates we should ignore packet (instead of
checking in multiple rules later). Also, nft method didn't do this at
all. Now, nft matches the behavior of nat.

Second, forward DNS traffic (we may need to intercept traffic to
localhost if a DNS server is running on localhost).

Third, ignore any local traffic packets. (Previously, we ignored local
traffic except DNS and then had the DNS rules). The nft method didn't
do this previously at all. It now matches the behavior of nat.

Lastly, list the subnets to redirect and/or exclude. This step is left
unchanged. Excluding the local port that we are listening on is
redundant with the third step, but should cause no harm.

In summary, this ordering simplifies the rules in nat and eliminates
differences that previously existed between nat and nft.

4 files changed, 51 insertions, 43 deletions

diff --git a/sshuttle/linux.py b/sshuttle/linux.py
index fabae1a..27a5a72 100644
--- a/sshuttle/linux.py
+++ b/sshuttle/linux.py
@@ -74,7 +74,7 @@ def ipt_ttl(family, *args):
         # with ttl 63.  This makes the client side not recapture those
         # connections, in case client == server.
         try:
-            argsplus = list(args) + ['-m', 'ttl', '!', '--ttl', '63']
+            argsplus = list(args)
             ipt(family, *argsplus)
         except Fatal:
             ipt(family, *args)
diff --git a/sshuttle/methods/nat.py b/sshuttle/methods/nat.py
index 3435240..18ec1fd 100644
--- a/sshuttle/methods/nat.py
+++ b/sshuttle/methods/nat.py
@@ -50,17 +50,24 @@ class Method(BaseMethod):
         _ipt('-I', 'OUTPUT', '1', *args)
         _ipt('-I', 'PREROUTING', '1', *args)
 
-        # Firstly we always skip all LOCAL addtrype address, i.e. avoid
-        # tunnelling the traffic designated to all local TCP/IP addresses.
-        _ipt('-A', chain, '-j', 'RETURN',
-             '-m', 'addrtype',
-             '--dst-type', 'LOCAL',
-             '!', '-p', 'udp')
-        # Skip LOCAL traffic if it's not DNS.
+        # This TTL hack allows the client and server to run on the
+        # same host. The connections the sshuttle server makes will
+        # have TTL set to 63.
+        _ipt_ttl('-A', chain, '-j', 'RETURN',  '-m', 'ttl', '--ttl', '63')
+
+        # Redirect DNS traffic as requested. This includes routing traffic
+        # to localhost DNS servers through sshuttle.
+        for _, ip in [i for i in nslist if i[0] == family]:
+            _ipt('-A', chain, '-j', 'REDIRECT',
+                 '--dest', '%s/32' % ip,
+                 '-p', 'udp',
+                 '--dport', '53',
+                 '--to-ports', str(dnsport))
+
+        # Don't route any remaining local traffic through sshuttle.
         _ipt('-A', chain, '-j', 'RETURN',
              '-m', 'addrtype',
-             '--dst-type', 'LOCAL',
-             '-p', 'udp', '!', '--dport', '53')
+             '--dst-type', 'LOCAL')
 
         # create new subnet entries.
         for _, swidth, sexclude, snet, fport, lport \
@@ -74,16 +81,9 @@ class Method(BaseMethod):
                      '--dest', '%s/%s' % (snet, swidth),
                      *tcp_ports)
             else:
-                _ipt_ttl('-A', chain, '-j', 'REDIRECT',
-                         '--dest', '%s/%s' % (snet, swidth),
-                         *(tcp_ports + ('--to-ports', str(port))))
-
-        for _, ip in [i for i in nslist if i[0] == family]:
-            _ipt_ttl('-A', chain, '-j', 'REDIRECT',
-                     '--dest', '%s/32' % ip,
-                     '-p', 'udp',
-                     '--dport', '53',
-                     '--to-ports', str(dnsport))
+                _ipt('-A', chain, '-j', 'REDIRECT',
+                     '--dest', '%s/%s' % (snet, swidth),
+                     *(tcp_ports + ('--to-ports', str(port))))
 
     def restore_firewall(self, port, family, udp, user):
         # only ipv4 supported with NAT
diff --git a/sshuttle/methods/nft.py b/sshuttle/methods/nft.py
index 32cba72..3ec47e0 100644
--- a/sshuttle/methods/nft.py
+++ b/sshuttle/methods/nft.py
@@ -34,6 +34,26 @@ class Method(BaseMethod):
         _nft('add rule', 'output jump %s' % chain)
         _nft('add rule', 'prerouting jump %s' % chain)
 
+        # This TTL hack allows the client and server to run on the
+        # same host. The connections the sshuttle server makes will
+        # have TTL set to 63.
+        _nft('add rule', chain, 'ip ttl == 63 return')
+
+        # Redirect DNS traffic as requested. This includes routing traffic
+        # to localhost DNS servers through sshuttle.
+        for _, ip in [i for i in nslist if i[0] == family]:
+            if family == socket.AF_INET:
+                _nft('add rule', chain, 'ip protocol udp ip daddr %s' % ip,
+                     'udp dport { 53 }',
+                     ('redirect to :' + str(dnsport)))
+            elif family == socket.AF_INET6:
+                _nft('add rule', chain, 'ip6 protocol udp ip6 daddr %s' % ip,
+                     'udp dport { 53 }',
+                     ('redirect to :' + str(dnsport)))
+
+        # Don't route any remaining local traffic through sshuttle
+        _nft('add rule', chain, 'fib daddr type local return')
+
         # create new subnet entries.
         for _, swidth, sexclude, snet, fport, lport \
                 in sorted(subnets, key=subnet_weight, reverse=True):
@@ -50,19 +70,9 @@ class Method(BaseMethod):
                      'ip daddr %s/%s' % (snet, swidth), 'return')))
             else:
                 _nft('add rule', chain, *(tcp_ports + (
-                     'ip daddr %s/%s' % (snet, swidth), 'ip ttl != 63',
+                     'ip daddr %s/%s' % (snet, swidth),
                      ('redirect to :' + str(port)))))
 
-        for _, ip in [i for i in nslist if i[0] == family]:
-            if family == socket.AF_INET:
-                _nft('add rule', chain, 'ip protocol udp ip daddr %s' % ip,
-                     'udp dport { 53 }', 'ip ttl != 63',
-                     ('redirect to :' + str(dnsport)))
-            elif family == socket.AF_INET6:
-                _nft('add rule', chain, 'ip6 protocol udp ip6 daddr %s' % ip,
-                     'udp dport { 53 }', 'ip ttl != 63',
-                     ('redirect to :' + str(dnsport)))
-
     def restore_firewall(self, port, family, udp, user):
         if udp:
             raise Exception("UDP not supported by nft method_name")
diff --git a/tests/client/test_methods_nat.py b/tests/client/test_methods_nat.py
index 83d0e36..11a901b 100644
--- a/tests/client/test_methods_nat.py
+++ b/tests/client/test_methods_nat.py
@@ -123,12 +123,8 @@ def test_setup_firewall(mock_ipt_chain_exists, mock_ipt_ttl, mock_ipt):
         call(AF_INET, 'nat', 'sshuttle-1025')
     ]
     assert mock_ipt_ttl.mock_calls == [
-        call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'REDIRECT',
-             '--dest', u'1.2.3.0/24', '-p', 'tcp', '--dport', '8000:9000',
-             '--to-ports', '1025'),
-        call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'REDIRECT',
-             '--dest', u'1.2.3.33/32', '-p', 'udp',
-             '--dport', '53', '--to-ports', '1027')
+        call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'RETURN',
+             '-m', 'ttl', '--ttl', '63')
     ]
     assert mock_ipt.mock_calls == [
         call(AF_INET, 'nat', '-D', 'OUTPUT', '-j', 'sshuttle-1025'),
@@ -139,14 +135,16 @@ def test_setup_firewall(mock_ipt_chain_exists, mock_ipt_ttl, mock_ipt):
         call(AF_INET, 'nat', '-F', 'sshuttle-1025'),
         call(AF_INET, 'nat', '-I', 'OUTPUT', '1', '-j', 'sshuttle-1025'),
         call(AF_INET, 'nat', '-I', 'PREROUTING', '1', '-j', 'sshuttle-1025'),
+        call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'REDIRECT',
+             '--dest', u'1.2.3.33/32', '-p', 'udp',
+             '--dport', '53', '--to-ports', '1027'),
         call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'RETURN',
-             '-m', 'addrtype', '--dst-type', 'LOCAL',
-             '!', '-p', 'udp'),
-        call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'RETURN',
-             '-m', 'addrtype', '--dst-type', 'LOCAL',
-             '-p', 'udp', '!', '--dport', '53'),
+             '-m', 'addrtype', '--dst-type', 'LOCAL'),
         call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'RETURN',
-             '--dest', u'1.2.3.66/32', '-p', 'tcp', '--dport', '8080:8080')
+             '--dest', u'1.2.3.66/32', '-p', 'tcp', '--dport', '8080:8080'),
+        call(AF_INET, 'nat', '-A', 'sshuttle-1025', '-j', 'REDIRECT',
+             '--dest', u'1.2.3.0/24', '-p', 'tcp', '--dport', '8000:9000',
+             '--to-ports', '1025')
     ]
     mock_ipt_chain_exists.reset_mock()
     mock_ipt_ttl.reset_mock()