Skip to content

Restore certificate validation for cached HTTP clients - #1334

Open
jpantonow wants to merge 1 commit into
ReadYouApp:mainfrom
jpantonow:fix/setupssl-sslcontextspec-6d0a3cf73013-e70fe9141c
Open

jpantonow wants to merge 1 commit into
ReadYouApp:mainfrom
jpantonow:fix/setupssl-sslcontextspec-6d0a3cf73013-e70fe9141c

Conversation

@jpantonow

Copy link
Copy Markdown

Summary

Affects OkHttpClientModuleKt.setupSsl in app/src/main/java/me/ash/reader/infrastructure/di/OkHttpClientModule.kt.

I found that cachingHttpClient defaulted trustAllCerts to true, 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 existing trustAllCerts=true path 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. SSLContextSpec is the rule this code did not satisfy.

How was this tested?

  • No automated test was added in this change
  • Project compiles successfully (: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.

Signed-off-by: jpantonow <jpantonow@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant