diff --git a/.github/scripts/install-nak.sh b/.github/scripts/install-nak.sh new file mode 100755 index 0000000..f04e897 --- /dev/null +++ b/.github/scripts/install-nak.sh @@ -0,0 +1,105 @@ +#!/bin/bash +# ── Install nak, checksum-verified ────────────────────────────────────────── +# nak signs the release announcement events, and the jobs that call this script +# hand it the publishing nsec on argv. An unverified download therefore runs +# with the signing key in reach, so the binary is staged, checked against a +# pinned SHA-256, and only then installed. +# +# Called from .github/workflows/package-openwrt.yml by both the .ipk (`build`) +# and .apk (`build-apk`) jobs, which is why it lives here rather than under +# packaging/openwrt-ipk/ — that directory is the .ipk payload tree. +# +# Exit 0 = installed and verified. Any non-zero exit means nothing was +# installed. +# ───────────────────────────────────────────────────────────────────────────── +set -euo pipefail + +NAK_VERSION="0.16.2" +INSTALL_PATH="/usr/local/bin/nak" + +ARCH=$(uname -m) +# Each arch carries the expected SHA-256 of its upstream release asset. +# +# Unlike the zig hashes in the same workflow, which come from ziglang.org's own +# https://ziglang.org/download/index.json, these are NOT upstream-attested: +# fiatjaf/nak publishes no checksum document, sidecar or SHA256SUMS alongside +# its release assets, so the only way to obtain a hash is to download the asset +# and compute it. These were derived that way on 2026-08-11 from +# https://github.com/fiatjaf/nak/releases/download/v0.16.2/nak-v0.16.2-linux- +# What the pin buys is therefore continuity, not authenticity: it detects the +# asset changing under a fixed tag, a corrupted or truncated transfer, and a +# substituted download, but it cannot attest that the bytes captured on that +# date were the bytes upstream intended. Bumping NAK_VERSION means re-deriving +# every hash below, and adding an arch means adding its hash here too. +case "$ARCH" in + x86_64|amd64) + NAK_ARCH="amd64" + NAK_SHA256="495243c070c4533ce96e98b6f34b7e97fd4be2da3353488b400233ed7ed0d4da" + ;; + aarch64|arm64) + NAK_ARCH="arm64" + NAK_SHA256="1fb8868c60ebf77dd86f90d6374ebf8557412baa37026d2844af932776085b88" + ;; + *) + echo "Unsupported architecture: $ARCH" + exit 1 + ;; +esac + +if [ -z "${NAK_SHA256:-}" ]; then + echo "No SHA-256 pinned for nak ${NAK_VERSION} on ${NAK_ARCH}." + echo "Add one to the case above, derived by downloading the asset:" + echo " curl -fsSL | sha256sum" + exit 1 +fi + +NAME="nak-v${NAK_VERSION}-linux-${NAK_ARCH}" +URL="https://github.com/fiatjaf/nak/releases/download/v${NAK_VERSION}/${NAME}" +# Stage outside the checkout so a failed attempt cannot leave a stray binary in +# the working tree, and so nothing lands at $INSTALL_PATH before it verifies. +NAK_TMP="$(mktemp -d)" +trap 'rm -rf "$NAK_TMP"' EXIT +TMP="${NAK_TMP}/${NAME}" + +# Download to a file and check it before installing. curl's own --retry does +# not cover a short read (exit 18), and a checksum mismatch needs a fresh +# download anyway, so the retry is an explicit bounded loop — the same failure +# mode that forced one on the zig step in this workflow. +verified="" +previous="" +for attempt in 1 2 3; do + rm -f "$TMP" + if curl -fsSL -o "$TMP" "$URL" && [ -s "$TMP" ]; then + actual="$(sha256sum < "$TMP" | cut -d' ' -f1)" + if [ "$actual" = "$NAK_SHA256" ]; then + echo "nak binary matches its pinned SHA-256 (${actual})" + verified=yes + break + fi + echo "nak binary failed its checksum on attempt ${attempt}:" + echo " expected ${NAK_SHA256}" + echo " actual ${actual}" + echo " size $(wc -c < "$TMP") bytes" + if [ "$actual" = "$previous" ]; then + echo "Two attempts fetched byte-identical content, so retrying is not" + echo "going to help: the pin is stale, upstream re-published, or the" + echo "source is serving the same bad file every time." + break + fi + previous="$actual" + else + echo "nak binary download failed on attempt ${attempt}" + fi + if [ "$attempt" -lt 3 ]; then + sleep $((attempt * 10)) + fi +done + +if [ -z "$verified" ]; then + echo "nak ${NAK_VERSION} (${NAK_ARCH}) did not download with its pinned" + echo "SHA-256 ${NAK_SHA256} in 3 attempts. Nothing installed at ${INSTALL_PATH}." + exit 1 +fi + +install -m 0755 "$TMP" "$INSTALL_PATH" +echo "Installed nak ${NAK_VERSION} (${NAK_ARCH}) at ${INSTALL_PATH}" diff --git a/.github/workflows/aur-publish-git.yml b/.github/workflows/aur-publish-git.yml index 8d9c47f..28f7ab4 100644 --- a/.github/workflows/aur-publish-git.yml +++ b/.github/workflows/aur-publish-git.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Patch PKGBUILD-git b2sums for local assets run: | @@ -42,7 +42,7 @@ jobs: awk '/^b2sums=\(/,/\)$/' packaging/aur/PKGBUILD-git - name: Publish to AUR - uses: KSXGitHub/github-actions-deploy-aur@v4.1.2 + uses: KSXGitHub/github-actions-deploy-aur@abe8ac26b51011c88be58c8809fd2ac674068ea5 # v4.1.2 with: pkgname: fips-git pkgbuild: packaging/aur/PKGBUILD-git diff --git a/.github/workflows/aur-publish.yml b/.github/workflows/aur-publish.yml index 286f0d9..50eadd1 100644 --- a/.github/workflows/aur-publish.yml +++ b/.github/workflows/aur-publish.yml @@ -41,7 +41,7 @@ jobs: set -euo pipefail pacman -Sy --noconfirm --needed base-devel namcap git curl - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Resolve package version id: ver @@ -176,7 +176,7 @@ jobs: echo "version=${TAG#v}" >> "$GITHUB_OUTPUT" echo "pkgrel=${PKGREL}" >> "$GITHUB_OUTPUT" - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: ref: ${{ steps.tag.outputs.tag }} @@ -188,7 +188,7 @@ jobs: run: bash packaging/aur/patch-pkgbuild.sh - name: Publish to AUR - uses: KSXGitHub/github-actions-deploy-aur@v4.1.2 + uses: KSXGitHub/github-actions-deploy-aur@abe8ac26b51011c88be58c8809fd2ac674068ea5 # v4.1.2 with: pkgname: fips pkgbuild: packaging/aur/PKGBUILD diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 741541a..c479b64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,7 +56,7 @@ jobs: name: CI parity runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Install Python deps run: pip3 install --quiet pyyaml - name: Check local and GitHub runners cover the same work @@ -67,6 +67,8 @@ jobs: run: python3 testing/check-trailing-log.py - name: Check nothing resolves the shared mutable test image run: bash testing/check-image-scoping.sh + - name: Check every action is pinned to a commit SHA + run: bash testing/check-action-pins.sh # Hermetic: synthetic ping functions, no containers, ~45s. Lives beside # the other two so both runners gate on it identically — putting it in # only one would create exactly the drift check-ci-parity.sh exists to @@ -79,8 +81,8 @@ jobs: name: Format check runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 - - uses: actions-rust-lang/setup-rust-toolchain@v1 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + - uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: components: rustfmt cache: false @@ -91,16 +93,16 @@ jobs: name: Clippy runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Install system dependencies run: sudo apt-get update && sudo apt-get install -y libdbus-1-dev - - uses: actions-rust-lang/setup-rust-toolchain@v1 + - uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: components: clippy cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -178,7 +180,7 @@ jobs: - os: windows-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Set SOURCE_DATE_EPOCH from git (Unix) if: runner.os != 'Windows' @@ -200,13 +202,13 @@ jobs: run: sudo nft -c -f packaging/common/fips.nft - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -235,7 +237,7 @@ jobs: # Upload the Linux binary so integration jobs can use it without rebuilding - name: Upload Linux binary if: matrix.os == 'ubuntu-latest' - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: fips-linux path: | @@ -256,7 +258,7 @@ jobs: runs-on: ubuntu-latest needs: [build] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Set SOURCE_DATE_EPOCH from git run: echo "SOURCE_DATE_EPOCH=$(git log -1 --format=%ct)" >> "$GITHUB_ENV" @@ -265,13 +267,13 @@ jobs: run: sudo apt-get update && sudo apt-get install -y libdbus-1-dev - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -288,7 +290,7 @@ jobs: run: cargo nextest run --all --profile ci - name: Publish test report (Checks tab) - uses: dorny/test-reporter@v2 + uses: dorny/test-reporter@df6247429542221bc30d46a036ee47af1102c451 # v2 if: always() with: name: Unit Tests @@ -297,7 +299,7 @@ jobs: fail-on-error: false - name: Publish test report (run summary) - uses: mikepenz/action-junit-report@v4 + uses: mikepenz/action-junit-report@db71d41eb79864e25ab0337e395c352e84523afe # v4 if: always() with: report_paths: target/nextest/ci/junit.xml @@ -318,19 +320,19 @@ jobs: runs-on: macos-latest needs: [build] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Set SOURCE_DATE_EPOCH from git run: echo "SOURCE_DATE_EPOCH=$(git log -1 --format=%ct)" >> "$GITHUB_ENV" - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -353,16 +355,16 @@ jobs: name: Unit tests (Windows) runs-on: windows-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -391,7 +393,7 @@ jobs: name: PowerShell lint (Windows packaging) runs-on: windows-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 - name: Run PSScriptAnalyzer shell: pwsh @@ -520,11 +522,11 @@ jobs: type: dns-resolver steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 # Fetch the pre-built Linux binary from job 1 - name: Download Linux binary - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: name: fips-linux path: _bin @@ -596,7 +598,7 @@ jobs: - name: Upload sim results on failure (chaos) if: matrix.type == 'chaos' && failure() - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: sim-results-${{ matrix.scenario }} path: testing/chaos/sim-results/ diff --git a/.github/workflows/package-linux.yml b/.github/workflows/package-linux.yml index d7eefa6..b0d5327 100644 --- a/.github/workflows/package-linux.yml +++ b/.github/workflows/package-linux.yml @@ -19,7 +19,7 @@ jobs: outputs: linux_package_version: ${{ steps.linux_version.outputs.linux_package_version }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -61,7 +61,7 @@ jobs: deb_arch: arm64 steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -72,14 +72,14 @@ jobs: run: sudo apt-get update && sudo apt-get install -y --no-install-recommends libdbus-1-dev llvm - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: cache: false rustflags: '' - name: Cache Cargo registry + build if: ${{ env.ACT != 'true' }} - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -140,7 +140,7 @@ jobs: - name: Upload artifact (GitHub only) if: ${{ env.ACT != 'true' }} - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: fips_${{ needs.determine-versioning.outputs.linux_package_version }}_${{ matrix.artifact_arch }}_linux path: | @@ -164,7 +164,7 @@ jobs: steps: - name: Download Linux artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: dist merge-multiple: true diff --git a/.github/workflows/package-macos.yml b/.github/workflows/package-macos.yml index e9b212e..a0ad927 100644 --- a/.github/workflows/package-macos.yml +++ b/.github/workflows/package-macos.yml @@ -19,7 +19,7 @@ jobs: outputs: macos_package_version: ${{ steps.macos_version.outputs.macos_package_version }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -61,7 +61,7 @@ jobs: target: x86_64-apple-darwin steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -69,14 +69,14 @@ jobs: run: echo "SOURCE_DATE_EPOCH=$(git log -1 --format=%ct)" >> "$GITHUB_ENV" - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: target: ${{ matrix.target }} cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -209,7 +209,7 @@ jobs: ( cd "$(dirname "$PKG")" && shasum -a 256 "$(basename "$PKG")" | tee "$(basename "$PKG").sha256" ) - name: Upload artifact - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: fips_${{ needs.determine-versioning.outputs.macos_package_version }}_${{ matrix.arch }}_macos path: | @@ -229,7 +229,7 @@ jobs: steps: - name: Download macOS artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: dist merge-multiple: true @@ -283,7 +283,7 @@ jobs: steps: - name: Download macOS artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: dist merge-multiple: true diff --git a/.github/workflows/package-openwrt.yml b/.github/workflows/package-openwrt.yml index 58b4db7..5d0037f 100644 --- a/.github/workflows/package-openwrt.yml +++ b/.github/workflows/package-openwrt.yml @@ -27,7 +27,7 @@ jobs: apk_version: ${{ steps.version.outputs.apk_version }} release_channel: ${{ steps.channel.outputs.release_channel }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -104,13 +104,13 @@ jobs: # x86 routers / VMs steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 - name: Install Rust toolchain (stable) if: matrix.rust_channel == 'stable' - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: target: ${{ matrix.rust_target }} cache: false @@ -124,7 +124,7 @@ jobs: - name: Cache Cargo registry + build if: ${{ env.ACT != 'true' }} - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -247,7 +247,7 @@ jobs: ls -lh out/ - name: Upload binaries artifact - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: fips-bins-${{ matrix.openwrt_arch }} path: out/ @@ -270,7 +270,7 @@ jobs: openwrt_arch: x86_64 steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -283,24 +283,17 @@ jobs: echo "PACKAGE_FILENAME=$PACKAGE_FILENAME" >> $GITHUB_ENV - name: Download prebuilt binaries - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: name: fips-bins-${{ matrix.openwrt_arch }} path: bins + # nak receives the signing nsec on argv further down, so the download is + # staged and checked against a pinned SHA-256 before it is installed. - name: Install nak shell: bash run: | - NAK_VERSION="0.16.2" - ARCH=$(uname -m) - case "$ARCH" in - x86_64|amd64) NAK_ARCH="amd64" ;; - aarch64|arm64) NAK_ARCH="arm64" ;; - *) echo "Unsupported architecture: $ARCH"; exit 1 ;; - esac - curl -fsSL "https://github.com/fiatjaf/nak/releases/download/v${NAK_VERSION}/nak-v${NAK_VERSION}-linux-${NAK_ARCH}" \ - -o /usr/local/bin/nak - chmod +x /usr/local/bin/nak + bash .github/scripts/install-nak.sh nak --version - name: Install jq @@ -346,6 +339,13 @@ jobs: fi shellcheck --version + # Its own step, and its own shell dialect. The shipped-scripts lint below + # runs --shell=sh with the OpenWrt rc.common exclude set, which misfires + # on a bash script; install-nak.sh is also not shipped in the package. + - name: Lint install-nak.sh + shell: bash + run: shellcheck --shell=bash .github/scripts/install-nak.sh + - name: Lint shipped shell scripts shell: bash run: | @@ -533,7 +533,7 @@ jobs: - name: Upload artifact (GitHub only) if: ${{ env.ACT != 'true' }} - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: ${{ env.PACKAGE_FILENAME }} path: dist/${{ env.PACKAGE_FILENAME }} @@ -688,7 +688,7 @@ jobs: APK_TOOLS_COMMIT: "b5a31c0d865342ad80be10d68f1bb3d3ad9b0866" steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -701,7 +701,7 @@ jobs: echo "PACKAGE_FILENAME=$PACKAGE_FILENAME" >> $GITHUB_ENV - name: Download prebuilt binaries - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: name: fips-bins-${{ matrix.openwrt_arch }} path: bins @@ -826,25 +826,18 @@ jobs: - name: Upload artifact (GitHub only) if: ${{ env.ACT != 'true' }} - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: ${{ env.PACKAGE_FILENAME }} path: dist/${{ env.PACKAGE_FILENAME }} retention-days: 30 + # nak receives the signing nsec on argv further down, so the download is + # staged and checked against a pinned SHA-256 before it is installed. - name: Install nak shell: bash run: | - NAK_VERSION="0.16.2" - ARCH=$(uname -m) - case "$ARCH" in - x86_64|amd64) NAK_ARCH="amd64" ;; - aarch64|arm64) NAK_ARCH="arm64" ;; - *) echo "Unsupported architecture: $ARCH"; exit 1 ;; - esac - curl -fsSL "https://github.com/fiatjaf/nak/releases/download/v${NAK_VERSION}/nak-v${NAK_VERSION}-linux-${NAK_ARCH}" \ - -o /usr/local/bin/nak - chmod +x /usr/local/bin/nak + bash .github/scripts/install-nak.sh nak --version - name: Install jq @@ -988,7 +981,7 @@ jobs: steps: - name: Download package artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: # Only the .ipk/.apk packages (named fips__.*), not the # fips-bins-* raw-binary artifacts shared between the build jobs. @@ -1005,7 +998,7 @@ jobs: > checksums-openwrt.txt - name: Create release - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2 with: files: | dist/*.ipk diff --git a/.github/workflows/package-windows.yml b/.github/workflows/package-windows.yml index ca13320..286d0ab 100644 --- a/.github/workflows/package-windows.yml +++ b/.github/workflows/package-windows.yml @@ -19,7 +19,7 @@ jobs: outputs: package_version: ${{ steps.version.outputs.package_version }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -52,7 +52,7 @@ jobs: needs: determine-versioning steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: fetch-depth: 0 @@ -63,13 +63,13 @@ jobs: echo "SOURCE_DATE_EPOCH=$epoch" >> $env:GITHUB_ENV - name: Install Rust toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 + uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1 with: cache: false rustflags: '' - name: Cache Cargo registry + build - uses: actions/cache@v5 + uses: actions/cache@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: | ~/.cargo/registry @@ -146,7 +146,7 @@ jobs: } - name: Upload artifact - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: fips_${{ needs.determine-versioning.outputs.package_version }}_x86_64_windows path: deploy/fips-*-windows-*.zip @@ -170,7 +170,7 @@ jobs: steps: - name: Download Windows artifacts - uses: actions/download-artifact@v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: dist merge-multiple: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fca1e0..4b82fb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -391,6 +391,73 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 0.16.4 carries an unsoundness advisory, and `nostr-relay-pool` itself is now marked unmaintained. +- The gateway DNS forwarder now validates an upstream answer before it becomes + a NAT mapping. It previously accepted whatever datagram arrived: the upstream + query reused the client's own transaction ID, the upstream socket was + wildcard-bound and never connected, the receive discarded the sender, neither + the response ID nor the question section was compared against what was asked, + and the returned address was not checked against the mesh prefix. Because the + extracted address is installed as a DNAT rule that carries no interface + constraint, a forged answer redirected traffic rather than only poisoning a + lookup. The upstream query now carries a random transaction ID, the socket is + connected so the kernel drops foreign sources, a response must match on ID, + question and type or it is discarded while the receive continues against the + original deadline, and the address goes through the validating parser with a + non-mesh answer refused before any allocation. One deliberate behaviour + change: the validation sits before the rcode check, so an upstream answering + FORMERR or REFUSED with an empty question section now yields SERVFAIL rather + than having its rcode relayed. Checking after the rcode would admit a forged + NXDOMAIN. Connecting the socket also means a dead upstream surfaces + ECONNREFUSED immediately instead of stalling for five seconds. + +- Private key writes no longer follow a symlink, and the key file's mode is + enforced rather than merely requested. The single write path opened with + create and truncate and no `O_NOFOLLOW`, so a symlink planted at the key path + was followed and its target overwritten, and it supplied the mode only + through `open(2)`, which the kernel honours on creation and ignores + otherwise, so a `fips.key` that already existed at 0644 stayed 0644 through + every rewrite. That second half needs no attacker: one `chmod`, or a restore + that did not preserve modes, leaves the key readable indefinitely. Both + writers now share an open helper carrying `O_NOFOLLOW`, and the private key + has its mode applied to the open descriptor before any secret bytes are + written. The public key keeps create-time mode instead, since forcing it + would reopen an operator-tightened `fips.pub` on every start. On Windows + neither protection applies and the file inherits the parent directory's + ACLs; that exclusion is deliberate and recorded at both writers. + +- An accepted inbound TCP connection no longer holds a slot indefinitely + without sending anything. The cap was tested at accept and the pool insert + and counter bump followed with no read in between, while the frame reader's + reads carried no deadline, so an unauthenticated remote held a slot by + connecting and staying silent. Pool keys are `ip:port`, so N sockets from one + address took N slots, and at the 256 default that locked out inbound peering + for as long as the sockets stayed open. The first frame on an inbound + connection now has a deadline, as a module constant rather than a new + configuration key, and the onion listener gets the same treatment for the + same accept-then-count ordering. Separately, the node's handshake reaper tore + down session state without closing the transport connection, so a peer that + sent msg1 and then stalled was forgotten by the node while its socket and + slot survived; the reaper now closes the connection too. **What this does not + close**: the deadline covers the first frame only, so a peer that sends one + well-formed frame and then goes silent still holds its slot. Closing that + needs a rolling idle deadline. + +- Every GitHub Action is pinned to a commit SHA, and the OpenWrt packaging + workflow verifies the helper binary it downloads. No reference in the + repository was pinned before: all sixty-six named a mutable tag and one named + a branch, including the jobs holding the AUR deploy key, the jobs with + release write scope, and the packaging jobs that run with a signing key in + the environment. Sixty-two are now full commit SHAs with the original tag + retained as a trailing comment. Four are left unpinned and justified in one + place: two actions read the tool to install from the ref name itself, so a + SHA would hand them a hex string where a toolchain name belongs. A guard + enforces the form on every sweep, treats an unreadable tree as an error + rather than a pass, and documents what it does not cover. The sharper hole + was not the tags: the OpenWrt workflow fetched a helper binary from a release + URL with no verification at all, in two jobs holding a signing key. That + download now checks a per-architecture pinned SHA-256, with the hash + provenance recorded honestly, upstream publishing no checksum document. + ## [0.4.1] - 2026-07-19 ### Changed diff --git a/src/bin/fipsctl.rs b/src/bin/fipsctl.rs index 60b4040..a64f564 100644 --- a/src/bin/fipsctl.rs +++ b/src/bin/fipsctl.rs @@ -436,7 +436,9 @@ fn main() { ); } - if key_path.exists() && !force { + // symlink_metadata rather than exists: a dangling symlink at the key + // path reports exists() == false and would slip past the guard. + if key_path.symlink_metadata().is_ok() && !force { eprintln!("error: key file already exists: {}", key_path.display()); eprintln!("Use --force to overwrite."); std::process::exit(1); @@ -452,9 +454,11 @@ fn main() { std::process::exit(1); } + // Non-fatal: the private key is already on disk by this point, so + // failing the whole run here would report failure for a keygen that + // did in fact produce the identity. if let Err(e) = write_pub_file(&pub_path, &npub) { - eprintln!("error: failed to write pub file: {e}"); - std::process::exit(1); + eprintln!("warning: failed to write pub file: {e}"); } eprintln!("{npub}"); diff --git a/src/config/mod.rs b/src/config/mod.rs index 3289247..9c3a9c7 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -239,26 +239,128 @@ pub fn read_key_file(path: &Path) -> Result { Ok(nsec) } -/// Write a bare bech32 nsec to a key file with restricted permissions. +/// Open a key or public key file for writing, without following a symlink at +/// the path. /// -/// On Unix, the file is created with mode 0600 (owner read/write only). -/// On Windows, the file inherits default ACLs from the parent directory. -pub fn write_key_file(path: &Path, nsec: &str) -> Result<(), ConfigError> { - use std::io::Write; - +/// On Unix the open carries `O_NOFOLLOW`, so a symlink pre-planted at `path` +/// fails the open instead of having its target truncated. When `enforce_mode` +/// is set, `mode` is then applied to the open descriptor (`fchmod`) before the +/// caller writes anything, so a file that already existed at a looser mode is +/// tightened before any secret bytes reach it. The permission change is made +/// through the descriptor rather than `std::fs::set_permissions`, which +/// re-resolves the name and would reopen the window the `O_NOFOLLOW` closes. +/// +/// `O_NOFOLLOW` covers only the *final* path component. An attacker who can +/// replace an intermediate directory of the key path is unaffected by it. +/// +/// On Windows neither the mode handling nor the symlink protection applies; +/// the file inherits the parent directory's ACLs. +fn open_mode_enforced( + path: &Path, + mode: u32, + enforce_mode: bool, +) -> std::io::Result { let mut opts = std::fs::OpenOptions::new(); opts.write(true).create(true).truncate(true); #[cfg(unix)] { use std::os::unix::fs::OpenOptionsExt; - opts.mode(0o600); + opts.mode(mode).custom_flags(libc::O_NOFOLLOW); } - let mut file = opts.open(path).map_err(|e| ConfigError::WriteKeyFile { + let file = opts.open(path)?; + + #[cfg(unix)] + if enforce_mode { + use std::os::unix::fs::PermissionsExt; + file.set_permissions(std::fs::Permissions::from_mode(mode))?; + } + + #[cfg(not(unix))] + let _ = (mode, enforce_mode); + + Ok(file) +} + +/// Classify a failure to open a key or public key file for writing. +/// +/// A refused open is reported as [`ConfigError::KeyPathIsSymlink`] when the +/// path is in fact a symlink. The check is on the path rather than on the +/// errno because `O_NOFOLLOW` reports a final-component symlink as `ELOOP` on +/// Linux and macOS but `EMLINK` on FreeBSD and `EFTYPE` on NetBSD, and this +/// module's `cfg(unix)` is deliberately broader than Linux. +fn classify_open_error(path: &Path, source: std::io::Error) -> ConfigError { + if path + .symlink_metadata() + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + { + return ConfigError::KeyPathIsSymlink { + path: path.to_path_buf(), + }; + } + ConfigError::WriteKeyFile { path: path.to_path_buf(), - source: e, - })?; + source, + } +} + +/// Warn about an existing identity key file the daemon will not rewrite. +/// +/// The persistent path reads an existing key and returns without writing it, +/// so a mode loosened by an operator `chmod` or by a restore that did not +/// preserve modes is otherwise never surfaced anywhere. Repairing the mode is +/// deliberately left to the operator; this only reports it. A symlinked key is +/// reported too, since the daemon does not manage the target's mode. +/// +/// Unix only: on Windows the file's ACLs are inherited from the parent +/// directory and there is no mode to inspect. +fn warn_unmanaged_key_file(path: &Path) { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let Ok(meta) = path.symlink_metadata() else { + return; + }; + + if meta.file_type().is_symlink() { + tracing::warn!( + path = %path.display(), + "Identity key file is a symlink; the daemon does not manage the mode of its target" + ); + return; + } + + if meta.is_file() && meta.permissions().mode() & 0o077 != 0 { + tracing::warn!( + path = %path.display(), + mode = format!("{:04o}", meta.permissions().mode() & 0o7777), + "Identity key file is accessible beyond its owner; expected mode 0600" + ); + } + } + + #[cfg(not(unix))] + let _ = path; +} + +/// Write a bare bech32 nsec to a key file with restricted permissions. +/// +/// On Unix, the file is opened with `O_NOFOLLOW` (a symlink at the path is +/// refused rather than followed) and forced to mode 0600 (owner read/write +/// only) before any key material is written, so an existing file at a looser +/// mode is corrected rather than inherited. +/// +/// Coverage gap: on Windows the file inherits default ACLs from the parent +/// directory, and neither the mode enforcement nor the symlink protection +/// applies. The exclusion is deliberate. +pub fn write_key_file(path: &Path, nsec: &str) -> Result<(), ConfigError> { + use std::io::Write; + + let mut file = + open_mode_enforced(path, 0o600, true).map_err(|e| classify_open_error(path, e))?; file.write_all(nsec.as_bytes()) .map_err(|e| ConfigError::WriteKeyFile { @@ -275,24 +377,20 @@ pub fn write_key_file(path: &Path, nsec: &str) -> Result<(), ConfigError> { /// Write a bare bech32 npub to a public key file. /// -/// On Unix, the file is created with mode 0644 (owner read/write, others read). -/// On Windows, the file inherits default ACLs from the parent directory. +/// On Unix, the file is opened with `O_NOFOLLOW` (a symlink at the path is +/// refused rather than followed) and created with mode 0644 (owner +/// read/write, others read). The mode is applied at creation only: this file +/// is rewritten on every persistent start, and forcing the mode would reopen +/// an operator-tightened `fips.pub` to world-readable each time. +/// +/// Coverage gap: on Windows the file inherits default ACLs from the parent +/// directory, and neither the mode handling nor the symlink protection +/// applies. The exclusion is deliberate. pub fn write_pub_file(path: &Path, npub: &str) -> Result<(), ConfigError> { use std::io::Write; - let mut opts = std::fs::OpenOptions::new(); - opts.write(true).create(true).truncate(true); - - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - opts.mode(0o644); - } - - let mut file = opts.open(path).map_err(|e| ConfigError::WriteKeyFile { - path: path.to_path_buf(), - source: e, - })?; + let mut file = + open_mode_enforced(path, 0o644, false).map_err(|e| classify_open_error(path, e))?; file.write_all(npub.as_bytes()) .map_err(|e| ConfigError::WriteKeyFile { @@ -354,7 +452,14 @@ pub fn resolve_identity( if key_path.exists() { let nsec = read_key_file(&key_path)?; let identity = Identity::from_secret_str(&nsec)?; - let _ = write_pub_file(&pub_path, &identity.npub()); + warn_unmanaged_key_file(&key_path); + if let Err(e) = write_pub_file(&pub_path, &identity.npub()) { + tracing::warn!( + path = %pub_path.display(), + error = %e, + "Failed to write the public key file" + ); + } return Ok(ResolvedIdentity { nsec, source: IdentitySource::KeyFile(key_path), @@ -378,7 +483,14 @@ pub fn resolve_identity( "Identity key found at the legacy path but not at the current default; \ using it so the node keeps its identity — move it to the current path" ); - let _ = write_pub_file(&pub_path, &identity.npub()); + warn_unmanaged_key_file(&legacy); + if let Err(e) = write_pub_file(&pub_path, &identity.npub()) { + tracing::warn!( + path = %pub_path.display(), + error = %e, + "Failed to write the public key file" + ); + } return Ok(ResolvedIdentity { nsec, source: IdentitySource::KeyFile(legacy), @@ -396,16 +508,30 @@ pub fn resolve_identity( match write_key_file(&key_path, &nsec) { Ok(()) => { - let _ = write_pub_file(&pub_path, &npub); + if let Err(e) = write_pub_file(&pub_path, &npub) { + tracing::warn!( + path = %pub_path.display(), + error = %e, + "Failed to write the public key file" + ); + } Ok(ResolvedIdentity { nsec, source: IdentitySource::Generated(key_path), }) } - Err(_) => Ok(ResolvedIdentity { - nsec, - source: IdentitySource::Ephemeral, - }), + Err(e) => { + tracing::warn!( + path = %key_path.display(), + error = %e, + "Failed to persist the generated identity key; this node is starting with an \ + ephemeral identity and its npub will change on every start" + ); + Ok(ResolvedIdentity { + nsec, + source: IdentitySource::Ephemeral, + }) + } } } else { // Ephemeral mode (default): fresh keypair every start, write key files @@ -418,8 +544,32 @@ pub fn resolve_identity( let _ = std::fs::create_dir_all(parent); } - let _ = write_key_file(&key_path, &nsec); - let _ = write_pub_file(&pub_path, &npub); + // symlink_metadata rather than exists: a dangling symlink at the key + // path reports exists() == false but is still an existing file the + // write is about to act on. + if key_path.symlink_metadata().is_ok() { + tracing::warn!( + path = %key_path.display(), + config_key = "node.identity.persistent", + "An existing key file at this path is being replaced by a fresh ephemeral \ + identity; set node.identity.persistent: true to keep the existing identity" + ); + } + + if let Err(e) = write_key_file(&key_path, &nsec) { + tracing::warn!( + path = %key_path.display(), + error = %e, + "Failed to write the ephemeral key file" + ); + } + if let Err(e) = write_pub_file(&pub_path, &npub) { + tracing::warn!( + path = %pub_path.display(), + error = %e, + "Failed to write the public key file" + ); + } Ok(ResolvedIdentity { nsec, @@ -472,6 +622,9 @@ pub enum ConfigError { source: std::io::Error, }, + #[error("refusing to write key file through a symlink: {path}")] + KeyPathIsSymlink { path: PathBuf }, + #[error("identity error: {0}")] Identity(#[from] IdentityError), @@ -1302,6 +1455,221 @@ node: assert_eq!(metadata.mode() & 0o777, 0o644); } + /// Collect formatted tracing events on the current thread. + /// + /// `resolve_identity` reports its identity-loss conditions only in the + /// log, so the log is what the tests have to assert on. Installed with + /// `tracing::subscriber::with_default`, which is thread-local, so parallel + /// tests do not see each other's events. + #[derive(Clone, Default)] + struct LogCapture(std::sync::Arc>>); + + impl LogCapture { + fn warnings(&self) -> Vec { + self.0 + .lock() + .unwrap() + .iter() + .filter(|line| line.starts_with("WARN")) + .cloned() + .collect() + } + } + + impl tracing_subscriber::Layer for LogCapture { + fn on_event( + &self, + event: &tracing::Event<'_>, + _ctx: tracing_subscriber::layer::Context<'_, S>, + ) { + struct Fields(String); + impl tracing::field::Visit for Fields { + fn record_debug( + &mut self, + field: &tracing::field::Field, + value: &dyn std::fmt::Debug, + ) { + self.0.push_str(&format!(" {}={:?}", field.name(), value)); + } + } + + let mut fields = Fields(event.metadata().level().to_string()); + event.record(&mut fields); + self.0.lock().unwrap().push(fields.0); + } + } + + fn capture_logs(f: impl FnOnce() -> T) -> (T, LogCapture) { + use tracing_subscriber::layer::SubscriberExt; + + let capture = LogCapture::default(); + let subscriber = tracing_subscriber::registry().with(capture.clone()); + let out = tracing::subscriber::with_default(subscriber, f); + (out, capture) + } + + #[cfg(unix)] + #[test] + fn test_write_key_file_refuses_symlink() { + let temp_dir = TempDir::new().unwrap(); + let victim = temp_dir.path().join("victim"); + let key_path = temp_dir.path().join("fips.key"); + + fs::write(&victim, "victim contents\n").unwrap(); + std::os::unix::fs::symlink(&victim, &key_path).unwrap(); + + let err = write_key_file(&key_path, "nsec1secret").unwrap_err(); + assert!(matches!(err, ConfigError::KeyPathIsSymlink { .. }), "{err}"); + + assert_eq!(fs::read_to_string(&victim).unwrap(), "victim contents\n"); + assert!( + key_path + .symlink_metadata() + .unwrap() + .file_type() + .is_symlink(), + "the symlink itself must be left in place, not replaced" + ); + } + + #[cfg(unix)] + #[test] + fn test_write_key_file_fixes_existing_mode() { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + + let temp_dir = TempDir::new().unwrap(); + let key_path = temp_dir.path().join("fips.key"); + + fs::write(&key_path, "nsec1old\n").unwrap(); + fs::set_permissions(&key_path, fs::Permissions::from_mode(0o644)).unwrap(); + + write_key_file(&key_path, "nsec1new").unwrap(); + + assert_eq!(fs::metadata(&key_path).unwrap().mode() & 0o777, 0o600); + assert_eq!(read_key_file(&key_path).unwrap(), "nsec1new"); + } + + #[cfg(unix)] + #[test] + fn test_write_pub_file_refuses_symlink() { + let temp_dir = TempDir::new().unwrap(); + let victim = temp_dir.path().join("victim"); + let pub_path = temp_dir.path().join("fips.pub"); + + fs::write(&victim, "victim contents\n").unwrap(); + std::os::unix::fs::symlink(&victim, &pub_path).unwrap(); + + let err = write_pub_file(&pub_path, "npub1test").unwrap_err(); + assert!(matches!(err, ConfigError::KeyPathIsSymlink { .. }), "{err}"); + assert_eq!(fs::read_to_string(&victim).unwrap(), "victim contents\n"); + } + + #[test] + fn test_ephemeral_over_existing_key_warns() { + let temp_dir = TempDir::new().unwrap(); + let config_path = temp_dir.path().join("fips.yaml"); + let key_path = temp_dir.path().join("fips.key"); + + fs::write(&config_path, "node:\n identity: {}\n").unwrap(); + let identity = crate::Identity::generate(); + let existing = crate::encode_nsec(&identity.keypair().secret_key()); + write_key_file(&key_path, &existing).unwrap(); + + let config = Config::load_file(&config_path).unwrap(); + let (resolved, logs) = + capture_logs(|| resolve_identity(&config, std::slice::from_ref(&config_path)).unwrap()); + + assert_ne!(resolved.nsec, existing); + let warnings = logs.warnings(); + assert!( + warnings + .iter() + .any(|w| w.contains(&key_path.display().to_string()) + && w.contains("node.identity.persistent")), + "expected a warning naming the key path and the config key, got {warnings:?}" + ); + } + + #[cfg(unix)] + #[test] + fn test_ephemeral_dangling_symlink_detected() { + let temp_dir = TempDir::new().unwrap(); + let config_path = temp_dir.path().join("fips.yaml"); + let key_path = temp_dir.path().join("fips.key"); + let target = temp_dir.path().join("absent-target"); + + fs::write(&config_path, "node:\n identity: {}\n").unwrap(); + std::os::unix::fs::symlink(&target, &key_path).unwrap(); + + let config = Config::load_file(&config_path).unwrap(); + let (_resolved, logs) = + capture_logs(|| resolve_identity(&config, std::slice::from_ref(&config_path)).unwrap()); + + let warnings = logs.warnings(); + assert!( + warnings + .iter() + .any(|w| w.contains(&key_path.display().to_string())), + "expected a warning naming the key path, got {warnings:?}" + ); + assert!( + !target.exists(), + "the write must not have been followed through the dangling symlink" + ); + } + + #[cfg(unix)] + #[test] + fn test_persistent_permissive_key_warns() { + use std::os::unix::fs::PermissionsExt; + + let temp_dir = TempDir::new().unwrap(); + let config_path = temp_dir.path().join("fips.yaml"); + let key_path = temp_dir.path().join("fips.key"); + + fs::write(&config_path, "node:\n identity:\n persistent: true\n").unwrap(); + let identity = crate::Identity::generate(); + let nsec = crate::encode_nsec(&identity.keypair().secret_key()); + write_key_file(&key_path, &nsec).unwrap(); + fs::set_permissions(&key_path, fs::Permissions::from_mode(0o644)).unwrap(); + + let config = Config::load_file(&config_path).unwrap(); + let (resolved, logs) = + capture_logs(|| resolve_identity(&config, std::slice::from_ref(&config_path)).unwrap()); + + // The key is still read: this warns, it does not refuse or repair. + assert_eq!(resolved.nsec, nsec); + let warnings = logs.warnings(); + assert!( + warnings + .iter() + .any(|w| w.contains(&key_path.display().to_string()) && w.contains("0644")), + "expected a warning naming the key path and its mode, got {warnings:?}" + ); + } + + /// Healthy-path regression guard only. This passes against the pre-fix + /// code vacuously, because that code warns about nothing at all, so it is + /// not evidence that the fix works: it only catches a future change that + /// starts warning on an ordinary first ephemeral start. + #[test] + fn test_ephemeral_first_run_does_not_warn() { + let temp_dir = TempDir::new().unwrap(); + let config_path = temp_dir.path().join("fips.yaml"); + + fs::write(&config_path, "node:\n identity: {}\n").unwrap(); + + let config = Config::load_file(&config_path).unwrap(); + let (_resolved, logs) = + capture_logs(|| resolve_identity(&config, std::slice::from_ref(&config_path)).unwrap()); + + assert!( + logs.warnings().is_empty(), + "first ephemeral start must be silent, got {:?}", + logs.warnings() + ); + } + #[test] fn test_key_file_empty_error() { let temp_dir = TempDir::new().unwrap(); diff --git a/src/gateway/dns.rs b/src/gateway/dns.rs index 7f840aa..cb71e36 100644 --- a/src/gateway/dns.rs +++ b/src/gateway/dns.rs @@ -9,7 +9,7 @@ use simple_dns::{CLASS, Packet, PacketFlag, RCODE, ResourceRecord, rdata}; -use simple_dns::{QTYPE, TYPE}; +use simple_dns::{QCLASS, QTYPE, TYPE}; use std::net::{Ipv6Addr, SocketAddr}; use tokio::net::UdpSocket; use tokio::sync::watch; @@ -57,16 +57,39 @@ fn extract_aaaa(packet: &Packet) -> Option { } /// Derive NodeAddr from a FIPS mesh address (fd00::/8). -/// The NodeAddr is bytes 1-15 of the IPv6 address prepended with the first byte. -fn node_addr_from_mesh(mesh_addr: Ipv6Addr) -> NodeAddr { - let bytes = mesh_addr.octets(); - // NodeAddr = first 16 bytes of SHA-256(pubkey), which maps to - // FipsAddress = fd + NodeAddr[1..16]. So NodeAddr[0] = bytes[1]. - // Actually, FipsAddress = [0xfd, nodeaddr[0..15]] - // So nodeaddr[0..15] = bytes[1..16] +/// Returns None unless the address carries the FIPS prefix. +fn node_addr_from_mesh(mesh_addr: Ipv6Addr) -> Option { + // FipsAddress = [0xfd, node_addr[0..15]], so node_addr[0..15] = bytes[1..16]. + let bytes = *crate::identity::FipsAddress::from_bytes(mesh_addr.octets()) + .ok()? + .as_bytes(); let mut node_bytes = [0u8; 16]; node_bytes[..15].copy_from_slice(&bytes[1..16]); - NodeAddr::from_bytes(node_bytes) + Some(NodeAddr::from_bytes(node_bytes)) +} + +/// Check that an upstream datagram answers the query we actually sent. +/// +/// Guards against off-path forgery: the transaction ID and question must +/// match, and the packet must be a response. Names are compared +/// case-insensitively because DNS names are case-insensitive on the wire +/// while `simple_dns` compares label bytes exactly. +fn upstream_response_matches( + response: &Packet, + upstream_id: u16, + upstream_qname: &str, + upstream_qclass: QCLASS, +) -> bool { + if !response.has_flags(PacketFlag::RESPONSE) || response.id() != upstream_id { + return false; + } + if response.questions.len() != 1 { + return false; + } + let question = &response.questions[0]; + question.qtype == QTYPE::TYPE(TYPE::AAAA) + && question.qclass == upstream_qclass + && question.qname.to_string().to_ascii_lowercase() == upstream_qname } /// Build a REFUSED DNS response. @@ -209,9 +232,16 @@ async fn handle_query( // Build an AAAA query for the daemon regardless of what the client asked // (A, AAAA, ANY, etc.). Mesh addresses are always IPv6, so the daemon // only returns useful answers for AAAA queries. + // The upstream transaction ID is drawn fresh so that an off-path forger + // cannot guess it from the client's query. Client-facing responses keep + // the client's own ID. + let upstream_id: u16 = rand::random(); + let question = query.questions.first()?; + let upstream_qname = question.qname.to_string().to_ascii_lowercase(); + let upstream_qclass = question.qclass; + let upstream_query_bytes = { - let question = query.questions.first()?; - let mut aaaa_query = Packet::new_query(query.id()); + let mut aaaa_query = Packet::new_query(upstream_id); let aaaa_question = simple_dns::Question::new( question.qname.clone(), QTYPE::TYPE(TYPE::AAAA), @@ -241,29 +271,50 @@ async fn handle_query( } }; - if let Err(e) = upstream_socket - .send_to(&upstream_query_bytes, upstream) - .await - { + // Connect the socket so the kernel drops datagrams from any source other + // than the configured upstream. + if let Err(e) = upstream_socket.connect(upstream).await { + warn!(error = %e, upstream = %upstream, "Failed to connect upstream socket"); + return build_servfail(&query); + } + + if let Err(e) = upstream_socket.send(&upstream_query_bytes).await { warn!(error = %e, "Failed to forward query to daemon"); return build_servfail(&query); } + // Keep reading until a datagram matches the query we sent, or the deadline + // passes. Datagrams that do not match are discarded rather than accepted. + let deadline = tokio::time::Instant::now() + UPSTREAM_TIMEOUT; let mut resp_buf = vec![0u8; MAX_DNS_SIZE]; - let resp_len = - match tokio::time::timeout(UPSTREAM_TIMEOUT, upstream_socket.recv(&mut resp_buf)).await { - Ok(Ok(len)) => len, - Ok(Err(e)) => { - warn!(error = %e, "Upstream recv error"); - return build_servfail(&query); + let upstream_response_bytes = loop { + let resp_len = + match tokio::time::timeout_at(deadline, upstream_socket.recv(&mut resp_buf)).await { + Ok(Ok(len)) => len, + Ok(Err(e)) => { + warn!(error = %e, upstream = %upstream, "Upstream recv error"); + return build_servfail(&query); + } + Err(_) => { + warn!(upstream = %upstream, "Upstream DNS timeout"); + return build_servfail(&query); + } + }; + + match Packet::parse(&resp_buf[..resp_len]) { + Ok(p) => { + if upstream_response_matches(&p, upstream_id, &upstream_qname, upstream_qclass) { + break resp_buf[..resp_len].to_vec(); + } + debug!(name = %fips_name, "Discarding unsolicited upstream datagram"); } Err(_) => { - warn!("Upstream DNS timeout"); - return build_servfail(&query); + debug!(name = %fips_name, "Discarding unparseable upstream datagram"); } - }; + } + }; - let upstream_response = match Packet::parse(&resp_buf[..resp_len]) { + let upstream_response = match Packet::parse(&upstream_response_bytes) { Ok(p) => p, Err(_) => return build_servfail(&query), }; @@ -292,8 +343,19 @@ async fn handle_query( } }; - // Derive NodeAddr from mesh address - let node_addr = node_addr_from_mesh(mesh_addr); + // Derive NodeAddr from mesh address. An answer outside fd00::/8 is not a + // mesh address and must never reach the NAT mapping path. + let node_addr = match node_addr_from_mesh(mesh_addr) { + Some(addr) => addr, + None => { + warn!( + name = %fips_name, + mesh_addr = %mesh_addr, + "Upstream AAAA is not a FIPS mesh address, rejecting" + ); + return build_servfail(&query); + } + }; // Allocate virtual IP from pool let mut pool_guard = pool.lock().await; @@ -347,11 +409,86 @@ async fn handle_query( mod tests { use super::*; + use simple_dns::{Name, Question}; + use tokio::sync::mpsc; + + const TEST_TTL: u32 = 60; + + /// Build a client-facing AAAA query. + fn build_query(id: u16, qname: &str) -> Vec { + let mut packet = Packet::new_query(id); + let question = Question::new( + Name::new_unchecked(qname), + QTYPE::TYPE(TYPE::AAAA), + CLASS::IN.into(), + false, + ); + packet.questions.push(question); + packet.build_bytes_vec_compressed().unwrap() + } + + /// Build an upstream NOERROR AAAA answer. + fn build_answer(id: u16, qname: &str, addr: &str) -> Vec { + let mut packet = Packet::new_reply(id); + packet.set_flags(PacketFlag::RESPONSE | PacketFlag::RECURSION_AVAILABLE); + let name = Name::new_unchecked(qname); + packet.questions.push(Question::new( + name.clone(), + QTYPE::TYPE(TYPE::AAAA), + CLASS::IN.into(), + false, + )); + let address: Ipv6Addr = addr.parse().unwrap(); + packet.answers.push(ResourceRecord::new( + name, + CLASS::IN, + TEST_TTL, + rdata::RData::AAAA(rdata::AAAA { + address: address.into(), + }), + )); + packet.build_bytes_vec_compressed().unwrap() + } + + /// A fake upstream that answers one query with a scripted list of + /// datagrams, in order, from its own socket. + fn spawn_upstream(socket: UdpSocket, replies: F) -> tokio::task::JoinHandle<()> + where + F: FnOnce(u16) -> Vec> + Send + 'static, + { + tokio::spawn(async move { + let mut buf = vec![0u8; MAX_DNS_SIZE]; + let (len, src) = socket.recv_from(&mut buf).await.unwrap(); + let observed_id = Packet::parse(&buf[..len]).unwrap().id(); + for reply in replies(observed_id) { + socket.send_to(&reply, src).await.unwrap(); + } + }) + } + + fn test_pool() -> std::sync::Arc> { + std::sync::Arc::new(tokio::sync::Mutex::new( + VirtualIpPool::new("fd01::/112", TEST_TTL as u64, 30).unwrap(), + )) + } + + /// Assert the response is an AAAA answer whose address came from the pool. + fn assert_pool_answer(response: &[u8]) -> Ipv6Addr { + let packet = Packet::parse(response).unwrap(); + assert_eq!(packet.rcode(), RCODE::NoError); + let addr = extract_aaaa(&packet).expect("expected an AAAA answer"); + assert!( + addr.octets()[0] == 0xfd && addr.octets()[1] == 0x01, + "expected a pool virtual IP, got {addr}" + ); + addr + } + #[test] fn test_node_addr_from_mesh() { // fd00::1 → node_addr bytes should be [0, 0, ..., 0, 1] in positions 0..15 let mesh: Ipv6Addr = "fd00::1".parse().unwrap(); - let node = node_addr_from_mesh(mesh); + let node = node_addr_from_mesh(mesh).unwrap(); let bytes = node.as_bytes(); // mesh = [0xfd, 0, 0, ..., 0, 1] // node = bytes[1..16] of mesh = [0, 0, ..., 0, 1] in first 15 bytes @@ -359,6 +496,186 @@ mod tests { assert_eq!(bytes[0], 0); } + #[test] + fn test_node_addr_from_mesh_rejects_non_mesh() { + let addr: Ipv6Addr = "2001:db8::1".parse().unwrap(); + assert!(node_addr_from_mesh(addr).is_none()); + } + + #[tokio::test] + async fn test_foreign_source_answer_not_accepted() { + let upstream_socket = UdpSocket::bind("[::1]:0").await.unwrap(); + let upstream = upstream_socket.local_addr().unwrap(); + let foreign = UdpSocket::bind("[::1]:0").await.unwrap(); + + // The fake upstream learns the gateway's ephemeral port from the query + // it receives, has a third socket forge an answer to that port, then + // sends the genuine answer itself. + let handle = tokio::spawn(async move { + let mut buf = vec![0u8; MAX_DNS_SIZE]; + let (len, src) = upstream_socket.recv_from(&mut buf).await.unwrap(); + let observed_id = Packet::parse(&buf[..len]).unwrap().id(); + let forged = build_answer(observed_id, "test.fips", "2001:db8::1"); + foreign.send_to(&forged, src).await.unwrap(); + let genuine = build_answer(observed_id, "test.fips", "fd00::1"); + upstream_socket.send_to(&genuine, src).await.unwrap(); + }); + + let pool = test_pool(); + let (event_tx, mut event_rx) = mpsc::channel(16); + let response = handle_query( + &build_query(0x1234, "test.fips"), + upstream, + TEST_TTL, + &pool, + &event_tx, + ) + .await + .unwrap(); + handle.await.unwrap(); + + assert_pool_answer(&response); + match event_rx.try_recv().unwrap() { + PoolEvent::MappingCreated { mesh_addr, .. } => { + assert_eq!(mesh_addr, "fd00::1".parse::().unwrap()); + } + other => panic!("unexpected event: {other:?}"), + } + assert!(matches!( + event_rx.try_recv(), + Err(mpsc::error::TryRecvError::Empty) + )); + } + + #[tokio::test] + async fn test_upstream_id_mismatch_discarded() { + let upstream_socket = UdpSocket::bind("[::1]:0").await.unwrap(); + let upstream = upstream_socket.local_addr().unwrap(); + let handle = spawn_upstream(upstream_socket, |id| { + vec![ + build_answer(id.wrapping_add(1), "test.fips", "2001:db8::1"), + build_answer(id, "test.fips", "fd00::1"), + ] + }); + + let pool = test_pool(); + let (event_tx, mut event_rx) = mpsc::channel(16); + let response = handle_query( + &build_query(0x1234, "test.fips"), + upstream, + TEST_TTL, + &pool, + &event_tx, + ) + .await + .unwrap(); + handle.await.unwrap(); + + assert_pool_answer(&response); + match event_rx.try_recv().unwrap() { + PoolEvent::MappingCreated { mesh_addr, .. } => { + assert_eq!(mesh_addr, "fd00::1".parse::().unwrap()); + } + other => panic!("unexpected event: {other:?}"), + } + } + + #[tokio::test] + async fn test_upstream_question_mismatch_discarded() { + let upstream_socket = UdpSocket::bind("[::1]:0").await.unwrap(); + let upstream = upstream_socket.local_addr().unwrap(); + let handle = spawn_upstream(upstream_socket, |id| { + vec![ + build_answer(id, "other.fips", "2001:db8::1"), + build_answer(id, "test.fips", "fd00::1"), + ] + }); + + let pool = test_pool(); + let (event_tx, mut event_rx) = mpsc::channel(16); + let response = handle_query( + &build_query(0x1234, "test.fips"), + upstream, + TEST_TTL, + &pool, + &event_tx, + ) + .await + .unwrap(); + handle.await.unwrap(); + + assert_pool_answer(&response); + match event_rx.try_recv().unwrap() { + PoolEvent::MappingCreated { mesh_addr, .. } => { + assert_eq!(mesh_addr, "fd00::1".parse::().unwrap()); + } + other => panic!("unexpected event: {other:?}"), + } + } + + #[tokio::test] + async fn test_non_mesh_aaaa_rejected() { + let upstream_socket = UdpSocket::bind("[::1]:0").await.unwrap(); + let upstream = upstream_socket.local_addr().unwrap(); + let handle = spawn_upstream(upstream_socket, |id| { + vec![build_answer(id, "test.fips", "2001:db8::1")] + }); + + let pool = test_pool(); + let (event_tx, mut event_rx) = mpsc::channel(16); + let response = handle_query( + &build_query(0x1234, "test.fips"), + upstream, + TEST_TTL, + &pool, + &event_tx, + ) + .await + .unwrap(); + handle.await.unwrap(); + + let packet = Packet::parse(&response).unwrap(); + assert_eq!(packet.rcode(), RCODE::ServerFailure); + assert!(matches!( + event_rx.try_recv(), + Err(mpsc::error::TryRecvError::Empty) + )); + } + + #[tokio::test] + async fn test_healthy_path_resolves() { + let upstream_socket = UdpSocket::bind("[::1]:0").await.unwrap(); + let upstream = upstream_socket.local_addr().unwrap(); + let handle = spawn_upstream(upstream_socket, |id| { + vec![build_answer(id, "test.fips", "fd00::1")] + }); + + let pool = test_pool(); + let (event_tx, mut event_rx) = mpsc::channel(16); + let response = handle_query( + &build_query(0x1234, "test.fips"), + upstream, + TEST_TTL, + &pool, + &event_tx, + ) + .await + .unwrap(); + handle.await.unwrap(); + + assert_pool_answer(&response); + match event_rx.try_recv().unwrap() { + PoolEvent::MappingCreated { mesh_addr, .. } => { + assert_eq!(mesh_addr, "fd00::1".parse::().unwrap()); + } + other => panic!("unexpected event: {other:?}"), + } + assert!(matches!( + event_rx.try_recv(), + Err(mpsc::error::TryRecvError::Empty) + )); + } + #[test] fn test_extract_fips_name() { // Build a simple AAAA query for test.fips diff --git a/src/node/dataplane/rx_loop.rs b/src/node/dataplane/rx_loop.rs index b538266..0a9bdd2 100644 --- a/src/node/dataplane/rx_loop.rs +++ b/src/node/dataplane/rx_loop.rs @@ -340,7 +340,7 @@ impl Node { crate::instr::tick_entry(instr_on, deadline.into_std(), std::time::Instant::now()); instr_step!(instr_on, crate::instr::Domain::Tick, crate::instr::Step::WholeTick, { instr_step!(instr_on, crate::instr::Domain::Tick, crate::instr::Step::CheckTimeouts, - self.check_timeouts()); + self.check_timeouts().await); let now_ms = Self::now_ms(); instr_step!(instr_on, crate::instr::Domain::Tick, crate::instr::Step::ReloadPeerAcl, self.reload_peer_acl().await); diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index e950d11..e9add64 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -57,7 +57,7 @@ impl Node { /// The stale/failed predicate and every registry mutation stay shell-side; /// the retry-then-teardown choreography is the pure /// [`Fmp::poll_timeouts`](crate::proto::fmp::Fmp::poll_timeouts) decision. - pub(in crate::node) fn check_timeouts(&mut self) { + pub(in crate::node) async fn check_timeouts(&mut self) { if self.connection_count() == 0 { return; } @@ -99,7 +99,7 @@ impl Node { ); } } - self.cleanup_stale_connection(link, now_ms); + self.cleanup_stale_connection(link, now_ms).await; } #[allow(unreachable_patterns)] _ => {} @@ -109,10 +109,15 @@ impl Node { /// Remove a handshake connection and all associated state. /// - /// Frees the session index, removes pending_outbound entry, and cleans up - /// the link and address mapping. Does not log — callers provide context-appropriate - /// log messages. - pub(in crate::node) fn cleanup_stale_connection(&mut self, link_id: LinkId, _now_ms: u64) { + /// Frees the session index, removes pending_outbound entry, closes the + /// underlying transport connection, and cleans up the link and address + /// mapping. Does not log — callers provide context-appropriate log + /// messages. + pub(in crate::node) async fn cleanup_stale_connection( + &mut self, + link_id: LinkId, + _now_ms: u64, + ) { // Take the connection off its machine BEFORE disposing the machine // (the machine owns it), keeping it readable for the index/link // cleanup below. The machine shares the connection's `link_id` and @@ -144,6 +149,23 @@ impl Node { let _ = self.index_allocator.free(idx); } + // Tear down the transport connection, not just the node-side state. + // A connection-oriented transport otherwise keeps the socket, its + // pool entry and its inbound-slot accounting alive after the node + // has forgotten the handshake that socket belonged to, so a peer + // that sends msg1 and then stalls holds an inbound slot forever. + // Closing twice is harmless: every `close_connection` implementation + // is `if let Some(conn) = pool.remove(addr)` and the connectionless + // default is a no-op, so the handshake paths that already close and + // then drop a link cannot be disturbed by this. + if let Some(link) = self.links.get(&link_id) { + let tid = link.transport_id(); + let addr = link.remote_addr().clone(); + if let Some(transport) = self.transports.get(&tid) { + transport.close_connection(&addr).await; + } + } + // Remove link and addr_to_link self.remove_link(&link_id); if let Some(transport_id) = transport_id { @@ -164,7 +186,7 @@ impl Node { if self.peer_timers.is_empty() { return; } - self.drive_handshake_timeouts(now_ms); + self.drive_handshake_timeouts(now_ms).await; self.drive_handshake_retransmits(now_ms).await; } @@ -183,7 +205,7 @@ impl Node { /// /// `check_timeouts` keeps reaping everything else — `is_failed()` legs and the /// idle-timeout of legs without a machine timer (inbound legs). - fn drive_handshake_timeouts(&mut self, now_ms: u64) { + async fn drive_handshake_timeouts(&mut self, now_ms: u64) { let timeout_ms = self.config().node.rate_limit.handshake_timeout_secs * 1000; let timer_links: Vec = self .peer_timers @@ -230,7 +252,7 @@ impl Node { self.note_handshake_timeout(peer, now_ms); } debug!(link_id = %link, "Handshake connection timed out"); - self.cleanup_stale_connection(link, now_ms); + self.cleanup_stale_connection(link, now_ms).await; } } } diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index ddcc1a1..ef30bcc 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -949,7 +949,7 @@ impl Node { .map(|(_, machine)| machine.link_id()) .collect(); for link_id in stale { - self.cleanup_stale_connection(link_id, now_ms); + self.cleanup_stale_connection(link_id, now_ms).await; } } } diff --git a/src/node/tests/handshake.rs b/src/node/tests/handshake.rs index 5c19256..cd3de0f 100644 --- a/src/node/tests/handshake.rs +++ b/src/node/tests/handshake.rs @@ -735,7 +735,7 @@ async fn test_stale_connection_cleanup() { // Connection was created at time 1000ms. check_timeouts uses SystemTime::now(), // which is far beyond the 30s timeout. The connection should be cleaned up. - node.check_timeouts(); + node.check_timeouts().await; // Verify everything was cleaned up assert_eq!( @@ -828,7 +828,7 @@ async fn test_failed_connection_cleanup() { assert_eq!(node.connection_count(), 1); // Failed connections should be cleaned up immediately regardless of age - node.check_timeouts(); + node.check_timeouts().await; assert_eq!( node.connection_count(), diff --git a/src/transport/framing.rs b/src/transport/framing.rs index 4fe0f24..b2edb82 100644 --- a/src/transport/framing.rs +++ b/src/transport/framing.rs @@ -178,6 +178,39 @@ pub async fn read_fmp_packet( Ok(packet) } +// ============================================================================ +// Test Frame Builders +// ============================================================================ + +/// Build a minimal established frame with the given payload_len. +/// Layout: [ver+phase:1][flags:1][payload_len:2 LE][12 bytes header][payload_len bytes][16 bytes tag] +/// +/// Lives at module scope so the transport modules that share this reader +/// (tcp, tor) can build wire-shaped frames in their own tests. +#[cfg(test)] +pub(crate) fn build_established_frame(payload_len: u16) -> Vec { + let total = PREFIX_SIZE + ESTABLISHED_REMAINING_HEADER + payload_len as usize + AEAD_TAG_SIZE; + let mut frame = vec![0u8; total]; + frame[0] = 0x00; // ver=0, phase=0 (established) + frame[1] = 0x00; // flags + frame[2..4].copy_from_slice(&payload_len.to_le_bytes()); + // Fill remaining with pattern for verification + for (i, byte) in frame[PREFIX_SIZE..total].iter_mut().enumerate() { + *byte = ((PREFIX_SIZE + i) & 0xFF) as u8; + } + frame +} + +/// Build a msg1 frame (114 bytes total). +#[cfg(test)] +pub(crate) fn build_msg1_frame() -> Vec { + let mut frame = vec![0xAA; MSG1_WIRE_SIZE]; + frame[0] = 0x01; // ver=0, phase=1 + frame[1] = 0x00; // flags + frame[2..4].copy_from_slice(&MSG1_PAYLOAD_LEN.to_le_bytes()); + frame +} + // ============================================================================ // Tests // ============================================================================ @@ -187,31 +220,6 @@ mod tests { use super::*; use std::io::Cursor; - /// Build a minimal established frame with the given payload_len. - /// Layout: [ver+phase:1][flags:1][payload_len:2 LE][12 bytes header][payload_len bytes][16 bytes tag] - fn build_established_frame(payload_len: u16) -> Vec { - let total = - PREFIX_SIZE + ESTABLISHED_REMAINING_HEADER + payload_len as usize + AEAD_TAG_SIZE; - let mut frame = vec![0u8; total]; - frame[0] = 0x00; // ver=0, phase=0 (established) - frame[1] = 0x00; // flags - frame[2..4].copy_from_slice(&payload_len.to_le_bytes()); - // Fill remaining with pattern for verification - for (i, byte) in frame[PREFIX_SIZE..total].iter_mut().enumerate() { - *byte = ((PREFIX_SIZE + i) & 0xFF) as u8; - } - frame - } - - /// Build a msg1 frame (114 bytes total). - fn build_msg1_frame() -> Vec { - let mut frame = vec![0xAA; MSG1_WIRE_SIZE]; - frame[0] = 0x01; // ver=0, phase=1 - frame[1] = 0x00; // flags - frame[2..4].copy_from_slice(&MSG1_PAYLOAD_LEN.to_le_bytes()); - frame - } - /// Build a msg2 frame (69 bytes total). fn build_msg2_frame() -> Vec { let mut frame = vec![0xBB; MSG2_WIRE_SIZE]; diff --git a/src/transport/tcp/mod.rs b/src/transport/tcp/mod.rs index 5d000fb..25b29ac 100644 --- a/src/transport/tcp/mod.rs +++ b/src/transport/tcp/mod.rs @@ -81,6 +81,9 @@ pub struct TcpTransport { /// fallback when this transport has no explicit `max_inbound_connections`. /// `None` means "not provided" — fall through to the built-in default. node_max_connections: Option, + /// Deadline from accept to the first complete inbound frame. Defaults to + /// `INBOUND_FIRST_FRAME_TIMEOUT`; overridable only from tests. + first_frame_timeout: Duration, /// Transport statistics. stats: Arc, } @@ -104,10 +107,22 @@ impl TcpTransport { accept_task: None, local_addr: None, node_max_connections: None, + first_frame_timeout: INBOUND_FIRST_FRAME_TIMEOUT, stats: Arc::new(TcpStats::new()), } } + /// Override the accept-to-first-frame deadline. + /// + /// Test-only: the accept loop is reachable from the test module only + /// through `start_async()`, which reads this field when it builds the + /// `AcceptConfig`, so there is no other way to drive the deadline at a + /// duration a unit test can wait for. + #[cfg(test)] + pub(crate) fn set_first_frame_timeout(&mut self, d: Duration) { + self.first_frame_timeout = d; + } + /// Set the node-wide `node.limits.max_connections` value. /// /// Used as the inbound-cap fallback when this transport instance has no @@ -196,6 +211,7 @@ impl TcpTransport { keepalive_secs: self.config.keepalive_secs(), recv_buf: self.config.recv_buf_size(), send_buf: self.config.send_buf_size(), + first_frame_timeout: self.first_frame_timeout, }; let accept_task = tokio::spawn(async move { @@ -414,6 +430,10 @@ impl TcpTransport { mtu, recv_stats, Direction::Outbound, + // Outbound connections hold no inbound slot and are not + // gated on an accept-loop insert. + None, + None, ) .await; }); @@ -663,6 +683,10 @@ impl TcpTransport { mss_mtu, recv_stats, Direction::Outbound, + // Outbound connections hold no inbound slot and are not + // gated on an accept-loop insert. + None, + None, ) .await; }); @@ -756,6 +780,20 @@ impl Transport for TcpTransport { // Accept Loop // ============================================================================ +/// Deadline from accept to the first complete inbound FMP frame. +/// +/// An accepted socket takes an inbound pool slot before any byte is read, +/// so without a deadline a remote that connects and stays silent holds +/// that slot for as long as it keeps the socket open. The node-layer +/// reaper cannot see such a socket: no frame means no link and no node +/// state to time out. The value matches the node-layer handshake reaper +/// (`handshake_timeout_secs`, `src/config/node.rs:101`), so a peer that +/// misses this deadline would have been reaped node-side anyway. +/// +/// Deliberately not a config key: `maint` takes no new operator-facing +/// TOML surface. +pub(crate) const INBOUND_FIRST_FRAME_TIMEOUT: Duration = Duration::from_secs(30); + /// Socket configuration parameters passed to the accept loop. struct AcceptConfig { mtu: u16, @@ -764,6 +802,7 @@ struct AcceptConfig { keepalive_secs: u64, recv_buf: usize, send_buf: usize, + first_frame_timeout: Duration, } /// TCP accept loop — runs as a spawned task when bind_addr is configured. @@ -783,6 +822,7 @@ async fn accept_loop( keepalive_secs, recv_buf, send_buf, + first_frame_timeout, } = cfg; debug!(transport_id = %transport_id, "TCP accept loop starting"); @@ -859,6 +899,12 @@ async fn accept_loop( let recv_stats = stats.clone(); let recv_addr = remote_addr.clone(); + // Readiness barrier: the receive task must not reach its + // cleanup path before the pool insert and counter bump below, + // or it would remove nothing and leave an orphaned entry with + // a permanently incremented inbound counter. + let (ready_tx, ready_rx) = tokio::sync::oneshot::channel(); + let recv_task = tokio::spawn(async move { tcp_receive_loop( read_half, @@ -869,6 +915,8 @@ async fn accept_loop( conn_mtu, recv_stats, Direction::Inbound, + Some(first_frame_timeout), + Some(ready_rx), ) .await; }); @@ -883,10 +931,15 @@ async fn accept_loop( let mut pool_guard = pool.lock().await; pool_guard.insert(remote_addr.clone(), conn); + drop(pool_guard); stats.record_connection_accepted(); stats.record_pool_inbound_added(); + // Release the receive task now that both the pool entry and + // the inbound counter are in place. + let _ = ready_tx.send(()); + debug!( transport_id = %transport_id, remote_addr = %remote_addr, @@ -917,6 +970,12 @@ async fn accept_loop( /// the cleanup path can decrement the correct `pool_inbound` / /// `pool_outbound` counter regardless of whether the matching pool /// entry survived to be removed. +/// +/// `first_frame_timeout` bounds the wait for the *first* complete frame +/// only, and is `Some` for inbound connections (which hold a capped pool +/// slot from accept) and `None` for outbound ones. `ready_rx`, when +/// present, is the accept loop's readiness barrier: the loop must not run +/// its cleanup before the accept loop has inserted the pool entry. #[allow(clippy::too_many_arguments)] async fn tcp_receive_loop( mut reader: tokio::net::tcp::OwnedReadHalf, @@ -927,6 +986,8 @@ async fn tcp_receive_loop( mtu: u16, stats: Arc, direction: Direction, + first_frame_timeout: Option, + ready_rx: Option>, ) { debug!( transport_id = %transport_id, @@ -934,39 +995,74 @@ async fn tcp_receive_loop( "TCP receive loop starting" ); - loop { - match read_fmp_packet(&mut reader, mtu).await { - Ok(data) => { - stats.record_recv(data.len()); + // An `Err` here means the accept loop went away between the insert and + // the signal. Fall through to the cleanup below rather than returning, + // so a pooled entry cannot be stranded with the counter incremented. + let admitted = match ready_rx { + Some(rx) => rx.await.is_ok(), + None => true, + }; - trace!( - transport_id = %transport_id, - remote_addr = %remote_addr, - bytes = data.len(), - "TCP packet received" - ); + if admitted { + let mut first = true; + loop { + let read = match first_frame_timeout { + // Bound the first read only. A silent remote otherwise holds + // its inbound slot for as long as it keeps the socket open. + Some(d) if first => { + match tokio::time::timeout(d, read_fmp_packet(&mut reader, mtu)).await { + Ok(result) => result, + Err(_) => { + // Not a recv error: `record_recv_error` means + // framing or I/O failure, and folding deadline + // expiries into it corrupts that counter. + debug!( + transport_id = %transport_id, + remote_addr = %remote_addr, + timeout_secs = d.as_secs_f64(), + "No complete frame within the first-frame deadline, dropping inbound connection" + ); + break; + } + } + } + _ => read_fmp_packet(&mut reader, mtu).await, + }; + first = false; - let packet = ReceivedPacket::new(transport_id, remote_addr.clone(), data); + match read { + Ok(data) => { + stats.record_recv(data.len()); - if packet_tx.send(packet).await.is_err() { + trace!( + transport_id = %transport_id, + remote_addr = %remote_addr, + bytes = data.len(), + "TCP packet received" + ); + + let packet = ReceivedPacket::new(transport_id, remote_addr.clone(), data); + + if packet_tx.send(packet).await.is_err() { + debug!( + transport_id = %transport_id, + "Packet channel closed, stopping TCP receive loop" + ); + break; + } + } + Err(e) => { + stats.record_recv_error(); + // EOF or protocol error — remove connection from pool debug!( transport_id = %transport_id, - "Packet channel closed, stopping TCP receive loop" + remote_addr = %remote_addr, + error = %e, + "TCP receive error, removing connection" ); break; } } - Err(e) => { - stats.record_recv_error(); - // EOF or protocol error — remove connection from pool - debug!( - transport_id = %transport_id, - remote_addr = %remote_addr, - error = %e, - "TCP receive error, removing connection" - ); - break; - } } } @@ -1102,9 +1198,33 @@ fn read_mss_mtu(stream: &std::net::TcpStream, default_mtu: u16) -> u16 { #[cfg(test)] mod tests { use super::*; + use crate::transport::framing::build_msg1_frame; use crate::transport::packet_channel; use tokio::time::{Duration, timeout}; + /// Poll `f` every 10ms until it holds or `limit` elapses. + async fn wait_until bool>(mut f: F, limit: Duration) -> bool { + let deadline = Instant::now() + limit; + loop { + if f() { + return true; + } + if Instant::now() >= deadline { + return false; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + fn capped_config(max_inbound: usize) -> TcpConfig { + TcpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + mtu: Some(1400), + max_inbound_connections: Some(max_inbound), + ..Default::default() + } + } + fn make_config() -> TcpConfig { TcpConfig { bind_addr: Some("127.0.0.1:0".to_string()), @@ -1707,4 +1827,324 @@ mod tests { t1.stop_async().await.unwrap(); t2.stop_async().await.unwrap(); } + + // ======================================================================== + // Inbound first-frame deadline + // ======================================================================== + + /// A socket that connects and sends nothing must have its inbound slot + /// released by the first-frame deadline. + /// + /// Break-check: with the `tokio::time::timeout` wrapper removed from the + /// first read, the socket parks on an unbounded `read_exact` and the + /// count stays at 1 for as long as the peer keeps the socket open, so + /// the second assertion fails. + #[tokio::test] + async fn idle_inbound_socket_releases_its_slot() { + let (tx, _rx) = packet_channel(100); + let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx); + transport.set_first_frame_timeout(Duration::from_millis(200)); + transport.start_async().await.unwrap(); + let listen = transport.local_addr().unwrap(); + + // Connect and say nothing. Held open for the whole test so that any + // slot release is the deadline's doing and not a client disconnect. + let squatter = TcpStream::connect(listen).await.unwrap(); + + assert!( + wait_until( + || transport.stats().pool_inbound_count() == 1, + Duration::from_secs(2) + ) + .await, + "an accepted socket should take an inbound slot" + ); + assert!( + wait_until( + || transport.stats().pool_inbound_count() == 0, + Duration::from_secs(2) + ) + .await, + "a silent inbound socket should lose its slot at the first-frame deadline" + ); + assert!( + transport.pool.lock().await.is_empty(), + "the pool entry should go with the slot" + ); + + drop(squatter); + transport.stop_async().await.unwrap(); + } + + /// With the cap filled by a silent socket, a genuine peer is refused + /// until the deadline frees the slot, and admitted afterwards. + /// + /// Break-check: without the deadline the squatter never releases, so the + /// genuine peer's frame is never delivered and the final receive times + /// out. + #[tokio::test] + async fn inbound_cap_recovers_after_first_frame_deadline() { + let (tx, mut rx) = packet_channel(100); + let mut transport = TcpTransport::new(TransportId::new(1), None, capped_config(1), tx); + transport.set_first_frame_timeout(Duration::from_millis(300)); + transport.start_async().await.unwrap(); + let listen = transport.local_addr().unwrap(); + + let squatter = TcpStream::connect(listen).await.unwrap(); + assert!( + wait_until( + || transport.stats().pool_inbound_count() == 1, + Duration::from_secs(2) + ) + .await, + "the squatter should fill the cap of one" + ); + + // While the cap is full a genuine peer is rejected outright. + let mut early = TcpStream::connect(listen).await.unwrap(); + let _ = early.write_all(&build_msg1_frame()).await; + assert!( + timeout(Duration::from_millis(200), rx.recv()) + .await + .is_err(), + "a peer arriving while the cap is full must not be admitted" + ); + drop(early); + + // The deadline frees the slot without the squatter disconnecting. + assert!( + wait_until( + || transport.stats().pool_inbound_count() == 0, + Duration::from_secs(2) + ) + .await, + "the deadline should free the slot the squatter took" + ); + + let mut genuine = TcpStream::connect(listen).await.unwrap(); + genuine.write_all(&build_msg1_frame()).await.unwrap(); + let packet = timeout(Duration::from_secs(2), rx.recv()) + .await + .expect("timeout waiting for the genuine peer's frame") + .expect("packet channel closed"); + assert_eq!(packet.data, build_msg1_frame()); + + drop(squatter); + drop(genuine); + transport.stop_async().await.unwrap(); + } + + /// Regression guard, not evidence that the fix works. + /// + /// The deadline is scoped to the first iteration, so an established + /// connection that then goes quiet cannot be dropped by it: this test + /// passes by construction under the current design. It is kept so that a + /// future general (every-read) idle deadline cannot silently start + /// reaping quiet links without a test going red. + #[tokio::test] + async fn established_connection_survives_long_idle() { + let (tx, mut rx) = packet_channel(100); + let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx); + transport.set_first_frame_timeout(Duration::from_millis(200)); + transport.start_async().await.unwrap(); + let listen = transport.local_addr().unwrap(); + + let mut peer = TcpStream::connect(listen).await.unwrap(); + peer.write_all(&build_msg1_frame()).await.unwrap(); + let packet = timeout(Duration::from_secs(2), rx.recv()) + .await + .expect("timeout") + .expect("packet channel closed"); + assert_eq!(packet.data, build_msg1_frame()); + + // Four deadlines' worth of silence after the first frame. + tokio::time::sleep(Duration::from_millis(800)).await; + + assert_eq!( + transport.stats().pool_inbound_count(), + 1, + "an established connection must not be dropped by the first-frame deadline" + ); + assert!(!transport.pool.lock().await.is_empty()); + + drop(peer); + transport.stop_async().await.unwrap(); + } + + /// A genuine peer that is slow to start, but finishes its first frame + /// inside the deadline, is admitted. + #[tokio::test] + async fn slow_first_frame_within_deadline_is_admitted() { + let (tx, mut rx) = packet_channel(100); + let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx); + transport.set_first_frame_timeout(Duration::from_secs(1)); + transport.start_async().await.unwrap(); + let listen = transport.local_addr().unwrap(); + + let mut peer = TcpStream::connect(listen).await.unwrap(); + tokio::time::sleep(Duration::from_millis(300)).await; + peer.write_all(&build_msg1_frame()).await.unwrap(); + + let packet = timeout(Duration::from_secs(2), rx.recv()) + .await + .expect("timeout") + .expect("packet channel closed"); + assert_eq!(packet.data, build_msg1_frame()); + assert_eq!(transport.stats().pool_inbound_count(), 1); + + drop(peer); + transport.stop_async().await.unwrap(); + } + + /// The honest-slow-peer case the wrapper actually kills: a first frame + /// that *starts* inside the deadline but completes after it. The + /// deadline covers the whole frame, not its first byte, so the drip is + /// dropped and its slot released. + #[tokio::test] + async fn byte_dripped_first_frame_past_deadline_is_dropped() { + let (tx, mut rx) = packet_channel(100); + let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx); + transport.set_first_frame_timeout(Duration::from_millis(300)); + transport.start_async().await.unwrap(); + let listen = transport.local_addr().unwrap(); + + let frame = build_msg1_frame(); + let mut peer = TcpStream::connect(listen).await.unwrap(); + // Prefix inside the deadline, remainder well past it. + peer.write_all(&frame[..4]).await.unwrap(); + tokio::time::sleep(Duration::from_millis(600)).await; + let _ = peer.write_all(&frame[4..]).await; + + assert!( + timeout(Duration::from_millis(500), rx.recv()) + .await + .is_err(), + "a first frame completing after the deadline must not be delivered" + ); + assert!( + wait_until( + || transport.stats().pool_inbound_count() == 0, + Duration::from_secs(2) + ) + .await, + "the dripped connection should have released its slot" + ); + + drop(peer); + transport.stop_async().await.unwrap(); + } + + /// Break-check for the readiness barrier's error path. + /// + /// Stands in for an accept loop aborted between the pool insert and the + /// `ready_tx.send()`: the sender is dropped, so `ready_rx.await` returns + /// `Err`. The receive loop must still fall through to its cleanup, or + /// the pooled entry and its inbound-counter increment are stranded with + /// no task left to undo them. A bare `return` on the error path fails + /// both assertions below. + #[tokio::test] + async fn receive_loop_cleans_up_when_readiness_signal_is_dropped() { + let (tx, _rx) = packet_channel(10); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let listen = listener.local_addr().unwrap(); + let client = TcpStream::connect(listen).await.unwrap(); + let (server, peer_addr) = listener.accept().await.unwrap(); + let remote = TransportAddr::from_string(&peer_addr.to_string()); + let (read_half, write_half) = server.into_split(); + + let pool: ConnectionPool = Arc::new(Mutex::new(HashMap::new())); + let stats = Arc::new(TcpStats::new()); + pool.lock().await.insert( + remote.clone(), + TcpConnection { + writer: Arc::new(Mutex::new(write_half)), + recv_task: tokio::spawn(async {}), + mtu: 1400, + established_at: Instant::now(), + direction: Direction::Inbound, + }, + ); + stats.record_pool_inbound_added(); + assert_eq!(stats.pool_inbound_count(), 1); + + let (ready_tx, ready_rx) = tokio::sync::oneshot::channel::<()>(); + drop(ready_tx); + + tcp_receive_loop( + read_half, + TransportId::new(1), + remote.clone(), + tx, + pool.clone(), + 1400, + stats.clone(), + Direction::Inbound, + Some(Duration::from_millis(50)), + Some(ready_rx), + ) + .await; + + assert!( + pool.lock().await.is_empty(), + "an aborted accept must not strand a pool entry" + ); + assert_eq!( + stats.pool_inbound_count(), + 0, + "an aborted accept must not strand an inbound-counter increment" + ); + drop(client); + } + + /// Invariant guard: a deadline that expires immediately still leaves no + /// orphaned pool entry or counter increment behind. + /// + /// This is not a break-check for the readiness barrier. On the + /// current-thread test runtime the accept loop queues for the pool lock + /// before the spawned receive task can run at all, so the insert wins + /// the race with or without the barrier. The barrier's error path is + /// break-checked in `receive_loop_cleans_up_when_readiness_signal_is_dropped`. + #[tokio::test] + async fn zero_deadline_leaves_no_orphaned_pool_entry() { + let (tx, _rx) = packet_channel(100); + let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx); + transport.set_first_frame_timeout(Duration::ZERO); + transport.start_async().await.unwrap(); + let listen = transport.local_addr().unwrap(); + + // Hold the pool across the accept so the receive task cannot reach + // its cleanup while the accept loop is mid-insert. + let guard = transport.pool.lock().await; + let client = TcpStream::connect(listen).await.unwrap(); + tokio::time::sleep(Duration::from_millis(100)).await; + drop(guard); + + // Sequence the checks off `connections_accepted`, which the accept + // loop bumps only after its insert. Reading the pool counter first + // would otherwise observe the pre-accept zero and prove nothing. + assert!( + wait_until( + || transport.stats().snapshot().connections_accepted == 1, + Duration::from_secs(2) + ) + .await, + "the accept loop should have admitted the connection" + ); + assert!( + wait_until( + || transport.stats().pool_inbound_count() == 0 + && transport + .pool + .try_lock() + .map(|p| p.is_empty()) + .unwrap_or(false), + Duration::from_secs(2) + ) + .await, + "an immediately expired deadline should leave neither a pool entry nor a counter increment" + ); + + drop(client); + transport.stop_async().await.unwrap(); + } } diff --git a/testing/check-action-pins.sh b/testing/check-action-pins.sh new file mode 100755 index 0000000..63a9e2c --- /dev/null +++ b/testing/check-action-pins.sh @@ -0,0 +1,145 @@ +#!/bin/bash +# ── GitHub Action pinning guard ───────────────────────────────────────────── +# Every third-party action this repository invokes must be referenced by a +# 40-character commit SHA, with its human-readable tag in a trailing comment. +# +# A tag is a mutable pointer. Whoever controls an action's repository can move +# `v6` to different code at any time, and several of the jobs here are worth +# moving it for: aur-publish.yml and aur-publish-git.yml hand an action +# AUR_SSH_PRIVATE_KEY, and the OpenWrt release jobs run with HIVE_CI_NSEC in +# the environment. A SHA is content-addressed and cannot be repointed. The +# trailing comment is required rather than optional so the pin stays legible: +# a bare 40-hex string tells a reader nothing about which release it is, and a +# pin nobody can read is a pin nobody updates. +# +# What counts as a violation: any `uses:` reference that is not +# * `owner/repo@<40 hex> # ` — the required form, comment mandatory; or +# * a local action, `./path` or `docker://...`; or +# * one of the individually justified references listed below. +# +# WHAT THIS GUARD DOES NOT COVER, so a green run is not read as "the workflows +# fetch nothing unverified": +# * the actions that the pinned actions themselves invoke. Pinning +# KSXGitHub/github-actions-deploy-aur removes the retag vector; it does not +# constrain what that action does with the SSH key it is given by design +# (aur-publish.yml, aur-publish-git.yml). +# * `pip3 install --quiet pyyaml` in ci.yml's ci-parity job, which holds +# `checks: write`. Unpinned entirely, version and hash both. +# * `cargo install cargo-zigbuild --version 0.19.8 --locked` in +# package-openwrt.yml. Version-pinned, not hash-pinned. +# * anything a workflow downloads at run time. The zig tarball and the nak +# binary are SHA-256 checked in their own steps; nothing here enforces that. +# +# Exit 0 = clean. Exit 1 = an unpinned reference. Exit 2 = the guard could not +# run; never treated as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$SCRIPT_DIR/.." + +# The one accepted form for a third-party action. The comment is mandatory +# rather than optional: an optional comment would let the checker accept a pin +# it cannot describe, and a pin nobody can read is a pin nobody updates. +PINNED_RE='^[^@]+@[0-9a-f]{40} +#.*$' + +# Individually justified unpinned references. Each entry is the exact ref text. +# +# Both of these actions read the tool they install from the ref name itself +# (`github.action_ref`), so replacing the ref with a SHA hands them a 40-hex +# string where a toolchain or tool name belongs and the step fails outright. +# They are not pinnable without also moving the selection into `with:`, which +# changes which toolchain resolves, and that is a separate decision from +# pinning. Note what stays exposed: both remain repointable by their upstream +# owners. +ALLOWED_REFS=( + 'dtolnay/rust-toolchain@nightly' + 'taiki-e/install-action@nextest' +) + +if ! command -v git >/dev/null 2>&1; then + echo "check-action-pins: git not available, cannot sweep" >&2 + exit 2 +fi +if [[ ! -d "$REPO_ROOT/.github" ]]; then + echo "check-action-pins: $REPO_ROOT/.github missing, refusing to pass" >&2 + exit 2 +fi + +# Tracked files only. Workflows plus any composite/local action definition: +# a future .yaml extension and a future .github/actions/ tree both have to be +# swept, or the guard silently narrows as the repository grows. +if ! tracked="$(git -C "$REPO_ROOT" ls-files -- '.github/workflows/*.yml' '.github/workflows/*.yaml' '.github/actions/*.yml' '.github/actions/*.yaml')"; then + echo "check-action-pins: git ls-files failed, refusing to pass" >&2 + exit 2 +fi +if [[ -z "$tracked" ]]; then + echo "check-action-pins: no tracked workflow or action files, refusing to pass" >&2 + exit 2 +fi +mapfile -t files < <(printf '%s\n' "$tracked") +if [[ ${#files[@]} -eq 0 ]]; then + echo "check-action-pins: empty file list, refusing to pass" >&2 + exit 2 +fi + +# True when this ref is one of the justified references above. +allowed_ref() { + local ref="$1" entry + for entry in "${ALLOWED_REFS[@]}"; do + [[ "$ref" == "$entry" ]] && return 0 + done + return 1 +} + +violations=0 +checked=0 + +for f in "${files[@]}"; do + [[ -f "$REPO_ROOT/$f" ]] || continue + + while IFS= read -r hit; do + n="${hit%%:*}" + text="${hit#*:}" + # A commented-out step is describing a reference, not resolving it. + [[ "$text" =~ ^[[:space:]]*# ]] && continue + + # Everything after `uses:`, with surrounding whitespace and any quoting + # removed. The trailing comment is part of the ref text on purpose: + # the accepted form requires it. + ref="${text#*uses:}" + ref="${ref#"${ref%%[![:space:]]*}"}" + ref="${ref%"${ref##*[![:space:]]}"}" + + checked=$((checked + 1)) + + # A local action or a container image is not a mutable upstream tag. + [[ "$ref" == ./* ]] && continue + [[ "$ref" == docker://* ]] && continue + [[ "$ref" =~ $PINNED_RE ]] && continue + allowed_ref "$ref" && continue + + echo "$f:$n: $ref" + violations=$((violations + 1)) + done < <(grep -nE '^[[:space:]]*(- )?uses:' "$REPO_ROOT/$f" 2>/dev/null) +done + +if [[ $checked -eq 0 ]]; then + echo "check-action-pins: no uses: references found at all, refusing to pass" >&2 + exit 2 +fi + +if [[ $violations -gt 0 ]]; then + echo "" + echo "check-action-pins: $violations action reference(s) are not pinned to a commit SHA." + echo "Required form: uses: owner/repo@<40-hex-commit-sha> # " + echo "Resolve one with:" + echo " git ls-remote https://github.com/owner/repo 'refs/tags/^{}' refs/tags/" + echo "and use the peeled (^{}) SHA when the tag is annotated." + echo "A tag is a mutable pointer its owner can repoint; several of these jobs" + echo "hold a signing key or an SSH deploy key while the action runs." + exit 1 +fi + +echo "check-action-pins: all $checked action reference(s) pinned or justified" +exit 0 diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 10a1d4c..7a4c1e3 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -1257,6 +1257,16 @@ run_image_scoping() { record "image-scoping" $rc } +# Every third-party action must be referenced by commit SHA. A tag is a mutable +# pointer, and the jobs holding the AUR deploy key and the release signing key +# are exactly the ones worth repointing it for. Static, and it costs nothing. +run_action_pins() { + local rc=0 + info "[action-pins] Checking that every action is pinned to a commit SHA" + "$SCRIPT_DIR/check-action-pins.sh" || rc=$? + record "action-pins" $rc +} + # Every daemon log string a test matches on must still be emitted by src/. # A stale one does not fail — it stops observing, and an expect-zero assertion # built on it then passes for the wrong reason. @@ -1311,6 +1321,7 @@ main() { run_log_strings run_trailing_log run_image_scoping + run_action_pins run_wait_converge if [[ "$TEST_ONLY" == true ]]; then