-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TLS certificate strings and custom SNI validation #242
Comments
As you probably know, this library is currently still just a wrapper around Paho C, which does all the network I/O including the TLS layer. So to load the certificate from memory, we'd need to add the feature to the upstream library. There have been requests for this across the projects for a while now, back to 2020 in #103. There's even a PR up for it in the C lib: eclipse-paho/paho.mqtt.c#1328. I'm with you 100%. I'd like to get this in. I can maybe push on the maintainer. For 2: (SNI): Yeah on this one, too. Same issue with the upstream, but this seems easy enough to get a PR up to the C lib. I'll put an Issue up on that repo to bring it up. |
If you have the ability to test out a C client for the SNI feature, that might help to get it landed. See: eclipse-paho/paho.mqtt.c#1582 If not, I can try to hack a branch of the Rust client here that wraps that C branch. |
But the host name check seems to still use the IP and fails when using
|
Yes. I just caught this as well when I did some additional testing. I’ll have a look shortly. |
OK. I pushed another commit into that branch in the Paho C repo, and it now uses the optional server name for both the SNI and host name checks. I need to run the unit tests and do some minor cleanup, but I think this is the basic fix. |
That fixed it, thanks! |
Arghhh... but it failed CI. Note quite done yet... |
OK. Found a couple copy-paste errors in what I did for the synchronous client which broke the high availability (multiple URLs) check. I kept sending the same URL to verify all the server names. Oops. I pushed a fix and all the unit tests in the C lib PR are working now. |
Please consider adding these features:
Allow setting the trust_store etc. to a string that contains the certificate, instead a path. Use case: crate ships with custom CA certificate of the server, avoid creating temporary files
Allow passing a custom server name that will be used for the SNI (server name indication), instead of using the host name. Use case: host name of the connection (IP) is different than the certificate's SNI (device serial number). See Node.js TLS servername parameter and paho.mqtt.c for how this could be implemented.
The text was updated successfully, but these errors were encountered: