RUST-2245 Implement GSSAPI auth support for Windows #1444

mattChiaravalloti · 2025-08-06T19:27:42Z

This PR implements GSSAPI auth support for Windows as described by the spec. Similar to the Linux and Mac implementation in #1413, this one took longer than expected. In the end, it turned out that the sspi crate was not as amenable to this task as initially thought at scope time. I decided to attempt to implement this using the native Windows api (via windows-sys) and modeling it exactly off of libmongoc since I know the C implementation works. Of course, that results in a bit more C-style code than Rust-style code, but after several weeks of investigating and implementing, this is finally working.

I updated the GSSAPI codepath to share common code between Linux/Mac and Windows as much as possible. I only separated out the platform-specific implementations and guarded each with the relevant cfg attributes.

This PR also updates the e2e tests to run on Windows. I observed this implementation passing my local testing on a Windows host, and the e2e tests passing on this patch. I anticipate the evergreen patch associated with this PR to succeed off the bat, as well.

…f sspi

…ames

codeowners-service-app · 2025-08-06T19:28:11Z

Assigned rishitb-mongodb for team dbx-rust because abr-egn is out of office.

.evergreen/config.yml

.evergreen/run-gssapi-tests.sh

Cargo.toml

src/client/auth/gssapi/mod.rs

src/client/auth/gssapi/nix.rs

src/client/auth/gssapi/windows.rs

mattChiaravalloti · 2025-08-06T19:50:07Z

Is it possible to make exceptions to the semgrep policy to allow for the use of these unsafe blocks, or will we need to get rid of them?

mattChiaravalloti · 2025-08-07T14:27:32Z

Is it possible to make exceptions to the semgrep policy to allow for the use of these unsafe blocks, or will we need to get rid of them?

I investigated further and am not confident SSPI support can be implemented the way our drivers require if we do not use these unsafe blocks.

There are limited crate options for kerberos auth. We've ruled out libgssapi-sys/libgssapi/cross-krb5 since they rely on MIT KRB5. I discovered sspi is insufficient for our needs (it is incapable of producing the token types needed for MongoDB's SASL auth flow when using GSSAPI). Looking through crates.io, there are not any other viable candidates for this, so the windows crate is likely our best option.

Using the native Windows API inherently requires unsafe code. Hopefully we can make an exception for this. I know in the ODBC driver, we have some required unsafe code that is permitted.

abr-egn · 2025-08-07T14:46:45Z

Is it possible to make exceptions to the semgrep policy to allow for the use of these unsafe blocks, or will we need to get rid of them?

I investigated further and am not confident SSPI support can be implemented the way our drivers require if we do not use these unsafe blocks.

There are limited crate options for kerberos auth. We've ruled out libgssapi-sys/libgssapi/cross-krb5 since they rely on MIT KRB5. I discovered sspi is insufficient for our needs (it is incapable of producing the token types needed for MongoDB's SASL auth flow when using GSSAPI). Looking through crates.io, there are not any other viable candidates for this, so the windows crate is likely our best option.

Using the native Windows API inherently requires unsafe code. Hopefully we can make an exception for this. I know in the ODBC driver, we have some required unsafe code that is permitted.

I haven't had the chance to look into this PR in detail, but it should work to use a nosemgrep comment, we've had to do that in a couple other places in the code base (example). My recollection of the policy (I'll verify later) is that as long as we've investigated and have manually validated that the thing semgrep would complain about is not, in fact, a problem it's fine.

abr-egn · 2025-08-08T14:39:48Z

src/client/auth/gssapi/nix.rs

+    error::{Error, Result},
+};
+
+pub(super) struct GssapiAuthenticator {


Not changed in this PR, but this being a fresh file made me realize - this is only ever either

GssapiAuthenticator { pending_ctx: Some(...), established_ctx: None, is_complete: false, ... }

or

GssapiAuthenticator { pending_ctx: None, established_ctx: Some(...), is_complete: true, ... }

right? That seems like it would be better modeled via a state enum, e.g.

enum CtxState { Pending(PendingClientCtx), Established(ClientCtx), } pub(super) struct GssapiAuthenticator { ctx: CtxState, user_principal: String, }

Ah that's a nice refactor. I'll make this change.

abr-egn · 2025-08-08T14:52:49Z

src/client/auth/gssapi/windows.rs

+    }
+
+    fn acquire_credentials_and_init(&mut self, password: Option<String>) -> Result<Vec<u8>> {
+        unsafe {


I believe the only unsafe portion of this is calling AcquireCredentialsHandleW, right? I'd rather see the unsafe blocks as narrowly scoped as possible.

Good point! I started broad to allow for whatever had to be done, but I'll narrow these down now like you suggested 👍

src/client/auth/gssapi/windows.rs

pmeredit · 2025-08-08T19:44:29Z

src/client/auth/gssapi/nix.rs

+
+    // Perform the final step of Kerberos authentication by gss_unwrap-ing the
+    // final server challenge, then wrapping the protocol bytes + user principal.
+    // The resulting token must be sent to the server.


This is like a monad transformer 😂

isabelatkinson

one tiny comment, I did a pass over the C-style code but will similarly defer to @kevinAlbs on the details

isabelatkinson · 2025-08-11T21:55:20Z

src/client/auth/gssapi/mod.rs

This filename should be able to stay the same - we stopped using mod.rs files a while ago (https://doc.rust-lang.org/reference/items/modules.html#module-source-filenames)

Thank you for pointing this out, I updated this to use the appropriate naming conventions.

kevinAlbs

The contribution is much appreciated! I have mostly minor comments and questions.

.evergreen/config.yml

src/test/auth/gssapi.rs

src/client/auth/gssapi/windows.rs

.evergreen/run-gssapi-tests.sh

mattChiaravalloti · 2025-08-12T16:54:18Z

I suspect the "project fails validation: sync with base branch & run evergreen validate -p <project>" error is related to the ongoing Github problems. When I ran evergreen validate locally just now after merging main, it said the config was valid 👍 We should be able to run this patch whenever Github service is resolved and the evergreen team gives the green light for things to run as expected again.

kevinAlbs

LGTM!

mattChiaravalloti · 2025-08-12T16:56:14Z

I retagged everyone, but feel free to just take a look at the parts you commented on previously. I know this is a big and confusing PR. Plus, with 4 reviewers on this it could get hectic having too many cooks. Kevin's LGTM on the core functionality will be the primary indicator this is ready to merge 👍 Thanks for all the review and guidance on this one.

mattChiaravalloti added 13 commits July 29, 2025 15:03

RUST-2245: Fix typo in e2e test logging

fe75d16

RUST-2245: Initial second attempt at GSSAPI on Windows

9dda3b3

RUST-2245: Refactor service_principal name creation

4186779

RUST-2245: Separate windows and non-windows implementations

3cf15d0

RUST-2245: Update windows implementation to use windows api instead o…

9175eb2

…f sspi

Merge branch 'main' into RUST-2245

f95788b

RUST-2245: Fix rustfmt

08ca68a

RUST-2245: Refactor to share more common code

0d6858b

RUST-2245: Standardize requirement of user_principal and use better n…

56661ab

…ames

RUST-2245: More clean up, refactoring, and renaming

a32a408

RUST-2245: Static analysis fixes

6bb9227

RUST-2245: Update e2e tests to handle windows

6654548

RUST-2245: Fix evergreen tasks

9a3ffa8

mattChiaravalloti requested a review from a team as a code owner August 6, 2025 19:27

mattChiaravalloti requested a review from abr-egn August 6, 2025 19:27

codeowners-service-app bot requested a review from rishitb-mongodb August 6, 2025 19:28

mattChiaravalloti commented Aug 6, 2025

View reviewed changes

mattChiaravalloti requested review from isabelatkinson and removed request for rishitb-mongodb August 6, 2025 19:40

RUST-2245: Add more comments

726e9f8

abr-egn requested a review from kevinAlbs August 8, 2025 14:54

abr-egn reviewed Aug 8, 2025

View reviewed changes

mattChiaravalloti requested a review from pmeredit August 8, 2025 17:16

pmeredit reviewed Aug 8, 2025

View reviewed changes

mattChiaravalloti added 2 commits August 8, 2025 15:52

RUST-2245: Reduce unsafe to smaller scopes, add nosemgrep comments

51c98bc

RUST-2245: Refactor GssapiAuthenticator

71b3f68

mattChiaravalloti removed the request for review from isabelatkinson August 8, 2025 20:12

isabelatkinson reviewed Aug 11, 2025

View reviewed changes

kevinAlbs reviewed Aug 12, 2025

View reviewed changes

mattChiaravalloti added 4 commits August 12, 2025 12:22

RUST-2245: Prefer module name as file name instead of mod.rs

90b7738

RUST-2245: Apply code review suggestions

9136594

RUST-2245: Remove unneeded security layer check

46f80a7

Merge branch 'main' into RUST-2245

4b066cd

mattChiaravalloti requested review from kevinAlbs, isabelatkinson, abr-egn and pmeredit August 12, 2025 16:54

kevinAlbs approved these changes Aug 12, 2025

View reviewed changes

abr-egn approved these changes Aug 12, 2025

View reviewed changes

isabelatkinson approved these changes Aug 12, 2025

View reviewed changes

RUST-2245 Implement GSSAPI auth support for Windows #1444

Are you sure you want to change the base?

RUST-2245 Implement GSSAPI auth support for Windows #1444

Uh oh!

Conversation

mattChiaravalloti commented Aug 6, 2025

Uh oh!

codeowners-service-app bot commented Aug 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattChiaravalloti commented Aug 6, 2025

Uh oh!

mattChiaravalloti commented Aug 7, 2025

Uh oh!

abr-egn commented Aug 7, 2025

Uh oh!

abr-egn Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

mattChiaravalloti Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

abr-egn Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

mattChiaravalloti Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pmeredit Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

isabelatkinson left a comment

Choose a reason for hiding this comment

Uh oh!

isabelatkinson Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

mattChiaravalloti Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

kevinAlbs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattChiaravalloti commented Aug 12, 2025

Uh oh!

kevinAlbs left a comment

Choose a reason for hiding this comment

Uh oh!

mattChiaravalloti commented Aug 12, 2025

Uh oh!

Uh oh!