MqttClientDefaultCertificateValidationHandler not used as default handler for MqttClient

[Levi--G]

Describe the bug

MqttClientDefaultCertificateValidationHandler does not seem to be used by default when using tls with the MqttClient, instead a method called InternalUserCertificateValidationCallback in MqttTcpChannel is handling the certificate.

This causes several settings to be ignored that MqttClientDefaultCertificateValidationHandler handles correctly:

• IgnoreCertificateRevocationErrors
• AllowUntrustedCertificates

Which component is your bug related to?

• Client
• ManagedClient (unsure)

To Reproduce

Steps to reproduce the behavior:

1. Using this version of MQTTnet 5.1.0.1559.
2. Run this code:

var mqttClientOptions = new MqttClientOptionsBuilder()
    .WithTcpServer("untrusted cert server", untrustercertserverport)
    .WithTlsOptions(b =>
    {
        b.UseTls(true);
// this setting is ignored by default
        b.WithAllowUntrustedCertificates(true);
        // this will make the sample work as expected: 
//b.WithCertificateValidationHandler(MqttClientDefaultCertificateValidationHandler.Handle);
    })
    .Build();
await client.ConnectAsync(mqttClientOptions, token);

3. See error:
   System.Security.Authentication.AuthenticationException: 'The remote certificate was rejected by the provided RemoteCertificateValidationCallback.'

Expected behavior

WithAllowUntrustedCertificates should work by default without setting the MqttClientDefaultCertificateValidationHandler

Screenshots

N/A

Additional context / logging

Source of MqttClientDefaultCertificateValidationHandler:

public static bool Handle(MqttClientCertificateValidationEventArgs eventArgs)
{
    if (eventArgs.SslPolicyErrors == SslPolicyErrors.None)
    {
        return true;
    }

    if (eventArgs.Chain.ChainStatus.Any(
            c => c.Status is X509ChainStatusFlags.RevocationStatusUnknown or X509ChainStatusFlags.Revoked or X509ChainStatusFlags.OfflineRevocation))
    {
        if (eventArgs.ClientOptions?.TlsOptions?.IgnoreCertificateRevocationErrors != true)
        {
            return false;
        }
    }

    if (eventArgs.Chain.ChainStatus.Any(c => c.Status == X509ChainStatusFlags.PartialChain))
    {
        if (eventArgs.ClientOptions?.TlsOptions?.IgnoreCertificateChainErrors != true)
        {
            return false;
        }
    }

    return eventArgs.ClientOptions?.TlsOptions?.AllowUntrustedCertificates == true;
}

Source of InternalUserCertificateValidationCallback:

bool InternalUserCertificateValidationCallback(object sender, X509Certificate x509Certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
{
    var certificateValidationHandler = _tcpOptions?.TlsOptions?.CertificateValidationHandler;
    if (certificateValidationHandler != null)
    {
        var eventArgs = new MqttClientCertificateValidationEventArgs(x509Certificate, chain, sslPolicyErrors, _tcpOptions);
        return certificateValidationHandler(eventArgs);
    }

    if (_tcpOptions?.TlsOptions?.IgnoreCertificateChainErrors ?? false)
    {
        sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateChainErrors;
    }

    return sslPolicyErrors == SslPolicyErrors.None;
}

Code example

Provided above.

[gajanak]

Double to #1547 from 2022 ?

[Levi--G]

Double to #1547 from 2022 ?

Very similar it is indeed. I did not go all the way back to 2022 in the issues to find it. However I think the fact that the internal callback is not actually using the settings declared in the options is a bug, rather than a feature as the original issue was classified. I merely suggested using the MqttClientDefaultCertificateValidationHandler to fix the issue that is really the options are just not working right now.