googleapis / googleapis/google-http-java-client

`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

Abierto
#1,866 8 comentarios 0 reacciones 1 asignado Reclamado por @ldetmer Ver en GitHub
priority: p3 type: feature request
Lenguaje dominante
Java
Estrellas
1.4k
Forks
473
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

(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: https://github.com/googleapis/google-http-java-client/issues/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: https://github.com/googleapis/google-http-java-client/blob/223dfef05114bd64acaba5424ee6d6c44f9223b9/google-http-client/src/main/java/com/google/api/client/util/SslUtils.java#L155

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:
https://github.com/googleapis/google-http-java-client/blob/223dfef05114bd64acaba5424ee6d6c44f9223b9/google-http-client/src/main/java/com/google/api/client/util/SslUtils.java#L155

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.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.