From a13859e845cf374f2bbbd53d1819ad400dcede66 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Thu, 30 Jul 2026 14:17:50 +0200 Subject: [PATCH 01/11] Fixed gcov rule. Fixed ABI compliance checker to use local installation (#3349) * Fixed gcov rule. Fixed ABI compliance checker to use local installation * Removed codecov for C++03. Fixed wrong call path for gdb after tests * Fixed crypto tests build break when encryption disabled * Fixed CI on macos. Tracking a problem on ABI * Refactored TestEnforcedEncryption to simplify initialization * Fixed ABI. Fixed mac warn build break on ENC=no * Added missing ASH variable to output * Tracking problem with ABI * Changed abi compliance checker to official package. Fixed options in configure-data * Preserved RES up until the end to prevent error blocking report download * Changed command call to swallow error result (prevents premature interrupt of the step) * Fixed uploading HTML report for ABI * I hate YAML * Using Create Browser Link instead * Changed ABI reporting to uploading html with no zipping * Updated Ubuntu packages before installing --------- Co-authored-by: Mikolaj Malecki --- .github/workflows/abi.yml | 41 +++++---- .github/workflows/macos.yml | 1 + .github/workflows/ubuntu-c++03.yml | 13 ++- .github/workflows/ubuntu-c++11.yml | 17 ++-- CMakeLists.txt | 21 +++-- configure-data.tcl | 59 ++++++------- scripts/codecov/update.sh | 5 ++ scripts/collect-gcov.sh | 7 -- srtcore/crypto.cpp | 1 + test/test_crypto.cpp | 5 +- test/test_enforced_encryption.cpp | 128 +++++++++++++---------------- 11 files changed, 157 insertions(+), 141 deletions(-) create mode 100755 scripts/codecov/update.sh delete mode 100644 scripts/collect-gcov.sh diff --git a/.github/workflows/abi.yml b/.github/workflows/abi.yml index 4d4d6012c6..846dbbb2e8 100644 --- a/.github/workflows/abi.yml +++ b/.github/workflows/abi.yml @@ -26,13 +26,13 @@ jobs: run: | cd gitview_pr mkdir _build && cd _build - cmake -DCMAKE_BUILD_TYPE=Debug ../ + cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_BONDING=1 -DENABLE_PKTINFO=1 -DENABLE_MAXREXMITBW=1 ../ - id: build name: Build and dump run: | - sudo apt install -y abi-dumper - sudo apt install -y tcl - cd gitview_pr/_build && cmake --build ./ + sudo apt install -y abi-dumper abi-compliance-checker tcl + cd gitview_pr + cd _build && cmake --build ./ make install DESTDIR=./installdir SRT_TAG_VERSION=v$(../scripts/get-build-version.tcl full) echo "SRT_TAG_VERSION=$SRT_TAG_VERSION" >> "$GITHUB_OUTPUT" @@ -49,6 +49,9 @@ jobs: echo "WILL CHECK ABI changes $SRT_BASE - $SRT_TAG_VERSION" echo "SRT_BASE=$SRT_BASE" >> "$GITHUB_OUTPUT" fi + echo "--- GITHUB_OUTPUT ---" + cat $GITHUB_OUTPUT + echo "---------------------" - id: upload_pr_dump uses: actions/upload-artifact@v4 with: @@ -81,7 +84,7 @@ jobs: fi cd gitview_base mkdir _build && cd _build - cmake -DCMAKE_BUILD_TYPE=Debug ../ + cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_BONDING=1 -DENABLE_PKTINFO=1 -DENABLE_MAXREXMITBW=1 ../ - id: build_tag name: Build and dump if: ${{ success() }} @@ -121,22 +124,30 @@ jobs: path: . - name: abi-check run: | - git clone https://github.com/lvc/abi-compliance-checker.git - #cd gitview_pr/submodules - #git submodule update --init abi-compliance-checker - cd abi-compliance-checker && sudo make install && cd ../ - #cd ../.. + sudo apt install -y abi-dumper abi-compliance-checker tcl echo "FILESYSTEM state before running abi-check at $PWD" ls -l sha256sum libsrt-base.dump sha256sum libsrt-pr.dump - abi-compliance-checker -l libsrt -old libsrt-base.dump -new libsrt-pr.dump - RES=$? + RES=0 + abi-compliance-checker -l libsrt -old libsrt-base.dump -new libsrt-pr.dump || RES=$? + # Flatten the report for download-preview + cd compat_reports + REPORT=$(find . -name *.html) + cp $REPORT compat_report.html + cd .. if (( $RES != 0 )); then echo "ABI/API Compatibility check failed with value $?" - exit $RES + echo "RES=$RES" >>$GITHUB_ENV + exit 0 fi - name: Download report - uses: actions/download-artifact@v4 + uses: actions/upload-artifact@v7 with: - path: compat_reports + name: abi-compliance-report + path: compat_reports/compat_report.html + archive: false + - name: Final result + run: | + echo "Final result: $RES" + exit $RES diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 7ec1cfb802..d8b1f3b892 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -16,6 +16,7 @@ jobs: run: | brew tap-new Haivision/gt-local brew extract --version=1.12.1 --force googletest Haivision/gt-local + brew trust Haivision/gt-local brew install googletest@1.12.1 # NOTE: 1.12.1 is the last version that requires C++11; might need update later # curl -o googletest.rb https://raw.githubusercontent.com/Homebrew/homebrew-core/23e7fb4dc0cc73facc3772815741e1deb87d6406/Formula/googletest.rb diff --git a/.github/workflows/ubuntu-c++03.yml b/.github/workflows/ubuntu-c++03.yml index 3f38598cad..8b9b685f56 100644 --- a/.github/workflows/ubuntu-c++03.yml +++ b/.github/workflows/ubuntu-c++03.yml @@ -19,11 +19,14 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - name: configure + - name: prepare run: | + sudo apt update sudo apt install -y tcl cmake libssl-dev gdb + - name: configure + run: | mkdir _build && cd _build - cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DENABLE_STDCXX_SYNC=OFF -DUSE_CXX_STD=03 -DENABLE_ENCRYPTION=ON -DENABLE_UNITTESTS=ON -DENABLE_BONDING=${{ matrix.bonding }} -DENABLE_TESTING=ON -DENABLE_EXAMPLES=ON -DENABLE_CODE_COVERAGE=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DENABLE_LOGGING=${{ matrix.logging }} + cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DENABLE_STDCXX_SYNC=OFF -DUSE_CXX_STD=03 -DENABLE_ENCRYPTION=ON -DENABLE_UNITTESTS=ON -DENABLE_BONDING=${{ matrix.bonding }} -DENABLE_TESTING=ON -DENABLE_EXAMPLES=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DENABLE_LOGGING=${{ matrix.logging }} - name: build # That below is likely SonarQube remains, which was removed earlier. #run: cd _build && build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build . @@ -34,9 +37,5 @@ jobs: ulimit -c unlimited cd _build && ctest --extra-verbose SUCCESS=$? - if [ -f core.test-srt ]; then gdb -batch ./test-srt -c core -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; + if [ -f core.test-srt ]; then gdb -batch ./test-srt -c core.test-srt -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; test $SUCCESS == 0; - - name: codecov - run: | - source ./scripts/collect-gcov.sh - bash <(curl -s https://codecov.io/bash) diff --git a/.github/workflows/ubuntu-c++11.yml b/.github/workflows/ubuntu-c++11.yml index 59efed7de8..e0152a09d4 100644 --- a/.github/workflows/ubuntu-c++11.yml +++ b/.github/workflows/ubuntu-c++11.yml @@ -19,17 +19,20 @@ jobs: - uses: actions/checkout@v3 - name: prepare run: | + sudo apt update RUNON=${{ matrix.machine }} if [[ $RUNON == ubuntu-latest ]]; then sudo apt install -y gdb fi + sudo apt install -y tcl cmake + [[ $RUNON == ubuntu-24.04-arm ]] || sudo apt install libssl-dev - name: configure run: | RUNON=${{ matrix.machine }} ENCRYPTION=ON [[ $RUNON == ubuntu-24.04-arm ]] && ENCRYPTION=OFF mkdir _build && cd _build - cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DUSE_CXX_STD=11 -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=$ENCRYPTION -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DENABLE_TESTING=ON -DENABLE_EXAMPLES=ON -DENABLE_CODE_COVERAGE=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON + cmake ../ -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DUSE_CXX_STD=11 -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=$ENCRYPTION -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DENABLE_TESTING=ON -DENABLE_EXAMPLES=ON -DENABLE_DEBUG=ON -DENABLE_CODE_COVERAGE=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON - name: build run: cd _build && cmake --build . - name: test @@ -38,9 +41,13 @@ jobs: ulimit -c unlimited cd _build && ctest --extra-verbose SUCCESS=$? - if [ -f core.test-srt ]; then gdb -batch ./test-srt -c core -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; + if [ -f core.test-srt ]; then gdb -batch ./test-srt -c core.test-srt -ex bt -ex "info thread" -ex quit; else echo "NO CORE - NO CRY!"; fi; test $SUCCESS == 0; - name: codecov - run: | - source ./scripts/collect-gcov.sh - bash <(curl -s https://codecov.io/bash) + uses: codecov/codecov-action@v4 + with: + token: ${{ secrets.CODECOV_TOKEN }} + # name: codecov + # run: | + # ./scripts/codecov/update.sh + # ./scripts/codecov/codecov upload-process ${CODECOV_TOKEN} --search-dir _build diff --git a/CMakeLists.txt b/CMakeLists.txt index 87aba46664..18895c0401 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -910,13 +910,24 @@ if (ENABLE_PROFILE) endif() if (ENABLE_CODE_COVERAGE) - if (HAVE_COMPILER_GNU_COMPAT) - add_definitions(-g -O0 --coverage) - link_libraries(--coverage) - message(STATUS "ENABLE_CODE_COVERAGE: ON") - else() + if (NOT HAVE_COMPILER_GNU_COMPAT) message(FATAL_ERROR "ENABLE_CODE_COVERAGE: option is not supported on this platform") endif() + + block() + if (ENABLE_DEBUG EQUAL 2) + elseif (NOT ENABLE_DEBUG) + else() + set (is_debug 1) + endif() + if (NOT is_debug) + message(FATAL_ERROR "ENABLE_CODE_COVERAGE: requires ENABLE_DEBUG or Debug build type") + endif() + endblock() + + add_definitions(--coverage) + link_libraries(--coverage) + message(STATUS "ENABLE_CODE_COVERAGE: ON") endif() # On Linux pthreads have to be linked even when using C++11 threads diff --git a/configure-data.tcl b/configure-data.tcl index d7b9d49376..0faec42b82 100644 --- a/configure-data.tcl +++ b/configure-data.tcl @@ -34,48 +34,49 @@ set internal_options { # Options that refer directly to variables used in CMakeLists.txt set cmake_options { + atomic-use-srt-sync-mutex "Use mutex to implement atomics (alias: --with-atomic=sync-mutex) (default: OFF)" cygwin-use-posix "Should the POSIX API be used for cygwin. Ignored if the system isn't cygwin. (default: OFF)" - enable-c++11 "Should the c++11 parts (srt-live-transmit) be enabled (default: ON, with gcc < 4.7 OFF)" + enable-aead-api-preview "Enable AEAD API preview in SRT (default: OFF)" enable-apps "Should the Support Applications be Built? (default: ON)" enable-bonding "Enable 'bonding' SRT feature (default: OFF)" - enable-testing "Should developer testing applications be built (default: OFF)" - enable-profile "Should instrument the code for profiling. Ignored for non-GNU compiler. (default: OFF)" - enable-logging "Should logging be enabled (default: ON)" - enable-heavy-logging "Should heavy debug logging be enabled (default: OFF)" + enable-c++11 "Should the c++11 parts (srt-live-transmit) be enabled (default: ON, with gcc < 4.7 OFF)" + enable-c++-deps "Extra library dependencies in srt.pc for C language (default: ON)" + enable-clang-tsa "Enable Clang's Thread-Safety-Analysis (default: OFF)" + enable-code-coverage "Enable code coverage reporting (default: OFF)" + enable-debug=<0,1,2> "Enable debug mode (0=disabled, 1=debug, 2=rel-with-debug)" + enable-encryption "Should encryption features be enabled (default: ON)" + enable-getnameinfo "In-logs sockaddr-to-string should do rev-dns (default: OFF)" enable-haicrypt-logging "Should logging in haicrypt be enabled (default: OFF)" + enable-heavy-logging "Should heavy debug logging be enabled (default: OFF)" + enable-inet-pton "Set to OFF to prevent usage of inet_pton when building against modern SDKs (default: ON)" + enable-logging "Should logging be enabled (default: ON)" + enable-maxrexmitbw "Enable SRTO_MAXREXMITBW (v1.6.0 API preview) (default: OFF)" + enable-monotonic-clock "Enforced clock_gettime with monotonic clock on GC CV /temporary fix for #729/ (default: OFF)" enable-pktinfo "Should pktinfo reading and using be enabled (POSIX only) (default: OFF)" + enable-profile "Should instrument the code for profiling. Ignored for non-GNU compiler. (default: OFF)" + enable-relative-libpath "Should applications contain relative library paths, like ../lib (default: OFF)" enable-shared "Should libsrt be built as a shared library (default: ON)" + enable-show-project-config "Enables use of ShowProjectConfig() in cmake (default: OFF)" + enable-sock-cloexec "Enable setting SOCK_CLOEXEC on a socket (default: ON)" enable-static "Should libsrt be built as a static library (default: ON)" - enable-relative-libpath "Should applications contain relative library paths, like ../lib (default: OFF)" - enable-getnameinfo "In-logs sockaddr-to-string should do rev-dns (default: OFF)" - enable-unittests "Enable Unit Tests (will download Google UT) (default: OFF)" - enable-unittests-discovery "Enable UT Discovery (will run when building) (default: ON)" - enable-encryption "Should encryption features be enabled (default: ON)" - enable-c++-deps "Extra library dependencies in srt.pc for C language (default: ON)" - use-static-libstdc++ "Should use static rather than shared libstdc++ (default: OFF)" - enable-inet-pton "Set to OFF to prevent usage of inet_pton when building against modern SDKs (default: ON)" - enable-code-coverage "Enable code coverage reporting (default: OFF)" - enable-monotonic-clock "Enforced clock_gettime with monotonic clock on GC CV /temporary fix for #729/ (default: OFF)" - enable-thread-check "Enable #include that implements THREAD_* macros" enable-stdc++-sync "Use standard C++11 chrono/threads instead of pthread wrapper (default: OFF, on Windows: ON)" - use-openssl-pc "Use pkg-config to find OpenSSL libraries (default: ON)" - openssl-use-static-libs "Link OpenSSL statically (default: OFF)." - use-busy-waiting "Enable more accurate sending times at a cost of potentially higher CPU load (default: OFF)" - use-gnustl "Get c++ library/headers from the gnustl.pc" - enable-sock-cloexec "Enable setting SOCK_CLOEXEC on a socket (default: ON)" - enable-show-project-config "Enables use of ShowProjectConfig() in cmake (default: OFF)" - enable-new-rcvbuffer "Enables the new receiver buffer implementation (default: ON)" - enable-clang-tsa "Enable Clang's Thread-Safety-Analysis (default: OFF)" - atomic-use-srt-sync-mutex "Use mutex to implement atomics (alias: --with-atomic=sync-mutex) (default: OFF)" - - use-enclib "Encryption library to be used: openssl(default), gnutls, mbedtls, botan" - enable-debug=<0,1,2> "Enable debug mode (0=disabled, 1=debug, 2=rel-with-debug)" - pkg-config-executable= "pkg-config executable" + enable-testing "Should developer testing applications be built (default: OFF)" + enable-thread-check "Enable #include that implements THREAD_* macros" + enable-unittests-discovery "Enable UT Discovery (will run when building) (default: ON)" + enable-unittests "Enable Unit Tests (will download Google UT) (default: OFF)" openssl-crypto-library= "OpenSSL: Path to a libcrypto library." openssl-include-dir= "OpenSSL: Path to includes." openssl-ssl-library= "OpenSSL: Path to a libssl library." + openssl-use-static-libs "Link OpenSSL statically (default: OFF)." + pkg-config-executable= "pkg-config executable" pthread-include-dir= "PThread: Path to includes" pthread-library= "PThread: Path to the pthread library." + srt-use-openssl-static-libs "Link OpenSSL libraries statically. (default: OFF)" + use-busy-waiting "Enable more accurate sending times at a cost of potentially higher CPU load (default: OFF)" + use-enclib "Encryption library to be used: openssl(default), gnutls, mbedtls, botan" + use-gnustl "Get c++ library/headers from the gnustl.pc" + use-openssl-pc "Use pkg-config to find OpenSSL libraries (default: ON)" + use-static-libstdc++ "Should use static rather than shared libstdc++ (default: OFF)" } set options $internal_options$cmake_options diff --git a/scripts/codecov/update.sh b/scripts/codecov/update.sh new file mode 100755 index 0000000000..1aadf9277c --- /dev/null +++ b/scripts/codecov/update.sh @@ -0,0 +1,5 @@ +#!/bin/bash +HERE=`dirname $0` +cd $HERE +curl -L -o codecov https://cli.codecov.io/latest/linux/codecov +chmod +x codecov diff --git a/scripts/collect-gcov.sh b/scripts/collect-gcov.sh deleted file mode 100644 index 7b458e6c1a..0000000000 --- a/scripts/collect-gcov.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -shopt -s globstar -gcov_data_dir="." -for x in ./**/*.o; do - echo "x: $x" - gcov "$gcov_data_dir/$x" -done diff --git a/srtcore/crypto.cpp b/srtcore/crypto.cpp index 93b5bbe60a..cf92175d5c 100644 --- a/srtcore/crypto.cpp +++ b/srtcore/crypto.cpp @@ -946,6 +946,7 @@ srt::EncryptionStatus srt::CCryptoControl::decrypt(CPacket& w_packet SRT_ATR_UNU HLOGC(cnlog.Debug, log << "decrypt: successfully decrypted, resulting length=" << rc); return ENCS_CLEAR; #else + (void)m_bErrorReported; // otherwise warning! return ENCS_NOTSUP; #endif } diff --git a/test/test_crypto.cpp b/test/test_crypto.cpp index c8cc2ce3bd..47fcad8f23 100644 --- a/test/test_crypto.cpp +++ b/test/test_crypto.cpp @@ -7,6 +7,9 @@ #include "gtest/gtest.h" #include "test_env.h" +#ifdef SRT_ENABLE_ENCRYPTION + + #include "crypto.h" #include "handshake.h" #include "hcrypt_msg.h" @@ -14,8 +17,6 @@ #include "socketconfig.h" #include "api.h" -#ifdef SRT_ENABLE_ENCRYPTION - // processSrtMsg_KMRSP must reject malformed wire-supplied lengths before they // reach the fixed-size stack buffer / uninitialised-read paths inside the // function. Built into the library unconditionally, so this test runs diff --git a/test/test_enforced_encryption.cpp b/test/test_enforced_encryption.cpp index 717b4549ca..b3807ffd1e 100644 --- a/test/test_enforced_encryption.cpp +++ b/test/test_enforced_encryption.cpp @@ -59,10 +59,10 @@ enum TEST_CASE TEST_CASE_D_3, TEST_CASE_D_4, TEST_CASE_D_5, + TEST_CASE_COUNT }; - -struct TestResultNonBlocking +struct Expect { int connect_ret; int accept_ret; @@ -72,26 +72,18 @@ struct TestResultNonBlocking int km_state [CHECK_SOCKET_COUNT]; }; - -struct TestResultBlocking -{ - int connect_ret; - int accept_ret; - int socket_state[CHECK_SOCKET_COUNT]; - int km_state[CHECK_SOCKET_COUNT]; -}; - - -template +template struct TestCase { + static const bool blocking = BLOCKING; + bool enforcedenc [PEER_COUNT]; const std::string (&password)[PEER_COUNT]; - TResult expected_result; + Expect expected_result; }; -typedef TestCase TestCaseNonBlocking; -typedef TestCase TestCaseBlocking; +typedef TestCase TestCaseNonBlocking; +typedef TestCase TestCaseBlocking; @@ -139,10 +131,10 @@ static const std::string s_pwd_no(""); const int IGNORE_EPOLL = -2; const int IGNORE_SRTS = -1; -const TestCaseNonBlocking g_test_matrix_non_blocking[] = +const TestCaseNonBlocking g_test_matrix_non_blocking[TEST_CASE_COUNT] = { - // ENFORCEDENC | Password | | EPoll wait | socket_state | KM State - // caller | listener | caller | listener | connect_ret accept_ret | ret | event | caller accepted | caller listener + // ENFORCEDENC | Password | | EPoll wait | socket_state | KM State + // caller | listener | caller | listener | connect_ret accept_ret | ret | event | caller accepted | caller listener /*A.1 */ { {true, true }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, 1, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, /*A.2 */ { {true, true }, {s_pwd_a, s_pwd_b}, { SRT_SUCCESS, SRT_INVALID_SOCK, 0, 0, {SRTS_BROKEN, IGNORE_SRTS}, {SRT_KM_S_UNSECURED, IGNORE_SRTS}}}, /*A.3 */ { {true, true }, {s_pwd_a, s_pwd_no}, { SRT_SUCCESS, SRT_INVALID_SOCK, 0, 0, {SRTS_BROKEN, IGNORE_SRTS}, {SRT_KM_S_UNSECURED, IGNORE_SRTS}}}, @@ -183,33 +175,33 @@ const TestCaseNonBlocking g_test_matrix_non_blocking[] = * * In the cases C.2-C.4 it is the listener who rejects the connection, so we don't have an accepted socket. */ -const TestCaseBlocking g_test_matrix_blocking[] = +const TestCaseBlocking g_test_matrix_blocking[TEST_CASE_COUNT] = { - // ENFORCEDENC | Password | | socket_state | KM State - // caller | listener | caller | listener | connect_ret accept_ret | caller accepted | caller listener -/*A.1 */ { {true, true }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, -/*A.2 */ { {true, true }, {s_pwd_a, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, -/*A.3 */ { {true, true }, {s_pwd_a, s_pwd_no}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, -/*A.4 */ { {true, true }, {s_pwd_no, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, -/*A.5 */ { {true, true }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, - -/*B.1 */ { {true, false }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, -/*B.2 */ { {true, false }, {s_pwd_a, s_pwd_b}, { SRT_INVALID_SOCK, -2, {SRTS_OPENED, SRTS_BROKEN}, {SRT_KM_S_BADSECRET, SRT_KM_S_BADSECRET}}}, -/*B.3 */ { {true, false }, {s_pwd_a, s_pwd_no}, { SRT_INVALID_SOCK, -2, {SRTS_OPENED, SRTS_BROKEN}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, -/*B.4 */ { {true, false }, {s_pwd_no, s_pwd_b}, { SRT_INVALID_SOCK, -2, {SRTS_OPENED, SRTS_BROKEN}, {SRT_KM_S_UNSECURED, SRT_KM_S_NOSECRET}}}, -/*B.5 */ { {true, false }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, - -/*C.1 */ { {false, true }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, -/*C.2 */ { {false, true }, {s_pwd_a, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, -/*C.3 */ { {false, true }, {s_pwd_a, s_pwd_no}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, -/*C.4 */ { {false, true }, {s_pwd_no, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, -/*C.5 */ { {false, true }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, - -/*D.1 */ { {false, false }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, -/*D.2 */ { {false, false }, {s_pwd_a, s_pwd_b}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_BADSECRET, SRT_KM_S_BADSECRET}}}, -/*D.3 */ { {false, false }, {s_pwd_a, s_pwd_no}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, -/*D.4 */ { {false, false }, {s_pwd_no, s_pwd_b}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_NOSECRET, SRT_KM_S_NOSECRET}}}, -/*D.5 */ { {false, false }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, + // ENFORCEDENC | Password | | Epoll wait (ignored) | socket_state | KM State + // caller | listener | caller | listener | connect_ret accept_ret | ret | event | caller accepted | caller listener +/*A.1 */ { {true, true }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, +/*A.2 */ { {true, true }, {s_pwd_a, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, +/*A.3 */ { {true, true }, {s_pwd_a, s_pwd_no}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, +/*A.4 */ { {true, true }, {s_pwd_no, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, +/*A.5 */ { {true, true }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, + +/*B.1 */ { {true, false }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, +/*B.2 */ { {true, false }, {s_pwd_a, s_pwd_b}, { SRT_INVALID_SOCK, -2, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, SRTS_BROKEN}, {SRT_KM_S_BADSECRET, SRT_KM_S_BADSECRET}}}, +/*B.3 */ { {true, false }, {s_pwd_a, s_pwd_no}, { SRT_INVALID_SOCK, -2, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, SRTS_BROKEN}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, +/*B.4 */ { {true, false }, {s_pwd_no, s_pwd_b}, { SRT_INVALID_SOCK, -2, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, SRTS_BROKEN}, {SRT_KM_S_UNSECURED, SRT_KM_S_NOSECRET}}}, +/*B.5 */ { {true, false }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, + +/*C.1 */ { {false, true }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, +/*C.2 */ { {false, true }, {s_pwd_a, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, +/*C.3 */ { {false, true }, {s_pwd_a, s_pwd_no}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, +/*C.4 */ { {false, true }, {s_pwd_no, s_pwd_b}, { SRT_INVALID_SOCK, SRT_INVALID_SOCK, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_OPENED, -1}, {SRT_KM_S_UNSECURED, -1}}}, +/*C.5 */ { {false, true }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, + +/*D.1 */ { {false, false }, {s_pwd_a, s_pwd_a}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_SECURED, SRT_KM_S_SECURED}}}, +/*D.2 */ { {false, false }, {s_pwd_a, s_pwd_b}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_BADSECRET, SRT_KM_S_BADSECRET}}}, +/*D.3 */ { {false, false }, {s_pwd_a, s_pwd_no}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, +/*D.4 */ { {false, false }, {s_pwd_no, s_pwd_b}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_NOSECRET, SRT_KM_S_NOSECRET}}}, +/*D.5 */ { {false, false }, {s_pwd_no, s_pwd_no}, { SRT_SUCCESS, 0, IGNORE_EPOLL, SRT_EPOLL_IN, {SRTS_CONNECTED, SRTS_CONNECTED}, {SRT_KM_S_UNSECURED, SRT_KM_S_UNSECURED}}}, }; @@ -321,14 +313,13 @@ class TestEnforcedEncryption int WaitOnEpoll(const TResult &expect); - template - const TestCase& GetTestMatrix(TEST_CASE test_case) const; + template + const TCase& GetTestMatrix(TEST_CASE test_case) const; - template + template void TestConnect(TEST_CASE test_case/*, bool is_blocking*/) { - const bool is_blocking = std::is_same::value; - if (is_blocking) + if (TCase::blocking) { ASSERT_NE(srt_setsockopt( m_caller_socket, 0, SRTO_RCVSYN, &s_yes, sizeof s_yes), SRT_ERROR); ASSERT_NE(srt_setsockopt( m_caller_socket, 0, SRTO_SNDSYN, &s_yes, sizeof s_yes), SRT_ERROR); @@ -344,7 +335,7 @@ class TestEnforcedEncryption } // Prepare input state - const TestCase &test = GetTestMatrix(test_case); + const auto& test = GetTestMatrix(test_case); ASSERT_EQ(SetEnforcedEncryption(PEER_CALLER, test.enforcedenc[PEER_CALLER]), SRT_SUCCESS); ASSERT_EQ(SetEnforcedEncryption(PEER_LISTENER, test.enforcedenc[PEER_LISTENER]), SRT_SUCCESS); @@ -356,7 +347,7 @@ class TestEnforcedEncryption const bool case_both_relaxed = !test.enforcedenc[PEER_LISTENER] && !test.enforcedenc[PEER_CALLER]; const bool case_sender_enc = test.password[PEER_CALLER] != ""; - const TResult &expect = test.expected_result; + const auto& expect = test.expected_result; // Start testing srt::sync::atomic caller_done; @@ -372,7 +363,7 @@ class TestEnforcedEncryption SRTSOCKET accepted_socket = -1; auto accepting_thread = std::thread([&] { - const int epoll_event = WaitOnEpoll(expect); + const int epoll_event = WaitOnEpoll(test); // In a blocking mode we expect a socket returned from srt_accept() if the srt_connect succeeded. // In a non-blocking mode we expect a socket returned from srt_accept() if the srt_connect succeeded, @@ -460,7 +451,7 @@ class TestEnforcedEncryption caller_done = true; - if (is_blocking == false) + if (TCase::blocking == false) accepting_thread.join(); if (m_is_tracing) @@ -475,7 +466,7 @@ class TestEnforcedEncryption // If a blocking call to srt_connect() returned error, then the state is not valid, // but we still check it because we know what it should be. This way we may see potential changes in the core behavior. - if (is_blocking) + if (TCase::blocking) { EXPECT_EQ(srt_getsockstate(m_caller_socket), expect.socket_state[CHECK_SOCKET_CALLER]); } @@ -496,7 +487,7 @@ class TestEnforcedEncryption EXPECT_EQ(srt_getsockstate(m_listener_socket), SRTS_LISTENING); EXPECT_EQ(GetKMState(m_listener_socket), SRT_KM_S_UNSECURED); - if (!is_blocking && case_both_relaxed && case_pw_failure && case_sender_enc) + if (!TCase::blocking && case_both_relaxed && case_pw_failure && case_sender_enc) { // Additionally check decryption failure does not trigger read-readiness (see issue #2503). @@ -543,7 +534,7 @@ class TestEnforcedEncryption srt_epoll_release(epollRead); } - if (is_blocking) + if (TCase::blocking) { // srt_accept() has no timeout, so we have to close the socket and wait for the thread to exit. // Just give it some time and close the socket. @@ -574,7 +565,7 @@ class TestEnforcedEncryption template<> -int TestEnforcedEncryption::WaitOnEpoll(const TestResultBlocking &) +int TestEnforcedEncryption::WaitOnEpoll(const TestCaseBlocking &) { return SRT_EPOLL_IN; } @@ -607,8 +598,9 @@ static std::ostream& PrintEpollEvent(std::ostream& os, int events, int et_events } template<> -int TestEnforcedEncryption::WaitOnEpoll(const TestResultNonBlocking &expect) +int TestEnforcedEncryption::WaitOnEpoll(const TestCaseNonBlocking &tcase) { + const auto& expect = tcase.expected_result; const int default_len = 3; SRT_EPOLL_EVENT ready[default_len]; const int epoll_res = srt_epoll_uwait(m_pollid, ready, default_len, 500); @@ -649,13 +641,13 @@ int TestEnforcedEncryption::WaitOnEpoll(const TestResultN template<> -const TestCase& TestEnforcedEncryption::GetTestMatrix(TEST_CASE test_case) const +const TestCaseBlocking& TestEnforcedEncryption::GetTestMatrix(TEST_CASE test_case) const { return g_test_matrix_blocking[test_case]; } template<> -const TestCase& TestEnforcedEncryption::GetTestMatrix(TEST_CASE test_case) const +const TestCaseNonBlocking& TestEnforcedEncryption::GetTestMatrix(TEST_CASE test_case) const { return g_test_matrix_non_blocking[test_case]; } @@ -740,20 +732,14 @@ TEST_F(TestEnforcedEncryption, SetGetDefault) } -#define CREATE_TEST_CASE_BLOCKING(CASE_NUMBER, DESC) TEST_F(TestEnforcedEncryption, CASE_NUMBER##_Blocking_##DESC)\ +#define CREATE_TEST_C(IFBLOCKING, CASE_NUMBER, DESC) TEST_F(TestEnforcedEncryption, CASE_NUMBER##_##IFBLOCKING##_##DESC)\ {\ - TestConnect(TEST_##CASE_NUMBER);\ + TestConnect(TEST_##CASE_NUMBER);\ } -#define CREATE_TEST_CASE_NONBLOCKING(CASE_NUMBER, DESC) TEST_F(TestEnforcedEncryption, CASE_NUMBER##_NonBlocking_##DESC)\ -{\ - TestConnect(TEST_##CASE_NUMBER);\ -} - - #define CREATE_TEST_CASES(CASE_NUMBER, DESC) \ - CREATE_TEST_CASE_NONBLOCKING(CASE_NUMBER, DESC) \ - CREATE_TEST_CASE_BLOCKING(CASE_NUMBER, DESC) + CREATE_TEST_C(Blocking, CASE_NUMBER, DESC) \ + CREATE_TEST_C(NonBlocking, CASE_NUMBER, DESC) #ifdef SRT_ENABLE_ENCRYPTION CREATE_TEST_CASES(CASE_A_1, Enforced_On_On_Pwd_Set_Set_Match) From fcae57145c000a9e7b72aa777adb8f85c2463242 Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Thu, 6 Aug 2026 11:43:45 +0200 Subject: [PATCH 02/11] [core] Added checks to prevent rogue CMD MSG to sneak thru (#3323) * [core] Added checks to prevent rogue CMD MSG to sneak thru * [core] Fixed OOB read in ACK payload parsing. * Added protection against rogue DROPREQ. Added status return for cmd dispatchers. Changed tests for cmd dispatchers to work on a connected socket. Added protection against rogue DROP in receiver buffer * Fixed codespell * Wrong keyword for CUnit in tests --------- Co-authored-by: Mikolaj Malecki --- srtcore/api.h | 1 + srtcore/core.cpp | 156 ++++++++++++++++++++++++---------- srtcore/core.h | 20 ++--- srtcore/crypto.cpp | 11 +-- test/test_bonding.cpp | 4 +- test/test_control_packets.cpp | 115 ++++++++++++++++++------- test/test_crypto.cpp | 8 -- test/test_env.h | 31 ++++++- test/test_fec_rebuilding.cpp | 52 +++++++++--- test/test_main.cpp | 40 +++++++++ 10 files changed, 324 insertions(+), 114 deletions(-) diff --git a/srtcore/api.h b/srtcore/api.h index 9f5e560bc1..96302f118b 100644 --- a/srtcore/api.h +++ b/srtcore/api.h @@ -245,6 +245,7 @@ class CUDTUnited friend class CUDTGroup; friend class CRendezvousQueue; friend class CCryptoControl; + friend class TestMockCUDT; public: CUDTUnited(); diff --git a/srtcore/core.cpp b/srtcore/core.cpp index ece511b3e1..1e98f75a20 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -8686,9 +8686,24 @@ void srt::CUDT::updateSndLossListOnACK(int32_t ackdata_seqno) leaveCS(m_StatsLock); } -void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_point& currtime) -{ +bool srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_point& currtime) +{ + // Valid ACK payloads are either LITE (just an ack seqno) or at least SMALL + // (RCVLASTACK + RTT + RTTVAR + BUFFERLEFT = 16 B). Anything else would OOB-read. + const size_t pktlen = ctrlpkt.getLength(); + const bool isLiteAck = pktlen == size_t(SEND_LITE_ACK); + if (!isLiteAck && pktlen < ACKD_TOTAL_SIZE_SMALL * ACKD_FIELD_SIZE) + { + LOGC(inlog.Warn, log << CONID() << "ACK: EPE: wrong payload size=" << pktlen + << " expected 4 or at least SMALL (" + << (ACKD_TOTAL_SIZE_SMALL * ACKD_FIELD_SIZE) + << ") - rejecting"); + return false; + } + const int32_t* ackdata = (const int32_t*)ctrlpkt.m_pcData; + + // Note: minimum of one 4-byte field is granted before the call. const int32_t ackdata_seqno = ackdata[ACKD_RCVLASTACK]; // Check the value of ACK in case when it was some rogue peer @@ -8700,10 +8715,9 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ // This check MUST BE DONE before making any operation on this number. LOGC(inlog.Error, log << CONID() << "ACK: IPE/EPE: received invalid ACK value: " << ackdata_seqno << " " << std::hex << ackdata_seqno << " (IGNORED)"); - return; + return false; } - const bool isLiteAck = ctrlpkt.getLength() == (size_t)SEND_LITE_ACK; HLOGC(inlog.Debug, log << CONID() << "ACK covers: " << m_iSndLastDataAck << " - " << ackdata_seqno << " [ACK=" << m_iSndLastAck << "]" << (isLiteAck ? "[LITE]" : "[FULL]")); @@ -8722,7 +8736,16 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ m_tsLastRspAckTime = currtime; m_iReXmitCount = 1; // Reset re-transmit count since last ACK } - return; + return true; + } + + const size_t acksize = pktlen / ACKD_FIELD_SIZE; // ACTUAL VALUE + + // Check minimum size acceptable. If less, reject it. + if (acksize < ACKD_TOTAL_SIZE_SMALL) + { + LOGC(inlog.Error, log << CONID() << "EPE: ACK msg received with too small size: " << pktlen); + return false; } // Decide to send ACKACK or not @@ -8764,7 +8787,7 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ updateBrokenConnection(); completeBrokenConnectionDependencies(SRT_ESECFAIL); // LOCKS! - return; + return false; } if (CSeqNo::seqcmp(ackdata_seqno, m_iSndLastAck) >= 0) @@ -8800,7 +8823,7 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ if (CSeqNo::seqoff(m_iSndLastFullAck, ackdata_seqno) <= 0) { // discard it if it is a repeated ACK - return; + return true; } m_iSndLastFullAck = ackdata_seqno; } @@ -8820,24 +8843,12 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ } #endif - size_t acksize = ctrlpkt.getLength(); // TEMPORARY VALUE FOR CHECKING - bool wrongsize = 0 != (acksize % ACKD_FIELD_SIZE); - acksize = acksize / ACKD_FIELD_SIZE; // ACTUAL VALUE - - if (wrongsize) - { - // Issue a log, but don't do anything but skipping the "odd" bytes from the payload. - LOGC(inlog.Warn, - log << CONID() << "Received UMSG_ACK payload is not evened up to 4-byte based field size - cutting to " - << acksize << " fields"); - } - // Start with checking the base size. if (acksize < ACKD_TOTAL_SIZE_SMALL) { LOGC(inlog.Warn, log << CONID() << "Invalid ACK size " << acksize << " fields - less than minimum required!"); // Ack is already interpreted, just skip further parts. - return; + return false; } // This check covers fields up to ACKD_BUFFERLEFT. @@ -8947,9 +8958,11 @@ void srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ enterCS(m_StatsLock); m_stats.sndr.recvdAck.count(1); leaveCS(m_StatsLock); + + return true; } -void srt::CUDT::processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsArrival) +bool srt::CUDT::processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsArrival) { int32_t ack = 0; @@ -8975,14 +8988,14 @@ void srt::CUDT::processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsAr } #endif - return; + return false; } LOGC(inlog.Error, log << CONID() << "ACK record not found, can't estimate RTT " << "(ACK number: " << ctrlpkt.getAckSeqNo() << ", last ACK sent: " << m_iAckSeqNo << ", RTT (EWMA): " << m_iSRTT << ")"); - return; + return false; } if (rtt <= 0) @@ -8990,7 +9003,7 @@ void srt::CUDT::processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsAr LOGC(inlog.Error, log << CONID() << "IPE: invalid RTT estimate " << rtt << ", possible time shift. Clock: " << SRT_SYNC_CLOCK_STR); - return; + return false; } // If increasing delay is detected. @@ -9045,12 +9058,13 @@ void srt::CUDT::processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsAr // Update last ACK that has been received by the sender if (CSeqNo::seqcmp(ack, m_iRcvLastAckAck) > 0) m_iRcvLastAckAck = ack; + return true; } -void srt::CUDT::processCtrlLossReport(const CPacket& ctrlpkt) +bool srt::CUDT::processCtrlLossReport(const CPacket& ctrlpkt) { const int32_t* losslist = (int32_t*)(ctrlpkt.m_pcData); - const size_t losslist_len = ctrlpkt.getLength() / 4; + const size_t losslist_len = ctrlpkt.getLength() / sizeof(int32_t); bool secure = true; @@ -9205,7 +9219,7 @@ void srt::CUDT::processCtrlLossReport(const CPacket& ctrlpkt) updateBrokenConnection(); completeBrokenConnectionDependencies(SRT_ESECFAIL); // LOCKS! - return; + return false; } // the lost packet (retransmission) should be sent out immediately @@ -9214,12 +9228,18 @@ void srt::CUDT::processCtrlLossReport(const CPacket& ctrlpkt) enterCS(m_StatsLock); m_stats.sndr.recvdNak.count(1); leaveCS(m_StatsLock); + + return true; } -void srt::CUDT::processCtrlHS(const CPacket& ctrlpkt) +bool srt::CUDT::processCtrlHS(const CPacket& ctrlpkt) { CHandShake req; - req.load_from(ctrlpkt.m_pcData, ctrlpkt.getLength()); + if (-1 == req.load_from(ctrlpkt.m_pcData, ctrlpkt.getLength())) + { + LOGC(inlog.Error, log << CONID() << "processCtrlHS: EPE: Handshake has wrong size: " << ctrlpkt.getLength()); + return false; + } HLOGC(inlog.Debug, log << CONID() << "processCtrl: got HS: " << req.show()); @@ -9326,21 +9346,51 @@ void srt::CUDT::processCtrlHS(const CPacket& ctrlpkt) else { HLOGC(inlog.Debug, log << CONID() << "processCtrl: ... not INDUCTION, not ERROR, not rendezvous - IGNORED."); + return false; } + return true; } -void srt::CUDT::processCtrlDropReq(const CPacket& ctrlpkt) +bool srt::CUDT::processCtrlDropReq(const CPacket& ctrlpkt) { // dropdata[0..1] are indexed unconditionally below. if (ctrlpkt.getLength() < 2 * sizeof(int32_t)) { LOGC(inlog.Warn, log << CONID() << "DROPREQ: payload " << ctrlpkt.getLength() << " bytes < " << (2 * sizeof(int32_t)) << " - rejecting"); - return; + return false; } const int32_t* dropdata = (const int32_t*) ctrlpkt.m_pcData; + // The wire format carries a (lo, hi) seqno range. Reject reversed + // ranges: dropMessage walks a circular buffer from offset(lo) to + // offset(hi)+1 via incPos(); when seqcmp(lo, hi) > 0 the loop wraps + // and clears nearly the entire receive buffer (DoS primitive). The + // analogous LOSSREPORT path already rejects reversed ranges. + + // This is for check only - one packet read will not spoil it + m_RcvBufferLock.lock(); + const int32_t hookseq_begin = m_pRcvBuffer->getStartSeqNo(); + m_RcvBufferLock.unlock(); + + int dist_begin = CSeqNo::seqoff(dropdata[0], hookseq_begin); + int dist_rel = CSeqNo::seqoff(dropdata[0], dropdata[1]); + + if (abs(dist_begin) > CSeqNo::m_iSeqNoTH/2) + { + LOGC(inlog.Warn, log << CONID() << "EPE: rcv DROPREQ low rng %" + << dropdata[0] << " - too distant to receiver buffer %" << hookseq_begin + << " by " << dist_begin << " (exceeds threshold)"); + return false; + } + if (dist_rel < 0 || dist_rel > CSeqNo::m_iSeqNoTH/2) + { + LOGC(inlog.Warn, log << CONID() << "EPE: rcv DROPREQ rng %" + << dropdata[0] << " - %" << dropdata[1] << " - REVERSED RANGE, DISCARDING"); + return false; + } + { CUniqueSync rcvtscc (m_RecvLock, m_RcvTsbPdCond); // With both TLPktDrop and TsbPd enabled, a message always consists only of one packet. @@ -9402,9 +9452,11 @@ void srt::CUDT::processCtrlDropReq(const CPacket& ctrlpkt) HLOGC(inlog.Debug, log << CONID() << "DROPREQ: dropping %" << dropdata[0] << "-" << dropdata[1] << " current %" << m_iRcvCurrSeqNo); } + + return true; } -void srt::CUDT::processCtrlShutdown() +bool srt::CUDT::processCtrlShutdown() { m_bShutdown = true; m_bClosing = true; @@ -9415,9 +9467,10 @@ void srt::CUDT::processCtrlShutdown() // just we know about this state prematurely thanks to this message. updateBrokenConnection(); completeBrokenConnectionDependencies(SRT_ECONNLOST); // LOCKS! + return true; } -void srt::CUDT::processCtrlUserDefined(const CPacket& ctrlpkt) +bool srt::CUDT::processCtrlUserDefined(const CPacket& ctrlpkt) { HLOGC(inlog.Debug, log << CONID() << "CONTROL EXT MSG RECEIVED:" << MessageTypeStr(ctrlpkt.getType(), ctrlpkt.getExtendedType()) @@ -9445,31 +9498,44 @@ void srt::CUDT::processCtrlUserDefined(const CPacket& ctrlpkt) { updateCC(TEV_CUSTOM, EventVariant(&ctrlpkt)); } + return true; } -void srt::CUDT::processCtrl(const CPacket &ctrlpkt) +bool srt::CUDT::processCtrl(const CPacket &ctrlpkt) { // Just heard from the peer, reset the expiration count. m_iEXPCount = 1; const steady_clock::time_point currtime = steady_clock::now(); m_tsLastRspTime = currtime; + // Extra check for the payload size: + // - must be aligned to int32_t + // - cannot be 0 (msgs with no args use 4-byte zero-filled padding). + size_t pktlen = ctrlpkt.getLength(); + if (!pktlen || pktlen % sizeof(int32_t) != 0) + { + LOGC(inlog.Error, log << CONID() << "EPE: incoming UMSG: " << ctrlpkt.getType() << " INVALID SIZE: " << pktlen + << " (expected > 0 and aligned to " << sizeof(int32_t) << " bytes)"); + return false; + } + HLOGC(inlog.Debug, log << CONID() << "incoming UMSG:" << ctrlpkt.getType() << " (" << MessageTypeStr(ctrlpkt.getType(), ctrlpkt.getExtendedType()) << ") socket=%" << ctrlpkt.id()); + bool result = false; switch (ctrlpkt.getType()) { case UMSG_ACK: // 010 - Acknowledgement - processCtrlAck(ctrlpkt, currtime); + result = processCtrlAck(ctrlpkt, currtime); break; case UMSG_ACKACK: // 110 - Acknowledgement of Acknowledgement - processCtrlAckAck(ctrlpkt, currtime); + result = processCtrlAckAck(ctrlpkt, currtime); break; case UMSG_LOSSREPORT: // 011 - Loss Report - processCtrlLossReport(ctrlpkt); + result = processCtrlLossReport(ctrlpkt); break; case UMSG_CGWARNING: // 100 - Delay Warning @@ -9483,19 +9549,19 @@ void srt::CUDT::processCtrl(const CPacket &ctrlpkt) break; case UMSG_KEEPALIVE: // 001 - Keep-alive - processKeepalive(ctrlpkt, currtime); + result = processKeepalive(ctrlpkt, currtime); break; case UMSG_HANDSHAKE: // 000 - Handshake - processCtrlHS(ctrlpkt); + result = processCtrlHS(ctrlpkt); break; case UMSG_SHUTDOWN: // 101 - Shutdown - processCtrlShutdown(); + result = processCtrlShutdown(); break; case UMSG_DROPREQ: // 111 - Msg drop request - processCtrlDropReq(ctrlpkt); + result = processCtrlDropReq(ctrlpkt); break; case UMSG_PEERERROR: // 1000 - An error has happened to the peer side @@ -9505,16 +9571,18 @@ void srt::CUDT::processCtrl(const CPacket &ctrlpkt) // if recvfile() fails (e.g., due to disk fail), blocked sendfile/send should return immediately // giving the app a chance to fix the issue m_bPeerHealth = false; + result = true; break; case UMSG_EXT: // 0x7FFF - reserved and user defined messages - processCtrlUserDefined(ctrlpkt); + result = processCtrlUserDefined(ctrlpkt); break; default: break; } + return result; } void srt::CUDT::updateSrtRcvSettings() @@ -12541,7 +12609,7 @@ bool srt::CUDT::runAcceptHook(CUDT *acore, const sockaddr* peer, const CHandShak return true; } -void srt::CUDT::processKeepalive(const CPacket& ctrlpkt, const time_point& tsArrival) +bool srt::CUDT::processKeepalive(const CPacket& ctrlpkt, const time_point& tsArrival) { // Here can be handled some protocol definition // for extra data sent through keepalive. @@ -12570,6 +12638,8 @@ void srt::CUDT::processKeepalive(const CPacket& ctrlpkt, const time_point& tsArr m_pRcvBuffer->updateTsbPdTimeBase(ctrlpkt.getMsgTimeStamp()); if (m_config.bDriftTracer) m_pRcvBuffer->addRcvTsbPdDriftSample(ctrlpkt.getMsgTimeStamp(), tsArrival, -1); + + return true; } namespace srt { diff --git a/srtcore/core.h b/srtcore/core.h index 31cedbd4ff..f2f1dc8bd2 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -1232,35 +1232,35 @@ class CUDT int sendCtrlAck(CPacket& ctrlpkt, int size); void sendLossReport(const std::vector< std::pair >& losslist); - void processCtrl(const CPacket& ctrlpkt); - + bool processCtrl(const CPacket& ctrlpkt); + /// @brief Process incoming control ACK packet. /// @param ctrlpkt incoming ACK packet /// @param currtime current clock time - void processCtrlAck(const CPacket& ctrlpkt, const time_point& currtime); + bool processCtrlAck(const CPacket& ctrlpkt, const time_point& currtime); /// @brief Process incoming control ACKACK packet. /// @param ctrlpkt incoming ACKACK packet /// @param tsArrival time when packet has arrived (used to calculate RTT) - void processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsArrival); + bool processCtrlAckAck(const CPacket& ctrlpkt, const time_point& tsArrival); /// @brief Process incoming loss report (NAK) packet. /// @param ctrlpkt incoming NAK packet - void processCtrlLossReport(const CPacket& ctrlpkt); + bool processCtrlLossReport(const CPacket& ctrlpkt); /// @brief Process incoming handshake control packet /// @param ctrlpkt incoming HS packet - void processCtrlHS(const CPacket& ctrlpkt); + bool processCtrlHS(const CPacket& ctrlpkt); /// @brief Process incoming drop request control packet /// @param ctrlpkt incoming drop request packet - void processCtrlDropReq(const CPacket& ctrlpkt); + bool processCtrlDropReq(const CPacket& ctrlpkt); /// @brief Process incoming shutdown control packet - void processCtrlShutdown(); + bool processCtrlShutdown(); /// @brief Process incoming user defined control packet /// @param ctrlpkt incoming user defined packet - void processCtrlUserDefined(const CPacket& ctrlpkt); + bool processCtrlUserDefined(const CPacket& ctrlpkt); /// @brief Update sender's loss list on an incoming acknowledgement. /// @param ackdata_seqno sequence number of a data packet being acknowledged @@ -1334,7 +1334,7 @@ class CUDT static void addLossRecord(std::vector& lossrecord, int32_t lo, int32_t hi); int32_t bake(const sockaddr_any& addr, int32_t previous_cookie = 0, int correction = 0); - void processKeepalive(const CPacket& ctrlpkt, const time_point& tsArrival); + bool processKeepalive(const CPacket& ctrlpkt, const time_point& tsArrival); SRT_ATTR_REQUIRES(m_RcvBufferLock) diff --git a/srtcore/crypto.cpp b/srtcore/crypto.cpp index cf92175d5c..ccc4714609 100644 --- a/srtcore/crypto.cpp +++ b/srtcore/crypto.cpp @@ -372,17 +372,16 @@ int srt::CCryptoControl::processSrtMsg_KMREQ( int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, unsigned srtv) { + uint32_t srtd[SRTDATA_MAXSIZE]; + size_t srtlen = len/sizeof(uint32_t); // Validate the wire-supplied length before using it: // - oversize would overflow the fixed-size stack buffer below; // - non-word-aligned or too-small payloads are malformed by protocol and would // feed uninitialised stack into downstream key-matching logic. - if (len > SRT_CMD_MAXSZ - || len < sizeof(uint32_t) - || (len % sizeof(uint32_t)) != 0) + if (srtlen > SRTDATA_MAXSIZE) { LOGC(cnlog.Error, log << "processSrtMsg_KMRSP: malformed len " << len - << " (must be a non-zero multiple of " << sizeof(uint32_t) - << ", up to " << SRT_CMD_MAXSZ << ") - rejecting"); + << " (must be up to " << SRT_CMD_MAXSZ << ") - rejecting"); return SRT_CMD_NONE; } @@ -390,8 +389,6 @@ int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len * But HaiCrypt expect network order message * Re-swap to cancel it. */ - uint32_t srtd[SRTDATA_MAXSIZE]; - size_t srtlen = len/sizeof(uint32_t); HtoNLA(srtd, srtdata, srtlen); int retstatus = -1; diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index 8414e43e34..5948eed7c8 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -542,8 +542,8 @@ TEST(Bonding, Options) #ifdef ENABLE_AEAD_API_PREVIEW EXPECT_NE(srt_getsockflag(grp, SRTO_CRYPTOMODE, &kms, &optsize), SRT_ERROR); - EXPECT_EQ(optsize, sizeof kms); - EXPECT_EQ(kms, 1); + EXPECT_EQ(optsize, (int) sizeof kms); + EXPECT_EQ(kms, uint32_t(SRT_KM_S_SECURING)); #endif #endif diff --git a/test/test_control_packets.cpp b/test/test_control_packets.cpp index 98ad378317..c588b8a3b2 100644 --- a/test/test_control_packets.cpp +++ b/test/test_control_packets.cpp @@ -1,3 +1,5 @@ +#include + #include "gtest/gtest.h" #include "test_env.h" @@ -16,7 +18,7 @@ namespace srt { public: CUDT* core; - void processCtrlDropReq(const CPacket& pkt) { core->processCtrlDropReq(pkt); } + bool processCtrl(const CPacket& pkt) { return core->processCtrl(pkt); } void processCtrlLossReport(const CPacket& pkt) { core->processCtrlLossReport(pkt); } int32_t rcvCurrSeqNo() const { return core->m_iRcvCurrSeqNo; } void setRcvCurrSeqNo(int32_t v) { core->m_iRcvCurrSeqNo = v; } @@ -24,24 +26,66 @@ namespace srt { }; } -// processCtrlDropReq must reject DROPREQs whose payload is smaller than two -// seqno words; otherwise dropdata[1] reads past the wire payload. -TEST(ControlPackets, DropReqRejectsShortPayload) +class ControlPackets: public srt::Test { - srt::TestInit srtinit; +public: + SRTSOCKET caller = SRT_INVALID_SOCK; + SRTSOCKET listener = SRT_INVALID_SOCK; + SRTSOCKET accepted = SRT_INVALID_SOCK; + CUDTSocket* pcaller = NULL; + TestMockControlPackets cmock; + + static void swipe(SRTSOCKET& sockid) + { + if (sockid == SRT_INVALID_SOCK) + return; + + EXPECT_NE(srt_close(sockid), SRT_ERROR); + sockid = SRT_INVALID_SOCK; + } + + void setup() override + { + caller = CUDT::uglobal().newSocket(&pcaller); + ASSERT_NE(caller, SRT_INVALID_SOCK); + cmock.core = &pcaller->core(); - CUDTSocket* s1 = NULL; - SRTSOCKET sid1 = CUDT::uglobal().newSocket(&s1); - ASSERT_NE(sid1, SRT_INVALID_SOCK); + ASSERT_NE(listener = srt_create_socket(), SRT_INVALID_SOCK); - TestMockControlPackets m1; - m1.core = &s1->core(); + srt::sockaddr_any sa = srt::CreateAddr("localhost", 5555, AF_INET); + ASSERT_NE(srt_bind(listener, sa.get(), sa.size()), SRT_ERROR); + ASSERT_NE(srt_listen(listener, 1), SRT_ERROR); + + std::thread spawned_connect( [this, &sa] { EXPECT_NE(srt_connect(caller, sa.get(), sa.size()), SRT_ERROR); }); + + accepted = srt_accept(listener, NULL, 0); + spawned_connect.join(); + ASSERT_NE(accepted, SRT_ERROR); + } + + void stop() + { + swipe(caller); + } + + void teardown() override + { + swipe(caller); + swipe(accepted); + swipe(listener); + } +}; +// processCtrlDropReq must reject DROPREQs whose payload is smaller than two +// seqno words; otherwise dropdata[1] reads past the wire payload. +TEST_F(ControlPackets, DropReqRejectsShortPayload) +{ const int32_t sentinel = 100; - m1.setRcvCurrSeqNo(sentinel); + cmock.setRcvCurrSeqNo(sentinel); CPacket pkt; pkt.allocate(1500); + pkt.setControl(UMSG_DROPREQ); // Each of these is shorter than the 8-byte minimum and must be rejected // by the guard at the top of processCtrlDropReq. @@ -49,13 +93,34 @@ TEST(ControlPackets, DropReqRejectsShortPayload) for (size_t i = 0; i < sizeof(short_lens) / sizeof(short_lens[0]); ++i) { pkt.setLength(short_lens[i]); - m1.processCtrlDropReq(pkt); - EXPECT_EQ(m1.rcvCurrSeqNo(), sentinel) + EXPECT_FALSE(cmock.processCtrl(pkt)); + EXPECT_EQ(cmock.rcvCurrSeqNo(), sentinel) << "DROPREQ with payload " << short_lens[i] << " bytes must not be processed"; } +} + +// processCtrlDropReq must reject DROPREQs whose (lo, hi) seqno range is +// reversed. Otherwise CRcvBuffer::dropMessage walks the circular buffer from +// offset(lo) past offset(hi)+1 via incPos() and wipes nearly every entry -- +// a DoS primitive triggerable by a single malicious DROPREQ. +TEST_F(ControlPackets, DropReqRejectsReversedRange) +{ + const int32_t sentinel = 1000; + cmock.setRcvCurrSeqNo(sentinel); + + CPacket pkt; + pkt.allocate(8); + int32_t* data = (int32_t*) pkt.m_pcData; + data[0] = 2000; // lo + data[1] = 1500; // hi (seqcmp(lo, hi) > 0) + pkt.setLength(8); + pkt.setControl(UMSG_DROPREQ); - pkt.deallocate(); - srt_close(sid1); + // With the guard, this returns before touching m_pRcvBuffer (NULL on + // an unconnected socket -- would crash if the guard were missing). + EXPECT_FALSE(cmock.processCtrl(pkt)); + + EXPECT_EQ(cmock.rcvCurrSeqNo(), sentinel); } // processCtrlLossReport must reject a LOSSREPORT whose final cell carries @@ -63,18 +128,8 @@ TEST(ControlPackets, DropReqRejectsShortPayload) // otherwise losslist[i+1] reads past the wire payload (4-byte OOB read of // adjacent heap). The handler should mark the connection broken via the // `secure = false` path. -TEST(ControlPackets, LossReportRejectsTrailingRangeFirst) +TEST_F(ControlPackets, LossReportRejectsTrailingRangeFirst) { - srt::TestInit srtinit; - - CUDTSocket* s = NULL; - SRTSOCKET sid = CUDT::uglobal().newSocket(&s); - ASSERT_NE(sid, SRT_INVALID_SOCK); - - TestMockControlPackets m; - m.core = &s->core(); - ASSERT_FALSE(m.isBroken()) << "fresh socket should not be marked broken"; - // Single 4-byte payload, high bit set => LOSSDATA_SEQNO_RANGE_FIRST. // Without the guard, the handler would dereference losslist[1] (the // missing HI cell), reading 4 bytes past the packet payload. @@ -85,12 +140,10 @@ TEST(ControlPackets, LossReportRejectsTrailingRangeFirst) pkt.setLength(sizeof(int32_t)); pkt.setControl(UMSG_LOSSREPORT); - m.processCtrlLossReport(pkt); + EXPECT_FALSE(cmock.processCtrl(pkt)); - EXPECT_TRUE(m.isBroken()) + EXPECT_TRUE(cmock.isBroken()) << "LOSSREPORT with trailing range-first marker must take the " "secure=false bail path and mark the connection broken"; - - pkt.deallocate(); - srt_close(sid); } + diff --git a/test/test_crypto.cpp b/test/test_crypto.cpp index 47fcad8f23..1d04b19238 100644 --- a/test/test_crypto.cpp +++ b/test/test_crypto.cpp @@ -30,14 +30,6 @@ TEST(CryptoKMRSP, RejectsMalformedLengths) // Oversize: would overflow uint32_t srtd[SRTDATA_MAXSIZE]. EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), SRT_CMD_MAXSZ + sizeof(uint32_t), srtv), srt::SRT_CMD_NONE); - - // Non-word-aligned: silently drops bytes and risks misinterpretation. - EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), 7, srtv), srt::SRT_CMD_NONE); - - // Empty / under-a-word: HtoNLA writes nothing and downstream code would read - // uninitialised stack from srtd[]. - EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), 0, srtv), srt::SRT_CMD_NONE); - EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), 3, srtv), srt::SRT_CMD_NONE); } diff --git a/test/test_env.h b/test/test_env.h index edceee6e65..3f90ccaf5d 100644 --- a/test/test_env.h +++ b/test/test_env.h @@ -7,6 +7,7 @@ #include #include "gtest/gtest.h" +#include "sync.h" namespace srt { @@ -145,8 +146,36 @@ class Test: public testing::Test } }; +class CUDT; +class CPacket; +struct CUnit; +class CUDTSocket; + +class TestMockCUDT +{ +public: + CUDT* core; + + TestMockCUDT() : core(NULL) {} + + bool setSocket(int32_t socket); + + // This is used in TestFEC; leaving with a single forwarder + // to keep the test as is. The class can be as well extended. + bool checkApplyFilterConfig(const std::string& s); + + bool processSrtMsg(const srt::CPacket *ctrlpkt); + int rcvKmState(); + int processData(CUnit* u); + CUDTSocket* locateSocket(int32_t s); + + void processCtrlAck(const CPacket& pkt, const sync::steady_clock::time_point& t); + int flowWindowSize() const; + void setFlowWindowSize(int v); +}; + struct sockaddr_any CreateAddr(const std::string& name, unsigned short port, int pref_family); -} //namespace +} //namespace srt #endif diff --git a/test/test_fec_rebuilding.cpp b/test/test_fec_rebuilding.cpp index 3910c959b9..da78aaa62f 100644 --- a/test/test_fec_rebuilding.cpp +++ b/test/test_fec_rebuilding.cpp @@ -109,18 +109,6 @@ static std::future spawn_connect(SRTSOCKET s, sockaddr_in& sa, int timeout_ }); } -namespace srt { - class TestMockCUDT - { - public: - CUDT* core; - - bool checkApplyFilterConfig(const string& s) - { - return core->checkApplyFilterConfig(s); - } - }; -} // The expected whole procedure of connection using FEC is // expected to: @@ -946,3 +934,43 @@ TEST_F(TestFECRebuilding, Rebuild) EXPECT_EQ(memcmp(skipped.data(), rebuilt.data(), rebuilt.size()), 0); } + +// processCtrlAck has two OOB-read sites for intermediate payload sizes: +// - ackdata[ACKD_RCVLASTACK] (index 0) is read up front, OOB for 0-3 byte payloads; +// - ackdata[ACKD_BUFFERLEFT] (index 3) is read in the slow path, OOB for 5-15 byte +// payloads (the lite-ACK fast path matches exactly 4 bytes). +// Valid payloads are LITE (4 B) or SMALL+ (>=16 B). The guard at the top of the +// handler rejects everything else. +TEST(TestCUDT, AckRejectsIntermediatePayload) +{ + srt::TestInit srtinit; + + CUDTSocket* s1 = NULL; + SRTSOCKET sid1 = CUDT::uglobal().newSocket(&s1); + + TestMockCUDT m1; + m1.core = &s1->core(); + + const int sentinel = 0x5A5A5A5A; + m1.setFlowWindowSize(sentinel); + + CPacket pkt; + pkt.allocate(1500); + + // Fill the payload with bytes that would be plausible ack-seqnos if interpreted + // as int32 (non-negative), so the ackdata_seqno < 0 early return doesn't mask + // the bug for the 0-3 byte cases. + std::memset(pkt.m_pcData, 0x01, 1500); + + const size_t bad_lens[] = { 0, 1, 3, 5, 8, 12, 15 }; + const sync::steady_clock::time_point now = sync::steady_clock::now(); + for (size_t i = 0; i < sizeof(bad_lens) / sizeof(bad_lens[0]); ++i) + { + pkt.setLength(bad_lens[i]); + m1.processCtrlAck(pkt, now); + EXPECT_EQ(m1.flowWindowSize(), sentinel) + << "ACK with payload " << bad_lens[i] << " bytes must not corrupt m_iFlowWindowSize"; + } + + srt_close(sid1); +} diff --git a/test/test_main.cpp b/test/test_main.cpp index e2243a3065..701a8670c3 100644 --- a/test/test_main.cpp +++ b/test/test_main.cpp @@ -8,6 +8,7 @@ #include "srt.h" #include "netinet_any.h" +#include "api.h" using namespace std; @@ -212,4 +213,43 @@ void UniqueSocket::close() } } +bool TestMockCUDT::checkApplyFilterConfig(const string& s) +{ + return core->checkApplyFilterConfig(s); +} + +bool TestMockCUDT::processSrtMsg(const srt::CPacket *ctrlpkt) +{ + return core->processSrtMsg(ctrlpkt); +} + +int TestMockCUDT::rcvKmState() +{ + return core->m_pCryptoControl->m_RcvKmState; +} + +int TestMockCUDT::processData(CUnit* u) +{ + return core->processData(u); +} + +CUDTSocket* TestMockCUDT::locateSocket(int32_t s) +{ + SRTSOCKET sock (s); + return CUDT::uglobal().locateSocket(sock); +} + +bool TestMockCUDT::setSocket(int32_t sock) +{ + CUDTSocket* s = locateSocket(sock); + if (!s) + return false; + core = &s->core(); + return true; +} + +void TestMockCUDT::processCtrlAck(const CPacket& pkt, const sync::steady_clock::time_point& t) { core->processCtrlAck(pkt, t); } +int TestMockCUDT::flowWindowSize() const { return core->m_iFlowWindowSize; } +void TestMockCUDT::setFlowWindowSize(int v) { core->m_iFlowWindowSize = v; } + } From 6f817b637824f9c4d563d9f2747bfbee010127ad Mon Sep 17 00:00:00 2001 From: Sektor van Skijlen Date: Wed, 26 Aug 2026 09:41:42 +0200 Subject: [PATCH 03/11] [core] Security improvements and enhancements (#3359) * Fixed vulnerabilities reported as SRTX-61 * Next portion of fixes from SRTX-62 * Fixed #11 for SRTX-62 * Added MD5 check to the dependency installer scripts * Applied safety fixes for Windows installer scripts * Post-review fixes for vulnerability issues * Post-review fixes, take 3 * Remaining parts of Take 3 * Fixes, take 4. Fixed ASSERT usage in FEC tests * Take 5, possibly final * Fixed build break with --disable-encryption * Fixed codespell --------- Co-authored-by: Mikolaj Malecki --- CMakeLists.txt | 7 +- apps/srt-file-transmit.cpp | 59 +++- scripts/codecov/update.sh | 43 ++- .../win-installer/ATTIC/old-install-nsis.ps1 | 128 ++++++++ scripts/win-installer/build-win-installer.ps1 | 3 +- scripts/win-installer/install-libsrt.ps1 | 3 +- scripts/win-installer/install-nsis.ps1 | 285 ++++++++++-------- scripts/win-installer/install-openssl.ps1 | 98 ++++-- srtcore/api.cpp | 93 ++++-- srtcore/buffer_snd.cpp | 7 + srtcore/common.h | 6 + srtcore/congctl.cpp | 6 +- srtcore/core.cpp | 179 ++++++----- srtcore/core.h | 19 +- srtcore/crypto.cpp | 120 ++++---- srtcore/crypto.h | 2 +- srtcore/fec.cpp | 176 ++++++----- srtcore/fec.h | 8 +- srtcore/group.cpp | 31 +- srtcore/group_backup.cpp | 24 ++ srtcore/group_backup.h | 4 + srtcore/handshake.cpp | 5 +- srtcore/list.cpp | 8 +- srtcore/list.h | 9 +- srtcore/packet.h | 7 +- srtcore/queue.cpp | 54 ++-- srtcore/srt.h | 7 +- test/test_bonding.cpp | 7 + test/test_crypto.cpp | 19 +- test/test_fec_rebuilding.cpp | 146 ++++----- 30 files changed, 1056 insertions(+), 507 deletions(-) create mode 100644 scripts/win-installer/ATTIC/old-install-nsis.ps1 mode change 100644 => 100755 scripts/win-installer/build-win-installer.ps1 mode change 100644 => 100755 scripts/win-installer/install-libsrt.ps1 mode change 100644 => 100755 scripts/win-installer/install-nsis.ps1 mode change 100644 => 100755 scripts/win-installer/install-openssl.ps1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 18895c0401..e10715a258 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -172,6 +172,7 @@ else() endif() option(ENABLE_APPS "Should the Support Applications be Built?" ON) option(ENABLE_BONDING "Should the bonding functionality be enabled?" OFF) +option(ENABLE_EXAMPLES "Compile also examples (NOTE: MAY BE UNSAFE!)" OFF) option(ENABLE_TESTING "Should the Developer Test Applications be Built?" OFF) option(ENABLE_PROFILE "Should instrument the code for profiling. Ignored for non-GNU compiler." $ENV{HAI_BUILD_PROFILE}) option(ENABLE_LOGGING "Should logging be enabled" ON) @@ -914,16 +915,14 @@ if (ENABLE_CODE_COVERAGE) message(FATAL_ERROR "ENABLE_CODE_COVERAGE: option is not supported on this platform") endif() - block() if (ENABLE_DEBUG EQUAL 2) elseif (NOT ENABLE_DEBUG) else() - set (is_debug 1) + set (ENABLE_STRICTLY_DEBUG_MODE 1) endif() - if (NOT is_debug) + if (NOT ENABLE_STRICTLY_DEBUG_MODE) message(FATAL_ERROR "ENABLE_CODE_COVERAGE: requires ENABLE_DEBUG or Debug build type") endif() - endblock() add_definitions(--coverage) link_libraries(--coverage) diff --git a/apps/srt-file-transmit.cpp b/apps/srt-file-transmit.cpp index 327ad68097..aa3696dc5f 100644 --- a/apps/srt-file-transmit.cpp +++ b/apps/srt-file-transmit.cpp @@ -465,6 +465,12 @@ bool DoDownload(UriParser& us, string directory, string filename, bool connected = false; int pollid = -1; string id; + // This will be set to TRUE if the ID has been obtained from the socket, + // while as caller socket it was set to this option beforehand. If it remains + // false, it means that it was the ID passed from the caller and extracted + // from the accepted socket - and as such the name can't be trusted, so + // if a file with this name exists, it will be not overwritten. + bool id_is_local = false; ofstream ofile; SRT_SOCKSTATUS status; SRTSOCKET efd; @@ -548,6 +554,7 @@ bool DoDownload(UriParser& us, string directory, string filename, cerr << "Source connected (caller), id [" << id << "]" << endl; connected = true; + id_is_local = true; } } break; @@ -579,10 +586,58 @@ bool DoDownload(UriParser& us, string directory, string filename, if (!ofile.is_open()) { - const char * fn = id.empty() ? filename.c_str() : id.c_str(); + std::string fn; + bool overwrite = false; + if (id.empty()) + { + fn = filename; + overwrite = true; + } + else + { + fn = id; + if (id_is_local) + overwrite = true; + } directory.append("/"); directory.append(fn); - ofile.open(directory.c_str(), ios::out | ios::trunc | ios::binary); + + std::ios::openmode flags = ios::out | ios::binary; + if (overwrite) + flags = flags | ios::trunc; + else + { + struct stat state; + int st = stat(directory.c_str(), &state); + if (st == 0) // File can be obtained + { + cerr << "Error: File exists: " << directory << endl; + cerr << "Error: As the name is remote-provided, overwriting denied for security reasons." << endl; + goto exit; + } + + // Additionally check if the path is PWD-based; + // reject any foreign-defined paths that are not + // effectively local. + + // NOTE: The file is always copied to the directory + // specified locally, with the original filename. Therefore + // it is not allowed that the file contain a path. + + static const size_t notfound = std::string::npos; + if ( fn.find('/') != notfound + || fn.find('\\') != notfound + || fn.find(':') != notfound + || fn.find("..") != notfound) // Any parent-referring + { + cerr << "Error: the foreign-specified path reaches outside PWD - REJECTED\n"; + cerr << "Path: " << directory << endl; + cerr << "NOTE: remote path is only allowed to point inside the current directory\n"; + goto exit; + } + } + + ofile.open(directory, flags); if (!ofile.is_open()) { diff --git a/scripts/codecov/update.sh b/scripts/codecov/update.sh index 1aadf9277c..2d2b8b62d0 100755 --- a/scripts/codecov/update.sh +++ b/scripts/codecov/update.sh @@ -1,5 +1,46 @@ #!/bin/bash HERE=`dirname $0` cd $HERE -curl -L -o codecov https://cli.codecov.io/latest/linux/codecov + +VERSION=hardcoded +if [[ -n $1 ]]; then + VERSION=$1 +fi + +if [[ $VERSION == latest ]]; then + BASEURL=https://cli.codecov.io/latest/linux/codecov + SHA= +else + BASEURL=https://cli.codecov.io/v11.3.1/linux/codecov + SHA="ca1d64196d2d34771084afe76ea657d581bf628e31d993ff8e52ea09cc88a56d codecov" + echo 'USING HARDCODED VERSION: 11.3.1. Use "update.sh latest" to get the latest version' + echo 'NOTE: Using the latest version is unsafe as SHA is also downloaded over the network' +fi + +echo "DOWNLOADING: $BASEURL" +rm -f codecov || { echo "Can't delete 'codecov'; please delete manually" ; exit 1; } + +curl -L -o codecov $BASEURL +FILESHA=$(sha256sum codecov) + +if [[ -z $SHA ]]; then + echo "DOWNLOADING HASH: ${BASEURL}.SHA256SUM" + curl -L -o codecov.SHA256SUM ${BASEURL}.SHA256SUM + SHA=$(cat codecov.SHA256SUM) + echo "Downloaded HASH: $SHA" +else + echo "Hardcoded HASH: $SHA" +fi + +if [[ "$FILESHA" == "$SHA" ]]; then + echo "Checksum SHA256 matches." +else + echo "ERROR: WRONG SHA256 CHECKSUM:" + echo "EXPECTED: $SHA" + echo "RECEIVED: $FILESHA" + rm codecov + echo "Corrupt file deleted" + exit 1 +fi + chmod +x codecov diff --git a/scripts/win-installer/ATTIC/old-install-nsis.ps1 b/scripts/win-installer/ATTIC/old-install-nsis.ps1 new file mode 100644 index 0000000000..0bf58afbc1 --- /dev/null +++ b/scripts/win-installer/ATTIC/old-install-nsis.ps1 @@ -0,0 +1,128 @@ +#----------------------------------------------------------------------------- +# +# SRT - Secure, Reliable, Transport +# Copyright (c) 2021, Thierry Lelegard +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +#----------------------------------------------------------------------------- + +<# + .DESCRIPTION + + Download, expand and install NSIS, the NullSoft Installer Scripting. + + IMPORTANT NOTE: This is the old version, left for historical reasons. Usage + of this script is discouraged because this scripts doesn't check the MD5 + checksum on the downloaded file and therefore is considered unsafe. + + .PARAMETER ForceDownload + + Force a download even if NSIS is already downloaded. + + .PARAMETER NoInstall + + Do not install the NSIS package. By default, NSIS is installed. + + .PARAMETER NoPause + + Do not wait for the user to press at end of execution. By default, + execute a "pause" instruction at the end of execution, which is useful + when the script was run from Windows Explorer. +#> +[CmdletBinding(SupportsShouldProcess=$true)] +param( + [switch]$ForceDownload = $false, + [switch]$NoInstall = $false, + [switch]$NoPause = $false +) + +Write-Output "NSIS download and installation procedure" +$NSISPage = "https://nsis.sourceforge.io/Download" +$FallbackURL = "http://prdownloads.sourceforge.net/nsis/nsis-3.05-setup.exe?download" + +# A function to exit this script. +function Exit-Script([string]$Message = "") +{ + $Code = 0 + if ($Message -ne "") { + Write-Output "ERROR: $Message" + $Code = 1 + } + if (-not $NoPause) { + pause + } + exit $Code +} + +# Local file names. +$RootDir = $PSScriptRoot +$TmpDir = "$RootDir\tmp" + +# Create the directory for external products when necessary. +[void] (New-Item -Path $TmpDir -ItemType Directory -Force) + +# Without this, Invoke-WebRequest is awfully slow. +$ProgressPreference = 'SilentlyContinue' + +# Get the HTML page for NSIS downloads. +$status = 0 +$message = "" +$Ref = $null +try { + $response = Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $NSISPage + $status = [int] [Math]::Floor($response.StatusCode / 100) +} +catch { + $message = $_.Exception.Message +} + +if ($status -ne 1 -and $status -ne 2) { + # Error fetch NSIS download page. + if ($message -eq "" -and (Test-Path variable:response)) { + Write-Output "Status code $($response.StatusCode), $($response.StatusDescription)" + } + else { + Write-Output "#### Error accessing ${NSISPage}: $message" + } +} +else { + # Parse HTML page to locate the latest installer. + $Ref = $response.Links.href | Where-Object { $_ -like "*/nsis-*-setup.exe?download" } | Select-Object -First 1 +} + +if (-not $Ref) { + # Could not find a reference to NSIS installer. + $Url = [System.Uri]$FallbackURL +} +else { + # Build the absolute URL's from base URL (the download page) and href links. + $Url = New-Object -TypeName 'System.Uri' -ArgumentList ([System.Uri]$NSISPage, $Ref) +} + +Write-Output "DOWNLOADING: $Url" + +$InstallerName = (Split-Path -Leaf $Url.LocalPath) +$InstallerPath = "$TmpDir\$InstallerName" + +# Download installer +if (-not $ForceDownload -and (Test-Path $InstallerPath)) { + Write-Output "$InstallerName already downloaded, use -ForceDownload to download again" +} +else { + Write-Output "Downloading $Url ..." + Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $Url -OutFile $InstallerPath + if (-not (Test-Path $InstallerPath)) { + Exit-Script "$Url download failed" + } +} + +# Install NSIS +if (-not $NoInstall) { + Write-Output "Installing $InstallerName" + Start-Process -FilePath $InstallerPath -ArgumentList @("/S") -Wait +} + +Exit-Script diff --git a/scripts/win-installer/build-win-installer.ps1 b/scripts/win-installer/build-win-installer.ps1 old mode 100644 new mode 100755 index c788e01abf..4778426ebb --- a/scripts/win-installer/build-win-installer.ps1 +++ b/scripts/win-installer/build-win-installer.ps1 @@ -1,4 +1,5 @@ -#----------------------------------------------------------------------------- +#!/usr/bin/env PowerShell +#----------------------------------------------------------------------------- # # SRT - Secure, Reliable, Transport # Copyright (c) 2021, Thierry Lelegard diff --git a/scripts/win-installer/install-libsrt.ps1 b/scripts/win-installer/install-libsrt.ps1 old mode 100644 new mode 100755 index ae1b8133e6..4aacb085c9 --- a/scripts/win-installer/install-libsrt.ps1 +++ b/scripts/win-installer/install-libsrt.ps1 @@ -1,4 +1,5 @@ -# SRT library download and install for Windows. +#!/usr/bin/env PowerShell +# SRT library download and install for Windows. # Copyright (c) 2021, Thierry Lelegard # All rights reserved. diff --git a/scripts/win-installer/install-nsis.ps1 b/scripts/win-installer/install-nsis.ps1 old mode 100644 new mode 100755 index 14879b5217..ea48d14ae3 --- a/scripts/win-installer/install-nsis.ps1 +++ b/scripts/win-installer/install-nsis.ps1 @@ -1,122 +1,163 @@ -#----------------------------------------------------------------------------- -# -# SRT - Secure, Reliable, Transport -# Copyright (c) 2021, Thierry Lelegard -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -#----------------------------------------------------------------------------- - -<# - .SYNOPSIS - - Download, expand and install NSIS, the NullSoft Installer Scripting. - - .PARAMETER ForceDownload - - Force a download even if NSIS is already downloaded. - - .PARAMETER NoInstall - - Do not install the NSIS package. By default, NSIS is installed. - - .PARAMETER NoPause - - Do not wait for the user to press at end of execution. By default, - execute a "pause" instruction at the end of execution, which is useful - when the script was run from Windows Explorer. -#> -[CmdletBinding(SupportsShouldProcess=$true)] -param( - [switch]$ForceDownload = $false, - [switch]$NoInstall = $false, - [switch]$NoPause = $false -) - -Write-Output "NSIS download and installation procedure" -$NSISPage = "https://nsis.sourceforge.io/Download" -$FallbackURL = "http://prdownloads.sourceforge.net/nsis/nsis-3.05-setup.exe?download" - -# A function to exit this script. -function Exit-Script([string]$Message = "") -{ - $Code = 0 - if ($Message -ne "") { - Write-Output "ERROR: $Message" - $Code = 1 - } - if (-not $NoPause) { - pause - } - exit $Code -} - -# Local file names. -$RootDir = $PSScriptRoot -$TmpDir = "$RootDir\tmp" - -# Create the directory for external products when necessary. -[void] (New-Item -Path $TmpDir -ItemType Directory -Force) - -# Without this, Invoke-WebRequest is awfully slow. -$ProgressPreference = 'SilentlyContinue' - -# Get the HTML page for NSIS downloads. -$status = 0 -$message = "" -$Ref = $null -try { - $response = Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $NSISPage - $status = [int] [Math]::Floor($response.StatusCode / 100) -} -catch { - $message = $_.Exception.Message -} - -if ($status -ne 1 -and $status -ne 2) { - # Error fetch NSIS download page. - if ($message -eq "" -and (Test-Path variable:response)) { - Write-Output "Status code $($response.StatusCode), $($response.StatusDescription)" - } - else { - Write-Output "#### Error accessing ${NSISPage}: $message" - } -} -else { - # Parse HTML page to locate the latest installer. - $Ref = $response.Links.href | Where-Object { $_ -like "*/nsis-*-setup.exe?download" } | Select-Object -First 1 -} - -if (-not $Ref) { - # Could not find a reference to NSIS installer. - $Url = [System.Uri]$FallbackURL -} -else { - # Build the absolute URL's from base URL (the download page) and href links. - $Url = New-Object -TypeName 'System.Uri' -ArgumentList ([System.Uri]$NSISPage, $Ref) -} - -$InstallerName = (Split-Path -Leaf $Url.LocalPath) -$InstallerPath = "$TmpDir\$InstallerName" - -# Download installer -if (-not $ForceDownload -and (Test-Path $InstallerPath)) { - Write-Output "$InstallerName already downloaded, use -ForceDownload to download again" -} -else { - Write-Output "Downloading $Url ..." - Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $Url -OutFile $InstallerPath - if (-not (Test-Path $InstallerPath)) { - Exit-Script "$Url download failed" - } -} - -# Install NSIS -if (-not $NoInstall) { - Write-Output "Installing $InstallerName" - Start-Process -FilePath $InstallerPath -ArgumentList @("/S") -Wait -} - -Exit-Script +#!/usr/bin/env PowerShell +#----------------------------------------------------------------------------- +# +# SRT - Secure, Reliable, Transport +# Copyright (c) 2021, Thierry Lelegard +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +#----------------------------------------------------------------------------- + +<# + .DESCRIPTION + + Download, expand and install NSIS, the NullSoft Installer Scripting. + + .PARAMETER ForceDownload + + Force a download even if NSIS is already downloaded. By default the + installer file is not redownloaded, if already found, although the + MD5 sum will still be checked. + + .PARAMETER NoInstall + + Do not install the NSIS package. By default, NSIS is installed. + + .PARAMETER NoPause + + Do not wait for the user to press at end of execution. By default, + execute a "pause" instruction at the end of execution, which is useful + when the script was run from Windows Explorer. +#> +[CmdletBinding(SupportsShouldProcess=$true)] +param( + [switch]$ForceDownload = $false, + [switch]$NoInstall = $false, + [switch]$NoPause = $false, + [switch]$Latest = $false +) + +# A function to exit this script. +function Exit-Script([string]$Message = "") +{ + $Code = 0 + if ($Message -ne "") { + Write-Output "ERROR: $Message" + $Code = 1 + } + if (-not $NoPause) { + pause + } + exit $Code +} + +# Local file names. +$RootDir = $PSScriptRoot +$TmpDir = "$RootDir\tmp" + +# Create the directory for external products when necessary. +[void] (New-Item -Path $TmpDir -ItemType Directory -Force) + +# Without this, Invoke-WebRequest is awfully slow. +$ProgressPreference = 'SilentlyContinue' + +# 1. Define Project URLs +$ProjectUrl = "https://sourceforge.net/projects/nsis/files/NSIS%203/" +$UserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64)" +$DownloadHead = "https://prdownloads.sourceforge.net/nsis/" + +# Latest download, on-demand only. Default is a hardcoded version. +if ($Latest) { + # 2. Find the Latest Version Folder + Write-Host "Checking for the latest NSIS version..." + $ProjectPage = Invoke-WebRequest -Uri $ProjectUrl -UserAgent $UserAgent -UseBasicParsing + # Matches version folders like "3.10", "3.12", etc. + $Versions = [regex]::matches($ProjectPage.Content, 'title="([\d\.]+)"') | + ForEach-Object { $_.Groups[1].Value } | + Sort-Object {[version]$_} -Descending + $LatestVer = $Versions[0] + Write-Host "Latest version found: $LatestVer" + + # 3. Build Folder and File Target URLs + $FolderUrl = "${ProjectUrl}${LatestVer}/" + $FolderPage = Invoke-WebRequest -Uri $FolderUrl -UserAgent $UserAgent -UseBasicParsing + + # Find the exact .exe installer filename (e.g., nsis-3.12-setup.exe) + $FileMatch = [regex]::match($FolderPage.Content, "nsis-${LatestVer}-setup\.exe") + if (-not $FileMatch.Success) { + throw "Could not find the setup.exe file for version $LatestVer" + } + $FileName = $FileMatch.Value + + # 4. Extract the Official MD5 Checksum + Write-Host "Extracting official MD5 checksum..." + # SourceForge stores file metadata in a JSON-like 'data-files' attribute or specific table rows + # This regex extracts the MD5 hash associated directly with the target filename + #$HashPattern = 'tr[^>]*?data-name="' + [regex]::Escape($FileName) + '"[^>]*?md5">([^<]+)' + #$MD5Match = [regex]::match($FolderPage.Content, $HashPattern) + + $filedata_inpage = [regex]::match($FolderPage.Content, "net.sf.files = ([^;]+);") + if (-not $filedata_inpage.Success) { + Exit-Script "net.sf.files not found" + } + + $filedata_json = $filedata_inpage.Groups[1].Value.Trim().ToLower() + + $filedata = ConvertFrom-Json $filedata_json + + $ExpectedMD5 = $filedata.$FileName.md5 + + if ($ExpectedMD5 -eq $null) { + Exit-Script "MD5 not found for $FileName" + } + + Write-Host "MD5: $ExpectedMD5" +} else { + Write-Host "USING PREDEFINED VERSION with hardcoded MD5: 3.12" + Write-Host "Use -Latest to force latest version; note that this can be prone to MITM attacks." + + $ExpectedMD5 = "d5d54c2a96c1bcb25764adc9f9ff97f2" + $FileName = "nsis-3.12-setup.exe" +} + +# 5. Download the File +$DownloadUrl = "$DownloadHead$FileName" + "?download" +$OutputPath = "$TmpDir\$FileName" + +Write-Host "Installer path: $OutputPath" + +if (-not $ForceDownload -and (Test-Path $OutputPath)) { + Write-Host "Already downloaded, use -ForceDownload to download anyway." +} else { + Write-Host "Downloading from: $DownloadUrl" + Invoke-WebRequest -Uri $DownloadUrl -UserAgent Download -UseBasicParsing -OutFile $OutputPath +} + +# 6. Verify the MD5 Checksum +Write-Host "Verifying checksum..." +$LocalMD5 = (Get-FileHash -Path $OutputPath -Algorithm MD5).Hash.ToLower() + +if ($LocalMD5 -eq $ExpectedMD5) { + Write-Host "Checksum OK." -ForegroundColor Green +} else { + Write-Error "ERROR: Checksum mismatch!" + Write-Host "Expected: $ExpectedMD5" -ForegroundColor Red + Write-Host "Got: $LocalMD5" -ForegroundColor Red + Remove-Item -Path $OutputPath -Force + Write-Host "Corrupted file removed." + Exit-Script "Installation not possible" +} + +if (-not $NoInstall) { + Write-Host "Installing $FileName" + Start-Process -FilePath $OutputPath -ArgumentList @("/S") -Wait +} else { + Write-Host "Installation not requested." +} + +Exit-Script + + + diff --git a/scripts/win-installer/install-openssl.ps1 b/scripts/win-installer/install-openssl.ps1 old mode 100644 new mode 100755 index c9ec580683..d0a95fb94b --- a/scripts/win-installer/install-openssl.ps1 +++ b/scripts/win-installer/install-openssl.ps1 @@ -1,4 +1,5 @@ -#----------------------------------------------------------------------------- +#!/usr/bin/env PowerShell +#----------------------------------------------------------------------------- # # SRT - Secure, Reliable, Transport # Copyright (c) 2021-2024, Thierry Lelegard @@ -32,7 +33,8 @@ param( [switch]$ForceDownload = $false, [switch]$NoInstall = $false, - [switch]$NoPause = $false + [switch]$NoPause = $false, + [switch]$Latest = $false ) Write-Output "OpenSSL download and installation procedure" @@ -64,36 +66,55 @@ $TmpDir = "$RootDir\tmp" # Without this, Invoke-WebRequest is awfully slow. $ProgressPreference = 'SilentlyContinue' -# Get the JSON configuration file for OpenSSL downloads. -$status = 0 -$message = "" -try { - $response = Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $PackageList - $status = [int] [Math]::Floor($response.StatusCode / 100) -} -catch { - $message = $_.Exception.Message -} -if ($status -ne 1 -and $status -ne 2) { - if ($message -eq "" -and (Test-Path variable:response)) { - Exit-Script "Status code $($response.StatusCode), $($response.StatusDescription)" - } - else { - Exit-Script "#### Error accessing ${PackageList}: $message" - } -} -$config = ConvertFrom-Json $Response.Content - -# Find the URL of the latest "universal" installer in the JSON config file. -$Url = $config.files | Get-Member | ForEach-Object { - $name = $_.name - $info = $config.files.$($_.name) - if (-not $info.light -and $info.installer -like "exe" -and $info.arch -like "universal") { - $info.url - } -} | Select-Object -Last 1 -if (-not $Url) { - Exit-Script "#### No universal installer found" +if ($Latest) { + + # Get the JSON configuration file for OpenSSL downloads. + $status = 0 + $message = "" + try { + Write-Output "Getting file index: $PackageList" + $response = Invoke-WebRequest -UseBasicParsing -UserAgent Download -Uri $PackageList + $status = [int] [Math]::Floor($response.StatusCode / 100) + } + catch { + $message = $_.Exception.Message + } + if ($status -ne 1 -and $status -ne 2) { + if ($message -eq "" -and (Test-Path variable:response)) { + Exit-Script "Status code $($response.StatusCode), $($response.StatusDescription)" + } + else { + Exit-Script "#### Error accessing ${PackageList}: $message" + } + } + $config = ConvertFrom-Json $Response.Content + + Write-Output "Getting installer file with .light and .installer =~ 'exe' and .arch =~ 'Universal'" + + # Find the URL of the latest "universal" installer in the JSON config file. + #$Url = + $have = 0 + $config.files | Get-Member | ForEach-Object { + $name = $_.name + $info = $config.files.$($_.name) + if (-not $info.light -and $info.installer -like "exe" -and $info.arch -like "universal") { + $found_info = $info + $have = 1 + } + } # | Select-Object -Last 1 + if (-not $have) { + Exit-Script "#### No universal installer found" + } + + $Url = $found_info.url + $xsum = $found_info.md5 +} else { + + Write-Host "USING PREDEFINED VERSION with hardcoded MD5: 4.0.1" + Write-Host "Use -Latest to force latest version; note that this can be prone to MITM attacks." + + $Url = "https://slproweb.com/download/WinUniversalOpenSSL-4_0_1.exe" + $xsum = "34051060e0e9f48e63cfc1f3356191f6" } $ExeName = (Split-Path -Leaf $Url) @@ -111,9 +132,20 @@ if (-not (Test-Path $ExePath)) { Exit-Script "$Url download failed" } +Write-Output "Will check tmp\$ExeName for MD5: $xsum ..." + +$filehash = get-filehash $ExePath -algorithm md5 + +if ($filehash.Hash -ne $xsum) { + Exit-Script "$ExePath MD5: $filehash - NOT MATCHING" +} + if (-not $NoInstall) { - Write-Output "Installing $ExeName" + Write-Output "CHECKSUM MATCHES. Installing $ExeName" Start-Process -FilePath $ExePath -ArgumentList @("/VERYSILENT", "/SUPPRESSMSGBOXES", "/NORESTART", "/ALLUSERS") -Wait +} else { + Write-Output "Installation not requested." } + Exit-Script diff --git a/srtcore/api.cpp b/srtcore/api.cpp index 7b57095717..c01eaa28c1 100644 --- a/srtcore/api.cpp +++ b/srtcore/api.cpp @@ -2834,36 +2834,66 @@ void srt::CUDTUnited::checkBrokenSockets() vector tbc; vector tbr; + bool forced_closing = m_bClosing; + for (sockets_t::iterator i = m_Sockets.begin(); i != m_Sockets.end(); ++i) { CUDTSocket* s = i->second; if (!s->core().m_bBroken) - continue; + { + if (!forced_closing) + { + continue; + } + else + { + // Set forcefully, we are in cleanup and close everything + LOGC(smlog.Warn, log << "CLEANUP: Forcefully breaking socket @" << s->m_SocketID); + s->core().m_bBroken = true; + } + } if (s->m_Status == SRTS_LISTENING) { - const steady_clock::duration elapsed = steady_clock::now() - s->m_tsClosureTimeStamp.load(); - // A listening socket should wait an extra 3 seconds - // in case a client is connecting. - if (elapsed < milliseconds_from(CUDT::COMM_CLOSE_BROKEN_LISTENER_TIMEOUT_MS)) - continue; + if (!forced_closing) + { + const steady_clock::duration elapsed = steady_clock::now() - s->m_tsClosureTimeStamp.load(); + // A listening socket should wait an extra 3 seconds + // in case a client is connecting. + if (elapsed < milliseconds_from(CUDT::COMM_CLOSE_BROKEN_LISTENER_TIMEOUT_MS)) + continue; + } } else { CUDT& u = s->core(); - enterCS(u.m_RcvBufferLock); - bool has_avail_packets = u.m_pRcvBuffer && u.m_pRcvBuffer->hasAvailablePackets(); - leaveCS(u.m_RcvBufferLock); + // For decent closing, just keep it as long as it still + // has data in the buffer. + if (!forced_closing) + { + enterCS(u.m_RcvBufferLock); + bool has_avail_packets = u.m_pRcvBuffer && u.m_pRcvBuffer->hasAvailablePackets(); + leaveCS(u.m_RcvBufferLock); - if (has_avail_packets) + if (has_avail_packets) + { + const int bc = u.m_iBrokenCounter.load(); + if (bc > 0) + { + // if there is still data in the receiver buffer, wait longer + s->core().m_iBrokenCounter.store(bc - 1); + continue; + } + } + } + else { - const int bc = u.m_iBrokenCounter.load(); - if (bc > 0) + // Forced closing: any data still in the buffer - delete them. + ScopedLock cgb (u.m_RcvBufferLock); + if (u.m_pRcvBuffer && u.m_pRcvBuffer->hasAvailablePackets()) { - // if there is still data in the receiver buffer, wait longer - s->core().m_iBrokenCounter.store(bc - 1); - continue; + u.m_pRcvBuffer->dropAll(); } } } @@ -2890,7 +2920,8 @@ void srt::CUDTUnited::checkBrokenSockets() { ls = m_ClosedSockets.find(s->m_ListenSocket); if (ls == m_ClosedSockets.end()) - continue; + continue; // END LOOP AS NOT FOUND + // OTHERWISE PROCEED with erasing as queued } enterCS(ls->second->m_AcceptLock); @@ -2911,6 +2942,9 @@ void srt::CUDTUnited::checkBrokenSockets() // other conditions applying on the socket that prevent it from being deleted. if (ps->isStillBusy()) { + // NOTE: you can't use forced_closing to prevent it because isStillBusy + // means that some facility has acquired it and is going to use it for + // operations; forced deletion may lead to UB/crash. HLOGC(smlog.Debug, log << "checkBrokenSockets: @" << ps->m_SocketID << " is still busy, SKIPPING THIS CYCLE."); continue; } @@ -3437,7 +3471,11 @@ void srt::CUDTUnited::updateMux(CUDTSocket* s, const sockaddr_any& reqaddr, cons m.m_pSndQueue = new CSndQueue; m.m_pSndQueue->init(m.m_pChannel, m.m_pTimer); m.m_pRcvQueue = new CRcvQueue; - m.m_pRcvQueue->init(128, s->core().maxPayloadSize(), m.m_iIPversion, 1024, m.m_pChannel, m.m_pTimer); + + // NOTE: Receiver Queue packet size must be of the maximum possible because you never + // know what kind of packet will come over the network, while this must accept any kind + // of packet. + m.m_pRcvQueue->init(128, s->core().controlPayloadSize(reqaddr.family()), m.m_iIPversion, 1024, m.m_pChannel, m.m_pTimer); // Rewrite the port here, as it might be only known upon return // from CChannel::open. @@ -3558,11 +3596,30 @@ void* srt::CUDTUnited::garbageCollect(void* p) UniqueLock gclock(self->m_GCStopLock); - while (!self->m_bClosing) + for (;;) { INCREMENT_THREAD_ITERATIONS(); self->checkBrokenSockets(); + if (self->m_bClosing) + { + // If GC is requested to close, it means the global cleanup + // was requested. But before exiting make sure all sockets + // and multiplexers are closed. + + { + SharedLock glock(self->m_GlobControlLock); + if (self->m_Sockets.empty() && self->m_ClosedSockets.empty()) + break; + + HLOGC(smlog.Debug, log << "GC: REQUESTED CLOSE, DELAYING EXIT - still " + << self->m_Sockets.size() << " running and " + << self->m_ClosedSockets.size() << " closed sockets"); + } + self->m_GCStopCond.wait_for(gclock, milliseconds_from(200)); + continue; + } + HLOGC(inlog.Debug, log << "GC: sleep 1 s"); self->m_GCStopCond.wait_for(gclock, seconds_from(1)); } diff --git a/srtcore/buffer_snd.cpp b/srtcore/buffer_snd.cpp index 2db82a686b..7b8c44f999 100644 --- a/srtcore/buffer_snd.cpp +++ b/srtcore/buffer_snd.cpp @@ -550,6 +550,13 @@ void CSndBuffer::ackData(int offset) { ScopedLock bufferguard(m_BufLock); + if (offset > m_iCount) + { + LOGC(bslog.Warn, log << "ackData: offset=" << offset << " > count=" << m_iCount + << " - adjusting"); + offset = m_iCount; + } + bool move = false; for (int i = 0; i < offset; ++i) { diff --git a/srtcore/common.h b/srtcore/common.h index 5a53c05e60..3d9bf65f19 100644 --- a/srtcore/common.h +++ b/srtcore/common.h @@ -95,6 +95,12 @@ modified by #define SRT_STATIC_ASSERT(cond, msg) #endif +#if HAVE_FULL_CXX11 +#define FUNID() __func__ +#else +#define FUNID() __FUNCTION__ +#endif + #include namespace srt_logging diff --git a/srtcore/congctl.cpp b/srtcore/congctl.cpp index 9bc43db8b7..8de14d8931 100644 --- a/srtcore/congctl.cpp +++ b/srtcore/congctl.cpp @@ -79,10 +79,10 @@ class LiveCC: public SrtCongestionControlBase m_llSndMaxBW = BW_INFINITE; // 1 Gbbps in Bytes/sec BW_INFINITE m_zMaxPayloadSize = parent->OPT_PayloadSize(); if (m_zMaxPayloadSize == 0) - m_zMaxPayloadSize = parent->maxPayloadSize(); + m_zMaxPayloadSize = parent->maxDataPayloadSize(); m_zSndAvgPayloadSize = m_zMaxPayloadSize; - m_zHeaderSize = parent->m_config.iMSS - parent->maxPayloadSize(); + m_zHeaderSize = parent->m_config.iMSS - parent->maxDataPayloadSize(); m_iMinNakInterval_us = 20000; //Minimum NAK Report Period (usec) m_iNakReportAccel = 2; //Default NAK Report Period (RTT) accelerator (send periodic NAK every RTT/2) @@ -319,7 +319,7 @@ class FileCC : public SrtCongestionControlBase /// and request ACK to be sent immediately. bool needsQuickACK(const CPacket& pkt) ATR_OVERRIDE { - if (pkt.getLength() < m_parent->maxPayloadSize()) + if (pkt.getLength() < m_parent->maxDataPayloadSize()) { // This is not a regular fixed size packet... // an irregular sized packet usually indicates the end of a message, so send an ACK immediately diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 1e98f75a20..9379da5464 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -1042,17 +1042,17 @@ string srt::CUDT::getstreamid(SRTSOCKET u) void srt::CUDT::clearData() { const size_t full_hdr_size = CPacket::UDP_HDR_SIZE - CPacket::HDR_SIZE; - m_iMaxSRTPayloadSize = m_config.iMSS - full_hdr_size; - HLOGC(cnlog.Debug, log << CONID() << "clearData: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize); + m_iMaxDataPayloadSize = m_config.iMSS - full_hdr_size; + HLOGC(cnlog.Debug, log << CONID() << "clearData: PAYLOAD SIZE: " << m_iMaxDataPayloadSize); - m_SndTimeWindow.initialize(full_hdr_size, m_iMaxSRTPayloadSize); - m_RcvTimeWindow.initialize(full_hdr_size, m_iMaxSRTPayloadSize); + m_SndTimeWindow.initialize(full_hdr_size, m_iMaxDataPayloadSize); + m_RcvTimeWindow.initialize(full_hdr_size, m_iMaxDataPayloadSize); m_iEXPCount = 1; m_iBandwidth = 1; // pkts/sec // XXX use some constant for this 16 m_iDeliveryRate = 16; - m_iByteDeliveryRate = 16 * m_iMaxSRTPayloadSize; + m_iByteDeliveryRate = 16 * m_iMaxDataPayloadSize; m_iAckSeqNo = 0; m_tsLastAckTime = steady_clock::now(); @@ -1822,7 +1822,7 @@ bool srt::CUDT::createSrtHandshake( // Now use the original function to store the actual SRT_HS data // ra_size after that - // NOTE: so far, ra_size is m_iMaxSRTPayloadSize expressed in number of elements. + // NOTE: so far, ra_size is controlPayloadSize() expressed in number of elements. // WILL BE CHANGED HERE. ra_size = fillSrtHandshake((p + offset), total_ra_size - offset, srths_cmd, HS_VERSION_SRT1); *pcmdspec = HS_CMDSPEC_CMD::wrap(srths_cmd) | HS_CMDSPEC_SIZE::wrap((uint32_t) ra_size); @@ -1837,7 +1837,7 @@ bool srt::CUDT::createSrtHandshake( // Now prepare the string with 4-byte alignment. The string size is limited // to half the payload size. Just a sanity check to not pack too much into // the conclusion packet. - size_t size_limit = m_iMaxSRTPayloadSize / 2; + size_t size_limit = controlPayloadSize() / 2; if (m_config.sStreamName.size() >= size_limit) { @@ -2223,7 +2223,7 @@ bool srt::CUDT::processSrtMsg(const CPacket *ctrlpkt) case SRT_CMD_KMRSP: { // KMRSP doesn't expect any following action - m_pCryptoControl->processSrtMsg_KMRSP(srtdata, len, m_uPeerSrtVersion); + m_pCryptoControl->processSrtMsg_KMRSP(srtdata, len, m_uPeerSrtVersion, false); return true; // nothing to do } @@ -2628,7 +2628,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, m_RejectReason = SRT_REJ_VERSION; // This means that a version with minimum 1.3.0 that features HSv5 is required, // hence all HSv4 clients should be rejected. - LOGP(cnlog.Error, "interpretSrtHandshake: minimum peer version 1.3.0 (HSv5 only), rejecting HSv4 client"); + LOGP(cnlog.Error, string(FUNID()) + ": minimum peer version 1.3.0 (HSv5 only), rejecting HSv4 client"); return false; } return true; // do nothing @@ -2666,7 +2666,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (IsSet(ext_flags, CHandShake::HS_EXT_HSREQ)) { - HLOGC(cnlog.Debug, log << CONID() << "interpretSrtHandshake: extracting HSREQ/RSP type extension"); + HLOGC(cnlog.Debug, log << CONID() << FUNID() << ": extracting HSREQ/RSP type extension"); uint32_t *begin = p; uint32_t *next = 0; size_t length = size / sizeof(uint32_t); @@ -2698,7 +2698,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, { // m_RejectReason already set LOGC(cnlog.Error, - log << CONID() << "interpretSrtHandshake: process HSREQ returned unexpected value " << rescmd); + log << CONID() << FUNID() << ": process HSREQ returned unexpected value " << rescmd); return false; } handshakeDone(); @@ -2729,7 +2729,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (m_RejectReason == SRT_REJ_UNKNOWN) m_RejectReason = SRT_REJ_ROGUE; LOGC(cnlog.Error, - log << CONID() << "interpretSrtHandshake: process HSRSP returned unexpected value " << rescmd); + log << CONID() << FUNID() << ": process HSRSP returned unexpected value " << rescmd); return false; } handshakeDone(); @@ -2739,7 +2739,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, { m_RejectReason = SRT_REJ_ROGUE; LOGC(cnlog.Warn, - log << CONID() << "interpretSrtHandshake: no HSREQ/HSRSP block found in the handshake msg!"); + log << CONID() << FUNID() << ": no HSREQ/HSRSP block found in the handshake msg!"); // This means that there can be no more processing done by FindExtensionBlock(). // And we haven't found what we need - otherwise one of the above cases would pass // and lead to exit this loop immediately. @@ -2758,7 +2758,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, } } - HLOGC(cnlog.Debug, log << CONID() << "interpretSrtHandshake: HSREQ done, checking KMREQ"); + HLOGC(cnlog.Debug, log << CONID() << FUNID() << ": HSREQ done, checking KMREQ"); // Now check the encrypted @@ -2766,7 +2766,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (IsSet(ext_flags, CHandShake::HS_EXT_KMREQ)) { - HLOGC(cnlog.Debug, log << CONID() << "interpretSrtHandshake: extracting KMREQ/RSP type extension"); + HLOGC(cnlog.Debug, log << CONID() << FUNID() << ": extracting KMREQ/RSP type extension"); #ifdef SRT_ENABLE_ENCRYPTION if (!m_pCryptoControl->hasPassphrase()) @@ -2799,7 +2799,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, int cmd = FindExtensionBlock(begin, length, (blocklen), (next)); HLOGC(cnlog.Debug, - log << CONID() << "interpretSrtHandshake: found extension: (" << cmd << ") " + log << CONID() << FUNID() << ": found extension: (" << cmd << ") " << MessageTypeStr(UMSG_EXT, cmd)); size_t bytelen = blocklen * sizeof(uint32_t); @@ -2819,7 +2819,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, m_RejectReason = SRT_REJ_IPE; // Something went wrong. HLOGC(cnlog.Debug, - log << CONID() << "interpretSrtHandshake: IPE/EPE KMREQ processing failed - returned " + log << CONID() << FUNID() << ": IPE/EPE KMREQ processing failed - returned " << res); return false; } @@ -2832,7 +2832,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, m_RejectReason = SRT_REJ_CRYPTO; LOGC(cnlog.Error, log << CONID() - << "interpretSrtHandshake: KMREQ result: Bad crypto mode - rejecting"); + << FUNID() << ": KMREQ result: Bad crypto mode - rejecting"); return false; } #endif @@ -2851,7 +2851,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, } LOGC(cnlog.Error, log << CONID() - << "interpretSrtHandshake: KMREQ result abnornal - rejecting per enforced encryption"); + << FUNID() << ": KMREQ result abnornal - rejecting per enforced encryption"); return false; } } @@ -2859,7 +2859,12 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, } else if (cmd == SRT_CMD_KMRSP) { - int res = m_pCryptoControl->processSrtMsg_KMRSP(begin + 1, bytelen, m_uPeerSrtVersion); + // Normally this is a handshake, but this one can be also called through + // the in-connected dispatcher and processCtrlHS(). The is_handshake can be + // still potentially set back to true inside when the KMX was detected as + // not done for the sake of HSv4. + bool is_handshake = m_parent->m_Status != SRTS_CONNECTED; + int res = m_pCryptoControl->processSrtMsg_KMRSP(begin + 1, bytelen, m_uPeerSrtVersion, is_handshake); if (m_config.bEnforcedEnc && res == -1) { if (m_pCryptoControl->m_SndKmState == SRT_KM_S_BADSECRET) @@ -2884,7 +2889,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, } else { - HLOGC(cnlog.Debug, log << CONID() << "interpretSrtHandshake: ... skipping " << MessageTypeStr(UMSG_EXT, cmd)); + HLOGC(cnlog.Debug, log << CONID() << FUNID() << ": ... skipping " << MessageTypeStr(UMSG_EXT, cmd)); if (NextExtensionBlock((begin), next, (length))) continue; } @@ -2926,7 +2931,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (IsSet(ext_flags, CHandShake::HS_EXT_CONFIG)) { - HLOGC(cnlog.Debug, log << CONID() << "interpretSrtHandshake: extracting various CONFIG extensions"); + HLOGC(cnlog.Debug, log << CONID() << FUNID() << ": extracting various CONFIG extensions"); uint32_t *begin = p; uint32_t *next = 0; @@ -2938,7 +2943,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, int cmd = FindExtensionBlock(begin, length, (blocklen), (next)); HLOGC(cnlog.Debug, - log << CONID() << "interpretSrtHandshake: found extension: (" << cmd << ") " + log << CONID() << FUNID() << ": found extension: (" << cmd << ") " << MessageTypeStr(UMSG_EXT, cmd)); const size_t bytelen = blocklen * sizeof(uint32_t); @@ -2947,7 +2952,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (!bytelen || bytelen > CSrtConfig::MAX_SID_LENGTH) { LOGC(cnlog.Error, - log << CONID() << "interpretSrtHandshake: STREAMID length " << bytelen << " is 0 or > " + log << CONID() << FUNID() << ": STREAMID length " << bytelen << " is 0 or > " << +CSrtConfig::MAX_SID_LENGTH << " - PROTOCOL ERROR, REJECTING"); return false; } @@ -2997,7 +3002,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (!bytelen || bytelen > CSrtConfig::MAX_CONG_LENGTH) { LOGC(cnlog.Error, - log << CONID() << "interpretSrtHandshake: CONGESTION-control type length " << bytelen + log << CONID() << FUNID() << ": CONGESTION-control type length " << bytelen << " is 0 or > " << +CSrtConfig::MAX_CONG_LENGTH << " - PROTOCOL ERROR, REJECTING"); return false; } @@ -3040,7 +3045,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, if (!bytelen || bytelen > CSrtConfig::MAX_PFILTER_LENGTH) { LOGC(cnlog.Error, - log << CONID() << "interpretSrtHandshake: packet-filter type length " << bytelen + log << CONID() << FUNID() << ": packet-filter type length " << bytelen << " is 0 or > " << +CSrtConfig::MAX_PFILTER_LENGTH << " - PROTOCOL ERROR, REJECTING"); return false; } @@ -3106,7 +3111,7 @@ bool srt::CUDT::interpretSrtHandshake(const CHandShake& hs, { // Found some block that is not interesting here. Skip this and get the next one. HLOGC(cnlog.Debug, - log << CONID() << "interpretSrtHandshake: ... skipping " << MessageTypeStr(UMSG_EXT, cmd)); + log << CONID() << FUNID() << ": ... skipping " << MessageTypeStr(UMSG_EXT, cmd)); } if (!NextExtensionBlock((begin), next, (length))) @@ -3792,7 +3797,8 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) // Inform the server my configurations. CPacket reqpkt; reqpkt.setControl(UMSG_HANDSHAKE); - reqpkt.allocate(m_iMaxSRTPayloadSize); + size_t hs_size = controlPayloadSize(serv_addr.family()); + reqpkt.allocate(hs_size); // XXX NOTE: Now the memory for the payload part is allocated automatically, // and such allocated memory is also automatically deallocated in the // destructor. If you use CPacket::allocate, remember that you must not: @@ -3807,7 +3813,6 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) // ID = 0, connection request reqpkt.set_id(0); - size_t hs_size = m_iMaxSRTPayloadSize; m_ConnReq.store_to((reqpkt.m_pcData), (hs_size)); // Note that CPacket::allocate() sets also the size @@ -3864,7 +3869,7 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) // next incoming packet. CPacket response; response.setControl(UMSG_HANDSHAKE); - response.allocate(m_iMaxSRTPayloadSize); + response.allocate(controlPayloadSize(serv_addr.family())); CUDTException e; EConnectStatus cst = CONN_CONTINUE; @@ -3916,7 +3921,7 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) } cst = CONN_CONTINUE; - response.setLength(m_iMaxSRTPayloadSize); + response.setLength(controlPayloadSize(serv_addr.family())); if (m_pRcvQueue->recvfrom(m_SocketID, (response)) > 0) { use_source_adr = response.udpDestAddr(); @@ -3969,7 +3974,7 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) m_RejectReason = SRT_REJ_ROGUE; // rejection or erroneous code. - reqpkt.setLength(m_iMaxSRTPayloadSize); + reqpkt.setLength(controlPayloadSize(serv_addr.family())); reqpkt.setControl(UMSG_HANDSHAKE); sendRendezvousRejection(serv_addr, (reqpkt)); } @@ -3997,12 +4002,12 @@ void srt::CUDT::startConnect(const sockaddr_any& serv_addr, int32_t forced_isn) // Now serialize the handshake again to the existing buffer so that it's // then sent later in this loop. - // First, set the size back to the original size, m_iMaxSRTPayloadSize because + // First, set the size back to the original size, controlPayloadSize() because // this is the size of the originally allocated space. It might have been // shrunk by serializing the INDUCTION handshake (which was required before // sending this packet to the output queue) and therefore be too // small to store the CONCLUSION handshake (with HSv5 extensions). - reqpkt.setLength(m_iMaxSRTPayloadSize); + reqpkt.setLength(controlPayloadSize(serv_addr.family())); HLOGC(cnlog.Debug, log << CONID() << "startConnect: creating HS CONCLUSION: buffer size=" << reqpkt.getLength()); @@ -4132,7 +4137,7 @@ bool srt::CUDT::processAsyncConnectRequest(EReadStatus rst, CPacket reqpkt; reqpkt.setControl(UMSG_HANDSHAKE); - reqpkt.allocate(m_iMaxSRTPayloadSize); + reqpkt.allocate(controlPayloadSize(serv_addr.family())); const steady_clock::time_point now = steady_clock::now(); setPacketTS(reqpkt, now); @@ -4450,7 +4455,7 @@ EConnectStatus srt::CUDT::processRendezvous( m_ConnReq.m_iReqType = rsp_type; m_ConnReq.m_extension = needs_extension; - // This must be done before prepareConnectionObjects(), because it sets ISN and m_iMaxSRTPayloadSize needed to create buffers. + // This must be done before prepareConnectionObjects(), because it sets ISN needed to create buffers. if (!applyResponseSettings(pResponse)) { LOGC(cnlog.Error, log << CONID() << "processRendezvous: peer settings rejected"); @@ -4514,7 +4519,7 @@ EConnectStatus srt::CUDT::processRendezvous( log << CONID() << "processRendezvous: HSREQ extension ok, creating HSRSP response. kmdatasize=" << kmdatasize); - w_reqpkt.setLength(m_iMaxSRTPayloadSize); + w_reqpkt.setLength(controlPayloadSize(serv_addr.family())); if (!createSrtHandshake(SRT_CMD_HSRSP, SRT_CMD_KMRSP, kmdata, kmdatasize, (w_reqpkt), (m_ConnReq))) @@ -4591,7 +4596,7 @@ EConnectStatus srt::CUDT::processRendezvous( // serialization. m_ConnReq.m_extension = needs_extension; - w_reqpkt.setLength(m_iMaxSRTPayloadSize); + w_reqpkt.setLength(controlPayloadSize(serv_addr.family())); if (m_RdvState == CHandShake::RDV_CONNECTED) { int cst = postConnect(pResponse, true, 0); @@ -4934,6 +4939,24 @@ EConnectStatus srt::CUDT::processConnectResponse(const CPacket& response, CUDTEx return postConnect(&response, false, eout); } +static size_t MinimumMSS(int family) +{ + const size_t full_hdr_size = CPacket::UDP_HDR_SIZE + CPacket::HDR_SIZE; + const size_t full_hdr_size_i6 = CPacket::UDP_HDR_SIZE_IPv6 + CPacket::HDR_SIZE; + + size_t min_mss_size = 4; // initial: required for passing any data + if (family == AF_INET6) + { + min_mss_size += full_hdr_size_i6; + } + else + { + min_mss_size += full_hdr_size; + } + + return min_mss_size; +} + bool srt::CUDT::applyResponseSettings(const CPacket* pHspkt /*[[nullable]]*/) ATR_NOEXCEPT { if (!m_ConnRes.valid()) @@ -4947,12 +4970,10 @@ bool srt::CUDT::applyResponseSettings(const CPacket* pHspkt /*[[nullable]]*/) AT m_config.iMSS = m_ConnRes.m_iMSS; const size_t full_hdr_size = CPacket::UDP_HDR_SIZE + CPacket::HDR_SIZE; - m_iMaxSRTPayloadSize = m_config.iMSS - full_hdr_size; - HLOGC(cnlog.Debug, log << CONID() << "applyResponseSettings: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize); + m_iMaxDataPayloadSize = m_config.iMSS - full_hdr_size; + HLOGC(cnlog.Debug, log << CONID() << "applyResponseSettings: PAYLOAD SIZE: " << m_iMaxDataPayloadSize); m_iFlowWindowSize = m_ConnRes.m_iFlightFlagSize; - const int udpsize = m_config.iMSS - CPacket::UDP_HDR_SIZE; - m_iMaxSRTPayloadSize = udpsize - CPacket::HDR_SIZE; m_iPeerISN = m_ConnRes.m_iISN; setInitialRcvSeq(m_iPeerISN); @@ -4964,7 +4985,7 @@ bool srt::CUDT::applyResponseSettings(const CPacket* pHspkt /*[[nullable]]*/) AT m_SourceAddr = pHspkt->udpDestAddr(); HLOGC(cnlog.Debug, - log << CONID() << "applyResponseSettings: HANDSHAKE CONCLUDED. SETTING: payload-size=" << m_iMaxSRTPayloadSize + log << CONID() << "applyResponseSettings: HANDSHAKE CONCLUDED. SETTING: payload-size=" << m_iMaxDataPayloadSize << " mss=" << m_ConnRes.m_iMSS << " flw=" << m_ConnRes.m_iFlightFlagSize << " peer-ISN=" << m_ConnRes.m_iISN << " local-ISN=" << m_iISN << " peerID=" << m_ConnRes.m_iID @@ -5943,16 +5964,17 @@ bool srt::CUDT::prepareBuffers(CUDTException* eout) try { - // CryptoControl has to be initialized and in case of RESPONDER the KM REQ must be processed (interpretSrtHandshake(..)) for the crypto mode to be deduced. + // CryptoControl has to be initialized and in case of RESPONDER + // the KM REQ must be processed (interpretSrtHandshake(..)) for the crypto mode to be deduced. const int authtag = getAuthTagSize(); - SRT_ASSERT(m_iMaxSRTPayloadSize != 0); + SRT_ASSERT(m_iMaxDataPayloadSize != 0); - HLOGC(rslog.Debug, log << CONID() << "Creating buffers: snd-plsize=" << m_iMaxSRTPayloadSize + HLOGC(rslog.Debug, log << CONID() << "Creating buffers: snd-plsize=" << m_iMaxDataPayloadSize << " snd-bufsize=" << 32 << " authtag=" << authtag); - m_pSndBuffer = new CSndBuffer(AF_INET, 32, m_iMaxSRTPayloadSize, authtag); + m_pSndBuffer = new CSndBuffer(AF_INET, 32, m_iMaxDataPayloadSize, authtag); SRT_ASSERT(m_iPeerISN != -1); m_pRcvBuffer = new srt::CRcvBuffer(m_iPeerISN, m_config.iRcvBufSize, m_pRcvQueue->m_pUnitQueue, m_config.bMessageAPI); // After introducing lite ACK, the sndlosslist may not be cleared in time, so it requires twice a space. @@ -5997,13 +6019,23 @@ void srt::CUDT::acceptAndRespond(const sockaddr_any& agent, const sockaddr_any& m_tsRcvPeerStartTime = steady_clock::time_point(); // will be set correctly at SRT HS + const size_t full_hdr_size = CPacket::UDP_HDR_SIZE + CPacket::HDR_SIZE; // Uses the smaller MSS between the peers - m_config.iMSS = std::min(m_config.iMSS, w_hs.m_iMSS); + size_t mss_size = std::min(m_config.iMSS, w_hs.m_iMSS); + if (mss_size < MinimumMSS(peer.family())) + { + HLOGC(cnlog.Debug, log << CONID() << "acceptAndRespond: peer's MSS=" << w_hs.m_iMSS + << " is too small, can't accept the connection"); + m_RejectReason = SRT_REJ_ROGUE; + w_hs.m_iReqType = URQFailure(m_RejectReason); + throw CUDTException(MJ_SETUP, MN_REJECTED, 0); + } - const size_t full_hdr_size = CPacket::UDP_HDR_SIZE + CPacket::HDR_SIZE; - m_iMaxSRTPayloadSize = m_config.iMSS - full_hdr_size; + m_config.iMSS = mss_size; + + m_iMaxDataPayloadSize = m_config.iMSS - full_hdr_size; - HLOGC(cnlog.Debug, log << CONID() << "acceptAndRespond: PAYLOAD SIZE: " << m_iMaxSRTPayloadSize); + HLOGC(cnlog.Debug, log << CONID() << "acceptAndRespond: PAYLOAD SIZE: " << m_iMaxDataPayloadSize); // exchange info for maximum flow window size m_iFlowWindowSize = w_hs.m_iFlightFlagSize; @@ -6026,7 +6058,6 @@ void srt::CUDT::acceptAndRespond(const sockaddr_any& agent, const sockaddr_any& rewriteHandshakeData(peer, (w_hs)); - // Prepare all structures if (!prepareConnectionObjects(w_hs, HSD_DRAW, 0)) { @@ -6154,7 +6185,7 @@ void srt::CUDT::acceptAndRespond(const sockaddr_any& agent, const sockaddr_any& // TODO: Here create CONCLUSION RESPONSE with: // - just the UDT handshake, if HS_VERSION_UDT4, // - if higher, the UDT handshake, the SRT HSRSP, the SRT KMRSP. - size_t size = m_iMaxSRTPayloadSize; + size_t size = controlPayloadSize(peer.family()); // Allocate the maximum possible memory for an SRT payload. // This is a maximum you can send once. CPacket rsppkt; @@ -6934,11 +6965,11 @@ int srt::CUDT::sendmsg2(const char *data, int len, SRT_MSGCTRL& w_mctrl) // out a message of a length that exceeds the total size of the sending // buffer (configurable by SRTO_SNDBUF). - if (m_config.bMessageAPI && len > int(m_config.iSndBufSize * m_iMaxSRTPayloadSize)) + if (m_config.bMessageAPI && len > int(m_config.iSndBufSize * m_iMaxDataPayloadSize)) { LOGC(aslog.Error, log << CONID() << "Message length (" << len << ") exceeds the size of sending buffer: " - << (m_config.iSndBufSize * m_iMaxSRTPayloadSize) << ". Use SRTO_SNDBUF if needed."); + << (m_config.iSndBufSize * m_iMaxDataPayloadSize) << ". Use SRTO_SNDBUF if needed."); throw CUDTException(MJ_NOTSUP, MN_XSIZE, 0); } @@ -7069,7 +7100,7 @@ int srt::CUDT::sendmsg2(const char *data, int len, SRT_MSGCTRL& w_mctrl) // Just return how many bytes were actually scheduled for writing. // XXX May be reasonable to add a flag that requires that the function // not return until the buffer is sent completely. - size = min(len, sndBuffersLeft() * m_iMaxSRTPayloadSize); + size = min(len, sndBuffersLeft() * m_iMaxDataPayloadSize); } { @@ -7361,7 +7392,7 @@ int srt::CUDT::receiveMessage(char* data, int len, SRT_MSGCTRL& w_mctrl, int by_ // After signaling the tsbpd for ready data, report the bandwidth. #if ENABLE_HEAVY_LOGGING - double bw = Bps2Mbps(int64_t(m_iBandwidth) * m_iMaxSRTPayloadSize ); + double bw = Bps2Mbps(int64_t(m_iBandwidth) * m_iMaxDataPayloadSize ); HLOGC(arlog.Debug, log << CONID() << "CURRENT BANDWIDTH: " << bw << "Mbps (" << m_iBandwidth << " buffers per second)"); #endif } @@ -7853,7 +7884,7 @@ void srt::CUDT::bstats(CBytePerfMon *perf, bool clear, bool instantaneous) const int64_t availbw = m_iBandwidth == 1 ? m_RcvTimeWindow.getBandwidth() : m_iBandwidth.load(); - perf->mbpsBandwidth = Bps2Mbps(availbw * (m_iMaxSRTPayloadSize + pktHdrSize)); + perf->mbpsBandwidth = Bps2Mbps(availbw * (m_iMaxDataPayloadSize + pktHdrSize)); if (tryEnterCS(m_ConnectionLock)) { @@ -8193,22 +8224,18 @@ void srt::CUDT::sendCtrl(UDTMessageType pkttype, const int32_t* lparam, void* rp // this is periodically NAK report; make sure NAK cannot be sent back too often // read loss list from the local receiver loss list - int32_t *data = new int32_t[m_iMaxSRTPayloadSize / 4]; - int losslen; - m_pRcvLossList->getLossArray(data, losslen, m_iMaxSRTPayloadSize / 4); - - if (0 < losslen) + FixedArray data (m_iMaxDataPayloadSize / sizeof(int32_t)); + int losslen = m_pRcvLossList->getLossArray(data); + if (losslen > 0) { - ctrlpkt.pack(pkttype, NULL, data, losslen * 4); + ctrlpkt.pack(pkttype, /*lparam*/ NULL, data.data(), losslen * sizeof(int32_t)); ctrlpkt.set_id(m_PeerID); - nbsent = m_pSndQueue->sendto(m_PeerAddr, ctrlpkt, m_SourceAddr); + nbsent = m_pSndQueue->sendto(m_PeerAddr, ctrlpkt, m_SourceAddr); enterCS(m_StatsLock); m_stats.rcvr.sentNak.count(1); leaveCS(m_StatsLock); } - - delete[] data; } // update next NAK time, which should wait enough time for the retansmission, but not too long @@ -8555,11 +8582,12 @@ int srt::CUDT::sendCtrlAck(CPacket& ctrlpkt, int size) data[ACKD_RCVSPEED] = m_RcvTimeWindow.getPktRcvSpeed((rcvRate)); data[ACKD_BANDWIDTH] = m_RcvTimeWindow.getBandwidth(); + // XXX Likely this can be removed already. //>>Patch while incompatible (1.0.2) receiver floating around if (m_uPeerSrtVersion == SrtVersion(1, 0, 2)) { data[ACKD_RCVRATE] = rcvRate; // bytes/sec - data[ACKD_XMRATE_VER102_ONLY] = data[ACKD_BANDWIDTH] * m_iMaxSRTPayloadSize; // bytes/sec + data[ACKD_XMRATE_VER102_ONLY] = data[ACKD_BANDWIDTH] * m_iMaxDataPayloadSize; // bytes/sec ctrlsz = ACKD_FIELD_SIZE * ACKD_TOTAL_SIZE_VER102_ONLY; } else if (m_uPeerSrtVersion >= SrtVersion(1, 0, 3)) @@ -8937,10 +8965,11 @@ bool srt::CUDT::processCtrlAck(const CPacket &ctrlpkt, const steady_clock::time_ /* SRT v1.0.2 Bytes-based stats: bandwidth (pcData[ACKD_XMRATE_VER102_ONLY]) and delivery rate (pcData[ACKD_RCVRATE]) in * bytes/sec instead of pkts/sec */ /* SRT v1.0.3 Bytes-based stats: only delivery rate (pcData[ACKD_RCVRATE]) in bytes/sec instead of pkts/sec */ + // XXX v1.0.2 handling can be likely deleted. if (acksize > ACKD_TOTAL_SIZE_UDTBASE) bytesps = ackdata[ACKD_RCVRATE]; else - bytesps = pktps * m_iMaxSRTPayloadSize; + bytesps = pktps * m_iMaxDataPayloadSize; m_iBandwidth = avg_iir<8>(m_iBandwidth.load(), bandwidth); m_iDeliveryRate = avg_iir<8>(m_iDeliveryRate.load(), pktps); @@ -9325,7 +9354,7 @@ bool srt::CUDT::processCtrlHS(const CPacket& ctrlpkt) CPacket rsppkt; rsppkt.setControl(UMSG_HANDSHAKE); - rsppkt.allocate(m_iMaxSRTPayloadSize); + rsppkt.allocate(controlPayloadSize()); // If createSrtHandshake failed, don't send anything. Actually it can only fail on IPE. // There is also no possible IPE condition in case of HSv4 - for this version it will always return true. @@ -9342,6 +9371,12 @@ bool srt::CUDT::processCtrlHS(const CPacket& ctrlpkt) m_tsLastSndTime.store(steady_clock::now()); } } + + // If a REJECTION HS has been sent, break also the connection locally. + if (initdata.m_iReqType >= URQ_FAILURE_TYPES) + { + processCtrlShutdown(); // always returns true + } } else { @@ -11924,7 +11959,7 @@ int srt::CUDT::processConnectRequest(const sockaddr_any& addr, CPacket& packet) if (conn != CONN_ACCEPT) return conn; - packet.setLength(m_iMaxSRTPayloadSize); + packet.setLength(controlPayloadSize(addr.family())); if (!acpu->createSrtHandshake(SRT_CMD_HSRSP, SRT_CMD_KMRSP, kmdata, kmdatasize, (packet), (hs))) @@ -12544,7 +12579,7 @@ bool srt::CUDT::runAcceptHook(CUDT *acore, const sockaddr* peer, const CHandShak if (!bytelen || bytelen > CSrtConfig::MAX_SID_LENGTH) { LOGC(cnlog.Error, - log << CONID() << "interpretSrtHandshake: STREAMID length " << bytelen << " is 0 or > " + log << CONID() << FUNID() << ": STREAMID length " << bytelen << " is 0 or > " << +CSrtConfig::MAX_SID_LENGTH << " - PROTOCOL ERROR, REJECTING"); return false; } diff --git a/srtcore/core.h b/srtcore/core.h index f2f1dc8bd2..fb9f733be8 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -475,7 +475,7 @@ class CUDT uint32_t peerLatency_us() const { return m_iPeerTsbPdDelay_ms * 1000; } int peerIdleTimeout_ms() const { return m_config.iPeerIdleTimeout_ms; } - size_t maxPayloadSize() const { return m_iMaxSRTPayloadSize; } + size_t maxDataPayloadSize() const { return m_iMaxDataPayloadSize; } size_t OPT_PayloadSize() const { return m_config.zExpPayloadSize; } size_t payloadSize() const { @@ -488,7 +488,7 @@ class CUDT // If SRTO_PAYLOADSIZE was remaining with 0 (default for FILE mode) // then return the maximum payload size per packet. - return m_iMaxSRTPayloadSize; + return m_iMaxDataPayloadSize; } int sndLossLength() { return m_pSndLossList->getLossLength(); } @@ -536,12 +536,21 @@ class CUDT int minSndSize(int len = 0) const { - const int ps = (int) maxPayloadSize(); + const int ps = (int) maxDataPayloadSize(); if (len == 0) // weird, can't use non-static data member as default argument! len = ps; return m_config.bMessageAPI ? (len+ps-1)/ps : 1; } + // This returns the biggest possible size for an SRT payload with + // the default 1500 MTU size. When in doubt, use AF_INET, which returns + // bigger size, if needed for a safe allocation. + static int controlPayloadSize(int family = AF_INET) + { + int header = family == AF_INET6 ? CPacket::UDP_HDR_SIZE_IPv6 : CPacket::UDP_HDR_SIZE; + return CPacket::ETH_MAX_MTU_SIZE - header; + } + static int32_t makeTS(const time_point& from_time, const time_point& tsStartTime) { // NOTE: @@ -879,7 +888,7 @@ class CUDT int sndSpaceLeft() { - return static_cast(sndBuffersLeft() * maxPayloadSize()); + return static_cast(sndBuffersLeft() * maxDataPayloadSize()); } int sndBuffersLeft() @@ -946,7 +955,7 @@ class CUDT #endif private: - int m_iMaxSRTPayloadSize; // Maximum/regular payload size, in bytes + int m_iMaxDataPayloadSize; // Maximum data payload size, in bytes int m_iTsbPdDelay_ms; // Rx delay to absorb burst, in milliseconds int m_iPeerTsbPdDelay_ms; // Tx delay that the peer uses to absorb burst, in milliseconds bool m_bTLPktDrop; // Enable Too-late Packet Drop diff --git a/srtcore/crypto.cpp b/srtcore/crypto.cpp index ccc4714609..c6b18911a1 100644 --- a/srtcore/crypto.cpp +++ b/srtcore/crypto.cpp @@ -370,7 +370,32 @@ int srt::CCryptoControl::processSrtMsg_KMREQ( return SRT_CMD_NONE; } -int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, unsigned srtv) +// RETURNS: +// first: the converted state_value, or fallback if it was wrong +// second: the value to be set in the opposite direction, if different +inline std::pair ErraticKMState(uint32_t state_value) +{ + if (state_value >= uint32_t(SRT_KM_S_E_SIZE)) + { + LOGC(cnlog.Fatal, log << "processSrtMsg_KMRSP: IPE: unknown peer state value: " << state_value); + return std::make_pair(SRT_KM_S_BADSECRET, SRT_KM_S_BADSECRET); + } + + SRT_KM_STATE state = SRT_KM_STATE(state_value); + switch (state) + { + case SRT_KM_S_NOSECRET: + return std::make_pair(state, SRT_KM_S_UNSECURED); + + case SRT_KM_S_UNSECURED: + return std::make_pair(state, SRT_KM_S_NOSECRET); + + default: ; // do nothing + } + return std::make_pair(state, state); +} + +int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, unsigned srtv, bool is_handshake) { uint32_t srtd[SRTDATA_MAXSIZE]; size_t srtlen = len/sizeof(uint32_t); @@ -385,13 +410,27 @@ int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len return SRT_CMD_NONE; } + // Still handle HSv4 post-handshake-handshake. XXX: DEPRECATED. Remove in next major version. + if (!is_handshake) + { + // HSv4 version is unidirectional and encryption is only set in one direction. + // So, as KMRSP handler, the agent should be expected to be a sender, hence m_hSndCrypto + // should be created, but m_hRcvCrypto not. In case of HSv5, both should be simultaneously + // either NULL or valid pointers. + if (!m_hRcvCrypto && m_SndKmState != SRT_KM_S_SECURED) + is_handshake = true; + + if (!m_hSndCrypto) + return SRT_CMD_NONE; + } + /* All 32-bit msg fields (if present) swapped on reception * But HaiCrypt expect network order message * Re-swap to cancel it. */ HtoNLA(srtd, srtdata, srtlen); - int retstatus = -1; + int retstatus = -1; // Error by default, unless all is confirmed // Since now, when CCryptoControl::decrypt() encounters an error, it will print it, ONCE, // until the next KMREQ is received as a key regeneration. @@ -399,55 +438,30 @@ int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len if (srtlen == 1) // Error report. Set accordingly. { - SRT_KM_STATE peerstate = SRT_KM_STATE(srtd[SRT_KMR_KMSTATE]); /* Bad or no passphrase */ - m_SndKmMsg[0].iPeerRetry = 0; - m_SndKmMsg[1].iPeerRetry = 0; - - switch (peerstate) - { - case SRT_KM_S_BADSECRET: - m_SndKmState = m_RcvKmState = SRT_KM_S_BADSECRET; - retstatus = -1; - break; - - // Default embraces two cases: - // NOSECRET: this KMRSP was sent by secured Peer, but Agent supplied no password. - // UNSECURED: this KMRSP was sent by unsecure Peer because Agent sent KMREQ. - - case SRT_KM_S_NOSECRET: - // This means that the peer did not set the password, while Agent did. - m_RcvKmState = SRT_KM_S_UNSECURED; - m_SndKmState = SRT_KM_S_NOSECRET; - retstatus = -1; - break; - - case SRT_KM_S_UNSECURED: - // This means that KMRSP was sent without KMREQ, to inform the Agent, - // that the Peer, unlike Agent, does use password. Agent can send then, - // but can't decrypt what Peer would send. - m_RcvKmState = SRT_KM_S_NOSECRET; - m_SndKmState = SRT_KM_S_UNSECURED; + SRT_KM_STATE peerstate, revstate; + Tie2(peerstate, revstate) = ErraticKMState(srtd[SRT_KMR_KMSTATE]); + if (peerstate == SRT_KM_S_UNSECURED) retstatus = 0; - break; -#ifdef ENABLE_AEAD_API_PREVIEW - case SRT_KM_S_BADCRYPTOMODE: - // The peer expects to use a different cryptographic mode (e.g. AES-GCM, not AES-CTR). - m_RcvKmState = SRT_KM_S_BADCRYPTOMODE; - m_SndKmState = SRT_KM_S_BADCRYPTOMODE; - retstatus = -1; - break; -#endif - default: - LOGC(cnlog.Fatal, log << "processSrtMsg_KMRSP: IPE: unknown peer error state: " - << KmStateStr(peerstate) << " (" << int(peerstate) << ")"); - m_RcvKmState = SRT_KM_S_NOSECRET; - m_SndKmState = SRT_KM_S_NOSECRET; - retstatus = -1; //This is IPE - break; + // If the erroneous KMRSP was received while the connection is established, we state + // the connection should be SECURED already, so no change of the state is done. + if (!is_handshake) + { + LOGC(cnlog.Warn, log << "processSrtMsg_KMRSP: rogue KMRSP received as KMX update - ignoring"); + // Still proceed with the summary logging. + } + else + { + m_SndKmMsg[0].iPeerRetry = 0; + m_SndKmMsg[1].iPeerRetry = 0; + m_SndKmState = peerstate; + m_RcvKmState = revstate; + LOGC(cnlog.Warn, log << "processSrtMsg_KMRSP: received failure report. STATE: " << KmStateStr(m_RcvKmState)); } - - LOGC(cnlog.Warn, log << "processSrtMsg_KMRSP: received failure report. STATE: " << KmStateStr(m_RcvKmState)); + } + else if (srtlen == 0) // 0-size is a special value for MsgLen, so avoid checks! + { + LOGC(cnlog.Warn, log << "processSrtMsg_KMRSP: IPE/EPE KM response key is empty - ignoring"); } else { @@ -467,14 +481,18 @@ int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len else { retstatus = -1; - LOGC(cnlog.Error, log << "processSrtMsg_KMRSP: IPE??? KM response key matches no key"); + LOGC(cnlog.Error, log << "processSrtMsg_KMRSP: IPE/EPE KM response key matches no key"); /* XXX INSECURE LOGC(cnlog.Error, log << "processSrtMsg_KMRSP: KM response: [" << FormatBinaryString((uint8_t*)srtd, len) << "] matches no key 0=[" << FormatBinaryString((uint8_t*)m_SndKmMsg[0].Msg, m_SndKmMsg[0].MsgLen) << "] 1=[" << FormatBinaryString((uint8_t*)m_SndKmMsg[1].Msg, m_SndKmMsg[1].MsgLen) << "]"); */ - m_SndKmState = m_RcvKmState = SRT_KM_S_BADSECRET; + // DO NOT change the state in case of KMX update with secured sender. + if (is_handshake) + { + m_SndKmState = m_RcvKmState = SRT_KM_S_BADSECRET; + } } HLOGC(cnlog.Debug, log << "processSrtMsg_KMRSP: key[0]: len=" << m_SndKmMsg[0].MsgLen << " retry=" << m_SndKmMsg[0].iPeerRetry << "; key[1]: len=" << m_SndKmMsg[1].MsgLen << " retry=" << m_SndKmMsg[1].iPeerRetry); @@ -508,7 +526,7 @@ int srt::CCryptoControl::processSrtMsg_KMREQ( return SRT_CMD_KMRSP; } -int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t*, size_t, unsigned) +int srt::CCryptoControl::processSrtMsg_KMRSP(const uint32_t*, size_t, unsigned, bool) { LOGP(cnlog.Error, "processSrtMsg_KMRSP: Encryption not enabled at compile time; not expected to receive SRT_CMD_KMRSP"); return SRT_CMD_NONE; diff --git a/srtcore/crypto.h b/srtcore/crypto.h index a1bb42d992..4a0d6aa02b 100644 --- a/srtcore/crypto.h +++ b/srtcore/crypto.h @@ -141,7 +141,7 @@ class CCryptoControl /// 1 - the given payload is the same as the currently used key /// 0 - there's no key in agent or the payload is error message with agent NOSECRET. /// -1 - the payload is error message with other state or it doesn't match the key - int processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, unsigned srtv); + int processSrtMsg_KMRSP(const uint32_t* srtdata, size_t len, unsigned srtv, bool is_handshake); void createFakeSndContext(); const unsigned char* getKmMsg_data(size_t ki) const { return m_SndKmMsg[ki].Msg; } diff --git a/srtcore/fec.cpp b/srtcore/fec.cpp index 4ee5e44284..498c521830 100644 --- a/srtcore/fec.cpp +++ b/srtcore/fec.cpp @@ -59,11 +59,11 @@ bool FECFilterBuiltin::verifyConfig(const SrtFilterConfig& cfg, string& w_error) string colspec = map_get(cfg.parameters, "cols"), rowspec = map_get(cfg.parameters, "rows"); - int out_rows = 1; + int out_rows = 1, out_cols = 2; if (colspec != "") { - int out_cols = atoi(colspec.c_str()); + out_cols = atoi(colspec.c_str()); if (out_cols < 2) { w_error = "at least 'cols' must be specified and > 1"; @@ -81,6 +81,12 @@ bool FECFilterBuiltin::verifyConfig(const SrtFilterConfig& cfg, string& w_error) } } + if (out_cols > MAX_GROUP_SIZE || out_rows > MAX_GROUP_SIZE || out_rows < -MAX_GROUP_SIZE) + { + w_error = "rows/cols size must not exceed 16-bit max value"; + return false; + } + // Extra interpret level, if found, default never. // Check only those that are managed. string level = map_get(cfg.parameters, "arq"); @@ -213,74 +219,85 @@ FECFilterBuiltin::FECFilterBuiltin(const SrtFilterInitializer &init, std::vector // Required to store in the header when rebuilding rcv.id = socketID(); - // Setup the bit matrix, initialize everything with false. - - // Vertical size (y) - rcv.cells.resize(sizeCol() * sizeRow(), false); - - // These sequence numbers are both the value of ISN-1 at the moment - // when the handshake is done. The sender ISN is generated here, the - // receiver ISN by the peer. Both should be known after the handshake. - // Later they will be updated as packets are transmitted. - - int32_t snd_isn = CSeqNo::incseq(sndISN()); - int32_t rcv_isn = CSeqNo::incseq(rcvISN()); - - // Alright, now we need to get the ISN from m_parent - // to extract the sequence number allowing qualification to the group. - // The base values must be prepared so that feedSource can qualify them. - - // SEPARATE FOR SENDING AND RECEIVING! - - // Now, assignment of the groups requires: - // For row groups, simply the size of the group suffices. - // For column groups, you need a whole matrix of all sequence - // numbers that are base sequence numbers for the group. - // Sequences that belong to this group are: - // 1. First packet has seq+1 towards the base. - // 2. Every next packet has this value + the size of the row group. - // So: group dispatching is: - // - get the column number - // - extract the group data for that column - // - check if the sequence is later than the group base sequence, if not, report no group for the packet - // - sanity check, if the seqdiff divided by row size gets 0 remainder - // - The result from the above division can't exceed the column size, otherwise - // it's another group. The number of currently collected data should be in 'collected'. + // This will catch any memory allocation error and translate + // to MJ_SETUP / MN_NORES. + try + { + // Setup the bit matrix, initialize everything with false. + + // Vertical size (y) + rcv.cells.resize(sizeCol() * sizeRow(), false); + + // These sequence numbers are both the value of ISN-1 at the moment + // when the handshake is done. The sender ISN is generated here, the + // receiver ISN by the peer. Both should be known after the handshake. + // Later they will be updated as packets are transmitted. + + int32_t snd_isn = CSeqNo::incseq(sndISN()); + int32_t rcv_isn = CSeqNo::incseq(rcvISN()); + + // Alright, now we need to get the ISN from m_parent + // to extract the sequence number allowing qualification to the group. + // The base values must be prepared so that feedSource can qualify them. + + // SEPARATE FOR SENDING AND RECEIVING! + + // Now, assignment of the groups requires: + // For row groups, simply the size of the group suffices. + // For column groups, you need a whole matrix of all sequence + // numbers that are base sequence numbers for the group. + // Sequences that belong to this group are: + // 1. First packet has seq+1 towards the base. + // 2. Every next packet has this value + the size of the row group. + // So: group dispatching is: + // - get the column number + // - extract the group data for that column + // - check if the sequence is later than the group base sequence, if not, report no group for the packet + // - sanity check, if the seqdiff divided by row size gets 0 remainder + // - The result from the above division can't exceed the column size, otherwise + // it's another group. The number of currently collected data should be in 'collected'. + + // Now set up the group starting sequences. + // The very first group in both dimensions will have the value of ISN in particular direction. + + // Set up sender part. + // + // Size: rows + // Step: 1 (next packet in group is 1 past the previous one) + // Slip: rows (first packet in the next group is distant to first packet in the previous group by 'rows') + HLOGC(pflog.Debug, log << "FEC: INIT: ISN { snd=" << snd_isn << " rcv=" << rcv_isn << " }; sender single row"); + ConfigureGroup(snd.row, snd_isn, 1, sizeRow()); + + // In the beginning we need just one reception group. New reception + // groups will be created in tact with receiving packets outside this one. + // The value of rcv.row[0].base will be used as an absolute base for calculating + // the index of the group for a given received packet. + rcv.rowq.resize(1); + HLOGP(pflog.Debug, "FEC: INIT: receiver first row"); + ConfigureGroup(rcv.rowq[0], rcv_isn, 1, sizeRow()); - // Now set up the group starting sequences. - // The very first group in both dimensions will have the value of ISN in particular direction. + if (sizeCol() > 1) + { + // Size: cols + // Step: rows (the next packet in the group is one row later) + // Slip: rows+1 (the first packet in the next group is later by 1 column + one whole row down) + + HLOGP(pflog.Debug, "FEC: INIT: sender first N columns"); + ConfigureColumns(snd.cols, snd_isn); + HLOGP(pflog.Debug, "FEC: INIT: receiver first N columns"); + ConfigureColumns(rcv.colq, rcv_isn); + } - // Set up sender part. - // - // Size: rows - // Step: 1 (next packet in group is 1 past the previous one) - // Slip: rows (first packet in the next group is distant to first packet in the previous group by 'rows') - HLOGC(pflog.Debug, log << "FEC: INIT: ISN { snd=" << snd_isn << " rcv=" << rcv_isn << " }; sender single row"); - ConfigureGroup(snd.row, snd_isn, 1, sizeRow()); - - // In the beginning we need just one reception group. New reception - // groups will be created in tact with receiving packets outside this one. - // The value of rcv.row[0].base will be used as an absolute base for calculating - // the index of the group for a given received packet. - rcv.rowq.resize(1); - HLOGP(pflog.Debug, "FEC: INIT: receiver first row"); - ConfigureGroup(rcv.rowq[0], rcv_isn, 1, sizeRow()); - - if (sizeCol() > 1) + // The bit markers that mark the received/lost packets will be expanded + // as packets come in. + rcv.cell_base = rcv_isn; + } + catch (std::bad_alloc& be) { - // Size: cols - // Step: rows (the next packet in the group is one row later) - // Slip: rows+1 (the first packet in the next group is later by 1 column + one whole row down) - - HLOGP(pflog.Debug, "FEC: INIT: sender first N columns"); - ConfigureColumns(snd.cols, snd_isn); - HLOGP(pflog.Debug, "FEC: INIT: receiver first N columns"); - ConfigureColumns(rcv.colq, rcv_isn); + LOGC(pflog.Error, log << "FEC: INIT: Memory allocation error with cols=" + << numberCols() << " rows=" << numberRows()); + throw CUDTException(MJ_SYSTEMRES, MN_MEMORY, 0); } - - // The bit markers that mark the received/lost packets will be expanded - // as packets come in. - rcv.cell_base = rcv_isn; } template @@ -544,11 +561,16 @@ void FECFilterBuiltin::ClipPacket(Group& g, const CPacket& pkt) // Clipping a control packet does merely the same, just the packet has // different contents, so it must be differetly interpreted. -void FECFilterBuiltin::ClipControlPacket(Group& g, const CPacket& pkt) +bool FECFilterBuiltin::ClipControlPacket(Group& g, const CPacket& pkt) { // Both length and timestamp must be taken as NETWORK ORDER // before applying the clip. + if (pkt.size() < 4) // paranoid check + { + return false; + } + const char* fec_header = pkt.data(); const char* payload = fec_header + 4; size_t payload_clip_len = pkt.size() - 4; @@ -567,6 +589,8 @@ void FECFilterBuiltin::ClipControlPacket(Group& g, const CPacket& pkt) << " LENGTH[ne]=" << g.length_clip << " TS[he]=" << g.timestamp_clip << " PL4=" << (*(uint32_t*)&g.payload_clip[0])); + + return true; } void FECFilterBuiltin::ClipRebuiltPacket(Group& g, Receive::PrivPacket& pkt) @@ -601,6 +625,10 @@ void FECFilterBuiltin::ClipData(Group& g, uint16_t length_net, uint8_t kflg, HLOGC(pflog.Debug, log << "FEC CLIP: data pkt.size=" << payload_size << " to a clip buffer size=" << payloadSize()); + // Paranoid check + if (payload_size > payloadSize()) + payload_size = payloadSize(); + // Payload goes "as is". for (size_t i = 0; i < payload_size; ++i) { @@ -887,7 +915,7 @@ bool FECFilterBuiltin::receive(const CPacket& rpkt, loss_seqs_t& loss_seqs) // - Both HangVertical and HangHorizontal okv = HangVertical(rpkt, isfec.colx, irrecover_col); - IF_HEAVY_LOGGING(bool discrep = (okv == HANG_CRAZY) ? int(okh) < HANG_CRAZY : false); + IF_HEAVY_LOGGING(bool discrep = (okv == HANG_CRAZY) ? okh < HANG_CRAZY : false); HLOGC(pflog.Debug, log << "FEC: HangVertical %" << rpkt.getSeqNo() << " msgno=" << rpkt.getMsgSeq() << " RESULT=" << hangname[okh] @@ -1162,7 +1190,11 @@ FECFilterBuiltin::EHangStatus FECFilterBuiltin::HangHorizontal(const CPacket& rp { if (!rowg.fec) { - ClipControlPacket(rowg, rpkt); + if (!ClipControlPacket(rowg, rpkt)) + { + LOGC(pflog.Error, log << "FEC/H: Rogue control packet received"); + return HANG_CRAZY; // That's the only "complete failure" statement + } rowg.fec = true; HLOGC(pflog.Debug, log << "FEC/H: FEC/CTL packet clipped, %" << seq << " base=%" << rowg.base); } @@ -1863,7 +1895,11 @@ FECFilterBuiltin::EHangStatus FECFilterBuiltin::HangVertical(const CPacket& rpkt { if (!colg.fec) { - ClipControlPacket(colg, rpkt); + if (!ClipControlPacket(colg, rpkt)) + { + LOGC(pflog.Error, log << "FEC/H: Rogue control packet received"); + return HANG_CRAZY; // That's the only "complete failure" statement + } colg.fec = true; HLOGC(pflog.Debug, log << "FEC/V: FEC/CTL packet clipped, %" << seq << " FOR COLUMN " << int(fec_col) << " base=%" << colg.base); diff --git a/srtcore/fec.h b/srtcore/fec.h index f4ed0e4cca..c5715786e4 100644 --- a/srtcore/fec.h +++ b/srtcore/fec.h @@ -34,6 +34,12 @@ class FECFilterBuiltin: public SrtPacketFilterBase public: + // rows/cols size can have this size at maximum, otherwise + // there's a risk to have an overflow memory size value on + // 32-bit systems, which may result in a false success followed + // by a crash. + static const int MAX_GROUP_SIZE = 0xFFFF; + size_t numberCols() const { return m_number_cols; } size_t numberRows() const { return m_number_rows; } @@ -217,7 +223,7 @@ class FECFilterBuiltin: public SrtPacketFilterBase EHangStatus HangHorizontal(const CPacket& pkt, bool fec_ctl, loss_seqs_t& irrecover); EHangStatus HangVertical(const CPacket& pkt, signed char fec_colx, loss_seqs_t& irrecover); - void ClipControlPacket(Group& g, const CPacket& pkt); + SRT_ATR_NODISCARD bool ClipControlPacket(Group& g, const CPacket& pkt); void ClipRebuiltPacket(Group& g, Receive::PrivPacket& pkt); void RcvRebuild(Group& g, int32_t seqno, Group::Type tp); int32_t RcvGetLossSeqHoriz(Group& g); diff --git a/srtcore/group.cpp b/srtcore/group.cpp index b873bb710d..bd034a516f 100644 --- a/srtcore/group.cpp +++ b/srtcore/group.cpp @@ -3587,9 +3587,8 @@ void CUDTGroup::sendBackup_RetryWaitBlocked(SendBackupCtx& w_sendBackupCtx CUDTSocket* s = m_Global.locateSocket(id, CUDTUnited::ERH_RETURN); // << LOCKS m_GlobControlLock! if (s) { - HLOGC(gslog.Debug, - log << "grp/sendBackup: swait/ex on @" << (id) - << " while waiting for any writable socket - CLOSING"); + HLOGC(gslog.Debug, log << "grp/sendBackup: swait/ex on @" << id + << " while waiting for any writable socket - CLOSING"); CUDT::uglobal().close(s); // << LOCKS m_GlobControlLock, then GroupLock! } else @@ -3621,6 +3620,32 @@ void CUDTGroup::sendBackup_RetryWaitBlocked(SendBackupCtx& w_sendBackupCtx throw CUDTException(MJ_CONNECTION, MN_CONNLOST, 0); } + // IMPORTANT! + // There was a socket deletion possibly done above, and as well there + // was the m_GroupLock lifted for that check activity, so potentially any socket + // from m_Group container could be deleted; review them and remove any dangling + // objects from w_sendBackupCtx. + // + // Due to the nature of the m_Group container, use mark-and-sweep method. + + set remain; + w_sendBackupCtx.getSocketIds( (remain) ); + + // MARK + for (gli_t d = m_Group.begin(); d != m_Group.end(); ++d) + { + remain.erase(d->id); + } + + HLOGC(gslog.Debug, log << "grp/sendBackup: RE-LOCK, checking members deleted in the meantime: " + << Printable(remain)); + + // SWEEP + for (set::iterator i = remain.begin(); i != remain.end(); ++i) + { + w_sendBackupCtx.deleteById(*i); + } + // Ok, now check if we have at least one write-ready. // Note that the procedure of activation of a new link in case of // no stable links found embraces also rexmit-sending and status diff --git a/srtcore/group_backup.cpp b/srtcore/group_backup.cpp index 9adb19607b..825791dab2 100644 --- a/srtcore/group_backup.cpp +++ b/srtcore/group_backup.cpp @@ -144,6 +144,30 @@ unsigned SendBackupCtx::countMembersByState(BackupMemberState st) const return m_stateCounter[st]; } +void SendBackupCtx::getSocketIds(std::set& ids) const +{ + typedef vector::const_iterator const_iter_t; + for (const_iter_t i = m_memberStates.begin(); i != m_memberStates.end(); ++i) + { + ids.insert(i->socketID); + } +} + +bool SendBackupCtx::deleteById(SRTSOCKET id) +{ + typedef vector::iterator iter_t; + for (iter_t i = m_memberStates.begin(); i != m_memberStates.end(); ++i) + { + if (i->socketID == id) + { + m_memberStates.erase(i); + return true; + } + } + + return false; +} + std::string SendBackupCtx::printMembers() const { stringstream ss; diff --git a/srtcore/group_backup.h b/srtcore/group_backup.h index 71a1f7abba..4705cc3ad3 100644 --- a/srtcore/group_backup.h +++ b/srtcore/group_backup.h @@ -107,6 +107,8 @@ namespace groups /// Higher weight comes first, same weight: stable first, then fresh active. void sortByWeightAndState(); + bool deleteById(SRTSOCKET id); + BackupMemberState getMemberState(const SocketData* pSocketDataIt) const; unsigned countMembersByState(BackupMemberState st) const; @@ -122,6 +124,8 @@ namespace groups const CRateEstimator& getRateEstimate() const { return m_rateEstimate; } + void getSocketIds(std::set& out) const; + private: std::vector m_memberStates; // TODO: consider std::map here? unsigned m_stateCounter[BKUPST_E_SIZE]; diff --git a/srtcore/handshake.cpp b/srtcore/handshake.cpp index c97b4e2a34..878b90452a 100644 --- a/srtcore/handshake.cpp +++ b/srtcore/handshake.cpp @@ -51,10 +51,11 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include "handshake.h" +#include "packet.h" #include "udt.h" #include "api.h" #include "core.h" -#include "handshake.h" #include "utilities.h" using namespace std; @@ -198,7 +199,7 @@ bool srt::CHandShake::valid() { if (m_iVersion < CUDT::HS_VERSION_UDT4 || m_iISN < 0 || m_iISN >= CSeqNo::m_iMaxSeqNo - || m_iMSS < 32 + || m_iMSS < int(CPacket::UDP_HDR_SIZE_IPv6 + CPacket::HDR_SIZE) || m_iFlightFlagSize < 2) return false; diff --git a/srtcore/list.cpp b/srtcore/list.cpp index 94eaa5da0d..8573ee258d 100644 --- a/srtcore/list.cpp +++ b/srtcore/list.cpp @@ -791,13 +791,12 @@ int32_t srt::CRcvLossList::getFirstLostSeq() const return m_caSeq[m_iHead].seqstart; } -void srt::CRcvLossList::getLossArray(int32_t* array, int& len, int limit) +int srt::CRcvLossList::getLossArray(FixedArray& array) { - len = 0; - + int len = 0; int i = m_iHead; - while ((len < limit - 1) && (-1 != i)) + while ((len < int(array.size()) - 1) && (i != -1)) { array[len] = m_caSeq[i].seqstart; if (SRT_SEQNO_NONE != m_caSeq[i].seqend) @@ -812,6 +811,7 @@ void srt::CRcvLossList::getLossArray(int32_t* array, int& len, int limit) i = m_caSeq[i].inext; } + return len; } srt::CRcvFreshLoss::CRcvFreshLoss(int32_t seqlo, int32_t seqhi, int initial_age) diff --git a/srtcore/list.h b/srtcore/list.h index 802ede9a2d..539c18478d 100644 --- a/srtcore/list.h +++ b/srtcore/list.h @@ -57,6 +57,7 @@ modified by #include "udt.h" #include "common.h" +#include "utilities.h" namespace srt { @@ -203,11 +204,9 @@ class CRcvLossList int32_t getFirstLostSeq() const; /// Get a encoded loss array for NAK report. - /// @param [out] array the result list of seq. no. to be included in NAK. - /// @param [out] len physical length of the result array. - /// @param [in] limit maximum length of the array. - - void getLossArray(int32_t* array, int& len, int limit); + /// @param [inout] array the result list of seq. no. to be included in NAK. + /// @return physical length of the result array. + int getLossArray(FixedArray& array); private: struct Seq diff --git a/srtcore/packet.h b/srtcore/packet.h index 6747e244c9..a735c4cb0d 100644 --- a/srtcore/packet.h +++ b/srtcore/packet.h @@ -372,7 +372,12 @@ class CPacket static const size_t HDR_SIZE = sizeof(HEADER_TYPE); // packet header size = SRT_PH_E_SIZE * sizeof(uint32_t) // Can also be calculated as: sizeof(struct ether_header) + sizeof(struct ip) + sizeof(struct udphdr). - static const size_t UDP_HDR_SIZE = 28; // 20 bytes IPv4 + 8 bytes of UDP { u16 sport, dport, len, csum }. + static const size_t BARE_UDP_HDR_SIZE = 8; // 8 bytes of UDP { u16 sport, dport, len, csum }. + static const size_t IPv4_HDR_SIZE = 20; // 20 bytes IPv4 + static const size_t IPv6_HDR_SIZE = 40; // 40 bytes IPv6 + + static const size_t UDP_HDR_SIZE = BARE_UDP_HDR_SIZE + IPv4_HDR_SIZE; + static const size_t UDP_HDR_SIZE_IPv6 = BARE_UDP_HDR_SIZE + IPv6_HDR_SIZE; static const size_t SRT_DATA_HDR_SIZE = UDP_HDR_SIZE + HDR_SIZE; diff --git a/srtcore/queue.cpp b/srtcore/queue.cpp index 69eb63a8d9..278d802be6 100644 --- a/srtcore/queue.cpp +++ b/srtcore/queue.cpp @@ -927,7 +927,7 @@ void srt::CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst return; HLOGC(cnlog.Debug, - log << "updateConnStatus: collected " << toProcess.size() << " for processing, " << toRemove.size() + log << FUNID() << ": collected " << toProcess.size() << " for processing, " << toRemove.size() << " to close"); // Repeat (resend) connection request. @@ -947,43 +947,45 @@ void srt::CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst // to interpret these data (for caller-listener this was already done by `processConnectRequest` // before calling this function), and it checks for the data presence. - EReadStatus read_st = rst; - EConnectStatus conn_st = cst; - CUDTUnited::SocketKeeper sk (CUDT::uglobal(), i->id); if (!sk.socket) { // Socket deleted already, so stop this and proceed to the next loop. - LOGC(cnlog.Error, log << "updateConnStatus: IPE: socket @" << i->id << " already closed, proceed to only removal from lists"); + LOGC(cnlog.Error, log << FUNID() << ": IPE: socket @" << i->id << " already closed, proceed to only removal from lists"); toRemove.push_back(*i); continue; } + EReadStatus read_st = rst; + EConnectStatus conn_st = cst; - if (cst != CONN_RENDEZVOUS && dest_id != 0) - { - if (i->id != dest_id) - { - HLOGC(cnlog.Debug, log << "updateConnStatus: cst=" << ConnectStatusStr(cst) << " but for RID @" << i->id - << " dest_id=@" << dest_id << " - resetting to AGAIN"); + // Ok, we should have 3 cases here: + // 1. id == 0 ==> conn_st cannot be == CONN_RENDEZVOUS; reset to AGAIN always + // 2. conn_st == CONN_RENDEZVOUS -> id > 0 and no "alien" sockets are expected to be in the loop -> never reset to AGAIN + // 3. id > 0 and no rendezvous -> reset to AGAIN, unless id == dest_id. - read_st = RST_AGAIN; - conn_st = CONN_AGAIN; - } - else - { - HLOGC(cnlog.Debug, log << "updateConnStatus: cst=" << ConnectStatusStr(cst) << " for @" - << i->id); - } + // Condition: + // IF CONN_RENDEZVOUS -> never reset to AGAIN. + // ELSE IF dest_id == id -> don't reset to AGAIN + // ELSE: reset to again. + + if (cst == CONN_RENDEZVOUS || i->id == dest_id) + { + HLOGC(cnlog.Debug, log << FUNID() << ": applied to @" << i->id + << (cst == CONN_RENDEZVOUS ? "[RDV] " : "") + << " with target @" << dest_id << " -- remains: cst=" << ConnectStatusStr(cst)); } else { - HLOGC(cnlog.Debug, log << "updateConnStatus: cst=" << ConnectStatusStr(cst) << " and dest_id=@" << dest_id - << " - NOT checking against RID @" << i->id); + HLOGC(cnlog.Debug, log << FUNID() << ": applied to @" << i->id + << " with target @" << dest_id << " -- resetting to AGAIN"); + + read_st = RST_AGAIN; + conn_st = CONN_AGAIN; } HLOGC(cnlog.Debug, - log << "updateConnStatus: processing async conn for @" << i->id << " FROM " << i->peeraddr.str()); + log << FUNID() << ": processing async conn for @" << i->id << " FROM " << i->peeraddr.str()); if (!i->u->processAsyncConnectRequest(read_st, conn_st, pkt, i->peeraddr)) { @@ -1004,14 +1006,14 @@ void srt::CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst for (vector::iterator i = toRemove.begin(); i != toRemove.end(); ++i) { - HLOGC(cnlog.Debug, log << "updateConnStatus: COMPLETING dep objects update on failed @" << i->id); + HLOGC(cnlog.Debug, log << FUNID() << ": COMPLETING dep objects update on failed @" << i->id); remove(i->id); CUDTUnited::SocketKeeper sk (CUDT::uglobal(), i->id); if (!sk.socket) { // This actually shall never happen, so it's a kind of paranoid check. - LOGC(cnlog.Error, log << "updateConnStatus: IPE: socket @" << i->id << " already closed, NOT ACCESSING its contents"); + LOGC(cnlog.Error, log << FUNID() << ": IPE: socket @" << i->id << " already closed, NOT ACCESSING its contents"); continue; } @@ -1048,7 +1050,7 @@ void srt::CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst if (find_if(toRemove.begin(), toRemove.end(), LinkStatusInfo::HasID(i->m_iID)) != toRemove.end()) { LOGC(cnlog.Error, - log << "updateConnStatus: processAsyncConnectRequest FAILED on @" << i->m_iID + log << FUNID() << ": processAsyncConnectRequest FAILED on @" << i->m_iID << ". Setting TTL as EXPIRED."); i->m_tsTTL = steady_clock::time_point(); // Make it expire right now, will be picked up at the next iteration @@ -1069,7 +1071,7 @@ bool srt::CRendezvousQueue::qualifyToHandle(EReadStatus rst, return false; // nothing to process. HLOGC(cnlog.Debug, - log << "updateConnStatus: updating after getting pkt with DST socket ID @" << iDstSockID + log << FUNID() << ": updating after getting pkt with DST socket ID @" << iDstSockID << " status: " << ConnectStatusStr(cst)); for (list::iterator i = m_lRendezvousID.begin(), i_next = i; i != m_lRendezvousID.end(); i = i_next) diff --git a/srtcore/srt.h b/srtcore/srt.h index 36054e7d81..43c6593726 100644 --- a/srtcore/srt.h +++ b/srtcore/srt.h @@ -641,10 +641,9 @@ enum SRT_KM_STATE SRT_KM_S_SECURING = 1, // Stream encrypted, exchanging Keying Material SRT_KM_S_SECURED = 2, // Stream encrypted, keying Material exchanged, decrypting ok. SRT_KM_S_NOSECRET = 3, // Stream encrypted and no secret to decrypt Keying Material - SRT_KM_S_BADSECRET = 4 // Stream encrypted and wrong secret is used, cannot decrypt Keying Material -#ifdef ENABLE_AEAD_API_PREVIEW - ,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong cryptographic mode is used, cannot decrypt. Since v1.5.2. -#endif + SRT_KM_S_BADSECRET = 4, // Stream encrypted and wrong secret is used, cannot decrypt Keying Material + SRT_KM_S_BADCRYPTOMODE = 5, // Stream encrypted but wrong cryptographic mode is used, cannot decrypt. Since v1.5.2. + SRT_KM_S_E_SIZE }; enum SRT_EPOLL_OPT diff --git a/test/test_bonding.cpp b/test/test_bonding.cpp index 5948eed7c8..bc870ce58d 100644 --- a/test/test_bonding.cpp +++ b/test/test_bonding.cpp @@ -1042,6 +1042,9 @@ TEST(Bonding, BackupPriorityBegin) EXPECT_EQ(backup->memberstate, SRT_GST_IDLE); acthr.join(); + + srt_close(g_listen_socket); + srt_close(ss); } @@ -1237,6 +1240,9 @@ TEST(Bonding, BackupPriorityTakeover) EXPECT_EQ(backup->memberstate, SRT_GST_RUNNING); acthr.join(); + + srt_close(g_listen_socket); + srt_close(ss); } @@ -1590,6 +1596,7 @@ TEST(Bonding, BackupPrioritySelection) acthr.join(); + srt_close(g_listen_socket); srt_close(ss); } diff --git a/test/test_crypto.cpp b/test/test_crypto.cpp index 1d04b19238..d531303d89 100644 --- a/test/test_crypto.cpp +++ b/test/test_crypto.cpp @@ -28,8 +28,11 @@ TEST(CryptoKMRSP, RejectsMalformedLengths) const unsigned srtv = srt::SrtVersion(1, 5, 3); // Oversize: would overflow uint32_t srtd[SRTDATA_MAXSIZE]. - EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), SRT_CMD_MAXSZ + sizeof(uint32_t), srtv), + EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), SRT_CMD_MAXSZ + sizeof(uint32_t), srtv, false), srt::SRT_CMD_NONE); + // Empty / under-a-word: HtoNLA writes nothing and downstream code would read + // uninitialised stack from srtd[]. + EXPECT_EQ(crypt.processSrtMsg_KMRSP(garbage.data(), 0, srtv, false), srt::SRT_CMD_NONE); } @@ -229,7 +232,7 @@ TEST_F(Crypto, KMREQ_EmptySEK_Does_Not_Downgrade_Secured) // Forged KMRSP claiming any peer-failure state must not downgrade a // SECURED session. The dispatcher accepts KMRSPs unconditionally so each // peerstate branch in processSrtMsg_KMRSP is reachable off-path. -TEST_F(Crypto, DISABLED_KMRSP_PeerFailure_Does_Not_Downgrade_Secured) +TEST_F(Crypto, KMRSP_PeerFailure_Does_Not_Downgrade_Secured) { using namespace srt; @@ -260,8 +263,10 @@ TEST_F(Crypto, DISABLED_KMRSP_PeerFailure_Does_Not_Downgrade_Secured) uint32_t wire = (uint32_t)wire_peerstates[i]; uint32_t input = 0; NtoHLA(&input, &wire, 1); - EXPECT_EQ(m_crypt.processSrtMsg_KMRSP(&input, sizeof(input), SrtVersion(1, 5, 3)), - SRT_CMD_NONE); + + // NOTE: We do not check the result here; success is only expected with + // successful report. + m_crypt.processSrtMsg_KMRSP(&input, sizeof(input), SrtVersion(1, 5, 3), false); EXPECT_EQ(m_crypt.m_RcvKmState, SRT_KM_S_SECURED) << "peerstate=" << (int)wire_peerstates[i] << " downgraded m_RcvKmState"; @@ -582,7 +587,7 @@ TEST_F(CryptoCtr, KmrspPeerNoSecretOnNonSecured) uint32_t wire = (uint32_t)SRT_KM_S_NOSECRET; uint32_t input = 0; NtoHLA(&input, &wire, 1); - fresh.processSrtMsg_KMRSP(&input, sizeof(input), SrtVersion(1, 5, 3)); + fresh.processSrtMsg_KMRSP(&input, sizeof(input), SrtVersion(1, 5, 3), true); // Per crypto.cpp KMRSP NOSECRET branch: RX -> UNSECURED, SND -> NOSECRET. EXPECT_EQ(fresh.m_RcvKmState, SRT_KM_S_UNSECURED); @@ -622,7 +627,7 @@ TEST_F(CryptoCtr, KmrspSuccessTransitionsToSecured) std::array km_nworder; NtoHLA(km_nworder.data(), reinterpret_cast(kmmsg), km_len); - fresh.processSrtMsg_KMRSP(km_nworder.data(), km_len, SrtVersion(1, 5, 3)); + fresh.processSrtMsg_KMRSP(km_nworder.data(), km_len, SrtVersion(1, 5, 3), true); EXPECT_EQ(fresh.m_RcvKmState, SRT_KM_S_SECURED); EXPECT_EQ(fresh.m_SndKmState, SRT_KM_S_SECURED); @@ -651,7 +656,7 @@ TEST_F(CryptoCtr, KmrspPeerUnsecuredOnNonSecured) uint32_t wire = (uint32_t)SRT_KM_S_UNSECURED; uint32_t input = 0; NtoHLA(&input, &wire, 1); - fresh.processSrtMsg_KMRSP(&input, sizeof(input), SrtVersion(1, 5, 3)); + fresh.processSrtMsg_KMRSP(&input, sizeof(input), SrtVersion(1, 5, 3), true); // Per crypto.cpp KMRSP UNSECURED branch: RX -> NOSECRET, SND -> UNSECURED. EXPECT_EQ(fresh.m_RcvKmState, SRT_KM_S_NOSECRET); diff --git a/test/test_fec_rebuilding.cpp b/test/test_fec_rebuilding.cpp index da78aaa62f..f73ee85648 100644 --- a/test/test_fec_rebuilding.cpp +++ b/test/test_fec_rebuilding.cpp @@ -247,6 +247,8 @@ TEST(TestFEC, ConfigExchange) TEST(TestFEC, ConfigExchangeFaux) { srt::TestInit srtinit; + using namespace std; + CUDTSocket* s1; @@ -259,12 +261,15 @@ TEST(TestFEC, ConfigExchangeFaux) "fec,cols:10,rows:-1", // E3: invalid value for rows "fec,cols:10,layout:stairwars", // E4: invalid value for layout "fec,cols:10,arq:sometimes", // E5: invalid value for arq - "fec,cols:10,weight:2" // F: invalid parameter name + "fec,cols:10,weight:2", // F: invalid parameter name + "fec,cols:80000,rows:70000", // oversized + "fec,cols:10,rows:-70000" // negative oversized rows }; for (auto badconfig: fec_config_wrong) { - ASSERT_EQ(srt_setsockflag(sid1, SRTO_PACKETFILTER, badconfig, (int)strlen(badconfig)), -1); + cout << "CASE: " << badconfig << endl; + EXPECT_EQ(srt_setsockflag(sid1, SRTO_PACKETFILTER, badconfig, (int)strlen(badconfig)), -1); } TestMockCUDT m1; @@ -285,27 +290,27 @@ TEST(TestFEC, ConfigExchangeFaux) TEST(TestFEC, Connection) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,cols:10,rows:10"; const char fec_config2 [] = "fec,cols:10,arq:never"; const char fec_config_final [] = "fec,cols:10,rows:10,arq:never,layout:staircase"; - ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); - ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); + EXPECT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); + EXPECT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); - srt_listen(l, 1); + EXPECT_NE(srt_listen(l, 1), -1); auto connect_res = spawn_connect(s, sa, 1); @@ -316,7 +321,7 @@ TEST(TestFEC, Connection) // that 1s might not be enough. SRTSOCKET la[] = { l }; SRTSOCKET a = srt_accept_bond(la, 1, 5000); - ASSERT_NE(a, SRT_ERROR); + EXPECT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); // Now that the connection is established, check negotiated config @@ -336,35 +341,36 @@ TEST(TestFEC, Connection) EXPECT_TRUE(filterConfigSame(caller_config, fec_config_final)); EXPECT_TRUE(filterConfigSame(accept_config, fec_config_final)); - srt_close(a); - srt_close(s); - srt_close(l); + // Exceptionally blocked here to test "forgotten socket cleanup" additionally + //srt_close(a); + //srt_close(s); + //srt_close(l); } TEST(TestFEC, ConnectionReorder) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,cols:10,rows:10"; const char fec_config2 [] = "fec,rows:10,cols:10"; const char fec_config_final [] = "fec,cols:10,rows:10,arq:onreq,layout:staircase"; - ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); - ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); + EXPECT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); + EXPECT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); int conntimeo = 10000; - ASSERT_NE(srt_setsockflag(s, SRTO_CONNTIMEO, &conntimeo, sizeof (conntimeo)), SRT_ERROR); + EXPECT_NE(srt_setsockflag(s, SRTO_CONNTIMEO, &conntimeo, sizeof (conntimeo)), SRT_ERROR); srt_listen(l, 1); @@ -375,7 +381,7 @@ TEST(TestFEC, ConnectionReorder) SRTSOCKET la[] = { l }; SRTSOCKET a = srt_accept_bond(la, 1, 5000); - ASSERT_NE(a, SRT_ERROR); + EXPECT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); // Now that the connection is established, check negotiated config @@ -402,25 +408,25 @@ TEST(TestFEC, ConnectionReorder) TEST(TestFEC, ConnectionFull1) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,cols:10,rows:20,arq:never,layout:even"; const char fec_config2 [] = "fec,layout:even,rows:20,cols:10,arq:never"; const char fec_config_final [] = "fec,cols:10,rows:20,arq:never,layout:even"; - ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); - ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); + EXPECT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); + EXPECT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); srt_listen(l, 1); @@ -430,7 +436,7 @@ TEST(TestFEC, ConnectionFull1) SRTSOCKET la[] = { l }; SRTSOCKET a = srt_accept_bond(la, 1, 5000); - ASSERT_NE(a, SRT_ERROR); + EXPECT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); // Now that the connection is established, check negotiated config @@ -457,25 +463,25 @@ TEST(TestFEC, ConnectionFull1) TEST(TestFEC, ConnectionFull2) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,cols:10,rows:20,arq:always,layout:even"; const char fec_config2 [] = "fec,layout:even,rows:20,cols:10,arq:always"; const char fec_config_final [] = "fec,cols:10,rows:20,arq:always,layout:even"; - ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); - ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); + EXPECT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); + EXPECT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); srt_listen(l, 1); @@ -486,7 +492,7 @@ TEST(TestFEC, ConnectionFull2) SRTSOCKET la[] = { l }; SRTSOCKET a = srt_accept_bond(la, 1, 5000); - ASSERT_NE(a, SRT_ERROR); + EXPECT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); // Now that the connection is established, check negotiated config @@ -513,27 +519,27 @@ TEST(TestFEC, ConnectionFull2) TEST(TestFEC, ConnectionMess) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,cols:,cols:10"; const char fec_config2 [] = "fec,cols:,rows:10"; const char fec_config_final [] = "fec,cols:10,rows:10,arq:onreq,layout:staircase"; - ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); - ASSERT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); + EXPECT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); + EXPECT_NE(srt_setsockflag(l, SRTO_PACKETFILTER, fec_config2, (sizeof fec_config2)-1), -1); - srt_listen(l, 1); + EXPECT_NE(srt_listen(l, 1), -1); auto connect_res = spawn_connect(s, sa); @@ -542,7 +548,7 @@ TEST(TestFEC, ConnectionMess) SRTSOCKET la[] = { l }; SRTSOCKET a = srt_accept_bond(la, 1, 5000); - ASSERT_NE(a, SRT_ERROR) << srt_getlasterror_str(); + EXPECT_NE(a, SRT_ERROR) << srt_getlasterror_str(); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); // Now that the connection is established, check negotiated config @@ -569,23 +575,23 @@ TEST(TestFEC, ConnectionMess) TEST(TestFEC, ConnectionForced) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,rows:20,cols:20"; const char fec_config_final [] = "fec,cols:20,rows:20"; - ASSERT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); + EXPECT_NE(srt_setsockflag(s, SRTO_PACKETFILTER, fec_config1, (sizeof fec_config1)-1), -1); srt_listen(l, 1); @@ -596,7 +602,7 @@ TEST(TestFEC, ConnectionForced) SRTSOCKET la[] = { l }; SRTSOCKET a = srt_accept_bond(la, 1, 5000); - ASSERT_NE(a, SRT_ERROR); + EXPECT_NE(a, SRT_ERROR); EXPECT_EQ(connect_res.get(), SRT_SUCCESS); // Now that the connection is established, check negotiated config @@ -619,17 +625,17 @@ TEST(TestFEC, ConnectionForced) TEST(TestFEC, RejectionConflict) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,cols:10,rows:10"; @@ -664,17 +670,17 @@ TEST(TestFEC, RejectionConflict) TEST(TestFEC, RejectionIncompleteEmpty) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,rows:10"; @@ -707,17 +713,17 @@ TEST(TestFEC, RejectionIncompleteEmpty) TEST(TestFEC, RejectionIncomplete) { - srt::TestInit srtinit; - - SRTSOCKET s = srt_create_socket(); - SRTSOCKET l = srt_create_socket(); - sockaddr_in sa; memset(&sa, 0, sizeof sa); sa.sin_family = AF_INET; sa.sin_port = htons(5555); ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr), 1); + srt::TestInit srtinit; + + SRTSOCKET s = srt_create_socket(); + SRTSOCKET l = srt_create_socket(); + srt_bind(l, (sockaddr*)& sa, sizeof(sa)); const char fec_config1 [] = "fec,rows:10"; From d99d2e1a3b1a213b03c7dfbea5133898935fdeea Mon Sep 17 00:00:00 2001 From: cmollahan <43041498+cmollahan@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:45:09 -0500 Subject: [PATCH 04/11] [doc] Update README.md (#3356) Updated link to SRT Alliance Deployment Guide --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9d6a6bcc6c..7e8c654462 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ In live streaming configurations, the SRT protocol maintains a constant end-to-e |:-----------------------------------------------------------------------------------------------------------------------------:|:------------------------------------------------------------------------------------:|:---------------------------------------------------------------------------------:| | [The SRT API](./docs#srt-api-documents) | [IETF Internet Draft](https://datatracker.ietf.org/doc/html/draft-sharabayko-srt-01) | [Sample Apps](./docs#sample-applications) | | Reference documentation for the SRT library API | The SRT Protocol Internet Draft | Instructions for using test apps (`srt-live-transmit`, `srt-file-transmit`, etc.) | -| [SRT Technical Overview](https://github.com/Haivision/srt/files/2489142/SRT_Protocol_TechnicalOverview_DRAFT_2018-10-17.pdf) | [SRT Deployment Guide](https://www.srtalliance.org/srt-deployment-guide/) | [SRT CookBook](https://srtlab.github.io/srt-cookbook) | +| [SRT Technical Overview](https://github.com/Haivision/srt/files/2489142/SRT_Protocol_TechnicalOverview_DRAFT_2018-10-17.pdf) | [SRT Deployment Guide](https://www3.haivision.com/srt-deployment-guide/) | [SRT CookBook](https://srtlab.github.io/srt-cookbook) | | Early draft technical overview (precursor to the Internet Draft) | A comprehensive overview of the protocol with deployment guidelines | Development notes on the SRT protocol | | [Innovation Labs Blog](https://medium.com/innovation-labs-blog/tagged/secure-reliable-transport) | [SRTLab YouTube Channel](https://www.youtube.com/channel/UCr35JJ32jKKWIYymR1PTdpA) | [Slack](https://srtalliance.slack.com) | | The blog on Medium with SRT-related technical articles | Technical YouTube channel with useful videos | Slack channels to get the latest updates and ask questions
[Join SRT Alliance on Slack](https://slackin-srtalliance.azurewebsites.net/) | From 899348d8318eb9a3c5a5b6ec43c4a1114288773a Mon Sep 17 00:00:00 2001 From: cl-ment Date: Fri, 28 Aug 2026 08:51:08 +0200 Subject: [PATCH 05/11] Change to version 1.5.7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clément Gérouville --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e10715a258..4933de247b 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ # cmake_minimum_required (VERSION 3.5 FATAL_ERROR) -set (SRT_VERSION 1.5.6) +set (SRT_VERSION 1.5.7) set (CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/scripts") include(CheckSymbolExists) From 4bd4a86d90b8cc83f69cfa47d95aa71e3bf21590 Mon Sep 17 00:00:00 2001 From: Andres Cera Date: Sun, 20 Sep 2026 06:08:52 -0500 Subject: [PATCH 06/11] feat(srtcore): SRTO_PERIODICNAKGATE tri-state (off/filter/suppress) --- apps/socketoptions.hpp | 1 + srtcore/core.cpp | 88 ++++++++++- srtcore/core.h | 2 + srtcore/socketconfig.cpp | 13 ++ srtcore/socketconfig.h | 2 + srtcore/srt.h | 8 + test/filelist.maf | 1 + test/test_periodic_nak_gate.cpp | 258 ++++++++++++++++++++++++++++++++ 8 files changed, 371 insertions(+), 2 deletions(-) create mode 100644 test/test_periodic_nak_gate.cpp diff --git a/apps/socketoptions.hpp b/apps/socketoptions.hpp index ee8f1595c5..11399ec0c2 100644 --- a/apps/socketoptions.hpp +++ b/apps/socketoptions.hpp @@ -235,6 +235,7 @@ const SocketOption srt_options [] { { "tlpktdrop", 0, SRTO_TLPKTDROP, SocketOption::PRE, SocketOption::BOOL, nullptr}, { "snddropdelay", 0, SRTO_SNDDROPDELAY, SocketOption::POST, SocketOption::INT, nullptr}, { "nakreport", 0, SRTO_NAKREPORT, SocketOption::PRE, SocketOption::BOOL, nullptr}, + { "periodicnakgate", 0, SRTO_PERIODICNAKGATE, SocketOption::PRE, SocketOption::INT, nullptr}, { "conntimeo", 0, SRTO_CONNTIMEO, SocketOption::PRE, SocketOption::INT, nullptr}, { "drifttracer", 0, SRTO_DRIFTTRACER, SocketOption::POST, SocketOption::BOOL, nullptr}, { "lossmaxttl", 0, SRTO_LOSSMAXTTL, SocketOption::POST, SocketOption::INT, nullptr}, diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 478d266ed9..76c72e1618 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -186,6 +186,7 @@ struct SrtOptionAction flags[SRTO_VERSION] = SRTO_R_PRE; flags[SRTO_CONNTIMEO] = SRTO_R_PRE; flags[SRTO_LOSSMAXTTL] = SRTO_POST_SPEC; + flags[SRTO_PERIODICNAKGATE] = SRTO_R_PRE; flags[SRTO_REORDERFREEZE] = SRTO_R_PRE; flags[SRTO_RCVLATENCY] = SRTO_R_PRE; flags[SRTO_PEERLATENCY] = SRTO_R_PRE; @@ -876,6 +877,11 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen) optlen = sizeof(bool); break; + case SRTO_PERIODICNAKGATE: + *(int32_t *)optval = m_config.iPeriodicNakGate; + optlen = sizeof(int32_t); + break; + case SRTO_REORDERFREEZE: *(bool *)optval = m_config.bReorderFreeze; optlen = sizeof(bool); @@ -12084,6 +12090,66 @@ int srt::CUDT::checkACKTimer(const steady_clock::time_point &currtime) return because_decision; } +static bool freshLossRangeLess(const pair& a, const pair& b) +{ + return CSeqNo::seqcmp(a.first, b.first) < 0; +} + +// CERALIVE periodic-NAK gate arm 1 ("filter"). Emits the receiver loss list with +// every range that is still inside its reorder TTL (m_FreshLoss) subtracted, so a +// merely-reordered sequence is not reported as lost. Both containers are ordered +// by sequence, so the subtraction is a single merge walk rather than a per-sequence +// membership test. +void srt::CUDT::buildFilteredLossReport(vector& w_lossdata) +{ + ScopedLock lk(m_RcvLossLock); + + FixedArray arr(m_iMaxDataPayloadSize / sizeof(int32_t)); + const int arrlen = m_pRcvLossList->getLossArray(arr); + + vector > fresh; + fresh.reserve(m_FreshLoss.size()); + for (size_t k = 0; k < m_FreshLoss.size(); ++k) + fresh.push_back(make_pair(m_FreshLoss[k].seq[0], m_FreshLoss[k].seq[1])); + sort(fresh.begin(), fresh.end(), freshLossRangeLess); + + for (int n = 0; n < arrlen; ) + { + int32_t lo, hi; + if (arr[n] & LOSSDATA_SEQNO_RANGE_FIRST) + { + lo = arr[n] & ~LOSSDATA_SEQNO_RANGE_FIRST; + hi = arr[n + 1]; + n += 2; + } + else + { + lo = hi = arr[n]; + n += 1; + } + + int32_t cur = lo; + for (size_t f = 0; f < fresh.size(); ++f) + { + const int32_t f_lo = fresh[f].first; + const int32_t f_hi = fresh[f].second; + if (CSeqNo::seqcmp(f_hi, cur) < 0) + continue; + if (CSeqNo::seqcmp(f_lo, hi) > 0) + break; + if (CSeqNo::seqcmp(f_lo, cur) > 0) + addLossRecord(w_lossdata, cur, CSeqNo::decseq(f_lo)); + const int32_t next = CSeqNo::incseq(f_hi); + if (CSeqNo::seqcmp(next, cur) > 0) + cur = next; + if (CSeqNo::seqcmp(cur, hi) > 0) + break; + } + if (CSeqNo::seqcmp(cur, hi) <= 0) + addLossRecord(w_lossdata, cur, hi); + } +} + int srt::CUDT::checkNAKTimer(const steady_clock::time_point& currtime) { // XXX The problem with working NAKREPORT with SRT_ARQ_ONREQ @@ -12118,8 +12184,26 @@ int srt::CUDT::checkNAKTimer(const steady_clock::time_point& currtime) if (currtime <= m_tsNextNAKTime.load()) return BECAUSE_NO_REASON; // wait for next NAK time - sendCtrl(UMSG_LOSSREPORT); - debug_decision = BECAUSE_NAKREPORT; + // CERALIVE periodic-NAK gate (SRTO_PERIODICNAKGATE). Arm 2 sends nothing + // from this site at all while still falling through to the timer advance + // below, reproducing irlserver/srt's SRTLAPATCHES behaviour exactly. + if (m_config.iPeriodicNakGate == 2) + { + debug_decision = BECAUSE_NAKREPORT; + } + else if (m_config.iPeriodicNakGate == 1) + { + vector lossdata; + buildFilteredLossReport((lossdata)); + if (!lossdata.empty()) + sendCtrl(UMSG_LOSSREPORT, NULL, &lossdata[0], (int)lossdata.size()); + debug_decision = BECAUSE_NAKREPORT; + } + else + { + sendCtrl(UMSG_LOSSREPORT); + debug_decision = BECAUSE_NAKREPORT; + } } m_tsNextNAKTime.store(currtime + m_tdNAKInterval); diff --git a/srtcore/core.h b/srtcore/core.h index fb9f733be8..37b9aff971 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -307,6 +307,7 @@ class CUDT friend class CUDTGroup; friend class TestMockCUDT; // unit tests friend class TestMockControlPackets; // unit tests + friend class TestMockPeriodicNakGate; // unit tests typedef sync::steady_clock::time_point time_point; typedef sync::steady_clock::duration duration; @@ -1390,6 +1391,7 @@ class CUDT void checkTimers(); void considerLegacySrtHandshake(const time_point &timebase); int checkACKTimer (const time_point& currtime); + void buildFilteredLossReport(std::vector& w_lossdata); int checkNAKTimer(const time_point& currtime); bool checkExpTimer (const time_point& currtime, int check_reason); // returns true if the connection is expired void checkRexmitTimer(const time_point& currtime); diff --git a/srtcore/socketconfig.cpp b/srtcore/socketconfig.cpp index e86cfb9098..582df2a231 100644 --- a/srtcore/socketconfig.cpp +++ b/srtcore/socketconfig.cpp @@ -568,6 +568,18 @@ struct CSrtConfigSetter } }; +template<> +struct CSrtConfigSetter +{ + static void set(CSrtConfig& co, const void* optval, int optlen) + { + const int val = cast_optval(optval, optlen); + if (val < 0 || val > 2) + throw CUDTException(MJ_NOTSUP, MN_INVAL, 0); + co.iPeriodicNakGate = val; + } +}; + template<> struct CSrtConfigSetter { @@ -971,6 +983,7 @@ int dispatchSet(SRT_SOCKOPT optName, CSrtConfig& co, const void* optval, int opt DISPATCH(SRTO_CONNTIMEO); DISPATCH(SRTO_DRIFTTRACER); DISPATCH(SRTO_LOSSMAXTTL); + DISPATCH(SRTO_PERIODICNAKGATE); DISPATCH(SRTO_REORDERFREEZE); DISPATCH(SRTO_MINVERSION); DISPATCH(SRTO_STREAMID); diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index 182aa38ef6..3552096209 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -268,6 +268,7 @@ struct CSrtConfig: CSrtMuxerConfig bool bRcvNakReport; // Enable Receiver Periodic NAK Reports int iMaxReorderTolerance; //< Maximum allowed value for dynamic reorder tolerance bool bReorderFreeze; // CERALIVE reorder-freeze: freeze reorder-tolerance decay (receiver-side opt-in) + int iPeriodicNakGate; // CERALIVE periodic-NAK gate: 0 = off, 1 = filter still-reorderable, 2 = suppress // For the use of CCryptoControl // HaiCrypt configuration @@ -323,6 +324,7 @@ struct CSrtConfig: CSrtMuxerConfig , bRcvNakReport(true) , iMaxReorderTolerance(0) // Sensible optimal value is 10, 0 preserves old behavior , bReorderFreeze(false) // Opt-in; default preserves stock adaptive decay + , iPeriodicNakGate(0) // Opt-in; default preserves stock periodic loss reporting , uKmRefreshRatePkt(0) , uKmPreAnnouncePkt(0) , uSrtVersion(SRT_DEF_VERSION) diff --git a/srtcore/srt.h b/srtcore/srt.h index a1995b06db..d94030f9fb 100644 --- a/srtcore/srt.h +++ b/srtcore/srt.h @@ -240,6 +240,14 @@ typedef enum SRT_SOCKOPT { SRTO_MAXREXMITBW = 63, // Maximum bandwidth limit for retransmision (Bytes/s) #endif + // CERALIVE periodic-NAK gate: receiver-side opt-in tri-state controlling the + // periodic (timer-driven) UMSG_LOSSREPORT. 0 = off (stock Haivision: report + // the whole receiver loss list), 1 = filter (drop sequences that are still + // within their reorder TTL and report the rest), 2 = suppress (send no + // periodic loss report at all; the NAK timer still advances). Appended HIGH to + // avoid colliding with future upstream option numbers; never gap-fill. + SRTO_PERIODICNAKGATE = 119, + // CERALIVE reorder-freeze: receiver-side opt-in to freeze the dynamic // reorder-tolerance decay (decoupled from SRTO_NAKREPORT). Appended HIGH to // avoid colliding with future upstream option numbers; never gap-fill. diff --git a/test/filelist.maf b/test/filelist.maf index e39d6ac592..0b20437bb2 100644 --- a/test/filelist.maf +++ b/test/filelist.maf @@ -18,6 +18,7 @@ test_ipv6.cpp test_listen_callback.cpp test_losslist_rcv.cpp test_losslist_snd.cpp +test_periodic_nak_gate.cpp test_many_connections.cpp test_muxer.cpp test_seqno.cpp diff --git a/test/test_periodic_nak_gate.cpp b/test/test_periodic_nak_gate.cpp new file mode 100644 index 0000000000..0af3eefada --- /dev/null +++ b/test/test_periodic_nak_gate.cpp @@ -0,0 +1,258 @@ +#include +#include + +#include "gtest/gtest.h" +#include "test_env.h" + +#include "srt.h" +#include "api.h" +#include "core.h" +#include "list.h" +#include "packet.h" + +using namespace srt; +using namespace srt::sync; + +namespace srt { + // Friend wrapper for unit tests that drive the private periodic-NAK decision. + // Declared as a friend in core.h. + class TestMockPeriodicNakGate + { + public: + CUDT* core; + + TestMockPeriodicNakGate(): core(NULL) {} + + int checkNAKTimer(const steady_clock::time_point& t) { return core->checkNAKTimer(t); } + + void buildFilteredLossReport(std::vector& out) { core->buildFilteredLossReport((out)); } + + void insertLoss(int32_t lo, int32_t hi) + { + ScopedLock lk(core->m_RcvLossLock); + core->m_pRcvLossList->insert(lo, hi); + } + + void addFreshLoss(int32_t lo, int32_t hi, int ttl) + { + ScopedLock lk(core->m_RcvLossLock); + core->m_FreshLoss.push_back(CRcvFreshLoss(lo, hi, ttl)); + } + + int lossLength() + { + ScopedLock lk(core->m_RcvLossLock); + return core->m_pRcvLossList->getLossLength(); + } + + uint32_t sentNakTotal() const { return core->m_stats.rcvr.sentNak.total.count(); } + + void setGate(int v) { core->m_config.iPeriodicNakGate = v; } + + void setNextNakTime(const steady_clock::time_point& t) { core->m_tsNextNAKTime.store(t); } + steady_clock::time_point nextNakTime() const { return core->m_tsNextNAKTime.load(); } + + void detachFromQueues() { core->m_bConnected = false; } + void reattachToQueues() { core->m_bConnected = true; } + }; +} + +namespace { + +const int kIntervals = 5; +const char kListenHost[] = "localhost"; +const int kListenPort = 5565; + +// Encoded form of a multi-sequence loss range as produced by CUDT::addLossRecord. +std::vector lossRange(int32_t lo, int32_t hi) +{ + std::vector v; + if (lo == hi) + { + v.push_back(lo); + return v; + } + v.push_back(lo | LOSSDATA_SEQNO_RANGE_FIRST); + v.push_back(hi); + return v; +} + +void appendRange(std::vector& v, int32_t lo, int32_t hi) +{ + const std::vector r = lossRange(lo, hi); + v.insert(v.end(), r.begin(), r.end()); +} + +} // namespace + +class PeriodicNakGate: public srt::Test +{ +public: + SRTSOCKET caller = SRT_INVALID_SOCK; + SRTSOCKET listener = SRT_INVALID_SOCK; + SRTSOCKET accepted = SRT_INVALID_SOCK; + CUDTSocket* pcaller = NULL; + TestMockPeriodicNakGate mock; + + static void swipe(SRTSOCKET& sockid) + { + if (sockid == SRT_INVALID_SOCK) + return; + + EXPECT_NE(srt_close(sockid), SRT_ERROR); + sockid = SRT_INVALID_SOCK; + } + + void setup() override + { + caller = CUDT::uglobal().newSocket(&pcaller); + ASSERT_NE(caller, SRT_INVALID_SOCK); + mock.core = &pcaller->core(); + + ASSERT_NE(listener = srt_create_socket(), SRT_INVALID_SOCK); + + srt::sockaddr_any sa = srt::CreateAddr(kListenHost, kListenPort, AF_INET); + ASSERT_NE(srt_bind(listener, sa.get(), sa.size()), SRT_ERROR); + ASSERT_NE(srt_listen(listener, 1), SRT_ERROR); + + std::thread spawned_connect([this, &sa] { EXPECT_NE(srt_connect(caller, sa.get(), sa.size()), SRT_ERROR); }); + + accepted = srt_accept(listener, NULL, 0); + spawned_connect.join(); + ASSERT_NE(accepted, SRT_ERROR); + } + + // Both CRcvQueue paths that call CUDT::checkTimers() refuse a socket whose + // m_bConnected is false, so clearing it makes this socket's NAK timer fire + // only when the test fires it. Without that, the queue worker's own ~10ms + // checkTimers() tick would race every count taken here. + void detachFromQueues() + { + mock.detachFromQueues(); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + + // Drives the periodic-NAK decision exactly `count` times, forcing the NAK + // timer to be due before each call, and returns the number of LOSSREPORT + // packets the send path actually emitted. + uint32_t runDueNakTimers(int count) + { + const uint32_t before = mock.sentNakTotal(); + for (int i = 0; i < count; ++i) + { + const steady_clock::time_point now = steady_clock::now(); + mock.setNextNakTime(now - milliseconds_from(1000)); + mock.checkNAKTimer(now); + EXPECT_GT(mock.nextNakTime(), now) << "the NAK timer must advance on every due tick"; + } + return mock.sentNakTotal() - before; + } + + void teardown() override + { + if (mock.core != NULL) + mock.reattachToQueues(); + swipe(caller); + swipe(accepted); + swipe(listener); + } +}; + +TEST(PeriodicNakGateOption, SetGetRoundTripAndRejection) +{ + srt::TestInit srtinit; + + MAKE_UNIQUE_SOCK(sock, "periodicnakgate round trip", srt_create_socket()); + ASSERT_NE(sock.ref(), SRT_INVALID_SOCK); + + int val = -1; + int len = sizeof val; + ASSERT_EQ(srt_getsockopt(sock, 0, SRTO_PERIODICNAKGATE, &val, &len), SRT_SUCCESS); + EXPECT_EQ(val, 0) << "SRTO_PERIODICNAKGATE must default to off"; + + for (int accepted_value = 0; accepted_value <= 2; ++accepted_value) + { + ASSERT_EQ(srt_setsockopt(sock, 0, SRTO_PERIODICNAKGATE, &accepted_value, sizeof accepted_value), SRT_SUCCESS) + << "SRTO_PERIODICNAKGATE must accept " << accepted_value; + + val = -1; + len = sizeof val; + ASSERT_EQ(srt_getsockopt(sock, 0, SRTO_PERIODICNAKGATE, &val, &len), SRT_SUCCESS); + EXPECT_EQ(val, accepted_value); + EXPECT_EQ(len, (int) sizeof(int)); + } + + const int three = 3; + EXPECT_EQ(srt_setsockopt(sock, 0, SRTO_PERIODICNAKGATE, &three, sizeof three), SRT_ERROR); + EXPECT_EQ(srt_getlasterror(NULL), SRT_EINVPARAM); + + const int negative = -1; + EXPECT_EQ(srt_setsockopt(sock, 0, SRTO_PERIODICNAKGATE, &negative, sizeof negative), SRT_ERROR); + EXPECT_EQ(srt_getlasterror(NULL), SRT_EINVPARAM); + + val = -1; + len = sizeof val; + ASSERT_EQ(srt_getsockopt(sock, 0, SRTO_PERIODICNAKGATE, &val, &len), SRT_SUCCESS); + EXPECT_EQ(val, 2) << "a rejected value must leave the last accepted one in place"; +} + +// Arm 2 is the upstream-exact suppress semantics: with a non-empty receiver loss +// list and a due NAK timer, the periodic site emits no LOSSREPORT at all, while +// the NAK timer still advances on every tick. Arm 0 (the default) is the control. +TEST_F(PeriodicNakGate, SuppressArmEmitsNoLossReports) +{ + detachFromQueues(); + + mock.insertLoss(1000, 1010); + ASSERT_GT(mock.lossLength(), 0); + + mock.setGate(2); + EXPECT_EQ(runDueNakTimers(kIntervals), 0u) + << "SRTO_PERIODICNAKGATE=2 must emit no periodic LOSSREPORT"; + + ASSERT_GT(mock.lossLength(), 0) << "the loss list must still be non-empty for the control arm"; + + mock.setGate(0); + EXPECT_EQ(runDueNakTimers(kIntervals), (uint32_t) kIntervals) + << "SRTO_PERIODICNAKGATE=0 must emit one periodic LOSSREPORT per due tick"; +} + +// Arm 1 subtracts the ranges that are still inside their reorder TTL and reports +// what is left, so a partially-reorderable loss list still produces a LOSSREPORT +// while a fully-reorderable one produces none. +TEST_F(PeriodicNakGate, FilterArmSubtractsFreshLossAndSendsTheRest) +{ + detachFromQueues(); + + mock.insertLoss(1000, 1010); + ASSERT_GT(mock.lossLength(), 0); + + std::vector unfiltered; + mock.buildFilteredLossReport(unfiltered); + std::vector whole_range; + appendRange(whole_range, 1000, 1010); + EXPECT_EQ(unfiltered, whole_range) << "with no fresh loss the whole range must be reported"; + + mock.addFreshLoss(1003, 1005, 20); + + std::vector filtered; + mock.buildFilteredLossReport(filtered); + + std::vector expected; + appendRange(expected, 1000, 1002); + appendRange(expected, 1006, 1010); + EXPECT_EQ(filtered, expected) << "the still-reorderable range must be subtracted, the rest kept"; + + mock.setGate(1); + EXPECT_EQ(runDueNakTimers(kIntervals), (uint32_t) kIntervals) + << "a partially-filtered loss list must still be reported"; + + mock.addFreshLoss(1000, 1010, 20); + + std::vector all_fresh; + mock.buildFilteredLossReport(all_fresh); + EXPECT_TRUE(all_fresh.empty()) << "a fully-reorderable loss list must filter down to nothing"; + + EXPECT_EQ(runDueNakTimers(kIntervals), 0u) + << "nothing left after filtering means no LOSSREPORT is sent"; +} From 5d6f591e19a5bf1222004a10ea07cab8135c0a82 Mon Sep 17 00:00:00 2001 From: Andres Cera Date: Sun, 20 Sep 2026 06:18:08 -0500 Subject: [PATCH 07/11] feat(srtcore): SRTO_SRTLAPATCHES compat enumerator mapping to CERALIVE options --- apps/socketoptions.hpp | 1 + docs/API/API-socket-options.md | 81 +++++++++++++++++++++++ docs/CERALIVE-PATCHES.md | 53 +++++++++++++-- srtcore/core.cpp | 6 ++ srtcore/socketconfig.cpp | 12 ++++ srtcore/socketconfig.h | 3 + srtcore/srt.h | 3 + test/filelist.maf | 1 + test/test_srtlapatches.cpp | 117 +++++++++++++++++++++++++++++++++ 9 files changed, 272 insertions(+), 5 deletions(-) create mode 100644 test/test_srtlapatches.cpp diff --git a/apps/socketoptions.hpp b/apps/socketoptions.hpp index 11399ec0c2..5498754cc7 100644 --- a/apps/socketoptions.hpp +++ b/apps/socketoptions.hpp @@ -235,6 +235,7 @@ const SocketOption srt_options [] { { "tlpktdrop", 0, SRTO_TLPKTDROP, SocketOption::PRE, SocketOption::BOOL, nullptr}, { "snddropdelay", 0, SRTO_SNDDROPDELAY, SocketOption::POST, SocketOption::INT, nullptr}, { "nakreport", 0, SRTO_NAKREPORT, SocketOption::PRE, SocketOption::BOOL, nullptr}, + { "srtlapatches", 0, SRTO_SRTLAPATCHES, SocketOption::PRE, SocketOption::BOOL, nullptr}, { "periodicnakgate", 0, SRTO_PERIODICNAKGATE, SocketOption::PRE, SocketOption::INT, nullptr}, { "conntimeo", 0, SRTO_CONNTIMEO, SocketOption::PRE, SocketOption::INT, nullptr}, { "drifttracer", 0, SRTO_DRIFTTRACER, SocketOption::POST, SocketOption::BOOL, nullptr}, diff --git a/docs/API/API-socket-options.md b/docs/API/API-socket-options.md index aaa1401abb..cd50a9f53d 100644 --- a/docs/API/API-socket-options.md +++ b/docs/API/API-socket-options.md @@ -238,6 +238,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`SRTO_PEERIDLETIMEO`](#SRTO_PEERIDLETIMEO) | 1.3.3 | pre | `int32_t` | ms | 5000 | 0.. | RW | GSD+ | | [`SRTO_PEERLATENCY`](#SRTO_PEERLATENCY) | 1.3.0 | pre | `int32_t` | ms | 0 | 0.. | RW | GSD | | [`SRTO_PEERVERSION`](#SRTO_PEERVERSION) | 1.1.0 | | `int32_t` | * | | | R | GS | +| [`SRTO_PERIODICNAKGATE`](#SRTO_PERIODICNAKGATE) | Ceralive | pre | `int32_t` | | 0 | [0, 2] | RW | GSD | | [`SRTO_RCVBUF`](#SRTO_RCVBUF) | | pre-bind | `int32_t` | bytes | 8192 payloads | \* | RW | GSD+ | | [`SRTO_RCVDATA`](#SRTO_RCVDATA) | | | `int32_t` | pkts | | | R | S | | [`SRTO_RCVKMSTATE`](#SRTO_RCVKMSTATE) | 1.2.0 | | `int32_t` | enum | | | R | S | @@ -245,6 +246,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`SRTO_RCVSYN`](#SRTO_RCVSYN) | | post | `bool` | | true | | RW | GSI | | [`SRTO_RCVTIMEO`](#SRTO_RCVTIMEO) | | post | `int32_t` | ms | -1 | -1, 0.. | RW | GSI | | [`SRTO_RENDEZVOUS`](#SRTO_RENDEZVOUS) | | pre | `bool` | | false | | RW | S | +| [`SRTO_REORDERFREEZE`](#SRTO_REORDERFREEZE) | Ceralive | pre | `bool` | | false | | W | GSD | | [`SRTO_RETRANSMITALGO`](#SRTO_RETRANSMITALGO) | 1.4.2 | pre | `int32_t` | | 1 | [0, 1] | RW | GSD | | [`SRTO_REUSEADDR`](#SRTO_REUSEADDR) | | pre-bind | `bool` | | true | | RW | GSD | | [`SRTO_SENDER`](#SRTO_SENDER) | 1.0.4 | pre | `bool` | | false | | W | S | @@ -254,6 +256,7 @@ The following table lists SRT API socket options in alphabetical order. Option d | [`SRTO_SNDKMSTATE`](#SRTO_SNDKMSTATE) | 1.2.0 | | `int32_t` | enum | | | R | S | | [`SRTO_SNDSYN`](#SRTO_SNDSYN) | | post | `bool` | | true | | RW | GSI | | [`SRTO_SNDTIMEO`](#SRTO_SNDTIMEO) | | post | `int32_t` | ms | -1 | -1.. | RW | GSI | +| [`SRTO_SRTLAPATCHES`](#SRTO_SRTLAPATCHES) | Ceralive | pre | `bool` | | false | | RW | GSD | | [`SRTO_STATE`](#SRTO_STATE) | | | `int32_t` | enum | | | R | S | | [`SRTO_STREAMID`](#SRTO_STREAMID) | 1.3.0 | pre | `string` | | "" | [512] | RW | GSD | | [`SRTO_TLPKTDROP`](#SRTO_TLPKTDROP) | 1.0.6 | pre | `bool` | | \* | | RW | GSD | @@ -1290,6 +1293,34 @@ See [`SRTO_VERSION`](#SRTO_VERSION) for the version format. --- +#### SRTO_PERIODICNAKGATE + +| OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | +| ------------------------ | -------- | -------- | ---------- | ------- | ------- | ------ | --- | ------ | +| `SRTO_PERIODICNAKGATE` | Ceralive | pre | `int32_t` | | 0 | [0, 2] | RW | GSD | + +**CeraLive extension** (`SRTO_PERIODICNAKGATE = 119`, value code in +`srtcore/srt.h`). Controls the receiver-side **periodic (timer-driven) loss +report** — the `UMSG_LOSSREPORT` that libsrt emits on each NAK timer expiry, +independently of an explicit NAK report request. + +- `0` — **off** (stock Haivision behaviour): report the entire receiver loss + list on every due NAK tick. +- `1` — **filter**: subtract the sequence ranges that are still inside their + reorder TTL and report only what remains, so a merely-reordered sequence is + not reported as lost. +- `2` — **suppress**: send no periodic loss report from this site at all, + while the NAK timer still advances. This is behaviourally identical to + `irlserver/srt`'s `SRTLAPATCHES` suppression and is the arm the D10 A/B + measures. + +Out-of-range values are rejected with `SRT_EINVPARAM`. The option is +receiver-side, opt-in, and inherited by accepted sockets from the listener. + +[Return to list](#list-of-options) + +--- + #### SRTO_RCVBUF | OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | @@ -1448,6 +1479,27 @@ procedure of `srt_bind` and then `srt_connect` (or `srt_rendezvous`) to one anot --- +#### SRTO_REORDERFREEZE + +| OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | +| ----------------------- | -------- | -------- | -------- | ------- | ------- | ------ | --- | ------ | +| `SRTO_REORDERFREEZE` | Ceralive | pre | `bool` | | false | | W | GSD | + +**CeraLive extension** (`SRTO_REORDERFREEZE = 120`, value code in +`srtcore/srt.h`). Freezes the receiver-side dynamic **reorder-tolerance decay**, +so a receiver on a deliberately-out-of-order (bonded/SRTLA) ingest can hold +reorder tolerance at its maximum instead of having stock adaptive decay drive it +toward zero on a clean ordered stream and trigger spurious retransmissions. + +It freezes only the decay — it does not change `SRTO_LOSSMAXTTL` +(`initial_loss_ttl`), is orthogonal to [`SRTO_NAKREPORT`](#SRTO_NAKREPORT), and +is a no-op on senders. Opt-in (default off) and inherited by accepted sockets +from the receive listener. + +[Return to list](#list-of-options) + +--- + #### SRTO_RETRANSMITALGO | OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | @@ -1628,6 +1680,35 @@ if in "non-blocking mode". The -1 value means no time limit. --- +#### SRTO_SRTLAPATCHES + +| OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | +| ----------------------- | -------- | -------- | -------- | ------- | ------- | ------ | --- | ------ | +| `SRTO_SRTLAPATCHES` | Ceralive | pre | `bool` | | false | | RW | GSD | + +**CeraLive compatibility enumerator** (`SRTO_SRTLAPATCHES = 118`, value code in +`srtcore/srt.h`). It reproduces `irlserver/srt`'s `SRTLAPATCHES` semantics as a +shim over the two options above, so a consumer written against that fork keeps +working against this SRT. + +Only **0 / non-zero** are meaningful (bool-like; a non-zero `int` is accepted): + +- non-zero ⇒ `SRTO_REORDERFREEZE = true` **and** `SRTO_PERIODICNAKGATE` set to + its D10 default (`SRTLA_PATCHES_DEFAULT_NAKGATE`, initially `2` = + upstream-exact suppress); the getter reads `true`. +- zero ⇒ both `SRTO_REORDERFREEZE = false` and `SRTO_PERIODICNAKGATE = 0`; the + getter reads `false`. + +The getter is the conjunction `bReorderFreeze && iPeriodicNakGate != 0`, so +writing either underlying option afterwards overrides this option's earlier +effect (last write wins). The three CeraLive option numbers are `118` +(`SRTO_SRTLAPATCHES`), `119` (`SRTO_PERIODICNAKGATE`), and `120` +(`SRTO_REORDERFREEZE`). + +[Return to list](#list-of-options) + +--- + #### SRTO_STATE | OptName | Since | Restrict | Type | Units | Default | Range | Dir | Entity | diff --git a/docs/CERALIVE-PATCHES.md b/docs/CERALIVE-PATCHES.md index 5bca8a2507..dd991dc45f 100644 --- a/docs/CERALIVE-PATCHES.md +++ b/docs/CERALIVE-PATCHES.md @@ -11,12 +11,14 @@ never by rebase/replay, so every upstream tag remains fully contained in the for history and the merge-base keeps advancing on each catch-up. The most recent sync is upstream **v1.5.7** (`899348d`, KMREQ and encryption-state validation, ACK and DROPREQ validation, FEC bounds, bonding BACKUP lifetime safety, and sample-tool path -validation). Both CeraLive patches below are retained. The published package and the +validation). All CeraLive patches below are retained. The published package and the immutable ABI comparison baseline remain `srt-v1.5.6+ceralive.1` pending the separate release cutover. -There are exactly **two** functional CeraLive patches to the C/C++ source. Everything -else the fork carries is packaging and CI (documented at the end for completeness). +The functional CeraLive changes to the C/C++ source are the three socket options +(`SRTO_REORDERFREEZE = 120`, `SRTO_PERIODICNAKGATE = 119`, `SRTO_SRTLAPATCHES = 118`) +and the deterministic socket-teardown fix. Everything else the fork carries is +packaging and CI (documented at the end for completeness). --- @@ -43,7 +45,48 @@ this as the **only** patch needed for BELABOX-parity baseline ("C is SAFE"). --- -## 2. Deterministic socket teardown +## 2. `SRTO_PERIODICNAKGATE` — periodic loss-report tri-state + +- **Type:** new receiver-side socket option, **default off** + (`SRTO_PERIODICNAKGATE = 119`). +- **Where:** `srtcore/srt.h` (enum), `srtcore/socketconfig.{h,cpp}` + (`CSrtConfig::iPeriodicNakGate` + setter), `srtcore/core.cpp` (the NAK-timer + decision, tagged `// CERALIVE periodic-NAK gate`), + `test/test_periodic_nak_gate.cpp` (tests). + +**Rationale.** Stock libsrt emits a `UMSG_LOSSREPORT` on every due NAK timer, +reporting the whole receiver loss list. On a deliberately-reordered bonded ingest +this re-reports sequences that are merely late. The option is a tri-state: +`0` = off (stock), `1` = filter (subtract ranges still inside their reorder TTL, +report the rest), `2` = suppress (send nothing from this site; the NAK timer +still advances — behaviourally identical to `irlserver/srt`'s `SRTLAPATCHES` +suppression). Out-of-range values are rejected with `SRT_EINVPARAM`. Which arm +ships as the compat default is decided by the D10 A/B; until then the compat shim +below installs `2`. + +--- + +## 3. `SRTO_SRTLAPATCHES` — irlserver compat enumerator + +- **Type:** new bool-like compatibility socket option, **default off** + (`SRTO_SRTLAPATCHES = 118`). +- **Where:** `srtcore/srt.h` (enum), `srtcore/socketconfig.{h,cpp}` (setter that + maps onto `bReorderFreeze` + `iPeriodicNakGate`, plus the + `SRTLA_PATCHES_DEFAULT_NAKGATE` default), `srtcore/core.cpp` (the conjunctive + getter), `apps/socketoptions.hpp` (`srtlapatches` URI row), + `test/test_srtlapatches.cpp` (tests). + +**Rationale.** Owns the "one switch reproduces `irlserver/srt`'s `SRTLAPATCHES`" +contract without a second code path: non-zero sets `SRTO_REORDERFREEZE = true` +and `SRTO_PERIODICNAKGATE` to `SRTLA_PATCHES_DEFAULT_NAKGATE` (initially `2`), +zero clears both. The getter is `bReorderFreeze && iPeriodicNakGate != 0`, so +writing either underlying option afterwards overrides it (last write wins). Only +`0`/non-zero are meaningful. The `SRTLA_PATCHES_DEFAULT_NAKGATE` default is set +by the D10 A/B (plan `upstream-rebase-hard-fork` todo 38). + +--- + +## 4. Deterministic socket teardown - **Commit:** `293ae6f45bf116c56d056b3a25312b2aade7dade` (2026-07-13) — *fix(core): make socket teardown deterministic* @@ -93,5 +136,5 @@ C/C++ patches. When syncing upstream, keep this file current: a new CeraLive C/C++ patch **must** be added here with its commit SHA and a one-paragraph rationale, and a patch that is retired (e.g. superseded by an upstream fix) **must** be moved to a "Retired" note -rather than silently dropped. Any functional change beyond these two patches is out of +rather than silently dropped. Any functional change beyond these patches is out of scope for the fork (see [`AGENTS.md`](../AGENTS.md) → SCOPE BOUNDARY). diff --git a/srtcore/core.cpp b/srtcore/core.cpp index 76c72e1618..6829fa6678 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -186,6 +186,7 @@ struct SrtOptionAction flags[SRTO_VERSION] = SRTO_R_PRE; flags[SRTO_CONNTIMEO] = SRTO_R_PRE; flags[SRTO_LOSSMAXTTL] = SRTO_POST_SPEC; + flags[SRTO_SRTLAPATCHES] = SRTO_R_PRE; flags[SRTO_PERIODICNAKGATE] = SRTO_R_PRE; flags[SRTO_REORDERFREEZE] = SRTO_R_PRE; flags[SRTO_RCVLATENCY] = SRTO_R_PRE; @@ -877,6 +878,11 @@ void srt::CUDT::getOpt(SRT_SOCKOPT optName, void *optval, int &optlen) optlen = sizeof(bool); break; + case SRTO_SRTLAPATCHES: + *(bool *)optval = m_config.bReorderFreeze && m_config.iPeriodicNakGate != 0; + optlen = sizeof(bool); + break; + case SRTO_PERIODICNAKGATE: *(int32_t *)optval = m_config.iPeriodicNakGate; optlen = sizeof(int32_t); diff --git a/srtcore/socketconfig.cpp b/srtcore/socketconfig.cpp index 582df2a231..509f65d65e 100644 --- a/srtcore/socketconfig.cpp +++ b/srtcore/socketconfig.cpp @@ -580,6 +580,17 @@ struct CSrtConfigSetter } }; +template<> +struct CSrtConfigSetter +{ + static void set(CSrtConfig& co, const void* optval, int optlen) + { + const bool on = cast_optval(optval, optlen); + co.bReorderFreeze = on; + co.iPeriodicNakGate = on ? SRTLA_PATCHES_DEFAULT_NAKGATE : 0; + } +}; + template<> struct CSrtConfigSetter { @@ -983,6 +994,7 @@ int dispatchSet(SRT_SOCKOPT optName, CSrtConfig& co, const void* optval, int opt DISPATCH(SRTO_CONNTIMEO); DISPATCH(SRTO_DRIFTTRACER); DISPATCH(SRTO_LOSSMAXTTL); + DISPATCH(SRTO_SRTLAPATCHES); DISPATCH(SRTO_PERIODICNAKGATE); DISPATCH(SRTO_REORDERFREEZE); DISPATCH(SRTO_MINVERSION); diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index 3552096209..284af2df5f 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -72,6 +72,9 @@ written by static const int SRT_OHEAD_DEFAULT_P100 = 25; +// Set by the D10 A/B (plan upstream-rebase-hard-fork todo 38); 2 = upstream-exact until measured +constexpr int SRTLA_PATCHES_DEFAULT_NAKGATE = 2; + // NOTE: SRT_VERSION is primarily defined in the build file. extern const int32_t SRT_DEF_VERSION; diff --git a/srtcore/srt.h b/srtcore/srt.h index d94030f9fb..138f2173fd 100644 --- a/srtcore/srt.h +++ b/srtcore/srt.h @@ -240,6 +240,9 @@ typedef enum SRT_SOCKOPT { SRTO_MAXREXMITBW = 63, // Maximum bandwidth limit for retransmision (Bytes/s) #endif + // CERALIVE compat: irlserver/srt SRTLAPATCHES semantics via REORDERFREEZE + PERIODICNAKGATE + SRTO_SRTLAPATCHES = 118, + // CERALIVE periodic-NAK gate: receiver-side opt-in tri-state controlling the // periodic (timer-driven) UMSG_LOSSREPORT. 0 = off (stock Haivision: report // the whole receiver loss list), 1 = filter (drop sequences that are still diff --git a/test/filelist.maf b/test/filelist.maf index 0b20437bb2..9871cee4ee 100644 --- a/test/filelist.maf +++ b/test/filelist.maf @@ -19,6 +19,7 @@ test_listen_callback.cpp test_losslist_rcv.cpp test_losslist_snd.cpp test_periodic_nak_gate.cpp +test_srtlapatches.cpp test_many_connections.cpp test_muxer.cpp test_seqno.cpp diff --git a/test/test_srtlapatches.cpp b/test/test_srtlapatches.cpp new file mode 100644 index 0000000000..1e6822867c --- /dev/null +++ b/test/test_srtlapatches.cpp @@ -0,0 +1,117 @@ +/* + * SRT - Secure, Reliable, Transport + * + * CERALIVE compatibility option tests. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include + +#include "test_env.h" + +#include "socketconfig.h" +#include "srt.h" + +// SRTO_SRTLAPATCHES is a compatibility shim over SRTO_REORDERFREEZE (bool) and +// SRTO_PERIODICNAKGATE (tri-state): setting it non-zero turns both on with the +// D10 default gate, setting it zero clears both. Only 0/non-zero are meaningful. +// Each underlying option may still be written afterwards; the last write wins. + +namespace { + +bool getSrtlaPatches(SRTSOCKET sock) +{ + bool val = false; + int len = sizeof val; + EXPECT_EQ(srt_getsockopt(sock, 0, SRTO_SRTLAPATCHES, &val, &len), SRT_SUCCESS); + return val; +} + +bool getReorderFreeze(SRTSOCKET sock) +{ + bool val = false; + int len = sizeof val; + EXPECT_EQ(srt_getsockopt(sock, 0, SRTO_REORDERFREEZE, &val, &len), SRT_SUCCESS); + return val; +} + +int getPeriodicNakGate(SRTSOCKET sock) +{ + int val = -1; + int len = sizeof val; + EXPECT_EQ(srt_getsockopt(sock, 0, SRTO_PERIODICNAKGATE, &val, &len), SRT_SUCCESS); + return val; +} + +} // namespace + +TEST(SrtlaPatchesOption, NonZeroEnablesReorderFreezeAndNakGate) +{ + srt::TestInit srtinit; + + MAKE_UNIQUE_SOCK(sock, "srtlapatches non-zero", srt_create_socket()); + ASSERT_NE(sock.ref(), SRT_INVALID_SOCK); + + EXPECT_FALSE(getSrtlaPatches(sock.ref())) << "SRTO_SRTLAPATCHES must default to off"; + + const int one = 1; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_SRTLAPATCHES, &one, sizeof one), SRT_SUCCESS); + + EXPECT_TRUE(getSrtlaPatches(sock.ref())) << "SRTO_SRTLAPATCHES=1 must read back as set"; + EXPECT_TRUE(getReorderFreeze(sock.ref())) << "SRTO_SRTLAPATCHES=1 must enable SRTO_REORDERFREEZE"; + EXPECT_EQ(getPeriodicNakGate(sock.ref()), SRTLA_PATCHES_DEFAULT_NAKGATE) + << "SRTO_SRTLAPATCHES=1 must install the D10 default gate"; + + // Only 0/non-zero are meaningful: any other non-zero int is accepted as "on". + const int two = 2; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_SRTLAPATCHES, &two, sizeof two), SRT_SUCCESS); + EXPECT_EQ(getPeriodicNakGate(sock.ref()), SRTLA_PATCHES_DEFAULT_NAKGATE); + EXPECT_TRUE(getReorderFreeze(sock.ref())); +} + +TEST(SrtlaPatchesOption, ZeroClearsBothUnderlyingOptions) +{ + srt::TestInit srtinit; + + MAKE_UNIQUE_SOCK(sock, "srtlapatches zero", srt_create_socket()); + ASSERT_NE(sock.ref(), SRT_INVALID_SOCK); + + const int one = 1; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_SRTLAPATCHES, &one, sizeof one), SRT_SUCCESS); + ASSERT_TRUE(getSrtlaPatches(sock.ref())); + + const int zero = 0; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_SRTLAPATCHES, &zero, sizeof zero), SRT_SUCCESS); + + EXPECT_FALSE(getSrtlaPatches(sock.ref())); + EXPECT_FALSE(getReorderFreeze(sock.ref())) << "SRTO_SRTLAPATCHES=0 must clear SRTO_REORDERFREEZE"; + EXPECT_EQ(getPeriodicNakGate(sock.ref()), 0) << "SRTO_SRTLAPATCHES=0 must clear SRTO_PERIODICNAKGATE"; +} + +TEST(SrtlaPatchesOption, ExplicitPeriodicNakGateAfterSrtlaPatchesWins) +{ + srt::TestInit srtinit; + + MAKE_UNIQUE_SOCK(sock, "srtlapatches override", srt_create_socket()); + ASSERT_NE(sock.ref(), SRT_INVALID_SOCK); + + const int one = 1; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_SRTLAPATCHES, &one, sizeof one), SRT_SUCCESS); + ASSERT_EQ(getPeriodicNakGate(sock.ref()), SRTLA_PATCHES_DEFAULT_NAKGATE); + + const int gate_override = 1; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_PERIODICNAKGATE, &gate_override, sizeof gate_override), SRT_SUCCESS); + + EXPECT_EQ(getPeriodicNakGate(sock.ref()), 1) + << "an explicit SRTO_PERIODICNAKGATE write after SRTO_SRTLAPATCHES must win"; + EXPECT_TRUE(getSrtlaPatches(sock.ref())) + << "the compat getter is a conjunction: freeze on AND gate non-zero"; + + const int gate_off = 0; + ASSERT_EQ(srt_setsockopt(sock.ref(), 0, SRTO_PERIODICNAKGATE, &gate_off, sizeof gate_off), SRT_SUCCESS); + EXPECT_FALSE(getSrtlaPatches(sock.ref())) + << "a zero gate makes the conjunction read false even with reorder-freeze still on"; +} From 51d500c428c8e618848ee63b16efef77959938c9 Mon Sep 17 00:00:00 2001 From: Andres Cera Date: Sun, 20 Sep 2026 06:41:25 -0500 Subject: [PATCH 08/11] fix(srtcore): declare SRTLA_PATCHES_DEFAULT_NAKGATE without constexpr `socketconfig.h` is compiled by the C++03 lane (.github/workflows/ubuntu-c++03.yml, -DUSE_CXX_STD=03 with -DCMAKE_COMPILE_WARNING_AS_ERROR=ON), where `constexpr` is rejected by -Werror=c++11-compat. Use `static const int`, the same form the adjacent SRT_OHEAD_DEFAULT_P100 uses, which is C++03-valid and equally a compile-time constant for the two consumers (socketconfig.cpp setter and test/test_srtlapatches.cpp). --- srtcore/socketconfig.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index 284af2df5f..1685270c9a 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -72,8 +72,10 @@ written by static const int SRT_OHEAD_DEFAULT_P100 = 25; -// Set by the D10 A/B (plan upstream-rebase-hard-fork todo 38); 2 = upstream-exact until measured -constexpr int SRTLA_PATCHES_DEFAULT_NAKGATE = 2; +// Set by the D10 A/B (plan upstream-rebase-hard-fork todo 38); 2 = upstream-exact until measured. +// Declared as `static const int` (not `constexpr`) because this header is compiled by the +// C++03 lane (.github/workflows/ubuntu-c++03.yml builds with -Werror=c++11-compat). +static const int SRTLA_PATCHES_DEFAULT_NAKGATE = 2; // NOTE: SRT_VERSION is primarily defined in the build file. extern const int32_t SRT_DEF_VERSION; From 6218234e4dea032e61a410c3e66a9266287340e6 Mon Sep 17 00:00:00 2001 From: Andres Cera Date: Sun, 20 Sep 2026 19:27:48 -0500 Subject: [PATCH 09/11] docs(srtcore): record the D10 A/B verdict for SRTLA_PATCHES_DEFAULT_NAKGATE The constant was introduced as a placeholder pinned to 2 pending the D10 A/B (plan upstream-rebase-hard-fork todos 36/38). That campaign has now run: 24/24 valid rows, 2 arms x 4 netem loss/reorder cells x 3 runs, no retries. Arm 1 (filter) won viewer-observed loss on 1 of 4 cells where the frozen rule requires at least 3, and the goodput guard held on all 4, so WINNER = 2. The measured winner equals the placeholder, so the value is unchanged and this commit is a no-op for the build. Only the comment changes, so that the default no longer reads as provisional and its provenance is explicit in the source. --- srtcore/socketconfig.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h index 1685270c9a..5e0ee8934c 100644 --- a/srtcore/socketconfig.h +++ b/srtcore/socketconfig.h @@ -72,7 +72,13 @@ written by static const int SRT_OHEAD_DEFAULT_P100 = 25; -// Set by the D10 A/B (plan upstream-rebase-hard-fork todo 38); 2 = upstream-exact until measured. +// D10 A/B confirmed 2 +// The value is no longer a placeholder: the D10 A/B (plan upstream-rebase-hard-fork +// todos 36/38) measured arm 1 (filter) against arm 2 (suppress, upstream-exact) over +// 24/24 valid rows (2 arms x 4 netem loss/reorder cells x 3 runs, no retries). Arm 1 won +// viewer-observed loss on 1 of 4 cells (the frozen rule needs >= 3) with the goodput +// guard holding on all 4, so the frozen rule resolves WINNER = 2. Released as +// libsrt1.5-ceralive 1.5.7+ceralive.2; see docs/CERALIVE-PATCHES.md section 2. // Declared as `static const int` (not `constexpr`) because this header is compiled by the // C++03 lane (.github/workflows/ubuntu-c++03.yml builds with -Werror=c++11-compat). static const int SRTLA_PATCHES_DEFAULT_NAKGATE = 2; From 99b87c78b9f9cb9434084ddbb6fe2116c2f5d1f7 Mon Sep 17 00:00:00 2001 From: Andres Cera Date: Sun, 20 Sep 2026 19:29:46 -0500 Subject: [PATCH 10/11] docs: release notes for libsrt1.5-ceralive 1.5.7+ceralive.2 Add a Releases section recording the SRTLA option set as it actually ships: Haivision base v1.5.7 (899348d, absorbed by true merge 7a6cc86), tag srt-v1.5.7+ceralive.2, both .deb filenames, and the additive-ABI statement. Two tables carry the contract downstream consumers depend on. The first gives the exact option numbers and semantics (118 SRTO_SRTLAPATCHES compat shim mapping onto 120 SRTO_REORDERFREEZE plus 119 SRTO_PERIODICNAKGATE = 2). The second is the four-site equivalence with upstream irlserver/srt SRTLAPATCHES=1: sites 1-3 exact, site 4 exact at the shipped default because SRTLA_PATCHES_DEFAULT_NAKGATE is 2, matching f2297192:srtcore/core.cpp:12018-12029. The D10 A/B is recorded as measured rather than pending, with its campaign shape, verdict and rule hash, and the three places that still described the default as provisional are corrected. Also corrects a documentation error found while writing the table: SRTO_REORDERFREEZE has no URI row in apps/socketoptions.hpp, so reorderfreeze= is not a supported URI spelling. Documented rather than added, since adding the row is a functional change outside this release. --- docs/CERALIVE-PATCHES.md | 115 +++++++++++++++++++++++++++++++++++---- 1 file changed, 104 insertions(+), 11 deletions(-) diff --git a/docs/CERALIVE-PATCHES.md b/docs/CERALIVE-PATCHES.md index dd991dc45f..1f47178ea9 100644 --- a/docs/CERALIVE-PATCHES.md +++ b/docs/CERALIVE-PATCHES.md @@ -11,9 +11,11 @@ never by rebase/replay, so every upstream tag remains fully contained in the for history and the merge-base keeps advancing on each catch-up. The most recent sync is upstream **v1.5.7** (`899348d`, KMREQ and encryption-state validation, ACK and DROPREQ validation, FEC bounds, bonding BACKUP lifetime safety, and sample-tool path -validation). All CeraLive patches below are retained. The published package and the -immutable ABI comparison baseline remain `srt-v1.5.6+ceralive.1` pending the separate -release cutover. +validation). All CeraLive patches below are retained. That sync ships as the +`1.5.7+ceralive.2` release described under [Releases](#releases) below; the immutable +ABI comparison baseline used by `.github/workflows/abi.yml` stays at +`srt-v1.5.6+ceralive.1` (the previously shipped release) and is advanced deliberately, +one release behind, never in the same change that cuts a release. The functional CeraLive changes to the C/C++ source are the three socket options (`SRTO_REORDERFREEZE = 120`, `SRTO_PERIODICNAKGATE = 119`, `SRTO_SRTLAPATCHES = 118`) @@ -60,9 +62,15 @@ this re-reports sequences that are merely late. The option is a tri-state: `0` = off (stock), `1` = filter (subtract ranges still inside their reorder TTL, report the rest), `2` = suppress (send nothing from this site; the NAK timer still advances — behaviourally identical to `irlserver/srt`'s `SRTLAPATCHES` -suppression). Out-of-range values are rejected with `SRT_EINVPARAM`. Which arm -ships as the compat default is decided by the D10 A/B; until then the compat shim -below installs `2`. +suppression, `f2297192:srtcore/core.cpp:12018-12029`). Out-of-range values are +rejected with `SRT_EINVPARAM`. + +Which arm ships as the compat default was decided by the **D10 A/B**, now measured: +**the winner is `2`** (suppress, upstream-exact parity). See +[Releases](#releases) → `1.5.7+ceralive.2` for the campaign shape and the +verdict. The option itself remains explicitly settable to +`1` by any consumer that wants the filter arm; the A/B only fixed what the +`SRTO_SRTLAPATCHES` compat shim installs. --- @@ -78,11 +86,14 @@ below installs `2`. **Rationale.** Owns the "one switch reproduces `irlserver/srt`'s `SRTLAPATCHES`" contract without a second code path: non-zero sets `SRTO_REORDERFREEZE = true` -and `SRTO_PERIODICNAKGATE` to `SRTLA_PATCHES_DEFAULT_NAKGATE` (initially `2`), -zero clears both. The getter is `bReorderFreeze && iPeriodicNakGate != 0`, so -writing either underlying option afterwards overrides it (last write wins). Only -`0`/non-zero are meaningful. The `SRTLA_PATCHES_DEFAULT_NAKGATE` default is set -by the D10 A/B (plan `upstream-rebase-hard-fork` todo 38). +and `SRTO_PERIODICNAKGATE` to `SRTLA_PATCHES_DEFAULT_NAKGATE` (**`2`**, fixed by +the D10 A/B), zero clears both. The getter is `bReorderFreeze && iPeriodicNakGate +!= 0`, so writing either underlying option afterwards overrides it (last write +wins). Only `0`/non-zero are meaningful. + +Upstream `irlserver/srt` numbers its own `SRTLAPATCHES` **120** — the number +CeraLive already uses for `SRTO_REORDERFREEZE`. The names match, the numbers do +not, and numbers are ABI: never renumber any of the three. --- @@ -109,6 +120,82 @@ non-deterministic teardown surfaces as flaky reconnects. --- +## Releases + +Package name `libsrt1.5-ceralive`; Debian version `+ceralive.`; +git tag `srt-v`; GitHub release name `CeraLive SRT `. +The Debian revision `` counts CeraLive releases and is independent of the +Haivision base, so `1.5.7+ceralive.2` is the second CeraLive release, not a second +patch on `1.5.7`. `dpkg --compare-versions 1.5.7+ceralive.2 gt 1.5.6+ceralive.1` is +true, so an apt upgrade from the previous release is ordinary. + +### 1.5.7+ceralive.2 — the SRTLA option set + +- **Haivision base:** **v1.5.7** (`899348d`), absorbed by true merge (`7a6cc86`). +- **Tag:** `srt-v1.5.7+ceralive.2`. **Packages:** + `libsrt1.5-ceralive_1.5.7+ceralive.2_{arm64,amd64}.deb`. +- **ABI:** additive only. Three new `SRT_SOCKOPT` enumerators and one new + `CSrtConfig` field; no existing symbol, struct layout, or option number changed. + SONAME stays `libsrt.so.1.5`. `abi.yml` compares against the previous release + `srt-v1.5.6+ceralive.1`. + +**Socket options — exact numbers and semantics.** + +| Number | Enumerator | Type | Semantics | URI key | +|---|---|---|---|---| +| `118` | `SRTO_SRTLAPATCHES` | bool-like (`0` / non-zero) | **Compat shim** carrying `irlserver/srt`'s option name. Non-zero ⇒ `SRTO_REORDERFREEZE = true` **and** `SRTO_PERIODICNAKGATE = SRTLA_PATCHES_DEFAULT_NAKGATE` (**`2`**). Zero ⇒ both cleared. Getter is the conjunction `bReorderFreeze && iPeriodicNakGate != 0`. | `srtlapatches` | +| `119` | `SRTO_PERIODICNAKGATE` | `int32_t`, tri-state `[0,2]` | `0` off (stock Haivision: the periodic `UMSG_LOSSREPORT` is always sent). `1` filter (subtract ranges still inside their reorder TTL; report the remainder). `2` suppress (send nothing from this site; the NAK timer still advances). Anything else ⇒ `SRT_EINVPARAM`. | `periodicnakgate` | +| `120` | `SRTO_REORDERFREEZE` | bool | Freeze `m_iReorderTolerance` at `iMaxReorderTolerance` — no decay on ordered/early runs. Pre-existing CeraLive option, unchanged by this release. | *none* — API only, or reached via `srtlapatches` | + +All three are receiver-side, **default off**, `SRTO_R_PRE`, and inherited by accepted +sockets from the listener. `118` deliberately does **not** add a third code path: it +is a pure write-through onto `120` + `119`, so setting either of those *after* it +overrides it (last write wins). + +`apps/socketoptions.hpp` carries URI rows for `srtlapatches` and `periodicnakgate` +only. `SRTO_REORDERFREEZE` has never had one, so `srt://…?reorderfreeze=1` is not a +supported spelling in the bundled sample tools; set it through the C API, or turn it +on together with the NAK gate via `?srtlapatches=1`. + +**Equivalence with upstream `irlserver/srt` `SRTLAPATCHES=1`.** Upstream gates four +sites behind one flag; CeraLive splits the same behaviour across `120` + `119`. +Verified by reading `f2297192:srtcore/core.cpp` against this tree. + +| # | Upstream gate site | CeraLive equivalent | Verdict | +|---|---|---|---| +| 1 | `initial_loss_ttl = srtlaPatches ? iMaxReorderTolerance : m_iReorderTolerance` | `initial_loss_ttl = m_iReorderTolerance`, ungated | **Exact.** Every write to `m_iReorderTolerance` was enumerated: it initialises to the max, its only decrements are sites 2 and 3, and its only other write is an increase capped at the max. So while `SRTO_REORDERFREEZE` is on, `m_iReorderTolerance == iMaxReorderTolerance` identically. | +| 2 | 50-consecutive-ordered decay, gated `!srtlaPatches` | same decay, gated `!bReorderFreeze` | **Exact.** | +| 3 | 10-consecutive-early decay, gated `!srtlaPatches` | same decay, gated `!bReorderFreeze` | **Exact.** | +| 4 | periodic NAK: `if (!srtlaPatches) sendCtrl(UMSG_LOSSREPORT)` — suppressed outright (`f2297192:srtcore/core.cpp:12018-12029`) | `SRTO_PERIODICNAKGATE`: `2` = suppress, `1` = filter | **Exact at the shipped default.** `SRTLA_PATCHES_DEFAULT_NAKGATE = 2`, so `SRTO_SRTLAPATCHES=1` reproduces upstream site 4 bit-for-bit. `1` is the deliberate CeraLive divergence and is opt-in only. | + +Sites 1-3 hold **only while `SRTO_REORDERFREEZE` is on** — site 1's equivalence is a +consequence of sites 2 and 3 being frozen, not an independent property. A future +ungated decay path would silently break it toward shorter loss TTLs. + +**The site-4 default was measured, not chosen.** The D10 A/B ran arm A +(`periodicnakgate=1`, filter) against arm B (`periodicnakgate=2`, suppress) on the +`srtla` compat harness under a pre-registered, hash-frozen rule: 4 netem cells +(`loss` 0.5 %/2 % × `reorder` 10 %/25 %), N = 3 per arm per cell, 24/24 rows valid +with no retries. Primary metric is viewer-observed loss +(`pktRcvDropTotal / (pktRecvUniqueTotal + pktRcvDropTotal)`), with a goodput guard. +A won loss on **1 of 4** cells where the rule requires ≥ 3, and the goodput guard +held on all 4, so **WINNER = 2**. Ties inside the pre-registered 0.100 pp noise band +were not broken in either arm's favour. Evidence: `docs/evidence/ab-periodic-nak/` +in `CERALIVE/srtla` (`rows.json`, `verdict.json`, `rule_sha256` +`3afffaf6743175a065855d4216791d82afc5a0612c30db84f251a63932d49fea`). + +**Also in this release** (from the v1.5.7 absorb): upstream KMREQ and +encryption-state validation, ACK and DROPREQ validation, FEC bounds checks, bonding +BACKUP lifetime safety, and sample-tool path validation. + +### 1.5.6+ceralive.1 — previous release + +Haivision base v1.5.6. `SRTO_REORDERFREEZE = 120` and the deterministic +socket-teardown fix (sections 1 and 4 above). Remains the immutable `abi.yml` +comparison baseline. + +--- + ## Packaging & CI (non-source CeraLive additions) These do not change the SRT protocol or the library ABI; they exist so the fork ships @@ -138,3 +225,9 @@ added here with its commit SHA and a one-paragraph rationale, and a patch that i retired (e.g. superseded by an upstream fix) **must** be moved to a "Retired" note rather than silently dropped. Any functional change beyond these patches is out of scope for the fork (see [`AGENTS.md`](../AGENTS.md) → SCOPE BOUNDARY). + +Every cut release **must** gain a [Releases](#releases) subsection in the same change +that tags it, naming the Haivision base commit, the tag, the two `.deb` filenames, and +whether the ABI is additive. A change to any of the three option numbers or to +`SRTLA_PATCHES_DEFAULT_NAKGATE` **must** restate the equivalence table above, because +that table is what downstream consumers rely on when they set `srtlapatches=1`. From 20e23b29aea6850a1dd76354495b7e53c5b6b469 Mon Sep 17 00:00:00 2001 From: Andres Cera Date: Sun, 20 Sep 2026 19:30:01 -0500 Subject: [PATCH 11/11] docs(api): state the resolved SRTLAPATCHES compat default, not a pending one SRTO_PERIODICNAKGATE=2 was described as "the arm the D10 A/B measures" and the SRTO_SRTLAPATCHES default as "initially 2". The A/B has run and selected 2, so both readings are now wrong in the same direction: they present a settled default as provisional. --- docs/API/API-socket-options.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/API/API-socket-options.md b/docs/API/API-socket-options.md index cd50a9f53d..d9748b3997 100644 --- a/docs/API/API-socket-options.md +++ b/docs/API/API-socket-options.md @@ -1311,8 +1311,9 @@ independently of an explicit NAK report request. not reported as lost. - `2` — **suppress**: send no periodic loss report from this site at all, while the NAK timer still advances. This is behaviourally identical to - `irlserver/srt`'s `SRTLAPATCHES` suppression and is the arm the D10 A/B - measures. + `irlserver/srt`'s `SRTLAPATCHES` suppression, and is the arm the D10 A/B + selected as the `SRTO_SRTLAPATCHES` compat default (see + [`CERALIVE-PATCHES.md`](../CERALIVE-PATCHES.md) → Releases). Out-of-range values are rejected with `SRT_EINVPARAM`. The option is receiver-side, opt-in, and inherited by accepted sockets from the listener. @@ -1694,8 +1695,8 @@ working against this SRT. Only **0 / non-zero** are meaningful (bool-like; a non-zero `int` is accepted): - non-zero ⇒ `SRTO_REORDERFREEZE = true` **and** `SRTO_PERIODICNAKGATE` set to - its D10 default (`SRTLA_PATCHES_DEFAULT_NAKGATE`, initially `2` = - upstream-exact suppress); the getter reads `true`. + its D10 default (`SRTLA_PATCHES_DEFAULT_NAKGATE`, `2` = upstream-exact + suppress); the getter reads `true`. - zero ⇒ both `SRTO_REORDERFREEZE = false` and `SRTO_PERIODICNAKGATE = 0`; the getter reads `false`.