Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Fix NegotitateStream (server) on Linux for NTLM #42522

Merged
merged 2 commits into from
Nov 11, 2019
Merged

Fix NegotitateStream (server) on Linux for NTLM #42522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01e6750
Fix NegotitateStream (server) on Linux for NTLM
Oct 21, 2019
f079bf9
Address PR feedback
Nov 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/Common/src/Interop/Unix/System.Net.Security.Native/Interop.GssBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal static partial class Interop
internal static partial class NetSecurityNative
{
[StructLayout(LayoutKind.Sequential)]
internal unsafe struct GssBuffer : IDisposable
internal struct GssBuffer : IDisposable
{
internal ulong _length;
internal IntPtr _data;
Expand Down Expand Up @@ -52,6 +52,10 @@ internal byte[] ToByteArray()
return destination;
}

internal unsafe ReadOnlySpan<byte> Span => (_data != IntPtr.Zero && _length != 0) ?
new ReadOnlySpan<byte>(_data.ToPointer(), checked((int)_length)) :
default;

public void Dispose()
{
if (_data != IntPtr.Zero)
Expand Down
9 changes: 8 additions & 1 deletion src/Common/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ internal static extern Status ReleaseName(
out Status minorStatus,
ref IntPtr inputName);

[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_AcquireAcceptorCred")]
internal static extern Status AcquireAcceptorCred(
out Status minorStatus,
out SafeGssCredHandle outputCredHandle);

[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_InitiateCredSpNego")]
internal static extern Status InitiateCredSpNego(
out Status minorStatus,
Expand Down Expand Up @@ -101,11 +106,13 @@ internal static extern Status InitSecContext(
[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_AcceptSecContext")]
internal static extern Status AcceptSecContext(
out Status minorStatus,
SafeGssCredHandle acceptorCredHandle,
ref SafeGssContextHandle acceptContextHandle,
byte[] inputBytes,
int inputLength,
ref GssBuffer token,
out uint retFlags);
out uint retFlags,
out bool isNtlmUsed);
davidsh marked this conversation as resolved.
Show resolved Hide resolved

[DllImport(Interop.Libraries.NetSecurityNative, EntryPoint="NetSecurityNative_DeleteSecContext")]
internal static extern Status DeleteSecContext(
Expand Down
17 changes: 16 additions & 1 deletion src/Common/src/Microsoft/Win32/SafeHandles/GssSafeHandles.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,21 @@ internal class SafeGssCredHandle : SafeHandle
{
private static readonly Lazy<bool> s_IsNtlmInstalled = new Lazy<bool>(InitIsNtlmInstalled);

public static SafeGssCredHandle CreateAcceptor()
{
SafeGssCredHandle retHandle = null;
Interop.NetSecurityNative.Status status;
Interop.NetSecurityNative.Status minorStatus;

status = Interop.NetSecurityNative.AcquireAcceptorCred(out minorStatus, out retHandle);
if (status != Interop.NetSecurityNative.Status.GSS_S_COMPLETE)
{
throw new Interop.NetSecurityNative.GssApiException(status, minorStatus);
}

return retHandle;
}

/// <summary>
/// returns the handle for the given credentials.
/// The method returns an invalid handle if the username is null or empty.
Expand Down Expand Up @@ -110,7 +125,7 @@ public static SafeGssCredHandle Create(string username, string password, bool is
if (status != Interop.NetSecurityNative.Status.GSS_S_COMPLETE)
{
retHandle.Dispose();
throw new Interop.NetSecurityNative.GssApiException(status, minorStatus, null);
throw new Interop.NetSecurityNative.GssApiException(status, minorStatus);
}
}

Expand Down
31 changes: 30 additions & 1 deletion src/Common/src/System/Net/ContextFlagsAdapterPal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ public ContextFlagMapping(Interop.NetSecurityNative.GssFlags gssFlag, ContextFla
private static readonly ContextFlagMapping[] s_contextFlagMapping = new[]
{
new ContextFlagMapping(Interop.NetSecurityNative.GssFlags.GSS_C_CONF_FLAG, ContextFlagsPal.Confidentiality),
new ContextFlagMapping(Interop.NetSecurityNative.GssFlags.GSS_C_IDENTIFY_FLAG, ContextFlagsPal.InitIdentify),
new ContextFlagMapping(Interop.NetSecurityNative.GssFlags.GSS_C_MUTUAL_FLAG, ContextFlagsPal.MutualAuth),
new ContextFlagMapping(Interop.NetSecurityNative.GssFlags.GSS_C_REPLAY_FLAG, ContextFlagsPal.ReplayDetect),
new ContextFlagMapping(Interop.NetSecurityNative.GssFlags.GSS_C_SEQUENCE_FLAG, ContextFlagsPal.SequenceDetect),
Expand All @@ -35,6 +34,20 @@ internal static ContextFlagsPal GetContextFlagsPalFromInterop(Interop.NetSecurit
{
ContextFlagsPal flags = ContextFlagsPal.None;

// GSS_C_IDENTIFY_FLAG is handled separately as its value can either be AcceptIdentify (used by server) or InitIdentify (used by client)
if ((gssFlags & Interop.NetSecurityNative.GssFlags.GSS_C_IDENTIFY_FLAG) != 0)
{
flags |= isServer ? ContextFlagsPal.AcceptIdentify : ContextFlagsPal.InitIdentify;
}

foreach (ContextFlagMapping mapping in s_contextFlagMapping)
{
if ((gssFlags & mapping.GssFlags) == mapping.GssFlags)
{
flags |= mapping.ContextFlag;
}
}

// GSS_C_INTEG_FLAG is handled separately as its value can either be AcceptIntegrity (used by server) or InitIntegrity (used by client)
if ((gssFlags & Interop.NetSecurityNative.GssFlags.GSS_C_INTEG_FLAG) != 0)
{
Expand All @@ -56,6 +69,22 @@ internal static Interop.NetSecurityNative.GssFlags GetInteropFromContextFlagsPal
{
Interop.NetSecurityNative.GssFlags gssFlags = 0;

// GSS_C_IDENTIFY_FLAG is set if either AcceptIdentify (used by server) or InitIdentify (used by client) is set
if (isServer)
{
if ((flags & ContextFlagsPal.AcceptIdentify) != 0)
{
gssFlags |= Interop.NetSecurityNative.GssFlags.GSS_C_IDENTIFY_FLAG;
}
}
else
{
if ((flags & ContextFlagsPal.InitIdentify) != 0)
{
gssFlags |= Interop.NetSecurityNative.GssFlags.GSS_C_IDENTIFY_FLAG;
}
}

// GSS_C_INTEG_FLAG is set if either AcceptIntegrity (used by server) or InitIntegrity (used by client) is set
if (isServer)
{
Expand Down
47 changes: 38 additions & 9 deletions src/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,14 @@ private static bool GssInitSecurityContext(

private static bool GssAcceptSecurityContext(
ref SafeGssContextHandle context,
SafeGssCredHandle credential,
byte[] buffer,
out byte[] outputBuffer,
out uint outFlags)
out uint outFlags,
out bool isNtlmUsed)
{
Debug.Assert(credential != null);

bool newContext = false;
if (context == null)
{
Expand All @@ -205,11 +209,13 @@ private static bool GssAcceptSecurityContext(
{
Interop.NetSecurityNative.Status minorStatus;
status = Interop.NetSecurityNative.AcceptSecContext(out minorStatus,
credential,
ref context,
buffer,
buffer?.Length ?? 0,
ref token,
out outFlags);
out outFlags,
out isNtlmUsed);

if ((status != Interop.NetSecurityNative.Status.GSS_S_COMPLETE) &&
(status != Interop.NetSecurityNative.Status.GSS_S_CONTINUE_NEEDED))
Expand Down Expand Up @@ -249,7 +255,15 @@ Interop.NetSecurityNative.Status status
throw new Interop.NetSecurityNative.GssApiException(status, minorStatus);
}

return Encoding.UTF8.GetString(token.ToByteArray());
ReadOnlySpan<byte> tokenBytes = token.Span;
int length = tokenBytes.Length;
if (length > 0 && tokenBytes[length - 1] == '\0')
{
// Some GSS-API providers (gss-ntlmssp) include the terminating null with strings, so skip that.
tokenBytes = tokenBytes.Slice(0, length - 1);
}

return Encoding.UTF8.GetString(tokenBytes);
}
finally
{
Expand All @@ -274,7 +288,7 @@ private static SecurityStatusPal EstablishSecurityContext(
if (NetEventSource.IsEnabled)
{
string protocol = isNtlmOnly ? "NTLM" : "SPNEGO";
NetEventSource.Info(null, $"requested protocol = {protocol}, target = {targetName}");
NetEventSource.Info(context, $"requested protocol = {protocol}, target = {targetName}");
}

context = new SafeDeleteNegoContext(credential, targetName);
Expand Down Expand Up @@ -305,7 +319,7 @@ private static SecurityStatusPal EstablishSecurityContext(
if (NetEventSource.IsEnabled)
{
string protocol = isNtlmOnly ? "NTLM" : isNtlmUsed ? "SPNEGO-NTLM" : "SPNEGO-Kerberos";
NetEventSource.Info(null, $"actual protocol = {protocol}");
NetEventSource.Info(context, $"actual protocol = {protocol}");
}

// Populate protocol used for authentication
Expand Down Expand Up @@ -396,9 +410,11 @@ internal static SecurityStatusPal AcceptSecurityContext(
SafeGssContextHandle contextHandle = negoContext.GssContext;
bool done = GssAcceptSecurityContext(
ref contextHandle,
negoContext.AcceptorCredential,
incomingBlob,
out resultBlob,
out uint outputFlags);
out uint outputFlags,
out bool isNtlmUsed);

Debug.Assert(resultBlob != null, "Unexpected null buffer returned by GssApi");
Debug.Assert(negoContext.GssContext == null || contextHandle == negoContext.GssContext);
Expand All @@ -413,9 +429,22 @@ internal static SecurityStatusPal AcceptSecurityContext(
contextFlags = ContextFlagsAdapterPal.GetContextFlagsPalFromInterop(
(Interop.NetSecurityNative.GssFlags)outputFlags, isServer: true);

SecurityStatusPalErrorCode errorCode = done ?
(negoContext.IsNtlmUsed && resultBlob.Length > 0 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.CompleteNeeded) :
SecurityStatusPalErrorCode.ContinueNeeded;
SecurityStatusPalErrorCode errorCode;
if (done)
{
if (NetEventSource.IsEnabled)
{
string protocol = isNtlmUsed ? "SPNEGO-NTLM" : "SPNEGO-Kerberos";
NetEventSource.Info(securityContext, $"AcceptSecurityContext: actual protocol = {protocol}");
}

negoContext.SetAuthenticationPackage(isNtlmUsed);
errorCode = (isNtlmUsed && resultBlob.Length > 0) ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.CompleteNeeded;
}
else
{
errorCode = SecurityStatusPalErrorCode.ContinueNeeded;
}

return new SecurityStatusPal(errorCode);
}
Expand Down
16 changes: 16 additions & 0 deletions src/Common/src/System/Net/Security/Unix/SafeDeleteNegoContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,20 @@ namespace System.Net.Security
{
internal sealed class SafeDeleteNegoContext : SafeDeleteContext
{
private SafeGssCredHandle _acceptorCredential;
private SafeGssNameHandle _targetName;
private SafeGssContextHandle _context;
private bool _isNtlmUsed;

public SafeGssCredHandle AcceptorCredential
{
get
{
_acceptorCredential ??= SafeGssCredHandle.CreateAcceptor();
return _acceptorCredential;
}
}

public SafeGssNameHandle TargetName
{
get { return _targetName; }
Expand Down Expand Up @@ -78,6 +88,12 @@ protected override void Dispose(bool disposing)
_targetName.Dispose();
_targetName = null;
}

if (_acceptorCredential != null)
Copy link
Member

Choose a reason for hiding this comment

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

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.

{
_acceptorCredential.Dispose();
_acceptorCredential = null;
}
}
base.Dispose(disposing);
}
Expand Down
39 changes: 36 additions & 3 deletions src/Native/Unix/System.Net.Security.Native/pal_gssapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,33 +299,53 @@ uint32_t NetSecurityNative_InitSecContextEx(uint32_t* minorStatus,
}

uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags)
uint32_t* retFlags,
int32_t* isNtlmUsed)
{
assert(minorStatus != NULL);
assert(acceptorCredHandle != NULL);
assert(contextHandle != NULL);
assert(inputBytes != NULL || inputLength == 0);
assert(outBuffer != NULL);
davidsh marked this conversation as resolved.
Show resolved Hide resolved
assert(isNtlmUsed != NULL);
// Note: *contextHandle is null only in the first call and non-null in the subsequent calls

GssBuffer inputToken = {.length = inputLength, .value = inputBytes};
GssBuffer gssBuffer = {.length = 0, .value = NULL};

gss_OID mechType = GSS_C_NO_OID;
uint32_t majorStatus = gss_accept_sec_context(minorStatus,
contextHandle,
GSS_C_NO_CREDENTIAL,
acceptorCredHandle,
&inputToken,
GSS_C_NO_CHANNEL_BINDINGS,
NULL,
NULL,
&mechType,
&gssBuffer,
retFlags,
NULL,
NULL);

#if HAVE_GSS_SPNEGO_MECHANISM
gss_OID ntlmMech = GSS_NTLM_MECHANISM;
#else
gss_OID ntlmMech = &gss_mech_ntlm_OID_desc;
#endif

*isNtlmUsed = (gss_oid_equal(mechType, ntlmMech) != 0) ? 1 : 0;

// The gss_ntlmssp provider doesn't support impersonation or delegation but fails to set the GSS_C_IDENTIFY_FLAG
// flag. So, we'll set it here to keep the behavior consistent with Windows platform.
if (*isNtlmUsed == 1)
{
*retFlags |= GSS_C_IDENTIFY_FLAG;
}

NetSecurityNative_MoveBuffer(&gssBuffer, outBuffer);
return majorStatus;
}
Expand Down Expand Up @@ -494,6 +514,19 @@ static uint32_t NetSecurityNative_AcquireCredWithPassword(uint32_t* minorStatus,
return majorStatus;
}

uint32_t NetSecurityNative_AcquireAcceptorCred(uint32_t* minorStatus,
GssCredId** outputCredHandle)
{
return gss_acquire_cred(minorStatus,
GSS_C_NO_NAME,
GSS_C_INDEFINITE,
GSS_C_NO_OID_SET,
GSS_C_ACCEPT,
outputCredHandle,
NULL,
NULL);
}

uint32_t NetSecurityNative_InitiateCredWithPassword(uint32_t* minorStatus,
int32_t isNtlm,
GssName* desiredName,
Expand Down
9 changes: 8 additions & 1 deletion src/Native/Unix/System.Net.Security.Native/pal_gssapi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ Shims the gss_release_name method.
*/
DLLEXPORT uint32_t NetSecurityNative_ReleaseName(uint32_t* minorStatus, GssName** inputName);

/*
Shims the gss_acquire_cred method with GSS_C_ACCEPT.
*/
DLLEXPORT uint32_t NetSecurityNative_AcquireAcceptorCred(uint32_t* minorStatus, GssCredId** outputCredHandle);

/*
Shims the gss_acquire_cred method with SPNEGO oids with GSS_C_INITIATE.
*/
Expand Down Expand Up @@ -133,11 +138,13 @@ DLLEXPORT uint32_t NetSecurityNative_InitSecContextEx(uint32_t* minorStatus,
Shims the gss_accept_sec_context method.
*/
DLLEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags);
uint32_t* retFlags,
int32_t* isNtlmUsed);

/*
Expand Down