Repository navigation
feat(mitm): add setSocketMark to the TPROXY addon (anti-loop primitive) - #4160
Conversation
The OUTPUT-based TPROXY recipe (#4156) marks all new local outbound conns to the target port — including the proxy's OWN upstream connections when it forwards, which would re-enter TPROXY (infinite loop). setSocketMark(fd, mark) lets the listener stamp SO_MARK on its upstream sockets so the mangle OUTPUT rule (-m mark ! --mark <bypass>) excludes them. Viability validated on the VPS (root): setSocketMark works on a fd, and a Node outbound socket's fd is reachable via socket._handle.fd, so the listener can mark its forward connections. The loader now requires both addon functions; tests + the addon build updated. 8/8 wrapper tests green.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new setSocketMark capability to the transparent socket TPROXY addon, allowing the proxy to set SO_MARK on its upstream connections to prevent routing loops. This includes the native C implementation, TypeScript bindings, and corresponding unit tests. The review feedback highlights a critical stability issue in the native SetSocketMark function, where a lack of argument validation (checking argc and the return values of napi_get_value_int32) could lead to uninitialized memory access and potential segmentation faults. Implementing the suggested validation is highly recommended to ensure robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| static napi_value SetSocketMark(napi_env env, napi_callback_info info) { | ||
| size_t argc = 2; | ||
| napi_value argv[2]; | ||
| napi_get_cb_info(env, info, &argc, argv, NULL, NULL); | ||
| int32_t fd = -1, mark = 0; | ||
| napi_get_value_int32(env, argv[0], &fd); | ||
| napi_get_value_int32(env, argv[1], &mark); | ||
| if (setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(mark)) < 0) { | ||
| THROW(env, "ESO_MARK", strerror(errno)); | ||
| } | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
In Node-API, if the JavaScript caller passes fewer than 2 arguments, napi_get_cb_info will update argc to the actual number of arguments passed, leaving the remaining elements of argv uninitialized. Accessing argv[0] or argv[1] when argc < 2 passes uninitialized garbage to napi_get_value_int32, which can cause a segmentation fault and crash the Node.js process.
To prevent this, we should validate that argc is at least 2, and also check the return status of napi_get_value_int32 to ensure the arguments are valid integers.
static napi_value SetSocketMark(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value argv[2];
napi_get_cb_info(env, info, &argc, argv, NULL, NULL);
if (argc < 2) {
THROW(env, "EINVAL", "Expected 2 arguments");
}
int32_t fd = -1, mark = 0;
if (napi_get_value_int32(env, argv[0], &fd) != napi_ok ||
napi_get_value_int32(env, argv[1], &mark) != napi_ok) {
THROW(env, "EINVAL", "Invalid argument types, expected integers");
}
if (setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(mark)) < 0) {
THROW(env, "ESO_MARK", strerror(errno));
}
return NULL;
}…e) (cheaper-inference#4160) Integrated into release/v3.8.29
Summary
Follow-up to #4148 — this commit landed just after #4148 was merged, so it didn't make it in; re-applied here.
Adds
setSocketMark(fd, mark)to the TPROXY native addon (setsockopt(SO_MARK)). The OUTPUT-based recipe (#4156) marks all new local outbound conns to the target port, which would include the proxy's own upstream connections when it forwards → re-entering TPROXY (infinite loop). The listener will stampSO_MARK(bypass) on its upstream sockets so themangle OUTPUTrule (-m mark ! --mark <bypass>) excludes them.Anti-loop viability validated on the VPS (root)
The listener can read a Node outbound socket's fd (
socket._handle.fd) and mark it — confirming the anti-loop is implementable. The loader now requires both addon functions; the wrapper throws clearly when unavailable. Addon not committed (build/git-ignored) → CI exercises the graceful path.Test Plan
node --test tproxy-transparent-socket.test.ts— 8/8npm run build:native:tproxycompiles with the new exporttypecheck:core— 0 ·eslint— 0🤖 Generated with Claude Code