Skip to content

Commit b84be9d

Browse files
rzikmrzikm
committed
WIP
1 parent b6fb762 commit b84be9d

File tree

8 files changed

+84
-109
lines changed

8 files changed

+84
-109
lines changed

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -439,9 +439,6 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth
439439
// Client side always verifies the server's certificate.
440440
Ssl.SslSetVerifyPeer(sslHandle);
441441

442-
// HACK: set a bogus code to indicate that we did not perform the validation yet
443-
// Ssl.SslSetVerifyResult(sslHandle, -1);
444-
445442
if (!string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && !IPAddress.IsValid(sslAuthenticationOptions.TargetHost))
446443
{
447444
// Similar to windows behavior, set SNI on openssl by default for client context, ignore errors.
@@ -474,8 +471,6 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth
474471
if (sslAuthenticationOptions.RemoteCertRequired)
475472
{
476473
Ssl.SslSetVerifyPeer(sslHandle);
477-
// HACK: set a bogus code to indicate that we did not perform the validation yet
478-
Ssl.SslSetVerifyResult(sslHandle, -1);
479474
}
480475

481476
if (sslAuthenticationOptions.CertificateContext != null)
@@ -552,8 +547,6 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context,
552547
}
553548
}
554549

555-
#pragma warning disable CS0618
556-
System.Console.WriteLine($"[{AppDomain.GetCurrentThreadId()}] SSL_do_handshake");
557550
int retVal = Ssl.SslDoHandshake(context, out Ssl.SslErrorCode errorCode);
558551
if (retVal != 1)
559552
{
@@ -562,13 +555,6 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context,
562555
return SecurityStatusPalErrorCode.CredentialsNeeded;
563556
}
564557

565-
if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_ASYNC)
566-
// if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_RETRY_VERIFY)
567-
{
568-
System.Console.WriteLine($"[{AppDomain.GetCurrentThreadId()}] SSL_ERROR_WANT_ASYNC");
569-
return SecurityStatusPalErrorCode.PeerCertVerifyRequired;
570-
}
571-
572558
if ((retVal != -1) || (errorCode != Ssl.SslErrorCode.SSL_ERROR_WANT_READ))
573559
{
574560
Exception? innerError = GetSslError(retVal, errorCode);

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -204,12 +204,6 @@ internal static SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl)
204204
[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetData")]
205205
internal static partial void SslSessionSetData(IntPtr session, IntPtr val);
206206

207-
[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetVerifyResult")]
208-
internal static partial void SslSetVerifyResult(SafeSslHandle ssl, long verifyResult);
209-
210-
[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetVerifyResult")]
211-
internal static partial long SslGetVerifyResult(SafeSslHandle ssl);
212-
213207
internal static class Capabilities
214208
{
215209
// needs separate type (separate static cctor) to be sure OpenSSL is initialized.
@@ -346,9 +340,6 @@ internal enum SslErrorCode
346340
SSL_ERROR_SYSCALL = 5,
347341
SSL_ERROR_ZERO_RETURN = 6,
348342

349-
SSL_ERROR_WANT_ASYNC = 9,
350-
SSL_ERROR_WANT_RETRY_VERIFY = 12,
351-
352343
// NOTE: this SslErrorCode value doesn't exist in OpenSSL, but
353344
// we use it to distinguish when a renegotiation is pending.
354345
// Choosing an arbitrarily large value that shouldn't conflict

‎src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
// (C) 2006 John Luke
1010
//
1111

12+
using System.ComponentModel;
1213
using System.Collections.Generic;
1314
using System.Globalization;
1415
using System.IO;
@@ -338,6 +339,49 @@ public async Task SendMailAsync_CanBeCanceled_CancellationToken()
338339
Assert.Equal(GetClientDomain(), server.ClientDomain);
339340
}
340341

342+
[Fact]
343+
public async Task SendAsync_CanBeCanceled_SendAsyncCancel()
344+
{
345+
using var server = new LoopbackSmtpServer(_output);
346+
using SmtpClient client = server.CreateClient();
347+
348+
server.ReceiveMultipleConnections = true;
349+
350+
bool first = true;
351+
352+
server.OnConnected += _ =>
353+
{
354+
if (first)
355+
{
356+
first = false;
357+
client.SendAsyncCancel();
358+
}
359+
};
360+
361+
var message = new MailMessage("[email protected]", "[email protected]", "Foo", "Bar");
362+
363+
TaskCompletionSource<AsyncCompletedEventArgs> tcs = new TaskCompletionSource<AsyncCompletedEventArgs>();
364+
client.SendCompleted += (s, e) =>
365+
{
366+
tcs.SetResult(e);
367+
};
368+
369+
client.SendAsync(message, null);
370+
AsyncCompletedEventArgs e = await tcs.Task.WaitAsync(TestHelper.PassingTestTimeout);
371+
Assert.True(e.Cancelled, "SendAsync should have been canceled");
372+
_output.WriteLine(e.Error?.ToString() ?? "No error");
373+
Assert.IsAssignableFrom<OperationCanceledException>(e.Error.InnerException);
374+
375+
// We should still be able to send mail on the SmtpClient instance
376+
await client.SendMailAsync(message).WaitAsync(TestHelper.PassingTestTimeout);
377+
378+
Assert.Equal("<[email protected]>", server.MailFrom);
379+
Assert.Equal("<[email protected]>", Assert.Single(server.MailTo));
380+
Assert.Equal("Foo", server.Message.Subject);
381+
Assert.Equal("Bar", server.Message.Body);
382+
Assert.Equal(GetClientDomain(), server.ClientDomain);
383+
}
384+
341385
private static string GetClientDomain() => IPGlobalProperties.GetIPGlobalProperties().HostName.Trim().ToLower();
342386
}
343387
}

‎src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,7 @@ private async Task ForceAuthenticationAsync<TIOAdapter>(bool receiveFirst, byte[
297297
{
298298
if (!receiveFirst)
299299
{
300-
int consumed;
301-
(token, consumed) = await NextMessage(reAuthenticationData).ConfigureAwait(false);
300+
token = NextMessage(reAuthenticationData, out int consumed);
302301
Debug.Assert(consumed == (reAuthenticationData?.Length ?? 0));
303302

304303
if (token.Size > 0)
@@ -490,14 +489,14 @@ private ProtocolToken ProcessTlsFrame(int frameSize)
490489
{
491490
int chunkSize = frameSize;
492491

493-
ReadOnlyMemory<byte> availableData = _buffer.EncryptedReadOnlyMemory;
492+
ReadOnlySpan<byte> availableData = _buffer.EncryptedReadOnlySpan;
494493

495494
// Often more TLS messages fit into same packet. Get as many complete frames as we can.
496495
while (_buffer.EncryptedLength - chunkSize > TlsFrameHelper.HeaderSize)
497496
{
498497
TlsFrameHeader nextHeader = default;
499498

500-
if (!TlsFrameHelper.TryGetFrameHeader(availableData.Slice(chunkSize).Span, ref nextHeader))
499+
if (!TlsFrameHelper.TryGetFrameHeader(availableData.Slice(chunkSize), ref nextHeader))
501500
{
502501
break;
503502
}
@@ -515,7 +514,7 @@ private ProtocolToken ProcessTlsFrame(int frameSize)
515514
chunkSize += frameSize;
516515
}
517516

518-
(ProtocolToken token, int consumed) = NextMessage(availableData.Slice(0, chunkSize)).GetAwaiter().GetResult();
517+
ProtocolToken token = NextMessage(availableData.Slice(0, chunkSize), out int consumed);
519518
_buffer.DiscardEncrypted(consumed);
520519
return token;
521520
}
@@ -524,15 +523,20 @@ private ProtocolToken ProcessTlsFrame(int frameSize)
524523
// This is to reset auth state on remote side.
525524
// If this write succeeds we will allow auth retrying.
526525
//
527-
private void SendAuthResetAndThrow(ReadOnlySpan<byte> alert, ExceptionDispatchInfo exception)
526+
private void SendAuthResetSignal(ReadOnlySpan<byte> alert, ExceptionDispatchInfo exception)
528527
{
529528
SetException(exception.SourceException);
530529

531-
if (alert.Length >= 0)
530+
if (alert.Length == 0)
532531
{
533-
InnerStream.Write(alert);
532+
//
533+
// We don't have an alert to send so cannot retry and fail prematurely.
534+
//
535+
exception.Throw();
534536
}
535537

538+
InnerStream.Write(alert);
539+
536540
exception.Throw();
537541
}
538542

@@ -587,26 +591,21 @@ private void CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions
587591
ProtocolToken alertToken = default;
588592
if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus))
589593
{
590-
ProcessFailedCertificateValidation(sslAuthenticationOptions, ref alertToken, sslPolicyErrors, chainStatus);
591-
}
592-
}
593-
594-
private void ProcessFailedCertificateValidation(SslAuthenticationOptions sslAuthenticationOptions, ref ProtocolToken alertToken, SslPolicyErrors sslPolicyErrors, X509ChainStatusFlags chainStatus)
595-
{
596-
if (sslAuthenticationOptions!.CertValidationDelegate != null)
597-
{
598-
// there may be some chain errors but the decision was made by custom callback. Details should be tracing if enabled.
599-
SendAuthResetAndThrow(new ReadOnlySpan<byte>(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.net_ssl_io_cert_custom_validation, null)));
600-
}
601-
else if (sslPolicyErrors == SslPolicyErrors.RemoteCertificateChainErrors && chainStatus != X509ChainStatusFlags.NoError)
602-
{
603-
// We failed only because of chain and we have some insight.
604-
SendAuthResetAndThrow(new ReadOnlySpan<byte>(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null)));
605-
}
606-
else
607-
{
608-
// Simple add sslPolicyErrors as crude info.
609-
SendAuthResetAndThrow(new ReadOnlySpan<byte>(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null)));
594+
if (sslAuthenticationOptions!.CertValidationDelegate != null)
595+
{
596+
// there may be some chain errors but the decision was made by custom callback. Details should be tracing if enabled.
597+
SendAuthResetSignal(new ReadOnlySpan<byte>(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.net_ssl_io_cert_custom_validation, null)));
598+
}
599+
else if (sslPolicyErrors == SslPolicyErrors.RemoteCertificateChainErrors && chainStatus != X509ChainStatusFlags.NoError)
600+
{
601+
// We failed only because of chain and we have some insight.
602+
SendAuthResetSignal(new ReadOnlySpan<byte>(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null)));
603+
}
604+
else
605+
{
606+
// Simple add sslPolicyErrors as crude info.
607+
SendAuthResetSignal(new ReadOnlySpan<byte>(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null)));
608+
}
610609
}
611610
}
612611

‎src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs‎

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
using System.Security.Authentication.ExtendedProtection;
1212
using System.Security.Cryptography;
1313
using System.Security.Cryptography.X509Certificates;
14-
using System.Threading.Tasks;
1514

1615
namespace System.Net.Security
1716
{
@@ -814,10 +813,9 @@ static DateTime GetExpiryTimestamp(SslStreamCertificateContext certificateContex
814813
}
815814

816815
//
817-
internal async Task<(ProtocolToken, int)> NextMessage(ReadOnlyMemory<byte> incomingBuffer)
816+
internal ProtocolToken NextMessage(ReadOnlySpan<byte> incomingBuffer, out int consumed)
818817
{
819-
(ProtocolToken token, int consumed) = await GenerateToken(incomingBuffer).ConfigureAwait(false);
820-
818+
ProtocolToken token = GenerateToken(incomingBuffer, out consumed);
821819
if (NetEventSource.Log.IsEnabled())
822820
{
823821
if (token.Failed)
@@ -826,7 +824,7 @@ static DateTime GetExpiryTimestamp(SslStreamCertificateContext certificateContex
826824
}
827825
}
828826

829-
return (token, consumed);
827+
return token;
830828
}
831829

832830
/*++
@@ -842,7 +840,7 @@ generates a set of bytes that will be sent next to
842840
Return:
843841
token - ProtocolToken with status and optionally buffer.
844842
--*/
845-
private async Task<(ProtocolToken, int)> GenerateToken(ReadOnlyMemory<byte> inputBuffer)
843+
private ProtocolToken GenerateToken(ReadOnlySpan<byte> inputBuffer, out int consumed)
846844
{
847845
bool cachedCreds = false;
848846
bool sendTrustList = false;
@@ -851,9 +849,6 @@ generates a set of bytes that will be sent next to
851849
ProtocolToken token = default;
852850
token.RentBuffer = true;
853851

854-
int consumed = 0;
855-
int tmpConsumed;
856-
857852
// We need to try get credentials at the beginning.
858853
// _credentialsHandle may be always null on some platforms but
859854
// _securityContext will be allocated on first call.
@@ -866,7 +861,6 @@ generates a set of bytes that will be sent next to
866861
{
867862
do
868863
{
869-
retry:
870864
thumbPrint = null;
871865
if (refreshCredentialNeeded)
872866
{
@@ -882,8 +876,8 @@ generates a set of bytes that will be sent next to
882876
token = SslStreamPal.AcceptSecurityContext(
883877
ref _credentialsHandle!,
884878
ref _securityContext,
885-
inputBuffer.Span,
886-
out tmpConsumed,
879+
inputBuffer,
880+
out consumed,
887881
_sslAuthenticationOptions);
888882
if (token.Status.ErrorCode == SecurityStatusPalErrorCode.HandshakeStarted)
889883
{
@@ -911,8 +905,8 @@ generates a set of bytes that will be sent next to
911905
ref _credentialsHandle!,
912906
ref _securityContext,
913907
hostName,
914-
inputBuffer.Span,
915-
out tmpConsumed,
908+
inputBuffer,
909+
out consumed,
916910
_sslAuthenticationOptions);
917911

918912
if (token.Status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded)
@@ -932,20 +926,6 @@ generates a set of bytes that will be sent next to
932926
_sslAuthenticationOptions);
933927
}
934928
}
935-
consumed += tmpConsumed;
936-
inputBuffer = inputBuffer.Slice(tmpConsumed);
937-
938-
if (token.Status.ErrorCode == SecurityStatusPalErrorCode.PeerCertVerifyRequired)
939-
{
940-
await Task.Yield();
941-
if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertValidationDelegate, _sslAuthenticationOptions.CertificateContext?.Trust, ref token, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus))
942-
{
943-
ProcessFailedCertificateValidation(_sslAuthenticationOptions, ref token, sslPolicyErrors, chainStatus);
944-
}
945-
946-
goto retry;
947-
948-
}
949929
} while (cachedCreds && _credentialsHandle == null);
950930
}
951931
finally
@@ -977,7 +957,7 @@ generates a set of bytes that will be sent next to
977957
}
978958
}
979959

980-
return (token, consumed);
960+
return token;
981961
}
982962

983963
internal ProtocolToken Renegotiate()
@@ -1236,12 +1216,12 @@ private ProtocolToken CreateShutdownToken()
12361216
return default;
12371217
}
12381218

1239-
return GenerateToken(default).GetAwaiter().GetResult().Item1;
1219+
return GenerateToken(default, out _);
12401220
}
12411221

12421222
private ProtocolToken GenerateAlertToken()
12431223
{
1244-
return GenerateToken(default).GetAwaiter().GetResult().Item1;
1224+
return GenerateToken(default, out _);
12451225
}
12461226

12471227
private static TlsAlertMessage GetAlertMessageFromChain(X509Chain chain)

‎src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -207,16 +207,6 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context
207207
errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan<byte>.Empty, ref token);
208208
}
209209

210-
if (errorCode == SecurityStatusPalErrorCode.PeerCertVerifyRequired)
211-
{
212-
token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.PeerCertVerifyRequired);
213-
return token;
214-
// System.Console.WriteLine($"Peer certificate verification required. setting VerifyResult to an error");
215-
// Interop.Ssl.SslSetVerifyResult((SafeSslHandle)context, 0);
216-
// // continue with the handshake, the callback will be invoked later.
217-
// errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan<byte>.Empty, ref token);
218-
}
219-
220210
// sometimes during renegotiation processing message does not yield new output.
221211
// That seems to be flaw in OpenSSL state machine and we have workaround to peek it and try it again.
222212
if (token.Size == 0 && Interop.Ssl.IsSslRenegotiatePending((SafeSslHandle)context))
@@ -249,9 +239,6 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context
249239

250240
public static SecurityStatusPal ApplyAlertToken(SafeDeleteContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage)
251241
{
252-
// All the alerts that we manually propagate do with certificate validation
253-
Interop.Ssl.SslSetVerifyResult((SafeSslHandle)securityContext!, 28);
254-
255242
// There doesn't seem to be an exposed API for writing an alert,
256243
// the API seems to assume that all alerts are generated internally by
257244
// SSLHandshake.

‎src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ internal enum SecurityStatusPalErrorCode
3434
Renegotiate,
3535
TryAgain,
3636
HandshakeStarted,
37-
PeerCertVerifyRequired,
3837

3938
// Errors
4039
OutOfMemory,

0 commit comments

Comments
 (0)