Skip to content
6 changes: 4 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ winauth = { version = "0.0.4", optional = true }

[target.'cfg(unix)'.dependencies]
libgssapi = { version = "0.8.1", optional = true, default-features = false }
libc = "0.2"

[dependencies.async-native-tls]
version = "0.4"
version = "0.5"
features = ["runtime-async-std"]
optional = true

Expand Down Expand Up @@ -191,7 +192,8 @@ all = [
"bigdecimal",
"native-tls",
]
default = ["tds73", "winauth", "native-tls"]
default = ["tds80", "winauth", "native-tls"]
tds80 = ["tds73"]
Comment on lines +195 to +196
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Document the breaking change in default features.

Changing the default feature set from tds73 to tds80 is a breaking change for downstream users. Users who relied on the previous defaults will now automatically receive TDS 8.0 behavior, including:

  • New EncryptionLevel::Strict support (requires TLS handshake with ALPN)
  • TDS 8.0 protocol capabilities
  • Different connection behavior when using defaults

Users who need the previous behavior must explicitly opt out of default features and select tds73 in their Cargo.toml:

[dependencies]
tiberius = { version = "0.12", default-features = false, features = ["tds73", "winauth", "native-tls"] }

This change should be prominently documented in the CHANGELOG/release notes with migration guidance.

Note: Since tds80 = ["tds73"], the tds80 feature is additive and preserves tds73 behavior while enabling additional functionality. However, the new code paths (Strict encryption, ALPN negotiation) may introduce behavioral differences that users should be aware of.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` around lines 195 - 196, The change to the Cargo.toml default
features now enabling tds80 (default = ["tds80", "winauth", "native-tls"]) is a
breaking change for downstream users; update the CHANGELOG/release notes to
clearly document the migration: explain that tds80 is now default, that tds80
includes tds73 but introduces new behavior (e.g., EncryptionLevel::Strict,
ALPN/TLS handshake differences and TDS 8.0 protocol capabilities), provide the
exact opt-out example using default-features = false and features = ["tds73",
"winauth", "native-tls"], and add a short guidance section listing likely
behavioral differences and how to test/restore previous behavior.

tds73 = []
docs = []
sql-browser-async-std = ["async-std"]
Expand Down
56 changes: 53 additions & 3 deletions src/client/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub struct Config {
pub(crate) trust: TrustConfig,
pub(crate) auth: AuthMethod,
pub(crate) readonly: bool,
pub(crate) hostname_in_certificate: Option<String>,
pub(crate) client_name: Option<String>,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -65,6 +67,8 @@ impl Default for Config {
trust: TrustConfig::Default,
auth: AuthMethod::None,
readonly: false,
hostname_in_certificate: None,
client_name: None,
}
}
}
Expand Down Expand Up @@ -149,14 +153,29 @@ impl Config {
/// Will panic in case `trust_cert` was called before.
///
/// - Defaults to validating the server certificate is validated against system's certificate storage.
pub fn trust_cert_ca(&mut self, path: impl ToString) {
pub fn trust_cert_ca(&mut self, path: impl Into<PathBuf>) {
if let TrustConfig::TrustAll = &self.trust {
panic!("'trust_cert' and 'trust_cert_ca' are mutual exclusive! Only use one.")
} else {
self.trust = TrustConfig::CaCertificateLocation(PathBuf::from(path.to_string()))
self.trust = TrustConfig::CaCertificateLocation(path.into())
}
}

/// Sets the hostname to be used for certificate validation.
/// If not set, the hostname from `host` will be used.
///
/// - Defaults to the value of `host`.
pub fn hostname_in_certificate(&mut self, hostname: impl ToString) {
self.hostname_in_certificate = Some(hostname.to_string());
}

/// Sets the client name to be sent to the server.
///
/// - Defaults to the current workstation id (hostname).
pub fn client_name(&mut self, name: impl ToString) {
self.client_name = Some(name.to_string());
}

/// Sets the authentication method.
///
/// - Defaults to `None`.
Expand Down Expand Up @@ -190,6 +209,12 @@ impl Config {
}
}

pub(crate) fn get_hostname_in_certificate(&self) -> &str {
self.hostname_in_certificate
.as_deref()
.unwrap_or_else(|| self.get_host())
}

/// Get the host address including port
pub fn get_addr(&self) -> String {
format!("{}:{}", self.get_host(), self.get_port())
Expand All @@ -210,7 +235,7 @@ impl Config {
/// |`database`|`<string>`|The name of the database.|
/// |`TrustServerCertificate`|`true`,`false`,`yes`,`no`|Specifies whether the driver trusts the server certificate when connecting using TLS. Cannot be used toghether with `TrustServerCertificateCA`|
/// |`TrustServerCertificateCA`|`<path>`|Path to a `pem`, `crt` or `der` certificate file. Cannot be used together with `TrustServerCertificate`|
/// |`encrypt`|`true`,`false`,`yes`,`no`,`DANGER_PLAINTEXT`|Specifies whether the driver uses TLS to encrypt communication.|
/// |`encrypt`|`strict`,`true`,`false`,`yes`,`no`,`DANGER_PLAINTEXT`|Specifies whether the driver uses TLS to encrypt communication.|
/// |`Application Name`, `ApplicationName`|`<string>`|Sets the application name for the connection.|
///
/// [ADO.NET connection string]: https://docs.microsoft.com/en-us/dotnet/framework/data/adonet/connection-strings
Expand Down Expand Up @@ -265,10 +290,18 @@ impl Config {
builder.trust_cert_ca(ca);
}

if let Some(hostname_in_cert) = s.hostname_in_certificate() {
builder.hostname_in_certificate(hostname_in_cert);
}

builder.encryption(s.encrypt()?);

builder.readonly(s.readonly());

if let Some(client_name) = s.client_name() {
builder.client_name(client_name);
}

Ok(builder)
}
}
Expand Down Expand Up @@ -346,6 +379,20 @@ pub(crate) trait ConfigString {
.map(|ca| ca.to_string())
}

fn hostname_in_certificate(&self) -> Option<String> {
self.dict()
.get("hostnameincertificate")
.or_else(|| self.dict().get("hostname in certificate"))
.map(|ca| ca.to_string())
}

fn client_name(&self) -> Option<String> {
self.dict()
.get("workstationid")
.or_else(|| self.dict().get("workstation id"))
.map(|name| name.to_string())
}

#[cfg(any(
feature = "rustls",
feature = "native-tls",
Expand All @@ -358,6 +405,9 @@ pub(crate) trait ConfigString {
Ok(true) => Ok(EncryptionLevel::Required),
Ok(false) => Ok(EncryptionLevel::Off),
Err(_) if val == "DANGER_PLAINTEXT" => Ok(EncryptionLevel::NotSupported),
Err(_) if val.eq_ignore_ascii_case("strict") && cfg!(feature = "tds80") => {
Ok(EncryptionLevel::Strict)
}
Comment on lines +408 to +410
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider improving error message when tds80 feature is disabled.

When encrypt=strict is specified but the tds80 feature is not enabled, the code falls through to return a generic "Not a valid boolean" error from parse_bool. This may confuse users who expect strict to work.

♻️ Proposed improvement for clearer error handling
                 Err(_) if val == "DANGER_PLAINTEXT" => Ok(EncryptionLevel::NotSupported),
-                Err(_) if val.eq_ignore_ascii_case("strict") && cfg!(feature = "tds80") => {
-                    Ok(EncryptionLevel::Strict)
-                }
+                Err(_) if val.eq_ignore_ascii_case("strict") => {
+                    #[cfg(feature = "tds80")]
+                    {
+                        Ok(EncryptionLevel::Strict)
+                    }
+                    #[cfg(not(feature = "tds80"))]
+                    {
+                        Err(crate::Error::Conversion(
+                            "encrypt=strict requires the 'tds80' feature to be enabled".into(),
+                        ))
+                    }
+                }
                 Err(e) => Err(e),
📝 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.

Suggested change
Err(_) if val.eq_ignore_ascii_case("strict") && cfg!(feature = "tds80") => {
Ok(EncryptionLevel::Strict)
}
Err(_) if val.eq_ignore_ascii_case("strict") => {
#[cfg(feature = "tds80")]
{
Ok(EncryptionLevel::Strict)
}
#[cfg(not(feature = "tds80"))]
{
Err(crate::Error::Conversion(
"encrypt=strict requires the 'tds80' feature to be enabled".into(),
))
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/config.rs` around lines 408 - 410, The parse path silently falls
through to parse_bool and yields a confusing "Not a valid boolean" when the user
supplies "strict" but the crate was compiled without the tds80 feature; update
the parsing logic (the FromStr/parse function that branches to
EncryptionLevel::Strict and calls parse_bool) to explicitly check for
val.eq_ignore_ascii_case("strict") when cfg!(not(feature = "tds80")) and return
a clear Err variant/message indicating that "strict" requires the tds80 feature
(or suggest enabling that feature), so users see a meaningful error instead of
the generic parse_bool failure.

Err(e) => Err(e),
})
.unwrap_or(Ok(EncryptionLevel::Off))
Expand Down
51 changes: 51 additions & 0 deletions src/client/config/ado_net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,21 @@ mod tests {
Ok(())
}

#[test]
#[cfg(any(
feature = "rustls",
feature = "native-tls",
feature = "vendored-openssl"
))]
fn encryption_parsing_strict() -> crate::Result<()> {
let test_str = "encrypt=strict";
let ado: AdoNetConfig = test_str.parse()?;

assert_eq!(EncryptionLevel::Strict, ado.encrypt()?);

Ok(())
}

#[test]
fn application_name_parsing() -> crate::Result<()> {
let test_str = "Application Name=meow";
Expand All @@ -484,4 +499,40 @@ mod tests {

Ok(())
}

#[test]
fn client_name_parsing() -> crate::Result<()> {
let test_str = "workstationid=meow";
let ado: AdoNetConfig = test_str.parse()?;

assert_eq!(Some("meow".into()), ado.client_name());

let test_str = "Workstation ID=meow";
let ado: AdoNetConfig = test_str.parse()?;

assert_eq!(Some("meow".into()), ado.client_name());

Ok(())
}

#[test]
fn hostname_in_certificate_parsing() -> crate::Result<()> {
let test_str = "HostNameInCertificate=foo.example.com";
let ado: AdoNetConfig = test_str.parse()?;

assert_eq!(
Some("foo.example.com".into()),
ado.hostname_in_certificate()
);

let test_str = "HostName In Certificate=foo.example.com";
let ado: AdoNetConfig = test_str.parse()?;

assert_eq!(
Some("foo.example.com".into()),
ado.hostname_in_certificate()
);

Ok(())
}
}
113 changes: 61 additions & 52 deletions src/client/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,17 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> Connection<S> {
context
};

let transport = Framed::new(MaybeTlsStream::Raw(tcp_stream), PacketCodec);
let transport = match config.encryption {
EncryptionLevel::Strict => {
event!(Level::INFO, "Performing a TLS handshake");
let mut pre_login_stream = TlsPreloginWrapper::new(tcp_stream);
pre_login_stream.handshake_complete();
let stream = create_tls_stream(&config, pre_login_stream).await?;
event!(Level::INFO, "TLS handshake successful");
Framed::new(MaybeTlsStream::Tls(stream), PacketCodec)
}
_ => Framed::new(MaybeTlsStream::Raw(tcp_stream), PacketCodec),
};

let mut connection = Self {
transport,
Expand All @@ -98,17 +108,7 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> Connection<S> {

let connection = connection.tls_handshake(&config, encryption).await?;

let mut connection = connection
.login(
config.auth,
encryption,
config.database,
config.host,
config.application_name,
config.readonly,
prelogin,
)
.await?;
let mut connection = connection.login(config, encryption, prelogin).await?;

connection.flush_done().await?;

Expand Down Expand Up @@ -284,34 +284,33 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> Connection<S> {

/// Defines the login record rules with SQL Server. Authentication with
/// connection options.
#[allow(clippy::too_many_arguments)]
async fn login<'a>(
async fn login(
mut self,
auth: AuthMethod,
config: Config,
encryption: EncryptionLevel,
db: Option<String>,
server_name: Option<String>,
application_name: Option<String>,
readonly: bool,
prelogin: PreloginMessage,
) -> crate::Result<Self> {
let mut login_message = LoginMessage::new();

if let Some(db) = db {
if let Some(db) = config.database {
login_message.db_name(db);
}

if let Some(server_name) = server_name {
if let Some(server_name) = config.host {
login_message.server_name(server_name);
}

if let Some(app_name) = application_name {
if let Some(app_name) = config.application_name {
login_message.app_name(app_name);
}

login_message.readonly(readonly);
if let Some(client_name) = config.client_name {
login_message.hostname(client_name);
}

login_message.readonly(config.readonly);

match auth {
match config.auth {
#[cfg(all(windows, feature = "winauth"))]
AuthMethod::Integrated => {
let mut client = NtlmSspiBuilder::new()
Expand Down Expand Up @@ -444,37 +443,47 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> Connection<S> {
config: &Config,
encryption: EncryptionLevel,
) -> crate::Result<Self> {
if encryption != EncryptionLevel::NotSupported {
event!(Level::INFO, "Performing a TLS handshake");

let Self {
transport, context, ..
} = self;
let mut stream = match transport.into_inner() {
MaybeTlsStream::Raw(tcp) => {
create_tls_stream(config, TlsPreloginWrapper::new(tcp)).await?
}
_ => unreachable!(),
};

stream.get_mut().handshake_complete();
event!(Level::INFO, "TLS handshake successful");
match encryption {
EncryptionLevel::NotSupported => {
event!(
Level::WARN,
"TLS encryption is not enabled. All traffic including the login credentials are not encrypted."
);
Ok(self)
}
EncryptionLevel::Strict => {
// In Strict mode, we should already be in TLS stream after prelogin, so just return self.
event!(
Level::TRACE,
"Already in TLS stream due to Strict encryption level, skipping handshake."
);
Ok(self)
}
EncryptionLevel::Off | EncryptionLevel::On | EncryptionLevel::Required => {
event!(Level::INFO, "Performing a TLS handshake");

let Self {
transport, context, ..
} = self;
let mut stream = match transport.into_inner() {
MaybeTlsStream::Raw(tcp) => {
create_tls_stream(config, TlsPreloginWrapper::new(tcp)).await?
}
_ => unreachable!(),
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

let transport = Framed::new(MaybeTlsStream::Tls(stream), PacketCodec);
stream.get_mut().handshake_complete();
event!(Level::INFO, "TLS handshake successful");

Ok(Self {
transport,
context,
flushed: false,
buf: BytesMut::new(),
})
} else {
event!(
Level::WARN,
"TLS encryption is not enabled. All traffic including the login credentials are not encrypted."
);
let transport = Framed::new(MaybeTlsStream::Tls(stream), PacketCodec);

Ok(self)
Ok(Self {
transport,
context,
flushed: false,
buf: BytesMut::new(),
})
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/client/tls_stream.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::Config;
use futures_util::io::{AsyncRead, AsyncWrite};

pub(crate) const TDS_ALPN_PROTOCOL_NAME: &str = "tds/8.0";

#[cfg(feature = "native-tls")]
mod native_tls_stream;

Expand Down
Loading