CHORE: add mock TDS FedAuth regression test + CI install for deferred access-token UAF (#594/#596)#652
Draft
saurabh500 wants to merge 7 commits into
Draft
CHORE: add mock TDS FedAuth regression test + CI install for deferred access-token UAF (#594/#596)#652saurabh500 wants to merge 7 commits into
saurabh500 wants to merge 7 commits into
Conversation
…UAF (#594/#596) Adds tests/test_024_mock_tds_fedauth.py which uses the mssql-mock-tds in-process TDS server to assert that SQL_COPT_SS_ACCESS_TOKEN round-trips byte-for-byte through the ODBC driver to the server's Login7 FedAuth feature. This guards the deferred connect-attribute use-after-free fixed in PR #596: a corrupted deferred buffer would surface as a missing/garbage token captured by the mock, failing the test. Skips cleanly when mssql-mock-tds or cryptography are unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…fedauth test runs (not skips)
Adds a reusable install step (eng/pipelines/steps/install-mock-tds.yml) backed by
install-mock-tds.{sh,ps1}, mirroring the existing install-mssql-py-core pattern.
The step installs mssql-mock-tds (pinned via eng/versions/mssql-mock-tds.version)
plus cryptography from the public mssql-rs_Public PyPI feed, then verifies import.
It is referenced from every test leg (Windows, macOS, and all Linux containers).
The scripts are best-effort: on a platform with no compatible wheel (e.g.
Alpine/musl) or if the sandbox feed is unavailable, they emit an Azure DevOps
pipeline warning and exit 0, so the leg stays green and test_024_mock_tds_fedauth
simply skips there instead of failing the build.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…pine wheels) The new dev build ships musllinux_1_2 wheels (x86_64 + aarch64), so the Alpine test legs now install the mock server and run test_024_mock_tds_fedauth instead of skipping. Updated the stale comments that called out musl as an unsupported platform; the install scripts stay best-effort in case the sandbox feed is ever unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
saurabh500
changed the title
The mock server resolves its TLS identity by trying valid_cert.pem + key.pem first (via create_test_identity, OpenSSL, non-Windows only) and only then falls back to identity.pfx, which its Python binding loads with an EMPTY password. macOS' Security framework rejects a password-less PKCS#12 with 'The user name or passphrase you entered is not correct.', and we can't change the empty password the binding uses -- so the .pfx path can never work on macOS. Fix: write the PEM pair on non-Windows (Linux + macOS), where create_test_identity re-packs them into a 3DES PKCS#12 with a non-empty password that macOS accepts; keep emitting identity.pfx on Windows, where OpenSSL isn't bundled and Schannel loads a password-less PKCS#12 fine. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
pr-size: large
…t test The mock server stores connections in a HashMap keyed by client socket address, so get_last_access_token() (which returns values().last()) reflects arbitrary hash ordering rather than connect order. The Windows SQL2022 leg flaked because 'last' resolved to the first connect's token. Assert instead that both distinct tokens were received byte-for-byte (order independent) and that two connections were recorded. That is the actual property guarding the per-attribute owned buffers from #596 -- two connects must not clobber each other's token. The single-connection tests keep using get_last_access_token(), which is deterministic with one stored entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…test_024 The new mssql-mock-tds build handles the TransactionManager (0x0E) request that Python DB-API drivers send after FedAuth login, so mssql_python now completes the connect instead of hanging on Linux CI (microsoft/mssql-rs#86). The mock records the FedAuth token only when the client connection actually closes. mssql_python auto-enables ODBC connection pooling on the first connect(), so conn.close() returned the socket to the pool and the mock never observed the close -- leaving has_received_token empty and failing all three tests. Add an autouse fixture that disables pooling (a process-global ODBC setting) for the duration of these tests and restores the prior configuration afterwards, so each connection is torn down promptly and its token recorded. Also refresh the now-stale comments that claimed the mock never completes the handshake. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
Adds
tests/test_024_mock_tds_fedauth.py, an integration test that uses themssql-mock-tdsin-process TDS server to guard the deferred connect-attribute use-after-free fixed in #596 (issue #594), and wires the mock server install into the PR-validation pipeline so the test actually runs (instead of skipping) on every test leg.Why
SQL_COPT_SS_ACCESS_TOKEN(1256) is a deferred ODBC connect attribute: the MS ODBC driver stashes the caller's pointer atSQLSetConnectAttrtime and only dereferences it later, duringSQLDriverConnect, to build the FedAuth Login7 packet. PR #568 briefly copied the value into a stack-local buffer that was freed before that deferred read — a use-after-free that variously produced SIGBUS, a server reset, or "Authentication token is missing in the federated authentication message." PR #596 fixed it by storing the value in Connection-owned member buffers.This regression is invisible to ordinary unit tests because it only manifests once a real driver actually transmits the token to a server. The mock TDS server gives us exactly that: it records the access token it received in the Login7 FedAuth feature.
What the test does
For each case it sets
attrs_before={SQL_COPT_SS_ACCESS_TOKEN: token_struct}, lets the real ODBC driver transmit the token to the mock server, and asserts the server captured the exact token. If the deferred buffer is ever corrupted again, the captured token won't match (or won't arrive) and the test fails.test_access_token_round_trips_to_server— direct guard: exact byte-for-byte round-trip.test_unique_access_token_transmitted_exactly— random unguessable token rules out any cached/constant fallback.test_distinct_tokens_on_sequential_connects— two sequential connects with different tokens, exercising the per-attribute owned buffers from FIX: Store deferred connect-attribute values in member buffers (#594) #596.CI integration
Adds a reusable install step (
eng/pipelines/steps/install-mock-tds.yml, backed byinstall-mock-tds.{sh,ps1}and pinned viaeng/versions/mssql-mock-tds.version), mirroring the existinginstall-mssql-py-corepattern. It installsmssql-mock-tds+cryptographyfrom the publicmssql-rs_Publicfeed and is referenced from every test leg (Windows, macOS, and all Linux containers incl. Alpine/musl and ARM64). The scripts are best-effort: if a compatible wheel is ever missing or the sandbox feed is unavailable, they emit a pipeline warning and exit 0, so the leg stays green and the test simply skips there.Notes
mssql-mock-tdsorcryptographyaren't installed, so it won't break CI legs that don't install the mock. The mock package currently lives on themssql-rs_Publicsandbox feed (from draft Rename mock TDS Python package + add sandbox publish pipeline (test-only) mssql-rs#80):Validation
Built the
ddbc_bindingsextension locally from this branch's source and ran the suite: 3 passed.Note
Draft pending the
mssql-mock-tdspackage being published to a stable feed (currently sandbox-only via microsoft/mssql-rs#80).Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com