From 99b1539dc51841da208a052ffd93b05c7784b282 Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 21 Nov 2024 16:14:29 +0100 Subject: [PATCH] feat: add support for SOCKS5 proxy (#1063) * 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->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 * Update CONTRIBUTING.md Co-authored-by: Mischan Toosarani-Hausberger * update crashpad submodule * update crashpad submodule (again) * updated CHANGELOG.md * updated CHANGELOG.md again --------- Co-authored-by: Mischan Toosarani-Hausberger --- CHANGELOG.md | 5 ++ CONTRIBUTING.md | 2 + examples/example.c | 8 +++ external/crashpad | 2 +- include/sentry.h | 32 +++++++++- src/backends/sentry_backend_crashpad.cpp | 4 +- src/sentry_options.c | 31 ++++++++-- src/sentry_options.h | 2 +- src/transports/sentry_transport_curl.c | 12 ++-- src/transports/sentry_transport_winhttp.c | 5 +- tests/__init__.py | 9 +++ tests/requirements.txt | 1 + tests/test_integration_crashpad.py | 75 ++++++++++++++++++++++- tests/test_integration_http.py | 65 +++++++++++++++++++- tests/unit/test_uninit.c | 2 +- 15 files changed, 230 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84fe2539f..20afa9699 100644 --- 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 index 2bdf6e097..041d99d24 100644 --- 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 index 750dffec1..4065cd40b 100644 --- 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 index b95f5c6c4..852f8fea9 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit b95f5c6c47ebd0511970df45f42cd866840454cc +Subproject commit 852f8fea9cbea7a63610b8c698f301bd1ba4b3cd diff --git a/include/sentry.h b/include/sentry.h index a0150d1ef..8b2c779db 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -972,9 +972,35 @@ 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); @@ -982,7 +1008,7 @@ 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 index 38e92c9de..e906fb4d3 100644 --- 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 index a18bf1894..38b2346af 100644 --- 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 index 5359e8009..07a2fdbe6 100644 --- 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 index 11f87e5d2..1912c4bbd 100644 --- 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 index ffb114eb9..c15eb5899 100644 --- 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 index 61380f90b..98e6218d1 100644 --- 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 index 306be91e8..50002c903 100644 --- 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 index 0340cfbb2..a0ffad812 100644 --- 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 index 14ff794ae..dbe4b7593 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -2,12 +2,14 @@ import json import os import shutil +import sys import time import uuid +import subprocess import pytest -from . import make_dsn, run, Envelope +from . import make_dsn, run, Envelope, is_proxy_running from .assertions import ( assert_attachment, assert_meta, @@ -607,3 +609,64 @@ def test_capture_minidump(cmake, httpserver): assert_attachment(envelope) assert_minidump(envelope) + + +@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_capture_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": "none"}) + + # make sure we are isolated from previous runs + shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) + + httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK") + + run( + tmp_path, + "sentry_example", + ["log", "start-session", "capture-event"] + + run_args, # only passes if given proxy is running + check=True, + env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)), + ) + if proxy_status == ["on"]: + assert len(httpserver.log) == 2 + elif proxy_status == ["off"]: + # Windows will send the request even if the proxy is not running + # macOS/Linux will not send the request if the proxy is not running + assert len(httpserver.log) == (2 if (sys.platform == "win32") else 0) + finally: + if proxy_process: + proxy_process.terminate() + proxy_process.wait() diff --git a/tests/unit/test_uninit.c b/tests/unit/test_uninit.c index bf9577ce7..5251344f1 100644 --- a/tests/unit/test_uninit.c +++ b/tests/unit/test_uninit.c @@ -63,7 +63,7 @@ SENTRY_TEST(invalid_dsn) SENTRY_TEST(invalid_proxy) { sentry_options_t *options = sentry_options_new(); - sentry_options_set_http_proxy(options, "invalid"); + sentry_options_set_proxy(options, "invalid"); TEST_CHECK(sentry_init(options) == 0);