Skip to content

Add opt-in DFS client support with referral resolution#326

Open
jahales wants to merge 23 commits into
TalAloni:masterfrom
jahales:master
Open

Add opt-in DFS client support with referral resolution#326
jahales wants to merge 23 commits into
TalAloni:masterfrom
jahales:master

Conversation

@jahales
Copy link
Copy Markdown
Contributor

@jahales jahales commented Nov 25, 2025

PR: Add DFS Client Support

Summary

This PR adds opt-in Distributed File System (DFS) client support to SMBLibrary, enabling automatic resolution of DFS paths to their underlying file server targets. DFS remains disabled by default to maintain full backward compatibility.

Motivation

DFS is widely used in enterprise environments to provide:

  • Unified namespace across multiple file servers
  • Transparent failover and load balancing
  • Site-aware target selection

Without DFS support, clients must manually resolve DFS paths before connecting.

Changes

Core DFS Client Implementation (SMBLibrary/Client/DFS/)

Component Description
DfsClientFactory Entry point for creating DFS-aware file stores
DfsClientOptions Configuration (enabled, caching TTLs, retries, site name)
DfsPathResolver Implements DFS path resolution algorithm per MS-DFSC
DfsAwareClientAdapter Wraps ISMBFileStore with transparent DFS resolution
DfsSessionManager Manages cross-server sessions for interlink scenarios
ReferralCache / DomainCache Caching infrastructure with TTL support
Smb2DfsReferralTransport Issues FSCTL_DFS_GET_REFERRALS via SMB2

DFS Protocol Types (SMBLibrary/DFS/)

  • ResponseGetDfsReferral — Extended to fully parse referral responses (v1–v4)
  • RequestGetDfsReferralEx — Support for FSCTL_DFS_GET_REFERRALS_EX with site hints
  • DfsReferralEntryV1V4 — Referral entry structures per MS-DFSC
  • DfsReferralHeaderFlags, DfsReferralEntryFlags, DfsServerType — Enums

Tests (SMBLibrary.Tests/)

  • 36 new test files covering:
    • Unit tests for all DFS components
    • Referral parsing tests with protocol vectors
    • Integration tests for end-to-end resolution
    • Lab tests for real DFS namespace validation

Documentation (docs/)

Document Description
dfs-usage.md DFS client configuration guide with examples
codebase-patterns.md Architecture and code style reference
lab-setup.md SMB/DFS test lab options (local, Windows, Samba)
lab-setup-hyperv.md Step-by-step Hyper-V Windows Server lab

Test Scripts (scripts/)

  • test-smb-client.ps1 — Basic SMB2 client validation
  • test-smb-client-full.ps1 — Comprehensive read/write/echo test
  • test-smb1.ps1 — SMB1 legacy protocol test

Other Changes

  • ClientExamples.md — Added DFS usage example
  • SMB2FileStore.cs — Minor adjustment for DFS compatibility

Usage Example

// Connect normally
SMB2Client client = new SMB2Client();
client.Connect("server.domain.com", SMBTransportType.DirectTCPTransport);
client.Login("DOMAIN", "username", "password");

// Get base file store
ISMBFileStore fileStore = client.TreeConnect("DfsShare", out status);

// Wrap with DFS support (opt-in)
DfsClientOptions options = new DfsClientOptions { Enabled = true };
ISMBFileStore dfsStore = DfsClientFactory.CreateDfsAwareFileStore(fileStore, null, options);

// DFS paths are now automatically resolved
dfsStore.CreateFile(out handle, out fileStatus, @"\DfsLink\file.txt", ...);

Backward Compatibility

This PR has zero impact on existing code. DFS support is entirely opt-in:

  • Disabled by default: DfsClientOptions.Enabled defaults to false
  • No changes to existing APIs: SMB2Client, SMB1Client, ISMBFileStore, and all existing interfaces remain unchanged
  • No new dependencies: Uses only existing SMBLibrary infrastructure
  • No behavioral changes: Unless you explicitly create a DFS-aware store via DfsClientFactory, all existing code paths are untouched
  • All existing tests pass: No regressions

What happens if you don't use DFS?

Nothing changes. Existing code like this continues to work exactly as before:

// This code is completely unaffected by this PR
SMB2Client client = new SMB2Client();
client.Connect(serverIP, SMBTransportType.DirectTCPTransport);
client.Login(domain, username, password);
ISMBFileStore store = client.TreeConnect("Share", out status);
// store behaves exactly as it always has — no DFS involvement

DFS is only activated when you explicitly opt in:

// Only this pattern enables DFS
DfsClientOptions options = new DfsClientOptions { Enabled = true };
ISMBFileStore dfsStore = DfsClientFactory.CreateDfsAwareFileStore(baseStore, null, options);

Design Principles

  1. Opt-in by defaultDfsClientOptions.Enabled = false preserves existing behavior
  2. No breaking changes — Existing APIs unchanged; DFS is additive
  3. Per-connection configuration — No global/static state
  4. Spec compliance — Implements MS-DFSC referral protocol
  5. Testable — Interfaces allow mocking; extensive test coverage

Testing

  • All existing tests pass
  • New tests cover DFS parsing, resolution, caching, and integration
  • Validated against Windows Server DFS Namespaces in lab environment

References

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Nov 25, 2025

Should be fully backwards-compatible / not break existing code. I tested what I could in a local environment, but I no longer have access to an enterprise environment.

Note that this code was written with the help of LLMs--if that's a concern, I can just keep this in a fork.

I am willing to be the maintainer for the DFS logic.

@TalAloni
Copy link
Copy Markdown
Owner

Hi Jacob,
What a nice surprise,
This is, by far, the largest contribution to this project.
Before I start looking under the hood, please see that you agree to the contribution terms.

I see that in addition to the code (and unit-tests) you also contributed documentation and PowerShell tests scripts that need to be run manually.
Any documentation regarding DFS should be much more succinct, anybody wishing to set up a DFS environment should be able to find that information elsewhere IMO.
I'm not a fan of adding PowerShell to my repo, all code should be in C# for uniformity and simplicity.

Thanks!
Tal

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Nov 26, 2025

Hey Tal,

Happy to give a little back to the open source community. I agree to the contribution terms. I removed the scripts and trimmed down the documentation. When I get time I'll try to make a better lab setup and do some cross-platform testing.

Thanks,
Jacob

@TalAloni
Copy link
Copy Markdown
Owner

Thanks Jacob,
I took an initial peek and I'm intrigued:

  1. I saw that you made changes to SMB2FileStore but it's not clear why it was mandatory to make that change, DeviceIOControl expects "object handle" which is then casted to FileId, I get that it's cleaner to have a method that accepts FileId, but IIUC you could have easily passed a FileId instance to the existing method and avoided this change.
    BTW: I opted to use object instead of FileId to hide internal implementation and to keep API uniformity with SMB1FileStore.

  2. The more intriguing part is that (other than the SMB2FileStore change that is not mandatory) you did not change existing client code, this means that in theory, someone can get DFS working by using the existing SMBLibrary NuGet package, and implement only the DFS part like you did, is that right?
    To be clear: I do think it would be nice to have DFS support as part of the SMBLibrary NuGet package - but I'm just trying to understand all the possible ways forward.

  3. I try to keep pull requests small, it's clear to me I'll start by first merging the request / response and associated data types first (the MS-DFSC data types) and some styling changes may be required. I can use "Co-authored-by" when pushing the code so in that case you don't have to be bothered with subjective code comments.

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Nov 26, 2025

Hey Tal,

  1. Yes, that's not strictly mandatory. You're welcome to undo that change.

  2. That's right, in theory the DFS implementation could be a separate repo altogether. I don't have a strong opinion here. I intended it to be opt-in that way there's low risk of impacting anyone already using your library.

  3. Sure, that makes sense to me.

I'm honestly open to whatever you prefer. I have no concerns with you modifying the code for style or anything like that and merging it piecemeal. Or if it's easier, I can maintain a separate wrapper package that consumes SMBLibrary as a dependency, and you can keep your focus on the core SMB implementation.

@TalAloni TalAloni force-pushed the master branch 3 times, most recently from 093b856 to 5c5c764 Compare November 29, 2025 19:42
@TalAloni
Copy link
Copy Markdown
Owner

TalAloni commented Nov 29, 2025

Hey Jacob,
I pushed the first part which includes the DFS data structures with you as co-author.

  • I noticed that you only implemented parsing logic (client logic) - I added GetBytes implementation for (future) server side logic.
  • I captured Windows Server 2008 R2 SP1 response with a single V4 referral for the purpose of testing parsing in a unit-test (would be great to get additional samples to increase coverage if you can capture them - especially one with multiple references and/or V2 from Windows 2000)
  • You strayed from the documentation here and there - hopefully I was able to catch and correct all the structures but it would be best if you check me on that / get some samples using WireShark for better coverage.
  • I understand why you kept all the V1-V4 parsing code in ResponseGetDfsReferral (due to the "shared" StringBuffer), I thought it was preferable to put the parsing code of each structure in the appropriate class. luckily, I couldn't find anything in the specifications preventing me from putting the strings immediately after each referral entry, and even if there was such a requirement, it would have been cleaner to pass a reference than to have a long ResponseGetDfsReferral class IMO.

If you have the time, it would be nice it you could rebase your branch and use the newly added data structures and see if I got everything right.
I probably will not have additional time to continue to the next part before next weekend.

Thanks!
Tal

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Nov 29, 2025

Hey Tal,

Good catch. I rebased and made a few minor updates. I disassembled my lab setup but I'll get that going again in a couple weeks. I'll see if I can setup some VMs with more Windows versions and start collecting traces. There's no rush on my end, so take your time and I appreciate you being willing to support this.

Thanks,
Jacob

@TalAloni
Copy link
Copy Markdown
Owner

Thanks,
Good catch yourself, thanks for those corrections, fixed and pushed.

@TalAloni
Copy link
Copy Markdown
Owner

TalAloni commented Nov 29, 2025

luckily, I couldn't find anything in the specifications preventing me from putting the strings immediately after each referral entry

I take that back, in "2.2.5 Referral Entry Types", it's clearly stated that "The strings referenced from the fields of a referral entry MUST follow the last referral entry in the RESP_GET_DFS_REFERRAL message".

I pushed a fix to the above.

@TalAloni
Copy link
Copy Markdown
Owner

According to the specs there should be a SiteNameLength field in REQ_GET_DFS_REFERRAL_EX but your code assumes RequestFileName will be immediately followed by SiteName. could you please use WireShark to capture such REQ_GET_DFS_REFERRAL_EX request?

jahales added a commit to jahales/SMBLibrary that referenced this pull request Dec 14, 2025
- Add RequestFileNameLength (2 bytes) before RequestFileName
- Add SiteNameLength (2 bytes) before SiteName when flag is set
- Update parsing to read length fields correctly
- Add tests with captured Windows 11 FSCTL_DFS_GET_REFERRALS_EX data

Addresses Tal's feedback on PR TalAloni#326 regarding missing length fields.
…ution

- Add DFS referral structures (V1-V4) and response codecs

- Implement DFS path resolver with caching support

- Add DFS-aware client adapter and file store factory

- Implement IOCTL request builder for DFS referrals

- Add comprehensive unit and integration tests

- Include client usage examples and documentation
- docs/codebase-patterns.md: Architecture and code style guide

- docs/dfs-usage.md: DFS client configuration and troubleshooting

- docs/lab-setup.md: General SMB/DFS lab setup options

- docs/lab-setup-hyperv.md: Detailed Hyper-V Windows Server lab walkthrough

- scripts/test-smb-client.ps1: Basic SMB2 client test

- scripts/test-smb-client-full.ps1: Comprehensive read/write/echo test

- scripts/test-smb1.ps1: SMB1 legacy protocol test

- ClientExamples.md: Add reference to DFS usage guide
- Updated client code to use SMBLibrary.DFS namespace
- Updated DfsReferralSelector to handle V1 ShareName vs V2/V3 NetworkAddress
- Updated DfsClientResolver and DfsPathResolver TTL handling for V2/V3
- Fixed tests to use correct V2 buffer format and enum names (ReferalServers)
- Removed incompatible V1 buffer tests pending rewrite
Per MS-DFSC 2.2.4, the R bit flag is named 'ReferralServers' (with double 'r')
- Fixed V3 NameListReferral parsing to use relative offsets per MS-DFSC
  (specialNameOffset and expandedNameOffset must be added to entry offset)
- Changed RequestGetDfsReferralEx namespace from SMBLibrary to SMBLibrary.DFS
  for consistency with other DFS files
Added tests for:
- V1 round-trip (ShareName serialization/parsing)
- V2 round-trip (all fields)
- V3 round-trip (with ServiceSiteGuid)
- Multiple V3 referrals
- Empty referral list
- DfsClientResolver with multiple V2 referrals
- DfsClientResolver with empty NetworkAddress (error case)
- DfsClientResolver with V1 ShareName
- Added GetBytes round-trip tests to DfsReferralEntryV1Tests, V2Tests, V3Tests, V4Tests
- Added Length calculation tests
- Added V3 NameListReferral round-trip test
- All 303 tests pass (NTFileStoreTests requires C:\Tests directory)
- Add RequestFileNameLength (2 bytes) before RequestFileName
- Add SiteNameLength (2 bytes) before SiteName when flag is set
- Update parsing to read length fields correctly
- Add tests with captured Windows 11 FSCTL_DFS_GET_REFERRALS_EX data

Addresses Tal's feedback on PR TalAloni#326 regarding missing length fields.
@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Dec 14, 2025

According to the specs there should be a SiteNameLength field in REQ_GET_DFS_REFERRAL_EX but your code assumes RequestFileName will be immediately followed by SiteName. could you please use WireShark to capture such REQ_GET_DFS_REFERRAL_EX request?

Sure. I just got my lab setup again so I can start running other tests too. Anything else you'd like me to capture? Pushed a fix.

dfs-capture.zip

@TalAloni
Copy link
Copy Markdown
Owner

Thank you for the capture, it has been useful for both RequestGetDfsReferralEx and multiple referral entries in the response.

I added RequestGetDfsReferralEx implementation, so I believe you can now use the code in \SMBLibrary\DFS as-is, correct me if I'm wrong or if something is not working correctly.

I'll try to start looking into the client code this weekend.

@TalAloni
Copy link
Copy Markdown
Owner

Thanks. fixed.

@TalAloni
Copy link
Copy Markdown
Owner

Hi Jacob,
I had a few busy weekends, I will try to find the time to continue soon.

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Jan 12, 2026

No problem at all. Honestly there's no rush on my end, so take your time.

@TalAloni TalAloni force-pushed the master branch 2 times, most recently from d8327d6 to 4910252 Compare February 26, 2026 19:32
@TalAloni
Copy link
Copy Markdown
Owner

TalAloni commented Apr 6, 2026

Sorry again for the delay, there is a lot to process here and I'm not a DFS expert,
Would it make sense to instantiate the SMB2DfsFileStore if and only if we perform a TreeConnect and the reply has the SMB2_SHAREFLAG_DFS_ROOT flag set?

jahales added 3 commits April 6, 2026 07:25
Add opt-in DfsClientOptions property to SMB2Client. When set with
Enabled=true, TreeConnect() checks SMB2_SHAREFLAG_DFS in the server
response and automatically wraps the file store with DfsAwareClientAdapter.

Default behavior (DfsClientOptions=null) is unchanged — zero overhead
for existing callers.

Also fix pre-existing null-handle crash in Smb2DfsReferralTransport
where passing null dfsHandle to DeviceIOControl would throw
NullReferenceException on FileID struct unboxing. Now defaults to the
well-known DFS referral FileID (0xFFFFFFFFFFFFFFFF) per MS-SMB2 2.2.31.
@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Apr 6, 2026

No problem at all.

I think so, yes, with one tweak: I’d key it off SMB2_SHAREFLAG_DFS rather than SMB2_SHAREFLAG_DFS_ROOT.

My reading is that DFS is the capability bit that matters for whether referral resolution may be needed, while DFS_ROOT is narrower and more descriptive of the share itself.

@TalAloni
Copy link
Copy Markdown
Owner

TalAloni commented Apr 6, 2026

I found it appealing to apply special behavior only to the DFS root server, where it's unlikely that the regular SMB2Client TreeConnect would work as expected, using SMB2_SHAREFLAG_DFS is more problematic in that regard because it can be returned by the namespace server. When I connect to the Namespace server I arguably want to communicate with that server, not the entire DFS tree.
So back to the original question, does it makes sense that when somebody wants the DFS abstraction they would have to connect to the root server?

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Apr 6, 2026

Sure, that makes sense to me. Thanks for clarifying.

@TalAloni
Copy link
Copy Markdown
Owner

TalAloni commented Apr 6, 2026

Thanks,

  1. The size of this pull request is a challenge, I want to extract a minimum valuable DFS client out of this 65 files pull request, it doesn't have to have unit-tests, it doesn't have to have readmes, it doesn't have to have caching, only a minimum set of code necessary for DFS resolution to work correctly. something that is small enough for me to be able to review and test.

  2. In a typical complex scenario we have the following:
    Assume I wish to access a file in \\Contoso.com\Europe\England\Finance

  • In this example \\Contoso.com\Europe represents the primary Namespace Server (Server 1),
    \\Contoso.com\Europe\England represents a nested namespace server (Server 2)
    The actual data share holding the file we want is \\FileServer\Finance (Server 3).
  • So IIUC you would need to create 3 SMB2Client instances to access the file (or call Connect 3 times), and I'm assuming you created the DfsSessionManager class to create those instances, but IIUC this code path is not used right now.
  1. You have a class called DfsAwareClientAdapter (not a perfect name since this is not technically an adapter IMO) which accepts an "inner" ISMBFileStore, IIUC this should work differently, the SMB2Client detects that we are dealing with a DFS root so it should send a FSCTL_DFS_GET_REFERRALS request and then connect to the most appropriate server and then create an "SMB2DfsFileStore". in the case of a folder target that points to a 3rd server, then the SMB2DfsFileStore should do its magic and create a new SMB2Client and SMB2FileStore to handle the filesystem operation on the actual server.

Did I get something wrong here?

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Apr 18, 2026

Hey @TalAloni,

I'm implementing SMB2DfsFileStore so TreeConnect() automatically returns it for DFS root shares.

When a DFS referral points to a target on another server, the client may need to authenticate a new SMB2 connection to that target.

For callers using Login(domainName, userName, password), one option is to retain those credentials privately on SMB2Client and reuse them for the target connection.

The unclear case is callers using Login(IAuthenticationClient), since that interface can authenticate the current connection but doesn't expose a way to recreate auth state for a referred target.

What's your preference for handling this? I could scope the next pr to just username/password login overloads for now. Or we could consider adding an additional authentication abstraction.

@TalAloni
Copy link
Copy Markdown
Owner

The unclear case is callers using Login(IAuthenticationClient), since that interface can authenticate the current connection but doesn't expose a way to recreate auth state for a referred target.

I did not see this one coming.

Introducing IAuthenticationClientProvider or Func<NTLMAuthenticationClientProvider, string> that accepts the host does not feel simple to use and would force dealing with constructor arguments related to Kerberos authentication which I do not want to do.

Adding
IAuthenticationClient.ReinitializeSecurityContext(string host)
is little better, and I think it would work for all cases.
(given that there is no reuse of the instance once we complete login to the server)

I'm thinking of alternatives, one thing that came to mind is
byte[] InitializeSecurityContext(string host, byte[] securityBlob);
but it makes implementation slightly more complicated.

I need some time to let this to sink in. someone advertised "no breaking changes" ;-)

one option is to retain those credentials privately on SMB2Client and reuse them for the target connection

That would be a step backward, IAuthenticationClient is what enabling Kerberos authentication which is usually used by a large part of the demographic that is interested in DFS.

@jahales
Copy link
Copy Markdown
Contributor Author

jahales commented Apr 18, 2026

Just to clarify my earlier "no breaking changes" framing: that was describing the original opt-in design from this PR, where DFS support lived outside the existing client flow via DfsClientFactory/DfsAwareClientAdapter and could even have remained a separate package on top of the existing SMBLibrary NuGet package.

The direction you outlined now is different: DFS is integrated directly into SMB2Client/TreeConnect(), and SMB2DfsFileStore is part of the core client flow. Under that approach, I think the right constraint is still to keep changes as small and safe as possible, but not to treat the original opt-in/no-API-change goal as absolute if a small authentication-surface change turns out to be required for the integrated design to work correctly.

So I agree the earlier "no breaking changes" statement should be read as applying to the original wrapper-based approach, not necessarily to the minimum integrated client you are now steering toward.

@TalAloni
Copy link
Copy Markdown
Owner

Just to clarify my earlier "no breaking changes" framing: that was describing the original opt-in design from this PR

Yes, I realize that - that's why I added the wink.

I think that adding
IAuthenticationClient.ResetSecurityContext(string host)
is a worthy price to pay to have DFS working, but do you think it will end there in terms of breaking changes? or are there additional changes required for the integrated design to work well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants