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

未关闭
#1,866 8 条评论 0 个 reaction 已指派 1 人 已被 @ldetmer 认领 在 GitHub 查看
priority: p3 type: feature request
主要语言
Java
星标
1.4k
派生
473
PR 合并指标
30 天内没有已合并 PR

描述

(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.

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。