From 956afd7ea76a7f5c2cb6879a2f6e6cdb5cc92e08 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 27 Oct 2026 09:07:52 -0500 Subject: [PATCH 1/3] CI: the SQL Server pin test runs on the 2025 leg, against a strict server, trusted and untrusted BACKLOG #3714 slice B, items 6 and 8. The 2025 sqlserver-store leg now starts a second container from the same digest-pinned image on 127.0.0.1:14330, after the other suites. It mounts a generated test certificate (serverAuth EKU, CN and SAN localhost) and an mssql.conf with forceencryption and forcestrict. The service container stays: forcestrict refuses every non-strict login, which every other suite makes. A wait step needs the server's ready line and a TLS-first (TDS 8.0) handshake. The pin test then runs twice: certificate untrusted, then in the runner's OpenSSL store. Each step fails if the test did not run. The test gains strict_chain (strict, no pin). The trusted arm asserts it connected, so its refused wrong pin is told apart from a chain refusal. The 2022 leg is unchanged. Proposed PR title: CI: the SQL Server certificate pin test runs on the 2025 leg against a strict server, with the certificate untrusted and trusted (BACKLOG #3714 items 6, 8) Proposed banner: #3714 items 6 and 8 BUILT, NOT YET MEASURED: the 2025 sqlserver-store leg runs the pin test against a forcestrict server in two arms; read both BACKLOG-2223-PIN lines on the PR run before closing. --- .github/workflows/ci.yml | 162 ++++++++++++++++++++++++++++++++++ tests/test_sqlserver_store.py | 41 +++++++-- 2 files changed, 196 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9592ca3595..42376b93f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3516,6 +3516,168 @@ jobs: tests/test_store_privilege_preflight.py tests/test_store_privilege_schema_split.py + # THE CERTIFICATE PIN UNDER STRICT ENCRYPTION, 2025 LEG ONLY (vault BACKLOG #3714 items 6 and 8). + # tests/test_sqlserver_store.py::test_server_certificate_pin_is_enforced_under_strict_encryption + # needs a server that does a TLS-first (TDS 8.0) handshake, and until this it had never run: both + # legs skipped it (PR 2533, run 40381630197). The `services:` container above cannot be that + # server, for two reasons. It starts before any step, so a certificate made in a step cannot be + # mounted into it. And `network.forcestrict = 1` refuses every non-strict login, which every + # suite above makes. So the leg keeps its service container and starts a SECOND container from + # the same digest-pinned image on 127.0.0.1:14330, after the other suites have finished. + # 2025 only: Microsoft's mssql-conf page lists network.forcestrict as "Applies to: SQL Server + # 2025 (17.x) and later versions", so the 2022 leg stays exactly as it was. + - name: Start a strict-encryption SQL Server for the certificate pin test (2025 only) + if: matrix.label == '2025' + timeout-minutes: 5 + env: + IMAGE: ${{ matrix.image }} + run: | + set -euo pipefail + dir="$RUNNER_TEMP/mssql-strict" + mkdir -p "$dir/tls" + # A throwaway test certificate: Server Authentication EKU, CN and SAN localhost, valid now. + openssl req -x509 -nodes -newkey rsa:2048 -sha256 -days 7 \ + -subj "/CN=localhost" \ + -addext "subjectAltName=DNS:localhost,IP:127.0.0.1" \ + -addext "extendedKeyUsage=serverAuth" \ + -keyout "$dir/tls/mssql.key" -out "$dir/tls/mssql.pem" + openssl x509 -in "$dir/tls/mssql.pem" -noout -subject -dates -ext subjectAltName,extendedKeyUsage + printf '%s\n' \ + '[network]' \ + 'tlscert = /var/opt/mssql/tls/mssql.pem' \ + 'tlskey = /var/opt/mssql/tls/mssql.key' \ + 'forceencryption = 1' \ + 'forcestrict = 1' > "$dir/mssql.conf" + # Read the account the image runs as rather than assume it, and give it the files: Microsoft + # requires them readable by mssql. The key is 400. + uid="$(docker run --rm --entrypoint id "$IMAGE" -u)" + echo "the image runs as uid ${uid}" + sudo chown -R "${uid}:0" "$dir/tls" "$dir/mssql.conf" + sudo chmod 500 "$dir/tls" + sudo chmod 400 "$dir/tls/mssql.key" + sudo chmod 444 "$dir/tls/mssql.pem" + sudo chmod 644 "$dir/mssql.conf" + docker run -d --name mssql-strict -p 127.0.0.1:14330:1433 \ + -e ACCEPT_EULA=Y -e MSSQL_SA_PASSWORD='Str0ng_P@ssw0rd!' \ + -v "$dir/mssql.conf:/var/opt/mssql/mssql.conf" \ + -v "$dir/tls:/var/opt/mssql/tls:ro" \ + "$IMAGE" + + - name: Wait for the strict SQL Server's TLS-first (TDS 8.0) handshake (2025 only) + if: matrix.label == '2025' + timeout-minutes: 5 + # Ready means BOTH the server's own ready line and a completed TLS-first handshake on the + # listener port, which is what a TDS 8.0 client opens with. Each grep reads a here-string, + # never a pipe, so a `grep -q` that stops reading early cannot fail the check. + run: | + out="" + for i in $(seq 1 60); do + state="$(docker inspect -f '{{.State.Status}}' mssql-strict 2>&1 || true)" + if [ "$state" != "running" ]; then + docker logs --tail 80 mssql-strict 2>&1 + echo "::error::the strict-mode SQL Server container is ${state}, not running. Its mssql.conf sets network.forcestrict = 1 (SQL Server 2025 only); read its log above." + exit 1 + fi + logs="$(docker logs mssql-strict 2>&1)" + if grep -q "SQL Server is now ready for client connections" <<<"$logs"; then + # Inside the `if`, so the shell's -e does not end the loop on a refused handshake. + if out="$(timeout 10 openssl s_client -connect 127.0.0.1:14330 -servername localhost -alpn tds/8.0 &1)" \ + && grep -q "BEGIN CERTIFICATE" <<<"$out"; then + echo "BACKLOG-3714-STRICT-READY try=${i} $(grep -m1 -E '^(New|Protocol)' <<<"$out") $(grep -m1 'ALPN' <<<"$out")" + exit 0 + fi + fi + echo "waiting for the strict SQL Server ($i)"; sleep 2 + done + echo "$out" + docker logs --tail 80 mssql-strict 2>&1 + echo "::error::no TLS-first (TDS 8.0) handshake on 127.0.0.1:14330 within about 120 s. The strict-mode SQL Server (network.forcestrict = 1, SQL Server 2025 only) is not serving strict encryption, so the certificate pin test cannot run." + exit 1 + + # The two arms run the same test with the same run body; MEFOR_TEST_SQLSERVER_PIN_TRUSTED names + # the arm. Each step fails unless the test RAN: a skip prints no BACKLOG-2223-PIN line, and a + # skipped pin test reads exactly like a passing one. + - name: Run the certificate pin test, certificate untrusted (2025 strict server) + if: matrix.label == '2025' + timeout-minutes: 5 + env: + MEFOR_TEST_SQLSERVER: "1" + MEFOR_TEST_SQLSERVER_STRICT: "1" + MEFOR_TEST_SQLSERVER_PIN_TRUSTED: "0" + MEFOR_STORE_BACKEND: sqlserver + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "14330" + MEFOR_STORE_DATABASE: master + MEFOR_STORE_AUTH: sql + MEFOR_STORE_USERNAME: sa + MEFOR_STORE_PASSWORD: "Str0ng_P@ssw0rd!" + PYTHONFAULTHANDLER: "1" + run: | + set -o pipefail + arm=untrusted + if [ "$MEFOR_TEST_SQLSERVER_PIN_TRUSTED" = "1" ]; then arm=trusted; fi + log="$RUNNER_TEMP/pin-${arm}.log" + status=0 + bash scripts/ci/retry-native-crash.sh pytest -v -rs --timeout=120 \ + "tests/test_sqlserver_store.py::test_server_certificate_pin_is_enforced_under_strict_encryption" \ + | tee "$log" || status=$? + line="$(grep -o "BACKLOG-2223-PIN arm=${arm} .*" "$log" | tail -n 1 || true)" + if [ -z "$line" ]; then + echo "::error::no BACKLOG-2223-PIN arm=${arm} line: the certificate pin test did not run (skipped, or failed before measuring)." + exit 1 + fi + echo "::notice title=BACKLOG-2223-PIN ${arm}::${line}" + exit "$status" + + - name: Trust the strict server's certificate in the runner's OpenSSL store (2025 only) + if: matrix.label == '2025' + timeout-minutes: 5 + run: | + set -euo pipefail + sudo cp "$RUNNER_TEMP/mssql-strict/tls/mssql.pem" /usr/local/share/ca-certificates/mefor-ci-mssql-strict.crt + sudo update-ca-certificates + # The runner's own OpenSSL must now verify the presented chain AND host name, or the + # trusted arm below would be another untrusted arm. The test checks the driver's view too. + if ! out="$(timeout 10 openssl s_client -connect 127.0.0.1:14330 -servername localhost -alpn tds/8.0 \ + -verify_return_error -verify_hostname localhost &1)"; then + echo "$out" + echo "::error::the runner's OpenSSL does not trust the strict server's certificate after update-ca-certificates" + exit 1 + fi + grep -m1 "Verify return code" <<<"$out" + + - name: Run the certificate pin test, certificate trusted (2025 strict server) + if: matrix.label == '2025' + timeout-minutes: 5 + env: + MEFOR_TEST_SQLSERVER: "1" + MEFOR_TEST_SQLSERVER_STRICT: "1" + MEFOR_TEST_SQLSERVER_PIN_TRUSTED: "1" + MEFOR_STORE_BACKEND: sqlserver + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "14330" + MEFOR_STORE_DATABASE: master + MEFOR_STORE_AUTH: sql + MEFOR_STORE_USERNAME: sa + MEFOR_STORE_PASSWORD: "Str0ng_P@ssw0rd!" + PYTHONFAULTHANDLER: "1" + run: | + set -o pipefail + arm=untrusted + if [ "$MEFOR_TEST_SQLSERVER_PIN_TRUSTED" = "1" ]; then arm=trusted; fi + log="$RUNNER_TEMP/pin-${arm}.log" + status=0 + bash scripts/ci/retry-native-crash.sh pytest -v -rs --timeout=120 \ + "tests/test_sqlserver_store.py::test_server_certificate_pin_is_enforced_under_strict_encryption" \ + | tee "$log" || status=$? + line="$(grep -o "BACKLOG-2223-PIN arm=${arm} .*" "$log" | tail -n 1 || true)" + if [ -z "$line" ]; then + echo "::error::no BACKLOG-2223-PIN arm=${arm} line: the certificate pin test did not run (skipped, or failed before measuring)." + exit 1 + fi + echo "::notice title=BACKLOG-2223-PIN ${arm}::${line}" + exit "$status" + # Postgres store backend (Track B): run the gated store suite against a real PostgreSQL service # container (Linux, so 1x minutes). Runs NIGHTLY + on-demand (workflow_dispatch — use # `gh workflow run ci.yml --ref ` to exercise it on a feature branch) + on PRs that touch diff --git a/tests/test_sqlserver_store.py b/tests/test_sqlserver_store.py index b7dd04e8ea..c1a06f82e9 100644 --- a/tests/test_sqlserver_store.py +++ b/tests/test_sqlserver_store.py @@ -6325,25 +6325,38 @@ def _presented_certificate_pem(host: str, port: int) -> str: @pytest.mark.skipif( os.getenv("MEFOR_TEST_SQLSERVER_STRICT") != "1", reason="set MEFOR_TEST_SQLSERVER_STRICT=1 only where the SQL Server is configured for TDS 8.0 " - "strict encryption; the default CI containers are not, so the TLS-first read is cut", + "strict encryption; the CI service containers are not, so the TLS-first read is cut. The 2025 " + "sqlserver-store leg starts a strict-configured container for this test (vault BACKLOG #3714)", ) async def test_server_certificate_pin_is_enforced_under_strict_encryption(tmp_path: Any) -> None: """The store's pinned string, ``ServerCertificate`` with ``Encrypt=strict``, connects with the certificate the server presents and is refused with any other. Vault BACKLOG #3009 measured on Windows that Driver 18.7 enforced the pin with strict and not with ``Encrypt=yes``, and that - measurement is the evidence the store change rests on. This would be the Linux driver's reading, + measurement is the evidence the store change rests on. This is the Linux driver's reading, toward closing step 1 of #2223, on a server configured for strict encryption. POSITIVE CONTROL: the right pin connects through the same builder, so the wrong pin's refusal is the pin's doing and not a strict handshake this server cannot make. The two strings differ only in the pinned file. + TWO ARMS (vault BACKLOG #3714 item 6). CI runs this test twice on the 2025 leg. The UNTRUSTED arm + runs first, with the server's certificate outside the runner's trust store. The TRUSTED arm sets + ``MEFOR_TEST_SQLSERVER_PIN_TRUSTED=1`` after CI adds that certificate to the OpenSSL store. In the + untrusted arm a driver that skipped the pin would still refuse the wrong pin through the chain, + so only the trusted arm can tell a pin refusal from a chain refusal: there the chain passes, and + ``strict_wrong`` must still be refused. ``strict_chain`` is strict with no pin, which is a chain + check alone. The trusted arm asserts it CONNECTED, because an arm whose chain does not pass + proves nothing about the pin. In the untrusted arm it is measured, and is expected REFUSED. + MEASUREMENT, not asserted: the same two pins beside ``Encrypt=yes``, the string the store sent - before #2223. The CI container's certificate is self-signed and not in the runner's trust store, - so ``yes_right`` is the control for ``yes_wrong``: if ``yes_right`` is REFUSED, the chain check - refused both and ``yes_wrong`` says nothing about the pin. The test is skipped in CI unless + before #2223. In the untrusted arm ``yes_right`` is the control for ``yes_wrong``: if + ``yes_right`` is REFUSED, the chain check refused both and ``yes_wrong`` says nothing about the + pin. In the trusted arm ``yes_right`` would connect through the chain. On a server that forces + strict encryption, though, every ``Encrypt=yes`` login is refused before the certificate matters, + so there both read REFUSED and say nothing about the pin. The test is skipped unless ``MEFOR_TEST_SQLSERVER_STRICT=1`` is set on a strict-configured server; where it runs, its - ``BACKLOG-2223-PIN`` warning line carries each outcome, and a refusal carries the driver's message.""" + ``BACKLOG-2223-PIN`` warning line names the arm and carries each outcome, and a refusal carries + the driver's message.""" import warnings import aioodbc @@ -6384,17 +6397,31 @@ async def outcome(dsn: str) -> str: assert row is not None and row[0] == 1 return "CONNECTED" + trusted = os.getenv("MEFOR_TEST_SQLSERVER_PIN_TRUSTED") == "1" right_dsn, wrong_dsn = pinned_dsn(right), pinned_dsn(wrong) assert "Encrypt=strict;" in right_dsn and "Encrypt=strict;" in wrong_dsn + # No pin and verification on: the builder sends Encrypt=yes, so strict here is the chain alone. + unpinned_dsn = connection_string( + base.model_copy(update={"trust_server_certificate": False, "ssl_root_cert": None}) + ) + assert "Encrypt=yes;" in unpinned_dsn and "ServerCertificate" not in unpinned_dsn results = { "strict_right": await outcome(right_dsn), "strict_wrong": await outcome(wrong_dsn), + "strict_chain": await outcome(unpinned_dsn.replace("Encrypt=yes;", "Encrypt=strict;")), "yes_right": await outcome(right_dsn.replace("Encrypt=strict;", "Encrypt=yes;")), "yes_wrong": await outcome(wrong_dsn.replace("Encrypt=strict;", "Encrypt=yes;")), } + arm = "trusted" if trusted else "untrusted" warnings.warn( - "BACKLOG-2223-PIN " + " ".join(f"{k}={v}" for k, v in results.items()), stacklevel=1 + f"BACKLOG-2223-PIN arm={arm} " + " ".join(f"{k}={v}" for k, v in results.items()), + stacklevel=1, ) + if trusted: + assert results["strict_chain"] == "CONNECTED", ( + "the trusted arm's chain check was refused, so the certificate did not reach the " + "driver's trust store and this arm cannot tell a pin refusal from a chain refusal" + ) assert results["strict_right"] == "CONNECTED", "the right pin was refused: the control failed" assert results["strict_wrong"].startswith("REFUSED"), "a pin of another certificate connected" From 09bc22cfb8d3ca855af2cc7a18aab56b2b3d4252 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 27 Oct 2026 09:31:41 -0500 Subject: [PATCH 2/3] CI: the strict pin leg waits for a strict login, trusts a leaf, and asserts both arms' chain BACKLOG #3714 slice B, items 6 and 8: code-review round 1 fixes. - The test certificate is CA:FALSE, made from a config file. - The container publishes 14330 on all interfaces, the probes use localhost. - The wait is bounded at 180 s inside its cap, and ends with a strict login. - The second server's memory is capped at 2048 MB. - The pin steps' cap covers the retry wrapper's three attempts. - The untrusted arm asserts strict_chain REFUSED; the right pin is asserted first, and the trusted arm's message carries the driver's text. - ADR 0173 and the retry wrapper's header no longer state stale facts. Proposed PR title: CI: the SQL Server certificate pin test runs on the 2025 leg against a strict server, with the certificate untrusted and trusted (BACKLOG #3714 items 6, 8) Proposed banner: #3714 items 6 and 8 BUILT, NOT YET MEASURED: the 2025 sqlserver-store leg runs the pin test against a forcestrict server in two arms; read both BACKLOG-2223-PIN lines on the PR run before closing. --- .github/workflows/ci.yml | 92 ++++++++++++++----- ...ss-terminating-and-originating-surfaces.md | 7 +- scripts/ci/retry-native-crash.sh | 2 +- tests/test_sqlserver_store.py | 31 ++++--- 4 files changed, 94 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42376b93f5..a1e225c720 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3523,9 +3523,11 @@ jobs: # server, for two reasons. It starts before any step, so a certificate made in a step cannot be # mounted into it. And `network.forcestrict = 1` refuses every non-strict login, which every # suite above makes. So the leg keeps its service container and starts a SECOND container from - # the same digest-pinned image on 127.0.0.1:14330, after the other suites have finished. + # the same digest-pinned image on port 14330, after the other suites have finished. # 2025 only: Microsoft's mssql-conf page lists network.forcestrict as "Applies to: SQL Server - # 2025 (17.x) and later versions", so the 2022 leg stays exactly as it was. + # 2025 (17.x) and later versions", so the 2022 leg stays exactly as it was. Because the server + # forces strict, every Encrypt=yes login is refused before the certificate matters, so the + # test's yes_right and yes_wrong readings say nothing about the pin on this leg. - name: Start a strict-encryption SQL Server for the certificate pin test (2025 only) if: matrix.label == '2025' timeout-minutes: 5 @@ -3536,18 +3538,31 @@ jobs: dir="$RUNNER_TEMP/mssql-strict" mkdir -p "$dir/tls" # A throwaway test certificate: Server Authentication EKU, CN and SAN localhost, valid now. - openssl req -x509 -nodes -newkey rsa:2048 -sha256 -days 7 \ - -subj "/CN=localhost" \ - -addext "subjectAltName=DNS:localhost,IP:127.0.0.1" \ - -addext "extendedKeyUsage=serverAuth" \ + # CA:FALSE, so the trusted arm adds a leaf to the runner's store and not an issuer that + # could sign for any host. A config file rather than -addext, so no OpenSSL default + # section adds a second basicConstraints. + printf '%s\n' \ + '[req]' 'distinguished_name = dn' 'x509_extensions = ext' 'prompt = no' \ + '[dn]' 'CN = localhost' \ + '[ext]' 'basicConstraints = critical,CA:FALSE' \ + 'subjectAltName = DNS:localhost,IP:127.0.0.1' 'extendedKeyUsage = serverAuth' \ + > "$dir/req.cnf" + openssl req -x509 -nodes -newkey rsa:2048 -sha256 -days 7 -config "$dir/req.cnf" \ -keyout "$dir/tls/mssql.key" -out "$dir/tls/mssql.pem" - openssl x509 -in "$dir/tls/mssql.pem" -noout -subject -dates -ext subjectAltName,extendedKeyUsage + openssl x509 -in "$dir/tls/mssql.pem" -noout -subject -dates \ + -ext basicConstraints,subjectAltName,extendedKeyUsage + # The runner's own copy of the certificate, for the readiness login below. The mounted + # directory is the mssql account's alone. + cp "$dir/tls/mssql.pem" "$dir/server.pem" + # The second server's memory is capped, since the service container is still running. printf '%s\n' \ '[network]' \ 'tlscert = /var/opt/mssql/tls/mssql.pem' \ 'tlskey = /var/opt/mssql/tls/mssql.key' \ 'forceencryption = 1' \ - 'forcestrict = 1' > "$dir/mssql.conf" + 'forcestrict = 1' \ + '[memory]' \ + 'memorylimitmb = 2048' > "$dir/mssql.conf" # Read the account the image runs as rather than assume it, and give it the files: Microsoft # requires them readable by mssql. The key is 400. uid="$(docker run --rm --entrypoint id "$IMAGE" -u)" @@ -3557,21 +3572,44 @@ jobs: sudo chmod 400 "$dir/tls/mssql.key" sudo chmod 444 "$dir/tls/mssql.pem" sudo chmod 644 "$dir/mssql.conf" - docker run -d --name mssql-strict -p 127.0.0.1:14330:1433 \ + # Published like the service container, so `localhost` reaches it over IPv4 or IPv6. + docker run -d --name mssql-strict -p 14330:1433 \ -e ACCEPT_EULA=Y -e MSSQL_SA_PASSWORD='Str0ng_P@ssw0rd!' \ -v "$dir/mssql.conf:/var/opt/mssql/mssql.conf" \ -v "$dir/tls:/var/opt/mssql/tls:ro" \ "$IMAGE" - - name: Wait for the strict SQL Server's TLS-first (TDS 8.0) handshake (2025 only) + - name: Wait for the strict SQL Server's TLS-first (TDS 8.0) handshake and a strict login (2025 only) if: matrix.label == '2025' timeout-minutes: 5 - # Ready means BOTH the server's own ready line and a completed TLS-first handshake on the - # listener port, which is what a TDS 8.0 client opens with. Each grep reads a here-string, - # never a pipe, so a `grep -q` that stops reading early cannot fail the check. + # Ready means three things in order: the server's own ready line, a completed TLS-first + # handshake on the listener port (what a TDS 8.0 client opens with), and a strict login + # pinning the certificate the step above made. The login is here so a server still settling + # its first boot is reported as not ready, not as the pin test's failed control. The loop is + # bounded by time, well inside the step cap, so its own diagnostic always prints. Each grep + # reads a here-string, never a pipe, so a `grep -q` that stops reading early cannot fail it. run: | + dir="$RUNNER_TEMP/mssql-strict" + cat > "$dir/login-probe.py" <<'PY' + import sys + + import pyodbc + + try: + pyodbc.connect( + "DRIVER={ODBC Driver 18 for SQL Server};SERVER=localhost,14330;DATABASE=master;" + "UID=sa;PWD={Str0ng_P@ssw0rd!};Encrypt=strict;" + "ServerCertificate={" + sys.argv[1] + "};", + timeout=5, + ).close() + except pyodbc.Error as exc: + print(f"strict login refused: {exc.args[-1] if exc.args else type(exc).__name__}") + sys.exit(1) + PY + stage="the server's ready line" out="" - for i in $(seq 1 60); do + deadline=$((SECONDS + 180)) + while [ "$SECONDS" -lt "$deadline" ]; do state="$(docker inspect -f '{{.State.Status}}' mssql-strict 2>&1 || true)" if [ "$state" != "running" ]; then docker logs --tail 80 mssql-strict 2>&1 @@ -3579,27 +3617,33 @@ jobs: exit 1 fi logs="$(docker logs mssql-strict 2>&1)" + # Each probe sits inside an `if`, so the shell's -e does not end the loop on a refusal. if grep -q "SQL Server is now ready for client connections" <<<"$logs"; then - # Inside the `if`, so the shell's -e does not end the loop on a refused handshake. - if out="$(timeout 10 openssl s_client -connect 127.0.0.1:14330 -servername localhost -alpn tds/8.0 &1)" \ + stage="a TLS-first (TDS 8.0) handshake on localhost:14330" + if out="$(timeout 5 openssl s_client -connect localhost:14330 -servername localhost -alpn tds/8.0 &1)" \ && grep -q "BEGIN CERTIFICATE" <<<"$out"; then - echo "BACKLOG-3714-STRICT-READY try=${i} $(grep -m1 -E '^(New|Protocol)' <<<"$out") $(grep -m1 'ALPN' <<<"$out")" - exit 0 + handshake="$(grep -m1 -E '^(New|Protocol)' <<<"$out") $(grep -m1 'ALPN' <<<"$out")" + stage="a strict login pinning the generated certificate" + if out="$(timeout 20 python "$dir/login-probe.py" "$dir/server.pem" 2>&1)"; then + echo "BACKLOG-3714-STRICT-READY after ${SECONDS}s: ${handshake}; strict login CONNECTED" + exit 0 + fi fi fi - echo "waiting for the strict SQL Server ($i)"; sleep 2 + echo "waiting for the strict SQL Server: ${stage} (${SECONDS}s)"; sleep 2 done echo "$out" docker logs --tail 80 mssql-strict 2>&1 - echo "::error::no TLS-first (TDS 8.0) handshake on 127.0.0.1:14330 within about 120 s. The strict-mode SQL Server (network.forcestrict = 1, SQL Server 2025 only) is not serving strict encryption, so the certificate pin test cannot run." + echo "::error::the strict-mode SQL Server (network.forcestrict = 1, SQL Server 2025 only) was not ready within 180 s; it never passed ${stage}. The last probe's output and the server log are above. The certificate pin test cannot run." exit 1 # The two arms run the same test with the same run body; MEFOR_TEST_SQLSERVER_PIN_TRUSTED names # the arm. Each step fails unless the test RAN: a skip prints no BACKLOG-2223-PIN line, and a - # skipped pin test reads exactly like a passing one. + # skipped pin test reads exactly like a passing one. The cap covers the retry wrapper's three + # attempts at pytest's 120 s each, so a native-crash retry reports through the wrapper. - name: Run the certificate pin test, certificate untrusted (2025 strict server) if: matrix.label == '2025' - timeout-minutes: 5 + timeout-minutes: 8 env: MEFOR_TEST_SQLSERVER: "1" MEFOR_TEST_SQLSERVER_STRICT: "1" @@ -3638,7 +3682,7 @@ jobs: sudo update-ca-certificates # The runner's own OpenSSL must now verify the presented chain AND host name, or the # trusted arm below would be another untrusted arm. The test checks the driver's view too. - if ! out="$(timeout 10 openssl s_client -connect 127.0.0.1:14330 -servername localhost -alpn tds/8.0 \ + if ! out="$(timeout 10 openssl s_client -connect localhost:14330 -servername localhost -alpn tds/8.0 \ -verify_return_error -verify_hostname localhost &1)"; then echo "$out" echo "::error::the runner's OpenSSL does not trust the strict server's certificate after update-ca-certificates" @@ -3648,7 +3692,7 @@ jobs: - name: Run the certificate pin test, certificate trusted (2025 strict server) if: matrix.label == '2025' - timeout-minutes: 5 + timeout-minutes: 8 env: MEFOR_TEST_SQLSERVER: "1" MEFOR_TEST_SQLSERVER_STRICT: "1" diff --git a/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md b/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md index c6212a9620..5b13d2ca6b 100644 --- a/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md +++ b/docs/adr/0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md @@ -1181,8 +1181,11 @@ decisions too: (*code*). The live Linux test of the pin, `test_server_certificate_pin_is_enforced_under_strict_encryption`, -runs only with `MEFOR_TEST_SQLSERVER_STRICT=1` on a strict-configured server. No CI job sets it, so -it is skipped in CI and the Linux reading is not yet taken. +runs only with `MEFOR_TEST_SQLSERVER_STRICT=1` on a strict-configured server. The 2025 +`sqlserver-store` CI leg sets it, against a second container that forces strict encryption, and +runs the test with the server's certificate untrusted and then trusted (vault BACKLOG #3714). The +Linux reading comes from that leg's two `BACKLOG-2223-PIN` lines, once a run is read. **CORRECTED 2026-10-27:** this read *"No +CI job sets it, so it is skipped in CI and the Linux reading is not yet taken."* The paragraph above is kept as written. Its first sentence, *"`connection_string` emits `ServerCertificate=` beside `Encrypt=yes`, never `Encrypt=strict`"*, describes the code before this diff --git a/scripts/ci/retry-native-crash.sh b/scripts/ci/retry-native-crash.sh index f7471169a5..e5c22d798b 100644 --- a/scripts/ci/retry-native-crash.sh +++ b/scripts/ci/retry-native-crash.sh @@ -66,7 +66,7 @@ set -uo pipefail attempts="${RETRY_NATIVE_CRASH_ATTEMPTS:-3}" -# The default serves the nine database-leg call sites, so only a caller where neither class is +# The default serves every database-leg call site, so only a caller where neither class is # established has to opt out. It names both known classes and does not pick one. default_cause=" -- a database-leg crash class: the pyodbc py3.14 parameter-binding segfault (mkleehammer/pyodbc#1459) or a handle freed under a running statement on the store cancel path (BACKLOG #2049); this wrapper cannot tell them apart, read the faulthandler dump" cause="${RETRY_NATIVE_CRASH_CAUSE-$default_cause}" diff --git a/tests/test_sqlserver_store.py b/tests/test_sqlserver_store.py index c1a06f82e9..c073cba7ac 100644 --- a/tests/test_sqlserver_store.py +++ b/tests/test_sqlserver_store.py @@ -6345,15 +6345,17 @@ async def test_server_certificate_pin_is_enforced_under_strict_encryption(tmp_pa untrusted arm a driver that skipped the pin would still refuse the wrong pin through the chain, so only the trusted arm can tell a pin refusal from a chain refusal: there the chain passes, and ``strict_wrong`` must still be refused. ``strict_chain`` is strict with no pin, which is a chain - check alone. The trusted arm asserts it CONNECTED, because an arm whose chain does not pass - proves nothing about the pin. In the untrusted arm it is measured, and is expected REFUSED. + check alone. Each arm asserts it: CONNECTED in the trusted arm, because an arm whose chain does + not pass proves nothing about the pin, and REFUSED in the untrusted arm, so that arm is not the + trusted condition under the wrong name. A refusal is not asserted to be a certificate mismatch; + its driver message is in the warning line. MEASUREMENT, not asserted: the same two pins beside ``Encrypt=yes``, the string the store sent - before #2223. In the untrusted arm ``yes_right`` is the control for ``yes_wrong``: if - ``yes_right`` is REFUSED, the chain check refused both and ``yes_wrong`` says nothing about the - pin. In the trusted arm ``yes_right`` would connect through the chain. On a server that forces - strict encryption, though, every ``Encrypt=yes`` login is refused before the certificate matters, - so there both read REFUSED and say nothing about the pin. The test is skipped unless + before #2223. On a server that does not force strict encryption, ``yes_right`` is the control + for ``yes_wrong`` in the untrusted arm: if ``yes_right`` is REFUSED, the chain check refused + both and ``yes_wrong`` says nothing about the pin. The CI server forces strict encryption, so + there every ``Encrypt=yes`` login is refused before the certificate matters, in both arms, and + both readings say nothing about the pin. The test is skipped unless ``MEFOR_TEST_SQLSERVER_STRICT=1`` is set on a strict-configured server; where it runs, its ``BACKLOG-2223-PIN`` warning line names the arm and carries each outcome, and a refusal carries the driver's message.""" @@ -6376,7 +6378,7 @@ async def test_server_certificate_pin_is_enforced_under_strict_encryption(tmp_pa wrong.write_bytes(make_self_signed("mf-test-not-the-server", [], 30)[0]) def pinned_dsn(pin: Any) -> str: - # CI trusts the container's self-signed certificate through the escape; the pin replaces that. + # Verification on, so the pin rides the secure posture and goes out with Encrypt=strict. return connection_string( base.model_copy(update={"trust_server_certificate": False, "ssl_root_cert": str(pin)}) ) @@ -6417,12 +6419,19 @@ async def outcome(dsn: str) -> str: f"BACKLOG-2223-PIN arm={arm} " + " ".join(f"{k}={v}" for k, v in results.items()), stacklevel=1, ) + # The right pin first: if it fails, the server or the login is at fault, and the chain + # assertions below would name the wrong cause. + assert results["strict_right"] == "CONNECTED", "the right pin was refused: the control failed" if trusted: assert results["strict_chain"] == "CONNECTED", ( - "the trusted arm's chain check was refused, so the certificate did not reach the " - "driver's trust store and this arm cannot tell a pin refusal from a chain refusal" + "the trusted arm's strict login with no pin was refused, so this arm cannot tell a pin " + f"refusal from a chain refusal (driver: {results['strict_chain']})" + ) + else: + assert results["strict_chain"].startswith("REFUSED"), ( + "the untrusted arm's strict login with no pin connected, so the certificate is already " + "trusted through the chain and this arm measured the trusted condition" ) - assert results["strict_right"] == "CONNECTED", "the right pin was refused: the control failed" assert results["strict_wrong"].startswith("REFUSED"), "a pin of another certificate connected" From 92fdc73da71d3ae5827294a2f59c0a412e9c60d9 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 27 Oct 2026 10:37:34 -0500 Subject: [PATCH 3/3] CI: the pin test reads ODBC keys, not substrings, so TrustServerCertificate no longer reads as a pin Run 40524945884, job 122262048822 (sql server 2025): the strict server came up and a strict login connected, then the untrusted arm failed before it measured. Its guard, "ServerCertificate" not in unpinned_dsn, is also true of TrustServerCertificate=no;, which every verified string carries. The DSN was right; the guard was wrong. The test now reads the string as keyword=value pairs, brace-aware, and sets Encrypt by key instead of a substring replace. A new server-free test checks the reader on the builder's own strings, with a password holding the text a substring check trips on. It runs on both SQL Server legs ahead of the strict step. PR title: CI: the SQL Server certificate pin test runs on the 2025 leg against a strict server, with the certificate untrusted and trusted (BACKLOG #3714 items 6, 8) BACKLOG #3714 --- tests/test_sqlserver_store.py | 100 ++++++++++++++++++++++++++++++++-- 1 file changed, 95 insertions(+), 5 deletions(-) diff --git a/tests/test_sqlserver_store.py b/tests/test_sqlserver_store.py index c073cba7ac..0e349d5610 100644 --- a/tests/test_sqlserver_store.py +++ b/tests/test_sqlserver_store.py @@ -6322,6 +6322,91 @@ def _presented_certificate_pem(host: str, port: int) -> str: return ssl.DER_cert_to_PEM_cert(der) +def _odbc_pairs(dsn: str) -> list[tuple[str, str]]: + """Split an ODBC connection string into ``(keyword, raw value)`` pairs, in order. + + Keys are read as keys, never as substrings: ``"ServerCertificate" in dsn`` is also true of + ``TrustServerCertificate=no;``, which every verified string carries (vault BACKLOG #3714). A + brace-quoted value is read whole, with ``}}`` as an escaped brace, so a ``;`` or ``=`` inside + one, as a password may hold, starts no keyword.""" + pairs: list[tuple[str, str]] = [] + i = 0 + while i < len(dsn): + eq = dsn.index("=", i) + j = eq + 1 + if dsn.startswith("{", j): + j += 1 + while True: + close = dsn.index("}", j) + if dsn.startswith("}}", close): + j = close + 2 + continue + j = close + 1 + break + end = dsn.find(";", j) + if end == -1: + end = len(dsn) + pairs.append((dsn[i:eq], dsn[eq + 1 : end])) + i = end + 1 + return pairs + + +def _odbc_value(dsn: str, keyword: str) -> str | None: + """The value ``keyword`` takes in ``dsn``, or ``None``. ODBC keywords are case-insensitive and + the last one wins, as the driver reads them.""" + values = [v for k, v in _odbc_pairs(dsn) if k.strip().lower() == keyword.lower()] + return values[-1] if values else None + + +def _odbc_with(dsn: str, keyword: str, value: str) -> str: + """``dsn`` with every ``keyword`` set to ``value`` and nothing else changed. A substring + ``replace`` would also rewrite matching text inside a brace-quoted value.""" + pairs = _odbc_pairs(dsn) + assert any(k.strip().lower() == keyword.lower() for k, _ in pairs), ( + f"no {keyword} in the string" + ) + return "".join(f"{k}={value if k.strip().lower() == keyword.lower() else v};" for k, v in pairs) + + +def test_odbc_keyword_reader_tells_server_certificate_from_trust_server_certificate() -> None: + """The pin test's string checks, on the builder's own strings, with no server. It needs none, + so it runs on both SQL Server legs ahead of the strict step that depends on it (vault BACKLOG + #3714). The password holds the text a substring check would trip on.""" + from messagefoundry.config.settings import StoreBackend, StoreSettings + from messagefoundry.store.sqlserver import connection_string + + base = StoreSettings( + backend=StoreBackend.SQLSERVER, + server="localhost", + port=14330, + database="master", + username="sa", + password="p}w;Encrypt=yes;ServerCertificate={x}", + ) + + def dsn(pin: str | None) -> str: + # model_copy, as the pin test does: the builder reads no file, so none need exist. + return connection_string( + base.model_copy(update={"trust_server_certificate": False, "ssl_root_cert": pin}) + ) + + unpinned, pinned = dsn(None), dsn("/tmp/presented.pem") + # CONTROL: the substring guard this replaced misreads the unpinned string as pinned. + assert "ServerCertificate" in unpinned + assert _odbc_value(unpinned, "ServerCertificate") is None + assert _odbc_value(unpinned, "TrustServerCertificate") == "no" + assert _odbc_value(unpinned, "Encrypt") == "yes" + assert _odbc_value(pinned, "ServerCertificate") == "{/tmp/presented.pem}" + assert _odbc_value(pinned, "Encrypt") == "strict" + # The password is one value, read whole, and a rewrite leaves it as it was. + assert _odbc_value(unpinned, "PWD") == "{p}}w;Encrypt=yes;ServerCertificate={x}}}" + assert "".join(f"{k}={v};" for k, v in _odbc_pairs(unpinned)) == unpinned + chain = _odbc_with(unpinned, "Encrypt", "strict") + assert _odbc_value(chain, "Encrypt") == "strict" + assert _odbc_value(chain, "PWD") == _odbc_value(unpinned, "PWD") + assert chain == unpinned.replace("Encrypt=yes;TrustServer", "Encrypt=strict;TrustServer") + + @pytest.mark.skipif( os.getenv("MEFOR_TEST_SQLSERVER_STRICT") != "1", reason="set MEFOR_TEST_SQLSERVER_STRICT=1 only where the SQL Server is configured for TDS 8.0 " @@ -6401,18 +6486,23 @@ async def outcome(dsn: str) -> str: trusted = os.getenv("MEFOR_TEST_SQLSERVER_PIN_TRUSTED") == "1" right_dsn, wrong_dsn = pinned_dsn(right), pinned_dsn(wrong) - assert "Encrypt=strict;" in right_dsn and "Encrypt=strict;" in wrong_dsn + # Keys, not substrings: "ServerCertificate" is also inside "TrustServerCertificate=no;", which + # every verified string carries (vault BACKLOG #3714; run 40524945884 failed on exactly that). + for dsn in (right_dsn, wrong_dsn): + assert _odbc_value(dsn, "Encrypt") == "strict" + assert _odbc_value(dsn, "ServerCertificate") is not None # No pin and verification on: the builder sends Encrypt=yes, so strict here is the chain alone. unpinned_dsn = connection_string( base.model_copy(update={"trust_server_certificate": False, "ssl_root_cert": None}) ) - assert "Encrypt=yes;" in unpinned_dsn and "ServerCertificate" not in unpinned_dsn + assert _odbc_value(unpinned_dsn, "Encrypt") == "yes" + assert _odbc_value(unpinned_dsn, "ServerCertificate") is None results = { "strict_right": await outcome(right_dsn), "strict_wrong": await outcome(wrong_dsn), - "strict_chain": await outcome(unpinned_dsn.replace("Encrypt=yes;", "Encrypt=strict;")), - "yes_right": await outcome(right_dsn.replace("Encrypt=strict;", "Encrypt=yes;")), - "yes_wrong": await outcome(wrong_dsn.replace("Encrypt=strict;", "Encrypt=yes;")), + "strict_chain": await outcome(_odbc_with(unpinned_dsn, "Encrypt", "strict")), + "yes_right": await outcome(_odbc_with(right_dsn, "Encrypt", "yes")), + "yes_wrong": await outcome(_odbc_with(wrong_dsn, "Encrypt", "yes")), } arm = "trusted" if trusted else "untrusted" warnings.warn(