Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UnixDomainSocketBinding default security on Windows doesn't work #5621

Open
mconnew opened this issue Aug 1, 2024 · 3 comments
Open

UnixDomainSocketBinding default security on Windows doesn't work #5621

mconnew opened this issue Aug 1, 2024 · 3 comments
Labels

Comments

@mconnew
Copy link
Member

mconnew commented Aug 1, 2024

On Windows, it uses NegotiateStream to establish the connection. As part of that, we pass a target name to NegotiateStream.AuthenticateAsClientAsync which is used to get a Kerberos ticket or decide to use NTLM. There's shared code which implicitly uses the hostname from the endpoint address Uri, but with UDS there is no hostname. This results in a bad target name being used (it ends up using host/) and authentication failing.

Workaround:
Construct you EndpointAddress like this:

var endpointAddress = new EndpointAddress(new Uri("net.uds://" + servicePath), new SpnEndpointIdentity("host/localhost"));

This will override the implicit target name to be host/localhost and the NegotiateStream authentication will succeed.

A few options to fix this.

  1. On the CreateChannel code path, we could create a new Uri from the passed in Uri which has the hostname portion populated with localhost if it's currently empty.
  2. On the CreateChannel code path, we could add an SpnEndpointIdentity("host/localhost") to the EndpointAddress if there isn't currently an identity.
  3. In the code which calculates the target name, treat an empty hostname as the empty string target name. So if the hostname is non-empty, generate the target name host/localhost, but if the hostname is empty, generate the target name String.Empty. I have verified String.Empty successfully authenticates.
@mconnew
Copy link
Member Author

mconnew commented Aug 12, 2024

On working out why the tests weren't failing, I found where the fix needs to be. In UnixDomainSocketChannelFactory<TChannel>.OnCreateChannel there's this code:

            if (address.Identity == null)
            {
                var hostIdentity = new DnsEndpointIdentity(address.Uri.Host ?? "localhost");
                var uriBuilder = new UriBuilder(address.Uri);
                uriBuilder.Host = null;
                address = new EndpointAddress(uriBuilder.Uri, hostIdentity,address.Headers.ToArray());
            }

The null check on address.Uri.Host needs to be a string.IsNullOrEmpty call instead. Also the tests (at least for CoreWCF, haven't checked WCF yet) had an extra catch block in the try/finally used for test cleanup which swallowed the failure, but this isn't what made it pass. A UriBuilder is used which provides a hostname of localhost by default in the Uri. So basically the code should be this:

            if (address.Identity == null)
            {
                var hostIdentity = new DnsEndpointIdentity(string.IsNullOrEmpty(address.Uri.Host) ? "localhost" : address.Uri.Host);
                var uriBuilder = new UriBuilder(address.Uri);
                uriBuilder.Host = null;
                address = new EndpointAddress(uriBuilder.Uri, hostIdentity,address.Headers.ToArray());
            }

@mconnew
Copy link
Member Author

mconnew commented Aug 12, 2024

@imcarolwang, can you create a PR to fix this issue. Take a look at the CoreWCF issue for a comment with a description of why the tests didn't fail in the unit tests there and make sure any of those issues are fixed in the WCF tests too. Basically don't use UriBuilder and make sure there isn't an extra catch block.

@imcarolwang
Copy link
Contributor

imcarolwang commented Aug 13, 2024

@imcarolwang, can you create a PR to fix this issue. Take a look at the CoreWCF issue for a comment with a description of why the tests didn't fail in the unit tests there and make sure any of those issues are fixed in the WCF tests too. Basically don't use UriBuilder and make sure there isn't an extra catch block.

Hi @mconnew , I checked the WCF test, it doesn’t include a catch block. And in CoreWCF, the LinuxSocketFilepath is an empty string, whereas in WCF, the UriBuilder is initialized with a non-null temporary path. Given these differences, it seems the fix applied in CoreWCF may not be necessary for WCF. Could you please advise if this approach is suitable?

Test in WCF: link

Update:
Please disregard my previous comments. I didn’t have the full picture at the time. I now understand the issue and will try work on fixing the bug in WCF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants