Merge pull request #549 from skuhl/nft-nat-update

Make nat and nft rules consistent; improve rule ordering.

4 files changed, 51 insertions, 43 deletions

diff --git a/sshuttle/linux.py b/sshuttle/linux.py
index 7794afe..93da430 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()