Repository navigation
Conversation
Signed-off-by: jpantonow <jpantonow@gmail.com>
conradlyn
added a commit
to conradlyn/ReadYou
that referenced
this pull request
Oct 20, 2026
Cherry-picks upstream PR ReadYouApp#1334 ("Restore certificate validation for cached HTTP clients"). Applied as a manual patch rather than `git cherry-pick` because OkHttpClientModule.kt is stored with CRLF in this fork (core.autocrlf=true, no .gitattributes), which makes the diff against upstream look like a whole-file rewrite. cachingHttpClient defaulted trustAllCerts to true, so the app-wide OkHttpClient used for feed fetching and Coil image loading accepted every certificate chain and every hostname. Anyone able to sit between the device and the network could replace article text and images. - OkHttpClientModule.kt: trustAllCerts defaults to false - res/xml/network_security_config.xml: new, cleartextTrafficPermitted plus system and user trust anchors - AndroidManifest.xml: reference the config from <application> The new config is required for the private-CA case: for apps targeting API 24+, Android ignores user-installed CAs unless the network security config opts into the "user" trust anchor. Closes the D1 item in PERFORMANCE-AUDIT.md. Co-Authored-By: ReadYouApp#1334
conradlyn
added a commit
to conradlyn/ReadYou
that referenced
this pull request
Oct 20, 2026
…ding - UPSTREAM-TRACKING.md 3.0.1: what actually landed, plus three things verified on the way that the PR description does not cover: platformTrustManager() was dead code before the default flipped and setupSsl is no longer called at all without a client certificate; networkSecurityConfig silently supersedes android:usesCleartextTraffic from Android 7.0 on, and the PR keeps plain HTTP working by spelling out cleartextTrafficPermitted="true"; the googlePlay flavor needs no matching edit because of manifest merging. - 3.0.2: clientCertificateAlias is a client certificate presented to the server, trustAllCerts is about validating the server. Not interchangeable. - Also corrects an earlier misstatement of mine: "intranet" and "your own RSS" are both irrelevant. Only the protocol and the issuing CA decide it. - PERFORMANCE-AUDIT.md: D1 is resolved. The old text argued the behaviour was an upstream trade-off for self-hosted FreshRSS; that premise was wrong and must not be used to revert this when merging upstream later.
conradlyn
added a commit
to conradlyn/ReadYou
that referenced
this pull request
Oct 20, 2026
…ll perf report UPSTREAM-TRACKING.md 3.0.1 gains the build evidence that was missing when the change was first pushed. The repository's GitHub Actions had been disabled at that moment, so the push produced no build and the only available conclusion was "offline checks passed", which does not prove the code compiles. Now that Actions is back, both workflows were re-run manually and the picture is complete: Assemble passed, the unit tests passed including the font baseline guard, and the released APK was inspected to confirm the new network security config is really inside it. The APK check needed one correction worth recording: AGP obfuscates resource names, so the file is res/8G.xml rather than res/xml/network_security_config.xml. Looking it up by name finds nothing and looks like the resource was dropped. It has to be located by content, where the binary XML string pool shows network-security-config, trust-anchors and the system/user certificates verbatim. READING-SCROLL-PERF.md was still untracked even though the investigation is over. It now goes in with the measured result: the WebView path is smooth on the MatePad Mini as of v0.16.2-tablet.22, the native renderer still has the higher ceiling, and B1 (handing scrolling back to the WebView) is now explicitly marked as not to be done.
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
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
Affects
OkHttpClientModuleKt.setupSslinapp/src/main/java/me/ash/reader/infrastructure/di/OkHttpClientModule.kt.I found that
cachingHttpClientdefaultedtrustAllCertstotrue, causing the default client path to install an app-defined trust manager and an always-accepting hostname verifier. Default connections should validate certificates and hostnames rather than accepting every chain.I changed the default to
false, restoring platform validation for callers that do not explicitly request trust-all behavior. The existingtrustAllCerts=truepath and client-certificate setup remain unchanged.Servers whose certificate the system does not trust (self-signed or from a private CA), accepted silently until now, are rejected. The added network security config lets their users install that CA on the device; without it Android ignores user-installed CAs for apps targeting API 24 or later.
I found this while running the app under RVSec, a runtime-verification tool that watches the app's calls to the Java cryptography and TLS APIs and checks each sequence of calls against usage rules written in CrySL.
SSLContextSpecis the rule this code did not satisfy.How was this tested?
:app:assembleFdroidDebug)I'm an undergraduate Computer Engineering student at the University of Brasília (UnB); this contribution is part of a research project on the analysis of Android apps, and I'm happy to adjust it to match the project's conventions.