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

Rework Linux/Kerberos native interop layer #38377

Merged
merged 1 commit into from
Jun 11, 2019

Conversation

davidsh
Copy link
Contributor

@davidsh davidsh commented Jun 9, 2019

The latest changes to the System.Net.Security.Native shim layer fixed a lot of important
bugs for Linux Kerberos usage. But this created a new problem since SqlClient ships
in out-of-band NuGet packages separate from the .NET Core runtime. SqlClient builds
out of the CoreFx repo and uses the common source includes for Kerberos authentication.
This created an unexpected dependency on the System.Net.Security.Native shim layer.
The recent changes to these API signatures caused problems with different combinations
of SqlClient NuGet packages and .NET Core 2.x versus .NET Core 3.0.

After discussion with the SqlClient team, we decided to rework the changes to these
native APIs so that they would remain compatible across all .NET Core versions.

Long-term, the plan is to implement #36896 to expose a Kerberos API in .NET Core which
could be used by SqlClient and other consumers.

Closes #37183
Closes #25205

The latest changes to the System.Net.Security.Native shim layer fixed a lot of important
bugs for Linux Kerberos usage. But this created a new problem since SqlClient ships
in out-of-band NuGet packages separate from the .NET Core runtime. SqlClient builds
out of the CoreFx repo and uses the common source includes for Kerberos authentication.
This created an unexpected dependency on the System.Net.Security.Native shim layer.
The recent changes to these API signatures caused problems with different combinations
of SqlClient NuGet packages and .NET Core 2.x versus .NET Core 3.0.

After discussion with the SqlClient team, we decided to rework the changes to these
native APIs so that they would remain compatible across all .NET Core versions.

Long-term, the plan is to implement #36896 to expose a Kerberos API in .NET Core which
could be used by SqlClient and other consumers.

Closes #37183
Closes #25205
@davidsh davidsh added this to the 3.0 milestone Jun 9, 2019
@davidsh davidsh self-assigned this Jun 9, 2019
@davidsh davidsh added the tenet-compatibility Incompatibility with previous versions or .NET Framework label Jun 9, 2019
@davidsh
Copy link
Contributor Author

davidsh commented Jun 9, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@davidsh davidsh requested review from stephentoub and a team June 9, 2019 13:07
@jkotas
Copy link
Member

jkotas commented Jun 9, 2019

How are we going to test that all possible combinations actually work?

I am wondering whether it would it be better to keep the verbatim 2.1 implementation of the shim APIs, without any changes and big comment at the top of the file that this is for SqlClient compat only and can be deleted once this compat hack does not need to be maintained anymore. And then have the live version of the shim APIs that SqlClient would not have dependency on anymore and that we can evolved without worrying about compatibility with SqlClient.

@davidsh
Copy link
Contributor Author

davidsh commented Jun 9, 2019

How are we going to test that all possible combinations actually work?

I've been manually testing the combinations to make sure things work ok.

I am wondering whether it would it be better to keep the verbatim 2.1 implementation of the shim APIs,

The original .NET Core 2.1 implementation of System.Net.Security.Native has bugs that we had to fix in .NET Core 3.0 master such as Negotiate Kerberos to NTLM fallback and channel binding token (CBT).
We plan to port some of these functional fixes in master to the release/2.1 branch in order to unblock some customers that are depending on the .NET Core 2.1 release (which is LTS). But these fixes and this .NET Core 3.0 master branch PR creates a compatible layer since we will keep the API signatures the same as .NET Core 2.1 for the APIs that SqlClient is using from this layer.

@@ -22,7 +22,7 @@ internal partial class SNIProxy
{
private const int DefaultSqlServerPort = 1433;
private const int DefaultSqlServerDacPort = 1434;
private const string SqlServerSpnHeader = "MSSQLSvc";
private const string SqlServerSpnHeader = "MSSQLSvc/";
Copy link
Member

Choose a reason for hiding this comment

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

should we use '@' here to avoid need to copy & replace later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "@" format is only for Linux. The Windows format always use "/". The current code (before this PR) has different files for Windows vs. Linux. This PR undoes that to go back to what it was before I made those change a month ago.

The problem is that this managed code here in CoreFx for SqlClient builds as separate package. And that package can run against an older .NET Core 2.1 runtime. And that runtime which uses System.Net.Security.Native doesn't understand the "@" format. So, right now the latest pre-release packages of System.Data.SqlClient (based on this master branch, .NET Core 3.0) is broken. So, this PR is trying to keep the managed layer the same as what it was in .NET Core 2.1.

This is why the logic for transformation of "/" to "@" moves to the C layer which doesn't build as a separate (out-of-band) package.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.

@davidsh davidsh merged commit 7f920b2 into dotnet:master Jun 11, 2019
@davidsh davidsh deleted the refactor_kerb branch June 11, 2019 02:06
davidsh pushed a commit to davidsh/corefx that referenced this pull request Aug 7, 2019
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.
danmoseley pushed a commit that referenced this pull request Aug 8, 2019
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.
wtgodbe pushed a commit that referenced this pull request Aug 8, 2019
* 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.
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
The latest changes to the System.Net.Security.Native shim layer fixed a lot of important
bugs for Linux Kerberos usage. But this created a new problem since SqlClient ships
in out-of-band NuGet packages separate from the .NET Core runtime. SqlClient builds
out of the CoreFx repo and uses the common source includes for Kerberos authentication.
This created an unexpected dependency on the System.Net.Security.Native shim layer.
The recent changes to these API signatures caused problems with different combinations
of SqlClient NuGet packages and .NET Core 2.x versus .NET Core 3.0.

After discussion with the SqlClient team, we decided to rework the changes to these
native APIs so that they would remain compatible across all .NET Core versions.

Long-term, the plan is to implement dotnet/corefx#36896 to expose a Kerberos API in .NET Core which
could be used by SqlClient and other consumers.

Closes dotnet/corefx#37183
Closes dotnet/corefx#25205

Commit migrated from dotnet/corefx@7f920b2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Security tenet-compatibility Incompatibility with previous versions or .NET Framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Out-of-band System.Data.SqlClient depends on inbox native shims System.Data.SqlClient packaging
3 participants