Fixes some style issues and minor bugs

Author: vieira

.prospector.yml

@@ -10,6 +10,11 @@ pylint:
     - bare-except
     - protected-access
     - no-else-return
+    - unused-argument
+    - method-hidden
+    - arguments-differ
+    - wrong-import-position
+    - raising-bad-type
 
 pep8:
   options:

sshuttle/client.py

@@ -112,8 +112,8 @@ def daemon_cleanup():
 
 class MultiListener:
 
-    def __init__(self, type=socket.SOCK_STREAM, proto=0):
-        self.type = type
+    def __init__(self, kind=socket.SOCK_STREAM, proto=0):
+        self.type = kind
         self.proto = proto
         self.v6 = None
         self.v4 = None
@@ -746,22 +746,22 @@ def main(listenip_v6, listenip_v4,
     # Last minute sanity checks.
     # These should never fail.
     # If these do fail, something is broken above.
-    if len(subnets_v6) > 0:
+    if subnets_v6:
         assert required.ipv6
         if redirectport_v6 == 0:
             raise Fatal("IPv6 subnets defined but not listening")
 
-    if len(nslist_v6) > 0:
+    if nslist_v6:
         assert required.dns
         assert required.ipv6
         if dnsport_v6 == 0:
             raise Fatal("IPv6 ns servers defined but not listening")
 
-    if len(subnets_v4) > 0:
+    if subnets_v4:
         if redirectport_v4 == 0:
             raise Fatal("IPv4 subnets defined but not listening")
 
-    if len(nslist_v4) > 0:
+    if nslist_v4:
         if dnsport_v4 == 0:
             raise Fatal("IPv4 ns servers defined but not listening")
 

sshuttle/cmdline.py

@@ -45,8 +45,8 @@ def main():
             if opt.listen:
                 ipport_v6 = None
                 ipport_v4 = None
-                list = opt.listen.split(",")
-                for ip in list:
+                lst = opt.listen.split(",")
+                for ip in lst:
                     family, ip, port = parse_ipport(ip)
                     if family == socket.AF_INET6:
                         ipport_v6 = (ip, port)

sshuttle/firewall.py

@@ -2,6 +2,7 @@
 import socket
 import signal
 import sshuttle.ssyslog as ssyslog
+import sshuttle.sdnotify as sdnotify
 import sys
 import os
 import platform
@@ -164,7 +165,7 @@ def main(method_name, syslog):
     _, _, ports = line.partition(" ")
     ports = ports.split(",")
     if len(ports) != 4:
-        raise Fatal('firewall: expected 4 ports but got %n' % len(ports))
+        raise Fatal('firewall: expected 4 ports but got %d' % len(ports))
     port_v6 = int(ports[0])
     port_v4 = int(ports[1])
     dnsport_v6 = int(ports[2])
@@ -203,14 +204,14 @@ def main(method_name, syslog):
     try:
         debug1('firewall manager: setting up.\n')
 
-        if len(subnets_v6) > 0 or len(nslist_v6) > 0:
+        if subnets_v6 or nslist_v6:
             debug2('firewall manager: setting up IPv6.\n')
             method.setup_firewall(
                 port_v6, dnsport_v6, nslist_v6,
                 socket.AF_INET6, subnets_v6, udp,
                 user)
 
-        if len(subnets_v4) > 0 or len(nslist_v4) > 0:
+        if subnets_v4 or nslist_v4:
             debug2('firewall manager: setting up IPv4.\n')
             method.setup_firewall(
                 port_v4, dnsport_v4, nslist_v4,
@@ -249,7 +250,7 @@ def main(method_name, syslog):
             pass
 
         try:
-            if len(subnets_v6) > 0 or len(nslist_v6) > 0:
+            if subnets_v6 or nslist_v6:
                 debug2('firewall manager: undoing IPv6 changes.\n')
                 method.restore_firewall(port_v6, socket.AF_INET6, udp, user)
         except:
@@ -262,7 +263,7 @@ def main(method_name, syslog):
                 pass
 
         try:
-            if len(subnets_v4) > 0 or len(nslist_v4) > 0:
+            if subnets_v4 or nslist_v4:
                 debug2('firewall manager: undoing IPv4 changes.\n')
                 method.restore_firewall(port_v4, socket.AF_INET, udp, user)
         except:

sshuttle/hostwatch.py

@@ -251,7 +251,7 @@ def _enqueue(op, *args):
 
 
 def _stdin_still_ok(timeout):
-    r, w, x = select.select([sys.stdin.fileno()], [], [], timeout)
+    r, _, _ = select.select([sys.stdin.fileno()], [], [], timeout)
     if r:
         b = os.read(sys.stdin.fileno(), 4096)
         if not b:

sshuttle/methods/__init__.py

@@ -74,7 +74,8 @@ def assert_features(self, features):
                     "Feature %s not supported with method %s.\n" %
                     (key, self.name))
 
-    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp, user):
+    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp,
+                       user):
         raise NotImplementedError()
 
     def restore_firewall(self, port, family, udp, user):

sshuttle/methods/ipfw.py

@@ -1,6 +1,4 @@
 import os
-import sys
-import struct
 import subprocess as ssubprocess
 from sshuttle.methods import BaseMethod
 from sshuttle.helpers import log, debug1, debug3, \
@@ -31,9 +29,9 @@
 if recvmsg == "python":
     def recv_udp(listener, bufsize):
         debug3('Accept UDP python using recvmsg.\n')
-        data, ancdata, msg_flags, srcip = listener.recvmsg(4096, socket.CMSG_SPACE(4))
+        data, ancdata, _, srcip = \
+                listener.recvmsg(4096, socket.CMSG_SPACE(4))
         dstip = None
-        family = None
         for cmsg_level, cmsg_type, cmsg_data in ancdata:
             if cmsg_level == socket.SOL_IP and cmsg_type == IP_RECVDSTADDR:
                 port = 53
@@ -44,13 +42,13 @@ def recv_udp(listener, bufsize):
 elif recvmsg == "socket_ext":
     def recv_udp(listener, bufsize):
         debug3('Accept UDP using socket_ext recvmsg.\n')
-        srcip, data, adata, flags = listener.recvmsg((bufsize,), socket.CMSG_SPACE(4))
+        srcip, data, adata, _ = \
+                listener.recvmsg((bufsize,), socket.CMSG_SPACE(4))
         dstip = None
-        family = None
         for a in adata:
             if a.cmsg_level == socket.SOL_IP and a.cmsg_type == IP_RECVDSTADDR:
                 port = 53
-                ip = socket.inet_ntop(socket.AF_INET, cmsg_data[0:4])
+                ip = socket.inet_ntop(socket.AF_INET, a.cmsg_data[0:4])
                 dstip = (ip, port)
                 break
         return (srcip, dstip, data[0])
@@ -75,7 +73,7 @@ def ipfw_rule_exists(n):
             if not ('ipttl 42' in line or 'check-state' in line):
                 log('non-sshuttle ipfw rule: %r\n' % line.strip())
                 raise Fatal('non-sshuttle ipfw rule #%d already exists!' % n)
-                found = True
+            found = True
     rv = p.wait()
     if rv:
         raise Fatal('%r returned %d' % (argv, rv))
@@ -193,7 +191,8 @@ def setup_udp_listener(self, udp_listener):
         #if udp_listener.v6 is not None:
         #    udp_listener.v6.setsockopt(SOL_IPV6, IPV6_RECVDSTADDR, 1)
 
-    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp, user):
+    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp,
+                       user):
         # IPv6 not supported
         if family not in [socket.AF_INET]:
             raise Exception(
@@ -224,31 +223,22 @@ def setup_firewall(self, port, dnsport, nslist, family, subnets, udp, user):
 
         ipfw_noexit('table', '124', 'flush')
         dnscount = 0
-        for f, ip in [i for i in nslist if i[0] == family]:
+        for _, ip in [i for i in nslist if i[0] == family]:
             ipfw('table', '124', 'add', '%s' % (ip))
             dnscount += 1
         if dnscount > 0:
             ipfw('add', '1', 'fwd', '127.0.0.1,%d' % dnsport,
                  'udp',
                  'from', 'any', 'to', 'table(124)',
                  'not', 'ipttl', '42')
-        """if udp:
-            ipfw('add', '1', 'skipto', '2',
-                 'udp',
-                 'from', 'any', 'to', 'table(125)')
-            ipfw('add', '1', 'fwd', '127.0.0.1,%d' % port,
-                 'udp',
-                 'from', 'any', 'to', 'table(126)',
-                 'not', 'ipttl', '42')
-        """
-        ipfw('add', '1', 'allow', 
+        ipfw('add', '1', 'allow',
              'udp',
              'from', 'any', 'to', 'any',
              'ipttl', '42')
 
         if subnets:
             # create new subnet entries
-            for f, swidth, sexclude, snet \
+            for _, swidth, sexclude, snet \
                 in sorted(subnets, key=lambda s: s[1], reverse=True):
                 if sexclude:
                     ipfw('table', '125', 'add', '%s/%s' % (snet, swidth))
@@ -265,4 +255,3 @@ def restore_firewall(self, port, family, udp, user):
         ipfw_noexit('table', '124', 'flush')
         ipfw_noexit('table', '125', 'flush')
         ipfw_noexit('table', '126', 'flush')
-

sshuttle/methods/nat.py

@@ -12,7 +12,8 @@ class Method(BaseMethod):
     # the multiple copies shouldn't have overlapping subnets, or only the most-
     # recently-started one will win (because we use "-I OUTPUT 1" instead of
     # "-A OUTPUT").
-    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp, user):
+    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp,
+                       user):
         # only ipv4 supported with NAT
         if family != socket.AF_INET:
             raise Exception(
@@ -50,7 +51,7 @@ def _ipm(*args):
         _ipt('-I', 'PREROUTING', '1', *args)
 
         # create new subnet entries.
-        for f, swidth, sexclude, snet, fport, lport \
+        for _, swidth, sexclude, snet, fport, lport \
                 in sorted(subnets, key=subnet_weight, reverse=True):
             tcp_ports = ('-p', 'tcp')
             if fport:
@@ -65,7 +66,7 @@ def _ipm(*args):
                          '--dest', '%s/%s' % (snet, swidth),
                          *(tcp_ports + ('--to-ports', str(port))))
 
-        for f, ip in [i for i in nslist if i[0] == family]:
+        for _, ip in [i for i in nslist if i[0] == family]:
             _ipt_ttl('-A', chain, '-j', 'REDIRECT',
                      '--dest', '%s/32' % ip,
                      '-p', 'udp',
@@ -97,8 +98,8 @@ def _ipm(*args):
         # basic cleanup/setup of chains
         if ipt_chain_exists(family, table, chain):
             if user is not None:
-                nonfatal(_ipm, '-D', 'OUTPUT', '-m', 'owner', '--uid-owner', str(user),
-                         '-j', 'MARK', '--set-mark', str(port))
+                nonfatal(_ipm, '-D', 'OUTPUT', '-m', 'owner', '--uid-owner',
+                         str(user), '-j', 'MARK', '--set-mark', str(port))
                 args = '-m', 'mark', '--mark', str(port), '-j', chain
             else:
                 args = '-j', chain

sshuttle/methods/pf.py

@@ -115,21 +115,21 @@ def add_anchors(self, anchor, status=None):
         if ('\nanchor "%s"' % anchor).encode('ASCII') not in status:
             self._add_anchor_rule(self.PF_PASS, anchor.encode('ASCII'))
 
-    def _add_anchor_rule(self, type, name, pr=None):
+    def _add_anchor_rule(self, kind, name, pr=None):
         if pr is None:
             pr = self.pfioc_rule()
 
         memmove(addressof(pr) + self.ANCHOR_CALL_OFFSET, name,
-                min(self.MAXPATHLEN, len(name)))  # anchor_call = name
+                min(self.MAXPATHLEN, len(name))) # anchor_call = name
         memmove(addressof(pr) + self.RULE_ACTION_OFFSET,
-                struct.pack('I', type), 4)  # rule.action = type
+                struct.pack('I', kind), 4) # rule.action = kind
 
         memmove(addressof(pr) + self.ACTION_OFFSET, struct.pack(
-            'I', self.PF_CHANGE_GET_TICKET), 4)  # action = PF_CHANGE_GET_TICKET
+            'I', self.PF_CHANGE_GET_TICKET), 4) # action = PF_CHANGE_GET_TICKET
         ioctl(pf_get_dev(), pf.DIOCCHANGERULE, pr)
 
         memmove(addressof(pr) + self.ACTION_OFFSET, struct.pack(
-            'I', self.PF_CHANGE_ADD_TAIL), 4)  # action = PF_CHANGE_ADD_TAIL
+            'I', self.PF_CHANGE_ADD_TAIL), 4) # action = PF_CHANGE_ADD_TAIL
         ioctl(pf_get_dev(), pf.DIOCCHANGERULE, pr)
 
     @staticmethod
@@ -176,9 +176,6 @@ class pfioc_natlook(Structure):
         freebsd.pfioc_natlook = pfioc_natlook
         return freebsd
 
-    def __init__(self):
-        super(FreeBsd, self).__init__()
-
     def enable(self):
         returncode = ssubprocess.call(['kldload', 'pf'])
         super(FreeBsd, self).enable()
@@ -197,14 +194,14 @@ def add_anchors(self, anchor):
             self._add_anchor_rule(self.PF_RDR, anchor.encode('ASCII'))
         super(FreeBsd, self).add_anchors(anchor, status=status)
 
-    def _add_anchor_rule(self, type, name):
-        pr = self.pfioc_rule()
+    def _add_anchor_rule(self, kind, name, pr=None):
+        pr = pr or self.pfioc_rule()
         ppa = self.pfioc_pooladdr()
 
         ioctl(pf_get_dev(), self.DIOCBEGINADDRS, ppa)
         # pool ticket
         memmove(addressof(pr) + self.POOL_TICKET_OFFSET, ppa[4:8], 4)
-        super(FreeBsd, self)._add_anchor_rule(type, name, pr=pr)
+        super(FreeBsd, self)._add_anchor_rule(kind, name, pr=pr)
 
     def add_rules(self, anchor, includes, port, dnsport, nslist, family):
         inet_version = self._inet_version(family)
@@ -224,7 +221,7 @@ def add_rules(self, anchor, includes, port, dnsport, nslist, family):
             for exclude, subnet in includes
         ]
 
-        if len(nslist) > 0:
+        if nslist:
             tables.append(
                 b'table <dns_servers> {%s}' %
                 b','.join([ns[1].encode("ASCII") for ns in nslist]))
@@ -294,7 +291,7 @@ def add_rules(self, anchor, includes, port, dnsport, nslist, family):
             for exclude, subnet in includes
         ]
 
-        if len(nslist) > 0:
+        if nslist:
             tables.append(
                 b'table <dns_servers> {%s}' %
                 b','.join([ns[1].encode("ASCII") for ns in nslist]))
@@ -440,24 +437,21 @@ def get_tcp_dstip(self, sock):
 
         return sock.getsockname()
 
-    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp, user):
-        tables = []
-        translating_rules = []
-        filtering_rules = []
-
+    def setup_firewall(self, port, dnsport, nslist, family, subnets, udp,
+                       user):
         if family not in [socket.AF_INET, socket.AF_INET6]:
             raise Exception(
                 'Address family "%s" unsupported by pf method_name'
                 % family_to_string(family))
         if udp:
             raise Exception("UDP not supported by pf method_name")
 
-        if len(subnets) > 0:
+        if subnets:
             includes = []
             # If a given subnet is both included and excluded, list the
             # exclusion first; the table will ignore the second, opposite
             # definition
-            for f, swidth, sexclude, snet, fport, lport \
+            for _, swidth, sexclude, snet, fport, lport \
                     in sorted(subnets, key=subnet_weight, reverse=True):
                 includes.append((sexclude, b"%s/%d%s" % (
                     snet.encode("ASCII"),