Skip to content

Commit 49ec5d9

Browse files
author
ExpressVPN
committed
Incremental improvements
* Correct typo in expressvpn_windows config * Freeze Selenium (to avoid an operadriver regression) * Pass the $PATH to the test runner on Ubuntu * Remove vestigial ssh_keys/ directory * Add an option to write a PID file * Add an option to punch a hole in the firewall (PF only) * Only log in color when we detect the terminal can handle it * Retry DNS lookups if DiG doesn't return any output * Add support for Network Extension VPNs * Better cleanups * Source a .source file on remote devices * Cleaner SSH implementation * Add a method to get host IP addresses * Improved connectivity to remote test devices * Add a debug tool for ip_responder server * Minor bugfixes and enhancements
1 parent 1e4346d commit 49ec5d9

File tree

19 files changed

+127
-42
lines changed

19 files changed

+127
-42
lines changed

configs/expressvpn_macos.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from xv_leak_tools.test_templating.templating import TemplateEvaluator, Replacee, Each
22

3-
NO_IPV6 = True
3+
NO_IPV6 = False
44

55
STRONGEST_SETTINGS = """Ensure Network Lock, IPv6 leak protection and Only user ExpressVPN DNS
66
servers are enabled. Also ensure access to LAN is allowed"""

configs/expressvpn_windows.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@
9494
'TestWindowsIPResponderDisruptAdapter',
9595
'TestWindowsIPResponderDisruptEnableNewAdapter',
9696
'TestWindowsIPResponderDisruptReorderAdapters',
97-
'TestWindowsIPResponderDisruptVPNConnection',
97+
'TestIPResponderDisruptVPNConnection',
9898
'TestIPResponderDisruptKillVPNProcess',
9999
'TestIPResponderDisruptVPNConnection',
100100

requirements_macos.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ parameterized
77
paramiko
88
psutil
99
pylint
10-
selenium
10+
selenium <= 3.6.0
1111

1212
https://pypi.python.org/packages/c8/20/3f767baf52ae14b5fa0c6a531651ecb9dbfdc5f75a3dc8eea740df72270c/pyobjc-framework-SystemConfiguration-3.2.1.tar.gz

run_tests.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ done
1717
unamestr=`uname`
1818
if [[ "$unamestr" == 'Linux' ]]; then
1919
echo "Leak tools require root permissions..."
20-
sudo -E $RUN_TESTS $@
20+
sudo env "PATH=$PATH" $RUN_TESTS $@
2121
elif [[ "$unamestr" == 'Darwin' ]]; then
2222
echo "Leak tools require root permissions..."
2323
sudo $RUN_TESTS $@

servers/dns_server/dns_server.py

+1-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
TXT: QTYPE.TXT,
2020
}
2121

22-
2322
class Record:
2423
def __init__(self, rdata_type, *args, rtype=None, rname=None, ttl=None, **kwargs):
2524
if isinstance(rdata_type, RD):
@@ -58,8 +57,7 @@ def as_rr(self, alt_rname):
5857
def sensible_ttl(self):
5958
if self._rtype in (QTYPE.NS, QTYPE.SOA):
6059
return 60 * 60 * 24
61-
else:
62-
return 300
60+
return 300
6361

6462
@property
6563
def is_soa(self):

servers/ip_responder/watch.sh

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#!/usr/bin/env bash
2+
3+
journalctl --lines 0 --follow _SYSTEMD_UNIT=ip_responder.service
4+

ssh_keys/.gitignore

-3
This file was deleted.

ssh_keys/keygen.sh

-3
This file was deleted.

tools/run_tests.py

+29
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,26 @@
99
import sys
1010
import tempfile
1111
import time
12+
import ipaddress
1213

1314
sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), '..'))
1415

1516
from xv_leak_tools import tools_user
17+
from xv_leak_tools.exception import XVEx
1618
from xv_leak_tools.helpers import exception_to_string, current_os
1719
from xv_leak_tools.import_helpers import import_by_filename
1820
from xv_leak_tools.log import L
1921
from xv_leak_tools.object_parser import object_from_command_line
2022
from xv_leak_tools.path import windows_safe_path, makedirs_safe
2123
from xv_leak_tools.test_execution.test_run_context import TestRunContext
2224
from xv_leak_tools.test_execution.test_runner import TestRunner
25+
from xv_leak_tools.network.macos.pf_firewall import PFCtl
26+
27+
def punch_hole_in_firewall(pf, ip):
28+
if current_os() != 'macos':
29+
raise XVEx('Editing the firewall is only supported for PF/macOS')
30+
pf.set_rules(["pass in quick from {} no state".format(ip),
31+
"pass out quick to {} no state".format(ip)])
2332

2433
def filter_tests(tests, regex):
2534
filtered_tests = []
@@ -76,13 +85,26 @@ def main(argv=None):
7685
'"INFO", "DEBUG", "VERBOSE"].')
7786
parser.add_argument(
7887
'-2', '--v2', default=False, action='store_true', help='DEPRECATED: Does nothing')
88+
parser.add_argument(
89+
'-f', '--firewall_exemption', default=None, metavar="IP", nargs='+',
90+
help='Punch a hole in the firewall for the given IP address.')
91+
parser.add_argument(
92+
'-p', '--pidfile', default=None, metavar='PATH_TO_PIDFILE', nargs=1,
93+
help='File to write the PID to.')
7994
args = parser.parse_args(argv)
8095

8196
args.log_level = args.log_level.upper() if args.log_level else None
8297
# We'll reconfigure in a second but lets get logging going asap!
8398
inital_log_level = args.log_level if args.log_level else "INFO"
8499
TestRunner.configure_logger(level=inital_log_level)
85100

101+
if args.pidfile:
102+
pid = os.getpid()
103+
path = os.path.abspath(args.pidfile.pop())
104+
L.debug("Writing PID {} to {}".format(pid, path))
105+
with open(path, 'w') as pidfile:
106+
pidfile.write(str(pid))
107+
86108
if args.output_root is None:
87109
if current_os() == "windows":
88110
args.output_root = tempfile.mkdtemp()
@@ -105,6 +127,13 @@ def main(argv=None):
105127
args.output_directory = os.path.join(args.output_root, args.run_directory)
106128
makedirs_safe(args.output_directory)
107129

130+
if args.firewall_exemption:
131+
pf = PFCtl()
132+
ips = [ipaddress.ip_network(ip).exploded for ip in args.firewall_exemption]
133+
ip_list = '{ ' + ', '.join(ips) + ' }'
134+
L.info("Punching hole for {}".format(ips))
135+
punch_hole_in_firewall(pf, ip_list)
136+
108137
context_dict = import_by_filename(args.context).CONTEXT
109138
cmd_line_overrides = [
110139
'run_directory',

tools/wrap_local_tests.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def _output_filename(input_file, output_file, output_format):
2828
if output_file:
2929
return output_file
3030

31-
return "{}.remote.{}".format(
31+
return "{}.remote.{}".format(
3232
os.path.splitext(os.path.basename(input_file))[0], output_format)
3333

3434
@staticmethod

xv_leak_tools/log.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def _configure_logger(logger_name, stream_format, file_format, logfile, level, l
7676
logger.addHandler(file_handler)
7777

7878
if stream_format:
79-
formatter = colorlog.ColoredFormatter(fmt=stream_format, log_colors=log_colors)
79+
formatter = colorlog.TTYColoredFormatter(fmt=stream_format, log_colors=log_colors)
8080
stream_handler = logging.StreamHandler(stream=sys.stdout)
8181
stream_handler.setFormatter(formatter)
8282
logger.addHandler(stream_handler)

xv_leak_tools/test_components/dns_tool/dig.py

+7-1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ def lookup(self, hostname, timeout, server=None):
7373
else:
7474
cmd = ['dig', "+time={}".format(timeout), hostname]
7575

76-
stdout = self._execute(cmd)[0]
76+
# Prevent the output from being empty
77+
stdout = None
78+
while not stdout:
79+
stdout = self._execute(cmd)[0]
80+
if not stdout:
81+
L.verbose("dig output was empty; doing another lookup.")
82+
7783
L.verbose("dig output: {}".format(stdout))
7884
return Dig._parse_output(stdout)

xv_leak_tools/test_components/firewall/macos/macos_firewall.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def block_ip(self, ip):
2222
# Delay initialize the PFCtl object to prevent VPN application connect from removing our
2323
# reference to the pf firewall. Some VPN apps take full ownership of the firewall which can
2424
# mean that the firewall will be disabled unless we initialize here.
25-
if self._pfctl == None:
25+
if self._pfctl is None:
2626
self._pfctl = PFCtl()
2727

2828
self._current_rules += MacOSFirewall._block_ip_rules(ip)

xv_leak_tools/test_components/route/route_builder.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from xv_leak_tools.factory import Builder
22
from xv_leak_tools.test_components.route.macos.macos_route import MacOSRoute
3-
from xv_leak_tools.test_components.route.windows.windows_route import WindowsRoute
43
from xv_leak_tools.test_components.route.linux.linux_route import LinuxRoute
54
from xv_leak_tools.test_components.component import ComponentNotSupported
65

@@ -16,5 +15,6 @@ def build(self, device, config):
1615
elif device.os_name() == 'macos':
1716
return MacOSRoute(device, config)
1817
elif device.os_name() == 'windows':
18+
from xv_leak_tools.test_components.route.windows.windows_route import WindowsRoute
1919
return WindowsRoute(device, config)
2020
raise ComponentNotSupported("Can't build a route component on {}".format(device.os_name()))

xv_leak_tools/test_components/route/windows/windows_route.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import re
22

3-
import netaddr
3+
import netaddr # pylint: disable=import-error
44

55
from xv_leak_tools.test_components.route.route import Route, RouteEntry
66
from xv_leak_tools.test_device.connector_helper import ConnectorHelper

xv_leak_tools/test_components/vpn_application/desktop_vpn_application.py

+37-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from abc import ABCMeta, abstractmethod
66

77
from xv_leak_tools.exception import XVEx
8+
from xv_leak_tools.helpers import current_os
89
from xv_leak_tools.log import L
910
from xv_leak_tools.network.common import RE_IPV4_ADDRESS
1011
from xv_leak_tools.test_components.vpn_application.vpn_application import VPNApplication
@@ -92,6 +93,40 @@ def detect(self):
9293
L.debug("Detected the following info about openvpn: {}".format(vpn_info))
9394
return vpn_info
9495

96+
# TODO: This is very ad-hoc for now. Some VPNs use Network Extensions to implement their VPN, e.g.
97+
# for IKEv2. Currently haven't found a reliable way to distinguish these VPNs yet so for now they
98+
# are just classes as "network extension" protocols. This will likely need work.
99+
class NEDetector(VPNDetector):
100+
101+
# pylint: disable=too-few-public-methods
102+
103+
def __init__(self, device):
104+
self._device = device
105+
self._ne_processes_cache = None
106+
107+
def _ne_processes(self):
108+
if self._ne_processes_cache:
109+
return self._ne_processes_cache
110+
self._ne_processes_cache = []
111+
self._ne_processes_cache += self._device.pgrep('neagent')
112+
return self._ne_processes_cache
113+
114+
def detect(self):
115+
if current_os() != "macos":
116+
return None
117+
118+
L.debug("Trying to determine if we're using a macOS network extension")
119+
vpn_info = VPNInfo()
120+
121+
if not self._ne_processes():
122+
L.debug('Not using a network extension')
123+
return None
124+
125+
L.info("Detected a VPN network extension (unknown protocol)")
126+
127+
vpn_info.vpn_processes = self._ne_processes()
128+
return vpn_info
129+
95130
class OpenVPNDetector(VPNDetector):
96131

97132
# pylint: disable=too-few-public-methods
@@ -335,7 +370,8 @@ def __init__(self, app_path, device, config):
335370
self._app_path = app_path
336371
self._vpn_detectors = [
337372
OpenVPNDetector(self._device),
338-
L2TPDetector(self._device)
373+
L2TPDetector(self._device),
374+
NEDetector(self._device),
339375
]
340376
self._routes_before_connect = None
341377

xv_leak_tools/test_device/connector_helper.py

+11-3
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ def _check_virtual_env(self):
2828
'''Check that virtualenv was properly setup on the device. If we don't check and there's
2929
a problem then this can lead to cryptic errors from the helper function below. Better to
3030
catch this early on.'''
31+
# source doesn't error if the sourced file is missing. This file "shouldn't" ever be
32+
# missing but let's check.
3133
try:
32-
# source doesn't error if the sourced fail is missing. This file "shouldn't" ever be
33-
# missing but let's check.
3434
self.check_command(['stat', '.pythonlocation'])
3535
self.check_command(['source', 'activate'])
36-
except XVProcessException as _:
36+
except XVProcessException:
3737
raise XVEx("Virtualenv doesn't appear to be setup properly for device '{}'".format(
3838
self._device.device_id()))
3939

@@ -56,6 +56,14 @@ def _create_temp_directory(self):
5656
# L.warning("Couldn't chown temp directory {} back to user {}".format(
5757
# self._device.temp_directory(), self._device.tools_user()))
5858

59+
def _remove_temp_directory(self):
60+
L.debug("Removing temp directory {}".format(self._device.temp_directory()))
61+
cmd = ['sudo', '-n', 'rm', '-rf', self._device.temp_directory()]
62+
ret, stdout, stderr = self._device.connector().execute(cmd)
63+
# TODO: warn instead of raising
64+
if ret != 0:
65+
raise XVProcessException(cmd, ret, stdout, stderr)
66+
5967
def check_command(self, cmd, root=False):
6068
cmd = ConnectorHelper._sanitize_cmd(cmd)
6169
ret, stdout, stderr = self.execute_command(cmd, root)

xv_leak_tools/test_device/simple_ssh_connector.py

+10-14
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
class SimpleSSHConnector(Connector):
1818

19-
# pylint: disable=too-few-public-methods,too-many-arguments
19+
# pylint: disable=too-few-public-methods,too-many-arguments,too-many-instance-attributes
2020

2121
# TODO: Account password shouldn't be needed. Paramiko is either or.
2222
def __init__(self, ips, username, account_password=None, ssh_key=None, ssh_password=None):
@@ -56,7 +56,6 @@ def _create_route_to_ip(self, ip):
5656
def _ssh_connect(self):
5757
connect_errors = ""
5858
for ip in self._ips:
59-
self._create_route_to_ip(ip)
6059
connect_dict = {
6160
'hostname': ip,
6261
'username': self._username,
@@ -159,22 +158,19 @@ def execute(self, cmd, root=False):
159158
if root and self._username.decode() != 'root':
160159
cmd = ['sudo', '-n'] + cmd
161160

161+
cmd = ["[", "-f", ".source", "]", "&&", ".", ".source;"] + cmd
162+
162163
L.verbose("SimpleSSHConnector: Executing remote command: '{}'".format(cmd))
163164

164-
chan = self._ssh_client.get_transport().open_session()
165-
chan.settimeout(2)
166165
# TODO: This will block if stdin on the remote machine blocks. Can't ctrl-c. Consider a
167166
# custom wrapper with select and timeout
168-
chan.exec_command(' '.join(cmd))
169-
bufsize = -1
170-
# stdin = chan.makefile('wb', bufsize)
171-
stdout = chan.makefile('r', bufsize)
172-
stderr = chan.makefile_stderr('r', bufsize)
173-
174-
retcode = chan.recv_exit_status()
175-
# paramiko splits output into a list AND keeps the newline. So joining like this puts it
176-
# back to how we'd "normally" expect output.
177-
return retcode, ''.join(stdout), ''.join(stderr)
167+
# get_pty is required for accessing environment variables.
168+
output = self._ssh_client.exec_command(' '.join(cmd))
169+
return_code = output[1].channel.recv_exit_status()
170+
stdout = output[1].read().decode().strip()
171+
stderr = output[2].read().decode().strip()
172+
173+
return return_code, stdout, stderr
178174

179175
def push(self, src, dst):
180176
self._ensure_connected()

xv_leak_tools/test_framework/test_case_remote.py

+19-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import json
22
import os
3+
import ipaddress
34

45
from xv_leak_tools.log import L
56
from xv_leak_tools.test_framework.test_case import TestCase
@@ -13,6 +14,18 @@ def __init__(self, devices, parameters):
1314
self.remote_test_config = self.parameters
1415
self.connector_helper = ConnectorHelper(self.test_device)
1516

17+
def host_ips(self):
18+
guest_ips = self.test_device.ips()
19+
guest_subnets = [ipaddress.ip_network(ip).supernet(new_prefix=24) for ip in guest_ips]
20+
21+
ips = set()
22+
for ip in self.test_device.local_ips():
23+
for subnet in guest_subnets:
24+
if ip in subnet:
25+
ips.add(ip.exploded)
26+
27+
return ips
28+
1629
def test(self):
1730
L.info("Running test {} completely on remote device".format(
1831
self.remote_test_config['name']))
@@ -22,10 +35,11 @@ def test(self):
2235
self.connector_helper.write_remote_file_from_contents(
2336
remote_config_path, json.dumps([self.remote_test_config]))
2437

25-
cmd = ['run_tests.py', self.test_device.output_directory(), '-c', remote_config_path]
26-
38+
cmd = ['./run_tests.sh', '-o', self.test_device.output_directory(),
39+
'-c', remote_config_path, '-x', '../contexts/internal_context.py',
40+
'-f', ' '.join(self.host_ips())]
2741
# TODO: Use .requires_root to figure out if the test needs root.
28-
ret, stdout, stderr = self.connector_helper.execute_python(cmd, root=True)
42+
ret, stdout, stderr = self.connector_helper.execute_command(cmd, root=True)
2943

3044
# TODO: Reconsider all this. Maybe we shouldn't display the output here as it screws up the
3145
# logs. Possibly better is to have ERROR log go to stderr then we just dump stderr here
@@ -38,7 +52,7 @@ def test(self):
3852
# TODO: Logger not satisfying my needs!
3953
print(line)
4054
L.info('*' * 80)
41-
L.info('END stdout from remote machine:')
55+
L.info('END stdout from remote machine')
4256
L.info('*' * 80)
4357

4458
if stderr:
@@ -49,7 +63,7 @@ def test(self):
4963
# TODO: Logger not satisfying my needs!
5064
print(line)
5165
L.error('*' * 80)
52-
L.error('END stderr from remote machine:')
66+
L.error('END stderr from remote machine')
5367
L.error('*' * 80)
5468

5569
if ret == 0:

0 commit comments

Comments
 (0)