Skip to content

Commit

Permalink
Fix handling SECBUFFER_EXTRA with renegotiation after handshake. (#10…
Browse files Browse the repository at this point in the history 
…3419)

* Add failing test

* WIP

* Propagate processed length from ASC as well

* Don't test renegotiation on unsupported platforms

* Commit forgotten change

* Remove unnecessary code

* Fix failures on Linux

* Fix Tls 13 post-handshake-auth byte-by-byte case on Linux
  • Loading branch information
@rzikm
rzikm committed Jul 9, 2024
1 parent 141d3a3 commit 1164d2f
Show file tree
Hide file tree
Showing 18 changed files with 349 additions and 157 deletions.
10 changes: 10 additions & 0 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,16 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context,
throw handshakeException;
}

// in case of TLS 1.3 post-handshake authentication, SslDoHandhaske
// may return SSL_ERROR_NONE while still expecting more data from
// the client. Attempts to send app data in this state would result
// in SSL_ERROR_WANT_READ from SslWrite, override the return status
// to continue waiting for the rest of the TLS frames
if (context.IsServer && token.Size == 0 && errorCode == Ssl.SslErrorCode.SSL_ERROR_NONE && Ssl.IsSslRenegotiatePending(context))
{
return SecurityStatusPalErrorCode.ContinueNeeded;
}

bool stateOk = Ssl.IsSslStateOK(context);
if (stateOk)
{
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ internal interface ISSPIInterface
unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCHANNEL_CRED* authdata, out SafeFreeCredentials outCredential);
unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential);
int AcquireDefaultCredential(string moduleName, Interop.SspiCli.CredentialUse usage, out SafeFreeCredentials outCredential);
int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags);
int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags);
int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, ref InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags);
int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags);
int EncryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, uint qop);
int DecryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, out uint qop);

Expand Down
8 changes: 4 additions & 4 deletions src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Cr
return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential);
}

public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, ref InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
{
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
}

public int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
public int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
{
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
}

public int EncryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, uint qop)
Expand Down
8 changes: 4 additions & 4 deletions src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Cr
return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential);
}

public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
public int AcceptSecurityContext(SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, ref InputSecurityBuffers inputBuffers, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
{
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
}

public int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
public int InitializeSecurityContext(ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness endianness, ref InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
{
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
}

public int EncryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, uint qop)
Expand Down
8 changes: 4 additions & 4 deletions src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,22 +141,22 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface
return outCredential;
}

internal static int InitializeSecurityContext(ISSPIInterface secModule, ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness datarep, InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
internal static int InitializeSecurityContext(ISSPIInterface secModule, ref SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, string? targetName, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness datarep, ref InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.InitializeSecurityContext(credential, context, targetName, inFlags);

int errorCode = secModule.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, datarep, inputBuffers, ref outToken, ref outFlags);
int errorCode = secModule.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, datarep, ref inputBuffers, ref outToken, ref outFlags);

if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.SecurityContextInputBuffers(nameof(InitializeSecurityContext), inputBuffers.Count, outToken.Size, (Interop.SECURITY_STATUS)errorCode);

return errorCode;
}

internal static int AcceptSecurityContext(ISSPIInterface secModule, SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness datarep, InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
internal static int AcceptSecurityContext(ISSPIInterface secModule, SafeFreeCredentials? credential, ref SafeDeleteSslContext? context, Interop.SspiCli.ContextFlags inFlags, Interop.SspiCli.Endianness datarep, ref InputSecurityBuffers inputBuffers, ref ProtocolToken outToken, ref Interop.SspiCli.ContextFlags outFlags)
{
if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.AcceptSecurityContext(credential, context, inFlags);

int errorCode = secModule.AcceptSecurityContext(credential, ref context, inputBuffers, inFlags, datarep, ref outToken, ref outFlags);
int errorCode = secModule.AcceptSecurityContext(credential, ref context, ref inputBuffers, inFlags, datarep, ref outToken, ref outFlags);

if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.SecurityContextInputBuffers(nameof(AcceptSecurityContext), inputBuffers.Count, outToken.Size, (Interop.SECURITY_STATUS)errorCode);

Expand Down
111 changes: 16 additions & 95 deletions src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ internal static unsafe int InitializeSecurityContext(
string? targetName,
Interop.SspiCli.ContextFlags inFlags,
Interop.SspiCli.Endianness endianness,
InputSecurityBuffers inSecBuffers,
ref InputSecurityBuffers inSecBuffers,
ref ProtocolToken outToken,
ref Interop.SspiCli.ContextFlags outFlags)
{
Expand Down Expand Up @@ -503,60 +503,15 @@ internal static unsafe int InitializeSecurityContext(

// In some cases schannel may not process all the given data.
// and it will return them back as SECBUFFER_EXTRA, expecting caller to
// feed them in again. Since we don't have good way how to flow the input back,
// we will try it again as separate call and we will return combined output from first and second try.
// That makes processing of outBuffer somewhat complicated.
// feed them in again. Propagate this information back up.
if (inSecBuffers.Count > 1 && inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA && inSecBuffers._item1.Type == SecurityBufferType.SECBUFFER_EMPTY)
{
// OS function did not use all provided data and turned EMPTY to EXTRA
// https://learn.microsoft.com/windows/win32/secauthn/extra-buffers-returned-by-schannel
inSecBuffers._item1.Type = inUnmanagedBuffer[1].BufferType;

int leftover = inUnmanagedBuffer[1].cbBuffer;
int processed = inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer;

/* skip over processed data and try it again. */
inUnmanagedBuffer[0].cbBuffer = leftover;
inUnmanagedBuffer[0].pvBuffer = inUnmanagedBuffer[0].pvBuffer + processed;
inUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_EMPTY;
inUnmanagedBuffer[1].cbBuffer = 0;

outUnmanagedBuffer.cbBuffer = 0;

if (outoutBuffer != IntPtr.Zero)
{
Interop.SspiCli.FreeContextBuffer(outoutBuffer);
outoutBuffer = IntPtr.Zero;
}

errorCode = MustRunInitializeSecurityContext(
ref inCredentials,
isContextAbsent,
(byte*)namePtr,
inFlags,
endianness,
&inSecurityBufferDescriptor,
refContext!,
ref outSecurityBufferDescriptor,
ref outFlags,
null);

if (isSspiAllocated)
{
outoutBuffer = outUnmanagedBuffer.pvBuffer;

if (outUnmanagedBuffer.cbBuffer > 0)
{
outToken.EnsureAvailableSpace(outUnmanagedBuffer.cbBuffer);
new Span<byte>((byte*)outUnmanagedBuffer.pvBuffer, outUnmanagedBuffer.cbBuffer).CopyTo(outToken.AvailableSpan);
outToken.Size += outUnmanagedBuffer.cbBuffer;
}
}

if (inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA)
{
// we are left with unprocessed data again. fail with SEC_E_INCOMPLETE_MESSAGE hResult.
errorCode = unchecked((int)0x80090318);
}
// since SecurityBuffer type does not have separate Length field,
// we point to the unused portion of the input buffer.
Debug.Assert(inSecBuffers._item0.Token.Length > inUnmanagedBuffer[1].cbBuffer);
inSecBuffers._item1.Token = inSecBuffers._item0.Token.Slice(inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer);
}
}
}
Expand Down Expand Up @@ -676,7 +631,7 @@ internal static unsafe int AcceptSecurityContext(
ref SafeDeleteSslContext? refContext,
Interop.SspiCli.ContextFlags inFlags,
Interop.SspiCli.Endianness endianness,
InputSecurityBuffers inSecBuffers,
ref InputSecurityBuffers inSecBuffers,
ref ProtocolToken outToken,
ref Interop.SspiCli.ContextFlags outFlags)
{
Expand Down Expand Up @@ -807,51 +762,17 @@ internal static unsafe int AcceptSecurityContext(
}
outToken.Size = length;

// In some cases schannel may not process all the given data.
// and it will return them back as SECBUFFER_EXTRA, expecting caller to
// feed them in again. Propagate this information back up.
if (inSecBuffers.Count > 1 && inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA && inSecBuffers._item1.Type == SecurityBufferType.SECBUFFER_EMPTY)
{
// OS function did not use all provided data and turned EMPTY to EXTRA
// https://learn.microsoft.com/windows/win32/secauthn/extra-buffers-returned-by-schannel

int leftover = inUnmanagedBuffer[1].cbBuffer;
int processed = inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer;

/* skip over processed data and try it again. */
inUnmanagedBuffer[0].cbBuffer = leftover;
inUnmanagedBuffer[0].pvBuffer = inUnmanagedBuffer[0].pvBuffer + processed;
inUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_EMPTY;
inUnmanagedBuffer[1].cbBuffer = 0;

outUnmanagedBuffer[0].cbBuffer = 0;
if (isSspiAllocated && outUnmanagedBuffer[0].pvBuffer != IntPtr.Zero)
{
Interop.SspiCli.FreeContextBuffer(outUnmanagedBuffer[0].pvBuffer);
outUnmanagedBuffer[0].pvBuffer = IntPtr.Zero;
}
inSecBuffers._item1.Type = inUnmanagedBuffer[1].BufferType;

errorCode = MustRunAcceptSecurityContext_SECURITY(
ref inCredentials,
isContextAbsent,
&inSecurityBufferDescriptor,
inFlags,
endianness,
refContext!,
ref outSecurityBufferDescriptor,
ref outFlags,
null);

index = outUnmanagedBuffer[0].cbBuffer == 0 && outUnmanagedBuffer[1].cbBuffer > 0 ? 1 : 0;
if (outUnmanagedBuffer[index].cbBuffer > 0)
{
outToken.EnsureAvailableSpace(outUnmanagedBuffer[index].cbBuffer);
new Span<byte>((byte*)outUnmanagedBuffer[index].pvBuffer, outUnmanagedBuffer[index].cbBuffer).CopyTo(outToken.AvailableSpan);
outToken.Size += outUnmanagedBuffer[index].cbBuffer;
}

if (inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA)
{
// we are left with unprocessed data again. fail with SEC_E_INCOMPLETE_MESSAGE hResult.
errorCode = unchecked((int)0x80090318);
}
// since SecurityBuffer type does not have separate Length field,
// we point to the unused portion of the input buffer.
Debug.Assert(inSecBuffers._item0.Token.Length > inUnmanagedBuffer[1].cbBuffer);
inSecBuffers._item1.Token = inSecBuffers._item0.Token.Slice(inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/libraries/Common/src/System/Net/Security/SecurityBuffer.Windows.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ internal void SetNextBuffer(InputSecurityBuffer buffer)
}

[StructLayout(LayoutKind.Auto)]
internal readonly ref struct InputSecurityBuffer
internal ref struct InputSecurityBuffer
{
public readonly SecurityBufferType Type;
public readonly ReadOnlySpan<byte> Token;
public readonly SafeHandle? UnmanagedToken;
public SecurityBufferType Type;
public ReadOnlySpan<byte> Token;
public SafeHandle? UnmanagedToken;

public InputSecurityBuffer(ReadOnlySpan<byte> data, SecurityBufferType tokentype)
{
Expand Down
Loading

0 comments on commit 1164d2f

Please sign in to comment.