diff --git a/sshuttle/client.py b/sshuttle/client.py index da23b7d..85694fc 100644 --- a/sshuttle/client.py +++ b/sshuttle/client.py @@ -15,7 +15,7 @@ import sshuttle.ssyslog as ssyslog import sshuttle.sdnotify as sdnotify from sshuttle.ssnet import SockWrapper, Handler, Proxy, Mux, MuxWrapper from sshuttle.helpers import log, debug1, debug2, debug3, Fatal, islocal, \ - resolvconf_nameservers + resolvconf_nameservers, which from sshuttle.methods import get_method, Features from sshuttle import __version__ try: @@ -196,11 +196,19 @@ class FirewallClient: ['--firewall']) if ssyslog._p: argvbase += ['--syslog'] - # Default to sudo unless on OpenBSD in which case use built in `doas` + + # Determine how to prefix the command in order to elevate privileges. if platform.platform().startswith('OpenBSD'): - elev_prefix = ['doas'] + elev_prefix = ['doas'] # OpenBSD uses built in `doas` else: elev_prefix = ['sudo', '-p', '[local sudo] Password: '] + + # Look for binary and switch to absolute path if we can find + # it. + path = which(elev_prefix[0]) + if path: + elev_prefix[0] = path + if sudo_pythonpath: elev_prefix += ['/usr/bin/env', 'PYTHONPATH=%s' % python_path] diff --git a/sshuttle/helpers.py b/sshuttle/helpers.py index a62bea9..185bf0c 100644 --- a/sshuttle/helpers.py +++ b/sshuttle/helpers.py @@ -1,6 +1,7 @@ import sys import socket import errno +import os logprefix = '' verbose = 0 @@ -99,3 +100,75 @@ def family_to_string(family): return "AF_INET" else: return str(family) + + +def get_env(): + """An environment for sshuttle subprocesses. See get_path().""" + env = { + 'PATH': get_path(), + 'LC_ALL': "C", + } + return env + + +def get_path(): + """Returns a string of paths separated by os.pathsep. + + Users might not have all of the programs sshuttle needs in their + PATH variable (i.e., some programs might be in /sbin). Use PATH + and a hardcoded set of paths to search through. This function is + used by our which() and get_env() functions. If which() and the + subprocess environments differ, programs that which() finds might + not be found at run time (or vice versa). + """ + path = [] + if "PATH" in os.environ: + path += os.environ["PATH"].split(os.pathsep) + # Python default paths. + path += os.defpath.split(os.pathsep) + # /sbin, etc are not in os.defpath and may not be in PATH either. + # /bin/ and /usr/bin below are probably redundant. + path += ['/bin', '/usr/bin', '/sbin', '/usr/sbin'] + + # Remove duplicates. Not strictly necessary. + path_dedup = [] + for i in path: + if i not in path_dedup: + path_dedup.append(i) + + return os.pathsep.join(path_dedup) + + +if sys.version_info >= (3, 3): + from shutil import which as _which +else: + # Although sshuttle does not officially support older versions of + # Python, some still run the sshuttle server on remote machines + # with old versions of python. + def _which(file, mode=os.F_OK | os.X_OK, path=None): + if path is not None: + search_paths = path.split(os.pathsep) + elif "PATH" in os.environ: + search_paths = os.environ["PATH"].split(os.pathsep) + else: + search_paths = os.defpath.split(os.pathsep) + + for p in search_paths: + filepath = os.path.join(p, file) + if os.path.exists(filepath) and os.access(filepath, mode): + return filepath + return None + + +def which(file, mode=os.F_OK | os.X_OK): + """A wrapper around shutil.which() that searches a predictable set of + paths and is more verbose about what is happening. See get_path() + for more information. + """ + path = get_path() + rv = _which(file, mode, path) + if rv: + debug2("which() found '%s' at %s\n" % (file, rv)) + else: + debug2("which() could not find '%s' in %s\n" % (file, path)) + return rv diff --git a/sshuttle/hostwatch.py b/sshuttle/hostwatch.py index a162212..d40eaf4 100644 --- a/sshuttle/hostwatch.py +++ b/sshuttle/hostwatch.py @@ -9,7 +9,7 @@ import platform import subprocess as ssubprocess import sshuttle.helpers as helpers -from sshuttle.helpers import log, debug1, debug2, debug3 +from sshuttle.helpers import log, debug1, debug2, debug3, get_env POLL_TIME = 60 * 15 NETSTAT_POLL_TIME = 30 @@ -125,14 +125,10 @@ def _check_dns(hostname): def _check_netstat(): debug2(' > netstat\n') - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } argv = ['netstat', '-n'] try: p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, stderr=null, - env=env) + env=get_env()) content = p.stdout.read().decode("ASCII") p.wait() except OSError: @@ -151,14 +147,10 @@ def _check_smb(hostname): if not _smb_ok: return debug2(' > smb: %s\n' % hostname) - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } argv = ['smbclient', '-U', '%', '-L', hostname] try: p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, stderr=null, - env=env) + env=get_env()) lines = p.stdout.readlines() p.wait() except OSError: @@ -214,14 +206,10 @@ def _check_nmb(hostname, is_workgroup, is_master): if not _nmb_ok: return debug2(' > n%d%d: %s\n' % (is_workgroup, is_master, hostname)) - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } argv = ['nmblookup'] + ['-M'] * is_master + ['--', hostname] try: p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, stderr=null, - env=env) + env=get_env) lines = p.stdout.readlines() rv = p.wait() except OSError: diff --git a/sshuttle/linux.py b/sshuttle/linux.py index 93da430..5f91df6 100644 --- a/sshuttle/linux.py +++ b/sshuttle/linux.py @@ -1,7 +1,6 @@ -import os import socket import subprocess as ssubprocess -from sshuttle.helpers import log, debug1, Fatal, family_to_string +from sshuttle.helpers import log, debug1, Fatal, family_to_string, get_env def nonfatal(func, *args): @@ -19,12 +18,8 @@ def ipt_chain_exists(family, table, name): else: raise Exception('Unsupported family "%s"' % family_to_string(family)) argv = [cmd, '-t', table, '-nL'] - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } try: - output = ssubprocess.check_output(argv, env=env) + output = ssubprocess.check_output(argv, env=get_env()) for line in output.decode('ASCII').split('\n'): if line.startswith('Chain %s ' % name): return True @@ -40,11 +35,7 @@ def ipt(family, table, *args): else: raise Exception('Unsupported family "%s"' % family_to_string(family)) debug1('%s\n' % ' '.join(argv)) - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } - rv = ssubprocess.call(argv, env=env) + rv = ssubprocess.call(argv, env=get_env()) if rv: raise Fatal('fw: %r returned %d' % (argv, rv)) @@ -55,11 +46,7 @@ def nft(family, table, action, *args): else: raise Exception('Unsupported family "%s"' % family_to_string(family)) debug1('%s\n' % ' '.join(argv)) - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } - rv = ssubprocess.call(argv, env=env) + rv = ssubprocess.call(argv, env=get_env()) if rv: raise Fatal('fw: %r returned %d' % (argv, rv)) diff --git a/sshuttle/methods/__init__.py b/sshuttle/methods/__init__.py index 7a1d493..7d774e1 100644 --- a/sshuttle/methods/__init__.py +++ b/sshuttle/methods/__init__.py @@ -1,9 +1,8 @@ -import os import importlib import socket import struct import errno -from sshuttle.helpers import Fatal, debug3 +from sshuttle.helpers import Fatal, debug3, which def original_dst(sock): @@ -87,27 +86,19 @@ class BaseMethod(object): return False -def _program_exists(name): - paths = (os.getenv('PATH') or os.defpath).split(os.pathsep) - for p in paths: - fn = '%s/%s' % (p, name) - if os.path.exists(fn): - return not os.path.isdir(fn) and os.access(fn, os.X_OK) - - def get_method(method_name): module = importlib.import_module("sshuttle.methods.%s" % method_name) return module.Method(method_name) def get_auto_method(): - if _program_exists('iptables'): + if which('iptables'): method_name = "nat" - elif _program_exists('nft'): + elif which('nft'): method_name = "nft" - elif _program_exists('pfctl'): + elif which('pfctl'): method_name = "pf" - elif _program_exists('ipfw'): + elif which('ipfw'): method_name = "ipfw" else: raise Fatal( diff --git a/sshuttle/methods/ipfw.py b/sshuttle/methods/ipfw.py index 486658f..8b4aebc 100644 --- a/sshuttle/methods/ipfw.py +++ b/sshuttle/methods/ipfw.py @@ -2,7 +2,7 @@ import os import subprocess as ssubprocess from sshuttle.methods import BaseMethod from sshuttle.helpers import log, debug1, debug3, \ - Fatal, family_to_string + Fatal, family_to_string, get_env recvmsg = None try: @@ -61,11 +61,7 @@ else: def ipfw_rule_exists(n): argv = ['ipfw', 'list'] - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } - p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, env=env) + p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, env=get_env()) found = False for line in p.stdout: @@ -85,11 +81,7 @@ _oldctls = {} def _fill_oldctls(prefix): argv = ['sysctl', prefix] - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } - p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, env=env) + p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, env=get_env()) for line in p.stdout: line = line.decode() assert(line[-1] == '\n') @@ -105,7 +97,7 @@ def _fill_oldctls(prefix): def _sysctl_set(name, val): argv = ['sysctl', '-w', '%s=%s' % (name, val)] debug1('>> %s\n' % ' '.join(argv)) - return ssubprocess.call(argv, stdout=open(os.devnull, 'w')) + return ssubprocess.call(argv, stdout=open(os.devnull, 'w'), env=get_env()) # No env: No output. (Or error that won't be parsed.) @@ -139,7 +131,7 @@ def sysctl_set(name, val, permanent=False): def ipfw(*args): argv = ['ipfw', '-q'] + list(args) debug1('>> %s\n' % ' '.join(argv)) - rv = ssubprocess.call(argv) + rv = ssubprocess.call(argv, env=get_env()) # No env: No output. (Or error that won't be parsed.) if rv: raise Fatal('%r returned %d' % (argv, rv)) @@ -148,7 +140,7 @@ def ipfw(*args): def ipfw_noexit(*args): argv = ['ipfw', '-q'] + list(args) debug1('>> %s\n' % ' '.join(argv)) - ssubprocess.call(argv) + ssubprocess.call(argv, env=get_env()) # No env: No output. (Or error that won't be parsed.) diff --git a/sshuttle/methods/pf.py b/sshuttle/methods/pf.py index c2cd16e..0ef0f46 100644 --- a/sshuttle/methods/pf.py +++ b/sshuttle/methods/pf.py @@ -11,7 +11,8 @@ from fcntl import ioctl from ctypes import c_char, c_uint8, c_uint16, c_uint32, Union, Structure, \ sizeof, addressof, memmove from sshuttle.firewall import subnet_weight -from sshuttle.helpers import debug1, debug2, debug3, Fatal, family_to_string +from sshuttle.helpers import debug1, debug2, debug3, Fatal, family_to_string, \ + get_env from sshuttle.methods import BaseMethod @@ -179,7 +180,7 @@ class FreeBsd(Generic): return freebsd def enable(self): - returncode = ssubprocess.call(['kldload', 'pf']) + returncode = ssubprocess.call(['kldload', 'pf'], env=get_env()) # No env: No output. super(FreeBsd, self).enable() if returncode == 0: @@ -189,7 +190,7 @@ class FreeBsd(Generic): super(FreeBsd, self).disable(anchor) if _pf_context['loaded_by_sshuttle'] and \ _pf_context['started_by_sshuttle'] == 0: - ssubprocess.call(['kldunload', 'pf']) + ssubprocess.call(['kldunload', 'pf'], env=get_env()) # No env: No output. def add_anchors(self, anchor): @@ -386,15 +387,10 @@ else: def pfctl(args, stdin=None): argv = ['pfctl'] + shlex.split(args) debug1('>> %s\n' % ' '.join(argv)) - - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } p = ssubprocess.Popen(argv, stdin=ssubprocess.PIPE, stdout=ssubprocess.PIPE, stderr=ssubprocess.PIPE, - env=env) + env=get_env()) o = p.communicate(stdin) if p.returncode: raise Fatal('%r returned %d' % (argv, p.returncode)) diff --git a/sshuttle/server.py b/sshuttle/server.py index 8e15866..456be2d 100644 --- a/sshuttle/server.py +++ b/sshuttle/server.py @@ -7,24 +7,6 @@ import sys import os import platform -if sys.version_info >= (3, 0): - from shutil import which -else: - # python2 compat: shutil.which is not available so we provide our - # own which command - def which(file, mode=os.F_OK | os.X_OK, path=None): - if path is not None: - search_paths = [path] - elif "PATH" in os.environ: - search_paths = os.environ["PATH"].split(os.pathsep) - else: - search_paths = os.defpath.split(os.pathsep) - - for p in search_paths: - filepath = os.path.join(p, file) - if os.path.exists(filepath) and os.access(filepath, mode): - return filepath - return None import sshuttle.ssnet as ssnet import sshuttle.helpers as helpers @@ -32,7 +14,7 @@ import sshuttle.hostwatch as hostwatch import subprocess as ssubprocess from sshuttle.ssnet import Handler, Proxy, Mux, MuxWrapper from sshuttle.helpers import b, log, debug1, debug2, debug3, Fatal, \ - resolvconf_random_nameserver + resolvconf_random_nameserver, which, get_env def _ipmatch(ipstr): @@ -100,11 +82,7 @@ def _route_iproute(line): def _list_routes(argv, extract_route): # FIXME: IPv4 only - env = { - 'PATH': os.environ['PATH'], - 'LC_ALL': "C", - } - p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, env=env) + p = ssubprocess.Popen(argv, stdout=ssubprocess.PIPE, env=get_env()) routes = [] for line in p.stdout: if not line.strip(): diff --git a/sshuttle/ssh.py b/sshuttle/ssh.py index e4dcf1b..ae235ed 100644 --- a/sshuttle/ssh.py +++ b/sshuttle/ssh.py @@ -12,7 +12,7 @@ import ipaddress from urllib.parse import urlparse import sshuttle.helpers as helpers -from sshuttle.helpers import debug2 +from sshuttle.helpers import debug2, which, get_path, Fatal def get_module_source(name): @@ -141,6 +141,17 @@ def connect(ssh_cmd, rhostport, python, stderr, options): argv = (sshl + portl + [rhost, '--', pycmd]) + + # Our which() function searches for programs in get_path() + # directories (which include PATH). This step isn't strictly + # necessary if ssh is already in the user's PATH, but it makes the + # error message friendlier if the user incorrectly passes in a + # custom ssh command that we cannot find. + abs_path = which(argv[0]) + if abs_path is None: + raise Fatal("Failed to find '%s' in path %s" % (argv[0], get_path())) + argv[0] = abs_path + (s1, s2) = socket.socketpair() def setup(): @@ -148,6 +159,7 @@ def connect(ssh_cmd, rhostport, python, stderr, options): s2.close() s1a, s1b = os.dup(s1.fileno()), os.dup(s1.fileno()) s1.close() + debug2('executing: %r\n' % argv) p = ssubprocess.Popen(argv, stdin=s1a, stdout=s1b, preexec_fn=setup, close_fds=True, stderr=stderr) diff --git a/tests/client/test_methods_pf.py b/tests/client/test_methods_pf.py index 42f1828..bf600ba 100644 --- a/tests/client/test_methods_pf.py +++ b/tests/client/test_methods_pf.py @@ -4,7 +4,7 @@ from socket import AF_INET, AF_INET6 import pytest from mock import Mock, patch, call, ANY from sshuttle.methods import get_method -from sshuttle.helpers import Fatal +from sshuttle.helpers import Fatal, get_env from sshuttle.methods.pf import FreeBsd, Darwin, OpenBsd @@ -316,7 +316,8 @@ def test_setup_firewall_freebsd(mock_pf_get_dev, mock_ioctl, mock_pfctl, b'to port 53 keep state\n'), call('-e'), ] - assert call(['kldload', 'pf']) in mock_subprocess_call.mock_calls + assert call(['kldload', 'pf'], env=get_env()) in \ + mock_subprocess_call.mock_calls mock_pf_get_dev.reset_mock() mock_ioctl.reset_mock() mock_pfctl.reset_mock()