feat(moq-native): add platform TLS verifier selection#1960
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
WalkthroughAdds platform-native TLS certificate verification support targeting Android, iOS, and macOS. A new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
rs/moq-native/src/tls.rs (1)
62-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public error variants.
Erroris public, so the new variants should have rustdoc in addition to their#[error(...)]display text.Proposed docs
+ /// Returned when platform verification is selected together with custom TLS roots. #[error("TLS platform verifier cannot be combined with custom TLS roots")] PlatformVerifierWithCustomRoots, + /// Returned when platform verification is requested on a target/backend that cannot use it. #[error("TLS platform verifier is not supported by {0}")] PlatformVerifierUnsupported(&'static str),As per coding guidelines, “Document every exported public API symbol: each exported Rust item and each exported JS/TS symbol, including notable public members, must have a doc comment.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rs/moq-native/src/tls.rs` around lines 62 - 66, The new public Error variants in tls.rs are missing rustdoc even though Error is exported. Add doc comments to PlatformVerifierWithCustomRoots and PlatformVerifierUnsupported in the Error enum, using their variant names to describe when each is returned, while keeping the existing #[error(...)] display strings unchanged.Source: Coding guidelines
rs/moq-ffi/src/android.rs (1)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd module-level and exported-symbol docs.
Per coding guidelines, every Rust module root needs a
//!doc block and each exported public symbol needs a doc comment. This module has neither a top-level//!block nor a doc comment on the publicJava_dev_moq_PlatformTLS_initializeNativeJNI entry point.📝 Suggested docs
+//! Android JNI entry points for initializing the platform TLS verifier. + use jni::EnvUnowned; use jni::errors::ThrowRuntimeExAndDefault; use jni::objects::{JClass, JObject}; +/// JNI entry point invoked by `dev.moq.PlatformTLS.initializeNative` to wire up +/// the rustls platform verifier with the Android `Context`. #[unsafe(no_mangle)] pub extern "system" fn Java_dev_moq_PlatformTLS_initializeNative<'local>(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rs/moq-ffi/src/android.rs` around lines 1 - 16, Add a top-level module doc block for this Rust module and a doc comment for the exported JNI entry point. In android.rs, add a `//!` module-level description at the top of the file, and document `Java_dev_moq_PlatformTLS_initializeNative` with a brief explanation of what the native initializer does so the exported symbol meets the coding guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kt/moq/build.gradle.kts`:
- Line 101: The moq Gradle module depends on rustls-platform-verifier, but the
current repository setup does not expose that artifact. Update the Gradle
configuration around build.gradle.kts to wire in the custom on-disk rustls
Android repository used for rustls:rustls-platform-verifier:0.1.1, and ensure
the repository is available before the implementation dependency in the moq
module is resolved.
In `@rs/moq-native/src/quiche.rs`:
- Around line 181-184: The quiche verification path in quiche.rs is ignoring the
webpki_roots policy when certs is empty, so explicit WebPki selection can fall
back to quiche defaults instead of the configured Mozilla/WebPKI roots. Update
the Verification::Roots handling to either install equivalent roots on the
builder even when certs is empty, or fail fast with an unsupported-verifier
error rather than silently proceeding. Use the Verification::Roots match branch
and the builder.with_root_certificates flow as the primary places to adjust.
---
Nitpick comments:
In `@rs/moq-ffi/src/android.rs`:
- Around line 1-16: Add a top-level module doc block for this Rust module and a
doc comment for the exported JNI entry point. In android.rs, add a `//!`
module-level description at the top of the file, and document
`Java_dev_moq_PlatformTLS_initializeNative` with a brief explanation of what the
native initializer does so the exported symbol meets the coding guidelines.
In `@rs/moq-native/src/tls.rs`:
- Around line 62-66: The new public Error variants in tls.rs are missing rustdoc
even though Error is exported. Add doc comments to
PlatformVerifierWithCustomRoots and PlatformVerifierUnsupported in the Error
enum, using their variant names to describe when each is returned, while keeping
the existing #[error(...)] display strings unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93fda425-6875-4a5d-a60b-42b27a08fbe2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
kt/moq/build.gradle.ktskt/moq/src/androidMain/kotlin/dev/moq/PlatformTLS.ktrs/moq-ffi/Cargo.tomlrs/moq-ffi/build.shrs/moq-ffi/src/android.rsrs/moq-ffi/src/lib.rsrs/moq-ffi/src/session.rsrs/moq-native/Cargo.tomlrs/moq-native/src/quiche.rsrs/moq-native/src/tls.rs
| dependsOn(jvmAndAndroidMain) | ||
| dependencies { | ||
| implementation("net.java.dev.jna:jna:5.18.1@aar") | ||
| implementation("rustls:rustls-platform-verifier:0.1.1") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Is a custom maven repo for the verifier configured anywhere in the Gradle setup?
rg -nP -C2 'rustlsPlatformVerifier|findRustlsPlatformVerifier|rustls-platform-verifier-android' kt/ 2>/dev/null
# What android-support version does the Rust crate resolve to?
cargo metadata --format-version 1 --manifest-path rs/moq-ffi/Cargo.toml 2>/dev/null \
| jq -r '.packages[] | select(.name=="rustls-platform-verifier-android") | .version'Repository: moq-dev/moq
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== build files =='
git ls-files 'kt/**/build.gradle.kts' 'kt/**/settings.gradle.kts' 'kt/**/gradle.properties' 'kt/**/gradle/libs.versions.toml' | sed 's#^`#-` #'
echo
echo '== rustls references =='
rg -n --hidden --glob '!**/build/**' --glob '!**/.gradle/**' 'rustls-platform-verifier|rustlsPlatformVerifier|findRustlsPlatformVerifier|cargo metadata|rustls' kt rs .github . 2>/dev/null | sed -n '1,200p'
echo
echo '== target file excerpt =='
cat -n kt/moq/build.gradle.kts | sed -n '1,180p'Repository: moq-dev/moq
Length of output: 21397
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== target file size =='
wc -l kt/moq/build.gradle.kts
echo
echo '== target file around dependency block =='
sed -n '70,120p' kt/moq/build.gradle.kts | cat -nRepository: moq-dev/moq
Length of output: 2135
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== relevant files =='
git ls-files | rg '^(kt/moq/build\.gradle\.kts|kt/.*/build\.gradle\.kts|.*Cargo\.toml|.*settings\.gradle(\.kts)?|.*libs\.versions\.toml)$' | sed -n '1,200p'
echo
echo '== kt/moq/build.gradle.kts excerpt =='
sed -n '1,180p' kt/moq/build.gradle.kts | cat -n
echo
echo '== search for repository declarations =='
rg -n --hidden --glob '!**/build/**' --glob '!**/.gradle/**' 'maven\s*\{|mavenLocal\(|mavenCentral\(|url\s*=\s*uri\(|rustls-platform-verifier' kt . 2>/dev/null | sed -n '1,200p'Repository: moq-dev/moq
Length of output: 8887
🌐 Web query:
rustls-platform-verifier-android Maven Central published rustls-platform-verifier 0.1.1
💡 Result:
The crate rustls-platform-verifier-android version 0.1.1 is published on crates.io [1], not Maven Central [2][3]. It serves as an internal implementation detail for the rustls-platform-verifier Rust crate to provide necessary Kotlin/JVM components for Android certificate validation [4][1]. Because it is not available on Maven Central, the project uses a mechanism where the Android components are bundled within the Rust crate [5][4]. To integrate this in an Android project, developers typically use a provided Gradle script helper [2][6]. This script works by locating the downloaded crate on the local file system (where Cargo stores it), identifying the internal Maven-compatible directory structure contained within that crate, and registering it as a local Maven repository for the Gradle build [2][5][6]. This allows the build system to treat the local files as a Maven dependency without the need for the artifact to be hosted on a public repository like Maven Central [4][6].
Citations:
- 1: https://crates.io/crates/rustls-platform-verifier-android/0.1.1
- 2: https://docs.rs/rustls-platform-verifier/latest/rustls_platform_verifier/
- 3: https://github.com/rustls/rustls-platform-verifier
- 4: https://docs.rs/crate/rustls-platform-verifier-android/0.1.1
- 5: https://crates.io/crates/rustls-platform-verifier
- 6: https://docs.rs/crate/rustls-platform-verifier/latest
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== root search for rustls / custom repo plumbing =='
rg -n --hidden --glob '!**/build/**' --glob '!**/.gradle/**' \
'rustls-platform-verifier-android|rustls-platform-verifier|cargo metadata|mavenLocal\(|maven \{|url\s*=\s*uri\(' . | sed -n '1,200p'
echo
echo '== build file around dependencies and repositories =='
sed -n '1,180p' kt/moq/build.gradle.kts | cat -nRepository: moq-dev/moq
Length of output: 8654
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== gradle files =='
git ls-files 'kt/**/build.gradle.kts' 'kt/**/settings.gradle.kts' | sed -n '1,200p'
echo
echo '== kt/moq/build.gradle.kts =='
cat -n kt/moq/build.gradle.kts | sed -n '1,180p'
echo
echo '== kt settings/root build excerpts =='
for f in $(git ls-files 'kt/**/settings.gradle.kts' 'kt/**/build.gradle.kts' | grep -v '^kt/moq/build.gradle.kts$'); do
echo "--- $f ---"
cat -n "$f" | sed -n '1,220p'
doneRepository: moq-dev/moq
Length of output: 7118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== kt tree =='
find kt -maxdepth 2 -type f | sort | sed -n '1,200p'
echo
echo '== rustls / maven repo helpers in kt scripts =='
rg -n --hidden --glob '!**/build/**' --glob '!**/.gradle/**' \
'rustls-platform-verifier|cargo metadata|mavenRepo|mavenLocal|repositories\s*\{|url\s*=\s*uri\(|artifactId|find.*repo' kt/scripts kt/moq kt/settings.gradle.kts 2>/dev/null | sed -n '1,240p'
echo
echo '== kt/settings.gradle.kts =='
cat -n kt/settings.gradle.kts | sed -n '1,220p'Repository: moq-dev/moq
Length of output: 1786
Add the rustls Android repository for this dependency.
mavenCentral()/google() don’t provide rustls:rustls-platform-verifier:0.1.1, so this implementation(...) needs the custom on-disk repo wired in somewhere in the Gradle setup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kt/moq/build.gradle.kts` at line 101, The moq Gradle module depends on
rustls-platform-verifier, but the current repository setup does not expose that
artifact. Update the Gradle configuration around build.gradle.kts to wire in the
custom on-disk rustls Android repository used for
rustls:rustls-platform-verifier:0.1.1, and ensure the repository is available
before the implementation dependency in the moq module is resolved.
| Verification::Roots { certs, .. } => { | ||
| if !certs.is_empty() { | ||
| builder = builder.with_root_certificates(certs); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not silently drop webpki_roots for quiche.
Verification::Roots { webpki_roots: true, certs: [] } is now a valid policy, but this branch ignores the flag and leaves quiche using its implicit defaults. That means explicit WebPki selection—and Android’s default verifier—may not actually use the configured Mozilla/WebPKI roots in this backend. Either install equivalent roots for quiche or return an unsupported-verifier error instead of proceeding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rs/moq-native/src/quiche.rs` around lines 181 - 184, The quiche verification
path in quiche.rs is ignoring the webpki_roots policy when certs is empty, so
explicit WebPki selection can fall back to quiche defaults instead of the
configured Mozilla/WebPKI roots. Update the Verification::Roots handling to
either install equivalent roots on the builder even when certs is empty, or fail
fast with an unsupported-verifier error rather than silently proceeding. Use the
Verification::Roots match branch and the builder.with_root_certificates flow as
the primary places to adjust.
1 participant
This PR adds an ability to set what kind of TLS verifier will be used when connecting with a server.
The previous approach didn't work on iOS and android platforms. The current approach allows the user to choose and also provides a sane defaults for each platform (it should be backwards compatible for platforms other than ios/android/macos).