google-http-client/src/main/java/com/google/api/client/util/SslUtils.java contains code that looks unsafe and so triggers TrustAllX509TrustManager on Android Lint

[paulthomson]

(Googler working on Android Studio)

We received this bug report: https://issuetracker.google.com/227306334 Lint error and project build problem with com.google.api-client:google-api-client-android library

It looks like this issue was already reported here in the past as a GitHub issue: #1794

Steps to reproduce

1. Create new Android project
2. Add a dependency on: implementation("com.google.http-client:google-http-client:1.42.3")
3. Run ./gradlew lint

com/google/api/client/util/SslUtils$1.class: Error: checkServerTrusted is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers [TrustAllX509TrustManager]

   Explanation for issues of type "TrustAllX509TrustManager":
   This check looks for X509TrustManager implementations whose
   checkServerTrusted or checkClientTrusted methods do nothing (thus trusting
   any certificate chain) which could result in insecure network traffic
   caused by trusting arbitrary TLS/SSL certificates presented by peers.

This lint check scans Java bytecode looking for X509TrustManager implementations whose checkServerTrusted or checkClientTrusted methods do nothing. Most lint checks just look at the developer's source code in Android Studio. Since this check looks at bytecode, it also scans the dependencies, and google-http-client seems to ship with some code that creates a X509TrustManager that looks unsafe:

google-http-java-client/google-http-client/src/main/java/com/google/api/client/util/SslUtils.java

Line 155 in 223dfef

public static SSLContext trustAllSSLContext() throws GeneralSecurityException {

Unfortunately, the check triggers even if the user's code doesn't actually call the unsafe code. However, since this is a potential security risk, the false-positive is possibly working as intended: it seems unnecessary for a real app (not a test version of the app) to ship with such code, and so it is arguably worth warning users about this.

Android Lint does look for suppression annotations. I tried experimenting locally, but there are two problems: (a) The usual SuppressWarnings annotation only has source retention, and so would not make it to the released jar; (b) currently, for bytecode lint checks, Lint only looks for suppress annotations called SuppressLint on fields. I will create a lint issue to track adding support for suppress annotations in bytecode on methods.

In my opinion, the ideal fix would be for this code:

google-http-java-client/google-http-client/src/main/java/com/google/api/client/util/SslUtils.java

Line 155 in 223dfef

public static SSLContext trustAllSSLContext() throws GeneralSecurityException {

to be removed, or moved to a different jar (intended to be used for testing purposes only), such that real released apps could depend on google-http-client without pulling in this code that looks unsafe.

[ldetmer]

@paulthomson apologies for the delay, are you still seeing this issue?

[ldetmer]

friendly ping @paulthomson , if this is still not an issue we will close

[paulthomson]

Sorry for the delay. Yes, this issue is still there.

[habeebahmed-google]

Hi, is there any update on this issue?

[ldetmer]

Thanks @paulthomson for your comment on PR 2091

I'm trying to find a more generic solution, however if I test with instructions provided (creating android project + including dependency "com.google.http-client:google-http-client:1.46.3"
and running gradlew lint, I don't see this issue flagged. Can you please share the project you are having this issue with? Do you have extra lint checks?

[paulthomson]

It works for me. I did not have to enable any extra lint checks. To a new sample project (created using the latest Android Studio), I added to app/build.gradle.kts:

dependencies {
// ...
    implementation("com.google.http-client:google-http-client:1.46.3") // added this
// ...
}

And then ran ./gradlew lint. This succeeds, but it writes a report:

Wrote HTML report to file:///data/AndroidStudioProjects/MyApplication35/app/build/reports/lint-results-debug.html

In the report, there are two security warnings for "Insecure TLS/SSL trust manager".

[ldetmer]

Thanks Paul for the reproducer, I am able to see the issue now.

I was able to ignore by adding ignore for that specific resouce:

<?xml version="1.0" encoding="UTF-8"?>
<lint>
  <issue id="TrustAllX509TrustManager" severity="ignore">
    <ignore path=".*/google-http-client/.*\.java" />
  </issue>

</lint>

Can you confirm this works for you? I don't want to add an android specific lint suppressor to the generic http project. I will prioritize this for a fix it and will try to come up with a more generic solution