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.