From a0a2d57fff063b6556f0429dcf81f2ef1bf998f8 Mon Sep 17 00:00:00 2001 From: Ganesh Viswanathan Date: Tue, 14 Jun 2022 21:34:39 -0500 Subject: [PATCH] Fix #152 - gracefully handle SSL connection abort --- HISTORY.txt | 4 +++ build.sh | 24 ++++++++----- px/libcurl/_multi.py | 85 +++++++++++++++++++++++--------------------- px/main.py | 31 ++++++++-------- px/mcurl.py | 19 ++++++---- px/version.py | 2 +- test.py | 40 +++++++++++---------- 7 files changed, 116 insertions(+), 89 deletions(-) diff --git a/HISTORY.txt b/HISTORY.txt index 7e14f3c..89a1faa 100644 --- a/HISTORY.txt +++ b/HISTORY.txt @@ -1,3 +1,7 @@ +v0.7.2 - 2022-06-14 +- Fixed #152 - handle connection errors in select loop gracefully +- Fixed #151 - handle libcurl 7.29 on Centos7 + v0.7.1 - 2022-06-13 - Fixed #146 - px --install was broken when run in cmd.exe, also when run as `python -m px` diff --git a/build.sh b/build.sh index 41f2444..224dd36 100644 --- a/build.sh +++ b/build.sh @@ -14,13 +14,6 @@ if [ -f "/.dockerenv" ]; then COMMAND="$3" fi - MUSL=`ldd /bin/ls | grep musl` - if [ -z "$MUSL" ]; then - PXBIN="/px/px.dist-linux-glibc-x86_64/px.dist/px" - else - PXBIN="/px/px.dist-linux-musl-x86_64/px.dist/px" - fi - if [ "$DISTRO" = "alpine" ]; then apk update && apk upgrade @@ -40,7 +33,7 @@ if [ -f "/.dockerenv" ]; then yum install -y ccache dbus-devel libffi-devel patchelf python36-cryptography upx fi - elif [ "$DISTRO" = "ubuntu" ] || [ "$DISTRO" = "debian" ]; then + elif [ "$DISTRO" = "ubuntu" ] || [ "$DISTRO" = "debian" ] || [ "$DISTRO" = "linuxmint" ]; then apt update -y && apt upgrade -y apt install -y curl dbus gnome-keyring psmisc python3 python3-dev python3-pip @@ -56,12 +49,27 @@ if [ -f "/.dockerenv" ]; then zypper -n install gcc fi + elif [ "$DISTRO" = "void" ]; then + + xbps-install -Suy xbps + xbps-install -Sy curl dbus gcc gnome-keyring psmisc python3 python3-devel + python3 -m ensurepip + + SHELL="sh" + else echo "Unknown distro $DISTRO" $SHELL exit fi + MUSL=`ldd /bin/ls | grep musl` + if [ -z "$MUSL" ]; then + PXBIN="/px/px.dist-linux-glibc-x86_64/px.dist/px" + else + PXBIN="/px/px.dist-linux-musl-x86_64/px.dist/px" + fi + dbus-run-session -- $SHELL -c 'echo "abc" | gnome-keyring-daemon --unlock' cd /px diff --git a/px/libcurl/_multi.py b/px/libcurl/_multi.py index 93832a5..2668f52 100644 --- a/px/libcurl/_multi.py +++ b/px/libcurl/_multi.py @@ -47,6 +47,7 @@ #include "curl.h" import ctypes as ct +from xml.dom.minidom import Attr from ._platform import CFUNC, defined from ._dll import dll @@ -524,45 +525,49 @@ class waitfd(ct.Structure): (1, "sockfd"), (1, "sockp"),)) -# Name: curl_push_callback -# -# Desc: This callback gets called when a new stream is being pushed by the -# server. It approves or denies the new stream. It can also decide -# to completely fail the connection. -# -# Returns: CURL_PUSH_OK, CURL_PUSH_DENY or CURL_PUSH_ERROROUT - -CURL_PUSH_OK = 0 -CURL_PUSH_DENY = 1 -CURL_PUSH_ERROROUT = 2 # added in 7.72.0 - -# forward declaration only -class pushheaders(ct.Structure): pass - -pushheader_bynum = CFUNC(ct.c_char_p, - ct.POINTER(pushheaders), - ct.c_size_t)( - ("curl_pushheader_bynum", dll), ( - (1, "h"), - (1, "num"),)) - -pushheader_byname = CFUNC(ct.c_char_p, - ct.POINTER(pushheaders), - ct.c_char_p)( - ("curl_pushheader_byname", dll), ( - (1, "h"), - (1, "name"),)) - -# typedef int (*curl_push_callback)(CURL *parent, -# CURL *easy, -# size_t num_headers, -# ct.POINTER(curl_pushheaders) headers, -# void *userp); -push_callback = CFUNC(ct.c_int, - ct.POINTER(CURL), # parent - ct.POINTER(CURL), # easy - ct.c_size_t, # num_headers - ct.POINTER(pushheaders), # headers - ct.c_void_p) # userp +# libcurl < 7.44 +try: + # Name: curl_push_callback + # + # Desc: This callback gets called when a new stream is being pushed by the + # server. It approves or denies the new stream. It can also decide + # to completely fail the connection. + # + # Returns: CURL_PUSH_OK, CURL_PUSH_DENY or CURL_PUSH_ERROROUT + + CURL_PUSH_OK = 0 + CURL_PUSH_DENY = 1 + CURL_PUSH_ERROROUT = 2 # added in 7.72.0 + + # forward declaration only + class pushheaders(ct.Structure): pass + + pushheader_bynum = CFUNC(ct.c_char_p, + ct.POINTER(pushheaders), + ct.c_size_t)( + ("curl_pushheader_bynum", dll), ( + (1, "h"), + (1, "num"),)) + + pushheader_byname = CFUNC(ct.c_char_p, + ct.POINTER(pushheaders), + ct.c_char_p)( + ("curl_pushheader_byname", dll), ( + (1, "h"), + (1, "name"),)) + + # typedef int (*curl_push_callback)(CURL *parent, + # CURL *easy, + # size_t num_headers, + # ct.POINTER(curl_pushheaders) headers, + # void *userp); + push_callback = CFUNC(ct.c_int, + ct.POINTER(CURL), # parent + ct.POINTER(CURL), # easy + ct.c_size_t, # num_headers + ct.POINTER(pushheaders), # headers + ct.c_void_p) # userp +except AttributeError: + pass # eof diff --git a/px/main.py b/px/main.py index f52bac8..7e2fcc1 100755 --- a/px/main.py +++ b/px/main.py @@ -294,14 +294,17 @@ def handle_one_request(self): try: httpserver.BaseHTTPRequestHandler.handle_one_request(self) except socket.error as error: + self.close_connection = True + easyhash = "" + if self.curl is not None: + easyhash = self.curl.easyhash + ": " + State.mcurl.stop(self.curl) + self.curl = None if "forcibly closed" in str(error): - dprint("Connection closed by client") + dprint(easyhash + "Connection closed by client") else: traceback.print_exc(file=sys.stdout) - dprint("Socket error: %s" % error) - self.close_connection = True - State.mcurl.stop(self.curl) - self.curl = None + dprint(easyhash + "Socket error: %s" % error) def address_string(self): host, port = self.client_address[:2] @@ -312,16 +315,16 @@ def log_message(self, format, *args): dprint(format % args) def do_curl(self): - dprint("Path = " + self.path) if self.curl is None: self.curl = mcurl.Curl(self.path, self.command, self.request_version, State.socktimeout) else: self.curl.reset(self.path, self.command, self.request_version, State.socktimeout) self.curl.set_debug() + dprint(self.curl.easyhash + ": Path = " + self.path) ipport = self.get_destination() if ipport is None: - dprint("Configuring proxy settings") + dprint(self.curl.easyhash + ": Configuring proxy settings") server = self.proxy_servers[0][0] port = self.proxy_servers[0][1] ret = self.curl.set_proxy(proxy = server, port = port) @@ -336,11 +339,11 @@ def do_curl(self): key = State.username pwd = keyring.get_password("Px", key) if len(key) == 0: - dprint("Using SSPI to login") + dprint(self.curl.easyhash + ": Using SSPI to login") key = ":" self.curl.set_auth(user = key, password = pwd, auth = State.auth) else: - dprint("Skipping auth proxying") + dprint(self.curl.easyhash + ": Skipping auth proxying") # Plain HTTP can be bridged directly if not self.curl.is_connect(): @@ -353,10 +356,10 @@ def do_curl(self): self.curl.set_useragent(State.useragent) if not State.mcurl.do(self.curl): - dprint("Connection failed: " + self.curl.errstr) + dprint(self.curl.easyhash + ": Connection failed: " + self.curl.errstr) self.send_error(self.curl.resp, self.curl.errstr) elif self.curl.is_connect(): - dprint("Connected") + dprint(self.curl.easyhash + ": SSL connected") self.send_response(200, "Connection established") self.send_header("Proxy-Agent", self.version_string()) self.end_headers() @@ -377,7 +380,7 @@ def do_PAC(self): self.send_response(200) for key, value in headers.items(): - dprint("Returning %s: %s" % (key, value)) + dprint(self.curl.easyhash + ": Returning %s: %s" % (key, value)) self.send_header(key, value) self.end_headers() @@ -419,10 +422,10 @@ def get_destination(self): servers, netloc, path = State.wproxy.find_proxy_for_url( ("https://" if "://" not in self.path else "") + self.path) if servers[0] == wproxy.DIRECT: - dprint("Direct connection") + dprint(self.curl.easyhash + ": Direct connection") return netloc else: - dprint("Proxy = " + str(servers)) + dprint(self.curl.easyhash + ": Proxy = " + str(servers)) self.proxy_servers = servers return None diff --git a/px/mcurl.py b/px/mcurl.py index e86f4b9..58b2604 100644 --- a/px/mcurl.py +++ b/px/mcurl.py @@ -125,8 +125,8 @@ def _write_callback(buffer, size, nitems, userdata): if curl.sentheaders: try: tsize = curl.client_wfile.write(bytes(buffer[:tsize])) - except (ConnectionError, BrokenPipeError) as exc: - dprint(curl.easyhash + ": Write error: " + str(exc)) + except ConnectionError as exc: + dprint(curl.easyhash + ": Error writing to client: " + str(exc)) return 0 else: dprint(curl.easyhash + ": Skipped %d bytes" % tsize) @@ -160,8 +160,8 @@ def _header_callback(buffer, size, nitems, userdata): return tsize try: return curl.client_wfile.write(data) - except (ConnectionError, BrokenPipeError) as exc: - dprint("Header write error: " + str(exc)) + except ConnectionError as exc: + dprint(curl.easyhash + ": Error writing header to client: " + str(exc)) return 0 return 0 @@ -298,7 +298,7 @@ def set_proxy(self, proxy, port = 0, noproxy = None): Set proxy options - returns False if this proxy server has auth failures """ if proxy in MCURL.failed: - dprint("Authentication issues with this proxy server") + dprint(self.easyhash + ": Authentication issues with this proxy server") return False self.proxy = proxy @@ -324,7 +324,7 @@ def set_auth(self, user, password = None, auth = "ANY"): if password is not None: libcurl.easy_setopt(self.easy, libcurl.CURLOPT_PROXYPASSWORD, password.encode("utf-8")) else: - dprint("Blank password for user") + dprint(self.easyhash + ": Blank password for user") if auth is not None: self.auth = auth @@ -697,7 +697,12 @@ def select(self, curl: Curl, client_sock, idle = 30): wdata = sdata source = "client" - data = i.recv(4096) + try: + data = i.recv(4096) + except ConnectionError as exc: + # Fix #152 - handle connection errors gracefully + dprint(curl.easyhash + ": Read error from %s: " % source + str(exc)) + data = "" datalen = len(data) if datalen != 0: cl += datalen diff --git a/px/version.py b/px/version.py index 5299823..a5fb79a 100644 --- a/px/version.py +++ b/px/version.py @@ -1,3 +1,3 @@ "Px version" -__version__ = "0.7.1" +__version__ = "0.7.2" diff --git a/test.py b/test.py index a5de8b0..dd9b91f 100644 --- a/test.py +++ b/test.py @@ -19,6 +19,7 @@ import tools +COUNT = 0 PROXY = "" PORT = 3128 USERNAME = "" @@ -37,17 +38,19 @@ except AttributeError: DEVNULL = open(os.devnull, 'wb') -def exec(cmd, shell = True): - p = subprocess.run(cmd, shell = shell, stdout = subprocess.PIPE, stderr = subprocess.STDOUT, check = False, timeout = 60) - try: - data = p.stdout.decode("utf-8") - except UnicodeDecodeError: - data = "" +def exec(cmd, port = 0, shell = True): + global COUNT + log = "%d-%d.txt" % (port, COUNT) + COUNT += 1 + with open(log, "wb") as l: + p = subprocess.run(cmd, shell = shell, stdout = l, stderr = subprocess.STDOUT, check = False, timeout = 60) + with open(log, "r") as l: + data = l.read() return p.returncode, data -def curl(url, method = "GET", data = "", proxy = ""): - cmd = ["curl", "-s", "-L", "-k", url] +def curl(url, port, method = "GET", data = "", proxy = ""): + cmd = ["curl", "-s", "-k", url] if method == "HEAD": cmd.append("--head") else: @@ -60,7 +63,7 @@ def curl(url, method = "GET", data = "", proxy = ""): writeflush(" ".join(cmd) + "\n") try: - return exec(cmd, shell = False) + return exec(cmd, port, shell = False) except subprocess.TimeoutExpired: return -1, "Subprocess timed out" @@ -115,13 +118,13 @@ def checkMethod(method, port, secure = False): if method in ["PUT", "POST", "PATCH"]: data = str(uuid.uuid4()) - aret, adata = curl(url, method, data) + aret, adata = curl(url, port, method, data) if aret != 0: writeflush("%s: Curl failed direct: %d\n%s\n" % (testname, aret, adata)) return False a = filterHeaders(adata) - bret, bdata = curl(url, method, data, proxy = "localhost:" + str(port)) + bret, bdata = curl(url, port, method, data, proxy = "localhost:" + str(port)) if bret != 0: writeflush("%s: Curl failed thru proxy: %d\n%s\n" % (testname, bret, bdata)) return False @@ -174,7 +177,7 @@ def runTest(test, cmd, offset, port): if sys.platform == "win32": cmd = "cmd /c start /wait /min " + cmd if "--norun" not in sys.argv: - subp = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + subp = subprocess.Popen(cmd, shell=True, stdout=DEVNULL, stderr=DEVNULL) if testproc in [installTest, uninstallTest]: # Wait for Px to exit for these tests @@ -271,7 +274,7 @@ def checkProc(lip, pport): def checkFilter(ips, port): def checkProc(lip, port): - rcode, _ = curl(url = "http://google.com", proxy = "%s:%d" % (lip, port)) + rcode, _ = curl(url = "http://google.com", port = port, proxy = "%s:%d" % (lip, port)) writeflush("Returned %d\n" % rcode) if rcode == 0: return True @@ -323,10 +326,9 @@ def httpTest(skip, port): del skip return run(port) -def installTest(cmd, skip): - del skip +def installTest(cmd, port): time.sleep(1) - ret, data = exec("reg query HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\Run /v Px") + ret, data = exec("reg query HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\Run /v Px", port) if ret != 0: writeflush("Failed: registry query failed: %d\n%s\n" % (ret, data)) return False @@ -335,10 +337,10 @@ def installTest(cmd, skip): return False return True -def uninstallTest(cmd, skip): +def uninstallTest(skip, port): del skip time.sleep(1) - ret, data = exec("reg query HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\Run /v Px") + ret, data = exec("reg query HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\Run /v Px", port) if ret == 0: writeflush("reg query passed after uninstall\n%s\n" % data) return False @@ -350,7 +352,7 @@ def quitTest(cmd, port): return False writeflush("cmd: " + cmd + "--quit\n") - ret, data = exec(cmd + "--quit") + ret, data = exec(cmd + "--quit", port) if ret != 0 or "Quitting Px .. DONE" not in data: writeflush("Failed: Unable to --quit Px: %d\n%s\n" % (ret, data + "\n")) return False