Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog.d/3714-odbc.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- **The SQL Server store, the DATABASE connector and the test harness build their TLS keywords in
one place.** Each used to carry its own brace quoting and its own `Encrypt` and
`TrustServerCertificate` tail. All three now call `messagefoundry/odbc_dsn.py`, and each builds
the same strings as before. Only the store sends a `ServerCertificate` pin. The shared code
refuses a pin beside `encrypt = false` or `trust_server_certificate = true`, a shape no current
caller sends: a pin goes out with `Encrypt=strict`, which would quietly override the weakened
posture the caller asked for. (`BACKLOG #3714`)
33 changes: 12 additions & 21 deletions harness/drivers/_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

from harness.endpoints import ENV_PREFIX, Endpoints
from harness.endpoints.database import ENVIRONMENT_ONLY
from messagefoundry.odbc_dsn import odbc_brace, odbc_connection_string
from messagefoundry.parsing.peek import DEFAULT_MAX_MESSAGE_BYTES

#: The ODBC driver the engine's SQL Server preset names, and so the one the harness dials with.
Expand Down Expand Up @@ -139,31 +140,21 @@ def target(endpoints: Endpoints, environ: Mapping[str, str] | None = None) -> Ta
)


def _brace(value: str) -> str:
"""ODBC-quote a value, doubling any ``}``, so a value cannot add a connection keyword."""
return "{" + value.replace("}", "}}") + "}"


def build_dsn(t: Target) -> str:
"""The ODBC connection string for ``t``. TLS is always on and always verified: the harness
never weakens a hop the engine itself would refuse to weaken."""
never weakens a hop the engine itself would refuse to weaken. The quoting and the TLS tail are
the engine's own (:mod:`messagefoundry.odbc_dsn`, BACKLOG #3714)."""
if any(ch in _ODBC_METACHARACTERS for ch in t.server):
raise ValueError("database_server must not contain ';', '{', '}', '=' or a newline")
return (
";".join(
[
f"DRIVER={_brace(ODBC_DRIVER)}",
f"SERVER={t.server},{t.port}",
f"DATABASE={_brace(t.database)}",
"APP={messagefoundry-harness}",
f"UID={_brace(t.username)}",
f"PWD={_brace(t.password)}",
"Encrypt=yes",
"TrustServerCertificate=no",
]
)
+ ";"
)
head = [
f"DRIVER={odbc_brace(ODBC_DRIVER)}",
f"SERVER={t.server},{t.port}",
f"DATABASE={odbc_brace(t.database)}",
"APP={messagefoundry-harness}",
f"UID={odbc_brace(t.username)}",
f"PWD={odbc_brace(t.password)}",
]
return odbc_connection_string(head, encrypt=True, trust_server_certificate=False)


def unavailable(endpoints: Endpoints, environ: Mapping[str, str] | None = None) -> str:
Expand Down
4 changes: 2 additions & 2 deletions messagefoundry/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4762,8 +4762,8 @@ def registry_filter(reg: Registry) -> Registry: # noqa: F811 (local shard-bound
api_tls_client_cert_files=settings.api.tls_client_cert_files,
# BACKLOG #299: the settings-level CRL files ([tls], [logging] forward, [auth] OIDC, [store],
# [api] client), so a CRL going stale on an OUTBOUND hop alerts before it fails a handshake.
# Vault BACKLOG #2223: plus the SQL Server store pin, whose expiry the driver does not check
# under Encrypt=strict.
# Vault BACKLOG #2223: plus the SQL Server store pin, whose expiry the engine watches itself
# (why: the `ssl_root_cert` row of docs/CONFIGURATION.md).
settings_crls=[*crls_from_settings(settings), *certs_from_settings(settings)],
# Reserve the engine's own API listener so no inbound can be wired onto it (it would collide
# with uvicorn at bind); surfaced as a clear PortConflictError at check/start instead.
Expand Down
6 changes: 3 additions & 3 deletions messagefoundry/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -957,9 +957,9 @@ class StoreSettings(_Section):
# CA-bundle pin (chain + hostname still verified).
# * SQL SERVER — the ODBC Driver 18.1+ `ServerCertificate` keyword pins the server's OWN certificate
# by file (an exact-cert match, brace-quoted STORE-5-safe), so a CA file matches nothing. It goes
# out with `Encrypt=strict`: on the Driver 18.7 build tested, the pin was enforced with strict and
# not with `Encrypt=yes` (vault BACKLOG #2223). Strict is TDS 8.0, so it needs SQL Server 2022 or
# later, the supported floor; an older server refuses the connection. Not a revocation control.
# out with `Encrypt=strict` (vault BACKLOG #2223). Why strict, the server version it needs, and the
# build and platform it was tested on: the `ssl_root_cert` row of docs/CONFIGURATION.md. Not a
# revocation control.
# REJECTED for SQLite (no TLS at all). A path, not a secret — it may live in the config file /
# connections.toml. Empty = use the system trust store (the secure default). Existence is checked at load
# (a missing file fails loud here, not confusingly at connect).
Expand Down
67 changes: 67 additions & 0 deletions messagefoundry/odbc_dsn.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# SPDX-License-Identifier: AGPL-3.0-or-later
# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors
"""Brace quoting and the TLS tail of a SQL Server ODBC Driver 18 connection string (BACKLOG #3714).

The SQL Server store (:func:`messagefoundry.store.sqlserver.connection_string`) and the DATABASE
connector (:func:`messagefoundry.transports.database._build_dsn`) both build their strings here.
``transports/`` may not import ``store/`` (``tests/test_dependency_boundaries.py``), so this is a
leaf module that imports nothing from the engine.

Two rules hold for every string built here:

* :func:`odbc_brace` quotes a free-text value, so a value holding ``;``, ``{``, ``}`` or ``=`` (a
password, say) cannot add keywords (STORE-5). It does not refuse a control character.
* :func:`odbc_connection_string` puts the TLS keywords **last**, after every other part.

The two builders used to say the tail wins because ODBC keeps the last value of a repeated keyword.
Microsoft's ODBC reference for ``SQLDriverConnect`` says a driver uses the FIRST occurrence, and
Microsoft's SQL Server driver has not been measured here either way. So the position is not the
guard against a second TLS keyword. The guard is that every head part is either a fixed keyword or
a quoted or validated value, so no head part can carry one.

What the pin does under ``Encrypt=strict``, and how far that was measured, is stated once, in the
``ssl_root_cert`` row of ``docs/CONFIGURATION.md``.
"""

from __future__ import annotations

__all__ = ["odbc_brace", "odbc_connection_string"]


def odbc_brace(value: str) -> str:
"""ODBC-quote ``value`` in braces, doubling any ``}`` inside it.

Inside braces the driver reads ``;``, ``{`` and ``=`` as text, and ``}}`` as one ``}``, so an
attacker-influenced value cannot end the value early and add a keyword."""
return "{" + value.replace("}", "}}") + "}"


def odbc_connection_string(
head: list[str],
*,
encrypt: bool,
trust_server_certificate: bool,
pin: str | None = None,
) -> str:
"""Join ``head`` and the TLS tail into one ``;``-terminated ODBC connection string.

``head`` is every ``KEY=value`` part but the TLS ones, already quoted. A list, so a joined
string cannot type-check here. The tail follows it: ``ServerCertificate`` when ``pin`` is set,
then ``Encrypt`` and ``TrustServerCertificate``. ``Encrypt`` is ``strict`` when a pin is sent,
and otherwise ``yes`` or ``no`` from ``encrypt``.

A pin beside ``encrypt=False`` or ``trust_server_certificate=True`` raises ``ValueError``. The
caller asked for a weakened hop, and a pin sends ``Encrypt=strict``, which Microsoft documents
as ignoring ``TrustServerCertificate``. So the string would quietly turn on encryption or
verification the caller turned off, and the two settings would disagree. No current caller
passes that shape. The pin file's own checks are the caller's job, as
:func:`messagefoundry.store.sqlserver.check_pinned_certificate` is the store's."""
if pin and (not encrypt or trust_server_certificate):
raise ValueError(
"an ODBC ServerCertificate pin needs encrypt on and trust_server_certificate off: "
"a pin sends Encrypt=strict, which would override the weakened posture asked for"
)
tail = [f"ServerCertificate={odbc_brace(pin)}"] if pin else []
tail.append(f"Encrypt={'strict' if pin else ('yes' if encrypt else 'no')}")
tail.append(f"TrustServerCertificate={'yes' if trust_server_certificate else 'no'}")
return ";".join([*head, *tail]) + ";"
60 changes: 25 additions & 35 deletions messagefoundry/store/sqlserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
weakened_tls_escape_permitted,
)
from messagefoundry.config.tls_policy import HopPosture, current_hop_posture
from messagefoundry.odbc_dsn import odbc_brace, odbc_connection_string
from messagefoundry.odbc_env import disable_driver_manager_pooling
from messagefoundry.parsing.binary import strip_documents as _strip_documents
from messagefoundry.redaction import codec_safe_str, safe_text
Expand Down Expand Up @@ -2711,20 +2712,15 @@ def _schema_hash() -> str:
return hashlib.sha256("\n".join(_SCHEMA).encode()).hexdigest()


def _odbc_brace(value: str) -> str:
"""ODBC-quote a value in braces, doubling any internal ``}`` — neutralizes ``; { } =`` inside it
so an attacker-influenced value (e.g. a password) can't inject extra connection keywords."""
return "{" + value.replace("}", "}}") + "}"


def check_pinned_certificate(
settings: StoreSettings, *, posture: HopPosture | None = None, now: float | None = None
) -> None:
"""Refuse a ``[store].ssl_root_cert`` pin the engine must not send. Raises ``ValueError`` naming
the setting. A no-op when the store sends no pin.

Vault BACKLOG #2223: under ``Encrypt=strict`` a pin match stands in for the driver's own chain,
host-name and expiry checks, and the pinned file is the connection's only trust anchor. So:
Vault BACKLOG #2223: the pinned file is the connection's only trust anchor, because of what a
pin match does under ``Encrypt=strict``. The ``ssl_root_cert`` row of ``docs/CONFIGURATION.md``
states that once, with the build and platform it was tested on. So:

* it takes the trust-anchor ACL and path checks with their own rules and messages
(:func:`~messagefoundry.auth.trust_anchors.enforced_anchor`): a file or folder a broad
Expand Down Expand Up @@ -2802,10 +2798,8 @@ def check_pinned_certificate(
def connection_string(settings: StoreSettings, *, posture: HopPosture | None = None) -> str:
"""Build an ODBC connection string for the Microsoft ODBC Driver 18 from store settings.

Free-text values are brace-quoted to prevent connection-string injection (STORE-5), and the
``Encrypt``/``TrustServerCertificate`` security flags are emitted **last** so — ODBC being
last-wins on duplicate keywords — nothing earlier can downgrade TLS. Identity fields are also
validated up front (see ``StoreSettings._no_odbc_injection``).
Quoting and the TLS tail: :mod:`messagefoundry.odbc_dsn` (STORE-5, BACKLOG #3714). Identity
fields are also validated up front (see ``StoreSettings._no_odbc_injection``).

``posture`` is the deriving instance's :class:`HopPosture` (threaded from ``open_store`` by the
serve/engine caller). ``None`` reads the ambient posture, and with none the escape is NOT
Expand Down Expand Up @@ -2834,42 +2828,37 @@ def connection_string(settings: StoreSettings, *, posture: HopPosture | None = N
parts = [
"DRIVER={ODBC Driver 18 for SQL Server}",
f"SERVER={settings.server},{settings.port}", # server validated; port is an int
f"DATABASE={_odbc_brace(settings.database or '')}",
f"DATABASE={odbc_brace(settings.database or '')}",
# No login-timeout keyword here, on purpose (BACKLOG #1626). `Connection Timeout=` is an
# ADO.NET keyword that ODBC Driver 18 silently ignores: measured against a black-hole address,
# a DSN carrying `Connection Timeout=2` still waited 15.1 s, the same as no timeout at all.
# The driver's login timeout is SQL_ATTR_LOGIN_TIMEOUT, which pyodbc sets from its `timeout=`
# argument, so every connect site passes `timeout=settings.connect_timeout` instead.
f"APP={_odbc_brace(settings.application_name)}",
f"APP={odbc_brace(settings.application_name)}",
]
if settings.auth is SqlAuth.SQL:
parts.append(f"UID={_odbc_brace(settings.username or '')}")
parts.append(f"PWD={_odbc_brace(settings.password or '')}")
parts.append(f"UID={odbc_brace(settings.username or '')}")
parts.append(f"PWD={odbc_brace(settings.password or '')}")
elif settings.auth is SqlAuth.INTEGRATED:
parts.append("Trusted_Connection=yes")
elif settings.auth is SqlAuth.ENTRA:
parts.append("Authentication=ActiveDirectoryDefault")
# AOAG multi-subnet fast failover (BACKLOG #100). Not a TLS keyword, so it goes before the
# Encrypt/TrustServerCertificate tail and cannot disturb the last-wins TLS posture.
# AOAG multi-subnet fast failover (BACKLOG #100). Not a TLS keyword, so it goes in the head,
# before the TLS tail (messagefoundry.odbc_dsn).
if settings.multi_subnet_failover:
parts.append("MultiSubnetFailover=Yes")
# Pin the DB server's certificate by file WITHOUT a machine-wide trust import (#45), via the ODBC
# Driver 18.1+ `ServerCertificate` keyword — only on the SECURE posture (encrypt on, verify on; the
# weakened branch above already refused/escaped). Brace-quoted (STORE-5). The pin goes out with
# Encrypt=strict: on the Driver 18.7 build tested on Windows, the pin was enforced with strict and
# NOT with Encrypt=yes, where a pin of the wrong certificate still connected (vault BACKLOG #2223,
# measured under #3009). Strict is TDS 8.0, so a server older than SQL Server 2022 refuses the
# connection; there is no fall back to an unenforced pin. Microsoft documents the exact match as
# done instead of the expiry, host-name and chain checks, so the engine checks the pinned
# certificate itself at store open (check_pinned_certificate). Not a revocation control. Both
# keywords stay inside the last-wins tail.
pin = settings.sqlserver_pin_path()
if pin:
parts.append(f"ServerCertificate={_odbc_brace(pin)}")
encrypt = "strict" if pin else ("yes" if settings.encrypt else "no")
parts.append(f"Encrypt={encrypt}")
parts.append(f"TrustServerCertificate={'yes' if settings.trust_server_certificate else 'no'}")
return ";".join(parts) + ";"
# Driver 18.1+ `ServerCertificate` keyword, only on the SECURE posture (encrypt on, verify on; the
# weakened branch above already refused or escaped). The pin goes out with Encrypt=strict, and the
# engine checks the pinned certificate itself at store open (check_pinned_certificate). Why strict,
# what it needs of the server, and what it was tested on: the `ssl_root_cert` row of
# docs/CONFIGURATION.md (vault BACKLOG #2223). Not a revocation control.
return odbc_connection_string(
parts,
encrypt=settings.encrypt,
trust_server_certificate=settings.trust_server_certificate,
pin=settings.sqlserver_pin_path(),
)


def _build_pool_executor(settings: StoreSettings, maxsize: int | None = None) -> ThreadPoolExecutor:
Expand Down Expand Up @@ -8953,7 +8942,8 @@ async def purge_search_presets(self, *, older_than: float, now: float | None = N
# ASVS 14.2.7. Whole-row DELETE — the criteria IS the payload and the row backs no count.
# `updated_at`/`last_used_at` are FLOAT epochs here (as on SQLite/PG), so the comparison needs no
# date handling. #306: key on the LATER of last-edited and last-used. T-SQL's GREATEST is SQL
# Server 2022+ and this backend's floor is 2016 SP1 (see _SCHEMA's CREATE OR ALTER guard), so the
# Server 2022+, newer than the 2016 SP1 that _SCHEMA's CREATE OR ALTER guard checks for, so this
# statement does not use it (the supported floor is docs/SYSTEM-REQUIREMENTS.md's). The
# null-safe greatest-of-two is spelled as the equivalent PREDICATE pair — `MAX(a,b) < cutoff` is
# exactly `a < cutoff AND (b IS NULL OR b < cutoff)`, and the NULL arm is what keeps a pre-#306
# row (never recall-stamped) purging on `updated_at` alone. `older_than` is bound TWICE.
Expand Down
Loading
Loading