Fix NegotitateStream (server) on Linux for NTLM #42522
Conversation
This PR is a follow up to PR dotnet#36827 which added support for Linux server-side GSS-API (AcceptSecContext). This enabled NegotitateStream AuthenticateAsServer* support. It also provided support for ASP.NET Core to allow Kestrel server to have Negotiate authentication on Linux. This PR fixes some problems with Negotiate (SPNEGO) fallback from Kerberos to NTLM. Notably it passes in a correct GSS Acceptor credential so that fallback will work correctly. As part of fixing that, I noticed some other problems with returning the user-identity when NTLM is used. This was tested in a separate enterprise testing environment that I have created. It builds on technologies that we have started using like docker containers and Azure pipelines (e.g. HttpStress). The environment is currently here: https://dev.azure.com/systemnetncl/Enterprise%20Testing. The extra Kerberos tests and container support is here: https://github.com/davidsh/networkingtests When the repo merge is completed, I will work with the infra team to see what things can be merged back into the main repo/CI pipeline and migrate the test sources to an appropriate place in the new repo. Contributes to #10041 Contributes to #24707 Contributes to #30150
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
|
Have you been able to verify that the changes fix the failures in the new tests? It seems that the docker infrastructure you linked to is pinned to the 3.0.100 sdk. |
Yes. The docker infrastructure is pinned to 3.0 SDK. But that was just a template as I was working out the mechanics of getting it set up. You might notice that the scripts can copy any files from the /patch directory into the container's shared framework. # Copy and patch files if present
echo "Patching Microsoft.NETCore.App/3.0.0 files"
ls -l /patch
cp -r /patch/. /usr/share/dotnet/shared/Microsoft.NETCore.App/3.0.0I built the 'master' branch on my machine and copied that output into a volume mount for the container. So, when the container runs on my machine, it uses the latest 'master' from my PR. Once the new repo is set up and the pipeline is modified for new repo, it will be more automated. |
src/Common/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.cs
Show resolved
Hide resolved
src/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
|
This was merged with a compile error in the all configurations leg:
It seems like the implicit cast from ROS to [] is not there for one of the New pull requests/builds are failing with that error: |
| @@ -78,6 +88,12 @@ protected override void Dispose(bool disposing) | |||
| _targetName.Dispose(); | |||
| _targetName = null; | |||
| } | |||
|
|
|||
| if (_acceptorCredential != null) | |||
There was a problem hiding this comment.
I know you've already merged this and there's existing code with this pattern, but this isn't thread safe. If Dispose is called on two threads at the same time, you can get a null reference exception. You could simply replace this with:
_acceptorCredential?.Dispose();
_acceptorCredential = null;The generated IL is thread safe when using the ?. operator.
This PR is a follow up to PR #36827 which added support for Linux server-side
GSS-API (AcceptSecContext). This enabled NegotitateStream AuthenticateAsServer*
support. It also provided support for ASP.NET Core to allow Kestrel server to have
Negotiate authentication on Linux.
This PR fixes some problems with Negotiate (SPNEGO) fallback from Kerberos to NTLM.
Notably it passes in a correct GSS Acceptor credential so that fallback will work
correctly. As part of fixing that, I noticed some other problems with returning the
user-identity when NTLM is used.
This was tested in a separate enterprise testing environment that I have created.
It builds on technologies that we have started using like docker containers and Azure
pipelines (e.g. HttpStress). The environment is currently here. The extra Kerberos tests
and container support is here.
When the repo merge is completed, I will work with the infra team to see what things
can be merged back into the main repo/CI pipeline and migrate the test sources to an
appropriate place in the new repo.
Contributes to #10041
Contributes to #24707
Contributes to #30150