Skip to content

fix buffer handling in Tls handshake #32267

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.

Sign up for GitHub

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

Merged
wfurt merged 2 commits into from
Feb 14, 2020
Merged

fix buffer handling in Tls handshake #32267

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
a14e748
fix buffer handling in Tls handshake
wfurt Feb 13, 2020
f62d288
feedback from review
wfurt Feb 14, 2020
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
11 changes: 7 additions & 4 deletions src/libraries/System.Net.Security/src/System.Net.Security.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,21 @@
<Link>Common\System\Net\DebugCriticalHandleZeroOrMinusOneIsInvalid.cs</Link>
</Compile>
<!-- System.Net common -->
<Compile Include="$(CommonPath)System\Net\ArrayBuffer.cs">
<Link>Common\System\Net\ArrayBuffer.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\Net\ExceptionCheck.cs">
<Link>Common\System\Net\ExceptionCheck.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\Net\LazyAsyncResult.cs">
<Link>Common\System\Net\LazyAsyncResult.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\Net\UriScheme.cs">
<Link>Common\System\Net\UriScheme.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs">
<Link>Common\System\Net\SecurityProtocol.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\Net\UriScheme.cs">
<Link>Common\System\Net\UriScheme.cs</Link>
</Compile>
<!-- Common -->
<Compile Include="$(CommonPath)System\NotImplemented.cs">
<Link>Common\System\NotImplemented.cs</Link>
Expand Down Expand Up @@ -471,4 +474,4 @@
<Reference Include="System.Security.Cryptography.OpenSsl" />
<Reference Include="System.Security.Cryptography.Primitives" />
</ItemGroup>
</Project>
</Project>
23 changes: 5 additions & 18 deletions src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,21 +717,21 @@ private bool AcquireServerCredentials(ref byte[] thumbPrint, ReadOnlySpan<byte>
}

//
internal ProtocolToken NextMessage(byte[] incoming, int offset, int count)
internal ProtocolToken NextMessage(ReadOnlySpan<byte> incomingBuffer)
{
if (NetEventSource.IsEnabled)
NetEventSource.Enter(this);

byte[] nextmsg = null;
SecurityStatusPal status = GenerateToken(incoming, offset, count, ref nextmsg);
SecurityStatusPal status = GenerateToken(incomingBuffer, ref nextmsg);

if (!_sslAuthenticationOptions.IsServer && status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
{
if (NetEventSource.IsEnabled)
NetEventSource.Info(this, "NextMessage() returned SecurityStatusPal.CredentialsNeeded");

SetRefreshCredentialNeeded();
status = GenerateToken(incoming, offset, count, ref nextmsg);
status = GenerateToken(incomingBuffer, ref nextmsg);
}

ProtocolToken token = new ProtocolToken(nextmsg, status);
Expand Down Expand Up @@ -763,27 +763,14 @@ server in response
Return:
status - error information
--*/
private SecurityStatusPal GenerateToken(byte[] input, int offset, int count, ref byte[] output)
private SecurityStatusPal GenerateToken(ReadOnlySpan<byte> inputBuffer, ref byte[] output)
{
if (NetEventSource.IsEnabled) NetEventSource.Enter(this, $"_refreshCredentialNeeded = {_refreshCredentialNeeded}");

if (offset < 0 || offset > (input == null ? 0 : input.Length))
{
NetEventSource.Fail(this, "Argument 'offset' out of range.");
throw new ArgumentOutOfRangeException(nameof(offset));
}

if (count < 0 || count > (input == null ? 0 : input.Length - offset))
{
NetEventSource.Fail(this, "Argument 'count' out of range.");
throw new ArgumentOutOfRangeException(nameof(count));
}

byte[] result = Array.Empty<byte>();
SecurityStatusPal status = default;
bool cachedCreds = false;
byte[] thumbPrint = null;
ReadOnlySpan<byte> inputBuffer = new ReadOnlySpan<byte>(input, offset, count);

//
// Looping through ASC or ISC with potentially cached credential that could have been
Expand Down Expand Up @@ -1155,7 +1142,7 @@ private ProtocolToken GenerateAlertToken()
byte[] nextmsg = null;

SecurityStatusPal status;
status = GenerateToken(null, 0, 0, ref nextmsg);
status = GenerateToken(default, ref nextmsg);

ProtocolToken token = new ProtocolToken(nextmsg, status);

Expand Down
6 changes: 3 additions & 3 deletions ...ibraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.Adapters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public partial class SslStream
{
private interface ISslIOAdapter
{
ValueTask<int> ReadAsync(byte[] buffer, int offset, int count);
ValueTask<int> ReadAsync(Memory<byte> buffer);
Copy link
Member

@stephentoub stephentoub Feb 14, 2020

Choose a reason for hiding this comment

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

Do we still need the ValueTask<int> ReadAsync(byte[] buffer, int offset, int count); overload?

Copy link
Member Author

@wfurt wfurt Feb 14, 2020

Choose a reason for hiding this comment

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

It is used in FillBufferAsync() but that is easy to fix.

ValueTask<int> ReadLockAsync(Memory<byte> buffer);
Task WriteLockAsync();
ValueTask WriteAsync(byte[] buffer, int offset, int count);
Expand All @@ -29,7 +29,7 @@ public AsyncSslIOAdapter(SslStream sslStream, CancellationToken cancellationToke
_sslStream = sslStream;
}

public ValueTask<int> ReadAsync(byte[] buffer, int offset, int count) => _sslStream.InnerStream.ReadAsync(new Memory<byte>(buffer, offset, count), _cancellationToken);
public ValueTask<int> ReadAsync(Memory<byte> buffer) => _sslStream.InnerStream.ReadAsync(buffer, _cancellationToken);

public ValueTask<int> ReadLockAsync(Memory<byte> buffer) => _sslStream.CheckEnqueueReadAsync(buffer);

Expand All @@ -46,7 +46,7 @@ public AsyncSslIOAdapter(SslStream sslStream, CancellationToken cancellationToke

public SyncSslIOAdapter(SslStream sslStream) => _sslStream = sslStream;

public ValueTask<int> ReadAsync(byte[] buffer, int offset, int count) => new ValueTask<int>(_sslStream.InnerStream.Read(buffer, offset, count));
public ValueTask<int> ReadAsync(Memory<byte> buffer) => new ValueTask<int>(_sslStream.InnerStream.Read(buffer.Span));

public ValueTask<int> ReadLockAsync(Memory<byte> buffer) => new ValueTask<int>(_sslStream.CheckEnqueueRead(buffer));

Expand Down
Loading