diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml index 7a7b441bd..fa1f8494c 100644 --- a/.github/workflows/codecov.yml +++ b/.github/workflows/codecov.yml @@ -22,7 +22,7 @@ jobs: mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config sudo apt-get update - sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov libtool libtool-bin autoconf + sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ lcov libtool libtool-bin autoconf autoconf-archive automake echo -e "Host localhost 127.0.0.1\n Port 2222\n\n" >> ~/.ssh/config @@ -59,10 +59,14 @@ jobs: - name: Build run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DCODE_COVERAGE=ON ../ - make -j`nproc` + make -j"${parallel_level}" popd - name: Build Test with code coverage diff --git a/.github/workflows/deploy_debian_repo.yml b/.github/workflows/deploy_debian_repo.yml index bf0bbc7eb..46f8a728c 100644 --- a/.github/workflows/deploy_debian_repo.yml +++ b/.github/workflows/deploy_debian_repo.yml @@ -262,6 +262,9 @@ jobs: local qemu_target="" case "${arch}" in + amd64) + qemu_target="x86_64" + ;; arm64) qemu_target="aarch64" ;; @@ -294,13 +297,11 @@ jobs: --output /tmp/qemu-binfmt-conf.sh chmod +x /tmp/qemu-binfmt-conf.sh - if ! /tmp/qemu-binfmt-conf.sh --path /usr/bin --suffix -static --persistent "${qemu_target}"; then - /tmp/qemu-binfmt-conf.sh \ - --qemu-path /usr/bin \ - --qemu-suffix -static \ - --persistent yes \ - "${qemu_target}" - fi + /tmp/qemu-binfmt-conf.sh \ + --qemu-path /usr/bin \ + --qemu-suffix=-static \ + --persistent yes \ + "${qemu_target}" } for distro in "${debian_suites[@]}"; do @@ -426,7 +427,22 @@ jobs: install -d -m 700 "${HOME}/.ssh" printf '%s\n' "${DEBIAN_REPO_SSH_PRIVATE_KEY}" > "${HOME}/.ssh/id_ed25519" chmod 600 "${HOME}/.ssh/id_ed25519" - ssh-keyscan github.com >> "${HOME}/.ssh/known_hosts" + ssh-keyscan -t rsa,ecdsa,ed25519 github.com > "${HOME}/.ssh/known_hosts" + if [[ ! -s "${HOME}/.ssh/known_hosts" ]]; then + echo "Failed to populate SSH known_hosts for github.com" >&2 + exit 1 + fi + chmod 600 "${HOME}/.ssh/known_hosts" + cat > "${HOME}/.ssh/config" <> "${GITHUB_ENV}" git config --global user.name "Jason Gauci" git config --global user.email "jgmath2000@gmail.com" diff --git a/.github/workflows/deploy_windows_winget.yml b/.github/workflows/deploy_windows_winget.yml new file mode 100644 index 000000000..2a741d238 --- /dev/null +++ b/.github/workflows/deploy_windows_winget.yml @@ -0,0 +1,157 @@ +name: Deploy Windows Release Asset + +on: + release: + types: [published] + workflow_dispatch: + inputs: + release_tag: + description: "Release tag to package. Leave blank to use the latest GitHub release." + required: false + default: "" + overwrite_asset: + description: "Overwrite an existing release asset with the same name." + required: true + type: boolean + default: false + +permissions: + contents: write + +defaults: + run: + shell: bash + +jobs: + deploy_windows: + name: deploy-windows-release-asset + runs-on: windows-latest + env: + CMAKE_BUILD_DIR: ${{ github.workspace }}/build + VCPKG_ROOT: ${{ github.workspace }}/external/vcpkg + INPUT_RELEASE_TAG: ${{ github.event.release.tag_name || inputs.release_tag }} + OVERWRITE_ASSET: ${{ inputs.overwrite_asset || false }} + steps: + - name: Resolve release + id: resolve_release + env: + GH_TOKEN: ${{ github.token }} + run: | + if [[ -n "${INPUT_RELEASE_TAG}" ]]; then + release_tag="${INPUT_RELEASE_TAG}" + if ! gh release view "${release_tag}" --repo "${GITHUB_REPOSITORY}" >/dev/null; then + echo "Release ${release_tag} does not exist in ${GITHUB_REPOSITORY}" >&2 + exit 1 + fi + else + release_tag="$(gh release view --repo "${GITHUB_REPOSITORY}" --json tagName --jq .tagName)" + fi + + if [[ -z "${release_tag}" || "${release_tag}" == "null" ]]; then + echo "Could not resolve a release tag" >&2 + exit 1 + fi + + version="${release_tag#et-v}" + if [[ "${version}" == "${release_tag}" ]]; then + echo "Expected release tag to start with et-v, got ${release_tag}" >&2 + exit 1 + fi + + asset_name="EternalTerminal-${version}-windows-x64.zip" + { + echo "RELEASE_TAG=${release_tag}" + echo "RELEASE_VERSION=${version}" + echo "ASSET_NAME=${asset_name}" + echo "PACKAGE_DIR=${RUNNER_TEMP}/EternalTerminal-${version}-windows-x64" + echo "PACKAGE_PATH=${RUNNER_TEMP}/${asset_name}" + } >> "${GITHUB_ENV}" + echo "release_tag=${release_tag}" >> "${GITHUB_OUTPUT}" + + - name: Check release asset + id: check_release_asset + env: + GH_TOKEN: ${{ github.token }} + run: | + existing_asset="$( + gh release view "${RELEASE_TAG}" \ + --repo "${GITHUB_REPOSITORY}" \ + --json assets \ + --jq ".assets[].name" | + grep -Fx "${ASSET_NAME}" || true + )" + + if [[ -n "${existing_asset}" && "${OVERWRITE_ASSET}" != "true" ]]; then + echo "Release asset ${ASSET_NAME} already exists and overwrite_asset is false; skipping Windows build." + echo "should_build=false" >> "${GITHUB_OUTPUT}" + else + echo "Release asset ${ASSET_NAME} will be built and uploaded." + echo "should_build=true" >> "${GITHUB_OUTPUT}" + fi + + - uses: actions/checkout@v4 + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + with: + ref: ${{ steps.resolve_release.outputs.release_tag }} + submodules: recursive + fetch-depth: 0 + + - name: Install Dependencies (Windows) + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + run: choco install -y ninja + + # Restore both vcpkg and its artifacts from the GitHub cache service. + - name: Restore vcpkg and its artifacts. + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + uses: actions/cache@v5 + with: + path: | + ${{ env.VCPKG_ROOT }} + !${{ env.VCPKG_ROOT }}/buildtrees + !${{ env.VCPKG_ROOT }}/packages + !${{ env.VCPKG_ROOT }}/downloads + # The key is composed in a way that it gets properly invalidated: this must happen whenever vcpkg's Git commit id changes, or the list of packages changes. In this case a cache miss must happen and a new entry with a new key with be pushed to GitHub the cache service. + # The key includes: hash of the vcpkg.json file, the hash of the vcpkg Git commit id, and the used vcpkg's triplet. The vcpkg's commit id would suffice, but computing an hash out it does not harm. + # Note: given a key, the cache content is immutable. If a cache entry has been created improperly, in order the recreate the right content the key must be changed as well, and it must be brand new (i.e. not existing already). + key: | + et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-windows-latest-11-release-1 + + - name: Show content of workspace after cache has been restored + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + run: find $RUNNER_WORKSPACE + shell: bash + + # On Windows runners, let's ensure to have the Developer Command Prompt environment setup correctly. As used here the Developer Command Prompt created is targeting x64 and using the default the Windows SDK. + - uses: ilammy/msvc-dev-cmd@v1 + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + + # Run CMake to generate Ninja project files, using the vcpkg's toolchain file to resolve and install the dependencies as specified in vcpkg.json. + - name: Install dependencies and generate project files (windows) + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + run: | + cmake -DDISABLE_TELEMETRY=ON -S "${{ github.workspace }}" -B "${{ env.CMAKE_BUILD_DIR }}" -GNinja -DCMAKE_BUILD_TYPE=RelWithDebInfo + + # Build the whole project with Ninja (which is spawn by CMake). + - name: Build + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + run: | + cmake --build "${{ env.CMAKE_BUILD_DIR }}" + ctest --parallel + + - name: Package + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + run: | + mkdir -p "${PACKAGE_DIR}" + cp "${CMAKE_BUILD_DIR}/et.exe" "${PACKAGE_DIR}/et.exe" + cp "${GITHUB_WORKSPACE}/LICENSE" "${PACKAGE_DIR}/LICENSE" + pushd "${PACKAGE_DIR}" + 7z a -tzip "${PACKAGE_PATH}" . + popd + + - name: Upload release asset + if: ${{ steps.check_release_asset.outputs.should_build == 'true' }} + env: + GH_TOKEN: ${{ github.token }} + run: | + upload_args=("${RELEASE_TAG}" "${PACKAGE_PATH}" --repo "${GITHUB_REPOSITORY}") + gh release upload "${upload_args[@]}" diff --git a/.github/workflows/linux_ci.yml b/.github/workflows/linux_ci.yml index 62bd4c207..715e96306 100644 --- a/.github/workflows/linux_ci.yml +++ b/.github/workflows/linux_ci.yml @@ -27,7 +27,7 @@ jobs: mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config sudo apt-get -o Acquire::ForceIPv4=true update - sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get -o Acquire::ForceIPv4=true install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ libtool libtool-bin autoconf + sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get -o Acquire::ForceIPv4=true install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ libtool libtool-bin autoconf autoconf-archive automake echo -e "Host localhost 127.0.0.1 ::1\n Port ${SSH_PORT}\n\n" >> ~/.ssh/config @@ -64,41 +64,57 @@ jobs: - name: Build with ubsan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_UNDEFINED=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + make -j"${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd if: matrix.sanitize == 'ubsan' - name: Build with asan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_ADDRESS=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + make -j"${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd if: matrix.sanitize == 'asan' - name: Build with msan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_MEMORY=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + make -j"${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd if: matrix.sanitize == 'msan' - name: Build with tsan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_THREAD=ON -DSANITIZE_LINK_STATIC=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + make -j"${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd if: matrix.sanitize == 'tsan' @@ -123,7 +139,7 @@ jobs: mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config sudo apt-get -o Acquire::ForceIPv4=true update - sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get -o Acquire::ForceIPv4=true install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ libtool libtool-bin autoconf + sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get -o Acquire::ForceIPv4=true install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ libtool libtool-bin autoconf autoconf-archive automake echo -e "Host localhost 127.0.0.1 ::1\n Port ${SSH_PORT}\n\n" >> ~/.ssh/config @@ -153,10 +169,14 @@ jobs: - name: Build for jumphost system test run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_UNDEFINED=ON ../ - cmake --build . --target et etserver etterminal --parallel `nproc` + cmake --build . --target et etserver etterminal --parallel "${parallel_level}" popd - name: Connect with jumphost @@ -183,7 +203,7 @@ jobs: mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config sudo apt-get -o Acquire::ForceIPv4=true update - sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get -o Acquire::ForceIPv4=true install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ libtool libtool-bin autoconf + sudo DEBIAN_FRONTEND=noninteractive ACCEPT_EULA=Y apt-get -o Acquire::ForceIPv4=true install -y curl zip unzip tar libssl-dev libcurl4-openssl-dev libunwind-dev git cmake ninja-build gdb protobuf-compiler libsodium-dev libgflags-dev libprotobuf-dev libutempter-dev g++ libtool libtool-bin autoconf autoconf-archive automake echo -e "Host localhost 127.0.0.1 ::1\n Port ${SSH_PORT}\n\n" >> ~/.ssh/config @@ -213,10 +233,14 @@ jobs: - name: Build for backwards compatibility system test run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_UNDEFINED=ON ../ - cmake --build . --target et etserver etterminal --parallel `nproc` + cmake --build . --target et etserver etterminal --parallel "${parallel_level}" popd - name: Backwards compatibility with et-v7.0.0 diff --git a/.github/workflows/mac_ci.yml b/.github/workflows/mac_ci.yml index 21d3afd7c..43d4a7958 100644 --- a/.github/workflows/mac_ci.yml +++ b/.github/workflows/mac_ci.yml @@ -21,7 +21,7 @@ jobs: run: | mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config - brew install cmake ninja; brew install protobuf libsodium automake autoconf libtool + brew install cmake ninja; brew install protobuf libsodium automake autoconf autoconf-archive libtool auth_header="$(git config --local --get http.https://github.com/.extraheader)" git submodule sync --recursive git submodule update --init --force --recursive @@ -47,11 +47,15 @@ jobs: - name: Test with ubsan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_UNDEFINED=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + cmake --build . --parallel "${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd rm -Rf build @@ -72,7 +76,7 @@ jobs: git submodule update --init --force --recursive mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config - brew install cmake ninja; brew install protobuf libsodium automake autoconf libtool + brew install cmake ninja; brew install protobuf libsodium automake autoconf autoconf-archive libtool # Restore both vcpkg and its artifacts from the GitHub cache service. - name: Restore vcpkg and its artifacts. @@ -95,11 +99,15 @@ jobs: - name: Test with asan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_ADDRESS=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + cmake --build . --parallel "${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd rm -Rf build @@ -120,7 +128,7 @@ jobs: git submodule update --init --force --recursive mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config - brew install cmake ninja; brew install protobuf libsodium automake autoconf libtool + brew install cmake ninja; brew install protobuf libsodium automake autoconf autoconf-archive libtool # Restore both vcpkg and its artifacts from the GitHub cache service. - name: Restore vcpkg and its artifacts. @@ -143,11 +151,15 @@ jobs: - name: Test with msan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_MEMORY=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + cmake --build . --parallel "${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd rm -Rf build @@ -168,7 +180,7 @@ jobs: git submodule update --init --force --recursive mkdir -p ~/.ssh/ echo -e "Host github.com\n\tStrictHostKeyChecking no\n" >> ~/.ssh/config - brew install cmake ninja; brew install protobuf libsodium automake autoconf libtool + brew install cmake ninja; brew install protobuf libsodium automake autoconf autoconf-archive libtool # Restore both vcpkg and its artifacts from the GitHub cache service. - name: Restore vcpkg and its artifacts. @@ -191,10 +203,14 @@ jobs: - name: Test with tsan run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi mkdir -p build pushd build cmake -DDISABLE_TELEMETRY=ON -DSANITIZE_THREAD=ON -DSANITIZE_LINK_STATIC=ON ../ - make -j`nproc` - TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel + cmake --build . --parallel "${parallel_level}" + TSAN_OPTIONS="suppressions=../test/test_tsan.suppression" ctest --parallel "${parallel_level}" --output-on-failure popd rm -Rf build diff --git a/.github/workflows/novcpkg_build_master.yml b/.github/workflows/novcpkg_build_master.yml index cbe5cfd41..9e6b018d1 100644 --- a/.github/workflows/novcpkg_build_master.yml +++ b/.github/workflows/novcpkg_build_master.yml @@ -13,20 +13,24 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest, macos-latest] - gcc: [11, 12, 13] + gcc: [13, 14, 15, 16] exclude: #- os: macos-latest # Allow one gcc variant for mac/windows, since they don't use gcc #gcc: 11 - os: macos-latest - gcc: 12 + gcc: 14 - os: macos-latest - gcc: 13 + gcc: 15 + - os: macos-latest + gcc: 16 #- os: windows-latest #gcc: 11 - os: windows-latest - gcc: 12 + gcc: 14 + - os: windows-latest + gcc: 15 - os: windows-latest - gcc: 13 + gcc: 16 env: # Indicates the CMake build directory where project files and binaries are being produced. CMAKE_BUILD_DIR: ${{ github.workspace }}/build @@ -35,7 +39,7 @@ jobs: - name: Install Dependencies (Linux) run: | sudo apt-get update && \ - sudo apt-get install --no-install-recommends \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ libboost-dev \ libsodium-dev \ libprotobuf-dev \ @@ -87,10 +91,15 @@ jobs: # Build the whole project with Ninja (which is spawn by CMake). - name: Build run: | - cmake --build "${{ env.CMAKE_BUILD_DIR }}" - ctest --parallel + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi + cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" + ctest --test-dir "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" --output-on-failure - uses: actions/upload-artifact@v6 + if: ${{ !env.ACT }} with: name: et-client-${{matrix.os}}-gcc${{matrix.gcc}} path: ${{ env.CMAKE_BUILD_DIR }}/et${{matrix.extension}} diff --git a/.github/workflows/novcpkg_build_release.yml b/.github/workflows/novcpkg_build_release.yml index 695db5760..e9665818c 100644 --- a/.github/workflows/novcpkg_build_release.yml +++ b/.github/workflows/novcpkg_build_release.yml @@ -15,20 +15,24 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest, macos-latest] - gcc: [11, 12, 13] + gcc: [13, 14, 15, 16] exclude: #- os: macos-latest # Allow one gcc variant for mac/windows, since they don't use gcc #gcc: 11 - os: macos-latest - gcc: 12 + gcc: 14 - os: macos-latest - gcc: 13 + gcc: 15 + - os: macos-latest + gcc: 16 #- os: windows-latest #gcc: 11 - os: windows-latest - gcc: 12 + gcc: 14 + - os: windows-latest + gcc: 15 - os: windows-latest - gcc: 13 + gcc: 16 env: # Indicates the CMake build directory where project files and binaries are being produced. CMAKE_BUILD_DIR: ${{ github.workspace }}/build @@ -37,7 +41,7 @@ jobs: - name: Install Dependencies (Linux) run: | sudo apt-get update && \ - sudo apt-get install --no-install-recommends \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ libboost-dev \ libsodium-dev \ libprotobuf-dev \ @@ -89,10 +93,15 @@ jobs: # Build the whole project with Ninja (which is spawn by CMake). - name: Build run: | - cmake --build "${{ env.CMAKE_BUILD_DIR }}" - ctest --parallel + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi + cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" + ctest --test-dir "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" --output-on-failure - uses: actions/upload-artifact@v6 + if: ${{ !env.ACT }} with: name: et-client-${{matrix.os}}-gcc${{matrix.gcc}} path: ${{ env.CMAKE_BUILD_DIR }}/et${{matrix.extension}} diff --git a/.github/workflows/portability_ci.yml b/.github/workflows/portability_ci.yml index 99e6eac7d..e7e18e87a 100644 --- a/.github/workflows/portability_ci.yml +++ b/.github/workflows/portability_ci.yml @@ -139,10 +139,16 @@ jobs: rsync sudo rm -rf /root/work sudo ln -s "${GITHUB_WORKSPACE}" /root/work + sudo mkdir -p /home/runner/work/EternalTerminal + sudo rsync -a --delete \ + --exclude build \ + "${GITHUB_WORKSPACE}/" \ + /home/runner/work/EternalTerminal/EternalTerminal/ - name: Start FreeBSD VM uses: vmactions/freebsd-vm@v1 with: + arch: ${{ env.ACT && 'aarch64' || '' }} usesh: true - name: Expose FreeBSD shell for act @@ -156,6 +162,8 @@ jobs: run: | if [ -f /root/work/CMakeLists.txt ]; then cd /root/work + elif [ -f /home/runner/work/EternalTerminal/EternalTerminal/CMakeLists.txt ]; then + cd /home/runner/work/EternalTerminal/EternalTerminal else cd "${GITHUB_WORKSPACE}" fi diff --git a/.github/workflows/vcpkg_build_master.yml b/.github/workflows/vcpkg_build_master.yml index 730221186..1ecbff386 100644 --- a/.github/workflows/vcpkg_build_master.yml +++ b/.github/workflows/vcpkg_build_master.yml @@ -13,7 +13,7 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest, macos-latest, windows-latest] - gcc: [11, 12, 13] + gcc: [13, 14, 15, 16] include: - os: windows-latest extension: .exe @@ -21,15 +21,19 @@ jobs: #- os: macos-latest # Allow one gcc variant for mac/windows, since they don't use gcc #gcc: 11 - os: macos-latest - gcc: 12 + gcc: 14 - os: macos-latest - gcc: 13 + gcc: 15 + - os: macos-latest + gcc: 16 #- os: windows-latest #gcc: 11 - os: windows-latest - gcc: 12 + gcc: 14 + - os: windows-latest + gcc: 15 - os: windows-latest - gcc: 13 + gcc: 16 env: # Indicates the CMake build directory where project files and binaries are being produced. CMAKE_BUILD_DIR: ${{ github.workspace }}/build @@ -40,7 +44,7 @@ jobs: - name: Install Dependencies (Linux) run: | sudo apt-get update && \ - sudo apt-get install --no-install-recommends \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ libboost-dev \ libsodium-dev \ libprotobuf-dev \ @@ -56,13 +60,16 @@ jobs: tar \ cmake \ libutempter-dev \ - libunwind-dev + libunwind-dev \ + autoconf \ + autoconf-archive \ + automake if: matrix.os == 'ubuntu-latest' - name: Install Dependencies (Windows) run: choco install -y ninja if: matrix.os == 'windows-latest' - name: Install Dependencies (macOS) - run: brew update && brew install ninja cmake automake autoconf libtool + run: brew update && brew install ninja cmake automake autoconf autoconf-archive libtool if: matrix.os == 'macos-latest' - uses: actions/checkout@v4 @@ -91,12 +98,9 @@ jobs: key: | et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-${{ matrix.os }}-${{ matrix.gcc }}-master-1 - - name: Show content of workspace after cache has been restored - run: find $RUNNER_WORKSPACE - shell: bash - # On Windows runners, let's ensure to have the Developer Command Prompt environment setup correctly. As used here the Developer Command Prompt created is targeting x64 and using the default the Windows SDK. - uses: ilammy/msvc-dev-cmd@v1 + if: matrix.os == 'windows-latest' # Run CMake to generate Ninja project files, using the vcpkg's toolchain file to resolve and install the dependencies as specified in vcpkg.json. - name: Install dependencies and generate project files (linux) env: @@ -118,11 +122,25 @@ jobs: # Build the whole project with Ninja (which is spawn by CMake). - name: Build + if: matrix.os != 'windows-latest' + run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi + cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" + ctest --test-dir "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" --output-on-failure + + - name: Build (Windows) + if: matrix.os == 'windows-latest' + shell: pwsh run: | - cmake --build "${{ env.CMAKE_BUILD_DIR }}" - ctest --parallel + $parallelLevel = if ($env:CI_PARALLEL_LEVEL) { $env:CI_PARALLEL_LEVEL } else { "2" } + cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel "$parallelLevel" + ctest --test-dir "${{ env.CMAKE_BUILD_DIR }}" --parallel "$parallelLevel" --output-on-failure - uses: actions/upload-artifact@v6 + if: ${{ !env.ACT }} with: name: et-client-${{matrix.os}}-gcc${{matrix.gcc}} path: ${{ env.CMAKE_BUILD_DIR }}/et${{matrix.extension}} diff --git a/.github/workflows/vcpkg_build_release.yml b/.github/workflows/vcpkg_build_release.yml index afe3bde7f..7f6c8c118 100644 --- a/.github/workflows/vcpkg_build_release.yml +++ b/.github/workflows/vcpkg_build_release.yml @@ -15,7 +15,7 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest, macos-latest, windows-latest] - gcc: [11, 12, 13] + gcc: [13, 14, 15, 16] include: - os: windows-latest extension: .exe @@ -23,15 +23,19 @@ jobs: #- os: macos-latest # Allow one gcc variant for mac/windows, since they don't use gcc #gcc: 11 - os: macos-latest - gcc: 12 + gcc: 14 - os: macos-latest - gcc: 13 + gcc: 15 + - os: macos-latest + gcc: 16 #- os: windows-latest #gcc: 11 - os: windows-latest - gcc: 12 + gcc: 14 + - os: windows-latest + gcc: 15 - os: windows-latest - gcc: 13 + gcc: 16 env: # Indicates the CMake build directory where project files and binaries are being produced. CMAKE_BUILD_DIR: ${{ github.workspace }}/build @@ -42,7 +46,7 @@ jobs: - name: Install Dependencies (Linux) run: | sudo apt-get update && \ - sudo apt-get install --no-install-recommends \ + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ libboost-dev \ libsodium-dev \ libprotobuf-dev \ @@ -58,13 +62,16 @@ jobs: tar \ cmake \ libutempter-dev \ - libunwind-dev + libunwind-dev \ + autoconf \ + autoconf-archive \ + automake if: matrix.os == 'ubuntu-latest' - name: Install Dependencies (Windows) run: choco install -y ninja if: matrix.os == 'windows-latest' - name: Install Dependencies (macOS) - run: brew update && brew install ninja cmake automake autoconf libtool + run: brew update && brew install ninja cmake automake autoconf autoconf-archive libtool if: matrix.os == 'macos-latest' - uses: actions/checkout@v4 @@ -93,12 +100,9 @@ jobs: key: | et-vcpkg-${{ hashFiles( 'vcpkg.json' ) }}-${{ hashFiles( '.git/modules/external/vcpkg/HEAD' )}}-${{ matrix.os }}-${{ matrix.gcc }}-release-1 - - name: Show content of workspace after cache has been restored - run: find $RUNNER_WORKSPACE - shell: bash - # On Windows runners, let's ensure to have the Developer Command Prompt environment setup correctly. As used here the Developer Command Prompt created is targeting x64 and using the default the Windows SDK. - uses: ilammy/msvc-dev-cmd@v1 + if: matrix.os == 'windows-latest' # Run CMake to generate Ninja project files, using the vcpkg's toolchain file to resolve and install the dependencies as specified in vcpkg.json. - name: Install dependencies and generate project files (linux) env: @@ -120,11 +124,25 @@ jobs: if: matrix.os == 'windows-latest' # Build the whole project with Ninja (which is spawn by CMake). - name: Build + if: matrix.os != 'windows-latest' + run: | + parallel_level="${CI_PARALLEL_LEVEL:-2}" + if [[ -n "${ACT:-}" ]]; then + parallel_level="${CI_PARALLEL_LEVEL:-14}" + fi + cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" + ctest --test-dir "${{ env.CMAKE_BUILD_DIR }}" --parallel "${parallel_level}" --output-on-failure + + - name: Build (Windows) + if: matrix.os == 'windows-latest' + shell: pwsh run: | - cmake --build "${{ env.CMAKE_BUILD_DIR }}" - ctest --parallel + $parallelLevel = if ($env:CI_PARALLEL_LEVEL) { $env:CI_PARALLEL_LEVEL } else { "2" } + cmake --build "${{ env.CMAKE_BUILD_DIR }}" --parallel "$parallelLevel" + ctest --test-dir "${{ env.CMAKE_BUILD_DIR }}" --parallel "$parallelLevel" --output-on-failure - uses: actions/upload-artifact@v6 + if: ${{ !env.ACT }} with: name: et-client-${{matrix.os}}-gcc${{matrix.gcc}} path: ${{ env.CMAKE_BUILD_DIR }}/et${{matrix.extension}} diff --git a/AGENTS.md b/AGENTS.md index 552596251..7ad4003de 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ # Testing -- To run unit tests: `pushd build; ninja && ctest --parallel; popd` +- To run unit tests: `pushd build; ninja && ctest --parallel --output-on-failure; popd` - To get code coverage: `bash coverage.sh` - Any time a new test is added, you must run cmake for cmake/ctest to recognize the new test. - To run lint: `bash format.sh` diff --git a/CMakeLists.txt b/CMakeLists.txt index 935554d3c..23306ecf8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -154,7 +154,7 @@ else() find_package(httplib CONFIG REQUIRED) find_package(cxxopts CONFIG REQUIRED) find_package(nlohmann_json CONFIG REQUIRED) - find_path(SIMPLEINI_INCLUDE_DIRS "ConvertUTF.c") + find_path(SIMPLEINI_INCLUDE_DIRS "SimpleIni.h") endif() # Optional packages @@ -233,8 +233,9 @@ IF(WIN32) ENDIF(WIN32) if(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g --coverage") - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g --coverage") + # CODE_COVERAGE define lets forked children flush gcov before _exit. + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g --coverage -DCODE_COVERAGE") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g --coverage -DCODE_COVERAGE") endif(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") if(DISABLE_CRASH_LOG) @@ -248,10 +249,16 @@ if(UNIX) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -ggdb3") endif() -# Enable C++-17 -set(CMAKE_CXX_STANDARD 17) +# Enable C++20 +set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) +if(APPLE OR FREEBSD) + # The bundled ThreadPool submodule still uses std::result_of. libc++ hides that + # C++17 type trait in C++20 unless this compatibility switch is enabled. + add_compile_definitions(_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS) +endif() + # Enable compile commands export for vscode add_definitions(-DCMAKE_EXPORT_COMPILE_COMMANDS=ON) @@ -314,7 +321,9 @@ elseif(WIN32) set(CORE_LIBRARIES OpenSSL::SSL ZLIB::ZLIB Ws2_32 Shlwapi dbghelp) elseif(APPLE) - set(CORE_LIBRARIES OpenSSL::SSL ZLIB::ZLIB util resolv) + find_library(SECURITY_FRAMEWORK Security REQUIRED) + set(CORE_LIBRARIES OpenSSL::SSL ZLIB::ZLIB util resolv + ${SECURITY_FRAMEWORK}) else() set(CORE_LIBRARIES httplib::httplib @@ -326,8 +335,16 @@ else() util resolv atomic - stdc++fs ) + # stdc++fs was a separate library for in GCC 8 and earlier. + # Starting with GCC 9, filesystem support is part of libstdc++ proper, and + # the separate -lstdc++fs was removed entirely in GCC 15. + # Only link it if the compiler still ships it. + include(CheckLibraryExists) + check_library_exists(stdc++fs "" "" HAVE_STDCXXFS) + if(HAVE_STDCXXFS) + list(APPEND CORE_LIBRARIES stdc++fs) + endif() endif() IF(Unwind_FOUND) @@ -414,6 +431,8 @@ add_library( src/base/SocketHandler.cpp src/base/PipeSocketHandler.hpp src/base/PipeSocketHandler.cpp + src/base/UserSocketOps.hpp + src/base/UserSocketOps.cpp src/base/TcpSocketHandler.hpp src/base/TcpSocketHandler.cpp src/base/UnixSocketHandler.hpp diff --git a/coverage.sh b/coverage.sh index 554168d41..20576eeed 100755 --- a/coverage.sh +++ b/coverage.sh @@ -12,7 +12,7 @@ pushd ./cov_build cmake ../ -DBUILD_TEST=ON -DBUILD_GTEST=ON -DCODE_COVERAGE=ON -DDISABLE_TELEMETRY=ON -G Ninja find . -name "*.gcda" -print0 | xargs -0 rm -f ninja -ctest --parallel +ctest --parallel --output-on-failure popd lcov --directory ./cov_build --capture --output-file ./code-coverage.info -rc lcov_branch_coverage=1 lcov --remove ./code-coverage.info \ diff --git a/external/Catch2 b/external/Catch2 index 2b60af89e..191fa38c9 160000 --- a/external/Catch2 +++ b/external/Catch2 @@ -1 +1 @@ -Subproject commit 2b60af89e23d28eefc081bc930831ee9d45ea58b +Subproject commit 191fa38c9b1596cd2576ab531d4ab4d5e8e05190 diff --git a/external/PlatformFolders b/external/PlatformFolders index 784f8ceb8..1df240bdc 160000 --- a/external/PlatformFolders +++ b/external/PlatformFolders @@ -1 +1 @@ -Subproject commit 784f8ceb8bbd042722caf2cdec427c7b80e0a960 +Subproject commit 1df240bdcbfe8b431f3fa224da086fa805b8162c diff --git a/external/cpp-httplib b/external/cpp-httplib index 3a1f379e7..2132205e1 160000 --- a/external/cpp-httplib +++ b/external/cpp-httplib @@ -1 +1 @@ -Subproject commit 3a1f379e751ef6555f06c5c1ef367a6fce26722c +Subproject commit 2132205e1a69c9fce8096f085b1b8d72efc759fa diff --git a/external/sentry-native b/external/sentry-native index a64d5bd8e..0f7802ffa 160000 --- a/external/sentry-native +++ b/external/sentry-native @@ -1 +1 @@ -Subproject commit a64d5bd8ee130f2cda196b6fa7d9b65bfa6d32e2 +Subproject commit 0f7802ffa38be970b9a18dda84b80f6cea388915 diff --git a/external/simpleini b/external/simpleini index 09c21bda1..877f7357d 160000 --- a/external/simpleini +++ b/external/simpleini @@ -1 +1 @@ -Subproject commit 09c21bda1dc1b578fa55f4a005d79b0afd481296 +Subproject commit 877f7357d1fa4232f1f3352e5028f99899210b27 diff --git a/external/vcpkg b/external/vcpkg index bdd229e13..f7410a9e2 160000 --- a/external/vcpkg +++ b/external/vcpkg @@ -1 +1 @@ -Subproject commit bdd229e13c66fa11acdc0bc8fed8e7474cd24aa5 +Subproject commit f7410a9e287d8aea1aab44d948c453aa29018321 diff --git a/src/base/BackedWriter.cpp b/src/base/BackedWriter.cpp index a26ed6760..1de25d5f1 100644 --- a/src/base/BackedWriter.cpp +++ b/src/base/BackedWriter.cpp @@ -1,5 +1,7 @@ #include "BackedWriter.hpp" +#include + namespace et { BackedWriter::BackedWriter(std::shared_ptr socketHandler_, std::shared_ptr cryptoHandler_, @@ -89,7 +91,9 @@ vector BackedWriter::recover(int64_t lastValidSequenceNumber) { int64_t messagesToRecover = sequenceNumber - lastValidSequenceNumber; if (messagesToRecover < 0) { - STFATAL << "Something went really wrong, client is ahead of server"; + // Attacker-controlled sequence numbers must not abort the process. + throw std::runtime_error( + "Invalid recovery sequence: client is ahead of server"); } if (messagesToRecover == 0) { return vector(); diff --git a/src/base/ClientConnection.cpp b/src/base/ClientConnection.cpp index e42a69142..f8aad7f63 100644 --- a/src/base/ClientConnection.cpp +++ b/src/base/ClientConnection.cpp @@ -73,7 +73,7 @@ void ClientConnection::closeSocketAndMaybeReconnect() { waitReconnect(); LOG(INFO) << "Closing socket"; closeSocket(); - if (!shuttingDown) { + if (!isShuttingDown()) { LOG(INFO) << "Socket closed, starting new reconnect thread"; reconnectThread = std::shared_ptr( new std::thread(&ClientConnection::pollReconnect, this)); @@ -91,9 +91,12 @@ void ClientConnection::waitReconnect() { void ClientConnection::pollReconnect() { el::Helpers::setThreadName("Reconnect"); LOG(INFO) << "Trying to reconnect to " << remoteEndpoint << endl; - while (socketFd == -1) { + while (true) { { lock_guard guard(connectionMutex); + if (socketFd != -1) { + break; + } if (shuttingDown) { LOG(INFO) << "Aborting reconnect loop because shutdown was called"; return; @@ -147,7 +150,7 @@ void ClientConnection::pollReconnect() { } } - if (socketFd == -1) { + if (isDisconnected()) { VLOG_EVERY_N(10, 1) << "Waiting to retry..."; std::this_thread::sleep_for(std::chrono::seconds(1)); } diff --git a/src/base/Connection.cpp b/src/base/Connection.cpp index 3b7a23ff6..eaec4408d 100644 --- a/src/base/Connection.cpp +++ b/src/base/Connection.cpp @@ -118,7 +118,8 @@ bool Connection::recover(int newSocketFd) { // Read the remote sequence number et::SequenceHeader remoteHeader = - socketHandler->readProto(newSocketFd, true); + socketHandler->readProto( + newSocketFd, true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH); { // Fetch the catchup bytes and send diff --git a/src/base/Connection.hpp b/src/base/Connection.hpp index 8a0b06a95..70b9dd229 100644 --- a/src/base/Connection.hpp +++ b/src/base/Connection.hpp @@ -47,11 +47,17 @@ class Connection { inline shared_ptr getWriter() { return writer; } /** @brief File descriptor of the currently connected socket or -1. */ - int getSocketFd() { return socketFd; } + int getSocketFd() { + lock_guard guard(connectionMutex); + return socketFd; + } inline shared_ptr getSocketHandler() { return socketHandler; } - inline bool isDisconnected() { return socketFd == -1; } + inline bool isDisconnected() { + lock_guard guard(connectionMutex); + return socketFd == -1; + } /** * @brief Returns true when writePacket() of `bytes` more will not block: @@ -67,7 +73,10 @@ class Connection { inline string getId() { return id; } - inline bool hasData() { return reader->hasData(); } + inline bool hasData() { + lock_guard guard(connectionMutex); + return reader && reader->hasData(); + } /** * @brief Closes the socket and invalidates the reader/writer. diff --git a/src/base/PipeSocketHandler.cpp b/src/base/PipeSocketHandler.cpp index 0702f6188..5e0fd4c55 100644 --- a/src/base/PipeSocketHandler.cpp +++ b/src/base/PipeSocketHandler.cpp @@ -1,5 +1,9 @@ #include "PipeSocketHandler.hpp" +#ifndef WIN32 +#include "UserSocketOps.hpp" +#endif + namespace et { PipeSocketHandler::PipeSocketHandler() {} @@ -90,6 +94,24 @@ int PipeSocketHandler::connect(const SocketEndpoint& endpoint) { return sockFd; } +#ifndef WIN32 +int PipeSocketHandler::connectAsUser(const SocketEndpoint& endpoint, uid_t uid, + gid_t gid) { + lock_guard mutexGuard(globalMutex); + + string pipePath = endpoint.name(); + VLOG(3) << "Connecting to " << endpoint << " as uid " << uid; + int sockFd = UserSocketOps::connectUnixAsUser(pipePath, uid, gid); + if (sockFd < 0) { + return -1; + } + initSocket(sockFd); + addToActiveSockets(sockFd); + LOG(INFO) << "Connected to endpoint " << endpoint << " as uid " << uid; + return sockFd; +} +#endif + set PipeSocketHandler::listen(const SocketEndpoint& endpoint) { lock_guard guard(globalMutex); @@ -117,6 +139,27 @@ set PipeSocketHandler::listen(const SocketEndpoint& endpoint) { return pipeServerSockets[pipePath]; } +#ifndef WIN32 +set PipeSocketHandler::listenAsUser(const SocketEndpoint& endpoint, + uid_t uid, gid_t gid) { + lock_guard guard(globalMutex); + + string pipePath = endpoint.name(); + if (pipeServerSockets.find(pipePath) != pipeServerSockets.end()) { + throw runtime_error("Tried to listen twice on the same path"); + } + + int fd = UserSocketOps::listenUnixAsUser(pipePath, uid, gid); + if (fd < 0) { + throw runtime_error(string("Failed to listen as user on ") + pipePath + + ": " + strerror(GetErrno())); + } + initServerSocket(fd); + pipeServerSockets[pipePath] = set({fd}); + return pipeServerSockets[pipePath]; +} +#endif + set PipeSocketHandler::getEndpointFds(const SocketEndpoint& endpoint) { lock_guard guard(globalMutex); diff --git a/src/base/PipeSocketHandler.hpp b/src/base/PipeSocketHandler.hpp index 7a2199574..3bc4edf28 100644 --- a/src/base/PipeSocketHandler.hpp +++ b/src/base/PipeSocketHandler.hpp @@ -17,10 +17,22 @@ class PipeSocketHandler : public UnixSocketHandler { * @brief Connects to a pipe identified by the endpoint name. */ virtual int connect(const SocketEndpoint& endpoint); +#ifndef WIN32 + /** + * @brief Connects to a UNIX socket after dropping to @p uid/@p gid. + */ + int connectAsUser(const SocketEndpoint& endpoint, uid_t uid, gid_t gid); +#endif /** * @brief Creates a listening UNIX socket and stores it internally. */ virtual set listen(const SocketEndpoint& endpoint); +#ifndef WIN32 + /** + * @brief Creates a listening UNIX socket after dropping to @p uid/@p gid. + */ + set listenAsUser(const SocketEndpoint& endpoint, uid_t uid, gid_t gid); +#endif /** * @brief Returns the listening fds for a previously registered pipe. */ diff --git a/src/base/ServerClientConnection.cpp b/src/base/ServerClientConnection.cpp index 32883eabc..3005f6491 100644 --- a/src/base/ServerClientConnection.cpp +++ b/src/base/ServerClientConnection.cpp @@ -25,13 +25,40 @@ ServerClientConnection::~ServerClientConnection() { } bool ServerClientConnection::recoverClient(int newSocketFd) { + // Detach the live session without closing it until recover succeeds, so a + // failed/malicious reconnect cannot force-disconnect the victim. + int oldSocketFd = -1; { lock_guard guard(connectionMutex); - if (socketFd != -1) { - closeSocket(); + oldSocketFd = socketFd; + if (reader) { + reader->invalidateSocket(); + } + if (writer) { + writer->invalidateSocket(); + } + socketFd = -1; + } + + bool success = recover(newSocketFd); + if (success) { + if (oldSocketFd != -1) { + socketHandler->close(oldSocketFd); + } + return true; + } + + if (oldSocketFd != -1) { + lock_guard guard(connectionMutex); + socketFd = oldSocketFd; + if (reader) { + reader->revive(oldSocketFd, vector()); + } + if (writer) { + writer->revive(oldSocketFd); } } - return recover(newSocketFd); + return false; } bool ServerClientConnection::verifyPasskey(const string& targetKey) { diff --git a/src/base/ServerClientConnection.hpp b/src/base/ServerClientConnection.hpp index 201aacd26..72a7eb693 100644 --- a/src/base/ServerClientConnection.hpp +++ b/src/base/ServerClientConnection.hpp @@ -20,8 +20,8 @@ class ServerClientConnection : public Connection { virtual ~ServerClientConnection(); /** - * @brief Tears down the old socket (if any) and attempts recovery on the new - * fd. + * @brief Attempts recovery on the new fd; closes the old socket only after + * recover succeeds. */ bool recoverClient(int newSocketFd); diff --git a/src/base/ServerConnection.cpp b/src/base/ServerConnection.cpp index 95aeee4a2..e91d4bb13 100644 --- a/src/base/ServerConnection.cpp +++ b/src/base/ServerConnection.cpp @@ -41,8 +41,8 @@ void ServerConnection::clientHandler(int clientSocketFd) { string clientId; bool createdClientConnection = false; try { - et::ConnectRequest request = - socketHandler->readProto(clientSocketFd, true); + et::ConnectRequest request = socketHandler->readProto( + clientSocketFd, true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH); { int version = request.version(); if (version != PROTOCOL_VERSION) { diff --git a/src/base/SocketHandler.cpp b/src/base/SocketHandler.cpp index 7075a0242..2520ab068 100644 --- a/src/base/SocketHandler.cpp +++ b/src/base/SocketHandler.cpp @@ -3,17 +3,33 @@ #include "base64.h" namespace et { -#define SOCKET_DATA_TRANSFER_TIMEOUT (30) +#define SOCKET_DATA_TRANSFER_TIMEOUT (SocketHandler::SOCKET_IDLE_TIMEOUT_SEC) void SocketHandler::readAll(int fd, void* buf, size_t count, bool timeout) { - time_t startTime = time(NULL); + if (timeout) { + readAll(fd, buf, count, SOCKET_IDLE_TIMEOUT_SEC, + SOCKET_ABSOLUTE_TIMEOUT_SEC); + } else { + readAll(fd, buf, count, 0, 0); + } +} + +void SocketHandler::readAll(int fd, void* buf, size_t count, int idleTimeoutSec, + int absoluteTimeoutSec) { + time_t absoluteStartTime = time(NULL); + time_t idleStartTime = absoluteStartTime; size_t pos = 0; while (pos < count) { + // Enforce deadlines on every iteration so a slow trickle that keeps the + // idle timer reset cannot hold the read open past the absolute limit. + time_t currentTime = time(NULL); + if ((idleTimeoutSec > 0 && currentTime > idleStartTime + idleTimeoutSec) || + (absoluteTimeoutSec > 0 && + currentTime > absoluteStartTime + absoluteTimeoutSec)) { + throw std::runtime_error("Socket Timeout"); + } + if (!waitOnSocketData(fd)) { - time_t currentTime = time(NULL); - if (timeout && currentTime > startTime + SOCKET_DATA_TRANSFER_TIMEOUT) { - throw std::runtime_error("Socket Timeout"); - } continue; } @@ -36,7 +52,8 @@ void SocketHandler::readAll(int fd, void* buf, size_t count, bool timeout) { } } else { pos += bytesRead; - startTime = time(NULL); + // Only the idle timer resets on progress; absolute deadline does not. + idleStartTime = time(NULL); } } } diff --git a/src/base/SocketHandler.hpp b/src/base/SocketHandler.hpp index 168fb16ca..4adcbddc8 100644 --- a/src/base/SocketHandler.hpp +++ b/src/base/SocketHandler.hpp @@ -38,13 +38,27 @@ class SocketHandler { */ virtual ssize_t write(int fd, const void* buf, size_t count) = 0; + /** @brief Idle-gap timeout (seconds) used when `readAll(..., true)`. */ + static constexpr int SOCKET_IDLE_TIMEOUT_SEC = 30; + /** + * @brief Absolute read deadline (seconds) used when `readAll(..., true)`. + * Unlike the idle timeout, this is not reset when bytes arrive. + */ + static constexpr int SOCKET_ABSOLUTE_TIMEOUT_SEC = 60; + /** * @brief Reads exactly `count` bytes, retrying on EAGAIN until the buffer * fills. - * @param timeout Whether to enforce the internal transfer timeout while - * waiting. + * @param timeout Whether to enforce idle + absolute transfer timeouts. */ void readAll(int fd, void* buf, size_t count, bool timeout); + /** + * @brief Reads exactly `count` bytes with explicit deadlines. + * @param idleTimeoutSec Max seconds without any progress; 0 disables. + * @param absoluteTimeoutSec Max seconds for the whole read; 0 disables. + */ + void readAll(int fd, void* buf, size_t count, int idleTimeoutSec, + int absoluteTimeoutSec); /** * @brief Attempts to write the full buffer and returns -1 on timeout/failure. * @return Total bytes written or -1 when the socket deadlocks. @@ -56,6 +70,16 @@ class SocketHandler { */ void writeAllOrThrow(int fd, const void* buf, size_t count, bool timeout); + /** @brief Default max length for length-prefixed protobuf reads (128 MiB). */ + static constexpr int64_t DEFAULT_MAX_PROTO_LENGTH = 128 * 1024 * 1024; + /** + * @brief Max length for pre-authentication / handshake protos (4 KiB). + * + * ConnectRequest and SequenceHeader are tiny; a large declared length would + * pin memory on a handler thread before any auth. + */ + static constexpr int64_t MAX_HANDSHAKE_PROTO_LENGTH = 4 * 1024; + /** * @brief Reads a length-prefixed protobuf from the socket. * @tparam T Protobuf message type. @@ -63,13 +87,22 @@ class SocketHandler { */ template inline T readProto(int fd, bool timeout) { + return readProto(fd, timeout, DEFAULT_MAX_PROTO_LENGTH); + } + + /** + * @brief Reads a length-prefixed protobuf, rejecting lengths above @p + * maxLength. + */ + template + inline T readProto(int fd, bool timeout, int64_t maxLength) { T t; int64_t length; readAll(fd, &length, sizeof(int64_t), timeout); - if (length < 0 || length > 128 * 1024 * 1024) { + if (length < 0 || length > maxLength) { // If the message is <= 0 or too big, assume this is a bad packet and // throw - string s = string("Invalid size (<0 or >128 MB): ") + to_string(length); + string s = string("Invalid size (<0 or >max): ") + to_string(length); throw std::runtime_error(s.c_str()); } if (length == 0) { diff --git a/src/base/UnixSocketHandler.cpp b/src/base/UnixSocketHandler.cpp index c2f48470e..ec72b3840 100644 --- a/src/base/UnixSocketHandler.cpp +++ b/src/base/UnixSocketHandler.cpp @@ -1,5 +1,7 @@ #include "UnixSocketHandler.hpp" +#include + namespace et { UnixSocketHandler::UnixSocketHandler() {} diff --git a/src/base/UserSocketOps.cpp b/src/base/UserSocketOps.cpp new file mode 100644 index 000000000..0cfbd6533 --- /dev/null +++ b/src/base/UserSocketOps.cpp @@ -0,0 +1,269 @@ +#include "UserSocketOps.hpp" + +#ifndef WIN32 +#include +#include +#include +#include + +#ifdef CODE_COVERAGE +extern "C" void __gcov_dump(void); +#endif + +namespace et { +namespace { +struct ResultHeader { + int status; // 0 ok, -1 error + int err; +}; + +void fatalClose(int fd) { + if (fd >= 0) { + ::close(fd); + } +} + +bool pathTooLong(const string& path) { + return path.size() >= sizeof(sockaddr_un::sun_path); +} +} // namespace + +void UserSocketOps::coverageExit(int code) { +#ifdef CODE_COVERAGE + // _exit skips atexit/gcov flush; dump so forked child lines count in reports. + __gcov_dump(); +#endif + _exit(code); +} + +void UserSocketOps::sendFd(int channel, int fdToSend, int status, int err) { + ResultHeader header{status, err}; + struct iovec iov; + iov.iov_base = &header; + iov.iov_len = sizeof(header); + + char control[CMSG_SPACE(sizeof(int))]; + memset(control, 0, sizeof(control)); + + struct msghdr msg; + memset(&msg, 0, sizeof(msg)); + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + + if (status == 0 && fdToSend >= 0) { + msg.msg_control = control; + msg.msg_controllen = sizeof(control); + struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + memcpy(CMSG_DATA(cmsg), &fdToSend, sizeof(int)); + } + + // Best-effort; child exits immediately after. + ::sendmsg(channel, &msg, 0); +} + +int UserSocketOps::recvFd(int channel, int* errOut) { + ResultHeader header; + struct iovec iov; + iov.iov_base = &header; + iov.iov_len = sizeof(header); + + char control[CMSG_SPACE(sizeof(int))]; + memset(control, 0, sizeof(control)); + + struct msghdr msg; + memset(&msg, 0, sizeof(msg)); + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = control; + msg.msg_controllen = sizeof(control); + + ssize_t n = ::recvmsg(channel, &msg, 0); + if (n != (ssize_t)sizeof(header)) { + if (errOut) { + *errOut = EIO; + } + return -1; + } + if (header.status != 0) { + if (errOut) { + *errOut = header.err ? header.err : EIO; + } + return -1; + } + + struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); + if (cmsg == nullptr || cmsg->cmsg_level != SOL_SOCKET || + cmsg->cmsg_type != SCM_RIGHTS || cmsg->cmsg_len < CMSG_LEN(sizeof(int))) { + if (errOut) { + *errOut = EIO; + } + return -1; + } + int fd = -1; + memcpy(&fd, CMSG_DATA(cmsg), sizeof(int)); + if (errOut) { + *errOut = 0; + } + return fd; +} + +int UserSocketOps::listenAtPath(const string& path) { + if (pathTooLong(path)) { + SetErrno(ENAMETOOLONG); + return -1; + } + + int fd = ::socket(AF_UNIX, SOCK_STREAM, 0); + if (fd < 0) { + return -1; + } + + sockaddr_un local; + memset(&local, 0, sizeof(local)); + local.sun_family = AF_UNIX; + strncpy(local.sun_path, path.c_str(), sizeof(local.sun_path) - 1); + + // Only removes a path the current credentials can unlink. + ::unlink(local.sun_path); + + if (::bind(fd, (struct sockaddr*)&local, sizeof(local)) < 0) { + int err = errno; + fatalClose(fd); + SetErrno(err); + return -1; + } + if (::listen(fd, 5) < 0) { + int err = errno; + fatalClose(fd); + SetErrno(err); + return -1; + } + if (::fchmod(fd, S_IRUSR | S_IWUSR | S_IXUSR) < 0) { + // fchmod on unix sockets is unsupported on some platforms; fall back to + // path chmod. Still running with the caller's credentials. + ::chmod(local.sun_path, S_IRUSR | S_IWUSR | S_IXUSR); + } + return fd; +} + +int UserSocketOps::connectAtPath(const string& path) { + if (pathTooLong(path)) { + SetErrno(ENAMETOOLONG); + return -1; + } + + int fd = ::socket(AF_UNIX, SOCK_STREAM, 0); + if (fd < 0) { + return -1; + } + + sockaddr_un remote; + memset(&remote, 0, sizeof(remote)); + remote.sun_family = AF_UNIX; + strncpy(remote.sun_path, path.c_str(), sizeof(remote.sun_path) - 1); + + if (::connect(fd, (struct sockaddr*)&remote, sizeof(remote)) < 0) { + int err = errno; + fatalClose(fd); + SetErrno(err); + return -1; + } + return fd; +} + +void UserSocketOps::childListen(int resultFd, const string& path) { + int fd = listenAtPath(path); + if (fd < 0) { + sendFd(resultFd, -1, -1, GetErrno()); + coverageExit(1); + } + sendFd(resultFd, fd, 0, 0); + fatalClose(fd); + coverageExit(0); +} + +void UserSocketOps::childConnect(int resultFd, const string& path) { + int fd = connectAtPath(path); + if (fd < 0) { + sendFd(resultFd, -1, -1, GetErrno()); + coverageExit(1); + } + sendFd(resultFd, fd, 0, 0); + fatalClose(fd); + coverageExit(0); +} + +int UserSocketOps::runAsUser(Op op, const string& path, uid_t uid, gid_t gid) { + int sv[2]; + if (::socketpair(AF_UNIX, SOCK_STREAM, 0, sv) < 0) { + return -1; + } + + pid_t pid = ::fork(); + if (pid < 0) { + int err = errno; + fatalClose(sv[0]); + fatalClose(sv[1]); + SetErrno(err); + return -1; + } + + if (pid == 0) { + fatalClose(sv[0]); + // Drop privileges before any path operation. Do not use logging here. + // Clear supplemental groups when running as root so we do not retain the + // parent's group set. setgroups(2) requires privilege and is skipped + // otherwise (e.g. already-unprivileged test processes). + if (::geteuid() == 0) { + if (::setgroups(1, &gid) != 0) { + sendFd(sv[1], -1, -1, errno); + coverageExit(1); + } + } + if (::setgid(gid) != 0) { + sendFd(sv[1], -1, -1, errno); + coverageExit(1); + } + if (::setuid(uid) != 0) { + sendFd(sv[1], -1, -1, errno); + coverageExit(1); + } + if (op == Op::LISTEN) { + childListen(sv[1], path); + } else { + childConnect(sv[1], path); + } + coverageExit(1); + } + + fatalClose(sv[1]); + int err = 0; + int fd = recvFd(sv[0], &err); + fatalClose(sv[0]); + + int status = 0; + while (::waitpid(pid, &status, 0) < 0) { + if (errno != EINTR) { + break; + } + } + + if (fd < 0) { + SetErrno(err ? err : EIO); + return -1; + } + return fd; +} + +int UserSocketOps::listenUnixAsUser(const string& path, uid_t uid, gid_t gid) { + return runAsUser(Op::LISTEN, path, uid, gid); +} + +int UserSocketOps::connectUnixAsUser(const string& path, uid_t uid, gid_t gid) { + return runAsUser(Op::CONNECT, path, uid, gid); +} +} // namespace et +#endif diff --git a/src/base/UserSocketOps.hpp b/src/base/UserSocketOps.hpp new file mode 100644 index 000000000..cc07040da --- /dev/null +++ b/src/base/UserSocketOps.hpp @@ -0,0 +1,56 @@ +#ifndef __ET_USER_SOCKET_OPS__ +#define __ET_USER_SOCKET_OPS__ + +#include "Headers.hpp" + +namespace et { +#ifndef WIN32 +/** + * @brief Create or connect UNIX sockets after dropping to a session uid/gid. + * + * etserver is multithreaded and cannot safely seteuid in-process. These helpers + * fork a child, drop privileges, perform the socket operation, and return the + * resulting fd to the parent via SCM_RIGHTS. + */ +class UserSocketOps { + public: + /** + * @brief unlink/bind/listen/fchmod a UNIX socket path as @p uid/@p gid. + * @return Listening fd owned by the caller, or -1 on failure (errno set). + */ + static int listenUnixAsUser(const string& path, uid_t uid, gid_t gid); + + /** + * @brief connect() to a UNIX socket path as @p uid/@p gid. + * @return Connected fd owned by the caller, or -1 on failure (errno set). + */ + static int connectUnixAsUser(const string& path, uid_t uid, gid_t gid); + + /** + * @brief Create a listening UNIX socket at @p path in the current process. + * + * Used after privilege drop in the forked child, and directly by unit tests. + * @return Listening fd, or -1 with errno set. + */ + static int listenAtPath(const string& path); + + /** + * @brief Connect to a UNIX socket at @p path in the current process. + * @return Connected fd, or -1 with errno set. + */ + static int connectAtPath(const string& path); + + private: + enum class Op : int { LISTEN = 1, CONNECT = 2 }; + + static int runAsUser(Op op, const string& path, uid_t uid, gid_t gid); + static void childListen(int resultFd, const string& path); + static void childConnect(int resultFd, const string& path); + static void sendFd(int channel, int fdToSend, int status, int err); + static int recvFd(int channel, int* errOut); + static void coverageExit(int code); +}; +#endif +} // namespace et + +#endif // __ET_USER_SOCKET_OPS__ diff --git a/src/htm/HtmServer.cpp b/src/htm/HtmServer.cpp index 0cc952dda..06d416be3 100644 --- a/src/htm/HtmServer.cpp +++ b/src/htm/HtmServer.cpp @@ -1,5 +1,7 @@ #include "HtmServer.hpp" +#include + #include "HtmHeaderCodes.hpp" #include "LogHandler.hpp" #include "MultiplexerState.hpp" diff --git a/src/htm/MultiplexerState.cpp b/src/htm/MultiplexerState.cpp index 7e0abb153..44bc722d5 100644 --- a/src/htm/MultiplexerState.cpp +++ b/src/htm/MultiplexerState.cpp @@ -1,5 +1,7 @@ #include "MultiplexerState.hpp" +#include + #include "HtmHeaderCodes.hpp" #include "JsonLib.hpp" diff --git a/src/terminal/TerminalClient.cpp b/src/terminal/TerminalClient.cpp index 2df893b10..6811fe876 100644 --- a/src/terminal/TerminalClient.cpp +++ b/src/terminal/TerminalClient.cpp @@ -1,5 +1,7 @@ #include "TerminalClient.hpp" +#include + #include "TelemetryService.hpp" #include "TunnelUtils.hpp" #include "WriteBuffer.hpp" @@ -193,6 +195,13 @@ void TerminalClient::run(const string& command, const bool noexit) { CLOG(INFO, "stdout") << "ET running, feel free to background..." << endl; } + // Launchers such as nohup(1) replace stdout with a regular file while + // leaving the tty on stdin, so the descriptor Console exposes for input is + // readable-but-empty (or not readable at all). select() always reports it + // ready and every read yields EOF/EBADF, which must not be mistaken for the + // user closing an interactive session -- doing so would tear down a + // backgrounded client and its port forwards immediately. + bool consoleInputDisabled = false; while (!connection->isShuttingDown()) { { lock_guard guard(shutdownMutex); @@ -208,7 +217,7 @@ void TerminalClient::run(const string& command, const bool noexit) { FD_ZERO(&rfd); int maxfd = -1; int consoleFd = -1; - if (console) { + if (console && !consoleInputDisabled) { consoleFd = console->getFd(); maxfd = consoleFd; FD_SET(consoleFd, &rfd); @@ -252,7 +261,7 @@ void TerminalClient::run(const string& command, const bool noexit) { } } - if (console) { + if (console && consoleFd >= 0) { // Check for data to send. if (FD_ISSET(consoleFd, &rfd)) { // Read from stdin and write to our client that will then send it to @@ -300,11 +309,21 @@ void TerminalClient::run(const string& command, const bool noexit) { TerminalPacketType::TERMINAL_BUFFER, protoToString(tb))); keepaliveTime = time(NULL) + keepaliveDuration; } else if (rc == 0) { - LOG(INFO) << "Console EOF"; - break; + if (isatty(consoleFd)) { + LOG(INFO) << "Console EOF"; + break; + } + LOG(INFO) << "Console is not a tty and is at EOF, disabling " + "console input"; + consoleInputDisabled = true; } else { if (savedErrno == EAGAIN || savedErrno == EWOULDBLOCK) { // Transient error, retry + } else if (!isatty(consoleFd)) { + LOG(INFO) << "Console is not a tty and cannot be read (" + << savedErrno << "): " << strerror(savedErrno) + << ", disabling console input"; + consoleInputDisabled = true; } else { LOG(INFO) << "Console read error: (" << savedErrno << "): " << strerror(savedErrno); diff --git a/src/terminal/TerminalServer.cpp b/src/terminal/TerminalServer.cpp index 9ba035315..0d609261a 100644 --- a/src/terminal/TerminalServer.cpp +++ b/src/terminal/TerminalServer.cpp @@ -1,6 +1,8 @@ #ifndef WIN32 #include "TerminalServer.hpp" +#include + #include "TelemetryService.hpp" #include "WriteBuffer.hpp" @@ -302,8 +304,8 @@ void TerminalServer::runTerminal( InitialResponse response; shared_ptr serverSocketHandler = getSocketHandler(); shared_ptr pipeSocketHandler(new PipeSocketHandler()); - shared_ptr portForwardHandler( - new PortForwardHandler(serverSocketHandler, pipeSocketHandler)); + shared_ptr portForwardHandler(new PortForwardHandler( + serverSocketHandler, pipeSocketHandler, userInfo.uid(), userInfo.gid())); map environmentVariables; for (const auto& envVar : payload.environmentvariables()) { diff --git a/src/terminal/UserTerminalHandler.cpp b/src/terminal/UserTerminalHandler.cpp index 011a223a3..466675a8f 100644 --- a/src/terminal/UserTerminalHandler.cpp +++ b/src/terminal/UserTerminalHandler.cpp @@ -1,6 +1,8 @@ #ifndef WIN32 #include "UserTerminalHandler.hpp" +#include + #include "ETerminal.pb.h" #include "RawSocketUtils.hpp" #include "ServerConnection.hpp" diff --git a/src/terminal/forwarding/ForwardSourceHandler.cpp b/src/terminal/forwarding/ForwardSourceHandler.cpp index 308013e3f..68535c6cc 100644 --- a/src/terminal/forwarding/ForwardSourceHandler.cpp +++ b/src/terminal/forwarding/ForwardSourceHandler.cpp @@ -3,11 +3,13 @@ namespace et { ForwardSourceHandler::ForwardSourceHandler( shared_ptr _socketHandler, const SocketEndpoint& _source, - const SocketEndpoint& _destination) + const SocketEndpoint& _destination, bool alreadyListening) : socketHandler(_socketHandler), source(_source), destination(_destination) { - socketHandler->listen(source); + if (!alreadyListening) { + socketHandler->listen(source); + } } ForwardSourceHandler::~ForwardSourceHandler() { diff --git a/src/terminal/forwarding/ForwardSourceHandler.hpp b/src/terminal/forwarding/ForwardSourceHandler.hpp index 7d7905aeb..87f7d1b6a 100644 --- a/src/terminal/forwarding/ForwardSourceHandler.hpp +++ b/src/terminal/forwarding/ForwardSourceHandler.hpp @@ -11,11 +11,15 @@ namespace et { */ class ForwardSourceHandler { public: - /** @brief Creates source/destination handlers used for local port forwarding. + /** + * @brief Creates source/destination handlers used for local port forwarding. + * @param alreadyListening If true, skip listen(); caller already registered + * the source endpoint (e.g. via listenAsUser). */ ForwardSourceHandler(shared_ptr _socketHandler, const SocketEndpoint& _source, - const SocketEndpoint& _destination); + const SocketEndpoint& _destination, + bool alreadyListening = false); ~ForwardSourceHandler(); diff --git a/src/terminal/forwarding/PortForwardHandler.cpp b/src/terminal/forwarding/PortForwardHandler.cpp index 9e81473ac..b2e7c6fce 100644 --- a/src/terminal/forwarding/PortForwardHandler.cpp +++ b/src/terminal/forwarding/PortForwardHandler.cpp @@ -1,11 +1,17 @@ #include "PortForwardHandler.hpp" +#include + +#include "PipeSocketHandler.hpp" + namespace et { PortForwardHandler::PortForwardHandler( shared_ptr _networkSocketHandler, - shared_ptr _pipeSocketHandler) + shared_ptr _pipeSocketHandler, uid_t userid, gid_t groupid) : networkSocketHandler(_networkSocketHandler), - pipeSocketHandler(_pipeSocketHandler) {} + pipeSocketHandler(_pipeSocketHandler), + sessionUid(userid), + sessionGid(groupid) {} void PortForwardHandler::update(vector* requests, vector* dataToSend) { @@ -75,14 +81,23 @@ PortForwardSourceResponse PortForwardHandler::createSource( sourceHandlers.push_back(handler); return PortForwardSourceResponse(); } else { - auto handler = shared_ptr(new ForwardSourceHandler( - pipeSocketHandler, source, pfsr.destination())); #ifndef WIN32 - if (userid >= 0 && groupid >= 0) { - FATAL_FAIL(::chmod(source.name().c_str(), S_IRUSR | S_IWUSR | S_IXUSR)); - FATAL_FAIL(::chown(source.name().c_str(), userid, groupid)); + // Perform unlink/bind/listen as the session user so a client-chosen path + // cannot delete or chown root-owned files. + auto concretePipe = + dynamic_pointer_cast(pipeSocketHandler); + if (concretePipe && userid != static_cast(-1) && + groupid != static_cast(-1)) { + concretePipe->listenAsUser(source, userid, groupid); + auto handler = + shared_ptr(new ForwardSourceHandler( + pipeSocketHandler, source, pfsr.destination(), true)); + sourceHandlers.push_back(handler); + return PortForwardSourceResponse(); } #endif + auto handler = shared_ptr(new ForwardSourceHandler( + pipeSocketHandler, source, pfsr.destination())); sourceHandlers.push_back(handler); return PortForwardSourceResponse(); } @@ -112,7 +127,21 @@ PortForwardDestinationResponse PortForwardHandler::createDestination( fd = networkSocketHandler->connect(ipv4Localhost); } } else { +#ifndef WIN32 + // Connect as the session user so root etserver cannot open privileged + // sockets (e.g. docker.sock) on behalf of an unprivileged client. + auto concretePipe = + dynamic_pointer_cast(pipeSocketHandler); + if (concretePipe && sessionUid != static_cast(-1) && + sessionGid != static_cast(-1)) { + fd = concretePipe->connectAsUser(pfdr.destination(), sessionUid, + sessionGid); + } else { + fd = pipeSocketHandler->connect(pfdr.destination()); + } +#else fd = pipeSocketHandler->connect(pfdr.destination()); +#endif } PortForwardDestinationResponse pfdresponse; pfdresponse.set_clientfd(pfdr.fd()); diff --git a/src/terminal/forwarding/PortForwardHandler.hpp b/src/terminal/forwarding/PortForwardHandler.hpp index b338bd5f5..fc7af75d0 100644 --- a/src/terminal/forwarding/PortForwardHandler.hpp +++ b/src/terminal/forwarding/PortForwardHandler.hpp @@ -14,9 +14,16 @@ namespace et { */ class PortForwardHandler { public: - /** @brief Constructs forwarding helpers for network and router sockets. */ + /** + * @brief Constructs forwarding helpers for network and router sockets. + * @param userid Session uid used for privilege-dropped UNIX socket ops + * ((uid_t)-1 to disable). + * @param groupid Session gid used with @p userid. + */ explicit PortForwardHandler(shared_ptr _networkSocketHandler, - shared_ptr _pipeSocketHandler); + shared_ptr _pipeSocketHandler, + uid_t userid = static_cast(-1), + gid_t groupid = static_cast(-1)); /** @brief Polls all handlers for new destination/data and sends * `PortForwardData`. */ void update(vector* requests, @@ -48,6 +55,10 @@ class PortForwardHandler { shared_ptr networkSocketHandler; /** @brief Handler used for the router/pipe-facing sockets. */ shared_ptr pipeSocketHandler; + /** @brief Session uid for UNIX connect/listen; (uid_t)-1 disables drop. */ + uid_t sessionUid; + /** @brief Session gid for UNIX connect/listen; (gid_t)-1 disables drop. */ + gid_t sessionGid; /** @brief Active destination handlers keyed by socket id. */ unordered_map> destinationHandlers; diff --git a/test/FakeConsole.hpp b/test/FakeConsole.hpp index 6acc7f9ab..885464959 100644 --- a/test/FakeConsole.hpp +++ b/test/FakeConsole.hpp @@ -60,16 +60,29 @@ class FakeConsole : public Console { socketHandler, endpoint, &serverClientFd); // Wait for server to spin up ::usleep(1000 * 1000); - clientServerFd = socketHandler->connect(endpoint); - FATAL_FAIL(clientServerFd); + int fd = socketHandler->connect(endpoint); + FATAL_FAIL(fd); + { + lock_guard lock(_mutex); + clientServerFd = fd; + } serverListenThread.join(); FATAL_FAIL(serverClientFd); LOG(INFO) << "FDs: " << clientServerFd << " " << serverClientFd; } virtual void teardown() { - socketHandler->close(clientServerFd); - socketHandler->close(serverClientFd); + int localClientServerFd; + int localServerClientFd; + { + lock_guard lock(_mutex); + localClientServerFd = clientServerFd; + localServerClientFd = serverClientFd; + clientServerFd = -1; + serverClientFd = -1; + } + socketHandler->close(localClientServerFd); + socketHandler->close(localServerClientFd); FATAL_FAIL(::remove(pipePath.c_str())); FATAL_FAIL(::remove(pipeDirectory.c_str())); } @@ -83,17 +96,37 @@ class FakeConsole : public Console { return fakeTerminalInfo; } - virtual int getFd() { return clientServerFd; } + virtual int getFd() { + lock_guard lock(_mutex); + return clientServerFd; + } + + bool isSetup() { + lock_guard lock(_mutex); + return clientServerFd >= 0 && serverClientFd >= 0; + } string getTerminalData(int count) { string s(count, '\0'); - socketHandler->readAll(serverClientFd, &s[0], count, false); + int fd; + { + lock_guard lock(_mutex); + fd = serverClientFd; + } + socketHandler->readAll(fd, &s[0], count, false); return s; } void simulateKeystrokes(const string& s) { - LOG(INFO) << "FDs: " << clientServerFd << " " << serverClientFd; - socketHandler->writeAllOrThrow(serverClientFd, s.c_str(), s.length(), + int localClientServerFd; + int localServerClientFd; + { + lock_guard lock(_mutex); + localClientServerFd = clientServerFd; + localServerClientFd = serverClientFd; + } + LOG(INFO) << "FDs: " << localClientServerFd << " " << localServerClientFd; + socketHandler->writeAllOrThrow(localServerClientFd, s.c_str(), s.length(), false); } diff --git a/test/Main.cpp b/test/Main.cpp index db783f6da..7c8a37356 100644 --- a/test/Main.cpp +++ b/test/Main.cpp @@ -44,6 +44,11 @@ int main(int argc, char** argv) { TelemetryService::get()->shutdown(); TelemetryService::destroy(); - FATAL_FAIL(fs::remove_all(logDirectory.c_str())); + try { + fs::remove_all(logDirectory); + } catch (const fs::filesystem_error& e) { + LOG(WARNING) << "Failed to remove test log directory " << logDirectory + << ": " << e.what(); + } return result; } diff --git a/test/e2e/README.md b/test/e2e/README.md index d9ac6fae5..f93eab439 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -36,7 +36,9 @@ bash test/e2e/run_e2e_test.sh discard 100000 ``` The script starts etserver (port 4444), a throttle proxy (port 4445 -> 4444 at -100KB/s), etterminal, and the et client in a tmux session. It then runs +100KB/s), etterminal, and the et client in a tmux session. It refuses to start +if either fixed port is already in use and cleanup only terminates processes +started by that invocation. It then runs `print_timestamps.py` and measures: - **display_lag**: how far behind the timestamps on screen are from real time @@ -137,5 +139,6 @@ crontab -r - **etserver crashes with EINVAL**: Fixed by handling EBADF/EINVAL in `waitOnSocketWritable()` (see Headers.hpp). The fd becomes invalid when the client disconnects mid-drain. -- **"Connection refused" on proxy**: The proxy process died. Check if port 4445 - is already in use (`lsof -i:4445`). +- **"TCP port ... is already in use"**: Another process owns one of the fixed + test ports. Stop that process yourself or run the test later; the harness + intentionally never kills an existing listener. diff --git a/test/e2e/do_scenario.sh b/test/e2e/do_scenario.sh index e86bc8083..ba9195612 100644 --- a/test/e2e/do_scenario.sh +++ b/test/e2e/do_scenario.sh @@ -31,13 +31,57 @@ KEY="E2ETestKey123456789012345678901A" SIDECAR="/tmp/et_e2e_sidecar_$$.txt" ETMUX="/tmp/et_e2e_client_$$.sock" PROXY_LOG="/tmp/et_e2e_proxy_$$.log" +FIFO="/tmp/et_e2e_demo_$$.fifo" +ETSERVER_PID="" +PROXY_PID="" +ETTERMINAL_PID="" + +# The scenario uses fixed local ports. Never reclaim them by killing an +# existing listener: it may belong to another test or a real service. +require_free_port() { + local port="$1" + if ! python3 - "$port" <<'PY' +import errno +import socket +import sys + +port = int(sys.argv[1]) +sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) +sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) +try: + sock.bind(("::", port)) +except OSError as exc: + if exc.errno == errno.EADDRINUSE: + sys.exit(1) + raise +finally: + sock.close() +PY + then + echo "ERROR: TCP port $port is already in use; refusing to stop an unrelated process." >&2 + exit 1 + fi +} + +stop_child() { + local pid="${1:-}" + [ -n "$pid" ] || return + if jobs -pr | grep -Fxq "$pid"; then + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fi +} + +require_free_port 4444 +require_free_port 4445 # The cleanup trap restores src/, proto/, and test dirs to HEAD (the trunk # scenario checks out old sources there). Any uncommitted work in those # paths would be silently destroyed, so refuse to run on a dirty tree. -if ! git -C "$REPO" diff --quiet -- src/ proto/ test/integration_tests/ test/unit_tests/ 2>/dev/null; then - echo "ERROR: uncommitted changes under src/, proto/, or test/. Commit them first:" - git -C "$REPO" status --short -- src/ proto/ test/integration_tests/ test/unit_tests/ | head +if ! git -C "$REPO" diff --quiet -- src/ proto/ test/integration_tests/ test/unit_tests/ test/e2e/do_scenario.sh test/e2e/run_all_scenarios.sh 2>/dev/null || + ! git -C "$REPO" diff --cached --quiet -- src/ proto/ test/integration_tests/ test/unit_tests/ test/e2e/do_scenario.sh test/e2e/run_all_scenarios.sh 2>/dev/null; then + echo "ERROR: uncommitted changes under paths restored by this harness. Commit them first:" + git -C "$REPO" status --short -- src/ proto/ test/integration_tests/ test/unit_tests/ test/e2e/do_scenario.sh test/e2e/run_all_scenarios.sh | head exit 1 fi @@ -52,10 +96,10 @@ C='\033[1;36m'; G='\033[1;32m'; R='\033[1;31m'; Y='\033[33m'; N='\033[0m' cleanup() { tmux -S "$ETMUX" kill-server 2>/dev/null - kill $ETSERVER_PID $PROXY_PID 2>/dev/null - kill -9 $(lsof -t -i:4444 2>/dev/null) $(lsof -t -i:4445 2>/dev/null) 2>/dev/null - pkill -9 -f "etterminal.*$ID" 2>/dev/null - rm -f /tmp/et_e2e_demo.fifo "$ETMUX" "$SIDECAR" "$PROXY_LOG" + stop_child "$ETSERVER_PID" + stop_child "$PROXY_PID" + stop_child "$ETTERMINAL_PID" + rm -f "$FIFO" "$ETMUX" "$SIDECAR" "$PROXY_LOG" # Restore only the dirs the trunk scenario checks out. Do NOT use # "git checkout HEAD -- ." here: it clobbers unrelated uncommitted work # in the tree (this exact mistake once reverted the flow-control @@ -117,9 +161,9 @@ echo "" # --- Start services --- echo -e "${C}--- Services ---${N}" -rm -f /tmp/et_e2e_demo.fifo +rm -f "$FIFO" -"$BUILD/etserver" --serverfifo=/tmp/et_e2e_demo.fifo --port=4444 --logdir="$OUTDIR" &>/dev/null & +"$BUILD/etserver" --serverfifo="$FIFO" --port=4444 --logdir="$OUTDIR" &>/dev/null & ETSERVER_PID=$! sleep 3 @@ -127,12 +171,14 @@ PYTHONUNBUFFERED=1 python3 -u "$DIR/throttle_proxy.py" 4445 4444 "$RATE" >"$PROX PROXY_PID=$! sleep 2 -"$BUILD/etterminal" --idpasskey="$ID/$KEY" --serverfifo=/tmp/et_e2e_demo.fifo --logdir="$OUTDIR" &>/dev/null & +"$BUILD/etterminal" --idpasskey="$ID/$KEY" --serverfifo="$FIFO" --logdir="$OUTDIR" &>/dev/null & +ETTERMINAL_PID=$! sleep 3 for i in $(seq 1 15); do - ES=$(lsof -i:4444 2>/dev/null | grep -c LISTEN) - PX=$(lsof -i:4445 2>/dev/null | grep -c LISTEN) + ES=0; PX=0 + kill -0 "$ETSERVER_PID" 2>/dev/null && ES=1 + kill -0 "$PROXY_PID" 2>/dev/null && PX=1 [ "$ES" -ge 1 ] && [ "$PX" -ge 1 ] && break sleep 1 done @@ -256,7 +302,8 @@ fi echo "{\"event\":\"ctrl_c\",\"latency\":$CTRL_C_LATENCY}" >> "$JSONL" echo "" -ES_ALIVE=$(lsof -i:4444 2>/dev/null | grep -c LISTEN) +ES_ALIVE=0 +kill -0 "$ETSERVER_PID" 2>/dev/null && ES_ALIVE=1 [ "$ES_ALIVE" -gt 0 ] && echo -e "etserver: ${G}alive${N}" || echo -e "etserver: ${R}CRASHED${N}" cp "$PROXY_LOG" "$OUTDIR/${SCENARIO}_proxy.log" 2>/dev/null diff --git a/test/e2e/run_all_scenarios.sh b/test/e2e/run_all_scenarios.sh index 689701b32..a12ba5932 100644 --- a/test/e2e/run_all_scenarios.sh +++ b/test/e2e/run_all_scenarios.sh @@ -24,10 +24,6 @@ echo "Output: $OUTDIR" echo "Started: $(date)" echo "" -kill -9 $(lsof -t -i:4444 2>/dev/null) $(lsof -t -i:4445 2>/dev/null) 2>/dev/null || true -pkill -9 -f throttle_proxy 2>/dev/null || true -sleep 2 - for SCENARIO in "trunk" "backpressure" "discard" "discard --disconnect"; do MODE=$(echo "$SCENARIO" | awk '{print $1}') EXTRA=$(echo "$SCENARIO" | awk '{$1=""; print $0}' | xargs) @@ -35,15 +31,11 @@ for SCENARIO in "trunk" "backpressure" "discard" "discard --disconnect"; do echo " Scenario: $MODE $EXTRA" echo "================================================================" - kill -9 $(lsof -t -i:4444 2>/dev/null) $(lsof -t -i:4445 2>/dev/null) 2>/dev/null || true - pkill -9 -f throttle_proxy 2>/dev/null || true - sleep 3 - bash "$DIR/do_scenario.sh" $SCENARIO --outdir "$OUTDIR" echo "" done -cd "$REPO" && git checkout HEAD -- . 2>/dev/null +cd "$REPO" && git checkout HEAD -- src/ proto/ test/integration_tests/ test/unit_tests/ 2>/dev/null cd build && cmake -DDISABLE_VCPKG=ON -GNinja .. 2>&1 | tail -1 && ninja -j4 2>&1 | tail -1 echo "================================================================" diff --git a/test/e2e/run_e2e_test.sh b/test/e2e/run_e2e_test.sh index ea4d0a713..3c77d8a84 100644 --- a/test/e2e/run_e2e_test.sh +++ b/test/e2e/run_e2e_test.sh @@ -16,33 +16,75 @@ FIFO="/tmp/et_e2e_$$.fifo" ID="E2E$(date +%s)" KEY="E2ETestKey123456789012345678901A" SIDECAR="/tmp/et_e2e_sidecar_$$.txt" +PROXY_LOG="/tmp/et_e2e_proxy_$$.log" +ETSERVER_PID="" +PROXY_PID="" +ETTERMINAL_PID="" + +# The test uses fixed local ports. Never reclaim them by killing an existing +# listener: it may belong to another test or a real service. +require_free_port() { + local port="$1" + if ! python3 - "$port" <<'PY' +import errno +import socket +import sys + +port = int(sys.argv[1]) +sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) +sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) +try: + sock.bind(("::", port)) +except OSError as exc: + if exc.errno == errno.EADDRINUSE: + sys.exit(1) + raise +finally: + sock.close() +PY + then + echo "ERROR: TCP port $port is already in use; refusing to stop an unrelated process." >&2 + exit 1 + fi +} + +stop_child() { + local pid="${1:-}" + [ -n "$pid" ] || return + if jobs -pr | grep -Fxq "$pid"; then + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fi +} cleanup() { tmux -S "$SOCK" kill-server 2>/dev/null || true - kill -9 $(lsof -t -i:4444 2>/dev/null) 2>/dev/null || true - kill -9 $(lsof -t -i:4445 2>/dev/null) 2>/dev/null || true - pkill -9 -f "etterminal.*$ID" 2>/dev/null || true - rm -f "$FIFO" "$SOCK" "$SIDECAR" || true + stop_child "$ETSERVER_PID" + stop_child "$PROXY_PID" + stop_child "$ETTERMINAL_PID" + rm -f "$FIFO" "$SOCK" "$SIDECAR" "$PROXY_LOG" || true } trap cleanup EXIT -cleanup 2>/dev/null + +require_free_port 4444 +require_free_port 4445 # Start services tmux -S "$SOCK" new-session -d -s t -x 200 -y 50 -tmux -S "$SOCK" send-keys "$BUILD/etserver --serverfifo=$FIFO --port=4444" Enter +"$BUILD/etserver" --serverfifo="$FIFO" --port=4444 &>/dev/null & +ETSERVER_PID=$! sleep 5 -tmux -S "$SOCK" split-window -v -sleep 1 -tmux -S "$SOCK" send-keys "python3 $DIR/throttle_proxy.py 4445 4444 $RATE" Enter +python3 "$DIR/throttle_proxy.py" 4445 4444 "$RATE" >"$PROXY_LOG" 2>&1 & +PROXY_PID=$! sleep 3 -tmux -S "$SOCK" split-window -v -sleep 1 -tmux -S "$SOCK" send-keys "$BUILD/etterminal --idpasskey='$ID/$KEY' --serverfifo=$FIFO" Enter +"$BUILD/etterminal" --idpasskey="$ID/$KEY" --serverfifo="$FIFO" &>/dev/null & +ETTERMINAL_PID=$! sleep 5 # Verify -ES=$(lsof -i:4444 2>/dev/null | grep -c LISTEN) -PX=$(lsof -i:4445 2>/dev/null | grep -c LISTEN) +ES=0; PX=0 +kill -0 "$ETSERVER_PID" 2>/dev/null && ES=1 +kill -0 "$PROXY_PID" 2>/dev/null && PX=1 if [ "$ES" -lt 1 ] || [ "$PX" -lt 1 ]; then echo "FAILED: etserver=$ES proxy=$PX" exit 1 @@ -58,7 +100,7 @@ tmux -S "$SOCK" send-keys -t t:et "$BUILD/et --idpasskey='$ID/$KEY' 127.0.0.1:44 sleep 10 # Verify proxy connection -PROXY_OUT=$(tmux -S "$SOCK" capture-pane -t t:0.1 -p) +PROXY_OUT=$(cat "$PROXY_LOG" 2>/dev/null) if ! echo "$PROXY_OUT" | grep -q connect; then echo "FAILED: proxy saw no connection" echo "$PROXY_OUT" | tail -5 @@ -98,5 +140,5 @@ done echo "" echo "Proxy stats:" -tmux -S "$SOCK" capture-pane -t t:0.1 -p | grep -E 'sent=|connect' | tail -3 -echo "etserver alive: $(lsof -i:4444 2>/dev/null | grep -c LISTEN)" +grep -E 'sent=|connect' "$PROXY_LOG" 2>/dev/null | tail -3 +echo "etserver alive: $(kill -0 "$ETSERVER_PID" 2>/dev/null && echo 1 || echo 0)" diff --git a/test/e2e/throughput_test.sh b/test/e2e/throughput_test.sh index c3d8439a8..b10c6c6f8 100644 --- a/test/e2e/throughput_test.sh +++ b/test/e2e/throughput_test.sh @@ -24,20 +24,60 @@ ID="TPT$(date +%s | tail -c 8)" KEY="E2ETestKey123456789012345678901A" ETMUX="/tmp/et_tpt_client_$$.sock" FIFO="/tmp/et_tpt_$$.fifo" +ETSERVER_PID="" +ETTERMINAL_PID="" + +# The test uses a fixed local port. Never reclaim it by killing an existing +# listener: it may belong to another test or a real service. +require_free_port() { + local port="$1" + if ! python3 - "$port" <<'PY' +import errno +import socket +import sys + +port = int(sys.argv[1]) +sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) +sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 0) +try: + sock.bind(("::", port)) +except OSError as exc: + if exc.errno == errno.EADDRINUSE: + sys.exit(1) + raise +finally: + sock.close() +PY + then + echo "ERROR: TCP port $port is already in use; refusing to stop an unrelated process." >&2 + exit 1 + fi +} + +stop_child() { + local pid="${1:-}" + [ -n "$pid" ] || return + if jobs -pr | grep -Fxq "$pid"; then + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fi +} cleanup() { tmux -S "$ETMUX" kill-server 2>/dev/null - kill $ETSERVER_PID 2>/dev/null - kill -9 $(lsof -t -i:$PORT 2>/dev/null) 2>/dev/null - pkill -9 -f "etterminal.*$ID" 2>/dev/null + stop_child "$ETSERVER_PID" + stop_child "$ETTERMINAL_PID" rm -f "$FIFO" "$ETMUX" } trap cleanup EXIT +require_free_port "$PORT" + "$BUILD/etserver" --serverfifo="$FIFO" --port=$PORT --logdir=/tmp &>/dev/null & ETSERVER_PID=$! sleep 2 "$BUILD/etterminal" --idpasskey="$ID/$KEY" --serverfifo="$FIFO" --logdir=/tmp &>/dev/null & +ETTERMINAL_PID=$! sleep 2 tmux -S "$ETMUX" new-session -d -s et -x 200 -y 50 diff --git a/test/integration_tests/TerminalTest.cpp b/test/integration_tests/TerminalTest.cpp index 3966bf9bb..51c35aa5f 100644 --- a/test/integration_tests/TerminalTest.cpp +++ b/test/integration_tests/TerminalTest.cpp @@ -24,6 +24,18 @@ namespace et { +void waitForFakeConsoleSetup(const shared_ptr& fakeConsole) { + const auto deadline = + std::chrono::steady_clock::now() + std::chrono::seconds(30); + while (std::chrono::steady_clock::now() < deadline) { + if (fakeConsole->isSetup()) { + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + REQUIRE(fakeConsole->isSetup()); +} + void readWriteTest(shared_ptr routerSocketHandler, shared_ptr fakeUserTerminal, SocketEndpoint serverEndpoint, @@ -55,6 +67,7 @@ void readWriteTest(shared_ptr routerSocketHandler, thread terminalClientThread( [terminalClient]() { terminalClient->run("", false); }); sleep(3); + waitForFakeConsoleSetup(fakeConsole); string s(1024, '\0'); for (int a = 0; a < 1024; a++) { @@ -181,6 +194,7 @@ void largeInputNoDeadlockTest(shared_ptr routerSocketHandler, thread terminalClientThread( [terminalClient]() { terminalClient->run("", false); }); sleep(3); + waitForFakeConsoleSetup(fakeConsole); // Before the bulk test, confirm the full client -> pty(`cat`) -> client echo // pipe is actually live, and drain anything the connection produced during @@ -265,6 +279,138 @@ void largeInputNoDeadlockTest(shared_ptr routerSocketHandler, uth.reset(); } +// A Console whose descriptor is a regular file opened read/write, reproducing +// what nohup(1) hands a backgrounded client: output lands in the file, and +// reads of the same descriptor always return EOF. +class FileBackedConsole : public Console { + public: + FileBackedConsole() : fd(-1) { + string tmpPath = GetTempDirectory() + string("et_test_nohup_XXXXXXXX"); + directory = string(mkdtemp(&tmpPath[0])); + path = directory + "/nohup.out"; + fd = ::open(path.c_str(), O_RDWR | O_CREAT | O_APPEND, 0600); + FATAL_FAIL(fd); + } + + virtual ~FileBackedConsole() {} + + virtual void setup() {} + + virtual void teardown() { + if (fd >= 0) { + ::close(fd); + fd = -1; + } + ::remove(path.c_str()); + ::remove(directory.c_str()); + } + + virtual TerminalInfo getTerminalInfo() { + TerminalInfo ti; + ti.set_row(24); + ti.set_column(80); + ti.set_width(640); + ti.set_height(480); + return ti; + } + + virtual int getFd() { return fd; } + + string readBackContents() { + string contents; + int readFd = ::open(path.c_str(), O_RDONLY); + if (readFd < 0) { + return contents; + } + char buf[4096]; + int rc; + while ((rc = ::read(readFd, buf, sizeof(buf))) > 0) { + contents.append(buf, rc); + } + ::close(readFd); + return contents; + } + + protected: + int fd; + string directory; + string path; +}; + +// A client whose console descriptor is not a tty must keep running: it can no +// longer accept keyboard input, but the session and its port forwards have to +// survive so a backgrounded client is not torn down the moment the first +// console read returns EOF. Console::getFd() is STDOUT_FILENO, so any +// launcher that points stdout at something other than the tty -- nohup(1), a +// shell redirect, a service manager -- lands here. +void nonTtyConsoleKeepsSessionAliveTest( + shared_ptr routerSocketHandler, + shared_ptr fakeUserTerminal, + SocketEndpoint serverEndpoint, + shared_ptr clientSocketHandler, + shared_ptr clientPipeSocketHandler, + const SocketEndpoint& routerEndpoint) { + auto fakeSubprocessUtils = make_shared(); + auto sshSetupHandler = make_shared(fakeSubprocessUtils); + auto [id, passkey] = sshSetupHandler->SetupSsh( + "", "localhost", "localhost", 2022, "", "", false, 0, "", "", {}); + + auto uth = shared_ptr( + new UserTerminalHandler(routerSocketHandler, fakeUserTerminal, true, + routerEndpoint, id + "/" + passkey)); + thread uthThread([uth]() { uth->run(); }); + sleep(1); + + auto fileConsole = make_shared(); + shared_ptr terminalClient( + new TerminalClient(clientSocketHandler, clientPipeSocketHandler, + serverEndpoint, id, passkey, fileConsole, false, "", + "", false, "", MAX_CLIENT_KEEP_ALIVE_DURATION, {})); + + std::atomic runReturned(false); + thread terminalClientThread([terminalClient, &runReturned]() { + terminalClient->run("", false); + runReturned = true; + }); + sleep(3); + + // Pre-fix, the first console read returned EOF and run() exited within + // milliseconds of the connection coming up. + REQUIRE(!runReturned.load()); + + // The session is not merely alive, it is still usable: remote output has to + // reach the same descriptor. + const string remoteOutput = "ET_NOHUP_OUTPUT_MARKER"; + fakeUserTerminal->simulateTerminalResponse(remoteOutput); + + std::promise sawOutputPromise; + auto sawOutputFuture = sawOutputPromise.get_future(); + thread readThread([&sawOutputPromise, fileConsole, remoteOutput]() { + while (fileConsole->readBackContents().find(remoteOutput) == string::npos) { + ::usleep(100 * 1000); + } + sawOutputPromise.set_value(true); + }); + bool sawOutput = sawOutputFuture.wait_for(std::chrono::seconds(30)) == + std::future_status::ready; + REQUIRE(sawOutput); + if (sawOutput) { + readThread.join(); + } else { + readThread.detach(); + } + + REQUIRE(!runReturned.load()); + + terminalClient->shutdown(); + terminalClientThread.join(); + terminalClient.reset(); + + uth->shutdown(); + uthThread.join(); + uth.reset(); +} + class LogInterceptHandler : public el::LogDispatchCallback { public: void handle(const el::LogDispatchData* data) { @@ -432,6 +578,13 @@ TEST_CASE_METHOD(EndToEndTestFixture, "LargeInputNoDeadlock", fakeConsole, routerEndpoint); } +TEST_CASE_METHOD(EndToEndTestFixture, "NonTtyConsoleKeepsSessionAlive", + "[EndToEndTest][integration]") { + nonTtyConsoleKeepsSessionAliveTest(routerSocketHandler, fakeUserTerminal, + serverEndpoint, clientSocketHandler, + clientPipeSocketHandler, routerEndpoint); +} + void simultaneousTerminalConnectionTest( LogInterceptHandler& logInterceptHandler, shared_ptr routerSocketHandler, diff --git a/test/unit_tests/BackedIOTest.cpp b/test/unit_tests/BackedIOTest.cpp index 9f21bfc30..465fd59f7 100644 --- a/test/unit_tests/BackedIOTest.cpp +++ b/test/unit_tests/BackedIOTest.cpp @@ -424,3 +424,37 @@ TEST_CASE("BackedWriter trims old data when connected and buffer exceeds 64MB", handler->close(fd); } + +TEST_CASE("BackedWriter recover rejects client-ahead sequence without aborting", + "[BackedIO]") { + auto handler = make_shared(); + auto encryptCrypto = make_shared( + "12345678901234567890123456789012", 0 /*verbosity*/); + const int fd = handler->createChannel(); + + BackedWriter writer(handler, encryptCrypto, fd); + REQUIRE(writer.write(Packet(1, "one")) == BackedWriterWriteState::SUCCESS); + writer.invalidateSocket(); + + REQUIRE_THROWS_AS(writer.recover(writer.getSequenceNumber() + 1), + std::runtime_error); +} + +TEST_CASE("SocketHandler readProto enforces max length before allocating", + "[SocketHandler]") { + FdSocketHandler handler; + int fds[2]; + REQUIRE(::pipe(fds) == 0); + + int64_t oversize = SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH + 1; + REQUIRE(handler.writeAllOrReturn(fds[1], &oversize, sizeof(oversize)) == + (int)sizeof(oversize)); + + REQUIRE_THROWS_AS( + handler.readProto( + fds[0], true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH), + std::runtime_error); + + handler.close(fds[0]); + handler.close(fds[1]); +} diff --git a/test/unit_tests/ClientConnectionTest.cpp b/test/unit_tests/ClientConnectionTest.cpp index 424de6a5a..25d3a71ed 100644 --- a/test/unit_tests/ClientConnectionTest.cpp +++ b/test/unit_tests/ClientConnectionTest.cpp @@ -198,6 +198,77 @@ TEST_CASE("ServerClientConnection verifies passkeys", handler->close(fds[1]); } +TEST_CASE("ServerClientConnection recoverClient keeps old socket on failure", + "[ServerClientConnection]") { + auto handler = make_shared(); + int live[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, live) == 0); + + const string key = "zyxwvutsrqponmlkjihgfedcba987654"; + ServerClientConnection connection(handler, "client-recover", live[0], key); + REQUIRE(connection.getSocketFd() == live[0]); + + int attack[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, attack) == 0); + + std::thread attacker([&]() { + // Read server SequenceHeader, then claim to be far ahead. + handler->readProto( + attack[1], true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH); + SequenceHeader bad; + bad.set_sequencenumber(999999); + handler->writeProto(attack[1], bad, true); + }); + + REQUIRE_FALSE(connection.recoverClient(attack[0])); + REQUIRE(connection.getSocketFd() == live[0]); + + attacker.join(); + connection.shutdown(); + handler->close(live[0]); + handler->close(live[1]); + // attack[0] closed inside recover on failure; attack[1] may still be open. + handler->close(attack[1]); +} + +TEST_CASE("ServerClientConnection recoverClient closes old socket on success", + "[ServerClientConnection]") { + auto handler = make_shared(); + int live[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, live) == 0); + + const string key = "zyxwvutsrqponmlkjihgfedcba987654"; + ServerClientConnection connection(handler, "client-recover-ok", live[0], key); + REQUIRE(connection.getSocketFd() == live[0]); + + int reconnect[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, reconnect) == 0); + + std::thread remote([&]() { + auto seqHeader = handler->readProto( + reconnect[1], true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH); + REQUIRE(seqHeader.sequencenumber() == 0); + + SequenceHeader seqResponse; + seqResponse.set_sequencenumber(0); + handler->writeProto(reconnect[1], seqResponse, true); + + auto catchup = handler->readProto(reconnect[1], true); + REQUIRE(catchup.buffer_size() == 0); + CatchupBuffer back; + handler->writeProto(reconnect[1], back, true); + }); + + REQUIRE(connection.recoverClient(reconnect[0])); + REQUIRE(connection.getSocketFd() == reconnect[0]); + + remote.join(); + connection.shutdown(); + handler->close(live[1]); + handler->close(reconnect[0]); + handler->close(reconnect[1]); +} + TEST_CASE("Connection recover exchanges sequence and catchup", "[Connection]") { auto handler = make_shared(); int live[2]; diff --git a/test/unit_tests/SecurityNoticesTest.cpp b/test/unit_tests/SecurityNoticesTest.cpp new file mode 100644 index 000000000..668cfb380 --- /dev/null +++ b/test/unit_tests/SecurityNoticesTest.cpp @@ -0,0 +1,561 @@ +/** + * Regression tests for public security notices under security_notices/. + * + * ANT-2026-VAMER5RC reconnect passkey proof is intentionally not covered: that + * requires a PROTOCOL_VERSION bump / wire-format change. + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "BackedWriter.hpp" +#include "PipeSocketHandler.hpp" +#include "PortForwardHandler.hpp" +#include "ServerClientConnection.hpp" +#include "ServerConnection.hpp" +#include "TestHeaders.hpp" +#include "UserSocketOps.hpp" + +using namespace et; + +namespace { +// Minimal socket handler that works with socketpairs for handshake tests. +class SocketPairHandler : public SocketHandler { + public: + void queueConnectFd(int fd) { connectQueue.push(fd); } + + bool hasData(int fd) override { return waitOnSocketData(fd); } + + ssize_t read(int fd, void* buf, size_t count) override { + return ::read(fd, buf, count); + } + + ssize_t write(int fd, const void* buf, size_t count) override { + return ::write(fd, buf, count); + } + + int connect(const SocketEndpoint&) override { + if (connectQueue.empty()) { + return -1; + } + int fd = connectQueue.front(); + connectQueue.pop(); + return fd; + } + + set listen(const SocketEndpoint&) override { return {}; } + set getEndpointFds(const SocketEndpoint&) override { return {}; } + int accept(int fd) override { return fd; } + void stopListening(const SocketEndpoint&) override {} + void close(int fd) override { ::close(fd); } + vector getActiveSockets() override { return {}; } + + private: + std::queue connectQueue; +}; + +class RecordingServerConnection : public ServerConnection { + public: + RecordingServerConnection(std::shared_ptr socketHandler, + const SocketEndpoint& endpoint) + : ServerConnection(std::move(socketHandler), endpoint) {} + + bool newClient( + shared_ptr serverClientState) override { + return true; + } +}; + +class FdSocketHandler : public SocketHandler { + public: + bool hasData(int fd) override { return waitOnSocketData(fd); } + ssize_t read(int fd, void* buf, size_t count) override { + return ::read(fd, buf, count); + } + ssize_t write(int fd, const void* buf, size_t count) override { + return ::write(fd, buf, count); + } + int connect(const SocketEndpoint&) override { return -1; } + set listen(const SocketEndpoint&) override { return {}; } + set getEndpointFds(const SocketEndpoint&) override { return {}; } + int accept(int) override { return -1; } + void stopListening(const SocketEndpoint&) override {} + void close(int fd) override { ::close(fd); } + vector getActiveSockets() override { return {}; } +}; + +string makeTempDir() { + string pattern = GetTempDirectory() + "et_secnotice_XXXXXX"; + string dir = string(mkdtemp(&pattern[0])); + REQUIRE_FALSE(dir.empty()); + return dir; +} + +// CI containers (Debian/FreeBSD) run tests as root. Privilege-drop tests must +// target a non-root uid so DAC actually applies after setuid. +bool unprivilegedTestUser(uid_t* uid, gid_t* gid) { + if (getuid() != 0) { + *uid = getuid(); + *gid = getgid(); + return true; + } + const char* candidates[] = {"nobody", "nfsnobody", nullptr}; + for (int i = 0; candidates[i] != nullptr; ++i) { + struct passwd* pw = getpwnam(candidates[i]); + // Reject uid 0 and macOS's nobody sentinel ((uid_t)-2 == 4294967294). + // Allow traditional nobody (65534) used on Linux/FreeBSD. + if (pw != nullptr && pw->pw_uid != 0 && pw->pw_uid <= 65534) { + *uid = pw->pw_uid; + *gid = pw->pw_gid; + return true; + } + } + return false; +} +} // namespace + +// --------------------------------------------------------------------------- +// ANT-2026-5PETM5BV — pre-auth slowloris / oversized ConnectRequest +// --------------------------------------------------------------------------- + +TEST_CASE( + "ANT-2026-5PETM5BV handshake readProto rejects oversized length before " + "allocating", + "[SecurityNotice][ANT-2026-5PETM5BV]") { + FdSocketHandler handler; + int fds[2]; + REQUIRE(::pipe(fds) == 0); + + // Exactly the old 128 MiB cap must also be rejected for handshake reads. + int64_t oversize = 128 * 1024 * 1024; + REQUIRE(oversize > SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH); + REQUIRE(handler.writeAllOrReturn(fds[1], &oversize, sizeof(oversize)) == + (int)sizeof(oversize)); + + REQUIRE_THROWS_AS( + handler.readProto( + fds[0], true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH), + std::runtime_error); + + handler.close(fds[0]); + handler.close(fds[1]); +} + +TEST_CASE( + "ANT-2026-5PETM5BV readAll absolute timeout fires under per-byte trickle", + "[SecurityNotice][ANT-2026-5PETM5BV]") { + FdSocketHandler handler; + int fds[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == 0); + // Non-blocking so a spuriously-readable fd cannot block forever inside + // read() and skip absolute-deadline checks. + for (int fd : {fds[0], fds[1]}) { + int flags = ::fcntl(fd, F_GETFL, 0); + REQUIRE(flags >= 0); + REQUIRE(::fcntl(fd, F_SETFL, flags | O_NONBLOCK) == 0); + } + + std::atomic stop{false}; + std::thread trickle([&]() { + // Keep resetting the idle timer with 1 byte ~every 200ms; without an + // absolute deadline this would never time out. + char b = 'x'; + while (!stop.load()) { + ssize_t n = ::write(fds[1], &b, 1); + if (n < 0 && errno != EAGAIN && errno != EWOULDBLOCK) { + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + }); + + // Always stop the writer and join, even on assertion failure. + struct TrickleGuard { + std::atomic& stop; + std::thread& trickle; + int& writeFd; + int& readFd; + ~TrickleGuard() { + stop.store(true); + if (writeFd >= 0) { + ::close(writeFd); + writeFd = -1; + } + if (trickle.joinable()) { + trickle.join(); + } + if (readFd >= 0) { + ::close(readFd); + readFd = -1; + } + } + } guard{stop, trickle, fds[1], fds[0]}; + + char buf[256]; + auto start = std::chrono::steady_clock::now(); + // Idle allowance is long; absolute deadline is short. + REQUIRE_THROWS_AS(handler.readAll(fds[0], buf, sizeof(buf), /*idle*/ 30, + /*absolute*/ 2), + std::runtime_error); + auto elapsedMs = std::chrono::duration_cast( + std::chrono::steady_clock::now() - start) + .count(); + // Must not wait anywhere near the 30s idle timeout. + REQUIRE(elapsedMs < 15000); +} + +TEST_CASE( + "ANT-2026-5PETM5BV ServerConnection rejects oversized ConnectRequest " + "length", + "[SecurityNotice][ANT-2026-5PETM5BV][ServerConnection]") { + auto handler = make_shared(); + SocketEndpoint endpoint; + endpoint.set_name("server"); + RecordingServerConnection server(handler, endpoint); + + int fds[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == 0); + + int64_t oversize = SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH + 1; + REQUIRE(handler->writeAllOrReturn(fds[0], &oversize, sizeof(oversize)) == + (int)sizeof(oversize)); + + // Must return (catch runtime_error), not abort the process. + REQUIRE_NOTHROW(server.clientHandler(fds[1])); + + handler->close(fds[0]); + server.shutdown(); +} + +// --------------------------------------------------------------------------- +// ANT-2026-VAMER5RC — pre-auth recover crash / force-disconnect +// (passkey-before-recover omitted: needs PROTOCOL_VERSION bump) +// --------------------------------------------------------------------------- + +TEST_CASE( + "ANT-2026-VAMER5RC BackedWriter recover throws instead of aborting on " + "client-ahead sequence", + "[SecurityNotice][ANT-2026-VAMER5RC][BackedIO]") { + class InMemorySocketHandler : public SocketHandler { + public: + int createChannel() { + int fd = nextFd++; + buffers[fd] = {}; + return fd; + } + bool hasData(int fd) override { return !buffers[fd].empty(); } + ssize_t read(int fd, void* buf, size_t count) override { + auto& q = buffers[fd]; + if (q.empty()) { + SetErrno(EPIPE); + return 0; + } + size_t n = std::min(count, q.size()); + for (size_t i = 0; i < n; ++i) { + static_cast(buf)[i] = q.front(); + q.pop_front(); + } + return n; + } + ssize_t write(int fd, const void* buf, size_t count) override { + auto* c = static_cast(buf); + for (size_t i = 0; i < count; ++i) { + buffers[fd].push_back(c[i]); + } + return count; + } + int connect(const SocketEndpoint&) override { return -1; } + set listen(const SocketEndpoint&) override { return {}; } + set getEndpointFds(const SocketEndpoint&) override { return {}; } + int accept(int) override { return -1; } + void stopListening(const SocketEndpoint&) override {} + void close(int) override {} + vector getActiveSockets() override { return {}; } + + private: + std::atomic nextFd{1}; + std::map> buffers; + }; + + auto handler = make_shared(); + auto crypto = make_shared("12345678901234567890123456789012", + 0 /*verbosity*/); + const int fd = handler->createChannel(); + BackedWriter writer(handler, crypto, fd); + REQUIRE(writer.write(Packet(1, "one")) == BackedWriterWriteState::SUCCESS); + writer.invalidateSocket(); + + REQUIRE_THROWS_AS(writer.recover(writer.getSequenceNumber() + 1), + std::runtime_error); +} + +TEST_CASE( + "ANT-2026-VAMER5RC recoverClient leaves victim socket open on bad sequence", + "[SecurityNotice][ANT-2026-VAMER5RC][ServerClientConnection]") { + auto handler = make_shared(); + int live[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, live) == 0); + + const string key = "zyxwvutsrqponmlkjihgfedcba987654"; + ServerClientConnection connection(handler, "client-recover", live[0], key); + REQUIRE(connection.getSocketFd() == live[0]); + REQUIRE(::fcntl(live[0], F_GETFD) != -1); + + int attack[2]; + REQUIRE(::socketpair(AF_UNIX, SOCK_STREAM, 0, attack) == 0); + + std::thread attacker([&]() { + handler->readProto( + attack[1], true, SocketHandler::MAX_HANDSHAKE_PROTO_LENGTH); + SequenceHeader bad; + bad.set_sequencenumber(999999); + handler->writeProto(attack[1], bad, true); + }); + + REQUIRE_FALSE(connection.recoverClient(attack[0])); + // Victim session must remain on the original fd, which must still be open. + REQUIRE(connection.getSocketFd() == live[0]); + REQUIRE(::fcntl(live[0], F_GETFD) != -1); + + attacker.join(); + connection.shutdown(); + handler->close(live[0]); + handler->close(live[1]); + handler->close(attack[1]); +} + +// --------------------------------------------------------------------------- +// ANT-2026-AVTT7HQH — reverse-tunnel source root unlink/chown +// --------------------------------------------------------------------------- + +#ifndef WIN32 +TEST_CASE( + "ANT-2026-AVTT7HQH createSource as user does not destroy undeletable file", + "[SecurityNotice][ANT-2026-AVTT7HQH][PortForwardHandler]") { + uid_t sessionUid = 0; + gid_t sessionGid = 0; + if (!unprivilegedTestUser(&sessionUid, &sessionGid)) { + SKIP("No unprivileged user available for privilege-drop test"); + } + + string dir = makeTempDir(); + string victim = dir + "/victim_file"; + { + int fd = ::open(victim.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0644); + REQUIRE(fd >= 0); + REQUIRE(::write(fd, "keepme", 6) == 6); + ::close(fd); + } + + if (getuid() == 0) { + // Root-owned directory: after setuid to nobody, unlink must fail. This is + // the etserver-as-root threat model from the notice. + REQUIRE(::chown(dir.c_str(), 0, 0) == 0); + REQUIRE(::chmod(dir.c_str(), 0755) == 0); + REQUIRE(::chown(victim.c_str(), 0, 0) == 0); + } else { + // Non-root: remove directory write so self cannot unlink. + REQUIRE(::chmod(dir.c_str(), 0555) == 0); + } + + auto networkHandler = make_shared(); + auto pipeHandler = make_shared(); + PortForwardHandler handler(networkHandler, pipeHandler, sessionUid, + sessionGid); + + PortForwardSourceRequest request; + SocketEndpoint source; + source.set_name(victim); + *request.mutable_source() = source; + SocketEndpoint destination; + destination.set_port(9); + *request.mutable_destination() = destination; + + PortForwardSourceResponse response = + handler.createSource(request, nullptr, sessionUid, sessionGid); + + REQUIRE(response.has_error()); + + if (getuid() != 0) { + REQUIRE(::chmod(dir.c_str(), 0755) == 0); + } + struct stat st; + REQUIRE(::stat(victim.c_str(), &st) == 0); + REQUIRE(S_ISREG(st.st_mode)); + + ::unlink(victim.c_str()); + ::rmdir(dir.c_str()); +} + +TEST_CASE( + "ANT-2026-AVTT7HQH createSource as user creates socket on writable path", + "[SecurityNotice][ANT-2026-AVTT7HQH][PortForwardHandler]") { + uid_t sessionUid = 0; + gid_t sessionGid = 0; + if (!unprivilegedTestUser(&sessionUid, &sessionGid)) { + SKIP("No unprivileged user available for privilege-drop test"); + } + + string dir = makeTempDir(); + string sockPath = dir + "/ok.sock"; + // Ensure the session user can create the socket in this directory. + REQUIRE(::chmod(dir.c_str(), 0777) == 0); + + { + auto networkHandler = make_shared(); + auto pipeHandler = make_shared(); + PortForwardHandler handler(networkHandler, pipeHandler, sessionUid, + sessionGid); + + PortForwardSourceRequest request; + SocketEndpoint source; + source.set_name(sockPath); + *request.mutable_source() = source; + SocketEndpoint destination; + destination.set_port(9); + *request.mutable_destination() = destination; + + PortForwardSourceResponse response = + handler.createSource(request, nullptr, sessionUid, sessionGid); + REQUIRE_FALSE(response.has_error()); + + struct stat st; + REQUIRE(::stat(sockPath.c_str(), &st) == 0); + REQUIRE(S_ISSOCK(st.st_mode)); + } + + ::unlink(sockPath.c_str()); + ::rmdir(dir.c_str()); +} + +TEST_CASE( + "ANT-2026-AVTT7HQH UserSocketOps listen fails on root-only path without " + "deleting it", + "[SecurityNotice][ANT-2026-AVTT7HQH][UserSocketOps]") { + uid_t sessionUid = 0; + gid_t sessionGid = 0; + if (!unprivilegedTestUser(&sessionUid, &sessionGid)) { + SKIP("No unprivileged user available for privilege-drop test"); + } + if (sessionUid == 0) { + SKIP("Test requires a non-root session user"); + } + // /dev/null is a privileged node; listen/unlink as an unprivileged user must + // fail and must not remove it. + REQUIRE(::access("/dev/null", F_OK) == 0); + int fd = UserSocketOps::listenUnixAsUser("/dev/null", sessionUid, sessionGid); + REQUIRE(fd < 0); + REQUIRE(::access("/dev/null", F_OK) == 0); +} + +// --------------------------------------------------------------------------- +// ANT-2026-A3WQS3AG — forward destination root connect to arbitrary unix path +// --------------------------------------------------------------------------- + +TEST_CASE( + "ANT-2026-A3WQS3AG createDestination as session user can reach own socket", + "[SecurityNotice][ANT-2026-A3WQS3AG][PortForwardHandler]") { + uid_t sessionUid = 0; + gid_t sessionGid = 0; + if (!unprivilegedTestUser(&sessionUid, &sessionGid)) { + SKIP("No unprivileged user available for privilege-drop test"); + } + + string dir = makeTempDir(); + REQUIRE(::chmod(dir.c_str(), 0777) == 0); + string path = dir + "/dest.sock"; + + int listenFd = UserSocketOps::listenUnixAsUser(path, sessionUid, sessionGid); + REQUIRE(listenFd >= 0); + + auto networkHandler = make_shared(); + auto pipeHandler = make_shared(); + PortForwardHandler handler(networkHandler, pipeHandler, sessionUid, + sessionGid); + + PortForwardDestinationRequest request; + SocketEndpoint destination; + destination.set_name(path); + *request.mutable_destination() = destination; + request.set_fd(7); + + PortForwardDestinationResponse response = handler.createDestination(request); + REQUIRE_FALSE(response.has_error()); + REQUIRE(response.has_socketid()); + + int accepted = ::accept(listenFd, nullptr, nullptr); + REQUIRE(accepted >= 0); + ::close(accepted); + ::close(listenFd); + ::unlink(path.c_str()); + ::rmdir(dir.c_str()); +} + +TEST_CASE( + "ANT-2026-A3WQS3AG createDestination as session user cannot open " + "mode-000 socket", + "[SecurityNotice][ANT-2026-A3WQS3AG][PortForwardHandler]") { + uid_t sessionUid = 0; + gid_t sessionGid = 0; + if (!unprivilegedTestUser(&sessionUid, &sessionGid)) { + SKIP("No unprivileged user available for privilege-drop test"); + } + if (sessionUid == 0) { + SKIP("Test requires a non-root session user"); + } + + string dir = makeTempDir(); + REQUIRE(::chmod(dir.c_str(), 0777) == 0); + string path = dir + "/denied.sock"; + + // Create the listener as root (or current user), then strip access. A root + // connect would often still succeed; connecting after setuid to the session + // user must fail. Keep the listen handler alive for the whole test. + auto listenPipeHandler = make_shared(); + int listenFd = -1; + SocketEndpoint listenEp; + listenEp.set_name(path); + if (getuid() == 0) { + set fds = listenPipeHandler->listen(listenEp); + REQUIRE_FALSE(fds.empty()); + listenFd = *fds.begin(); + } else { + listenFd = UserSocketOps::listenUnixAsUser(path, sessionUid, sessionGid); + } + REQUIRE(listenFd >= 0); + REQUIRE(::chmod(path.c_str(), 0) == 0); + + auto networkHandler = make_shared(); + auto pipeHandler = make_shared(); + PortForwardHandler handler(networkHandler, pipeHandler, sessionUid, + sessionGid); + + PortForwardDestinationRequest request; + SocketEndpoint destination; + destination.set_name(path); + *request.mutable_destination() = destination; + request.set_fd(8); + + PortForwardDestinationResponse response = handler.createDestination(request); + REQUIRE(response.has_error()); + REQUIRE_FALSE(response.has_socketid()); + + if (getuid() == 0) { + listenPipeHandler->stopListening(listenEp); + } else { + ::close(listenFd); + } + ::chmod(path.c_str(), 0700); + ::unlink(path.c_str()); + ::rmdir(dir.c_str()); +} +#endif diff --git a/test/unit_tests/UnixSocketHandlerTest.cpp b/test/unit_tests/UnixSocketHandlerTest.cpp index 8fab30189..9109ecf7a 100644 --- a/test/unit_tests/UnixSocketHandlerTest.cpp +++ b/test/unit_tests/UnixSocketHandlerTest.cpp @@ -45,3 +45,67 @@ TEST_CASE("AcceptDoesNotAbortWhenNoPendingConnection", "[UnixSocketHandler]") { FATAL_FAIL(::remove(pipePath.c_str())); FATAL_FAIL(::remove(pipeDirectory.c_str())); } + +#ifndef WIN32 +TEST_CASE("PipeSocketHandler listenAsUser and connectAsUser", + "[UnixSocketHandler][PipeSocketHandler]") { + shared_ptr socketHandler(new PipeSocketHandler()); + + string tmpPath = GetTempDirectory() + string("et_test_user_XXXXXXXX"); + string pipeDirectory = string(mkdtemp(&tmpPath[0])); + string pipePath = pipeDirectory + "/pipe"; + + SocketEndpoint endpoint; + endpoint.set_name(pipePath); + + uid_t uid = getuid(); + gid_t gid = getgid(); + set serverFds = socketHandler->listenAsUser(endpoint, uid, gid); + REQUIRE(!serverFds.empty()); + + REQUIRE_THROWS_AS(socketHandler->listenAsUser(endpoint, uid, gid), + std::runtime_error); + + int clientFd = socketHandler->connectAsUser(endpoint, uid, gid); + REQUIRE(clientFd >= 0); + + int accepted = socketHandler->accept(*serverFds.begin()); + REQUIRE(accepted >= 0); + + socketHandler->close(accepted); + socketHandler->close(clientFd); + socketHandler->stopListening(endpoint); + FATAL_FAIL(::remove(pipePath.c_str())); + FATAL_FAIL(::remove(pipeDirectory.c_str())); +} + +TEST_CASE("PipeSocketHandler connectAsUser returns -1 when path missing", + "[UnixSocketHandler][PipeSocketHandler]") { + shared_ptr socketHandler(new PipeSocketHandler()); + + string tmpPath = GetTempDirectory() + string("et_test_user_XXXXXXXX"); + string pipeDirectory = string(mkdtemp(&tmpPath[0])); + string pipePath = pipeDirectory + "/missing"; + + SocketEndpoint endpoint; + endpoint.set_name(pipePath); + + int clientFd = socketHandler->connectAsUser(endpoint, getuid(), getgid()); + REQUIRE(clientFd < 0); + + FATAL_FAIL(::remove(pipeDirectory.c_str())); +} + +TEST_CASE("PipeSocketHandler listenAsUser throws when path cannot bind", + "[UnixSocketHandler][PipeSocketHandler]") { + if (getuid() == 0) { + SKIP("Test requires a non-root process"); + } + + shared_ptr socketHandler(new PipeSocketHandler()); + SocketEndpoint endpoint; + endpoint.set_name("/dev/null_et_listen_as_user_should_fail"); + REQUIRE_THROWS_AS(socketHandler->listenAsUser(endpoint, getuid(), getgid()), + std::runtime_error); +} +#endif diff --git a/test/unit_tests/UserSocketOpsTest.cpp b/test/unit_tests/UserSocketOpsTest.cpp new file mode 100644 index 000000000..3012b2b05 --- /dev/null +++ b/test/unit_tests/UserSocketOpsTest.cpp @@ -0,0 +1,173 @@ +#include "TestHeaders.hpp" +#include "UserSocketOps.hpp" + +#ifndef WIN32 +#include +#include +#include + +using namespace et; + +namespace { +string makeTempDir() { + string dirTemplate = GetTempDirectory() + "et_user_sock_XXXXXX"; + return string(mkdtemp(&dirTemplate[0])); +} + +string longUnixPath() { + // sun_path is typically 108 bytes including NUL. + return string(sizeof(sockaddr_un::sun_path) + 8, 'x'); +} +} // namespace + +TEST_CASE("UserSocketOps listen and connect as current user", + "[UserSocketOps]") { + string dir = makeTempDir(); + string path = dir + "/sock"; + + uid_t uid = getuid(); + gid_t gid = getgid(); + + int listenFd = UserSocketOps::listenUnixAsUser(path, uid, gid); + REQUIRE(listenFd >= 0); + + struct stat st; + REQUIRE(::stat(path.c_str(), &st) == 0); + REQUIRE(S_ISSOCK(st.st_mode)); + + int connFd = UserSocketOps::connectUnixAsUser(path, uid, gid); + REQUIRE(connFd >= 0); + + int client = ::accept(listenFd, nullptr, nullptr); + REQUIRE(client >= 0); + + REQUIRE(::write(connFd, "ping", 4) == 4); + char buf[4]; + REQUIRE(::read(client, buf, 4) == 4); + REQUIRE(string(buf, 4) == "ping"); + REQUIRE(::write(client, "pong", 4) == 4); + REQUIRE(::read(connFd, buf, 4) == 4); + REQUIRE(string(buf, 4) == "pong"); + + ::close(client); + ::close(connFd); + ::close(listenFd); + ::unlink(path.c_str()); + ::rmdir(dir.c_str()); +} + +TEST_CASE("UserSocketOps listenAtPath and connectAtPath in-process", + "[UserSocketOps]") { + string dir = makeTempDir(); + string path = dir + "/sock"; + + int listenFd = UserSocketOps::listenAtPath(path); + REQUIRE(listenFd >= 0); + + struct stat st; + REQUIRE(::stat(path.c_str(), &st) == 0); + REQUIRE(S_ISSOCK(st.st_mode)); + + int connFd = UserSocketOps::connectAtPath(path); + REQUIRE(connFd >= 0); + + int client = ::accept(listenFd, nullptr, nullptr); + REQUIRE(client >= 0); + REQUIRE(::write(connFd, "ok", 2) == 2); + char buf[2]; + REQUIRE(::read(client, buf, 2) == 2); + REQUIRE(string(buf, 2) == "ok"); + + ::close(client); + ::close(connFd); + ::close(listenFd); + ::unlink(path.c_str()); + ::rmdir(dir.c_str()); +} + +TEST_CASE("UserSocketOps listenAtPath rejects oversized path", + "[UserSocketOps]") { + int fd = UserSocketOps::listenAtPath(longUnixPath()); + REQUIRE(fd < 0); + REQUIRE(GetErrno() == ENAMETOOLONG); +} + +TEST_CASE("UserSocketOps connectAtPath rejects oversized path", + "[UserSocketOps]") { + int fd = UserSocketOps::connectAtPath(longUnixPath()); + REQUIRE(fd < 0); + REQUIRE(GetErrno() == ENAMETOOLONG); +} + +TEST_CASE("UserSocketOps listenUnixAsUser rejects oversized path", + "[UserSocketOps]") { + int fd = UserSocketOps::listenUnixAsUser(longUnixPath(), getuid(), getgid()); + REQUIRE(fd < 0); + REQUIRE(GetErrno() == ENAMETOOLONG); +} + +TEST_CASE("UserSocketOps connectUnixAsUser rejects oversized path", + "[UserSocketOps]") { + int fd = UserSocketOps::connectUnixAsUser(longUnixPath(), getuid(), getgid()); + REQUIRE(fd < 0); + REQUIRE(GetErrno() == ENAMETOOLONG); +} + +TEST_CASE("UserSocketOps listenAtPath fails when path is a directory", + "[UserSocketOps]") { + string dir = makeTempDir(); + // Path is itself a directory: unlink fails, bind must fail. + int fd = UserSocketOps::listenAtPath(dir); + REQUIRE(fd < 0); + + ::rmdir(dir.c_str()); +} + +TEST_CASE("UserSocketOps connectAtPath fails when nothing listens", + "[UserSocketOps]") { + string dir = makeTempDir(); + string path = dir + "/missing"; + int fd = UserSocketOps::connectAtPath(path); + REQUIRE(fd < 0); + + ::rmdir(dir.c_str()); +} + +TEST_CASE("UserSocketOps connectUnixAsUser fails when nothing listens", + "[UserSocketOps]") { + string dir = makeTempDir(); + string path = dir + "/missing"; + int fd = UserSocketOps::connectUnixAsUser(path, getuid(), getgid()); + REQUIRE(fd < 0); + + ::rmdir(dir.c_str()); +} + +TEST_CASE("UserSocketOps listen as user cannot unlink root-only path", + "[UserSocketOps]") { + if (getuid() == 0) { + SKIP("Test requires a non-root process"); + } + + // A path under /dev that a normal user cannot replace. + string path = "/dev/null_et_should_not_bind"; + int fd = UserSocketOps::listenUnixAsUser(path, getuid(), getgid()); + REQUIRE(fd < 0); +} + +TEST_CASE("UserSocketOps listenAtPath replaces an existing socket path", + "[UserSocketOps]") { + string dir = makeTempDir(); + string path = dir + "/sock"; + + int first = UserSocketOps::listenAtPath(path); + REQUIRE(first >= 0); + ::close(first); + + int second = UserSocketOps::listenAtPath(path); + REQUIRE(second >= 0); + ::close(second); + ::unlink(path.c_str()); + ::rmdir(dir.c_str()); +} +#endif