fix(security): 默认校验服务器证书,随机串改用 SecureRandom,签名比较改为定时安全 - #4081
fix(security): 默认校验服务器证书,随机串改用 SecureRandom,签名比较改为定时安全#4081devin-ai-integration[bot] wants to merge 2 commits into
Conversation
- DefaultApacheHttpClientBuilder / DefaultHttpComponentsClientBuilder 默认不再信任所有证书, hc5 实现同时改用 DefaultHostnameVerifier,新增 skipServerCertificateVerification 开关供调试场景使用 - WxCryptUtil、RandomUtils、wxpay v3 SignUtils 改用 SecureRandom 生成随机串/nonce - WxCryptUtil 回调签名比较改为 MessageDigest.isEqual,避免时序攻击 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec981e9609
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (this.skipServerCertificateVerification) { | ||
| //忽略掉对服务器端证书的校验 | ||
| .loadTrustMaterial((TrustStrategy) (chain, authType) -> true).build(); | ||
| sslcontext = SSLContexts.custom().loadTrustMaterial((TrustStrategy) (chain, authType) -> true).build(); |
There was a problem hiding this comment.
当调用者把 DefaultApacheHttpClientBuilder.get().setSkipServerCertificateVerification(true) 用于自签名抓包或测试环境时,这里创建的 trust-all SSLConnectionSocketFactory 实际不会参与建连:prepare() 已经用注册表里的 this.sslConnectionSocketFactory 构造了 PoolingHttpClientConnectionManager,随后在 builder 上调用的 setSSLSocketFactory(...) 会被该 connection manager 覆盖。因此 Apache 4 默认客户端仍会按默认 trust store 校验证书,和本次改好的 HC5 客户端行为不一致;应把根据开关生成的 socket factory 注册进 connection manager。
Useful? React with 👍 / 👎.
binarywang
left a comment
There was a problem hiding this comment.
发现两处需要修改后再合并:
-
Apache HttpClient 4 的
skipServerCertificateVerification没有接入实际请求使用的连接池。prepare()先以字段sslConnectionSocketFactory创建并注册PoolingHttpClientConnectionManager,再调用HttpClientBuilder#setSSLSocketFactory(buildSSLConnectionSocketFactory())。在已显式设置 connection manager 时,后者不会替换 manager registry 的 socket factory,因此开关设为true后请求仍会执行系统证书校验,自签名抓包代理无法使用。请在创建 HTTPS registry 时使用按开关构造的 factory,并保留显式自定义 factory 的优先级。 -
新增
ServerCertificateVerificationTest未登记到weixin-java-common/src/test/resources/testng.xml;该模块 Surefire 固定执行该 suite,CI/常规模块测试不会运行它。且当前只验证 Lombok getter/setter,无法发现第 1 项的 TLS 配置未生效。请将测试接入 suite,并增加真实 TLS 行为验证:默认拒绝无效证书、显式跳过后可连接本地自签名 HTTPS 服务(Apache 4 与 HC5)。
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
两处均已修复,见 3da9f64。 1. 证书校验开关接入连接池
SSLConnectionSocketFactory httpsSocketFactory = this.resolveSSLConnectionSocketFactory();
registry = ...register("https", httpsSocketFactory)...
httpClientBuilder.setConnectionManager(connectionManager).setSSLSocketFactory(httpsSocketFactory)
2. 测试接入 suite 并改为真实 TLS 行为验证
另外发现一个既有问题: |
Summary
安全扫描后修复三类问题,均位于对外默认路径上。
1. 默认 HTTP 客户端关闭了 TLS 校验(最严重)
DefaultApacheHttpClientBuilder(mp/miniapp/cp/open/channel/qidian 的默认客户端)无条件信任任意证书;DefaultHttpComponentsClientBuilder(hc5)在此基础上还使用NoopHostnameVerifier,等于对所有微信接口调用完全放弃 TLS 身份校验,中间人可窃取 access_token、jsapi_ticket、用户数据等。两个 builder 都新增
skipServerCertificateVerification(默认false,@Data生成 getter/setter),为抓包代理等自签名证书调试场景保留显式开关,行为变化仅影响此前依赖“忽略证书”的用法。2. 加解密随机数使用
java.util.RandomWxCryptUtil.genRandomStr()生成的 16 位随机前缀是 AES-CBC 报文中唯一的每条消息熵(IV 固定为 aesKey 前 16 字节),RandomUtils.getRandomStr()用于 JSAPI 签名 nonceStr,wxpay v3 SignUtils.genRandomStr()用于支付请求 nonce_str,三处均改为SecureRandom。3. 回调签名比较非定时安全
WxCryptUtil.decryptContent的signature.equals(msgSignature)改为MessageDigest.isEqual(...)。其余各 service 的checkSignature仍使用equals,属于同类但风险更低的问题,未在本 PR 内改动。扫描中未发现的问题:硬编码密钥/凭据、SQL 注入、CORS 配置、暴露的调试端点(SDK 内无 Controller/Endpoint);XXE 防护(
XmlUtils、WxCryptUtil、BaseWxPayResult)与 XStream 白名单已就位;依赖版本未见已知高危 CVE。验证
mvn -DskipTests install(全仓库)通过。DefaultApacheHttpClientBuilder/DefaultHttpComponentsClientBuilder实测:https://self-signed.badssl.com抛SSLHandshakeException: PKIX path building failed,https://wrong.host.badssl.com抛SSLPeerUnverifiedException,https://api.weixin.qq.com/cgi-bin/getcallbackip返回 200。WxCryptUtil加解密回环正常、错误签名仍抛“加密消息签名校验失败”。ServerCertificateVerificationTest断言两个 builder 默认不跳过证书校验(TestNG 通过;注意仓库根 POM 中 surefire 配置为<skip>true</skip>,测试需手工执行)。WxCryptUtilTest在本机手工执行失败,但在改动前的 develop 上同样失败(commons-codec对样例 EncodingAESKey 的 base64 严格校验),与本次改动无关。Link to Devin session: https://app.devin.ai/sessions/cf7c33c129b7468093297a5c3e1e6427
Requested by: @binarywang