From c12b18cf7a5af2929aedb4b1c01a6ed0113f42e0 Mon Sep 17 00:00:00 2001 From: Ralf Becher Date: Mon, 31 Aug 2026 18:26:21 +0200 Subject: [PATCH] Patch dependency CVEs, add CI, and attribute the upstream fork Closes the four open Dependabot advisories, and adds the CI and Dependabot configuration the repository had none of. Package bumps: - log4net 2.0.15 -> 3.3.0 in all three projects that reference it (CVE-2026-40021). Dependabot's own PR covered two of the three and left TDSProtocolTests behind; this covers all of them. - Microsoft.Data.SqlClient 5.1.2 -> 5.1.3 (CVE-2024-0056). Neither advisory is exploitable here, and that is worth recording so the urgency is not misremembered later. The log4net issue is in XmlLayout and XmlLayoutSchemaLog4J; src/TDSProxy/log4net.config configures PatternLayout exclusively. SqlClient is referenced only by TestConnection, a manual smoke test, and the Dockerfile publishes TDSProxy.csproj alone -- so it never ships. The log4net major bump is a real API risk rather than a version change, so it was verified by building and running the tests on net6.0, not assumed: the surface used is LogManager.GetLogger, ILog and XmlConfigurator.ConfigureAndWatch, all unchanged in 3.x. TestConnection carried a hardcoded connection string inherited from upstream, naming an internal host, a domain user and a password, in a public repository. It now reads TDSPROXY_TEST_CONNECTION_STRING from the environment and explains what to point it at. CI runs the pin check first and every other job needs it: running it alongside them would be too late, since a parallel job has already executed its own uses: before the check could fail. Actions are pinned to commit SHAs with the release named in a comment, and scripts/check-action-pins.sh verifies the two agree so a hash swapped for one taken from a fork stops looking like a routine bump. The README now says this is a fork of tech-software/TDSProxy and what this fork changed. LICENSE already retained the original copyright, which is what MIT requires; the repository simply read as original work. The licensing section also records why there is no third-party notice file -- attribution clauses attach on distribution, and nothing here publishes an image -- and what would have to change if one were ever pushed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01X9JVxmeMMFL7bRGq4vbbgf --- .github/dependabot.yml | 46 ++++++ .github/workflows/ci.yml | 47 ++++++ README.md | 34 +++- scripts/check-action-pins.sh | 158 +++++++++++++++++++ src/TDSProtocol/TDSProtocol.csproj | 2 +- src/TDSProtocolTests/TDSProtocolTests.csproj | 2 +- src/TDSProxy/TDSProxy.csproj | 2 +- src/TestConnection/Program.cs | 88 +++++++---- src/TestConnection/TestConnection.csproj | 2 +- 9 files changed, 346 insertions(+), 35 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/ci.yml create mode 100755 scripts/check-action-pins.sh diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..38046d2 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,46 @@ +version: 2 +updates: + # NuGet packages across every project in the solution. Grouped so a routine + # week arrives as one PR rather than one per package — but version updates and + # security updates are separate groups, because a group covers version updates + # only unless it says otherwise, and security advisories would otherwise each + # open a PR of their own. + - package-ecosystem: nuget + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 5 + groups: + nuget-dependencies: + applies-to: version-updates + patterns: + - "*" + nuget-security: + applies-to: security-updates + patterns: + - "*" + + # The Actions used by the workflows. These are SHA-pinned, and + # scripts/check-action-pins.sh verifies each SHA still matches the release its + # comment names — so a bump here is checked rather than trusted. + - package-ecosystem: github-actions + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 5 + groups: + github-actions: + applies-to: version-updates + patterns: + - "*" + github-actions-security: + applies-to: security-updates + patterns: + - "*" + + # The runtime and SDK base images the Dockerfile pulls. + - package-ecosystem: docker + directory: "/" + schedule: + interval: weekly + open-pull-requests-limit: 5 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..0ca0cb8 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,47 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + # Runs before everything else, and every other job needs it. Running it + # alongside them would be too late: a parallel job has already executed its own + # `uses:` before this could fail, so for a forged pin the attacker's code runs + # anyway and the check only blocks the merge, not the run. + pins: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Verify every Action is pinned to its named release + run: ./scripts/check-action-pins.sh + + build: + needs: pins + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Install .NET SDK + uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 + with: + dotnet-version: "6.0.x" + + - name: Restore + run: dotnet restore src/TDSProxy.sln + + - name: Build + run: dotnet build src/TDSProxy.sln -c Release --no-restore + + - name: Test + run: dotnet test src/TDSProxy.sln -c Release --no-build --verbosity normal diff --git a/README.md b/README.md index 26ec8eb..5ae0417 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,17 @@ TDSProxy is a proxy server for the MS SQL Server TDS (Tabular Data Stream) Protocol. It sits between SQL clients and SQL Server, handling TLS termination and authentication with support for pluggable authenticators. +> **This is a fork of [tech-software/TDSProxy](https://github.com/tech-software/TDSProxy)**, the +> original work of Tech Software, and is distributed under its MIT licence — see +> [LICENSE](LICENSE), which retains the original copyright. +> +> This fork makes it cross-platform and container-friendly: retargeted to .NET 6, +> a Docker image with an OpenSSL configuration that still permits TLS 1.0 to +> legacy backends, JSON/`appsettings.json` configuration in place of the .NET +> Framework `app.config` model, and fixes to the SSL handshake adapter. The +> upstream history is not present in this repository, so treat it as a hard fork +> rather than a branch that can be merged back. + ## Features - TDS protocol proxy for SQL Server connections @@ -202,4 +213,25 @@ Authenticators implement `IAuthenticator` and are loaded via MEF. Place authenti ## License -See LICENSE file for details. +MIT — see [LICENSE](LICENSE), which retains the copyright of +[Tech Software](https://github.com/tech-software/TDSProxy), whose project this is +forked from. That notice must travel with any copy you pass on. + +### Third-party dependencies + +The proxy builds against log4net (Apache-2.0) and a set of +`Microsoft.Extensions.*` and `System.*` packages (MIT). All are permissive and +none is copyleft, so nothing among them constrains how this is used or modified. + +There is deliberately no `THIRD-PARTY-NOTICES.md` here. The attribution clauses +in those licences attach when you *distribute* the software — handing someone a +binary copy — and nothing in this repository does: no image is published to any +registry, there are no releases, and the Dockerfile is built locally by whoever +runs it. NuGet fetches every package above onto the build machine directly. + +That changes the moment an image is pushed anywhere. Publishing one redistributes +those compiled assemblies, which makes their notices mandatory rather than +courteous — Apache-2.0 §4(d) additionally requires carrying forward log4net's own +NOTICE file. If that day comes, generate the notice from the restored package +graph rather than writing it by hand, and copy it into the image alongside +`LICENSE`. diff --git a/scripts/check-action-pins.sh b/scripts/check-action-pins.sh new file mode 100755 index 0000000..2912a24 --- /dev/null +++ b/scripts/check-action-pins.sh @@ -0,0 +1,158 @@ +#!/usr/bin/env bash +# +# Verify that every third-party Action used by the workflows is pinned to an +# immutable commit SHA, comes from an owner we trust, and carries a version +# comment that really does name the release that SHA was cut from. +# +# ./scripts/check-action-pins.sh # every check +# ./scripts/check-action-pins.sh --offline # skip the upstream tag lookups +# +# A git tag is a movable label, so `uses: actions/checkout@v7` runs whichever +# commit that label points at when the job starts, and the owner of the action +# can repoint it at any time. Pinning to a SHA fixes the code, but it also +# makes the reference unreadable to a human, and that is the weakness this +# guards. A pull request can swap the hash for one taken from an attacker's +# fork while leaving the reassuring `# v7.0.1` comment untouched, and the diff +# then looks exactly like a routine Dependabot bump. Forks share object storage +# with their parent, so such a commit is even reachable under the real +# repository's URL. Only resolving the tag tells the two apart. +# +# Version comments must name exact patch releases, never major tags. A major +# tag such as v7 moves with every release, so checking against it would turn CI +# red the moment upstream ships v7.0.2, for a pin that is still perfectly good. +# Patch tags never move, so this check stays quiet until something is wrong. + +set -euo pipefail + +cd "$(dirname "$0")/.." + +# Only actions published by these owners may appear in a workflow. This is the +# check that carries the most weight: a SHA matching its own tag says nothing +# about whether the action belongs here at all, because `evil/action@ +# # v1` verifies perfectly well against evil's own v1 tag. Adding an owner +# here has to be a deliberate edit, reviewed as such. +ALLOWED_OWNERS="actions astral-sh docker peter-evans pypa" + +offline=false +case "${1:-}" in + --offline) offline=true ;; + "") ;; + *) echo "usage: $0 [--offline]" >&2; exit 2 ;; +esac + +# Resolve a tag to the commit it names, following annotated tags through to +# their target. git ls-remote needs no token and is not rate limited, unlike +# the REST API, which matters because hosted runners share outbound addresses. +# +# Three outcomes have to stay distinguishable. A tag that resolves prints its +# SHA. A tag that does not exist prints nothing and returns 0, because +# ls-remote reports a missing ref as success. A lookup that could not be made +# at all, from a network failure or a repository that is not there, returns +# non-zero and writes the reason to stderr. Collapsing the last two would let +# an unreachable network read as a verified pin, and swallowing the error under +# set -e would kill the run with no file or line to point at. +resolve_tag() { + out=$(git ls-remote "https://github.com/$1" "refs/tags/$2" "refs/tags/$2^{}" 2>&1) || { + status=$? + printf 'git ls-remote exit %s: %s\n' "$status" "$(printf '%s' "$out" | tr '\n' ' ')" >&2 + return "$status" + } + printf '%s\n' "$out" | + awk '{ sha[$2] = $1 } + END { print (("refs/tags/'"$2"'^{}") in sha) \ + ? sha["refs/tags/'"$2"'^{}"] \ + : sha["refs/tags/'"$2"'"] }' +} + +failures=0 +checked=0 + +fail() { + echo "::error file=$1,line=$2::$3" + echo " $1:$2: $3" >&2 + failures=$((failures + 1)) +} + +# Every `uses:` line in every workflow, carrying its file and line number so a +# failure points straight at the offending reference. +while IFS=: read -r file line body; do + # `uses: owner/repo@ref # comment`, with the step's leading "- " optional. + spec=$(printf '%s\n' "$body" | sed -E 's/^[[:space:]]*(-[[:space:]]+)?uses:[[:space:]]*//; s/[[:space:]]*#.*$//; s/[[:space:]]*$//') + comment=$(printf '%s\n' "$body" | sed -nE 's/.*#[[:space:]]*(v[0-9][^[:space:]]*).*/\1/p') + + # Actions living in this repository are covered by our own review. + case "$spec" in + ./*|.\\*) continue ;; + esac + + checked=$((checked + 1)) + + # A container action is third-party executable code exactly as a repository + # action is, and an image tag such as :latest is every bit as movable as a + # git tag, so one cannot simply be waved through for having no git ref to + # resolve. A digest is what pins an image, and anything else is rejected. + # Note the residual gap: a digest fixes which image runs without saying + # whether it ought to run here, because there is no registry equivalent of + # ALLOWED_OWNERS. Adding a container action stays a deliberate act. + case "$spec" in + docker://*) + if ! printf '%s\n' "$spec" | grep -qE '^docker://[^@[:space:]]+@sha256:[0-9a-f]{64}$'; then + fail "$file" "$line" "$spec is a container action that is not pinned by digest; write it as docker://image@sha256:<64 hex digits>" + fi + continue + ;; + esac + + repo=${spec%%@*} + ref=${spec#*@} + owner=${repo%%/*} + + # A subdirectory action such as owner/repo/path@ref still pins the whole + # repository, so trim the path before resolving anything. + repo=$(printf '%s\n' "$repo" | cut -d/ -f1,2) + + allowed=false + for candidate in $ALLOWED_OWNERS; do + [ "$owner" = "$candidate" ] && allowed=true + done + if [ "$allowed" = false ]; then + fail "$file" "$line" "owner '$owner' is not in ALLOWED_OWNERS ($ALLOWED_OWNERS); add it deliberately in scripts/check-action-pins.sh if this action is meant to run here" + continue + fi + + if ! printf '%s\n' "$ref" | grep -qE '^[0-9a-f]{40}$'; then + fail "$file" "$line" "$repo is pinned to '$ref', which is a movable tag; pin it to the 40-character commit SHA that tag points at" + continue + fi + + if [ -z "$comment" ]; then + fail "$file" "$line" "$repo@${ref:0:8} has no version comment; append '# vX.Y.Z' naming the release this SHA was cut from" + continue + fi + + if ! printf '%s\n' "$comment" | grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+$'; then + fail "$file" "$line" "$repo carries the comment '# $comment'; name an exact patch release such as v7.0.1, because major tags move with every release and would fail this check for a good pin" + continue + fi + + [ "$offline" = true ] && continue + + # Folding stderr into the capture keeps the reason in $actual when the + # lookup fails, and the condition context exempts this from set -e so one + # unreachable repository cannot mask the findings on every later pin. + if ! actual=$(resolve_tag "$repo" "$comment" 2>&1); then + fail "$file" "$line" "could not resolve $repo $comment upstream: $actual" + elif [ -z "$actual" ]; then + fail "$file" "$line" "$repo has no tag $comment upstream" + elif [ "$actual" != "$ref" ]; then + fail "$file" "$line" "$repo $comment is upstream commit $actual, but the workflow pins $ref" + fi +done < <(grep -rnE '^[[:space:]]*(-[[:space:]]+)?uses:' .github/workflows) + +if [ "$failures" -gt 0 ]; then + echo >&2 + echo "$failures of $checked action pin(s) failed verification" >&2 + exit 1 +fi + +echo "all $checked action pin(s) verified$([ "$offline" = true ] && echo " (offline: SHA and comment format only)")" diff --git a/src/TDSProtocol/TDSProtocol.csproj b/src/TDSProtocol/TDSProtocol.csproj index 6e8c87d..4bef8a0 100644 --- a/src/TDSProtocol/TDSProtocol.csproj +++ b/src/TDSProtocol/TDSProtocol.csproj @@ -13,7 +13,7 @@ - + diff --git a/src/TDSProtocolTests/TDSProtocolTests.csproj b/src/TDSProtocolTests/TDSProtocolTests.csproj index 6241e83..dd9124b 100644 --- a/src/TDSProtocolTests/TDSProtocolTests.csproj +++ b/src/TDSProtocolTests/TDSProtocolTests.csproj @@ -11,7 +11,7 @@ - + diff --git a/src/TDSProxy/TDSProxy.csproj b/src/TDSProxy/TDSProxy.csproj index 8658052..9ce2f14 100644 --- a/src/TDSProxy/TDSProxy.csproj +++ b/src/TDSProxy/TDSProxy.csproj @@ -11,7 +11,7 @@ - + diff --git a/src/TestConnection/Program.cs b/src/TestConnection/Program.cs index 63cdfdb..dc03ce3 100644 --- a/src/TestConnection/Program.cs +++ b/src/TestConnection/Program.cs @@ -1,30 +1,58 @@ -using System; -using Microsoft.Data.SqlClient; - -namespace TestConnection -{ - static class Program - { - static void Main() - { - // Use SqlConnection for cross-platform compatibility - var cn = new SqlConnection("Server=pc-mattw.techsoftwareinc.com;Database=IRBM_Dev;User Id=dev\\mattw;Password=dev;MultipleActiveResultSets=false;TrustServerCertificate=true"); - cn.Open(); - while (true) - { - var cmd = cn.CreateCommand(); - cmd.CommandText = "SELECT * FROM Note"; - var dr = cmd.ExecuteReader(); - while (dr.Read()) - { - for (int i = 0; i < dr.FieldCount; i++) - Console.Write("{0}\t", dr.GetValue(i)); - Console.WriteLine(); - } - - if (Console.ReadKey(false).Key == ConsoleKey.Escape) - break; - } - } - } -} +using System; +using Microsoft.Data.SqlClient; + +namespace TestConnection +{ + static class Program + { + // A manual smoke test: point it at a proxy listener and confirm a real TDS + // session survives the round trip. Not part of the published image — the + // Dockerfile builds TDSProxy.csproj alone — and not run by CI, which has no + // SQL Server to talk to. + // + // The connection string is read from the environment rather than compiled in. + // It previously carried a live-looking one (an internal host, a domain user + // and a password) inherited from upstream, which is not something a public + // repository should hold regardless of whether the host still answers. + const string ConnectionStringVariable = "TDSPROXY_TEST_CONNECTION_STRING"; + + static int Main() + { + var connectionString = Environment.GetEnvironmentVariable(ConnectionStringVariable); + if (string.IsNullOrWhiteSpace(connectionString)) + { + Console.Error.WriteLine( + $"Set {ConnectionStringVariable} to the connection string to test, e.g.\n" + + $" export {ConnectionStringVariable}=" + + "'Server=127.0.0.1,1435;Database=YourDb;User Id=you;Password=...;TrustServerCertificate=true'\n" + + "Point Server at the proxy's listener, not at SQL Server directly."); + return 2; + } + + var query = Environment.GetEnvironmentVariable("TDSPROXY_TEST_QUERY") ?? "SELECT 1"; + + using var cn = new SqlConnection(connectionString); + cn.Open(); + while (true) + { + using (var cmd = cn.CreateCommand()) + { + cmd.CommandText = query; + using var dr = cmd.ExecuteReader(); + while (dr.Read()) + { + for (int i = 0; i < dr.FieldCount; i++) + Console.Write("{0}\t", dr.GetValue(i)); + Console.WriteLine(); + } + } + + Console.WriteLine("Press Esc to quit, any other key to repeat."); + if (Console.ReadKey(true).Key == ConsoleKey.Escape) + break; + } + + return 0; + } + } +} diff --git a/src/TestConnection/TestConnection.csproj b/src/TestConnection/TestConnection.csproj index 16935f8..a9d692a 100644 --- a/src/TestConnection/TestConnection.csproj +++ b/src/TestConnection/TestConnection.csproj @@ -10,7 +10,7 @@ - +