Skip to content

Commit 1164d2f

Browse files
rzikmrzikm
authored
Fix handling SECBUFFER_EXTRA with renegotiation after handshake. (#103419)
* 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
1 parent 141d3a3 commit 1164d2f

18 files changed

+349
-157
lines changed

‎src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs

+10
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,16 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context,
573573
throw handshakeException;
574574
}
575575

576+
// in case of TLS 1.3 post-handshake authentication, SslDoHandhaske
577+
// may return SSL_ERROR_NONE while still expecting more data from
578+
// the client. Attempts to send app data in this state would result
579+
// in SSL_ERROR_WANT_READ from SslWrite, override the return status
580+
// to continue waiting for the rest of the TLS frames
581+
if (context.IsServer && token.Size == 0 && errorCode == Ssl.SslErrorCode.SSL_ERROR_NONE && Ssl.IsSslRenegotiatePending(context))
582+
{
583+
return SecurityStatusPalErrorCode.ContinueNeeded;
584+
}
585+
576586
bool stateOk = Ssl.IsSslStateOK(context);
577587
if (stateOk)
578588
{

‎src/libraries/Common/src/Interop/Windows/SspiCli/ISSPIInterface.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ internal interface ISSPIInterface
1515
unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCHANNEL_CRED* authdata, out SafeFreeCredentials outCredential);
1616
unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.CredentialUse usage, Interop.SspiCli.SCH_CREDENTIALS* authdata, out SafeFreeCredentials outCredential);
1717
int AcquireDefaultCredential(string moduleName, Interop.SspiCli.CredentialUse usage, out SafeFreeCredentials outCredential);
18-
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);
19-
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);
18+
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);
19+
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);
2020
int EncryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, uint qop);
2121
int DecryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, out uint qop);
2222

‎src/libraries/Common/src/Interop/Windows/SspiCli/SSPIAuthType.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,14 @@ public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Cr
5050
return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential);
5151
}
5252

53-
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)
53+
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)
5454
{
55-
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
55+
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
5656
}
5757

58-
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)
58+
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)
5959
{
60-
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
60+
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
6161
}
6262

6363
public int EncryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, uint qop)

‎src/libraries/Common/src/Interop/Windows/SspiCli/SSPISecureChannelType.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ public unsafe int AcquireCredentialsHandle(string moduleName, Interop.SspiCli.Cr
4949
return SafeFreeCredentials.AcquireCredentialsHandle(moduleName, usage, authdata, out outCredential);
5050
}
5151

52-
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)
52+
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)
5353
{
54-
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
54+
return SafeDeleteContext.AcceptSecurityContext(ref credential, ref context, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
5555
}
5656

57-
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)
57+
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)
5858
{
59-
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, inputBuffers, ref outToken, ref outFlags);
59+
return SafeDeleteContext.InitializeSecurityContext(ref credential, ref context, targetName, inFlags, endianness, ref inputBuffers, ref outToken, ref outFlags);
6060
}
6161

6262
public int EncryptMessage(SafeDeleteContext context, ref Interop.SspiCli.SecBufferDesc inputOutput, uint qop)

‎src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,22 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandle(ISSPIInterface
141141
return outCredential;
142142
}
143143

144-
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)
144+
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)
145145
{
146146
if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.InitializeSecurityContext(credential, context, targetName, inFlags);
147147

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

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

152152
return errorCode;
153153
}
154154

155-
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)
155+
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)
156156
{
157157
if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.AcceptSecurityContext(credential, context, inFlags);
158158

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

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

‎src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs

+16-95
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ internal static unsafe int InitializeSecurityContext(
364364
string? targetName,
365365
Interop.SspiCli.ContextFlags inFlags,
366366
Interop.SspiCli.Endianness endianness,
367-
InputSecurityBuffers inSecBuffers,
367+
ref InputSecurityBuffers inSecBuffers,
368368
ref ProtocolToken outToken,
369369
ref Interop.SspiCli.ContextFlags outFlags)
370370
{
@@ -503,60 +503,15 @@ internal static unsafe int InitializeSecurityContext(
503503

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

514-
int leftover = inUnmanagedBuffer[1].cbBuffer;
515-
int processed = inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer;
516-
517-
/* skip over processed data and try it again. */
518-
inUnmanagedBuffer[0].cbBuffer = leftover;
519-
inUnmanagedBuffer[0].pvBuffer = inUnmanagedBuffer[0].pvBuffer + processed;
520-
inUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_EMPTY;
521-
inUnmanagedBuffer[1].cbBuffer = 0;
522-
523-
outUnmanagedBuffer.cbBuffer = 0;
524-
525-
if (outoutBuffer != IntPtr.Zero)
526-
{
527-
Interop.SspiCli.FreeContextBuffer(outoutBuffer);
528-
outoutBuffer = IntPtr.Zero;
529-
}
530-
531-
errorCode = MustRunInitializeSecurityContext(
532-
ref inCredentials,
533-
isContextAbsent,
534-
(byte*)namePtr,
535-
inFlags,
536-
endianness,
537-
&inSecurityBufferDescriptor,
538-
refContext!,
539-
ref outSecurityBufferDescriptor,
540-
ref outFlags,
541-
null);
542-
543-
if (isSspiAllocated)
544-
{
545-
outoutBuffer = outUnmanagedBuffer.pvBuffer;
546-
547-
if (outUnmanagedBuffer.cbBuffer > 0)
548-
{
549-
outToken.EnsureAvailableSpace(outUnmanagedBuffer.cbBuffer);
550-
new Span<byte>((byte*)outUnmanagedBuffer.pvBuffer, outUnmanagedBuffer.cbBuffer).CopyTo(outToken.AvailableSpan);
551-
outToken.Size += outUnmanagedBuffer.cbBuffer;
552-
}
553-
}
554-
555-
if (inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA)
556-
{
557-
// we are left with unprocessed data again. fail with SEC_E_INCOMPLETE_MESSAGE hResult.
558-
errorCode = unchecked((int)0x80090318);
559-
}
511+
// since SecurityBuffer type does not have separate Length field,
512+
// we point to the unused portion of the input buffer.
513+
Debug.Assert(inSecBuffers._item0.Token.Length > inUnmanagedBuffer[1].cbBuffer);
514+
inSecBuffers._item1.Token = inSecBuffers._item0.Token.Slice(inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer);
560515
}
561516
}
562517
}
@@ -676,7 +631,7 @@ internal static unsafe int AcceptSecurityContext(
676631
ref SafeDeleteSslContext? refContext,
677632
Interop.SspiCli.ContextFlags inFlags,
678633
Interop.SspiCli.Endianness endianness,
679-
InputSecurityBuffers inSecBuffers,
634+
ref InputSecurityBuffers inSecBuffers,
680635
ref ProtocolToken outToken,
681636
ref Interop.SspiCli.ContextFlags outFlags)
682637
{
@@ -807,51 +762,17 @@ internal static unsafe int AcceptSecurityContext(
807762
}
808763
outToken.Size = length;
809764

765+
// In some cases schannel may not process all the given data.
766+
// and it will return them back as SECBUFFER_EXTRA, expecting caller to
767+
// feed them in again. Propagate this information back up.
810768
if (inSecBuffers.Count > 1 && inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA && inSecBuffers._item1.Type == SecurityBufferType.SECBUFFER_EMPTY)
811769
{
812-
// OS function did not use all provided data and turned EMPTY to EXTRA
813-
// https://learn.microsoft.com/windows/win32/secauthn/extra-buffers-returned-by-schannel
814-
815-
int leftover = inUnmanagedBuffer[1].cbBuffer;
816-
int processed = inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer;
817-
818-
/* skip over processed data and try it again. */
819-
inUnmanagedBuffer[0].cbBuffer = leftover;
820-
inUnmanagedBuffer[0].pvBuffer = inUnmanagedBuffer[0].pvBuffer + processed;
821-
inUnmanagedBuffer[1].BufferType = SecurityBufferType.SECBUFFER_EMPTY;
822-
inUnmanagedBuffer[1].cbBuffer = 0;
823-
824-
outUnmanagedBuffer[0].cbBuffer = 0;
825-
if (isSspiAllocated && outUnmanagedBuffer[0].pvBuffer != IntPtr.Zero)
826-
{
827-
Interop.SspiCli.FreeContextBuffer(outUnmanagedBuffer[0].pvBuffer);
828-
outUnmanagedBuffer[0].pvBuffer = IntPtr.Zero;
829-
}
770+
inSecBuffers._item1.Type = inUnmanagedBuffer[1].BufferType;
830771

831-
errorCode = MustRunAcceptSecurityContext_SECURITY(
832-
ref inCredentials,
833-
isContextAbsent,
834-
&inSecurityBufferDescriptor,
835-
inFlags,
836-
endianness,
837-
refContext!,
838-
ref outSecurityBufferDescriptor,
839-
ref outFlags,
840-
null);
841-
842-
index = outUnmanagedBuffer[0].cbBuffer == 0 && outUnmanagedBuffer[1].cbBuffer > 0 ? 1 : 0;
843-
if (outUnmanagedBuffer[index].cbBuffer > 0)
844-
{
845-
outToken.EnsureAvailableSpace(outUnmanagedBuffer[index].cbBuffer);
846-
new Span<byte>((byte*)outUnmanagedBuffer[index].pvBuffer, outUnmanagedBuffer[index].cbBuffer).CopyTo(outToken.AvailableSpan);
847-
outToken.Size += outUnmanagedBuffer[index].cbBuffer;
848-
}
849-
850-
if (inUnmanagedBuffer[1].BufferType == SecurityBufferType.SECBUFFER_EXTRA)
851-
{
852-
// we are left with unprocessed data again. fail with SEC_E_INCOMPLETE_MESSAGE hResult.
853-
errorCode = unchecked((int)0x80090318);
854-
}
772+
// since SecurityBuffer type does not have separate Length field,
773+
// we point to the unused portion of the input buffer.
774+
Debug.Assert(inSecBuffers._item0.Token.Length > inUnmanagedBuffer[1].cbBuffer);
775+
inSecBuffers._item1.Token = inSecBuffers._item0.Token.Slice(inSecBuffers._item0.Token.Length - inUnmanagedBuffer[1].cbBuffer);
855776
}
856777
}
857778
}

‎src/libraries/Common/src/System/Net/Security/SecurityBuffer.Windows.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ internal void SetNextBuffer(InputSecurityBuffer buffer)
3636
}
3737

3838
[StructLayout(LayoutKind.Auto)]
39-
internal readonly ref struct InputSecurityBuffer
39+
internal ref struct InputSecurityBuffer
4040
{
41-
public readonly SecurityBufferType Type;
42-
public readonly ReadOnlySpan<byte> Token;
43-
public readonly SafeHandle? UnmanagedToken;
41+
public SecurityBufferType Type;
42+
public ReadOnlySpan<byte> Token;
43+
public SafeHandle? UnmanagedToken;
4444

4545
public InputSecurityBuffer(ReadOnlySpan<byte> data, SecurityBufferType tokentype)
4646
{

0 commit comments

Comments
 (0)