Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process more TLS frames at one when available #50815

Merged
merged 16 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Merge branch 'main' of https://github.com/dotnet/runtime into ssl_buf…
…fers2
  • Loading branch information
wfurt committed Jun 12, 2021
commit 6b2c58379d6d5fc56ce14c4bba3783a0841c31a3
Original file line number Diff line number Diff line change
Expand Up @@ -345,36 +345,23 @@ internal static int Encrypt(SafeSslHandle context, ReadOnlySpan<byte> input, ref
return retVal;
}

internal static int Decrypt(SafeSslHandle context, ReadOnlySpan<byte> input, Span<byte> output, out Ssl.SslErrorCode errorCode)
internal static int Decrypt(SafeSslHandle context, Span<byte> buffer, out Ssl.SslErrorCode errorCode)
{
#if DEBUG
ulong assertNoError = Crypto.ErrPeekError();
Debug.Assert(assertNoError == 0, "OpenSsl error queue is not empty, run: 'openssl errstr " + assertNoError.ToString("X") + "' for original error.");
#endif
errorCode = Ssl.SslErrorCode.SSL_ERROR_NONE;

int retVal = BioWrite(context.InputBio!, input);
Exception? innerError = null;
BioWrite(context.InputBio!, buffer);

if (retVal == input.Length)
int retVal = Ssl.SslRead(context, ref MemoryMarshal.GetReference(buffer), buffer.Length);
if (retVal > 0)
{
unsafe
{
fixed (byte* fixedBuffer = output)
{
retVal = Ssl.SslRead(context, fixedBuffer, output.Length);
}
}

if (retVal > 0)
{
return retVal;
}
return retVal;
}

errorCode = GetSslError(context, retVal, out innerError);
retVal = 0;

errorCode = GetSslError(context, retVal, out Exception? innerError);
switch (errorCode)
{
// indicate end-of-file
Expand All @@ -389,10 +376,10 @@ internal static int Decrypt(SafeSslHandle context, ReadOnlySpan<byte> input, Spa
break;

default:
throw new SslException(SR.Format(SR.net_ssl_decrypt_failed, errorCode), innerError);
throw new SslException(SR.Format(SR.net_ssl_decrypt_failed, errorCode), innerError);
}

return retVal;
return 0;
}

internal static SafeX509Handle GetPeerCertificate(SafeSslHandle context)
Expand Down Expand Up @@ -497,17 +484,9 @@ private static int BioRead(SafeBioHandle bio, byte[] buffer, int count)
return bytes;
}

private static int BioWrite(SafeBioHandle bio, ReadOnlySpan<byte> buffer)
private static void BioWrite(SafeBioHandle bio, ReadOnlySpan<byte> buffer)
{
int bytes;
unsafe
{
fixed (byte* bufPtr = buffer)
{
bytes = Ssl.BioWrite(bio, bufPtr, buffer.Length);
}
}

int bytes = Ssl.BioWrite(bio, ref MemoryMarshal.GetReference(buffer), buffer.Length);
if (bytes != buffer.Length)
{
throw CreateSslException(SR.net_ssl_write_bio_failed_error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -903,12 +903,12 @@ internal SecurityStatusPal Encrypt(ReadOnlyMemory<byte> buffer, ref byte[] outpu
return secStatus;
}

internal SecurityStatusPal Decrypt(ReadOnlySpan<byte> input, Span<byte> output, out int outputOffset, out int outputCount)
internal SecurityStatusPal Decrypt(Span<byte> buffer, out int outputOffset, out int outputCount)
{
SecurityStatusPal status = SslStreamPal.DecryptMessage(_securityContext!, input, output, out outputOffset, out outputCount);
SecurityStatusPal status = SslStreamPal.DecryptMessage(_securityContext!, buffer, out outputOffset, out outputCount);
if (NetEventSource.Log.IsEnabled() && status.ErrorCode == SecurityStatusPalErrorCode.OK)
{
NetEventSource.DumpBuffer(this, output.Slice(outputOffset, outputCount));
NetEventSource.DumpBuffer(this, buffer.Slice(outputOffset, outputCount));
}

return status;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,7 @@ private void ReturnReadBufferIfEmpty()
}
}


private bool HaveFullTlsFrame(out int frameSize)
{
if (_internalBufferCount < SecureChannel.ReadHeaderSize)
Expand All @@ -821,6 +822,7 @@ private bool HaveFullTlsFrame(out int frameSize)
return _internalBufferCount >= frameSize;
}


private ValueTask<int> GetFullFrameIfNeed<TIOAdapter>(TIOAdapter adapter)
wfurt marked this conversation as resolved.
Show resolved Hide resolved
where TIOAdapter : IReadWriteAdapter
{
Expand Down Expand Up @@ -909,6 +911,7 @@ async ValueTask<int> InternalGetFullFrameIfNeed(TIOAdapter adap, ValueTask<int>
}
}


private SecurityStatusPal DecryptData(int frameSize, out int decryptedCount, out int decryptedOffset)
wfurt marked this conversation as resolved.
Show resolved Hide resolved
{
// Set _decrytpedBytesOffset/Count to the current frame we have (including header)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we should assert that _decryptedBytesCount is 0 here, right? Otherwise we could be overwriting unconsumed decrypted data.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, the asset is good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Set _decrytpedBytesOffset/Count to the current frame we have (including header)
// Set _decryptedBytesOffset/Count to the current frame we have (including header)

Expand All @@ -919,7 +922,7 @@ private SecurityStatusPal DecryptData(int frameSize, out int decryptedCount, out
lock (_handshakeLock)
{
ThrowIfExceptionalOrNotAuthenticated();
status = _context!.Decrypt(new ReadOnlySpan<byte>(_internalBuffer, _internalOffset, frameSize), _internalBuffer.AsSpan(_decryptedBytesOffset), out decryptedOffset, out decryptedCount);
status = _context!.Decrypt(new Span<byte>(_internalBuffer, _internalOffset, frameSize), out decryptedOffset, out decryptedCount);
_decryptedBytesCount = decryptedCount;
if (decryptedCount > 0)
{
Expand All @@ -941,11 +944,12 @@ private SecurityStatusPal DecryptData(int frameSize, out int decryptedCount, out
// and EncryptData() will work normally e.g. no waiting, just exclusion with DecryptData()


if (_sslAuthenticationOptions!.AllowRenegotiation || SslProtocol == SslProtocols.Tls13)
if (_sslAuthenticationOptions!.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != 0)
{
// create TCS only if we plan to proceed. If not, we will throw in block bellow outside of the lock.
wfurt marked this conversation as resolved.
Show resolved Hide resolved
// Tls1.3 does not have renegotiation. However on Windows this error code is used
// for session management e.g. anything lsass needs to see.
// We also allow it when explicitly requested using RenegotiateAsync().
_handshakeWaiter = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
}
}
Expand Down Expand Up @@ -978,6 +982,11 @@ private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, M
{
if (_decryptedBytesCount != 0)
{
if (renegotiation)
{
throw new InvalidOperationException(SR.net_ssl_renegotiate_data);
}

processedLength = CopyDecryptedData(buffer);
if (processedLength == buffer.Length || !HaveFullTlsFrame(out payloadBytes))
{
Expand All @@ -988,7 +997,7 @@ private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, M
buffer = buffer.Slice(processedLength);
}

if (receivedEOF)
if (receivedEOF && _internalBufferCount == 0)
{
// We received EOF during previous read but had buffered data to return.
return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ public static SecurityStatusPal EncryptMessage(

public static SecurityStatusPal DecryptMessage(
SafeDeleteSslContext securityContext,
ReadOnlySpan<byte> input,
Span<byte> output,
Span<byte> buffer,
out int offset,
out int count)
{
Expand All @@ -124,9 +123,9 @@ public static SecurityStatusPal DecryptMessage(
{
SafeSslHandle sslHandle = securityContext.SslContext;

securityContext.Write(input);
securityContext.Write(buffer);

PAL_SSLStreamStatus ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, input, out int read);
PAL_SSLStreamStatus ret = Interop.AndroidCrypto.SSLStreamRead(sslHandle, buffer, out int read);
if (ret == PAL_SSLStreamStatus.Error)
return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,33 +143,32 @@ public static SecurityStatusPal EncryptMessage(

public static SecurityStatusPal DecryptMessage(
SafeDeleteSslContext securityContext,
ReadOnlySpan<byte> input,
Span<byte> output,
out int outputOffset,
out int outputCount)
Span<byte> buffer,
out int offset,
out int count)
{
outputOffset = 0;
outputCount = 0;
offset = 0;
count = 0;

try
{
SafeSslHandle sslHandle = securityContext.SslContext;
securityContext.Write(input);
securityContext.Write(buffer);

unsafe
{
fixed (byte* ptr = output)
fixed (byte* ptr = buffer)
{
PAL_TlsIo status = Interop.AppleCrypto.SslRead(sslHandle, ptr, output.Length, out int written);
PAL_TlsIo status = Interop.AppleCrypto.SslRead(sslHandle, ptr, buffer.Length, out int written);
if (status < 0)
{
return new SecurityStatusPal(
SecurityStatusPalErrorCode.InternalError,
Interop.AppleCrypto.CreateExceptionForOSStatus((int)status));
}

outputCount = written;
outputOffset = 0;
count = written;
offset = 0;

switch (status)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.Security.Authentication;
using System.Security.Authentication.ExtendedProtection;
using System.Security.Cryptography.X509Certificates;
using Microsoft.Win32.SafeHandles;

namespace System.Net.Security
Expand Down Expand Up @@ -48,20 +49,41 @@ public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateC

public static SecurityStatusPal EncryptMessage(SafeDeleteSslContext securityContext, ReadOnlyMemory<byte> input, int headerSize, int trailerSize, ref byte[] output, out int resultSize)
{
return EncryptDecryptHelper(securityContext, input.Span, Span<byte>.Empty, encrypt: true, newBuffer: ref output, resultSize: out resultSize);
try
{
resultSize = Interop.OpenSsl.Encrypt(securityContext.SslContext, input.Span, ref output, out Interop.Ssl.SslErrorCode errorCode);

return MapNativeErrorCode(errorCode);
}
catch (Exception ex)
{
resultSize = 0;
return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, ex);
}
}

public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityContext, ReadOnlySpan<byte> input, Span<byte> output, out int outputOffset, out int outputCount)
public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityContext, Span<byte> buffer, out int offset, out int count)
{
byte[] buffer = Array.Empty<byte>();
outputOffset = 0;
outputCount = 0;
SecurityStatusPal retVal = EncryptDecryptHelper(securityContext, input, output, encrypt: false, out int resultSize, ref buffer);
if (retVal.ErrorCode == SecurityStatusPalErrorCode.OK ||
retVal.ErrorCode == SecurityStatusPalErrorCode.Renegotiate)
offset = 0;
count = 0;

try
{
// outputOffset = 0;
outputCount = resultSize;
int resultSize = Interop.OpenSsl.Decrypt(securityContext.SslContext, buffer, out Interop.Ssl.SslErrorCode errorCode);

SecurityStatusPal retVal = MapNativeErrorCode(errorCode);

if (retVal.ErrorCode == SecurityStatusPalErrorCode.OK ||
retVal.ErrorCode == SecurityStatusPalErrorCode.Renegotiate)
{
count = resultSize;
}

return retVal;
}
catch (Exception ex)
{
return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, ex);
}
}

Expand Down Expand Up @@ -167,42 +189,6 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia
return Interop.Ssl.SslGetAlpnSelected(context.SslContext);
}

private static SecurityStatusPal EncryptDecryptHelper(SafeDeleteSslContext securityContext, ReadOnlySpan<byte> input, Span<byte> output, bool encrypt, out int resultSize, ref byte[] newBuffer)
{
resultSize = 0;
try
{
Interop.Ssl.SslErrorCode errorCode = Interop.Ssl.SslErrorCode.SSL_ERROR_NONE;
SafeSslHandle scHandle = securityContext.SslContext;

if (encrypt)
{
resultSize = Interop.OpenSsl.Encrypt(scHandle, input, ref newBuffer, out errorCode);
}
else
{
resultSize = Interop.OpenSsl.Decrypt(scHandle, input, output, out errorCode);
}

switch (errorCode)
{
case Interop.Ssl.SslErrorCode.SSL_ERROR_RENEGOTIATE:
return new SecurityStatusPal(SecurityStatusPalErrorCode.Renegotiate);
case Interop.Ssl.SslErrorCode.SSL_ERROR_ZERO_RETURN:
return new SecurityStatusPal(SecurityStatusPalErrorCode.ContextExpired);
case Interop.Ssl.SslErrorCode.SSL_ERROR_NONE:
case Interop.Ssl.SslErrorCode.SSL_ERROR_WANT_READ:
return new SecurityStatusPal(SecurityStatusPalErrorCode.OK);
default:
return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, new Interop.OpenSsl.SslException((int)errorCode));
}
}
catch (Exception ex)
{
return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, ex);
}
}

public static SecurityStatusPal ApplyAlertToken(ref SafeFreeCredentials? credentialsHandle, SafeDeleteContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage)
{
// There doesn't seem to be an exposed API for writing an alert,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public static Exception GetException(SecurityStatusPal status)

internal const bool StartMutualAuthAsAnonymous = true;
internal const bool CanEncryptEmptyMessage = true;
internal const bool DecryptsInPlace = true;

public static void VerifyPackageInfo()
{
Expand Down Expand Up @@ -314,16 +313,16 @@ public static unsafe SecurityStatusPal EncryptMessage(SafeDeleteSslContext secur
}
}

public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? securityContext, ReadOnlySpan<byte> input, Span<byte> output, out int outputOffset, out int outputCount)
public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? securityContext, Span<byte> buffer, out int offset, out int count)
{
const int NumSecBuffers = 4; // data + empty + empty + empty
fixed (byte* bufferPtr = input)
fixed (byte* bufferPtr = buffer)
{
Interop.SspiCli.SecBuffer* unmanagedBuffer = stackalloc Interop.SspiCli.SecBuffer[NumSecBuffers];
Interop.SspiCli.SecBuffer* dataBuffer = &unmanagedBuffer[0];
dataBuffer->BufferType = SecurityBufferType.SECBUFFER_DATA;
dataBuffer->pvBuffer = (IntPtr)bufferPtr;
dataBuffer->cbBuffer = input.Length;
dataBuffer->cbBuffer = buffer.Length;

for (int i = 1; i < NumSecBuffers; i++)
{
Expand All @@ -341,21 +340,21 @@ public static unsafe SecurityStatusPal DecryptMessage(SafeDeleteSslContext? secu

// Decrypt may repopulate the sec buffers, likely with header + data + trailer + empty.
// We need to find the data.
outputCount = 0;
outputOffset = 0;
count = 0;
offset = 0;
for (int i = 0; i < NumSecBuffers; i++)
{
// Successfully decoded data and placed it at the following position in the buffer,
if ((errorCode == Interop.SECURITY_STATUS.OK && unmanagedBuffer[i].BufferType == SecurityBufferType.SECBUFFER_DATA)
// or we failed to decode the data, here is the encoded data.
|| (errorCode != Interop.SECURITY_STATUS.OK && unmanagedBuffer[i].BufferType == SecurityBufferType.SECBUFFER_EXTRA))
{
outputOffset = (int)((byte*)unmanagedBuffer[i].pvBuffer - bufferPtr);
outputCount = unmanagedBuffer[i].cbBuffer;
offset = (int)((byte*)unmanagedBuffer[i].pvBuffer - bufferPtr);
count = unmanagedBuffer[i].cbBuffer;

// output is ignored on Windows. We always decrypt in place and we set outputOffset to indicate where the data start.
Debug.Assert(outputOffset >= 0 && outputCount >= 0, $"Expected offset and count greater than 0, got {outputOffset} and {outputCount}");
Debug.Assert(checked(outputOffset + outputCount) <= input.Length, $"Expected offset+count <= buffer.Length, got {outputOffset}+{outputCount}>={input.Length}");
Debug.Assert(offset >= 0 && count >= 0, $"Expected offset and count greater than 0, got {offset} and {count}");
Debug.Assert(checked(offset + count) <= buffer.Length, $"Expected offset+count <= buffer.Length, got {offset}+{count}>={buffer.Length}");

break;
}
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.