Azure-Samples / MqttApplicationSamples

Samples implementing common PubSub patterns for Edge and Cloud Brokers
MIT License
26 stars 25 forks source link

Review CustomChain validation #69

Closed rido-min closed 1 year ago

rido-min commented 1 year ago

Updates the ChainValidation to reuse the chain provided by the certificate validation callback.

rido-min commented 1 year ago

I was waiting to see if https://github.com/dotnet/MQTTnet/pull/1851 was merged, but unfortunately we dont have any ETA for now.

so I'd like to merge this PR, as this will be used across other MQTT deliverables

@CIPop any thoughts?

CIPop commented 1 year ago

@rido-min the chain validation routine used is not advised for TLS validation. Ideally, TLS validation is performed by the TLS stack (as in the MQTTNet PR).

Because the validation might be dangerous in some instances, I recommend making it discoverable for the application developer / static analysis tools. This is what I have in mind:

  1. Expose or use the RemoteCertificateCallback from the client's SslStream.
  2. Create a new, public API object containing the code from X509ChainValidator
  3. Exemplify within the application how to use the callback, by calling the X509ChainValidator within the callback.

This way, the app-code must contain RemoteCertificateValidationCallback, making it obvious that custom certificate validation is being used.