Fix Kerberos/NTLM for multiple domain/realm environments in Linux by davidsh · Pull Request #36329 · dotnet/corefx

davidsh · 2019-03-24T21:56:30Z

This PR addresses some issues reported by a customer (not thru GitHub). They noticed that
multiple domain scenarios were working in .NET Core 2.0 but broke in .NET Core 2.1 on Linux.

While the previous PR #35383 solved the Negotiate Kerberos to NTLM fallback issue,
it added more complexity than necessary. The retry logic I added wasn't really necessary
because the original code that used GSS_KRB5_NT_PRINCIPAL_NAME format for the target name
was wrong. That logic only works for pure Kerberos environments and doesn't handle domain
or realm referrals. So, it only handles the default Kerberos realm on the single Linux
client. In addition, using GSS_KRB5_NT_PRINCIPAL_NAME defeats the logic of the SPNEGO mechanism.
That is why I originally needed to add the retry logic using GSS_C_NT_HOSTBASED_SERVICE format
for the target name.

The multiple domain/realm scenario worked in .NET Core 2.0 because it used CurlHandler.
And libcurl always uses GSS_C_NT_HOSTBASED_SERVICE format for target name.

This PR reworks the logic to use the GSS_C_NT_HOSTBASED_SERVICE format. It also removes the
now unneeded retry logic. I tested this against Windows and Linux as well as pure Kerberos
and Kerberos to NTLM fallback (using SPNEGO).

I added more tests. These tests are currently part of the enterprise scenario tests we are
building. They are activated via environment variables.

By definining all of the environment variables below, I am able to run the System.Net.Security
and System.Net.Http enterprise-scenario tests. Both SocketsHttpHandler in System.Net.Http and
NegotiateStream in System.Net.Security use the same common GSS-API logic in CoreFx.

Define domain-joined server remote endpoint:

COREFX_NET_SECURITY_NEGOSERVERURI
COREFX_DOMAINJOINED_HTTPHOST
COREFX_NET_AD_DOMAINNAME
COREFX_NET_AD_PASSWORD
COREFX_NET_AD_USERNAME

Define standalone server remote endpoint:

COREFX_NET_SERVER_PASSWORD
COREFX_NET_SERVER_USERNAME
COREFX_WINDOWSSERVER_HTTPHOST

This PR addresses some issues reported by a customer (not thru GitHub). They noticed that multiple domain scenarios were working in .NET Core 2.0 but broke in .NET Core 2.1 on Linux While the previous PR dotnet#35383 solved the Negotiate Kerberos to NTLM fallback issue, it added more complexity than necessary. The retry logic I added wasn't really necessary because the original code that used GSS_KRB5_NT_PRINCIPAL_NAME format for the target name was wrong. That logic only works for pure Kerberos environments and doesn't handle domain or realm referrals. So, it only handles the default Kerberos realm on the the single Linux client. In addition, using GSS_KRB5_NT_PRINCIPAL_NAME defeats the logic of the SPNEGO mechanism. That is why I originally needed to add the retry logic using GSS_C_NT_HOSTBASED_SERVICE format for the target name. The multiple domain/realm scenario worked in .NET Core 2.0 because it used CurlHandler. And libcurl always uses GSS_C_NT_HOSTBASED_SERVICE format for target name. This PR reworks the logic to use the GSS_C_NT_HOSTBASED_SERVICE format. It also removes the now unneeded retry logic. I tested this against Windows and Linux as well as pure Kerberos and Kerberos to NTLM fallback (using SPNEGO). I added more tests. These tests are currently part of the enterprise scenario tests we are building. They are activated via environment variables. By definining all of the environment variables below, I am able to run the System.Net.Security and System.Net.Http enterprise-scenario tests. Both SocketsHttpHandler in System.Net.Http and NegotiateStream in System.Net.Security use the same common GSS-API logic in CoreFx. Define domain-joined server remote endpoint: * COREFX_NET_SECURITY_NEGOSERVERURI * COREFX_DOMAINJOINED_HTTPHOST * COREFX_NET_AD_DOMAINNAME * COREFX_NET_AD_PASSWORD * COREFX_NET_AD_USERNAME Define standalone server remote endpoint: * COREFX_NET_SERVER_PASSWORD * COREFX_NET_SERVER_USERNAME * COREFX_WINDOWSSERVER_HTTPHOST

davidsh · 2019-03-24T22:18:25Z

/azp run corefx-outerloop-linux

azure-pipelines · 2019-03-24T22:18:37Z

Azure Pipelines successfully started running 1 pipeline(s).

stephentoub · 2019-03-25T14:45:42Z

src/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs

+                // Convert any "SERVICE/HOST" style of targetName to use "SERVICE@HOST" style.
+                // This is because the System.Net.Security.Native GSS-API layer uses
+                // GSS_C_NT_HOSTBASED_SERVICE format for targetName.
+                _targetName = SafeGssNameHandle.CreateTarget(targetName.Replace('/', '@'));


Is there any situation where / might appear where it shouldn't be replaced? e.g. could / be escaped somehow?

No. The formats for SPN are very specific. Windows always uses SERVICE/HOST for example for Service Principal Names. And there can't be embedded slashes in those. It would be a malformed name and generate errors anyways.

stephentoub · 2019-03-25T14:48:52Z

src/Native/Unix/System.Net.Security.Native/pal_gssapi.c

+                                                retFlags,
+                                                NULL);
+
+    if (isNtlm)


Seems like this could be:

*isNtlmUsed = (isNtlm || majorStatus != GSS_S_COMPLETE || gss_oid_equal(outmech, krbMech) == 0) ? 1 : 0;

?

This is C code and not C#. Does the tertiary operation work in C? :)

Yes, C has a ternary operator.

Thank you! I didn't realize that. I'll make the changes you suggested.

wfurt

LGTM.

davidsh · 2019-03-25T16:47:43Z

/azp run corefx-outerloop-linux

azure-pipelines · 2019-03-25T16:47:54Z

Azure Pipelines successfully started running 1 pipeline(s).

rmkerr · 2019-03-25T18:27:10Z

src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Authentication.cs

+        {
+            if (IsCurlHandler)
+            {
+                throw new SkipTestException("Skipping test on CurlHandler (libCurl)");


I haven't seen this before. Is there a new pattern for skipping tests?

Yes. It requires ConditionalFact or ConditionalTheory even if you don't specify a condition. And it requires another using statement as well.

using Microsoft.DotNet.XUnitExtensions;

We've started to use this pattern in our updated tests.

And using this new pattern shows up better in the test logs because it will say that it skipped the test and print out the reason why.

rmkerr

Removing code and increasing functionality at the same time? I'm a huge fan of that. LGTM.

This PR is a follow-up to PR dotnet#36329. In that previous PR, I changed to use the GSS_C_NT_HOSTBASED_SERVICE format for the SPN. That requires using a '@' separator character instead of a "/". I had assumed since System.Data.SqlClient was including the source files from Common for SafeDeleteNegoContext.cs that it would have the corresponding change to use the proper separator character. However, it appears that the file was only included to get the project building. It was using a separate file to calculate the SPN. The customer requiring these fixes reported that Kerberos was working properly now for HttpClient and NegotiateStream. But SqlClient was still broken. This PR completes the fix so that SqlClient will work properly with Kerberos especially in multiple domain/realm environments. This fix was manually tested in the enterprise scenario test lab.

This PR is a follow-up to PR #36329. In that previous PR, I changed to use the GSS_C_NT_HOSTBASED_SERVICE format for the SPN. That requires using a '@' separator character instead of a "/". I had assumed since System.Data.SqlClient was including the source files from Common for SafeDeleteNegoContext.cs that it would have the corresponding change to use the proper separator character. However, it appears that the file was only included to get the project building. It was using a separate file to calculate the SPN. The customer requiring these fixes reported that Kerberos was working properly now for HttpClient and NegotiateStream. But SqlClient was still broken. This PR completes the fix so that SqlClient will work properly with Kerberos especially in multiple domain/realm environments. This fix was manually tested in the enterprise scenario test lab.

This PR ports some important Kerberos auth fixes from the 3.0 to 2.1 LTS branch. These fixes help enterprise customers that have complex Kerberos authentication scenarios that involve cross Windows (Active Directory) and Linux (Kerberos) domains/realms. These fixes are from PRs: * dotnet#38465 - Use 'Host' header when calculating SPN for Kerberos auth * dotnet#38377 - Use GSS_C_NT_HOSTBASED_SERVICE format for Linux Kerberos SPN and are related to issue dotnet#36329.

This PR ports some important Kerberos auth fixes from the 3.0 to 2.1 LTS branch. These fixes help enterprise customers that have complex Kerberos authentication scenarios that involve cross Windows (Active Directory) and Linux (Kerberos) domains/realms. These fixes are from PRs: * #38465 - Use 'Host' header when calculating SPN for Kerberos auth * #38377 - Use GSS_C_NT_HOSTBASED_SERVICE format for Linux Kerberos SPN and are related to issue #36329.

* Update BuildTools to rc1-04230-01 * Port Kerberos auth fixes to 2.1 branch (#40109) This PR ports some important Kerberos auth fixes from the 3.0 to 2.1 LTS branch. These fixes help enterprise customers that have complex Kerberos authentication scenarios that involve cross Windows (Active Directory) and Linux (Kerberos) domains/realms. These fixes are from PRs: * #38465 - Use 'Host' header when calculating SPN for Kerberos auth * #38377 - Use GSS_C_NT_HOSTBASED_SERVICE format for Linux Kerberos SPN and are related to issue #36329.

…tnet/corefx#36329) This PR addresses some issues reported by a customer (not thru GitHub). They noticed that multiple domain scenarios were working in .NET Core 2.0 but broke in .NET Core 2.1 on Linux. While the previous PR dotnet/corefx#35383 solved the Negotiate Kerberos to NTLM fallback issue, it added more complexity than necessary. The retry logic I added wasn't really necessary because the original code that used GSS_KRB5_NT_PRINCIPAL_NAME format for the target name was wrong. That logic only works for pure Kerberos environments and doesn't handle domain or realm referrals. So, it only handles the default Kerberos realm on the the single Linux client. In addition, using GSS_KRB5_NT_PRINCIPAL_NAME defeats the logic of the SPNEGO mechanism. That is why I originally needed to add the retry logic using GSS_C_NT_HOSTBASED_SERVICE format for the target name. The multiple domain/realm scenario worked in .NET Core 2.0 because it used CurlHandler. And libcurl always uses GSS_C_NT_HOSTBASED_SERVICE format for target name. This PR reworks the logic to use the GSS_C_NT_HOSTBASED_SERVICE format. It also removes the now unneeded retry logic. I tested this against Windows and Linux as well as pure Kerberos and Kerberos to NTLM fallback (using SPNEGO). I added more tests. These tests are currently part of the enterprise scenario tests we are building. They are activated via environment variables. By defining all of the environment variables below, I am able to run the System.Net.Security and System.Net.Http enterprise-scenario tests. Both SocketsHttpHandler in System.Net.Http and NegotiateStream in System.Net.Security use the same common GSS-API logic in CoreFx. Define domain-joined server remote endpoint: * COREFX_NET_SECURITY_NEGOSERVERURI * COREFX_DOMAINJOINED_HTTPHOST * COREFX_NET_AD_DOMAINNAME * COREFX_NET_AD_PASSWORD * COREFX_NET_AD_USERNAME Define standalone server remote endpoint: * COREFX_WINDOWSSERVER_HTTPHOST * COREFX_NET_SERVER_PASSWORD * COREFX_NET_SERVER_USERNAME Commit migrated from dotnet/corefx@658ec73

This PR is a follow-up to PR dotnet/corefx#36329. In that previous PR, I changed to use the GSS_C_NT_HOSTBASED_SERVICE format for the SPN. That requires using a '@' separator character instead of a "/". I had assumed since System.Data.SqlClient was including the source files from Common for SafeDeleteNegoContext.cs that it would have the corresponding change to use the proper separator character. However, it appears that the file was only included to get the project building. It was using a separate file to calculate the SPN. The customer requiring these fixes reported that Kerberos was working properly now for HttpClient and NegotiateStream. But SqlClient was still broken. This PR completes the fix so that SqlClient will work properly with Kerberos especially in multiple domain/realm environments. This fix was manually tested in the enterprise scenario test lab. Commit migrated from dotnet/corefx@78c4c8b

davidsh added tenet-compatibility Incompatibility with previous versions or .NET Framework area-System.Net.Security labels Mar 24, 2019

davidsh added this to the 3.0 milestone Mar 24, 2019

davidsh self-assigned this Mar 24, 2019

davidsh force-pushed the kerbntlm_noretry branch 2 times, most recently from 0b989eb to 28b8d58 Compare March 24, 2019 22:17

davidsh requested review from stephentoub and wfurt March 25, 2019 13:30

stephentoub reviewed Mar 25, 2019

View reviewed changes

wfurt approved these changes Mar 25, 2019

View reviewed changes

Address PR feedback

c00ed74

rmkerr reviewed Mar 25, 2019

View reviewed changes

rmkerr approved these changes Mar 25, 2019

View reviewed changes

davidsh merged commit 658ec73 into dotnet:master Mar 25, 2019

davidsh deleted the kerbntlm_noretry branch March 25, 2019 18:39

davidsh mentioned this pull request Mar 31, 2019

Fix SqlClient Kerberos on Linux #36513

Merged

davidsh mentioned this pull request Aug 7, 2019

[release/2.1] Port Kerberos auth fixes to 2.1 branch #40109

Merged

Conversation

davidsh commented Mar 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidsh commented Mar 24, 2019

Uh oh!

azure-pipelines bot commented Mar 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wfurt left a comment

Choose a reason for hiding this comment

Uh oh!

davidsh commented Mar 25, 2019

Uh oh!

azure-pipelines bot commented Mar 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidsh Mar 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmkerr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davidsh commented Mar 24, 2019 •

edited

Loading

davidsh Mar 25, 2019 •

edited

Loading