fix: support TLS 1.3 on Android <= 9 #1348
Conversation
WalkthroughThe changes modify fee management and enhance TLS support. In the CrowdNode integration, the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant F as FeeInfo
participant L as Lazy Property (value)
participant G as Gson Parser
C->>F: call getNormal()
F->>L: Access lazy property value
L->>G: Parse rawValue JSON string
G-->>L: Return List<FeeLadder>
L-->>F: Set lazy value
F-->>C: Return fee data from getNormal()
sequenceDiagram
participant A as WalletApplication
participant S as Security Framework
participant C as Conscrypt Provider
A->>A: onCreate()
A->>A: Check Android API version (≤29)
alt API condition met
A->>S: insertProviderAt(Conscrypt, 1)
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/model/CrowdNodeFeeInfo.kt(2 hunks)wallet/build.gradle(1 hunks)wallet/src/de/schildbach/wallet/WalletApplication.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (3)
wallet/build.gradle (1)
83-83: Verify the Conscrypt version for security vulnerabilities.The current version 2.5.2 might not be the latest. Let's verify if there are any security advisories or newer versions available.
✅ Verification successful
🌐 Web query:
What is the latest version of org.conscrypt:conscrypt-android and are there any known security vulnerabilities in version 2.5.2?💡 Result:
The latest version of `org.conscrypt:conscrypt-android` is **2.5.3**, as indicated by its Maven Central listing[1][8]. This artifact provides Android-specific cryptographic functionality through BoringSSL integration and is maintained by Google[1][4][5]. For **version 2.5.2**, there are no direct references to security vulnerabilities in the provided sources, though general risks should be considered: 1. **Historical vulnerabilities**: - Older Conscrypt versions had critical flaws like CVE-2016-2461 (AAD mishandling allowing authentication spoofing)[2] and CVE-2016-0818 (certificate validation bypass)[7], but these predate v2.x. - Recent advisories (CERT-In CIVN-2025-0013) warn about Android component vulnerabilities including Conscrypt[3], though they don't specify versions. 2. **Upgrade recommendations**: - Version 2.5.3 includes unspecified updates over 2.5.2[1][8]. - Android Mainline updates since v14 allow faster security patches via APEX modules[5], reducing reliance on library versions. Developers using Conscrypt should: - Upgrade to v2.5.3 unless constrained by compatibility requirements - Monitor Android Security Bulletins[3][7] - Use `NetworkSecurityConfig` for certificate management[5] - Validate native library loading[6][10] No active exploits for v2.5.2 are documented in the sources, but following standard supply-chain security practices remains crucial given open source dependencies' prevalence[1][3]. Citations: - 1: https://central.sonatype.com/artifact/org.conscrypt/conscrypt-android - 2: https://www.cvedetails.com/cve/CVE-2016-2461/ - 3: https://www.moneycontrol.com/technology/government-warns-about-multiple-security-flaws-in-android-what-users-need-to-know-article-12932684.html - 4: https://central.sonatype.com/artifact/org.conscrypt/conscrypt-android/2.5.2 - 5: https://source.android.com/docs/core/ota/modular-system/conscrypt - 6: https://groups.google.com/g/conscrypt/c/FgIPOsbTEl0 - 7: https://source.android.com/docs/security/bulletin/2016-03-01 - 8: https://mvnrepository.com/artifact/org.conscrypt/conscrypt-android/2.5.2 - 9: https://github.com/google/conscrypt - 10: https://github.com/google/conscrypt/issues/649
Update Conscrypt Dependency Version
- The current version 2.5.2 in wallet/build.gradle has no documented active security vulnerabilities.
- The latest available version is 2.5.3, which includes updates and improvements.
- While 2.5.2 remains secure based on available information, upgrading to 2.5.3 is recommended to benefit from these enhancements and ensure the best security posture.
wallet/src/de/schildbach/wallet/WalletApplication.java (1)
342-346: LGTM! Conscrypt initialization is correctly implemented.The implementation properly:
- Checks for Android Q (API 29) and below
- Inserts Conscrypt provider at the correct position (index 1)
- Enables TLS 1.3 support for older Android versions
integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/model/CrowdNodeFeeInfo.kt (1)
48-62: LGTM! Efficient implementation of fee ladder storage.The change to store fee ladder as a JSON string with lazy parsing is a good optimization:
- Reduces initial memory usage
- Improves startup time
- Maintains compatibility with the API format
| val value: List<FeeLadder> by lazy { | ||
| Gson().fromJson(rawValue, object : TypeToken<List<FeeLadder>>() {}.type) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON parsing.
The lazy property could throw JsonSyntaxException or JsonParseException if the rawValue string is malformed. Consider adding error handling to gracefully handle parsing failures.
val value: List<FeeLadder> by lazy {
- Gson().fromJson(rawValue, object : TypeToken<List<FeeLadder>>() {}.type)
+ try {
+ Gson().fromJson(rawValue, object : TypeToken<List<FeeLadder>>() {}.type)
+ } catch (e: com.google.gson.JsonSyntaxException) {
+ log.error("Failed to parse fee ladder: ${e.message}")
+ emptyList()
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val value: List<FeeLadder> by lazy { | |
| Gson().fromJson(rawValue, object : TypeToken<List<FeeLadder>>() {}.type) | |
| } | |
| val value: List<FeeLadder> by lazy { | |
| try { | |
| Gson().fromJson(rawValue, object : TypeToken<List<FeeLadder>>() {}.type) | |
| } catch (e: com.google.gson.JsonSyntaxException) { | |
| log.error("Failed to parse fee ladder: ${e.message}") | |
| emptyList() | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wallet/proguard.cfg (1)
227-229: Minor naming inconsistency in the comment.It appears the comment says "Conscript" instead of "Conscrypt," which might be a simple typo. To maintain clarity and consistency, consider updating the comment to match the library’s correct spelling.
You can fix it with the following diff:
-# Conscript +# Conscrypt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wallet/proguard.cfg(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
Issue being fixed or feature implemented
N/A
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
• New Features
- Added TLS 1.3 support on older Android devices to enhance secure connectivity.
- Introduced a new method of handling fee information, improving data access and management.
• Chores
- Added new dependency for enhanced security capabilities.
- Updated ProGuard configuration to retain necessary classes for the Conscrypt library.