[RORDEV-1456] Fix for CVE-2023-3894, CVE suppressions update #1101
Conversation
📝 WalkthroughWalkthroughThis update migrates the codebase from using the Elasticsearch High Level REST Client ( Sequence Diagram(s)sequenceDiagram
participant App as ROR Plugin
participant AuditSink as RestClientAuditSinkService
participant ES as Elasticsearch Cluster
App->>AuditSink: submit(indexName, documentId, jsonRecord)
loop For each RestClient in clients
AuditSink->>ES: PUT /{indexName}/ror_audit_evt/{documentId} (jsonRecord)
ES-->>AuditSink: HTTP response (async)
AuditSink->>AuditSink: Log error if response not 2xx or on failure
end
App->>AuditSink: close()
AuditSink->>ES: Close RestClient(s)
Old vs. New Audit Submission Flow sequenceDiagram
participant App as ROR Plugin
participant OldSink as HighLevelClientAuditSinkService
participant NewSink as RestClientAuditSinkService
participant ES as Elasticsearch Cluster
App->>OldSink: submit(indexName, documentId, jsonRecord)
OldSink->>ES: IndexRequest via RestHighLevelClient (async)
ES-->>OldSink: IndexResponse (async)
OldSink->>OldSink: Log error if not 2xx or on failure
App->>NewSink: submit(indexName, documentId, jsonRecord)
NewSink->>ES: PUT /{indexName}/ror_audit_evt/{documentId} via RestClient (async)
ES-->>NewSink: HTTP response (async)
NewSink->>NewSink: Log error if not 2xx or on failure
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
119-126:⚠️ Potential issueTrust-All SSL defeats the CVE mitigation goal
Creating a blanket
TrustManagerand disabling hostname verification completely nullifies TLS authentication and opens the door to MITM attacks – exactly what many CVEs try to prevent.If this is strictly required for on-prem test clusters, gate it behind a config flag and default to the JVM truststore + proper hostname verification.
es710x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
119-126: 💡 Verification agent🧩 Analysis chain
Trust-all SSL disables transport security
configureSsl()installs a permissive trust manager and disables hostname verification, effectively turning TLS into plaintext-with-encryption-only.
Unless this code runs exclusively in test environments, please:
- Load a real truststore (or allow injection of one).
- Keep
NoopHostnameVerifierbehind a feature flag.Leaving it as-is re-introduces MITM risks that CVE-oriented PRs usually strive to eliminate.
🏁 Script executed:
#!/usr/bin/env bash set -e echo "🔍 Locating usages of configureSsl:" rg "configureSsl" -n . echo echo "🔍 Locating definition or usages of createTrustAllManager:" rg "createTrustAllManager" -n .Length of output: 21783
Enforce strict SSL verification in RestClientAuditSinkService.configureSsl
This method currently installs a “trust‐all” TrustManager and disables hostname checks unconditionally, reintroducing MITM risk in any non-test environment. We need to:
• Inject or load a real truststore instead of always calling
createTrustAllManager()
• WrapNoopHostnameVerifier.INSTANCEin a feature flag (or config flag) so hostname verification remains enabled by defaultLocations needing updates (repeat for each ES client version you maintain):
- es710x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (lines 119–126)
Suggested diff sketch:
- private def configureSsl(): HttpAsyncClientBuilder => HttpAsyncClientBuilder = { builder => - val trustAllCerts = createTrustAllManager() - val sslContext = SSLContext.getInstance("TLSv1.2") - sslContext.init(null, Array(trustAllCerts), null) - builder - .setSSLContext(sslContext) - .setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE) - } + private def configureSsl(): HttpAsyncClientBuilder => HttpAsyncClientBuilder = { builder => + // Load real TrustManagers from configured keystore, fallback to test‐only trust‐all + val trustManagers = config.trustStorePath + .map(path => loadTrustManagersFromPath(path)) + .getOrElse(Array(createTrustAllManager())) + + val sslContext = SSLContext.getInstance("TLSv1.2") + sslContext.init(null, trustManagers, null) + + builder + .setSSLContext(sslContext) + .setSSLHostnameVerifier( + if (config.disableHostnameVerification) NoopHostnameVerifier.INSTANCE + else SSLConnectionSocketFactory.getDefaultHostnameVerifier() + ) + }Please apply similar changes across all
RestClientAuditSinkService.scalavariants.
♻️ Duplicate comments (28)
es79x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (2)
35-47: Sameparparallelism concern as raised for the 7.8.x modulePlease refer to the earlier comment in
es78x– the reasoning and suggested fix apply here verbatim.
128-132:getAcceptedIssuersshould return an empty arrayMirror the fix suggested for the 7.8.x service to avoid potential NPEs.
es77x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
62-68: Duplicate of the comment made for modulees717x; the same resource-leak applies here.
119-132: Same SSL trust-all concerns andgetAcceptedIssuersnull return as highlighted fores717x.
54-57: Same path-encoding observation applies.es711x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
62-68: See identical leak issue noted ines717x.
119-132: SSL trust-all andnullissuer array repeats here.
54-57: URL-encoding concern repeats here.es714x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (6)
39-47: See identical comment ines70x– the additionalparlayer is unnecessary.
49-51: See identical comment ines70xabout swallowingIOExceptionon close.
53-57: See identical comment ines70xregarding hard-coded_doctype.
93-101: See identical comment ines70x– missingsetPathPrefix.
119-126: See identical comment ines70x– trust-all SSL is a critical issue.
128-132: See identical comment ines70x– return empty array instead ofnull.es74x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (6)
39-47: See identical comment ines70x– the additionalparlayer is unnecessary.
49-51: See identical comment ines70xabout swallowingIOExceptionon close.
53-57: See identical comment ines70xregarding hard-coded_doctype.
93-101: See identical comment ines70x– missingsetPathPrefix.
119-126: See identical comment ines70x– trust-all SSL is a critical issue.
128-132: See identical comment ines70x– return empty array instead ofnull.es716x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (4)
39-47: Duplicate of the concurrency remark fores710x. Please refer to that comment.
53-57: Duplicate of the path-encoding remark fores710x.
119-126: Duplicate of the TLS trust-all concern raised ines710x.
128-132: Duplicate of thegetAcceptedIssuersnitpick raised ines710x.es72x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (4)
39-47: Duplicate of the concurrency remark fores710x.
53-57: Duplicate of the path-encoding remark fores710x.
119-126: Duplicate of the TLS trust-all concern raised ines710x.
128-132: Duplicate of thegetAcceptedIssuersnitpick raised ines710x.
🧹 Nitpick comments (13)
suppressions_cve.xml (1)
136-138: Typo in notes may confuse future maintainers
org.elasticserarch→org.elasticsearch.- file name: elasticsearch-rest-client-X.Y.Z.jar - The CVEs affect the org.elasticserarch:elasticsearch package, which is not a dependency of elasticsearch-rest-client + file name: elasticsearch-rest-client-X.Y.Z.jar - The CVEs affect the org.elasticsearch:elasticsearch package, which is not a dependency of elasticsearch-rest-clientes78x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
35-47: Parallel collections introduce needless thread contention
clients.toList.par.foreachdelegates to the global ForkJoinPool, spawning extra threads while the underlyingRestClientcall is already asynchronous. For small client lists this adds overhead; for large lists it may saturate the common pool.- clients.toList.par.foreach { client => + clients.toList.foreach { client =>Keep it simple unless benchmarking proves a throughput benefit.
es717x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
128-132: Return an empty array instead ofnull
getAcceptedIssuersreturningnullviolates theX509TrustManagercontract and can trigger NPEs in some TLS stacks.- override def getAcceptedIssuers: Array[X509Certificate] = null + override def getAcceptedIssuers: Array[X509Certificate] = Array.empty
54-57: URL-encodeindexName/documentIdIf either contains characters that need escaping (e.g. “/”, “ ”) the request path will be invalid.
UseURLEncoder.encode(Java) orio.lemonlabs.urihelpers to build the path safely.
40-46: Parallel collections may over-subscribe threads
RestClientalready dispatches requests on its own async pool. Adding.parspawns extra threads and can hurt throughput.
Consider plain sequentialforeach; the client will handle concurrency.es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (2)
39-47: Parallel collection may create needless thread-pool contention
clients.toList.par.foreachspins work on the globalForkJoinPool.
Because the HTTP call itself is already asynchronous (performRequestAsync), adding another layer of parallelism gives zero benefit and may even starve other CPU-bound tasks on the same pool.- clients.toList.par.foreach { client => + clients.toList.foreach { client =>If you really want to fan-out the call you could keep the
parbut then block the pool less by switching toforeach { … }onclients.par(which materialises no new list).
Either way, measure first.
128-132:getAcceptedIssuersshould return an empty array, notnullReturning
nullviolates the contract ofX509TrustManagerand triggers NPEs in some SSL implementations.- override def getAcceptedIssuers: Array[X509Certificate] = null + override def getAcceptedIssuers: Array[X509Certificate] = Array.emptyes710x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (2)
53-57: Encode path segments to avoid 400 / routing issues
indexNameordocumentIdcan legally contain characters that require URL-encoding (space,#,/, …).
Consider building the path viaURLEncoderorio.lemonlabs.urito avoid surprises:- val request = new Request("PUT", s"/$indexName/_doc/$documentId") + val encoded = s"/${Uri.encode(indexName)}/_doc/${Uri.encode(documentId)}" + val request = new Request("PUT", encoded)
128-132: Return an empty array instead ofnullforgetAcceptedIssuersWhile the JDK tolerates
null, returningArray.emptyis cleaner and avoids accidental NPEs in future upgrades:- override def getAcceptedIssuers: Array[X509Certificate] = null + override def getAcceptedIssuers: Array[X509Certificate] = Array.emptyes67x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (2)
40-46: Avoid per-call creation of parallel collections
clients.toList.parconverts the (already allocated) list into a fresh parallel collection on everysubmitinvocation.
With high audit throughput this produces a lot of short-lived objects and thread-pool churn.- clients.toList.par.foreach { client => + clients.par.foreach { client =>
NonEmptyListalready exposes aparview via its ownparmethod (Scala 2.13) or you can keep the original list around.
Net result: less GC pressure, same parallelism.
121-128: Trust-all SSL is handy for tests, dangerous in production
setSSLContextwith an all-trustingX509TrustManagerandNoopHostnameVerifierdisables every certificate check.
Anyone with network access can MITM the audit traffic.Recommendation:
• Gate this behaviour behind a config flag (default off).
• When disabled, honour JVM default trust store or a user-supplied one.If you need help wiring a configurable SSLContext, let me know.
es73x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (2)
40-46: Drop redundant.toListbefore.parSame performance comment as in the 6.7 module – avoid recreating a list and parallel collection on each call.
- clients.toList.par.foreach { client => + clients.par.foreach { client =>
119-126: All-trusting SSL context – secure by default, configurable for devCopy of the earlier remark: disable this in production or make it opt-in to avoid silent MITM vulnerabilities.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
audit/build.gradle(1 hunks)build-base/src/main/groovy/readonlyrest.plugin-common-conventions.gradle(1 hunks)core/build.gradle(1 hunks)es67x/build.gradle(1 hunks)es67x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es67x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es67x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es70x/build.gradle(1 hunks)es70x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(2 hunks)es710x/build.gradle(1 hunks)es710x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es710x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es710x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es711x/build.gradle(1 hunks)es711x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es711x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es711x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es714x/build.gradle(1 hunks)es714x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es714x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es714x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es716x/build.gradle(1 hunks)es716x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es716x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es716x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es717x/build.gradle(1 hunks)es717x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es717x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es717x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es72x/build.gradle(1 hunks)es72x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es72x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es73x/build.gradle(1 hunks)es73x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es73x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es74x/build.gradle(1 hunks)es74x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es74x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es77x/build.gradle(1 hunks)es77x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es77x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es77x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es78x/build.gradle(1 hunks)es78x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es78x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es78x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)es79x/build.gradle(1 hunks)es79x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala(2 hunks)es79x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala(0 hunks)es79x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala(1 hunks)gradle.properties(1 hunks)ror-shadowed-libs/build.gradle(3 hunks)ror-tools-core/build.gradle(1 hunks)ror-tools/build.gradle(1 hunks)suppressions_cve.xml(2 hunks)
💤 Files with no reviewable changes (9)
- es67x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es716x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es711x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es710x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es79x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es714x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es78x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es717x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
- es77x/src/main/scala/tech/beshu/ror/es/services/HighLevelClientAuditSinkService.scala
🧰 Additional context used
🧬 Code Graph Analysis (10)
es79x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es79x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
create(78-81)
es711x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es711x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
RestClientAuditSinkService(35-74)RestClientAuditSinkService(76-133)create(78-81)
es74x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es74x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
create(78-81)
es716x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es716x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
RestClientAuditSinkService(35-74)RestClientAuditSinkService(76-133)create(78-81)
es77x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es77x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
create(78-81)
es714x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es714x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
RestClientAuditSinkService(35-74)RestClientAuditSinkService(76-133)create(78-81)
es73x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es73x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
create(78-81)
es78x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es78x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
create(78-81)
es67x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (1)
es67x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (3)
RestClientAuditSinkService(35-76)RestClientAuditSinkService(78-135)create(80-83)
es67x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (10)
RestClientAuditSinkService(35-74)RestClientAuditSinkService(76-133)submit(39-47)createRequest(53-57)createResponseListener(59-74)close(49-51)create(78-81)createRestClient(83-102)configureCredentials(104-117)configureSsl(119-126)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: ror (Run all tests IT_es90x)
- GitHub Check: ror (Run all tests IT_es67x)
- GitHub Check: ror (Run all tests IT_es80x)
- GitHub Check: ror (Run all tests IT_es70x)
- GitHub Check: ror (Run all tests UNIT)
- GitHub Check: ror (Run all tests IT_es710x)
- GitHub Check: ror (Run all tests IT_es810x)
- GitHub Check: ror (Run all tests LICENSE)
- GitHub Check: ror (Run all tests IT_es818x)
- GitHub Check: ror (Run all tests IT_es717x)
- GitHub Check: ror (CVE check Job)
🔇 Additional comments (48)
gradle.properties (1)
2-2: Confirm plugin version bump.
ThepluginVersionwas updated from1.64.0-pre6to1.64.0-pre7, which aligns with the targeted release. Please verify that related CI/CD pipelines and downstream scripts consume the new version correctly, and confirm that leavingpublishedPluginVersionat1.63.0is intentional.es72x/build.gradle (1)
42-42: Switched to low-level Elasticsearch REST client.
The high-level REST client dependency has been replaced withelasticsearch-rest-client, parameterized bymoduleEsVersion. This matches the PR goal to standardize on the low-level client for audit submissions.es70x/build.gradle (1)
42-42: Switched to low-level Elasticsearch REST client.
Consistent with other modules, the high-level client dependency is replaced byelasticsearch-rest-clientusing the same version variable. This aligns with the refactoring of the audit sink service.es73x/build.gradle (1)
42-42: Switched to low-level Elasticsearch REST client.
The dependency change mirrors other version-specific modules, ensuring end-to-end use of the low-level REST client across ES 7.3.x.ror-tools-core/build.gradle (1)
41-41: Fixed suppression file path typo.
The path was corrected from"supressions_cve.xml"to"suppressions_cve.xml", ensuring dependency-check picks up the intended suppressions config.es714x/build.gradle (1)
42-42: Migrated to low-level REST client dependency
Replaced the deprecated high-level client withorg.elasticsearch.client:elasticsearch-rest-client, matching the newRestClientAuditSinkServiceimplementations.Please confirm there are no remaining references to
elasticsearch-rest-high-level-clientelsewhere in this module.core/build.gradle (1)
61-61: Fixed typo in CVE suppression file path
Correctedsupressions_cve.xmltosuppressions_cve.xmlfor thedependencyCheck.suppressionFilessetting. Ensure the file exists at the project root.ror-tools/build.gradle (1)
46-46: Fixed typo in CVE suppression file path
Updated thesuppressionFilesentry to the correctly spelledsuppressions_cve.xml. Verify consistency with other modules’ configurations.audit/build.gradle (1)
39-39: Fixed typo in CVE suppression file path
Changedsupressions_cve.xmltosuppressions_cve.xmlunderdependencyCheck. Confirm this aligns with the root-level file.es711x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Updated import to RestClientAuditSinkService
Swapped outHighLevelClientAuditSinkServiceforRestClientAuditSinkServiceto complete the migration to the low-level REST client.
98-98: Switched remote audit sink creation to low-level REST client
Now invokingRestClientAuditSinkService.create(remote)for remote clusters. Please verify that authentication and SSL settings are correctly applied in your test environments.es716x/build.gradle (1)
42-42: Library migration to address CVE-2023-3894The dependency has been properly changed from
elasticsearch-rest-high-level-clienttoelasticsearch-rest-client, maintaining the same version variable. This change correctly addresses the CVE vulnerability by avoiding the affected high-level client.es78x/build.gradle (1)
42-42: Library migration to address CVE-2023-3894The dependency has been properly changed from
elasticsearch-rest-high-level-clienttoelasticsearch-rest-client, maintaining the same version variable. This change correctly addresses the CVE vulnerability by avoiding the affected high-level client.build-base/src/main/groovy/readonlyrest.plugin-common-conventions.gradle (1)
62-62: Fixed typo in suppressions file referenceCorrected the file path from "supressions_cve.xml" to "suppressions_cve.xml", ensuring CVE suppressions are properly applied during dependency checks.
es74x/build.gradle (1)
42-42: Library migration to address CVE-2023-3894The dependency has been properly changed from
elasticsearch-rest-high-level-clienttoelasticsearch-rest-client, maintaining the same version variable. This change correctly addresses the CVE vulnerability by avoiding the affected high-level client.es72x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Updated import to use the new RestClientAuditSinkServiceThe import statement has been correctly updated to use the new low-level client-based service, aligning with the dependency changes.
99-99: Updated service implementation usageThe service creation call has been properly updated to use
RestClientAuditSinkService.create()instead of the previous high-level client implementation, completing the CVE remediation.es717x/build.gradle (1)
42-42: Security improvement: Migrated from high-level to low-level REST clientReplacing
elasticsearch-rest-high-level-clientwithelasticsearch-rest-clientaddresses CVE-2023-3894 while maintaining the same version (moduleEsVersion). This change aligns with the PR objective of fixing the vulnerability.es79x/build.gradle (1)
42-42: Security improvement: Migrated from high-level to low-level REST clientReplacing
elasticsearch-rest-high-level-clientwithelasticsearch-rest-clientaddresses CVE-2023-3894 while maintaining the same version (moduleEsVersion). This change aligns with the PR objective of fixing the vulnerability.es710x/build.gradle (1)
42-42: Security improvement: Migrated from high-level to low-level REST clientReplacing
elasticsearch-rest-high-level-clientwithelasticsearch-rest-clientaddresses CVE-2023-3894 while maintaining the same version (moduleEsVersion). This change aligns with the PR objective of fixing the vulnerability.es711x/build.gradle (1)
45-45: Security improvement: Migrated from high-level to low-level REST clientReplacing
elasticsearch-rest-high-level-clientwithelasticsearch-rest-clientaddresses CVE-2023-3894 while maintaining the same version (moduleEsVersion). This change aligns with the PR objective of fixing the vulnerability.es79x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Updated import to use new RestClient-based serviceImport updated to use
RestClientAuditSinkServiceinstead ofHighLevelClientAuditSinkService, consistent with the build.gradle changes to address CVE-2023-3894.
98-98:❓ Verification inconclusive
Implemented service migration from HighLevelClient to RestClient
Updated to use
RestClientAuditSinkService.create(remote)for remote audit clusters, implementing the migration from the vulnerable high-level client to the secured low-level REST client.Verify that the new
RestClientAuditSinkServicemaintains the same security capabilities:
🏁 Script executed:
#!/bin/bash # Check if the new service properly implements SSL configuration ast-grep --pattern "private def configureSsl()"Length of output: 49
🏁 Script executed:
#!/bin/bash # Locate the RestClientAuditSinkService implementation and search for SSL/keystore configuration file=$(fd -t f RestClientAuditSinkService.scala) if [[ -n "$file" ]]; then echo "Found service file at: $file" rg -n 'ssl|SSL|keystore|truststore|certificate' --color=never "$file" else echo "RestClientAuditSinkService.scala not found in the repo" fiLength of output: 15368
#!/bin/bash # Inspect SSL/keystore configuration in the es79x service implementation file="es79x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala" if [[ -f "$file" ]]; then echo "Checking SSL-related settings in $file" rg -n 'ssl|SSL|keystore|truststore|certificate|configureSsl' --color=never "$file" || echo "→ No SSL-related methods or fields found" else echo "ERROR: $file not found" fi
Verify SSL configuration in RestClientAuditSinkService
I ran a search for any SSL/keystore/truststore/certificate references (including
configureSsl) in the new low-level client implementation and found nothing. Please confirm that the REST client is secured with the same SSLContext or keystore/truststore settings as before (e.g., viasetHttpClientConfigCallback).Locations to review:
- es79x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala: no SSL-related methods or fields detected
es710x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use the low-level REST client-based serviceThe import statement is correctly updated to use
RestClientAuditSinkServiceinstead of the deprecated high-level REST client service, addressing CVE-2023-3894.
98-98:✅ Verification successful
Client implementation switched to address CVE-2023-3894
Good migration from
HighLevelClientAuditSinkServicetoRestClientAuditSinkService. The new implementation properly maintains the same functionality while using the low-level REST client API instead of the vulnerable high-level client.
🏁 Script executed:
#!/bin/bash # Verify that no high-level client dependency remains in the build files echo "Checking for elasticsearch-rest-high-level-client dependency in es710x:" grep -r "elasticsearch-rest-high-level-client" es710x/build.gradle echo "Should be using elasticsearch-rest-client instead:" grep -r "elasticsearch-rest-client" es710x/build.gradleLength of output: 523
RestClientAuditSinkService Migration Verified – No High-Level Client Remains
Confirmed that the
elasticsearch-rest-high-level-clientdependency has been fully removed and only the low-levelelasticsearch-rest-clientis declared in es710x/build.gradle. No further changes needed—approving this migration.es74x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use the low-level REST client-based serviceThe import statement is correctly updated to use
RestClientAuditSinkServiceinstead of the deprecated high-level REST client service, addressing CVE-2023-3894.
99-99:✅ Verification successful
Client implementation switched to address CVE-2023-3894
Appropriate migration from
HighLevelClientAuditSinkServicetoRestClientAuditSinkService. The new implementation maintains the same audit submission functionality while using the more secure low-level REST client API.
🏁 Script executed:
#!/bin/bash # Verify that no high-level client dependency remains in the build files echo "Checking for elasticsearch-rest-high-level-client dependency in es74x:" grep -r "elasticsearch-rest-high-level-client" es74x/build.gradle echo "Should be using elasticsearch-rest-client instead:" grep -r "elasticsearch-rest-client" es74x/build.gradleLength of output: 519
Confirmed: CVE-2023-3894 migration complete and high-level client removed
- Verified that
elasticsearch-rest-high-level-clientis no longer referenced in es74x/build.gradle- Confirmed use of
org.elasticsearch.client:elasticsearch-rest-clientat versionmoduleEsVersionRestClientAuditSinkService replaces HighLevelClientAuditSinkService as intended—approving these changes.
es716x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use the low-level REST client-based serviceThe import statement is correctly updated to use
RestClientAuditSinkServiceinstead of the deprecated high-level REST client service, addressing CVE-2023-3894.
98-98:✅ Verification successful
Client implementation switched to address CVE-2023-3894
Good migration from
HighLevelClientAuditSinkServicetoRestClientAuditSinkService. The new implementation correctly maintains the same audit sink functionality while using the low-level REST client API.
🏁 Script executed:
#!/bin/bash # Verify that no high-level client dependency remains in the build files echo "Checking for elasticsearch-rest-high-level-client dependency in es716x:" grep -r "elasticsearch-rest-high-level-client" es716x/build.gradle echo "Should be using elasticsearch-rest-client instead:" grep -r "elasticsearch-rest-client" es716x/build.gradleLength of output: 523
Approved: Low-Level REST Client Migration Verified
- Confirmed removal of
elasticsearch-rest-high-level-clientfrom es716x/build.gradle- Verified presence of
org.elasticsearch.client:elasticsearch-rest-clientdependency- Ensures
RestClientAuditSinkServicenow replacesHighLevelClientAuditSinkService, addressing CVE-2023-3894es67x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use the low-level REST client-based serviceThe import statement is correctly updated to use
RestClientAuditSinkServiceinstead of the deprecated high-level REST client service, addressing CVE-2023-3894.
99-99:✅ Verification successful
Client implementation switched to address CVE-2023-3894
Appropriate migration from
HighLevelClientAuditSinkServicetoRestClientAuditSinkService. The new implementation correctly maintains the same functionality while using the more secure low-level REST client API.
🏁 Script executed:
#!/bin/bash # Verify that no high-level client dependency remains in the build files echo "Checking for elasticsearch-rest-high-level-client dependency in es67x:" grep -r "elasticsearch-rest-high-level-client" es67x/build.gradle echo "Should be using elasticsearch-rest-client instead:" grep -r "elasticsearch-rest-client" es67x/build.gradleLength of output: 519
Migration to low-level REST client confirmed and approved
- Verified removal of
elasticsearch-rest-high-level-clientfrom es67x/build.gradle- Confirmed presence of
implementation group: 'org.elasticsearch.client', name: 'elasticsearch-rest-client', version: moduleEsVersionSwitching to
RestClientAuditSinkService.create(remote)fully addresses CVE-2023-3894 while preserving audit functionality. No further changes required.es70x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use RestClientAuditSinkServiceCorrectly imports the new RestClientAuditSinkService which uses the lower-level REST client instead of the vulnerable high-level client.
99-99: Service creation updated to use lower-level REST clientReplacing HighLevelClientAuditSinkService with RestClientAuditSinkService properly addresses CVE-2023-3894 by migrating from the vulnerable high-level REST client to the more secure low-level implementation while maintaining the same functionality.
es73x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use RestClientAuditSinkServiceCorrectly imports the new RestClientAuditSinkService class instead of the vulnerable HighLevelClientAuditSinkService.
99-99: Service creation updated to use lower-level REST clientThe update correctly switches to RestClientAuditSinkService which properly implements the same audit functionality using the more secure low-level client API to address CVE-2023-3894.
es78x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use RestClientAuditSinkServiceImport correctly updated to use the new RestClientAuditSinkService implementation.
98-98: Service creation updated to use lower-level REST clientCorrectly migrates from HighLevelClientAuditSinkService to RestClientAuditSinkService to address CVE-2023-3894 vulnerability. The implementation maintains the same audit submission logic while using the more secure lower-level REST client.
es714x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Import updated to use RestClientAuditSinkServiceImport correctly switched to use the new RestClientAuditSinkService which addresses the security vulnerability.
98-98: Service creation updated to use lower-level REST clientProperly migrates to RestClientAuditSinkService implementation which uses the more secure low-level REST client instead of the vulnerable high-level client, addressing CVE-2023-3894 while maintaining the same functionality pattern.
es77x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Appropriate migration to RestClientAuditSinkServiceUpdated import to use the new lower-level REST client implementation, addressing CVE-2023-3894.
98-98: Correctly updated service implementationService creation now uses the low-level REST client implementation which addresses security vulnerabilities in the high-level client.
es717x/src/main/scala/tech/beshu/ror/es/IndexLevelActionFilter.scala (2)
40-40: Appropriate migration to RestClientAuditSinkServiceUpdated import to use the new lower-level REST client implementation, addressing CVE-2023-3894.
98-98: Correctly updated service implementationService creation now uses the low-level REST client implementation which addresses security vulnerabilities in the high-level client.
es67x/build.gradle (1)
42-42: Correctly updated dependency to lower-level clientChanged from high-level REST client to the low-level REST client, addressing CVE-2023-3894. This aligns with the service implementation changes in the Scala code.
es77x/build.gradle (1)
42-42: Correctly updated dependency to lower-level clientChanged from high-level REST client to the low-level REST client, addressing CVE-2023-3894. This aligns with the service implementation changes in the Scala code.
ror-shadowed-libs/build.gradle (2)
18-23:❓ Verification inconclusive
Shadow plugin swap may break
ShadowJartask typeThe build now applies
com.gradleup.shadow, yet later still imports/targets
com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar.
If the GradleUp fork does not expose the exact same task class (package & name), the build will fail at configuration time.-import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import com.gradleup.shadow.tasks.ShadowJar // verify actual FQN in plugin docsPlease double-check the plugin’s API or keep the original plugin to avoid a class-not-found error.
Also applies to: 72-76
Verify ShadowJar import for com.gradleup.shadow plugin
The build applies the GradleUp Shadow fork but still imports the original task class, which may no longer exist:
• File:
ror-shadowed-libs/build.gradle
– Lines 18–23
– Lines 72–76Suggested diff (adjust FQN to match plugin API):
-import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar +import com.gradleup.shadow.tasks.ShadowJar // confirm exact package/name in plugin docsPlease review the com.gradleup.shadow plugin documentation to ensure the
ShadowJartask class’s package and name haven’t changed, or revert to the original plugin to prevent a ClassNotFoundException at configuration time.
55-58:❓ Verification inconclusive
Verify Jackson/SnakeYAML version availability
Versions
2.19.0(Jackson) and2.4(SnakeYAML) are ahead of the latest releases currently published to Maven Central. A typo or unpublished snapshot will break the dependency resolution.Run:
Adjust the version numbers or use a version catalog/
platformto prevent silent downgrades.
🏁 Script executed:
#!/bin/bash # List versions known to Maven Central for lib in jackson-databind jackson-dataformat-yaml snakeyaml; do mvn -q versions:display-dependency-updates -Dincludes="com.fasterxml.*:$lib" # or use `curl` against search.maven.org doneLength of output: 746
🏁 Script executed:
#!/bin/bash set -e # Query Maven Central for latest versions of each library declare -A libs=( ["jackson-databind"]="com.fasterxml.jackson.core" ["jackson-dataformat-yaml"]="com.fasterxml.jackson.dataformat" ["snakeyaml"]="org.yaml" ) for artifact in "${!libs[@]}"; do group=${libs[$artifact]} latest=$(curl -s "https://search.maven.org/solrsearch/select?q=g:\"$group\"+AND+a:\"$artifact\"&rows=1&wt=json" \ | jq -r '.response.docs[0].latestVersion') echo "$artifact: $latest" doneLength of output: 437
Verify Jackson & SnakeYAML version availability
The versions declared in ror-shadowed-libs/build.gradle (lines 55–58) may not exist on Maven Central (jackson-dataformat-yaml: 2.19.0, jackson-databind: 2.19.0, snakeyaml: 2.4). A typo or unpublished snapshot will break dependency resolution.
Please confirm these versions are published—e.g.:
mvn versions:display-dependency-updates \ -Dincludes=com.fasterxml.jackson.core:jackson-databind,com.fasterxml.jackson.dataformat:jackson-dataformat-yaml,org.yaml:snakeyaml # or curl "https://search.maven.org/solrsearch/select?q=g:%22GROUP%22+AND+a:%22ARTIFACT%22&rows=1&wt=json"Then adjust to the latest published releases or manage them via a BOM/version catalog to prevent silent downgrades.
es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala (1)
53-57: Hard-coded_doctype may break on ES 8While 7.x still tolerates the pseudo-type
_doc, Elasticsearch 8.x removed the type entirely.
Consider computing the path conditionally or feature-probing the cluster version to keep forward compatibility.
es78x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es717x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es717x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es70x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es710x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es67x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
es73x/src/main/scala/tech/beshu/ror/es/services/RestClientAuditSinkService.scala
Show resolved
Hide resolved
🚨Security Fix (ES) CVE-2023-3894