Skip to content

chore: upgrade sspi to 0.21.3 - #174

Merged
afiffon merged 3 commits into
mainfrom
chore/upgrade-sspi-0-21-3
Sep 20, 2026
Merged

afiffon merged 3 commits into
mainfrom
chore/upgrade-sspi-0-21-3

Conversation

@afiffon

@afiffon afiffon commented Sep 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • upgrade sspi to 0.21.3 and align the RustCrypto dependency set
  • update the SSPI username logging API and clean up manifest warnings
  • scope test and RDMA-only dependencies to their actual consumers

Builds on the dependency work in #172. Thanks to @chdalski for the original update; they are also credited as a co-author on the commit.

Validation

  • cargo test --workspace --locked --no-run
  • CI-equivalent single-threaded and multi-threaded test builds and SMB unit tests with warnings denied

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened session authentication and setup validation, including clearer handling of invalid states, missing tokens, session keys, and signatures.
    • Improved compression error handling without changing its externally visible behavior.
    • Updated session setup logging to display the authenticated user name consistently.
  • Compatibility

    • Improved transport feature compatibility for asynchronous and RDMA configurations.
    • Refined file information trait constraints for broader compiler compatibility.
  • Maintenance

    • Standardized test macro usage and simplified package metadata.

Fixes #166

Co-authored-by: Christoph Dalski <chdalski.coding@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 1266fa9b-4b17-4baf-bdad-a723c51050f9

📥 Commits

Reviewing files that changed from the base of the PR and between 71d13c9 and 41f5c17.

📒 Files selected for processing (2)
  • crates/smb/src/session/authenticator.rs
  • crates/smb/src/session/setup.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0fd88c91-d561-40ab-8fa3-005d103408c7

📥 Commits

Reviewing files that changed from the base of the PR and between 662ccee and 71d13c9.

📒 Files selected for processing (2)
  • crates/smb/src/session/authenticator.rs
  • crates/smb/src/session/setup.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The pull request revises SMB session authentication, updates SSPI and cryptography dependencies, changes transport feature configuration, updates test macro wiring, and removes redundant Cargo metadata.

Changes

Authentication and workspace updates

Layer / File(s) Summary
SSPI authenticator hardening
crates/smb/src/session/authenticator.rs, crates/smb/Cargo.toml
Validates unexpected SSPI statuses, returns an error for short session keys, and adds tests for final SPNEGO MIC handling. SSPI and cryptography dependencies move to newer releases.
Session setup state flow
crates/smb/src/session/setup.rs
Derives expected setup statuses, validates session responses, and separates final authentication from session setup.
Workspace dependency and feature wiring
Cargo.toml, crates/*/Cargo.toml, smb-cli/Cargo.toml
Removes homepage and README metadata, makes transport dependencies optional, and updates async and rdma feature membership.
Test wiring and API cleanup
crates/smb-tests/*, crates/smb-fscc/src/info_classes.rs, crates/smb/src/compression.rs
Re-exports pastey for generated test macros, updates FileInfoType lifetime syntax, and removes a redundant error operator.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant SMBClient
  participant SessionSetup
  participant Authenticator
  participant SMBServer
  SMBClient->>SessionSetup: Start session setup
  SessionSetup->>Authenticator: Generate authentication token
  Authenticator->>SMBServer: Send SPNEGO token
  SMBServer-->>SessionSetup: Return setup status and response
  SessionSetup->>Authenticator: Complete final authentication
  Authenticator-->>SessionSetup: Return validated session key
  SessionSetup-->>SMBClient: Establish authenticated session
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The pull request also changes code and manifests with no demonstrated connection to [#166]. Examples include removal of workspace and package readme/homepage metadata, the FileInfoType lifetime-… Remove these unrelated changes from this pull request or submit them in separate pull requests. Keep dependency changes that are required for the sspi upgrade and the NTLM session-setup implementation.
Docstring Coverage ⚠️ Warning Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: upgrading sspi to version 0.21.3.
Linked Issues check ✅ Passed For [#166], crates/smb/Cargo.toml updates sspi from =0.18.7 to =0.21.3, which removes the blocked dependency range. The new SessionSetup flow completes the final SPNEGO token, checks authent…
Full details: Out of Scope Changes check

Explanation

The pull request also changes code and manifests with no demonstrated connection to [#166]. Examples include removal of workspace and package readme/homepage metadata, the FileInfoType lifetime-bound change, smb-tests pastey re-export changes, the redundant compression error-expression cleanup, and RDMA/test dependency scoping. These changes are separate from the SSPI, SPNEGO, session-key, and SMB-signing objectives.

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

A rabbit checks each token’s trail
The session keys no longer fail
SPNEGO MICs are tested bright
Cargo paths are trimmed just right
Pastey hops through macros small
Stable crates now serve them all

Comment @coderabbitai help to get the list of available commands.

@afiffon

afiffon commented Sep 20, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@afiffon
afiffon disabled auto-merge September 20, 2026 17:32
@afiffon
afiffon merged commit f8876b4 into main Sep 20, 2026
9 checks passed
@afiffon
afiffon deleted the chore/upgrade-sspi-0-21-3 branch September 20, 2026 17:32
@github-project-automation github-project-automation Bot moved this to Done in smb-rs Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

NTLM authentication fails with sspi 0.18.8+ due to missing SPN and pinned dependency range

1 participant