DanielLeens commented on PR #11576: URL: https://github.com/apache/seatunnel/pull/11576#issuecomment-5391457548
Good question -- default JVM trust store is fine for this PR. The immediate problem is `factory.useSslProtocol()` called with no arguments, which installs RabbitMQ's `TrustEverythingTrustManager` and disables certificate validation entirely -- that is the actual vulnerability (any TLS listener, valid or not, is accepted). Switching to a proper `SSLContext` (e.g. `SSLContext.getDefault()`, or an explicit `TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm())` initialized with a null KeyStore) plus `enableHostnameVerification()` closes that gap using the JVM's already-configured CA trust store, which is what most users already rely on for other TLS-capable connectors in this repo. I'd defer configurable custom truststore/keystore options to a follow-up rather than growing this PR's scope -- `connector-elasticsearch`/`connector-easysearch` already have a precedent for that kind of dedicated `SSLUtils` if a future RabbitMQ user needs a private CA, but nothing here needs it to fix the actual bug. Minimal surface for this PR, please. Thanks for taking Issue 3 as well -- agreed a `serialVersionUID` matters here specifically because `RabbitmqConfig` rides along in Zeta's execution plan and checkpoint state, so an implicit UID recalculated across a class-shape change would throw `InvalidClassException` on restore after an upgrade. Go ahead and bundle everything into one push as planned; I'll do a full re-review once it's up. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
