SSLProtocols default value is invalid under .net 4.6 and earlier

[simonthum]

Describe the bug

Just enabling TLS in MQTTnet will yield a Sslprotocols value of SslProtocols.None. This value is invalid under .net 4.6 and earlier, a fact enforced in .net 4.8 too. It throws ArgumentException, see below.

Nota bene: In 4.7 and beyond, SslProtocols.None leaves it to the OS to decide, which is not a bad idea, so in any case the fix should change the default only for affected targets.

.net side enforcement:

https://referencesource.microsoft.com/#System/net/System/Net/SecureProtocols/_SslState.cs,165

Default 1:

https://github.com/dotnet/MQTTnet/blob/d5691346a9d2433e3530fda571f1c8defbe70c6f/Source/MQTTnet/Client/Options/MqttClientOptionsBuilderTlsParameters.cs#L22

Default 2:

https://github.com/dotnet/MQTTnet/blob/d5691346a9d2433e3530fda571f1c8defbe70c6f/Source/MQTTnet/Client/Options/MqttClientTlsOptions.cs#L29

Which project is your bug related to?

• Client
• ManagedClient

To Reproduce

Open a connection using TLS in a .net 4.6 vanilla (no special SSL settings, neither in .net nor MQTTnet) compiled app. You do not need an actual broker. We got this tace (german):

MQTTnet.Exceptions.MqttCommunicationException: Der angegebene Wert ist in der Enumeration SslProtocolType ungültig.
Parametername: sslProtocolType ---> System.ArgumentException: Der angegebene Wert ist in der Enumeration SslProtocolType ungültig.
Parametername: sslProtocolType
   bei System.Net.Security.SslState.ValidateCreateContext(Boolean isServer, String targetHost, SslProtocols enabledSslProtocols, X509Certificate serverCertificate, X509CertificateCollection clientCertificates, Boolean remoteCertRequired, Boolean checkCertRevocationStatus, Boolean checkCertName)
   bei System.Net.Security.SslStream.BeginAuthenticateAsClient(String targetHost, X509CertificateCollection clientCertificates, SslProtocols enabledSslProtocols, Boolean checkCertificateRevocation, AsyncCallback asyncCallback, Object asyncState)
   bei System.Net.Security.SslStream.<>c__DisplayClass32_0.<AuthenticateAsClientAsync>b__0(AsyncCallback callback, Object state)
   bei System.Threading.Tasks.TaskFactory`1.FromAsyncImpl(Func`3 beginMethod, Func`2 endFunction, Action`1 endAction, Object state, TaskCreationOptions creationOptions)
   bei System.Net.Security.SslStream.AuthenticateAsClientAsync(String targetHost, X509CertificateCollection clientCertificates, SslProtocols enabledSslProtocols, Boolean checkCertificateRevocation)
   bei MQTTnet.Implementations.MqttTcpChannel.<ConnectAsync>d__15.MoveNext()
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   bei System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   bei System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   bei MQTTnet.Internal.MqttTaskTimeout.<WaitAsync>d__0.MoveNext()
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   bei System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   bei System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   bei MQTTnet.Adapter.MqttChannelAdapter.<ConnectAsync>d__28.MoveNext()
   --- Ende der internen Ausnahmestapelüberwachung ---
   bei MQTTnet.Adapter.MqttChannelAdapter.WrapAndThrowException(Exception exception)
   bei MQTTnet.Adapter.MqttChannelAdapter.<ConnectAsync>d__28.MoveNext()
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   bei System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   bei System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   bei MQTTnet.Client.MqttClient.<ConnectAsync>d__34.MoveNext()
--- Ende der Stapelüberwachung vom vorhergehenden Ort, an dem die Ausnahme ausgelöst wurde ---
   bei System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   bei MQTTnet.Client.MqttClient.<ConnectAsync>d__34.MoveNext()

Expected behavior

Some default suite of SSL versions should be picked to at least attempt a connection.

Code example

clientOptionsBuilder.WithTls(); // Would work if we weren't still on .net 4.6 ;)

[simonthum]

This was last changed in Oct. 2021: https://github.com/dotnet/MQTTnet/pull/1271

I favor leaving SSLprotocols to the OS, but older .net will fail, so you have to specify. Note this is an enum Flag, you can specify SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, no need to single out a specific revision.

[simonthum]

To be clear, this isn't so bad. It's quite straightforward to get from error message to the workaround, i.e. explicitly naming TLS versions. But I still think a default value should not cause needless failure.

[chkr1011]

That should be a small change. I will consider adding this soon.

[simonthum]

Thank you!

[SeppPenner]

The preprocessor directives can be found here: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives (Just for reference that it's here as well).

I have added #if NET452 || NET46 || NET461 || NET462. Is there any other versions (NetStandard, UWP, ...) concerned as well with this problem?

[simonthum]

@SeppPenner None that I know of. (451 and 45 are not on your support list, I suppose)

[simonthum]

Ms devotes a document to the topic. I linked the relevant section, it's a long read.

If I'm reading this right .net 4.7 is affected, but not 4.7.1 and above. Others are not mentioned.

[SeppPenner]

@SeppPenner None that I know of. (451 and 45 are not on your support list, I suppose)

Yes, only .NET Framework 4.5.2+ is supported :)

Ms devotes a document to the topic. I linked the relevant section, it's a long read.

If I'm reading this right .net 4.7 is affected, but not 4.7.1 and above. Others are not mentioned.

I agree, seems like 4.7 is affected as well.

[simonthum]

Maybe is is more straightforward to use an overload without SslProtocol value when it's value is None.