Skip to content

Commit

Permalink
Rework Linux/Kerberos native interop layer
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@davidsh
davidsh authored and David Shulman committed Jun 9, 2019
1 parent a5eec2c commit a217605
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 78 deletions.
18 changes: 16 additions & 2 deletions src/Common/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ internal static extern Status ImportUserName(
int inputNameByteCount,
out SafeGssNameHandle outputName);

[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_ImportTargetName")]
internal static extern Status ImportTargetName(
[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_ImportPrincipalName")]
internal static extern Status ImportPrincipalName(
out Status minorStatus,
string inputName,
int inputNameByteCount,
Expand Down Expand Up @@ -69,6 +69,20 @@ internal static extern Status ReleaseCred(
ref IntPtr credHandle);

[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_InitSecContext")]
internal static extern Status InitSecContext(
out Status minorStatus,
SafeGssCredHandle initiatorCredHandle,
ref SafeGssContextHandle contextHandle,
bool isNtlmOnly,
SafeGssNameHandle targetName,
uint reqFlags,
byte[] inputBytes,
int inputLength,
ref GssBuffer token,
out uint retFlags,
out bool isNtlmUsed);

[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_InitSecContextEx")]
internal static extern Status InitSecContext(
out Status minorStatus,
SafeGssCredHandle initiatorCredHandle,
Expand Down
2 changes: 1 addition & 1 deletion src/Common/src/Microsoft/Win32/SafeHandles/GssSafeHandles.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public static SafeGssNameHandle CreateTarget(string name)
Debug.Assert(!string.IsNullOrEmpty(name), "Invalid target name passed to SafeGssNameHandle create");
SafeGssNameHandle retHandle;
Interop.NetSecurityNative.Status minorStatus;
Interop.NetSecurityNative.Status status = Interop.NetSecurityNative.ImportTargetName(
Interop.NetSecurityNative.Status status = Interop.NetSecurityNative.ImportPrincipalName(
out minorStatus, name, Encoding.UTF8.GetByteCount(name), out retHandle);

if (status != Interop.NetSecurityNative.Status.GSS_S_COMPLETE)
Expand Down
62 changes: 37 additions & 25 deletions src/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,6 @@ private static bool GssInitSecurityContext(
out uint outFlags,
out bool isNtlmUsed)
{
// If a TLS channel binding token (cbt) is available then get the pointer
// to the application specific data.
IntPtr cbtAppData = IntPtr.Zero;
int cbtAppDataSize = 0;
if (channelBinding != null)
{
int appDataOffset = Marshal.SizeOf<SecChannelBindings>();
Debug.Assert(appDataOffset < channelBinding.Size);
cbtAppData = channelBinding.DangerousGetHandle() + appDataOffset;
cbtAppDataSize = channelBinding.Size - appDataOffset;
}

outputBuffer = null;
outFlags = 0;

Expand All @@ -138,19 +126,43 @@ private static bool GssInitSecurityContext(
try
{
Interop.NetSecurityNative.Status minorStatus;
status = Interop.NetSecurityNative.InitSecContext(out minorStatus,
credential,
ref context,
isNtlm,
cbtAppData,
cbtAppDataSize,
targetName,
(uint)inFlags,
buffer,
(buffer == null) ? 0 : buffer.Length,
ref token,
out outFlags,
out isNtlmUsed);

if (channelBinding != null)
{
// If a TLS channel binding token (cbt) is available then get the pointer
// to the application specific data.
int appDataOffset = Marshal.SizeOf<SecChannelBindings>();
Debug.Assert(appDataOffset < channelBinding.Size);
IntPtr cbtAppData = channelBinding.DangerousGetHandle() + appDataOffset;
int cbtAppDataSize = channelBinding.Size - appDataOffset;
status = Interop.NetSecurityNative.InitSecContext(out minorStatus,
credential,
ref context,
isNtlm,
cbtAppData,
cbtAppDataSize,
targetName,
(uint)inFlags,
buffer,
(buffer == null) ? 0 : buffer.Length,
ref token,
out outFlags,
out isNtlmUsed);
}
else
{
status = Interop.NetSecurityNative.InitSecContext(out minorStatus,
credential,
ref context,
isNtlm,
targetName,
(uint)inFlags,
buffer,
(buffer == null) ? 0 : buffer.Length,
ref token,
out outFlags,
out isNtlmUsed);
}

if ((status != Interop.NetSecurityNative.Status.GSS_S_COMPLETE) &&
(status != Interop.NetSecurityNative.Status.GSS_S_CONTINUE_NEEDED))
Expand Down
5 changes: 1 addition & 4 deletions src/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,7 @@ public SafeDeleteNegoContext(SafeFreeNegoCredentials credential, string targetNa
{
try
{
// 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('/', '@'));
_targetName = SafeGssNameHandle.CreateTarget(targetName);
}
catch
{
Expand Down
69 changes: 62 additions & 7 deletions src/Native/Unix/System.Net.Security.Native/pal_gssapi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,33 +144,88 @@ NetSecurityNative_ImportUserName(uint32_t* minorStatus, char* inputName, uint32_
return gss_import_name(minorStatus, &inputNameBuffer, GSS_C_NT_USER_NAME, outputName);
}

uint32_t NetSecurityNative_ImportTargetName(uint32_t* minorStatus,
char* inputName,
uint32_t inputNameLen,
GssName** outputName)
uint32_t NetSecurityNative_ImportPrincipalName(uint32_t* minorStatus,
char* inputName,
uint32_t inputNameLen,
GssName** outputName)
{
assert(minorStatus != NULL);
assert(inputName != NULL);
assert(outputName != NULL);
assert(*outputName == NULL);

// Principal name will usually be in the form SERVICE/HOST. But SPNEGO protocol prefers
// GSS_C_NT_HOSTBASED_SERVICE format. That format uses '@' separator instead of '/' between
// service name and host name. So convert input string into that format.
char* ptrSlash = memchr(inputName, '/', inputNameLen);
char* inputNameCopy = NULL;
if (ptrSlash != NULL)
{
inputNameCopy = (char*) malloc(inputNameLen);
if (inputNameCopy != NULL)
{
memcpy(inputNameCopy, inputName, inputNameLen);
inputNameCopy[ptrSlash - inputName] = '@';
inputName = inputNameCopy;
}
else
{
*minorStatus = 0;
return GSS_S_BAD_NAME;
}
}

GssBuffer inputNameBuffer = {.length = inputNameLen, .value = inputName};
return gss_import_name(minorStatus, &inputNameBuffer, GSS_C_NT_HOSTBASED_SERVICE, outputName);
uint32_t result = gss_import_name(minorStatus, &inputNameBuffer, GSS_C_NT_HOSTBASED_SERVICE, outputName);

if (inputNameCopy != NULL)
{
free(inputNameCopy);
}

return result;
}

uint32_t NetSecurityNative_InitSecContext(uint32_t* minorStatus,
GssCredId* claimantCredHandle,
GssCtxId** contextHandle,
uint32_t isNtlm,
void* cbt,
int32_t cbtSize,
GssName* targetName,
uint32_t reqFlags,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags,
int32_t* isNtlmUsed)
{
return NetSecurityNative_InitSecContextEx(minorStatus,
claimantCredHandle,
contextHandle,
isNtlm,
NULL,
0,
targetName,
reqFlags,
inputBytes,
inputLength,
outBuffer,
retFlags,
isNtlmUsed);
}

uint32_t NetSecurityNative_InitSecContextEx(uint32_t* minorStatus,
GssCredId* claimantCredHandle,
GssCtxId** contextHandle,
uint32_t isNtlm,
void* cbt,
int32_t cbtSize,
GssName* targetName,
uint32_t reqFlags,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags,
int32_t* isNtlmUsed)
{
assert(minorStatus != NULL);
assert(contextHandle != NULL);
Expand Down
18 changes: 15 additions & 3 deletions src/Native/Unix/System.Net.Security.Native/pal_gssapi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ NetSecurityNative_ImportUserName(uint32_t* minorStatus, char* inputName, uint32_
/*
Shims the gss_import_name method with nametype = GSS_C_NT_USER_NAME.
*/
DLLEXPORT uint32_t NetSecurityNative_ImportTargetName(uint32_t* minorStatus,
DLLEXPORT uint32_t NetSecurityNative_ImportPrincipalName(uint32_t* minorStatus,
char* inputName,
uint32_t inputNameLen,
GssName** outputName);
Expand Down Expand Up @@ -107,8 +107,6 @@ DLLEXPORT uint32_t NetSecurityNative_InitSecContext(uint32_t* minorStatus,
GssCredId* claimantCredHandle,
GssCtxId** contextHandle,
uint32_t isNtlm,
void* cbt,
int32_t cbtSize,
GssName* targetName,
uint32_t reqFlags,
uint8_t* inputBytes,
Expand All @@ -117,6 +115,20 @@ DLLEXPORT uint32_t NetSecurityNative_InitSecContext(uint32_t* minorStatus,
uint32_t* retFlags,
int32_t* isNtlmUsed);

DLLEXPORT uint32_t NetSecurityNative_InitSecContextEx(uint32_t* minorStatus,
GssCredId* claimantCredHandle,
GssCtxId** contextHandle,
uint32_t isNtlm,
void* cbt,
int32_t cbtSize,
GssName* targetName,
uint32_t reqFlags,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags,
int32_t* isNtlmUsed);

/*
Shims the gss_accept_sec_context method.
*/
Expand Down
2 changes: 0 additions & 2 deletions src/System.Data.SqlClient/src/System.Data.SqlClient.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,6 @@
<Compile Include="System\Data\SqlClient\SNI\SNINpHandle.cs" />
<Compile Include="System\Data\SqlClient\SNI\SNIPacket.cs" />
<Compile Include="System\Data\SqlClient\SNI\SNIProxy.cs" />
<Compile Include="System\Data\SqlClient\SNI\SNIProxy.Windows.cs" Condition="'$(TargetsNetCoreApp)' != 'true' OR '$(TargetsUnix)' != 'true'" />
<Compile Include="System\Data\SqlClient\SNI\SNIProxy.Unix.cs" Condition="'$(TargetsNetCoreApp)' == 'true' AND '$(TargetsUnix)' == 'true'" />
<Compile Include="System\Data\SqlClient\SNI\SNITcpHandle.cs" />
<Compile Include="System\Data\SqlClient\SNI\SslOverTdsStream.cs" />
<Compile Include="System\Data\SqlClient\SNI\SNICommon.cs" />
Expand Down
16 changes: 0 additions & 16 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.Unix.cs
View file

This file was deleted.

16 changes: 0 additions & 16 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.Windows.cs
View file

This file was deleted.

4 changes: 2 additions & 2 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIProxy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/";

public static readonly SNIProxy Singleton = new SNIProxy();

Expand Down Expand Up @@ -341,7 +341,7 @@ private static byte[] GetSqlServerSPN(string hostNameOrAddress, string portOrIns
// If the DNS lookup failed, then resort to using the user provided hostname to construct the SPN.
fullyQualifiedDomainName = hostEntry?.HostName ?? hostNameOrAddress;
}
string serverSpn = SqlServerSpnHeader + SpnServiceHostSeparator + fullyQualifiedDomainName;
string serverSpn = SqlServerSpnHeader + fullyQualifiedDomainName;
if (!string.IsNullOrWhiteSpace(portOrInstanceName))
{
serverSpn += ":" + portOrInstanceName;
Expand Down

0 comments on commit a217605

Please sign in to comment.