Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Implement server-side of NegotiateStream on Unix - #36827

Merged
Tratcher merged 17 commits into
masterfrom
tratcher/kerb
Apr 19, 2019
Merged

Tratcher merged 17 commits into
masterfrom
tratcher/kerb

Conversation

@Tratcher

Copy link
Copy Markdown
Member

#8221 @davidsh I'm starting this draft PR to show you the scope of the changes needed for this feature. I'll follow up with you offline.

The current code works end-to-end using a Windows client to authenticate via Kerberos to a Linux server that has been linked to the Windows domain. Ignore the tests, they were there there for manual verification and debugging. We'll need to discuss what's possible to automate with the given infrastructure.

We also need to work out what variants need to be supported and tested, and which should be blocked with explicit checks and error messages? E.g. NTLM, non-default credentials, Mac, etc.. cc: @blowdart

I'm not including here an new public APIs for AspNetCore, nor have I tested the HTTP scenario, but that should follow easily from this foundational work.

@stephentoub

Copy link
Copy Markdown
Member

to show you the scope of the changes needed for this feature

It's great to see it's not that much. Thanks for working on this.

@Tratcher

Copy link
Copy Markdown
Member Author

I've added a test to be that should work in @davidsh's preconfigured environment. Here is the client that the test expects to be available: https://github.com/Tratcher/NegotiateClient

I'll do another pass to clean up the product code.

@davidsh

davidsh commented Apr 16, 2019

Copy link
Copy Markdown
Contributor

Here is the client that the test expects to be available: https://github.com/Tratcher/NegotiateClient

Where is the NegotiateStream server code that should be run on a Linux VM?

@Tratcher

Copy link
Copy Markdown
Member Author

@davidsh it's inside the test. The test expects to be running on whichever server you want to test (windows, linux, etc.) The test is not OS specific.
https://github.com/dotnet/corefx/pull/36827/files#diff-7f4d72c7dab179e38c761bf82e1836acR184

Comment thread src/System.Net.Security/tests/FunctionalTests/NegotiateStreamKerberosTest.cs Outdated
Comment thread src/System.Net.Security/src/System/Net/Security/NegotiateStreamPal.Unix.cs Outdated
Comment thread src/System.Net.Security/tests/FunctionalTests/NegotiateStreamKerberosTest.cs Outdated
@Tratcher
Tratcher marked this pull request as ready for review April 17, 2019 17:12
Comment thread src/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs Outdated
Comment thread src/System.Net.Security/tests/FunctionalTests/NegotiateStreamKerberosTest.cs Outdated
Comment thread src/Common/tests/System/Net/Configuration.Security.cs
Comment thread src/Common/src/System/Net/NTAuthentication.Common.cs Outdated
Comment thread src/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
Comment thread src/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs Outdated
Comment thread src/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
ref byte[] resultBlob,
ref ContextFlagsPal contextFlags)
{
throw new PlatformNotSupportedException(SR.net_nego_server_not_supported);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we delete the associated string from the .resx, or is it still needed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it can be deleted.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still referenced in a few places.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the places it's used. We can leave the string in the .resx for now. But I think we should be able to eventually fix the other PlatformNotSupportedException's later and remove the string.

@davidsh davidsh modified the milestones: Future, 3.0 Apr 19, 2019
@davidsh

davidsh commented Apr 19, 2019

Copy link
Copy Markdown
Contributor

I tried out this PR change in my enterprise test environment. I was able to confirm that the NegotiateStream server scenario works (once all the required Linux keytab files are generated with the right SPN's etc.).

I think it is important that this PR get in for .NET Core 3.0 Preview 5. It's ok that the tests are still basically disabled. They will be reworked a lot over the coming weeks etc.

@davidsh

davidsh commented Apr 19, 2019

Copy link
Copy Markdown
Contributor

cc: @karelz

@karelz

karelz commented Apr 19, 2019

Copy link
Copy Markdown
Member

Just for my understanding: How is it going to help us in Preview 5 vs. Preview 6? Do we expect more validation via some existing code paths?
(BTW: If it gets merged on Friday, it will flow into Preview 5 automatically)

@Tratcher

Copy link
Copy Markdown
Member Author

Preview5 gives us a much earlier chance for external feedback. E.g. I'll ask potential ASP.NET Core Kerberos customers to test NegotiateStream in their environments to make sure the fundamentals are working. That should avoid any surprises when we give them the ASP.NET layer to test later.

@Tratcher
Tratcher merged commit 0a9a366 into master Apr 19, 2019
@Tratcher
Tratcher deleted the tratcher/kerb branch April 19, 2019 05:27
@karelz

karelz commented Apr 19, 2019

Copy link
Copy Markdown
Member

OK, if there is a way for customers to somehow exercise it (through ASP.NET Core scenarios), then it totally makes sense. I wasn't aware there are settings that can expose the path which was not working at all until 30 seconds ago :)

@Tratcher
Tratcher restored the tratcher/kerb branch August 6, 2019 18:33
@jkotas
jkotas deleted the tratcher/kerb branch August 19, 2019 12:26
davidsh pushed a commit to davidsh/corefx that referenced this pull request Nov 10, 2019
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
davidsh pushed a commit to davidsh/corefx that referenced this pull request Nov 10, 2019
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
davidsh added a commit that referenced this pull request Nov 11, 2019
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:
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
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants