Merge pull request #1515 from guardicore/proxy-test

Fix proxy schema for tunneling

monkey/infection_monkey/control.py

@@ -22,6 +22,7 @@ from infection_monkey.config import GUID, WormConfiguration
 from infection_monkey.network.info import local_ips
 from infection_monkey.transport.http import HTTPConnectProxy
 from infection_monkey.transport.tcp import TcpProxy
+from infection_monkey.utils.environment import is_windows_os
 
 requests.packages.urllib3.disable_warnings()
 
@@ -35,8 +36,6 @@ PBA_FILE_DOWNLOAD = "https://%s/api/pba/download/%s"
 # elsewhere.
 TIMEOUT_IN_SECONDS = 15
 
-PROXY_SCHEMA = "%s:%s"
-
 
 class ControlClient(object):
     proxies = {}
@@ -113,14 +112,32 @@ class ControlClient(object):
                 logger.info("Starting tunnel lookup...")
                 proxy_find = tunnel.find_tunnel(default=default_tunnel)
                 if proxy_find:
-                    proxy_address, proxy_port = proxy_find
+                    ControlClient.set_proxies(proxy_find)
-                    logger.info("Found tunnel at %s:%s" % (proxy_address, proxy_port))
-                    ControlClient.proxies["https"] = PROXY_SCHEMA % (proxy_address, proxy_port)
                     return ControlClient.find_server()
                 else:
                     logger.info("No tunnel found")
                     return False
 
+    @staticmethod
+    def set_proxies(proxy_find):
+        """
+        Note: The proxy schema changes between different versions of requests and urllib3,
+        which causes the machine to not open a tunnel back.
+        If we get "ValueError: check_hostname requires server_hostname" or
+        "Proxy URL had not schema, should start with http:// or https://" errors,
+        the proxy schema needs to be changed.
+        Keep this in mind when upgrading to newer python version or when urllib3 and
+        requests are updated there is possibility that the proxy schema is changed.
+        https://github.com/psf/requests/issues/5297
+        https://github.com/psf/requests/issues/5855
+        """
+        proxy_address, proxy_port = proxy_find
+        logger.info("Found tunnel at %s:%s" % (proxy_address, proxy_port))
+        if is_windows_os():
+            ControlClient.proxies["https"] = f"http://{proxy_address}:{proxy_port}"
+        else:
+            ControlClient.proxies["https"] = f"{proxy_address}:{proxy_port}"
+
     @staticmethod
     def keepalive():
         if not WormConfiguration.current_server:

monkey/infection_monkey/network/httpfinger.py

@@ -44,7 +44,7 @@ class HTTPFinger(HostFinger):
                         logger.info("Port %d is open on host %s " % (port[0], host))
                         break  # https will be the same on the same port
                 except Timeout:
-                    logger.debug(f"Timout while requesting headers from {url}")
+                    logger.debug(f"Timeout while requesting headers from {url}")
                 except ConnectionError:  # Someone doesn't like us
                     logger.debug(f"Connection error while requesting headers from {url}")
 

monkey/tests/unit_tests/infection_monkey/test_control.py

@@ -0,0 +1,16 @@
+import pytest
+
+from monkey.infection_monkey.control import ControlClient
+
+
+@pytest.mark.parametrize(
+    "is_windows_os,expected_proxy_string",
+    [(True, "http://8.8.8.8:45455"), (False, "8.8.8.8:45455")],
+)
+def test_control_set_proxies(monkeypatch, is_windows_os, expected_proxy_string):
+    monkeypatch.setattr("monkey.infection_monkey.control.is_windows_os", lambda: is_windows_os)
+    control_client = ControlClient()
+
+    control_client.set_proxies(("8.8.8.8", "45455"))
+
+    assert control_client.proxies["https"] == expected_proxy_string