feat(sdk): Add mTLS client certificate support#6024
Open
lazard36 wants to merge 7 commits into
Open
Conversation
Add support for mutual TLS (mTLS) authentication by allowing clients to provide a client certificate (identity) that will be presented to the server during the TLS handshake. Changes: - Add `client_identity` field to `HttpSettings` in `native.rs` - Add `client_certificate()` method to `ClientBuilder` - Expose `client_certificate()` via FFI for Swift bindings The feature accepts PKCS#12/PFX certificate data that can be used with reqwest::Identity for mutual TLS authentication.
The `reqwest::Identity` type and related methods have different availability depending on the TLS backend: - `Identity` is available with both `native-tls` and `rustls-tls` - `Identity::from_pkcs12_der()` is only available with `native-tls` - `Identity::from_pem()` is available with both backends This commit fixes compilation errors when building with different TLS feature combinations by: 1. Making the `Identity` import conditional on TLS features 2. Making the `client_identity` field in `HttpSettings` conditional 3. Making the `client_certificate()` method conditional on TLS features 4. In FFI, restricting PKCS#12 client certificate support to `native-tls` only, since `from_pkcs12_der()` is not available with rustls
…compatibility The doctest was using Identity::from_pkcs12_der() which only exists with the native-tls feature. Changed to Identity::from_pem() which works with both native-tls and rustls-tls backends. Also updated the documentation to clarify that PKCS#12 format requires the native-tls feature.
The reqwest::Identity API varies between TLS backends, so the doctest is marked as ignore since it's demonstrating the concept rather than providing runnable code.
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
|
Updated the client_certificate doctest to use the correct reqwest Identity constructors: - from_pkcs8_pem(cert_pem, key_pem) for rustls-tls - from_pkcs12_der(p12_data, password) for native-tls The doctest is marked as ignore since API availability varies by TLS backend.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6024 +/- ##
==========================================
+ Coverage 88.59% 88.95% +0.36%
==========================================
Files 364 359 -5
Lines 104341 99290 -5051
Branches 104341 99290 -5051
==========================================
- Hits 92438 88324 -4114
+ Misses 7537 6976 -561
+ Partials 4366 3990 -376 ☔ View full report in Codecov by Sentry. |
Contributor
|
Our TLS situation is a bit chaotic currently as the switch to the This will have to wait for #6334 and rustls/rustls-platform-verifier#221 which will let us revert #6328 as described in #6319. |
Contributor
|
Alright, our cert verification situation got resolved (more info at #6319). If you're still up for it, this can be pushed over the finish line. It would need a rebase, though. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds mutual TLS (mTLS) authentication support to the matrix-rust-sdk, enabling clients to provide client certificates during TLS handshakes.
client_identityfield toHttpSettingsfor storing client certificatesclient_certificate()method onClientBuildernative-tlsandrustls-tlsbackends with proper conditional compilationNote: This PR supersedes #5988 and includes fixes for the TLS conditional compilation issues raised during review.
Key changes:
reqwest::Identityimport and usage is now conditional on TLS features being enablednative-tlsonly (sincefrom_pkcs12_der()is not available with rustls)client_certificate()method works with both TLS backends (users can useIdentity::from_pem()with rustls)Test plan
native-tlsfeaturerustls-tlsfeature