Skip to content

Review CustomChain validation#69

Merged
rido-min merged 1 commit intomainfrom
feat/ca-file
Oct 11, 2023
Merged

Review CustomChain validation#69
rido-min merged 1 commit intomainfrom
feat/ca-file

Conversation

@rido-min
Copy link
Copy Markdown
Contributor

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

@rido-min rido-min requested a review from CIPop September 25, 2023 21:55
@rido-min
Copy link
Copy Markdown
Contributor Author

I was waiting to see if dotnet/MQTTnet#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
Copy link
Copy Markdown

CIPop commented Oct 10, 2023

@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.

@rido-min rido-min merged commit a5ce9d4 into main Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants