feat: TDS 8 strict encryption for Microsoft Fabric, Azure SQL, and SQL Server 2025+#420
feat: TDS 8 strict encryption for Microsoft Fabric, Azure SQL, and SQL Server 2025+#420iemejia wants to merge 15 commits into
Conversation
Implement EncryptionLevel::Strict which performs the TLS handshake directly on the TCP stream before any TDS traffic, as required by TDS 8 (Microsoft Fabric Data Warehouse, SQL Server 2022+ strict mode). - Add EncryptionLevel::Strict variant - Add tls_handshake_strict() that bypasses TDS packet wrapping - Support encrypt=strict in ADO.NET and JDBC connection strings - Map Strict to ENCRYPT_ON (0x01) on the wire in PRELOGIN - Make encryption negotiation non-negotiable for strict mode
Complete the TDS 8 strict encryption implementation to support Microsoft Fabric SQL Data Warehouse end-to-end connectivity: - Handle routing reconnection flow (gateway -> backend redirect) - Add FEDAUTHREQUIRED in PRELOGIN for AAD auth state setup - Add AZURESQLSUPPORT (0x08) feature extension in LOGIN - Fix FEATUREEXTACK decoder to handle AZURESQLSUPPORT ack and unknown features gracefully (no more panics) - Fix Packet::encode to use offset-aware length patching - Fix packet_id to start from 1 (not 0) - Add TRACEID support in PRELOGIN for backend connections - Add pipelined PRELOGIN+LOGIN via feed()+flush() for strict mode - Set TDS version to SqlServer2022 for strict mode LOGIN - Set client_lcid=0x0409 and clt_int_name='tiberius' defaults - Add Config fields: strict_pipelined, login_server_name, instance_name - Add ALPN 'ms-tds' for strict TLS connections (native-tls) - Use UseTSQL flag in LOGIN type_flags - Add integration tests proving Fabric connectivity with routing Tested against live Fabric endpoint: connects, authenticates with AAD token, handles routing redirect, and executes queries including parameterized statements.
Key fixes for Azure SQL Database TDS 8 strict mode compatibility: 1. Per-packet TLS record alignment: Changed send() from batch flush (feed all + flush once) to per-packet send (feed+flush each). In TDS 8 strict mode, Azure SQL's gateway requires each TDS packet to arrive as a separate TLS record. Batching multiple TDS packets into a single TLS write caused connection resets. 2. Packet ID incrementing: Per MS-TDS 2.2.3.1, PacketID must increment by 1 (mod 256) for each packet within a multi-packet message. Previously all packets in a message reused the same ID. 3. TDS version in LOGIN7: Changed from 0x08000000 (SqlServer2022) to 0x74000004 (SqlServerN/TDS 7.4). TDS 8 is a transport-mode indicator, not a protocol version — the LOGIN7 version field must still report 7.4. 4. FEDAUTH library type: Using SECURITYTOKEN (0x01) for pre-obtained JWT tokens, not MSAL (0x02). MSAL type has a different FeatureData format without inline token. 5. Removed ALPN "ms-tds": Azure SQL Database gateways reject TLS connections that advertise this ALPN extension. Tested against: - Azure SQL Database (a test Azure SQL Database instance): gateway accepts LOGIN, returns routing redirect, backend accepts LOGIN with FEDAUTH token - Microsoft Fabric: all 5 integration tests pass (pipelined backend path unaffected by send() change) - 120 unit tests pass
- Add tests/azure_sql.rs: 5 integration tests for Azure SQL Database with TDS 8 strict encryption on the gateway + regular TLS-upgrade on the backend worker (the correct pattern for Azure SQL) - Gateway (port 1433): accepts TDS 8 strict (TLS-first) - Backend worker (port 11010): requires regular TLS-upgrade - Tests: connectivity, metadata, multiple queries, DDL/DML, regular encryption - Remove unused FED_AUTH_LIBRARY_MSAL constant - Suppress dead_code warnings on hostname/client_pid/client_prog_ver setters Key finding: Azure SQL backend workers do NOT support TDS 8 strict mode. After routing redirect, clients must reconnect with EncryptionLevel::Required (standard TLS-upgrade flow), not EncryptionLevel::Strict.
7 integration tests covering: - Basic strict mode connection and query - Server metadata verification (version, database, transport) - Multiple sequential queries over a single connection - DDL/DML operations (CREATE TABLE, INSERT, UPDATE, DELETE) - Encryption verification via sys.dm_exec_connections - Large result set (1000 rows) to stress TLS framing - CA certificate validation (non-trust-all path) Tests run against SQL Server 2025 with network.forcestrict=1 (strict mode is only available in SQL Server 2025+, not 2022 on Linux). Tests skip gracefully when SQL_SERVER_PASSWORD env var is not set.
…d Azure SQL Brings Fabric and Azure SQL integration tests to parity with SQL Server: - DDL/DML: CREATE TABLE, INSERT, UPDATE, DELETE, SELECT with params - Large result set: 1000 rows to stress TLS framing across packets - Encryption verification: CONNECTIONPROPERTY + sys.dm_exec_connections - String/unicode: CONCAT, unicode chars, 8000-char strings Fabric-specific considerations: - Uses permanent table (not #temp) since Fabric DW doesn't support local temps - Large result tries tpch_sf1.lineitem first, falls back to recursive CTE - dm_exec_connections access is optional (may not be available on all SKUs) Total integration test count: 24 (9 Fabric + 8 Azure SQL + 7 SQL Server)
- Use VARCHAR(100) instead of NVARCHAR(100) — Fabric DW does not support nvarchar with specified length - Replace recursive CTE with OPTION(MAXRECURSION) by cross-join row generation — Fabric DW disallows query hints - Skip DDL test gracefully when identity lacks CREATE TABLE permission (e.g., read-only lakehouse endpoints)
- Apply cargo fmt across all changed files - Remove dead debug TDS frame dump to /tmp in feed_to_wire() - Remove unnecessary #[allow(dead_code)] on feed_to_wire (it's used) - Remove unused lifetime 'a on login() function - Use eq_ignore_ascii_case() instead of to_ascii_lowercase() comparisons in TLS cert extension matching
Add async TokenProvider trait enabling dynamic token acquisition on each connection/reconnection, replacing the need for static pre-obtained tokens in long-lived applications. Changes: - Define TokenProvider trait with async get_token() method (uses async-trait) - Add AuthMethod::AADTokenProvider(Arc<dyn TokenProvider>) variant - Add AuthMethod::token_provider() constructor and is_aad() helper - Manual Clone/Debug/PartialEq/Eq impls for AuthMethod (Arc<dyn> breaks derives) - Resolve token via provider in both pipelined and standard login paths - Export TokenProvider from crate root - Integration test verifying provider is called twice (gateway + backend routing)
When the caller does not explicitly set the encryption level, the client now automatically upgrades to Strict mode for known endpoints: - *.datawarehouse.fabric.microsoft.com (Fabric SQL endpoints) - *.pbidedicated.windows.net (Fabric backend servers) This means users connecting to Fabric no longer need to specify encrypt=strict in connection strings — the client detects it from the hostname. Explicit encryption settings are always respected. Implementation: - Add encryption_explicit flag to Config (tracks user intent) - Add has_encrypt_key() to ConfigString trait (detects conn string key) - Add resolve_encryption() method with hostname heuristic - Call resolve_encryption() at start of Connection::connect() - Emit tracing::info event when auto-detection fires - 9 unit tests for the detection heuristic - Integration test verifying end-to-end auto-detect with Fabric
Wrap Tls and Io errors from the strict handshake with contextual guidance: the target hostname, server requirements (SQL Server 2025+ with forcestrict=1, Azure SQL, or Fabric), and the suggested fix (encrypt=true instead of encrypt=strict). Add 3 unit tests for the wrapping logic and 1 integration test that verifies the message against a non-strict TCP endpoint.
- Client::connection_encryption() returns the negotiated EncryptionLevel, allowing pool implementations to verify security properties of managed connections (e.g., assert all pooled connections use Strict). - Client::is_healthy() executes a lightweight SELECT 1 round-trip, suitable for pool is_valid / health-check hooks. - Store EncryptionLevel in Connection struct, set to the final negotiated value (Strict for TDS 8, or the PRELOGIN-negotiated level otherwise). - Integration tests verify both methods on live strict connections.
…osoft.com) Verify tiberius connects to SQL Database in Microsoft Fabric, which uses the same gateway/backend architecture as Azure SQL: - Gateway: supports TDS 8 strict TLS on *.database.fabric.microsoft.com - Backend: routes to *.worker.database.windows.net with regular TLS upgrade Key finding: Fabric SQL DB does NOT require strict mode (unlike Data Warehouse). Both encrypt=strict and encrypt=true work. No auto-detect upgrade needed for this endpoint pattern. 7 integration tests: strict, required, metadata, DDL/DML, health check, connection_encryption, and works-without-strict verification.
- cargo fmt: fix line-length formatting in unit tests and integration tests - Fix resolve_encryption() doc comment to list both auto-detect patterns (*.datawarehouse.fabric.microsoft.com and *.pbidedicated.windows.net) - Remove unused `resolved` variable binding in connect() — the return value was redundant with config.encryption after mutation - Rename misleading `is_backend` to `include_trace_id` in finish_connect_after_tls() to match the parameter it maps to
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR introduces TDS 8.0 strict encryption support, enabling TLS-before-prelogin handshakes for cloud endpoints and dynamic AAD token refresh. Core changes include a new ChangesTDS 8 Strict Encryption Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/tls_stream/native_tls_stream.rs`:
- Around line 53-59: The TrustAll branch explicitly disables SNI via
native_builder.use_sni(false) while the strict branch does not, causing
divergent SNI behavior; update the strict-path code around TrustConfig (the
branch handling non-TrustAll vs TrustConfig::TrustAll and the native_builder.*
calls such as native_builder.danger_accept_invalid_certs and
native_builder.use_sni) to either (a) match the non-strict branch by disabling
SNI where appropriate, or (b) add a concise inline comment in the strict branch
explaining why SNI must remain enabled for strict mode (e.g., required for cloud
endpoint routing), so the divergence is intentional and documented.
- Around line 66-67: The current conversion native_builder.into() won't compile;
instead call native_builder.build() to produce a native_tls::TlsConnector, then
wrap/convert that into the async connector before using connect — e.g. build the
native connector with native_builder.build(), convert/wrap it into an
async_native_tls connector (via async_native_tls::TlsConnector::from(...) or the
async-native-tls builder API), and then call
connector.connect(config.get_host(), stream).await?; reference native_builder,
native_builder.build(), async_native_tls::TlsConnector::from, connector.connect,
and config.get_host() to find the exact lines to change.
In `@src/tds/codec/token/token_feature_ext_ack.rs`:
- Around line 62-89: The code reads a wire-controlled data_len and allocates
Vec<u8> without checks in the FeatureEXTACK parsing (branches that push
FeatureAck::AzureSqlSupport, ::ColumnEncryption, ::Utf8Support, and ::Unknown);
add a safe upper bound check (e.g. compare data_len against a
MAX_FEATURE_PAYLOAD constant like 1_048_576 or another protocol-appropriate
limit) before allocating or reading, return an error (or io::Error::new with
InvalidData) if data_len exceeds the limit, and prefer using Vec::try_reserve or
read into a limited buffer to avoid unbounded allocations in the parsing
function that builds the features vector and pushes FeatureAck variants.
In `@tests/fabric_sqldb.rs`:
- Around line 40-50: The helper functions get_endpoint, get_database, and
get_token currently call env::var(...).unwrap() which can panic with unhelpful
messages; change them to return Result<String, anyhow::Error> (or Result<String,
env::VarError>) with descriptive error text, e.g. "missing
FABRIC_SQLDB_DATABASE" / "missing FABRIC_SQLDB_TOKEN", or alternatively expand
the existing skip macro to check all three env vars before running tests; update
call sites in tests to propagate or unwrap the Result with context (following
the pattern in fabric.rs's get_aad_token()) so missing env vars produce clear,
consistent errors instead of panics.
In `@tests/sql_server_tds8.rs`:
- Around line 442-446: The test assertion in tests/sql_server_tds8.rs currently
expects the exact substring "encrypt=true" in err_msg which is brittle; update
the assertion to check for a broader marker such as "encrypt=" so minor wording
changes (e.g., "encrypt=true" -> "encrypt: true") won't break the test—locate
the assert block referencing err_msg and replace the contains("encrypt=true")
check with contains("encrypt=") (still using the same err_msg variable and
assert! macro).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b3d9868d-2384-4d8b-bdf0-0471cc535948
📒 Files selected for processing (21)
Cargo.tomlsrc/client.rssrc/client/auth.rssrc/client/config.rssrc/client/config/ado_net.rssrc/client/config/jdbc.rssrc/client/connection.rssrc/client/tls_stream/native_tls_stream.rssrc/lib.rssrc/tds.rssrc/tds/codec/header.rssrc/tds/codec/login.rssrc/tds/codec/packet.rssrc/tds/codec/pre_login.rssrc/tds/codec/token/token_error.rssrc/tds/codec/token/token_feature_ext_ack.rssrc/tds/context.rstests/azure_sql.rstests/fabric.rstests/fabric_sqldb.rstests/sql_server_tds8.rs
| let connector: TlsConnector = native_builder.into(); | ||
| return Ok(connector.connect(config.get_host(), stream).await?); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does async-native-tls 0.4 support From<native_tls::TlsConnectorBuilder> for TlsConnector?
💡 Result:
Yes. async-native-tls 0.4 supports converting from its own TlsConnectorBuilder type via From for TlsConnector; the TlsConnector docs show impl From for TlsConnector with fn from(builder: TlsConnectorBuilder) -> Self (i.e., TlsConnector::from(builder)). [1] However, the docs page we found does not indicate a blanket From<native_tls::TlsConnectorBuilder> implementation. It only documents From<TlsConnectorBuilder> (async-native-tls’s builder), not native_tls::TlsConnectorBuilder. [1] Conclusion: For async-native-tls 0.4, you can rely on From<async_native_tls::TlsConnectorBuilder> for async_native_tls::TlsConnector, but support for From<native_tls::TlsConnectorBuilder> is not evidenced by the authoritative docs result we located. [1]
Citations:
This conversion will not compile.
Line 66 attempts to convert native_tls_crate::TlsConnectorBuilder directly to async_native_tls::TlsConnector using .into(). According to async-native-tls 0.4 documentation, the From trait is only implemented for async_native_tls::TlsConnectorBuilder, not for native_tls::TlsConnectorBuilder.
The native-tls builder must first be built into a native_tls::TlsConnector via .build(), then wrapped appropriately for async use. Alternatively, use async-native-tls's own builder API throughout the strict mode path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/tls_stream/native_tls_stream.rs` around lines 66 - 67, The current
conversion native_builder.into() won't compile; instead call
native_builder.build() to produce a native_tls::TlsConnector, then wrap/convert
that into the async connector before using connect — e.g. build the native
connector with native_builder.build(), convert/wrap it into an async_native_tls
connector (via async_native_tls::TlsConnector::from(...) or the async-native-tls
builder API), and then call connector.connect(config.get_host(), stream).await?;
reference native_builder, native_builder.build(),
async_native_tls::TlsConnector::from, connector.connect, and config.get_host()
to find the exact lines to change.
There was a problem hiding this comment.
This is a false positive. async-native-tls 0.4.0 does implement From<native_tls::TlsConnectorBuilder> for TlsConnector — verified in the crate source at async-native-tls-0.4.0/src/lib.rs. The code compiles and all 168 tests pass against live SQL Server, Azure SQL, and Fabric endpoints. No change needed.
There was a problem hiding this comment.
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set chat.allow_non_org_members: true in your configuration.
| assert!( | ||
| err_msg.contains("encrypt=true"), | ||
| "Error message should suggest alternative, got: {}", | ||
| err_msg | ||
| ); |
There was a problem hiding this comment.
Loosen the strict TLS guidance assertion to avoid brittle failures.
Line 443 hard-codes encrypt=true; benign wording updates in the error text can break this test despite correct behavior. Assert on a broader guidance marker (e.g., encrypt=) instead.
Suggested change
assert!(
- err_msg.contains("encrypt=true"),
- "Error message should suggest alternative, got: {}",
+ err_msg.contains("encrypt="),
+ "Error message should suggest an encrypt setting alternative, got: {}",
err_msg
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert!( | |
| err_msg.contains("encrypt=true"), | |
| "Error message should suggest alternative, got: {}", | |
| err_msg | |
| ); | |
| assert!( | |
| err_msg.contains("encrypt="), | |
| "Error message should suggest an encrypt setting alternative, got: {}", | |
| err_msg | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/sql_server_tds8.rs` around lines 442 - 446, The test assertion in
tests/sql_server_tds8.rs currently expects the exact substring "encrypt=true" in
err_msg which is brittle; update the assertion to check for a broader marker
such as "encrypt=" so minor wording changes (e.g., "encrypt=true" -> "encrypt:
true") won't break the test—locate the assert block referencing err_msg and
replace the contains("encrypt=true") check with contains("encrypt=") (still
using the same err_msg variable and assert! macro).
There was a problem hiding this comment.
Intentionally kept as-is. The assertion tests our specific error message wording — we control that message text in wrap_strict_tls_error() and it intentionally says encrypt=true (the exact connection string parameter users need). If we ever change the guidance text, we want this test to fail so we consciously update it. Loosening to encrypt= would mask unintentional regressions in user-facing error messages.
There was a problem hiding this comment.
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set chat.allow_non_org_members: true in your configuration.
- Add size guard (1 MiB max) on FEATUREEXTACK payload allocations to prevent malformed/malicious packets from causing large memory allocs. Extract read_feature_data() helper to deduplicate the pattern. - Replace panic!() in FEDAUTH data_len handling with proper Error::Protocol - Add comment explaining why SNI stays enabled in strict TrustAll path (cloud endpoints need SNI for tenant routing) - Use expect() with descriptive messages instead of unwrap() in fabric_sqldb.rs test helpers for clearer failure diagnostics
Addressing CodeRabbit review commentsPushed commit 1. SNI handling in strict TrustAll (nitpick) — FixedAdded a comment explaining that SNI must remain enabled in strict mode because cloud endpoints (Azure SQL, Fabric) use SNI to route the TLS connection to the correct tenant/gateway, even in trust-all mode. The non-strict 2.
|
1 participant
Summary
This PR implements TDS 8 strict encryption (
encrypt=strict), enabling tiberius to connect to:*.datawarehouse.fabric.microsoft.com)*.database.fabric.microsoft.com)*.database.windows.net)forcestrict=1Fixes #412
What is TDS 8 Strict Mode?
TDS 8 introduces a "strict" transport mode where the TLS handshake happens before any TDS traffic (TLS-first), as opposed to the traditional flow where PRELOGIN is sent in cleartext and TLS is negotiated after. This provides stronger security guarantees and is required by Microsoft Fabric Data Warehouse endpoints.
Changes
Core Implementation
EncryptionLevel::Strictvariant — performs TLS-first handshake directly on the TCP stream before any TDS packetsAuthentication
TokenProvidertrait —async fn get_token()for dynamic AAD/Entra ID token refresh on each connection/reconnection, supporting long-lived applications where tokens expireAuthMethod::AADTokenProvider(Arc<dyn TokenProvider>)— stores the provider with manualClone/Debug/PartialEq/EqimplsAuto-Detection
*.datawarehouse.fabric.microsoft.comor*.pbidedicated.windows.netautomatically upgrade to strict modeConnection Pooling Support
Client::connection_encryption()— returns the negotiatedEncryptionLevelfor pool security verificationClient::is_healthy()— executesSELECT 1for pool health-check hooksError Handling
encrypt=truefallback suggestion)Configuration
encrypt=strictsupported in both ADO.NET and JDBC connection string formatsConfig::strict_pipelined()— enables pipelined mode for backend reconnection after routingConfig::login_server_name()— sets the original gateway hostname in LOGIN for routed connectionsIntegration Tests
36 integration tests across 4 target environments (all skip gracefully when env vars are not set):
forcestrict=1)Architecture Findings
*.datawarehouse.fabric.microsoft.com(strict)*.pbidedicated.windows.net(strict)*.database.fabric.microsoft.com(strict OK)*.worker.database.windows.net(regular)*.database.windows.net(strict OK)*.worker.database.windows.net(regular)Security Design
ms-tdsALPN extensionTesting Locally