Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ winauth = { version = "0.0.4", optional = true }

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

[dependencies.async-native-tls]
version = "0.4"
Expand Down Expand Up @@ -202,3 +203,4 @@ bigdecimal = ["bigdecimal_"]
rustls = ["tokio-rustls", "tokio-util", "rustls-pemfile", "rustls-native-certs"]
native-tls = ["async-native-tls"]
vendored-openssl = ["opentls"]
sspi-rs = ["sspi"]
25 changes: 17 additions & 8 deletions src/client/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,22 @@ impl Debug for SqlServerAuth {
}

#[derive(Clone, PartialEq, Eq)]
#[cfg(any(all(windows, feature = "winauth"), doc))]
#[cfg_attr(feature = "docs", doc(all(windows, feature = "winauth")))]
#[cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"), doc))]
#[cfg_attr(
feature = "docs",
doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs")))
)]
Comment on lines +30 to +33
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 | 🟡 Minor

Incorrect doc attribute syntax — missing cfg() wrapper.

The doc(any(...)) syntax is invalid. It should be doc(cfg(any(...))) to properly display the "Available on..." badge in rustdoc. Compare with the existing Integrated variant at line 76 which correctly uses doc(cfg(any(...))).

This same issue appears at lines 41-44, 62-65, and 97.

Proposed fix for all occurrences
 #[cfg_attr(
     feature = "docs",
-    doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs")))
+    doc(cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"))))
 )]

Apply the same pattern to lines 41-44, 62-65, and fix line 97:

-    #[cfg_attr(feature = "docs", doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"))))]
+    #[cfg_attr(
+        feature = "docs",
+        doc(cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"))))
+    )]
📝 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
#[cfg_attr(
feature = "docs",
doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs")))
)]
#[cfg_attr(
feature = "docs",
doc(cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"))))
)]

pub struct WindowsAuth {
pub(crate) user: String,
pub(crate) password: String,
pub(crate) domain: Option<String>,
}

#[cfg(any(all(windows, feature = "winauth"), doc))]
#[cfg_attr(feature = "docs", doc(all(windows, feature = "winauth")))]
#[cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"), doc))]
#[cfg_attr(
feature = "docs",
doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs")))
)]
impl Debug for WindowsAuth {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("WindowsAuth")
Expand All @@ -52,8 +58,11 @@ pub enum AuthMethod {
/// Authenticate directly with SQL Server.
SqlServer(SqlServerAuth),
/// Authenticate with Windows credentials.
#[cfg(any(all(windows, feature = "winauth"), doc))]
#[cfg_attr(feature = "docs", doc(cfg(all(windows, feature = "winauth"))))]
#[cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"), doc))]
#[cfg_attr(
feature = "docs",
doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs")))
)]
Windows(WindowsAuth),
/// Authenticate as the currently logged in user. On Windows uses SSPI and
/// Kerberos on Unix platforms.
Expand Down Expand Up @@ -84,8 +93,8 @@ impl AuthMethod {
}

/// Construct a new Windows authentication configuration.
#[cfg(any(all(windows, feature = "winauth"), doc))]
#[cfg_attr(feature = "docs", doc(cfg(all(windows, feature = "winauth"))))]
#[cfg(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"), doc))]
#[cfg_attr(feature = "docs", doc(any(all(windows, feature = "winauth"), all(unix, feature = "sspi-rs"))))]
pub fn windows(user: impl AsRef<str>, password: impl ToString) -> Self {
let (domain, user) = match user.as_ref().find('\\') {
Some(idx) => (Some(&user.as_ref()[..idx]), &user.as_ref()[idx + 1..]),
Expand Down
2 changes: 2 additions & 0 deletions src/client/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ pub(crate) trait ConfigString {
Some(val) if val.to_lowercase() == "sspi" || Self::parse_bool(val)? => {
Ok(AuthMethod::Integrated)
}

// Should sspi-rs take over the default behaviour here if enabled?
_ => Ok(AuthMethod::sql_server(user.unwrap_or(""), pw.unwrap_or(""))),
}
}
Expand Down
87 changes: 85 additions & 2 deletions src/client/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
};
use asynchronous_codec::Framed;
use bytes::BytesMut;
#[cfg(any(windows, feature = "integrated-auth-gssapi"))]
#[cfg(any(windows, feature = "integrated-auth-gssapi", feature = "sspi-rs"))]
use codec::TokenSspi;
use futures_util::io::{AsyncRead, AsyncWrite};
use futures_util::ready;
Expand All @@ -40,6 +40,12 @@ use tracing::{event, Level};
#[cfg(all(windows, feature = "winauth"))]
use winauth::{windows::NtlmSspiBuilder, NextBytes};

#[cfg(all(unix, feature = "sspi-rs"))]
use sspi::{
builders::EmptyInitializeSecurityContext, AuthIdentity, BufferType, ClientRequestFlags,
CredentialUse, DataRepresentation, Ntlm, SecurityBuffer, Sspi, SspiImpl, Username,
};

/// A `Connection` is an abstraction between the [`Client`] and the server. It
/// can be used as a `Stream` to fetch [`Packet`]s from and to `send` packets
/// splitting them to the negotiated limit automatically.
Expand Down Expand Up @@ -120,7 +126,7 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> Connection<S> {
TokenStream::new(self).flush_done().await
}

#[cfg(any(windows, feature = "integrated-auth-gssapi"))]
#[cfg(any(windows, feature = "integrated-auth-gssapi", feature = "sspi-rs"))]
/// Flush the incoming token stream until receiving `SSPI` token.
async fn flush_sspi(&mut self) -> crate::Result<TokenSspi> {
TokenStream::new(self).flush_sspi().await
Expand Down Expand Up @@ -381,6 +387,83 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> Connection<S> {

self.send(header, next_token).await?;
}

#[cfg(all(unix, feature = "sspi-rs", not(all(windows, feature = "winauth"))))]
AuthMethod::Windows(auth) => {
let mut ntlm = Ntlm::new();

let username = Username::new(&auth.user, auth.domain.as_deref())
.map_err(|e| sspi::Error::from(e))?;

let identity = AuthIdentity {
username,
password: auth.password.clone().into(),
};

let mut creds = ntlm
.acquire_credentials_handle()
.with_credential_use(CredentialUse::Outbound)
.with_auth_data(&identity)
.execute(&mut ntlm)?;

let spn = self.context.spn().to_string();

let mut input = vec![SecurityBuffer::new(Vec::new(), BufferType::Token)];
let mut output = vec![SecurityBuffer::new(Vec::new(), BufferType::Token)];

let mut builder = ntlm
.initialize_security_context()
.with_credentials_handle(&mut creds.credentials_handle)
.with_context_requirements(
ClientRequestFlags::CONFIDENTIALITY | ClientRequestFlags::ALLOCATE_MEMORY,
)
.with_target_data_representation(DataRepresentation::Native)
.with_target_name(&spn)
.with_input(&mut input)
.with_output(&mut output);

let _ = ntlm
.initialize_security_context_impl(&mut builder)?
.resolve_to_result()?;

login_message.integrated_security(Some(output[0].buffer.clone()));
let id = self.context.next_packet_id();
self.send(PacketHeader::login(id), login_message).await?;
self = self.post_login_encryption(encryption);

let sspi_bytes = self.flush_sspi().await?;

let mut input = vec![SecurityBuffer::new(
sspi_bytes.as_ref().to_vec(),
BufferType::Token,
)];
let mut output = vec![SecurityBuffer::new(Vec::new(), BufferType::Token)];

let mut builder = ntlm
.initialize_security_context()
.with_credentials_handle(&mut creds.credentials_handle)
.with_context_requirements(
ClientRequestFlags::CONFIDENTIALITY | ClientRequestFlags::ALLOCATE_MEMORY,
)
.with_target_data_representation(DataRepresentation::Native)
.with_target_name(&spn)
.with_input(&mut input)
.with_output(&mut output);

let _ = ntlm
.initialize_security_context_impl(&mut builder)?
.resolve_to_result()?;

event!(Level::TRACE, authenticate_len = output[0].buffer.len());

let id = self.context.next_packet_id();
self.send(
PacketHeader::login(id),
TokenSspi::new(output[0].buffer.clone()),
)
.await?;
}

#[cfg(all(windows, feature = "winauth"))]
AuthMethod::Windows(auth) => {
let spn = self.context.spn().to_string();
Expand Down
12 changes: 11 additions & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ pub enum Error {
/// An error from the GSSAPI library.
#[error("GSSAPI Error: {}", _0)]
Gssapi(String),
/// An error in the sspi-rs library.
#[error("sspi-rs Error {}", _0)]
SspiRs(String),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
#[error(
"Server requested a connection to an alternative address: `{}:{}`",
host,
Expand Down Expand Up @@ -83,7 +86,7 @@ impl Error {

impl From<uuid::Error> for Error {
fn from(e: uuid::Error) -> Self {
Self::Conversion(format!("Error convertiong a Guid value {}", e).into())
Self::Conversion(format!("Error converting a Guid value {}", e).into())
}
}

Expand Down Expand Up @@ -157,3 +160,10 @@ impl From<libgssapi::error::Error> for Error {
Error::Gssapi(format!("{}", err))
}
}

#[cfg(all(unix, feature = "sspi-rs"))]
impl From <sspi::Error> for Error {
fn from(err: sspi::Error) -> Self {
Error::SspiRs(format!("{}", err))
}
}
Comment on lines +170 to +179
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

Add doc cfg attribute for consistency with GSSAPI impl.

The From<libgssapi::error::Error> impl (lines 159-168) includes a #[cfg_attr(feature = "docs", doc(cfg(...)))] attribute, but this impl is missing it.

📚 Proposed fix to add doc cfg attribute
 #[cfg(all(unix, feature = "sspi-rs"))]
+#[cfg_attr(
+    feature = "docs",
+    doc(cfg(all(unix, feature = "sspi-rs")))
+)]
 impl From<sspi::Error> for Error {
     fn from(err: sspi::Error) -> Self {
         Error::SspiRs(format!("{}", err))
     }
 }
📝 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
#[cfg(all(unix, feature = "sspi-rs"))]
impl From<sspi::Error> for Error {
fn from(err: sspi::Error) -> Self {
Error::SspiRs(format!("{}", err))
}
}
#[cfg(all(unix, feature = "sspi-rs"))]
#[cfg_attr(
feature = "docs",
doc(cfg(all(unix, feature = "sspi-rs")))
)]
impl From<sspi::Error> for Error {
fn from(err: sspi::Error) -> Self {
Error::SspiRs(format!("{}", err))
}
}

2 changes: 1 addition & 1 deletion src/tds/codec/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl<'a> LoginMessage<'a> {
}
}

#[cfg(any(all(unix, feature = "integrated-auth-gssapi"), windows))]
#[cfg(any(all(unix, any(feature = "integrated-auth-gssapi", feature = "sspi-rs")), windows))]
pub fn integrated_security(&mut self, bytes: Option<Vec<u8>>) {
if bytes.is_some() {
self.option_flags_2.insert(OptionFlag2::IntegratedSecurity);
Expand Down
2 changes: 1 addition & 1 deletion src/tds/codec/token/token_sspi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl AsRef<[u8]> for TokenSspi {
}

impl TokenSspi {
#[cfg(any(windows, all(unix, feature = "integrated-auth-gssapi")))]
#[cfg(any(windows, all(unix, any(feature = "integrated-auth-gssapi", feature = "sspi-rs"))))]
pub fn new(bytes: Vec<u8>) -> Self {
Self(bytes)
}
Expand Down
2 changes: 1 addition & 1 deletion src/tds/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl Context {
self.spn = Some(format!("MSSQLSvc/{}:{}", host.as_ref(), port));
}

#[cfg(any(windows, all(unix, feature = "integrated-auth-gssapi")))]
#[cfg(any(windows, all(unix, any(feature = "integrated-auth-gssapi", feature = "sspi-rs"))))]
pub fn spn(&self) -> &str {
self.spn.as_deref().unwrap_or("")
}
Expand Down
2 changes: 1 addition & 1 deletion src/tds/stream/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ where
}
}

#[cfg(any(windows, feature = "integrated-auth-gssapi"))]
#[cfg(any(windows, feature = "integrated-auth-gssapi", feature = "sspi-rs"))]
pub(crate) async fn flush_sspi(self) -> crate::Result<TokenSspi> {
let mut stream = self.try_unfold();
let mut last_error = None;
Expand Down