feat: add support for SOCKS5 proxy (#1063)

JoshuaMoelans · supervacuus · web-flow · commit 99b1539dc518 · 2024-11-21T16:14:29.000+01:00
* Added socks5_proxy to options

* correct options freeing

* Update CHANGELOG.md

* Add socks5_proxy attribute to curl_transport_state

* formatting

* refactor to use single proxy attribute

* format

* Added back deprecated http_proxy API

* formatting

* refactor sentry_transport_winhttp.c to use opts-&gt;proxy

* formatting

* update crashpad dependency

* fix: update minimum libcurl version check to 7.21.7

* update crashpad dependency

* test: add proxy integration tests

* get latest crashpad changes

* feat: add example.c run-args for http and socks5 proxy

* test: changed test name + added more mitmdump startup time

* chore: format

* test: add mitmproxy to requirements.txt

* test: add skip for proxy test on non-MacOS platforms

* test: move socksproxy-off assert into AssertionError check for Linux test

* test: change expected http log length to match platform

* test: order of operations

* test: run http proxy tests on all platforms

* test: add special case for Windows Autoproxy

* apply suggestions from code review

* chore: added comments about proxy behaviour

* cleaned up test and moved comments to proper place
(let's hope I didn't mess up CI)

* chore: fix minor comment typo

* chore: added api documentation

* Update include/sentry.h

Co-authored-by: Mischan Toosarani-Hausberger &lt;mischan@abovevacant.com&gt;

* Update CONTRIBUTING.md

Co-authored-by: Mischan Toosarani-Hausberger &lt;mischan@abovevacant.com&gt;

* update crashpad submodule

* update crashpad submodule (again)

* updated CHANGELOG.md

* updated CHANGELOG.md again

---------

Co-authored-by: Mischan Toosarani-Hausberger &lt;mischan@abovevacant.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## Unreleased
+
+**Features**:
+- Add SOCKS5 proxy support for macOS and Linux. ([#1063](https://github.com/getsentry/sentry-native/pull/1063))
+
 ## 0.7.15
 
 **Fixes**:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -146,6 +146,8 @@ The example currently supports the following commands:
 - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event.
 - `override-sdk-name`: Changes the SDK name via the options at runtime.
 - `stack-overflow`: Provokes a stack-overflow.
+- `http-proxy`: Uses a localhost `HTTP` proxy on port 8080.
+- `socks5-proxy`: Uses a localhost `SOCKS5` proxy on port 1080.
 
 Only on Windows using crashpad with its WER handler module:
 
diff --git a/examples/example.c b/examples/example.c
@@ -236,6 +236,14 @@ main(int argc, char **argv)
         sentry_options_set_sdk_name(options, "sentry.native.android.flutter");
     }
 
+    if (has_arg(argc, argv, "http-proxy")) {
+        sentry_options_set_proxy(options, "http://127.0.0.1:8080");
+    }
+
+    if (has_arg(argc, argv, "socks5-proxy")) {
+        sentry_options_set_proxy(options, "socks5://127.0.0.1:1080");
+    }
+
     sentry_init(options);
 
     if (!has_arg(argc, argv, "no-setup")) {
diff --git a/external/crashpad b/external/crashpad
@@ -1 +1 @@
-Subproject commit b95f5c6c47ebd0511970df45f42cd866840454cc
+Subproject commit 852f8fea9cbea7a63610b8c698f301bd1ba4b3cd
diff --git a/include/sentry.h b/include/sentry.h
@@ -972,17 +972,43 @@ SENTRY_API void sentry_options_set_dist_n(
 SENTRY_API const char *sentry_options_get_dist(const sentry_options_t *opts);
 
 /**
- * Configures the http proxy.
+ * Configures the proxy.
  *
- * The given proxy has to include the full scheme, eg. `http://some.proxy/`.
+ * The given proxy has to include the full scheme,
+ * eg. `http://some.proxy/` or `socks5://some.proxy/`.
+ *
+ * Not every transport behaves the same way when configuring a proxy.
+ * On Windows if a transport can't connect to the proxy it will fall back on a
+ * connection without proxy. This is also true for the crashpad_handler
+ * transport on macOS for a socks proxy, but not for a http proxy.
+ * All transports that use libcurl (Linux and the Native SDK transport on macOS)
+ * will honor the proxy settings and not fall back.
+ */
+SENTRY_API void sentry_options_set_proxy(
+    sentry_options_t *opts, const char *proxy);
+SENTRY_API void sentry_options_set_proxy_n(
+    sentry_options_t *opts, const char *proxy, size_t proxy_len);
+
+/**
+ * Returns the configured proxy.
+ */
+SENTRY_API const char *sentry_options_get_proxy(const sentry_options_t *opts);
+
+/**
+ * Configures the proxy.
+ *
+ * This is a **deprecated** alias for `sentry_options_set_proxy(_n)`.
+ *
+ * The given proxy has to include the full scheme,
+ * eg. `http://some.proxy/.
  */
 SENTRY_API void sentry_options_set_http_proxy(
     sentry_options_t *opts, const char *proxy);
 SENTRY_API void sentry_options_set_http_proxy_n(
     sentry_options_t *opts, const char *proxy, size_t proxy_len);
 
 /**
- * Returns the configured http proxy.
+ * Returns the configured proxy.
  */
 SENTRY_API const char *sentry_options_get_http_proxy(
     const sentry_options_t *opts);
diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp
@@ -440,8 +440,8 @@ crashpad_backend_startup(
         SENTRY_TRACEF("using minidump URL \"%s\"", minidump_url);
     }
     bool success = data->client->StartHandler(handler, database, database,
-        minidump_url ? minidump_url : "",
-        options->http_proxy ? options->http_proxy : "", annotations, arguments,
+        minidump_url ? minidump_url : "", options->proxy ? options->proxy : "",
+        annotations, arguments,
         /* restartable */ true,
         /* asynchronous_start */ false, attachments);
     sentry_free(minidump_url);
diff --git a/src/sentry_options.c b/src/sentry_options.c
@@ -90,7 +90,7 @@ sentry_options_free(sentry_options_t *opts)
     sentry_free(opts->user_agent);
     sentry_free(opts->environment);
     sentry_free(opts->dist);
-    sentry_free(opts->http_proxy);
+    sentry_free(opts->proxy);
     sentry_free(opts->ca_certs);
     sentry_free(opts->transport_thread_name);
     sentry__path_free(opts->database_path);
@@ -235,25 +235,44 @@ sentry_options_get_dist(const sentry_options_t *opts)
     return opts->dist;
 }
 
+void
+sentry_options_set_proxy_n(
+    sentry_options_t *opts, const char *proxy, size_t proxy_len)
+{
+    sentry_free(opts->proxy);
+    opts->proxy = sentry__string_clone_n(proxy, proxy_len);
+}
+
+void
+sentry_options_set_proxy(sentry_options_t *opts, const char *proxy)
+{
+    sentry_free(opts->proxy);
+    opts->proxy = sentry__string_clone(proxy);
+}
+
+const char *
+sentry_options_get_proxy(const sentry_options_t *opts)
+{
+    return opts->proxy;
+}
+
 void
 sentry_options_set_http_proxy_n(
     sentry_options_t *opts, const char *proxy, size_t proxy_len)
 {
-    sentry_free(opts->http_proxy);
-    opts->http_proxy = sentry__string_clone_n(proxy, proxy_len);
+    sentry_options_set_proxy_n(opts, proxy, proxy_len);
 }
 
 void
 sentry_options_set_http_proxy(sentry_options_t *opts, const char *proxy)
 {
-    sentry_free(opts->http_proxy);
-    opts->http_proxy = sentry__string_clone(proxy);
+    sentry_options_set_proxy(opts, proxy);
 }
 
 const char *
 sentry_options_get_http_proxy(const sentry_options_t *opts)
 {
-    return opts->http_proxy;
+    return sentry_options_get_proxy(opts);
 }
 
 void
diff --git a/src/sentry_options.h b/src/sentry_options.h
@@ -35,7 +35,7 @@ typedef struct sentry_options_s {
     char *release;
     char *environment;
     char *dist;
-    char *http_proxy;
+    char *proxy;
     char *ca_certs;
     char *transport_thread_name;
     char *sdk_name;
diff --git a/src/transports/sentry_transport_curl.c b/src/transports/sentry_transport_curl.c
@@ -17,7 +17,7 @@ typedef struct curl_transport_state_s {
     sentry_dsn_t *dsn;
     CURL *curl_handle;
     char *user_agent;
-    char *http_proxy;
+    char *proxy;
     char *ca_certs;
     sentry_rate_limiter_t *ratelimiter;
     bool debug;
@@ -54,7 +54,7 @@ sentry__curl_bgworker_state_free(void *_state)
     sentry__rate_limiter_free(state->ratelimiter);
     sentry_free(state->ca_certs);
     sentry_free(state->user_agent);
-    sentry_free(state->http_proxy);
+    sentry_free(state->proxy);
     sentry_free(state);
 }
 
@@ -85,7 +85,7 @@ sentry__curl_transport_start(
         };
 
         if (!sentry__check_min_version(
-                curl_version, (sentry_version_t) { 7, 10, 7 })) {
+                curl_version, (sentry_version_t) { 7, 21, 7 })) {
             SENTRY_WARNF("`libcurl` is at unsupported version `%u.%u.%u`",
                 curl_version.major, curl_version.minor, curl_version.patch);
             return 1;
@@ -101,7 +101,7 @@ sentry__curl_transport_start(
     curl_bgworker_state_t *state = sentry__bgworker_get_state(bgworker);
 
     state->dsn = sentry__dsn_incref(options->dsn);
-    state->http_proxy = sentry__string_clone(options->http_proxy);
+    state->proxy = sentry__string_clone(options->proxy);
     state->user_agent = sentry__string_clone(options->user_agent);
     state->ca_certs = sentry__string_clone(options->ca_certs);
     state->curl_handle = curl_easy_init();
@@ -215,8 +215,8 @@ sentry__curl_send_task(void *_envelope, void *_state)
     curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&info);
     curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, header_callback);
 
-    if (state->http_proxy) {
-        curl_easy_setopt(curl, CURLOPT_PROXY, state->http_proxy);
+    if (state->proxy) {
+        curl_easy_setopt(curl, CURLOPT_PROXY, state->proxy);
     }
     if (state->ca_certs) {
         curl_easy_setopt(curl, CURLOPT_CAINFO, state->ca_certs);
diff --git a/src/transports/sentry_transport_winhttp.c b/src/transports/sentry_transport_winhttp.c
@@ -69,9 +69,8 @@ sentry__winhttp_transport_start(
     sentry__bgworker_setname(bgworker, opts->transport_thread_name);
 
     // ensure the proxy starts with `http://`, otherwise ignore it
-    if (opts->http_proxy
-        && strstr(opts->http_proxy, "http://") == opts->http_proxy) {
-        const char *ptr = opts->http_proxy + 7;
+    if (opts->proxy && strstr(opts->proxy, "http://") == opts->proxy) {
+        const char *ptr = opts->proxy + 7;
         const char *slash = strchr(ptr, '/');
         if (slash) {
             char *copy = sentry__string_clone_n(ptr, slash - ptr);
diff --git a/tests/__init__.py b/tests/__init__.py
@@ -8,6 +8,7 @@
 import pytest
 import pprint
 import textwrap
+import socket
 
 sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 
@@ -33,6 +34,14 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456):
     )
 
 
+def is_proxy_running(host, port):
+    try:
+        with socket.create_connection((host, port), timeout=1):
+            return True
+    except ConnectionRefusedError:
+        return False
+
+
 def run(cwd, exe, args, env=dict(os.environ), **kwargs):
     __tracebackhide__ = True
     if os.environ.get("ANDROID_API"):
diff --git a/tests/requirements.txt b/tests/requirements.txt
@@ -5,3 +5,4 @@ msgpack==1.0.8
 pytest-xdist==3.5.0
 clang-format==19.1.3
 pywin32==308; sys_platform == "win32"
+mitmproxy==11.0.0
diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py
@@ -1,11 +1,12 @@
 import os
 import shutil
+import subprocess
 import sys
 import time
 
 import pytest
 
-from . import make_dsn, run, Envelope
+from . import make_dsn, run, Envelope, is_proxy_running
 from .assertions import (
     assert_crashpad_upload,
     assert_session,
@@ -38,6 +39,78 @@ def test_crashpad_capture(cmake, httpserver):
     assert len(httpserver.log) == 2
 
 
+@pytest.mark.parametrize(
+    "run_args",
+    [
+        pytest.param(["http-proxy"]),  # HTTP proxy test runs on all platforms
+        pytest.param(
+            ["socks5-proxy"],
+            marks=pytest.mark.skipif(
+                sys.platform not in ["darwin", "linux"],
+                reason="SOCKS5 proxy tests are only supported on macOS and Linux",
+            ),
+        ),
+    ],
+)
+@pytest.mark.parametrize("proxy_status", [(["off"]), (["on"])])
+def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_status):
+    if not shutil.which("mitmdump"):
+        pytest.skip("mitmdump is not installed")
+
+    proxy_process = None  # store the proxy process to terminate it later
+
+    try:
+        if proxy_status == ["on"]:
+            # start mitmdump from terminal
+            if run_args == ["http-proxy"]:
+                proxy_process = subprocess.Popen(["mitmdump"])
+                time.sleep(5)  # Give mitmdump some time to start
+                if not is_proxy_running("localhost", 8080):
+                    pytest.fail("mitmdump (HTTP) did not start correctly")
+            elif run_args == ["socks5-proxy"]:
+                proxy_process = subprocess.Popen(["mitmdump", "--mode", "socks5"])
+                time.sleep(5)  # Give mitmdump some time to start
+                if not is_proxy_running("localhost", 1080):
+                    pytest.fail("mitmdump (SOCKS5) did not start correctly")
+
+        tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"})
+
+        # make sure we are isolated from previous runs
+        shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True)
+
+        env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver))
+        httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data(
+            "OK"
+        )
+
+        try:
+            with httpserver.wait(timeout=10) as waiting:
+                child = run(
+                    tmp_path, "sentry_example", ["log", "crash"] + run_args, env=env
+                )
+                assert child.returncode  # well, it's a crash after all
+        except AssertionError:
+            # we fail on macOS/Linux if the http proxy is not running
+            if run_args == ["http-proxy"] and proxy_status == ["off"]:
+                assert len(httpserver.log) == 0
+                return
+            # we only fail on linux if the socks5 proxy is not running
+            elif run_args == ["socks5-proxy"] and proxy_status == ["off"]:
+                assert len(httpserver.log) == 0
+                return
+
+        assert waiting.result
+
+        # Apple's NSURLSession will send the request even if the socks proxy fails
+        # https://forums.developer.apple.com/forums/thread/705504?answerId=712418022#712418022
+        # Windows also provides fallback for http proxies
+        assert len(httpserver.log) == 1
+    finally:
+        if proxy_process:
+            proxy_process.terminate()
+            proxy_process.wait()
+
+
 def test_crashpad_reinstall(cmake, httpserver):
     tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"})
 
diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py
diff --git a/tests/unit/test_uninit.c b/tests/unit/test_uninit.c
