mirror of
https://github.com/coturn/coturn.git
synced 2026-10-24 00:08:57 +03:00
dependabot/github_actions/github/codeql-action-4.39.0
2134 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
dependabot[bot]
|
2d25099522 |
build(deps): bump github/codeql-action from 4.38.3 to 4.39.0
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.38.3 to 4.39.0. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v4.38.3...v4.39.0) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.39.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
dependabot[bot]
|
596c48bff2 |
build(deps): bump github/codeql-action from 4.38.2 to 4.38.3 (#2098)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.38.2 to 4.38.3. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.38.3</h2> <ul> <li><em>Upcoming breaking change</em>: CodeQL version 2.21.2 and earlier were discontinued on 24 September 2026 alongside GitHub Enterprise Server 3.17, and will be unsupported by the next minor release of the CodeQL Action. Added a deprecation warning for customers using these versions of CodeQL. <a href="https://redirect.github.com/github/codeql-action/pull/4188">#4188</a></li> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.2">2.27.2</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4203">#4203</a></li> <li>Fixed a bug where the decision of whether to use a per-language bundle did not account for custom configurations that reference queries outside of compiled CodeQL packs. This issue was caught during internal testing and did not affect any customer repositories. We will resume the roll out of per-language bundles in the coming weeks. <a href="https://redirect.github.com/github/codeql-action/pull/4184">#4184</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.38.3 - 08 Oct 2026</h2> <ul> <li><em>Upcoming breaking change</em>: CodeQL version 2.21.2 and earlier were discontinued on 24 September 2026 alongside GitHub Enterprise Server 3.17, and will be unsupported by the next minor release of the CodeQL Action. Added a deprecation warning for customers using these versions of CodeQL. <a href="https://redirect.github.com/github/codeql-action/pull/4188">#4188</a></li> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.2">2.27.2</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4203">#4203</a></li> <li>Fixed a bug where the decision of whether to use a per-language bundle did not account for custom configurations that reference queries outside of compiled CodeQL packs. This issue was caught during internal testing and did not affect any customer repositories. We will resume the roll out of per-language bundles in the coming weeks. <a href="https://redirect.github.com/github/codeql-action/pull/4184">#4184</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
Pavel Punsky
|
fc8eb910fb |
Fix OAuth token validity on TURN refresh (#2097)
## Summary Fix OAuth TURN token handling : - Revalidate the presented access token on every Allocate and Refresh, even when the session already has a cached MAC key. - Track the token's absolute expiry and cap each response lifetime to the time still available. - Reject tokens with zero lifetime or timestamps outside the permitted clock-skew window, and reject empty or malformed AS-RS key material. ## Validation - macOS CMake build and CTest: 21/21 passed. - macOS TURN system scripts: core, configuration, mobility, multiplex peer, stateless nonce, IPv6 relay, and DTLS default passed. RFC 5780 skipped because a second loopback address was unavailable. - New OAuth end-to-end probe passed, including Refresh after expiry and malformed-key cases. - ASan fuzzing smoke tests passed for both local targets. - Linux CMake build and CTest: 20/20 passed. The OAuth probe and all required Linux system scripts passed, including RFC 5780 and IPv6 relay. - Packaged Debian Docker image Bats tests: 19 tests, 0 failures, 2 expected skips. - clang-format 15 check passed for every changed C/H file. |
||
|
Pavel Punsky
|
ac601f832d |
key OAuth MESSAGE-INTEGRITY with the whole mac_key (#2096)
RFC 7635 Section 6.2 keys HMAC-SHA1 MESSAGE-INTEGRITY with the token's full 160-bit mac_key. The OAuth path went through the long-term-credential helpers, which hash only get_hmackey_size(SHA1) = 16 bytes, so requests from RFC 7635 compliant clients failed with 401 and coturn's responses could not be verified by them. turnutils_uclient shared the truncation, so the bundled client never noticed. Add integrity helpers that take an explicit key length and use the whole mac_key for OAuth sessions, both when checking requests (userdb and check_stun_auth) and when signing responses. --oauth-legacy-mac-key on turnserver (and on turnutils_uclient) restores the 16-byte truncation for deployments that depend on it; the server logs a warning when it is on. A mac_key whose last 4 bytes are zero hashes the same in both modes. Fixes #2091 and #644 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
ba8635d646 |
Keep the last good peer IP lists when the DB refresh fails (#2094)
## What Every 5 seconds the auth thread calls `update_white_and_black_lists()` to reload the dynamic `allowed_peer_ip` / `denied_peer_ip` tables. `get_ip_list()` ignored the driver's return code and always handed back a list, so when the DB read failed the refresh swapped in an **empty** list and freed the previous rules. A failed read looked the same as an intentionally empty policy. The fix: - `update_white_and_black_lists()` now keeps the last good allowed/denied lists when the driver reports an error, and logs `Cannot read ... peer IP list from the DB; keeping the previous one`. - `get_ip_list()` (used by the CLI/web admin pages) keeps its existing behavior: it still returns an empty list on failure. - Two drivers reported success on real failures, and now return `-1`: - **Redis**: an error reply to `KEYS` or `SMEMBERS`, or a `SMEMBERS` that gets no reply (dropped connection mid-read). Before, any of these produced a partial or empty list. - **MongoDB**: a missing `realm` collection (no connection), and a cursor error during iteration (now checked with `mongoc_cursor_error`). - The SQLite, PostgreSQL and MySQL drivers already returned `-1` on failure. ## Why Reported in GHSA-73m9-w4hw-4m4q (CWE-755) by Wenhao Wu. During a refresh that fails (a locked SQLite file, or a MySQL/PostgreSQL timeout or restart), every entry that exists only in the dynamic denied list is dropped from memory until the next successful refresh. An authenticated client can `CREATE-PERMISSION` to a denied peer in that window. Permissions aren't rechecked against the ACL later, so the permission outlives the list's recovery. Static `--denied-peer-ip` rules are not affected, and the attacker needs valid TURN credentials plus a DB read failure. So this is Low severity. ## Notes for reviewers - I chose "keep last good" over "fail closed" (deny every peer while the DB is unreachable). Fail-closed would turn a DB blip into a relay outage. Keeping the last good lists matches what the refresh would have loaded if the DB had answered. - If the very first load at startup fails, the lists stay empty until a refresh succeeds. That is the same as before. - The allowed list gets the same treatment, so a DB failure no longer drops whitelist entries either. ## Validation - **New unit test** `test_ip_list_read_failure_reported` (`tests/test_sqlite_dbd.c`): with the DB held under `BEGIN EXCLUSIVE`, the SQLite driver returns `-1` instead of an empty list. After `COMMIT` it returns the row. - **End-to-end repro of the advisory** (SQLite userdb, `denied_peer_ip = 127.0.0.1`, `--allow-loopback-peers`, static `--user` so auth works while the DB is locked; lock held across a refresh tick): | Build | DB healthy | DB locked | |---|---|---| | master | 403 | **allowed** (data relayed to the denied peer) | | this PR | 403 | 403, server logs "keeping the previous one" | - **Unit tests**: 21/21 on macOS and 20/20 on Linux (container). - **Linux container**: all 8 `examples/run_tests*.sh` scripts passed with no `FAIL` lines. - **Fuzzing smoke tests**: `fuzzing/run-local.sh ASan 0/1 -runs=1` passed. - **Formatting**: clean with `clang-format` 15. - **MongoDB driver**: CMake doesn't build it in CI or locally, so `dbd_mongo.c` was compiled on its own against Debian's `libmongoc-dev`. No new warnings. It has not been run against a live MongoDB, and the Redis error paths have not been run against a live Redis. - The packaged Docker image (bats) tests were not run, because only the userdb refresh and the driver error paths changed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
d8107aff80 |
build: pin openssl@3 on macOS CI and reject mixed OpenSSL majors (#2095)
## Problem Every macOS job in CI currently fails: `turnserver` segfaults on the first TLS client in `examples/run_tests.sh` (e.g. run 37231739523 on #2093, which only touches `dbd_sqlite.c`). Cause: Homebrew's `openssl` formula now resolves to `openssl@4` (4.0.3), but the libevent bottle's `libevent_openssl` still links `openssl@3`. coturn builds against OpenSSL 4 and passes its `SSL*` to libevent running on OpenSSL 3 via `bufferevent_openssl_socket_new`, which crashes in `SSL_set_bio`. macOS two-level namespaces keep both copies loaded. ## Changes - `.github/workflows/macos.yml`: install `openssl@3` and export `OPENSSL_ROOT_DIR` / `PKG_CONFIG_PATH` for both the autotools and CMake jobs. - `cmake/CheckLibeventOpenSSL.cmake` (new), used from `src/apps/common/CMakeLists.txt`: on macOS, fail configure when `libevent_openssl` links a different OpenSSL major than the one found. - `configure`: same check for the autotools build (Darwin only). - `TURN_ALLOW_OPENSSL_MISMATCH` (CMake option / env var) overrides the check. ## Testing On macOS (both `openssl@3` 3.6.5 and `openssl@4` 4.0.3 installed): - CMake configure with the default OpenSSL (`openssl@4`) fails with the new error. With `-DOPENSSL_ROOT_DIR=$(brew --prefix openssl@3)` it passes, and `-DTURN_ALLOW_OPENSSL_MISMATCH=ON` also passes. - `./configure` with `openssl@4` first on `PKG_CONFIG_PATH` aborts; with `openssl@3` first it passes, and the override also passes. - With the `openssl@3` build: `ctest` 21/21, and `run_tests.sh` (TCP/TLS/UDP/DTLS), `run_tests_conf`, `dtls_default`, `ipv6_relay`, `mobility_quota`, `mobility_resume_flood`, `stateless_binding`, `stateless_nonce` all pass with no FAIL or segfault. On Linux (Docker): CMake configure, build and `ctest` (20/20) pass; the check is a no-op there. I did not complete the Linux `run_tests*.sh` pass. The PR's own macOS CI jobs are the real end-to-end check. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
315af34623 |
Bound SQLite userdb '~' expansion to the destination buffer (#2093)
## What `fix_user_directory()` in `src/apps/relay/dbdrivers/dbd_sqlite.c` replaces a leading `~` in the SQLite DB path with `$HOME`. It copied the result back into `persistent_users_db_t.userdb` (1025 bytes) using the **expanded** length as the `strncpy` bound, so a long `HOME` wrote past the buffer. The fix: - Passes the destination size in, builds the expanded path with `snprintf` into a scratch buffer, and returns `false` if the result would not fit. - Makes `get_sqlite_connection()` refuse to open the DB when the path doesn't fit. It logs `SQLite DB path is too long after expanding '~' to the HOME directory` and takes the same exit as a failed `sqlite3_open`. - Runs the expansion once under `pthread_once`. Previously every thread's first connection rewrote the shared `userdb` string in place with no lock, so two threads connecting concurrently could see a half-written or double-expanded path. ## Why Reported in GHSA-7jh8-7795-v8jw (CWE-787) by @yvppost-alt. `userdb` lives in the global `turn_params`. With the unpatched code: - A HOME of about 1.1 KB silently overwrites `userdb_sanitized`. The startup error then prints the tail of HOME instead of the path. - A HOME of about 20 KB writes past the end of `turn_params`. Under ASan, `HOME=<20 KB> turnadmin -l -b '~/turndb'` reports a `global-buffer-overflow` in `fix_user_directory`. Only the local operator controls HOME and the DB path (neither binary is setuid), so this is local-only and Low severity. It is still a real memory-safety bug. ## Notes for reviewers - Boundary: an expanded path of 1024 characters is accepted and one of 1025 is rejected. Normal `~/turndb` expansion, including with leading spaces, behaves as before. - If the HOME lookup fails, the function still leaves the path as-is and lets `sqlite3_open` handle it (it returns `true`), matching previous behavior. ## Validation - Unit tests: 21/21 on macOS and 20/20 on Linux (container). - Linux container: all 8 `examples/run_tests*.sh` scripts passed with no `FAIL` lines. - Fuzzing smoke tests: `fuzzing/run-local.sh ASan 0/1 -runs=1` passed. - Formatting: clean with `clang-format` 15. - On Linux, the advisory repro now gets the "too long" error, and a normal `~/turndb` add/list round-trip still works. - The packaged Docker image (bats) tests were not run, because only the SQLite path handling changed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
alhuda
|
59d62de44f |
defer cli session close out of the telnet parse callback (#2085)
`run_cli_input` runs inside libtelnet's `_process` loop: `telnet_recv` (from `cli_socket_input_handler_bev`) dispatches `TELNET_EV_DATA` to `cli_telnet_event_handler` -> `run_cli_input`. On `quit`/`bye`/`exit`/`q`, `drain`, and the 5th failed password it called `close_cli_session`, which does `telnet_free(cs->ts)` and frees the `telnet_t` that `_process` is still walking. Repro: to a cli/telnet session send a closing command followed by a telnet `IAC` byte in the same segment, e.g. `quit\xff\xf1`. The `IAC` makes `_process` flush the pending `quit` DATA mid-buffer, so the free happens before `_process` returns. Against the real `libtelnet.c`, ASan reports `heap-use-after-free WRITE of size 4 at libtelnet.c:961 in _process` (the `telnet->state = TELNET_STATE_IAC` after the flush), freed by the event handler, allocated by `telnet_init`. The 5th-failed-password branch reaches this before authentication. Expected: the parser finishes the buffer, then the session is torn down. Actual: `_process` writes to and keeps reading the freed `telnet_t`. Fix: give `cli_session` a `to_be_closed` flag, set it instead of calling `close_cli_session` from within the callback (and skip further commands once set), and do the real `close_cli_session` in `cli_socket_input_handler_bev` after `telnet_recv` returns. Plain `quit\n` never tripped it because `telnet_init` uses flags `0` (no `NVT_EOL`), so without an `IAC` the whole line is flushed as the last thing `_process` does. The remaining `close_cli_session` calls (input-error path, the `BEV_EVENT_EOF`/`ERROR` handler, accept failure) run outside `telnet_recv` and are unchanged. `ctest` and `examples/run_tests.sh` pass. |
||
|
dependabot[bot]
|
e971d8775b |
build(deps): bump github/codeql-action from 4.38.1 to 4.38.2 (#2086)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.38.1 to 4.38.2. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.38.2</h2> <ul> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.1">2.27.1</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4160">#4160</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.38.2 - 24 Sept 2026</h2> <ul> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.1">2.27.1</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4160">#4160</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
alhuda
|
c477242884 |
reject a mismatched realm on connection-bind in check_stun_auth (#2087)
Repro: on a TCP allocation, send `CONNECTION_BIND` for a live connection id with the allocation's `USERNAME`, `NONCE` and a valid `MESSAGE-INTEGRITY`, but a `REALM` naming a different realm. Expected: `441`, as every other authenticated method answers a realm mismatch. Actual: the request authenticates, and the allocation's session now carries the other realm's options. Cause: for `CONNECTION_BIND` the `REALM` branch of `check_stun_auth` looks the request's realm up with `get_realm_options_by_name` and stores the result in `ss->realm_options`, before `USERNAME`, `NONCE` and `MESSAGE-INTEGRITY` are checked. `ss` here is the allocation owner's session (found through the connection id), and its realm name is what `release_allocation_quota` decrements and what `good_peer_addr` matches the per-realm allowed/denied peer ranges against. The lookup also runs for a request that has not been verified yet, and `get_realm` adds an entry for a name it has not seen. Fix: drop the special case so the shared comparison applies (RFC 6062 Section 5.4: ConnectionBind carries the allocation's credentials, so the realm has to match). The `oauth` handling in that branch is unchanged. `test_turn_server_send` gains two cases: a `CONNECTION_BIND` in the allocation's realm still authenticates, and one naming another realm gets `441` with the session realm untouched and no realm lookup. The second fails before the change and passes after; `get_realm_options_by_name` moves from the abort stubs to a functional stub for it. The other `get_realm_options_by_name` callers take the realm from the DB result or from the original mobility session, so this is the only site. `ctest`, the `examples/` scripts (macOS and the Linux container), the fuzz smoke targets and the Docker image tests stay green. |
||
|
alhuda
|
dd644d0a7b |
validate redis reply key prefix before offsetting past it (#2080)
`redis_list_origins`, `redis_get_ip_list` and `redis_reread_realms`
advance each key returned by `KEYS turn/origin/*` / `keys
turn/realm/*...` past a fixed prefix without checking the key carries
it. `redis_list_secrets` in the same file already guards this with
`strstr(s, "turn/realm/") == s`; these three siblings do not.
Repro: point coturn at a redis backend whose `KEYS turn/origin/*` reply
contains a key shorter than `turn/origin/` (e.g. `x`).
Cause: `str + strlen("turn/origin/")` (and `+ strlen("turn/realm/")`)
runs the pointer past the key's `NUL`; the offset string is then
`turn_strdup`'d / map-inserted (out-of-bounds heap read), and in
`redis_get_ip_list` `strstr` walks off the end so `*sep = 0` writes out
of bounds.
Fix: validate the prefix with `strstr(key, prefix) == key` (plus a
`NULL` guard) before offsetting, matching `redis_list_secrets`.
`redis_reread_realms` and `redis_get_ip_list` run every 5s from the auth
thread, so no admin action is needed to reach them. Confirmed under
ASan: `heap-buffer-overflow` READ 10 bytes past a 2-byte key allocation
on the unpatched code, clean after the guard.
|
||
|
dependabot[bot]
|
179a8e9734 |
build(deps): bump github/codeql-action from 4.38.0 to 4.38.1 (#2084)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.38.0 to 4.38.1. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.38.1</h2> <ul> <li>The CodeQL Action now has experimental support for CodeQL releases for which per-language bundles are available. Per-language bundles support analysis for a single language and are therefore smaller than the combined bundles that allow analysis for all supported languages. As a result, per-language bundles take up less space on disk and are faster to download. We expect to roll this change out to everyone in the coming weeks. <a href="https://redirect.github.com/github/codeql-action/pull/4146">#4146</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.38.1 - 18 Sept 2026</h2> <ul> <li>The CodeQL Action now has experimental support for CodeQL releases for which per-language bundles are available. Per-language bundles support analysis for a single language and are therefore smaller than the combined bundles that allow analysis for all supported languages. As a result, per-language bundles take up less space on disk and are faster to download. We expect to roll this change out to everyone in the coming weeks. <a href="https://redirect.github.com/github/codeql-action/pull/4146">#4146</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
dependabot[bot]
|
dcdfddd0fb |
build(deps): bump github/codeql-action from 4.37.9 to 4.38.0 (#2082)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.9 to 4.38.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.38.0</h2> <ul> <li>On GitHub-hosted runners, the CodeQL Action now deletes unused CodeQL bundles from the toolcache before downloading a different bundle, which frees up disk space for the analysis. We expect to roll this change out to everyone in September. <a href="https://redirect.github.com/github/codeql-action/pull/4124">#4124</a></li> <li>The CodeQL Action now supports CodeQL releases that are compatible with Linux Arm64 and downloads the native <code>linux-arm64</code> CodeQL bundle when available. <a href="https://redirect.github.com/github/codeql-action/pull/4072">#4072</a></li> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.0">2.27.0</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4129">#4129</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.38.0 - 09 Sept 2026</h2> <ul> <li>On GitHub-hosted runners, the CodeQL Action now deletes unused CodeQL bundles from the toolcache before downloading a different bundle, which frees up disk space for the analysis. We expect to roll this change out to everyone in September. <a href="https://redirect.github.com/github/codeql-action/pull/4124">#4124</a></li> <li>The CodeQL Action now supports CodeQL releases that are compatible with Linux Arm64 and downloads the native <code>linux-arm64</code> CodeQL bundle when available. <a href="https://redirect.github.com/github/codeql-action/pull/4072">#4072</a></li> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.27.0">2.27.0</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4129">#4129</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
alhuda
|
290fe696ce |
stop convert_string_key_to_binary reading past a short key (#2081)
Repro: configure a static user with a short hex key, e.g. `turnserver --user=u:0x1234`; under an ASan build `convert_string_key_to_binary` reports a `heap-buffer-overflow READ`. Cause: `convert_string_key_to_binary` (`src/apps/relay/dbdrivers/dbdriver.c`) decodes `2*sz` hex chars from `keysource` into an `hmackey_t` with no bound on the source length, trusting every caller to pass at least `2*sz` chars. `add_static_user_account` (`src/apps/relay/userdb.c`, the `--user` / config `user=` parser) breaks that contract: for a `0x` key shorter than `2*sz` it logs `Wrong key format` but then calls the helper anyway, so it reads past the end of the key string. The five DB-driver callers (`redis`/`sqlite`/`pgsql`/`mysql`/`mongo`) all skip the decode on a short key; this one only logged. Fix: bound the helper to stop at the terminating NUL (the check short-circuits on the first hex char before touching the second, so the NUL itself is never overstepped, and a full-length `2*sz` key decodes exactly as before), and reject the short-key account in `add_static_user_account` like its siblings. Verified: `tests/test_convert_key.c` (added) decodes a full key unchanged and asserts a short/odd key stops at the NUL; it trips ASan / fails on a short key before the fix and passes after. `ctest` and the `examples/` protocol tests stay green. |
||
|
alhuda
|
346f7613d3 |
copy denied range before unlocking blacklist in good_peer_addr (#2083)
1. Cause: on a match in the dynamic denied list `good_peer_addr` calls `ioa_unlock_blacklist` and only then logs `bl->rs[i].str`. Auth thread 0 runs `update_white_and_black_lists` every 5 s, which swaps that list and frees the old one once no reader holds the lock, so the log line reads freed memory. 2. Repro: configure a DB `denied_peer_ip` range and send `CreatePermission` (or `ChannelBind` / `Connect`) for a peer inside it while the list refreshes. With a second thread doing the same swap and free in a loop, ASan reports `heap-use-after-free` in `good_peer_addr` at the `TURN_LOG_FUNC` call, freed by the refresher; TSan flags the same read as a data race. 3. Fix: copy the range string to a stack buffer while the read lock is held and log the copy after the unlock, the way `get_realm_options_by_origin` copies the realm name before its unlock. The whitelist branch never touches its list after unlocking and the static `server->ip_blacklist` is never freed, so this is the only site. `test_turn_server_send` gains a case that replaces the published list from the unlock hook and checks the denial log still names the original range: it fails before the change and passes after. |
||
Kai Ren
|
1986df21e4 | docker: Upgrade Coturn to 4.18.0 version docker/4.18.0-r0 | ||
|
Pavel Punsky
|
23c6c1d32a | Update version to 4.18.0 4.18.0 | ||
|
Pavel Punsky
|
36b2f14516 |
Update help with missing arguments (#2079)
## Summary - Remove the deprecated `--no-dtls` and `--no-cli` turnserver flags — both were already no-ops (DTLS and CLI are off unless `--dtls`/`--cli` is passed), so the flags only added deprecation-warning noise. - Update `README.turnserver` and the `Usage` help text in `mainrelay.c` to drop the removed flags. - Strip now-unused `--no-dtls`/`--no-cli` invocations from the example test scripts and update `run_tests_dtls_default.sh` to drop the coverage for the deprecated flag's warning/compat behavior. - Update `CLAUDE.md`'s test-suite notes and load-test snippet to match. ## Test plan - [ ] `cmake --build build && ctest --test-dir build --output-on-failure` - [ ] `cd examples && ./run_tests.sh && ./run_tests_conf.sh && ./run_tests_dtls_default.sh` 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Pavel Punsky
|
62cb2cc07e | Add missing cli arguments to help | ||
|
Pavel Punsky
|
1038f47bcc |
Deprecate option drop-invalid-packets (#2078)
This option has been on by default for multiple versions - removing the flag completely |
||
|
Pavel Punsky
|
d0440b6def | Replace ubuntu 24.04 with 26.04LTS (#2076) | ||
Junki Lee
|
c29ea0221b |
hiredis: clean up failed async connection setup (#2073)
## Summary - free non-NULL Hiredis async contexts returned with an error - clean up partially initialized libevent state in attach and reconnect paths - keep failed reconnect state retryable - add real Hiredis/libevent fault-injection regression tests ## Problem `redisAsyncConnect()` can return a non-NULL context with `ac->err` set. The caller still owns that context, but the initial attach path did not free it, while the reconnect path could treat it as a valid connection. If either `event_new()` call failed, the initial attach path freed only its wrapper, while reconnect retained a partial context/event state. Cleanup must go through `redisAsyncFree()` because the registered libevent cleanup callback owns the events. Freeing those events separately would result in a double-free. ## Fix - call `redisAsyncFree()` for Hiredis error contexts - clean up the async context when either event allocation fails - release attach-only wrapper strings after context cleanup - reset the reconnect context to `NULL` so a later retry can succeed ## Testing The new test uses the real Hiredis and libevent implementations. It wraps only `event_new()` to inject deterministic read/write event allocation failures. It covers: - Hiredis error contexts during initial attach and reconnect - read and write event allocation failures - successful retry after reconnect setup fails Results: - same unpatched commit plus the regression test: 6/6 cases fail - patched ASan/UBSan/LSan regression test: 6/6 cases pass - patched ASan/UBSan/LSan CTest: 16/16 pass when excluding the pre-existing `test_turn_ports` teardown leak - Release build and CTest: 17/17 pass - `turnutils_rfc5769check`: all checks pass - `git diff --check` and clang-format check pass ## Tool-assisted discovery This was found while evaluating Vul-RAG and PailGen on coturn. PailGen's initial event-failure cleanup draft conflicted with Hiredis' cleanup callback and was rejected after ASan detected a double-free. The final cleanup and regression tests were manually ownership-reviewed and validated using the real libraries. This is presented as a robustness and resource-cleanup fix. I am not claiming a demonstrated remote security impact. Signed-off-by: Junki Lee <junkilee80@gmail.com> |
||
|
alhuda
|
9bb6d3b245 |
reject 0x1f control character in SASLprep (#2074)
Repro: put a raw `0x1F` byte into a credential (`--user`, a `turnusers.txt` line, or an admin-added user) and run it through `SASLprep`; the value is accepted unchanged. Cause: the RFC 4013 Section 2.3 / RFC 3454 table C.2.1 control-range check in `SASLprep` uses `c < 0x1F`, so `0x1F` (Unit Separator) slips through while `0x00`..`0x1E` are correctly rejected. Fix: compare with `c <= 0x1F` so the whole `U+0000`..`U+001F` control range is rejected, matching the C.2.1 prohibited set (`0x7F` and the C1 range are already handled). `tests/test_saslprep.c` walks the whole control range; it fails on `0x1f` with the old bound and passes after. Also hardened alongside the fix: the `turnadmin -p` rejection branch this change makes reachable for `0x1F` passwords logged the raw password at ERROR level; it now logs a fixed message instead, matching the no-password-logging idiom in `add_static_user_account`. --- **Upgrade note:** SASLprep now rejects byte `0x1F` (unit separator), completing the RFC 4013 / RFC 3454 Table C.2.1 control-character range (`0x00`–`0x1F`). Previously `0x1F` was the only byte in that range that was accepted. Impact on existing deployments that carry `0x1F` in a credential: - A `user=` line in turnserver.conf (or `-u` on the command line) whose **username** contains `0x1F` is now rejected at load: the server still starts, logs one `Wrong user name` ERROR, and continues **without that account** — its clients will get 401. - `turnadmin` now refuses `-u`, `-r`, or `-p` values containing `0x1F` for **all** commands, including `-d` (delete). A database account created with `0x1F` before this change still authenticates on the wire but can no longer be managed with turnadmin; remove or rename it via direct database access. - Audit for affected credentials before upgrading, e.g. `grep -P '^user=.*\x1f' /etc/turnserver.conf` and the equivalent query against your user database. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01M3hK3GoPexQVTyKfGov1LD --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Pavel Punsky <eakraly@users.noreply.github.com> |
||
pannal
|
32935e1cb3 |
Return allocation mismatch for ChannelBind without allocation (#2067)
When a client sends ChannelBind after its TURN allocation has expired or disappeared, the server should report Allocation Mismatch so the client can recreate the allocation. The current handler leaves the error unset and the generic response path emits Bad Request instead. ## What changes - Return error 437, Allocation Mismatch, when ChannelBind has no valid allocation for its 5-tuple. - Add a server-handler regression test for the expired-allocation case. ## Verification The focused server test executable passes all 22 cases. The new regression test fails against the previous handler because it does not set the required 437 error. --------- Co-authored-by: pannal <panniiii@gmail.com> Co-authored-by: Pavel Punsky <eakraly@users.noreply.github.com> |
||
Philipp Hancke
|
176ce42f56 |
Silently ignore attributes after message-integrity (#2075)
as stipulated by https://datatracker.ietf.org/doc/html/rfc8489#section-9 which says Note that agents MUST ignore all attributes that follow MESSAGE- INTEGRITY, with the exception of the MESSAGE-INTEGRITY-SHA256 and FINGERPRINT attributes. Claims this leads to auth bypass are overrated. |
||
|
Pavel Punsky
|
225f1416b6 |
Move session id out of log message prefixes (#2072)
## Summary Session-scoped log lines started with `session %018llu: `, so every message began with a variable value and messages could not be matched or aggregated by a constant prefix. This moves the session id to a `(session %018llu)` suffix so each message starts with constant text, while keeping the id on every line for correlation. Example: ``` before: session 000000000000000042: peer 203.0.113.7 deleted after: peer 203.0.113.7 deleted (session 000000000000000042) ``` Details: - 36 log call sites changed across `ns_turn_server.c`, `ns_turn_allocation.c`, and `ns_ioalib_engine_impl.c` (includes the capitalized `Session %018llu to be forcefully canceled` variant). - Sites that logged `__FUNCTION__` right after the id move the function name into the suffix too: `(session %018llu, %s)` — otherwise the line would still start with a variable. - The `log_socket_event` helper and the allocation status/delete lines gain constant `socket event:` / `allocation` lead-ins, because their first logged value is itself a variable. - No information is removed from any log line; only ordering changes. - The `mobility handoff completed` marker that `examples/run_tests_mobile.sh` greps for still matches. ## Validation - `clang-format` clean - CMake build with `-DBUILD_TESTING=ON`: unit tests 18/18 passed - `examples/run_tests.sh`: zero FAIL lines - `examples/run_tests_mobile.sh`: passed, handoff marker found (x6) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
5a9ed37ae4 |
build(deps): bump github/codeql-action from 4.37.8 to 4.37.9 (#2071)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.8 to 4.37.9. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.37.9</h2> <ul> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.26.4">2.26.4</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4106">#4106</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.37.9 - 26 Aug 2026</h2> <ul> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.26.4">2.26.4</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4106">#4106</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
Pavel Punsky
|
9c6c48406a |
Split unauthorized reason (#2065)
## Summary
Rejected authentications currently all log the same generic line —
`Cannot find credentials of user <...>` — whether the user is unknown,
the password is wrong, or a time-limited (`--use-auth-secret`) username
has simply expired. That makes routine credential expiry
indistinguishable from misconfiguration or an attack when triaging logs
or dashboards.
This PR splits the rejection cause into three cases and reports it in
both the log and a new Prometheus metric:
- **expired** — the time-limited username's timestamp is in the past
(rejected before any integrity verification)
- **integrity_mismatch** — MESSAGE-INTEGRITY did not verify against any
known secret or key (wrong credentials)
- **not_found** — unknown user (the previous generic message, kept as
the fallback)
## Changes
- New `turn_key_lookup_result` enum
([ns_turn_session.h](src/server/ns_turn_session.h)), threaded from
`get_user_key()` through `struct auth_message` and
`get_username_resume_cb` into the session, so `check_stun_auth()` can
name the actual cause in the 401 log line.
- In the time-limited-credential path, an expired timestamp now takes an
explicit early return before the per-secret HMAC loop (behavior
unchanged — expired usernames were already rejected without integrity
work; the restructure makes the cheap-reject property explicit and gives
it a distinct cause). A zero timestamp (no parseable timestamp in the
username) still reports `not_found`, not `expired`.
- New Prometheus counter `turn_auth_credential_failures{cause=...}`.
Increments are accumulated in lock-free thread-local counters and
flushed once per second alongside the existing 401-mitigation counters,
keeping the prom registry lock off the relay path.
- Link stubs for the new symbols in `tests/test_alt_server_stubs.c`.
No wire-visible behavior changes: the responses are still 401
challenges, and the cause split only affects logging and metrics.
## Testing
- `cmake -DBUILD_TESTING=ON` build + `ctest`: 18/18 pass
- clang-format-15 formatting verified on all touched files
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Pavel Punsky
|
0feab35326 |
Make the multiplex-peer example tests request EVEN-PORT never, not ra… (#2070)
…ndomly run_client drove turnutils_uclient with -c only. In clnet_allocate, -c leaves the EVEN-PORT slot to be picked at random between 0 and -1, and multiplex-peer strictly rejects EVEN-PORT with error 508, so roughly half of all attempts were thrown away. The script compensated with an eight-attempt retry loop, which still failed outright about once in 256 per run_client call and about once in 50 per script run - observed on https://github.com/coturn/coturn/actions/runs/31279481991 (amazonlinux:2023), where all eight attempts of the first UDP case drew EVEN-PORT. Pass --no-even-port, which forces the slot to -1 unconditionally, and drop the retry loop it makes dead. Nothing is lost in coverage: EVEN-PORT under multiplex-peer is defined to be a 508, so the random draw only ever decided whether a run was wasted. A retry that can no longer fire for its stated reason would silently swallow a genuine regression. expect_peer_endpoint_limit already passed --no-even-port for the same reason. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
3f0d4f6667 |
Log only the request line for web-admin HTTP requests (#2069)
The web-admin request logger printed the whole buffer unless it contained the substring "pwd", so POST bodies carrying a realm shared secret (add_secret) or OAuth keying material (oauth_ikm) were written to the log verbatim, along with every request header. The HTTPS branch also logged unconditionally, unlike the plain-HTTP branch beside it. Log the request line alone and gate the HTTPS branch on the verbose flag. The path is logged separately by handle_https, so no diagnostic is lost. Fixes #362 Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
70ca74e0bd |
fix: shrink oversized stack frames flagged by MSVC C6262 (#2068)
Code scanning reports twelve "Function uses N bytes of stack" alerts, all from a ~64 KB stun_buffer, an app_ur_session (which holds two of them), or a 64 KB scratch array declared as a local. The largest frame is 270 KB in turnutils_uclient's allocation flood; several sit on relay and listener threads, whose stacks are far smaller than the main thread's. - ns_turn_msg.c: stun_attr_add_padding_str() built its all-zero PADDING value in a 64 KB local. It runs on relay threads for RFC 5780 responses, so allocate exactly padding_len instead. - ns_ioalib_engine_impl.c: ssl_read()/ssl_send() passed a 64 KB buffer to ERR_error_string(), which documents a 256-byte minimum. Use ERR_error_string_n() with a 256-byte buffer, and read SSL_get_error() before popping the error queue so the logged pair is deterministic. - turn_admin_server.c, stunclient.c, natdiscovery.c, uclient.c: heap the stun_buffer/app_ur_session locals, freeing through a single exit path. The allocation flood reuses one pair across iterations rather than reallocating per loop. - udpserver.c: the non-Linux echo path gets a module-static buffer, matching the rationale already used by the Linux recvmmsg path in the same file. - uclient.c: the discard handler's drain buffer shrinks to 4 KB; the bytes are thrown away, so the loop just needs somewhere to put them. Measured with -fstack-usage: every flagged frame drops from 65-270 KB to 64 B - 4.2 KB. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
285a9b76c9 |
Merge commit from fork
The UDP listener's stateless first-packet fast path short-circuits a first datagram from an unknown source when the outcome does not depend on session state, so it never allocates a child socket or a session. Its credential-mode gate sat above the packet classification, so the short-circuit only applied in long-term-credential mode; in no-auth mode the same first packets fell through to create_ioa_socket_from_fd + open_client_connection_session and held that state until the to-be-allocated watchdog expired, even though the relay never acts on them. Move the credential-mode gate below the classification: the decisions that do not depend on credential type are made at the listener in every mode, and only the 401/438 challenge synthesis stays long-term-only. A real request (e.g. ALLOCATE) still takes the session path, and an error-producing request is still reaped on the initial packet, so the bytes on the wire are unchanged. Rename udp_stateless_nonce_fast_path to udp_stateless_fast_path to reflect the mode-independent role, and document it in docs/stateless-nonce.md. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
tangxinfa
|
1dfa48d29e |
fix: external ip must valid if the provider responses EDNS (#2060)
Google may response IP and EDNS records, e.g.
$ dig @ns1.google.com o-o.myaddr.l.google.com TXT -4 +short | tr -d \"
edns0-client-subnet 103.252.16.0/24
172.253.5.26
It pass the is_valid_ip() checking, but not a valid ip address, fix it
by examine the first line only.
|
||
|
alhuda
|
cf1023ca85 |
skip null memcpy on zero-length stun auth attribute (#2066)
Repro: build with `-fsanitize=undefined` and send an unauthenticated STUN request carrying a well-formed 4-byte `REALM`, `USERNAME` or `NONCE` TLV with length `0`. Cause: `stun_attr_get_value()` returns `NULL` for a zero-length attribute, so `check_stun_auth()` reaches `memcpy(dst, stun_attr_get_value(sar), alen)` with a `NULL` source and `alen == 0`. `memcpy` is declared `nonnull`, so this is undefined behaviour; glibc UBSan reports `null pointer passed as argument 2, which is declared to never be null`. The same shape sits on the pre-auth stateless-nonce fast path in `udp_get_string_attr()`. The sibling `ORIGIN` readers in both files already guard with `if (sarlen > 0)`. Fix: wrap each copy in `if (alen)` so an empty attribute skips the `memcpy` and only writes the terminator. Behaviour is unchanged (the destination is still an empty string). Verified with a reproducer that drives the real `stun_attr_*` getters over a zero-length `USERNAME`: `gcc -fsanitize=undefined` trips on the unpatched sink and runs clean after. `ctest` 18/18 and `run_tests.sh` / `run_tests_conf.sh` pass. |
||
|
Pavel Punsky
|
3a8980ab97 |
Log only the request line for web-admin HTTP requests (#2064)
The web-admin request logger printed the whole buffer unless it contained the substring "pwd", so POST bodies carrying a realm shared secret (add_secret) or OAuth keying material (oauth_ikm) were written to the log verbatim, along with every request header. The HTTPS branch also logged unconditionally, unlike the plain-HTTP branch beside it. Log the request line alone and gate the HTTPS branch on the verbose flag. The path is logged separately by handle_https, so no diagnostic is lost. Fixes #362 Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
a0dc3eb47d |
build(deps): bump github/codeql-action from 4.37.7 to 4.37.8 (#2063)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.7 to 4.37.8. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.37.8</h2> <p>No user facing changes.</p> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.37.8 - 21 Aug 2026</h2> <p>No user facing changes.</p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
Pavel Punsky
|
9600cbd603 |
ci: guard against test-script hangs (timeout wrappers + job timeouts) (#2061)
## Problem The Clang workflow's `sanitize (thread)` job has been hanging for **hours** (e.g. #2058's run hung 4h28m; an earlier run hit the **6h** GitHub default), while normal runs finish in 5–15 min. **Root cause (investigated under TSan):** `turnutils_uclient`'s threaded load-gen has data races. ThreadSanitizer reports them in `__turn_getMSTime()` ([uclient.c:1027-1035](../blob/master/src/apps/uclient/uclient.c#L1027)) — which writes the shared `current_time` / `current_mstime` / `show_statistics` globals from both the **main thread** (`start_mclient`) and every **sender thread** (`sender_timer_handler`) with no synchronization — plus the shared session counters in `start_mclient`. Under the TSan build's timing these races intermittently leave the client **spinning after the transfer has already completed**. `run_tests_ipv6_relay.sh` (scenario 2, `-y` c2c) runs the client synchronously and only greps the log afterwards, so a client that never exits blocks the script forever. With no job timeout, the job then runs to GitHub's 6h default. This is **not** a regression from any recent change — the ASan job runs the identical steps and passes; the hang is purely the TSan-timing-exposed load-gen race. ## Fix — two independent guards 1. **`timeout`-wrap the blocking `turnutils_uclient` calls** in the run_tests scripts (`run_tests.sh`, `run_tests_conf.sh`, `run_tests_mobile.sh`, `run_tests_ipv6_relay.sh`). Success is already decided by grepping the log for the completion marker, which is written *before* the client starts spinning — so a post-completion kill still passes, while a genuine failure still shows no marker. Scripts that already bound the client (`ratelimit_401`, `dtls_default`, the load-gen smoke) are unchanged. 2. **`timeout-minutes` on every CI job that runs the test scripts** (`clang` sanitize, `cmake` build, `linux` build, `macos` build + build-cmake). Any future hang now fails in tens of minutes instead of six hours. ## Validation (Linux container) - **No regression** (plain autotools build): `run_tests.sh`, `run_tests_conf.sh`, `run_tests_mobile.sh`, `run_tests_ipv6_relay.sh` all pass with the wrappers. - **TSan build**: `run_tests_ipv6_relay.sh` now **passes (rc=0)** instead of hanging. - All four workflow YAMLs parse; `bash -n` clean on all four scripts. ## Follow-up (separate PR) The real fix is to eliminate the `turnutils_uclient` load-gen data races (make `__turn_getMSTime`'s time cache thread-safe and audit the shared session/counter state the sender/listener pools touch). This PR only stops the CI bleeding. |
||
|
Pavel Punsky
|
fe087b6afe |
turnutils_peer: reflect DSCP/TOS on echoed datagrams (#2059)
## What `turnutils_peer` now echoes each datagram back with the **same DSCP/TOS marking it arrived with**, instead of a bare TOS of 0. ## Why Reported in #385: DSCP markers appear lost across a TURN relay. coturn actually reflects TOS correctly in **both** directions — but the reflection on the peer→client leg mirrors whatever the *peer* sends, and `turnutils_peer` sent an unmarked (CS0) echo. So the test tooling itself made the relay's return-leg DSCP handling impossible to observe, and matched the exact "CS7 → CS0" symptom in the report. With the peer reflecting, the full round-trip DSCP path is testable. ## How - On Linux, the marking is read off each datagram's ancillary data (`IP_RECVTOS` / `IPV6_RECVTCLASS`) on the `recvmmsg` path. A batch that shares one marking — a single steady flow, and what UDP-GSO already requires — sets the socket TOS once and keeps the `sendmmsg`/GSO fast path; a mixed batch falls back to a per-packet echo so each reply keeps its own marking. - Off-Linux the peer echoes without TOS reflection — the same tradeoff the relay itself makes (RFC 5766 "alternative behavior"), so no new cross-platform ancillary-data code is introduced. ## Test New `examples/run_tests_dscp.sh` system test: it marks the client socket (TOS `0x22`, as `turnutils_uclient` already does), drives traffic through a loopback relay, and asserts via `tcpdump` that the DSCP survives on **both** the relay→peer and listener→client legs. It SKIPs (exit 0) off-Linux or without `tcpdump` / capture privileges, matching the style of `run_tests_rfc5780.sh`. ## Validation Validated in a Linux container: - `ctest`: 17/17 pass - `run_tests_dscp.sh`: both legs carry `tos 0x22` (forward + return) — OK - `run_tests.sh` incl. the `-Y packet` GSO load-gen smoke: OK (batched/GSO fast path preserved) - `clang-format-15`: clean Closes #385 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
1e3570dd1e |
Advertise IPv6 allocations under their real relayed transport address (#2058)
Fixes #1322. The issue reports `clnet_allocate: !!!: relay addr cannot be received (2)` when asking `turnutils_uclient` for an IPv6 relay. It turns out to be two independent bugs that both end in that same line, one on each side of the wire. Both are reproduced by the new test, which fails on `master` and passes here. ## What the RFC requires RFC 8656 Section 3 defines the relayed transport address as *"A transport address on the TURN server that is used for relaying packets between the client and a peer"*, and Section 7.2 leaves the server two outcomes for an Allocate carrying `REQUESTED-ADDRESS-FAMILY`: > If the server does not support the address family requested by the client in REQUESTED-ADDRESS-FAMILY, or if the allocation of the requested address family is disabled by local policy, it MUST generate an Allocate error response, and it MUST include an ERROR-CODE attribute with the 440 (Address Family not Supported) response code. Section 7.3 puts one duty on the client: > If the client receives an Allocate success response, then it MUST check that the mapped address and the relayed transport address or addresses are part of an address family or families that the client understands and is prepared to handle. ## 1. `--external-ip` was applied across address families (server) `external_ip` is a single `ioa_addr` and `--external-ip` may only be given once, but it was substituted for the relayed address of *both* families. On a server with an IPv4 `--external-ip`, an IPv6 allocation was therefore advertised under an unrelated IPv4 address that no peer can reach — neither an IPv6 relay nor a 440. ``` --relay-ip=127.0.0.1 --relay-ip=::1 --external-ip=127.0.0.1, client over ::1 with -x INFO IPv4. Received relay addr: 127.0.0.1:30654 INFO clnet_allocate: !!!: relay addr cannot be received (2) ``` The relay socket really was IPv6; only the advertised address was wrong. As a second effect, the substituted family made the response draw `LIFETIME` from the IPv4 relay session that was never created for that allocation. `turn_set_xor_relayed_addr()` now applies the external IP only to a relay of its own family, which is the same rule the `ioa_addr_add_mapping()` setup path already follows in `mainrelay.c`. The option's documentation is updated to match. ## 2. `turnutils_uclient` rejected an IPv6 relay it can handle (client) The acceptance test took an IPv6 relayed address only when IPv6 had been requested *explicitly*, and consulted the global `default_address_family` rather than the `af` argument the caller passed to `clnet_allocate()`. Against a server running `--allocation-default-address-family=keep` (or the deprecated `--keep-address-family`), which answers an IPv6 client with an IPv6 relay and no `REQUESTED-ADDRESS-FAMILY` in play, a perfectly usable relay was refused — as reported in the issue thread. Section 7.3 asks only that the client refuse a family it *cannot handle*, so the check now rejects solely on a contradiction with the family this allocation requested. Note this does not make `keep` conformant — it is a deliberate deviation, and `turnserver -h` says as much (*"It breaks RFC6156 section-4.2 (violates default IPv4)"*). The client simply stops rejecting a relay it is able to use. ## Testing `examples/run_tests_ipv6_relay.sh` covers both configurations end to end, asserting a completed relayed round trip rather than just a successful allocate. It uses ports off the defaults the other scripts share, and SKIPs where no IPv6 loopback is bindable. Wired into the clang, cmake, linux and macos workflows. Verified red before green: reverting only `src/` and rebuilding turns both cases into failures. - macOS: build + `ctest` 18/18; Linux container: build + `ctest` 17/17 - Linux: `run_tests.sh`, `run_tests_conf.sh`, `run_tests_mobile.sh`, `run_tests_multiplex_peer.sh`, `run_tests_rfc5780.sh`, `run_tests_stateless_nonce.sh`, `run_tests_ipv6_relay.sh`, `run_tests_dtls_default.sh` — all OK - Fuzzing smoke tests (ASan targets 0 and 1) — OK - Packaged Docker image (debian, arm64) + bats — 19 tests, 0 failures, 2 skipped ## Deliberately not in this PR - The dual-allocation `508` guard when `external_ip_set` is left as is; with family-aware rewriting it could now be relaxed to allow a v4-rewritten + v6-as-is pair, but that is a separate behavioral change. - `--external-ip` stays single-family. A per-family form would be the complete answer for IPv6-NAT deployments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Pavel Punsky
|
b3dc634b37 |
Build docker images with optimizations on (#2057)
Enable `-O3` for docker images Enable optimizations where makes sense in cmake builds |
||
|
dependabot[bot]
|
3b109589b5 |
build(deps): bump repolevedavaj/install-nsis from 1.2.1 to 1.3.0 (#2054)
Bumps [repolevedavaj/install-nsis](https://github.com/repolevedavaj/install-nsis) from 1.2.1 to 1.3.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/repolevedavaj/install-nsis/releases">repolevedavaj/install-nsis's releases</a>.</em></p> <blockquote> <h2>v1.3.0</h2> <!-- raw HTML omitted --> <ul> <li>README: Update example revisions (<a href="https://redirect.github.com/repolevedavaj/install-nsis/issues/47">#47</a>) <a href="https://github.com/xavier2k6"><code>@xavier2k6</code></a></li> </ul> <h2>🚀 New features and improvements</h2> <ul> <li>Verify the downloads, move the EnVar plugin upstream, add floating version tags (<a href="https://redirect.github.com/repolevedavaj/install-nsis/issues/56">#56</a>) <a href="https://github.com/repolevedavaj"><code>@repolevedavaj</code></a></li> </ul> <h2>📦 Dependency updates</h2> <ul> <li>Bump release-drafter/release-drafter from 7.2.0 to 7.3.1 (<a href="https://redirect.github.com/repolevedavaj/install-nsis/issues/43">#43</a>) @<a href="https://github.com/apps/dependabot">dependabot[bot]</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
alhuda
|
a31b69d5a4 |
don't release port 65535 when turnports_allocate fails (#2056)
Repro: `turnports_allocate()` returns `-1` from inside `turnports_allocate_even()`'s loop when no port is free (an RFC 8656 EVEN-PORT `ALLOCATE` against a drained or fragmented relay pool). Cause: the `-1` is `& 1`-tested, classed as an odd port, and passed to `turnports_release(turnports *, uint16_t port)`, whose parameter narrows `-1` to `65535`. With the default `max-port` (`HIGH_DEFAULT_PORTS_BOUNDARY`) that port is in range, so a live allocation still holding `65535` has its status overwritten and the port re-queued into the free list while the owning session's relay socket stays bound. A later allocation can then hand the same relay port to a second session. Fix: `break` out of the loop when `turnports_allocate()` returns `< 0`. The added `test_turn_ports` case marks `65535` as held, forces the allocator down its `-1` path, and asserts `turnports_allocate_even()` fails without touching the held port. It fails before the change and passes after. |
||
|
dependabot[bot]
|
ed8f14b51c |
build(deps): bump github/codeql-action from 4.37.6 to 4.37.7 (#2055)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.6 to 4.37.7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/releases">github/codeql-action's releases</a>.</em></p> <blockquote> <h2>v4.37.7</h2> <ul> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.26.3">2.26.3</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4085">#4085</a></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/github/codeql-action/blob/main/CHANGELOG.md">github/codeql-action's changelog</a>.</em></p> <blockquote> <h2>4.37.7 - 13 Aug 2026</h2> <ul> <li>Update default CodeQL bundle version to <a href="https://github.com/github/codeql-action/releases/tag/codeql-bundle-v2.26.3">2.26.3</a>. <a href="https://redirect.github.com/github/codeql-action/pull/4085">#4085</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
||
|
Pavel Punsky
|
7c24c88a4c |
Document RFC 8489 and RFC 8656 instead of only the specs they obsolete (#2049)
README.md and STATUS.md advertised RFC 5389, RFC 5766 and RFC 6156 without mentioning the RFCs that obsolete them, so the docs simultaneously claimed conformance to dead specs and understated what the code actually does: dual allocation, ADDRESS-ERROR-CODE, the 440/443/508 error codes and the RFC 8656 channel-number range are all implemented and none were mentioned. Both files now name RFC 8489 and RFC 8656, annotate the obsoleted specs as such, and state the two gaps explicitly rather than leaving them implied - the RFC 8489 authentication additions and RFC 8656 ICMP relaying are absent. The transport and relay-protocol lists cite RFC 8656 rather than RFC 5766. Drops the TURN-bis draft-04 line: that draft was published as RFC 8656, now listed above it. Merge order: the RFC 8489 entry credits first-wins handling of repeated attributes, which is only complete once the Send DATA and ChannelBind XOR-PEER-ADDRESS fixes have landed. |
||
|
Pavel Punsky
|
495a4437fb |
Size USERNAME, REALM and NONCE limits in bytes per RFC 8489 (#2047)
The three limits were byte counts standing in for character counts, and the auth path silently truncated to them. RFC 8489 Section 14.3 caps USERNAME at fewer than 509 bytes; Sections 14.9 and 14.10 cap REALM and NONCE at fewer than 128 characters, which is up to 763 bytes once UTF-8 encoded. coturn used 512/127/127, so USERNAME was simultaneously too loose (509-512 accepted, should be rejected) and REALM and NONCE far too tight: a conformant 127-character UTF-8 realm was cut off at 127 bytes, mid-codepoint, and the realm comparison then failed with 437/441 rather than anything diagnostic. check_stun_auth() now rejects an over-long value with 400 instead of truncating, matching handle_turn_allocate(), which already answered 400 "User name is too long" - the two paths disagreed. The client-side challenge parser rejects an over-long realm or nonce for the same reason: a truncated value fails the later integrity check with an unrelated error. ts_ur_super_session grows 18520 -> 19152 bytes and turn_session_info 1848 -> 2480, both +632, from the wider realm buffers. These are installed public headers, so the struct-size change is visible to libturnclient consumers. The turn_realm_option and turn_admin_user realm columns stay varchar(127): the RFC limit governs what must be parsed off the wire, not how long an operator's own realm may be, and those columns sit in composite primary keys where widening risks the InnoDB index-length limit. get_bold_admin_title() bounded each append by the length of the value being appended rather than by the space left in its 1025-byte buffer; at a 763-byte realm that bound exceeds the buffer, so it now uses the remaining space. |
||
|
Pavel Punsky
|
aa419aa876 |
Bind the first XOR-PEER-ADDRESS in ChannelBind, not the last (#2048)
RFC 8489 Section 14 permits any attribute to appear more than once and prescribes first-wins: only the first occurrence needs to be processed and duplicates may be ignored. handle_turn_channel_bind() had no duplicate check at all, so each XOR-PEER-ADDRESS overwrote the previous one and the channel bound to whichever address came last, making peer selection attribute-order dependent. The permission installed alongside the channel followed the same last address. CreatePermission is deliberately not changed: it legitimately carries several peer addresses (RFC 8656 Section 9.2), which is why its addr_found is a counter rather than a first-wins flag. Adds the first ChannelBind coverage to the server-core harness: a sanity test that one peer address binds, plus the two first-wins assertions, which fail against the previous behaviour. The path needed no new link stubs. |
||
|
Pavel Punsky
|
91cd52377f |
Ignore a duplicate DATA attribute in Send instead of discarding (#2046)
RFC 8489 Section 14 permits any attribute to appear more than once and prescribes first-wins: only the first occurrence needs to be processed and duplicates may be ignored. handle_turn_send() instead treated a second DATA attribute as 400 "Data duplication", which discards the indication - a stricter-than-spec reading that drops datagrams the server could relay. The first occurrence now wins and later ones are ignored, matching the XOR-PEER-ADDRESS case directly above it. test_send_with_duplicate_data_is_discarded pinned the old behaviour as a characterization test and is replaced by test_send_uses_first_of_duplicate_data, which asserts the first payload is the one relayed. |
||
|
Pavel Punsky
|
32eb04abae |
uclient: stamp per-send metadata into the packet, not a shared buffer (#2053)
## Problem `client_write()` stamped each send's `message_info` — the session's sequence number and the send timestamp — into the head of the **global** `buffer_to_send`, then copied that buffer into the outgoing packet: ```c message_info *mi = (message_info *)buffer_to_send; mi->msgnum = elem->wmsgnum; mi->mstime = current_mstime; ... memcpy(elem->out_buffer.buf + 4, buffer_to_send, clmessage_length); ``` With more than one sender thread those two steps interleave: a second sender can overwrite `msgnum`/`mstime` between the first sender's stamp and its copy, so a packet goes out carrying another session's values. The peer echoes the fields back and `client_read()` derives all three quality metrics from them — the loss count (`mi.msgnum != elem->recvmsgnum + 1`), the latency sample, and the jitter. A swapped stamp is therefore scored against the wrong session. Two sender threads are the auto-scaled default from `-m 4` upward (`UCLIENT_AUTO_SENDERS_TARGET`). ThreadSanitizer flags it as a write/write race between sender threads on `buffer_to_send`, at the `msgnum` and `mstime` offsets. ## Scope, honestly The race is real and structural, but I could not demonstrate a measurable corruption rate: 0 lost packets over 18,000 messages with `--sender-threads 4`. The stamp-to-copy window is a few instructions, so collisions are rare on loopback. Treat this as removing undefined behaviour and a latent wrong-metric source, not as a claim that existing measurements are wrong. ## Fix The per-send fields now go into the **destination** after the payload is copied, so nothing per-session is written to shared storage. `buffer_to_send` becomes `payload_template`: filled once during setup and read-only for the rest of the run, which is what makes sharing it safe — the rename is what keeps the invariant from being reintroduced. The Send-indication path stamps at the DATA value, whose offset is the message length captured *before* the attribute is appended, plus its 4-byte header. `stamp_message_info()` copies through a zero-initialized local rather than casting the destination to `message_info *`: the destination sits at an arbitrary offset in the message, `mstime` is 64-bit, and the struct's internal padding would otherwise reach the wire uninitialized. `mainuclient.c` already forces `clmessage_length >= sizeof(message_info)` for every mode that stamps. ## Verification ThreadSanitizer over the load-gen smoke, same invocation and duration for both: | build | races | `buffer_to_send` races | |---|---:|---:| | master | 24 | 2 | | this branch | 22 | 0 | Exactly the two targeted races removed, none introduced. Functional parity checked against master at `-m 60 -n 300 --sender-threads 4 --listener-threads 2` (0% loss, comparable jitter, comparable runtime) and at `-Y packet -m 8 --sender-threads 4` (~15-16M sends). Also passing: `ctest` 18/18, and `run_tests.sh`, `run_tests_conf.sh`, `run_tests_mobile.sh`, `run_tests_multiplex_peer.sh` with zero `FAIL`. `make lint` clean. ## Still open The other 13 races are pre-existing and untouched here: `current_time`/`current_mstime` written by several threads, the `start_full_timer` publication, per-thread stat accumulators read by the progress print, and one session struct written by both its listener and its sender. All are in `turnutils_uclient` — a TSan-instrumented `turnserver` and `turnutils_peer` reported zero races throughout. 🤖 Generated with [Claude Code](https://claude.com/claude-code) |
||
|
Pavel Punsky
|
458b082e30 |
uclient: start the worker pools after session creation, not before (#2052)
## Problem The `sanitize (thread)` CI job fails intermittently on the `-Y packet` load-gen smoke in `run_tests.sh` with `uclient exited with 66` — for example [run 31284306946](https://github.com/coturn/coturn/actions/runs/31284306946), where it fired on a PR whose diff could not reach `turnutils_uclient` at all. `turnutils_uclient` spawned the listener and sender pools **before** the session-creation loop, so worker threads ran concurrently with `start_client` / `start_c2c`. Those build MESSAGE-INTEGRITY and run TLS handshakes on the main thread, while a listener verifying integrity on a received packet calls into the same OpenSSL library state. ThreadSanitizer reports the overlap inside libcrypto: ``` WARNING: ThreadSanitizer: data race Write of size 8 by main thread: #0 free #1 OPENSSL_LH_doall #3 start_client uclient.c Previous read of size 1 by thread T1: #0 strcmp #2 stun_check_message_integrity_by_key_str #4 process_received_buffer uclient.c #6 client_input_handler ``` All six races in that run share the `start_client -> start_mclient -> main` stack. Workers also touched session structs the main thread was still initialising, and iterated `elems[]` while it was still being appended to. ## Fix The pools were started early only because `pick_listener_base()` and `pick_sender_id()` need them while sessions are built — which needs the arrays and event bases, not running threads. So pool construction is split from thread spawning: - `init_listener_pool()` / `init_sender_pool()` allocate the state, create the event bases and arm the per-sender timers, called where the old calls were. - `start_listener_threads()` / `start_sender_threads()` now only `pthread_create`, called once `total_clients` is final. An armed timer stays dormant until its base is dispatched, which now happens only in the thread. Teardown already joined both pools before freeing sessions and bases, so startup was the only unguarded edge. **Behaviour change:** a spawn failure can no longer fall back to the single-threaded model — sessions are already bound to listener bases that would then never be dispatched, i.e. a silent run where those sessions never receive. It is fatal instead. ## Verification Reproduced and verified under TSan locally (`-Y packet -m 4 -l 100 -c --listener-threads 1 --sender-threads 2`, 6s, matching the CI invocation): | build | races, 4/4 runs | |---|---| | master | **24** | | this branch | **15** | The 9 eliminated are exactly the setup-phase class. The local reproduction caught the same bug with a clearer victim than libcrypto's internals: |
||
|
Kai Ren
|
34230574e9 | docker: Upgrade Coturn to 4.17.2 version docker/4.17.2-r0 |