You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "capistrant (via GitHub)" <gi...@apache.org> on 2025/06/30 03:11:08 UTC

[PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

capistrant opened a new pull request, #18178:
URL: https://github.com/apache/druid/pull/18178

   <!-- Thanks for trying to help us make Apache Druid be the best it can be! Please fill out as much of the following information as is possible (where relevant, and remove it when irrelevant) to help make the intention and scope of this PR clear in order to ease review. -->
   
   <!-- Please read the doc for contribution (https://github.com/apache/druid/blob/master/CONTRIBUTING.md) before making this PR. Also, once you open a PR, please _avoid using force pushes and rebasing_ since these make it difficult for reviewers to see what you've changed in response to their reviews. See [the 'If your pull request shows conflicts with master' section](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master) for more details. -->
   
   <!-- Replace XXXX with the id of the issue fixed in this PR. Remove this section if there is no corresponding issue. Don't reference the issue in the title of this pull-request. -->
   
   <!-- If you are a committer, follow the PR action item checklist for committers:
   https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers. -->
   
   ### Description
   
   Builds on #18139 which is in regards to addressing a [kafka cve](https://www.cve.org/CVERecord?id=CVE-2025-27817)
   
   Ideally we'd jump to kafka 4.x which has a strict deny all default instead of kafka 3.9.1 which defaults to allowAll. Jumping to 4.x would allow us to avoid the static block in our code slipping in the denyAll default. However, we have a lot of test code that relies on being able to run KafkaServer embedded and this is now gone in 4.x. Initially I was looking into making the jump to 4.x anyways. I was going to migrate as much of the tests that rely on KafkaServer to MockConsumer as I could and then disable the rest of the tests in place of relying more on the kafka ITs. However, I then realized that the new Druid Simulations introduced by @kfaraz also rely on kafka 3.x KafkaServer. So jumping to 4 right now would knee-cap that new and really nice test functionality. At this point, I decided to propose the change in this PR. It is an idea suggested by @kgyrtkirk where we can use 3.9.1 and some static blocks on on our side to protect Druid from this CVE. Operators could still follow
  the Kafka patch nodes to set the system property with a legitimate allow list for the two consumer configs called out under the CVE. This approach buys us time to move to kafka 4 and find long term replacements to using KafkaServer in our tests to run Kafka embedded.
   
   <!-- Describe the goal of this PR, what problem are you fixing. If there is a corresponding issue (referenced above), it's not necessary to repeat the description here, however, you may choose to keep one summary sentence. -->
   
   <!-- Describe your patch: what did you change in code? How did you fix the problem? -->
   
   <!-- If there are several relatively logically separate changes in this PR, create a mini-section for each of them. For example: -->
   
   
   <!--
   In each section, please describe design decisions made, including:
    - Choice of algorithms
    - Behavioral aspects. What configuration values are acceptable? How are corner cases and error conditions handled, such as when there are insufficient resources?
    - Class organization and design (how the logic is split between classes, inheritance, composition, design patterns)
    - Method organization and design (how the logic is split between methods, parameters and return types)
    - Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
   -->
   
   
   <!-- It's good to describe an alternative design (or mention an alternative name) for every design (or naming) decision point and compare the alternatives with the designs that you've implemented (or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
   
   <!-- If there was a discussion of the design of the feature implemented in this PR elsewhere (e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list), link to that discussion from this PR description and explain what have changed in your final design compared to your original proposal or the consensus version in the end of the discussion. If something hasn't changed since the original discussion, you can omit a detailed discussion of those aspects of the design here, perhaps apart from brief mentioning for the sake of readability of this PR description. -->
   
   <!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
   
   #### Release note
   <!-- Give your best effort to summarize your changes in a couple of sentences aimed toward Druid users. 
   
   If your change doesn't have end user impact, you can skip this section.
   
   For tips about how to write a good release note, see [Release notes](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#release-notes).
   
   -->
   
   
   <hr>
   
   ##### Key changed/added classes in this PR
    * `KafkaLookupExtractorFactory`
    * `KafkaREcordSupplier`
   
   <hr>
   
   <!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
   
   This PR has:
   
   - [ ] been self-reviewed.
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] a release note entry in the PR description.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "capistrant (via GitHub)" <gi...@apache.org>.
capistrant commented on PR #18178:
URL: https://github.com/apache/druid/pull/18178#issuecomment-3019430855

   Tested in a Druid cluster and confirmed that it appears to work for creating kafka supervisors, but that the UX will suck as is. Overlord logs contain a good message, but it is not making it's way into the error reporting exposed in the console for the supervisor. Will investigate if there is easy way to propagate the root cause 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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "capistrant (via GitHub)" <gi...@apache.org>.
capistrant merged PR #18178:
URL: https://github.com/apache/druid/pull/18178


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "capistrant (via GitHub)" <gi...@apache.org>.
capistrant commented on code in PR #18178:
URL: https://github.com/apache/druid/pull/18178#discussion_r2175518223


##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java:
##########
@@ -63,6 +63,17 @@
 
 public class KafkaRecordSupplier implements RecordSupplier<KafkaTopicPartition, Long, KafkaRecordEntity>
 {
+  // by default, we reject all URLs for OAuthBearer authentication
+  // CVE ref: https://www.cve.org/CVERecord?id=CVE-2025-27817
+  // Upgrade kafka dependencies to 4.x to remove the need for this static block
+  static {
+    final String allowedSaslOauthbearerUrlsConfig = "org.apache.kafka.sasl.oauthbearer.allowed.urls";
+    String allowedUrlsProp = System.getProperty(allowedSaslOauthbearerUrlsConfig);
+    if (allowedUrlsProp == null) {
+      System.setProperty(allowedSaslOauthbearerUrlsConfig, "notallowed");
+    }
+  }

Review Comment:
   Do you mean as a static method in AuthorizationUtils and then the static blocks in the two classes who create the KafkaConsumer would call that method?



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "capistrant (via GitHub)" <gi...@apache.org>.
capistrant commented on PR #18178:
URL: https://github.com/apache/druid/pull/18178#issuecomment-3017697209

   I'll add a more full description on 6/30, but I wanted to get it up rather than wait.
   
   I need to try this out in a local cluster and see what the UX is like too. Ideally we could get a better DruidException with meaningful messaging in front of the person submitting a Kafka supervisor 


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on code in PR #18178:
URL: https://github.com/apache/druid/pull/18178#discussion_r2175562717


##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java:
##########
@@ -63,6 +63,17 @@
 
 public class KafkaRecordSupplier implements RecordSupplier<KafkaTopicPartition, Long, KafkaRecordEntity>
 {
+  // by default, we reject all URLs for OAuthBearer authentication
+  // CVE ref: https://www.cve.org/CVERecord?id=CVE-2025-27817
+  // Upgrade kafka dependencies to 4.x to remove the need for this static block
+  static {
+    final String allowedSaslOauthbearerUrlsConfig = "org.apache.kafka.sasl.oauthbearer.allowed.urls";
+    String allowedUrlsProp = System.getProperty(allowedSaslOauthbearerUrlsConfig);
+    if (allowedUrlsProp == null) {
+      System.setProperty(allowedSaslOauthbearerUrlsConfig, "notallowed");
+    }
+  }

Review Comment:
   Yes. Not a blocker for getting this merged though.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "capistrant (via GitHub)" <gi...@apache.org>.
capistrant commented on PR #18178:
URL: https://github.com/apache/druid/pull/18178#issuecomment-3019563746

   > Tested in a Druid cluster and confirmed that it appears to work for creating kafka supervisors, but that the UX will suck as is. Overlord logs contain a good message, but it is not making it's way into the error reporting exposed in the console for the supervisor. Will investigate if there is easy way to propagate the root cause up
   
   Idk if there is much we can do: [kafka code](https://github.com/apache/kafka/blob/3.9.1/clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerDelegateCreator.java#L56)
   
   This wraps the original Exception that has the good log about why creating the consumer failed due to a disallowed url. The supervisor UI exposes the generic KafkaException under recent errors, but to see what caused it, you need to look in the Overlord log. A pretty poor experience, especially for clusters with operators who make the supervisor API self serve to users who may not know or be able to reference the underlying overlord logs.


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "capistrant (via GitHub)" <gi...@apache.org>.
capistrant commented on PR #18178:
URL: https://github.com/apache/druid/pull/18178#issuecomment-3021162065

   Thank you for the review @kfaraz!


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #18178:
URL: https://github.com/apache/druid/pull/18178#discussion_r2175201438


##########
extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/KafkaRecordSupplierTest.java:
##########
@@ -252,6 +257,38 @@
     recordSupplier.close();
   }
 
+  @Test
+  public void test_defaultRejectAllUrlsForSaslOauthBearerUrlConsumerProperty() throws ExecutionException, InterruptedException
+  {
+    // Insert data
+    insertData();
+
+    Set<StreamPartition<KafkaTopicPartition>> partitions = ImmutableSet.of(
+        StreamPartition.of(TOPIC, PARTITION_0),
+        StreamPartition.of(TOPIC, PARTITION_1)
+    );

Review Comment:
   ## Unread local variable
   
   Variable 'Set<StreamPartition<KafkaTopicPartition>> partitions' is never read.
   
   [Show more details](https://github.com/apache/druid/security/code-scanning/9295)



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Bump kafka client dep to 3.9.1 and mitigate CVE-2025-27817 (druid)

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on code in PR #18178:
URL: https://github.com/apache/druid/pull/18178#discussion_r2175435186


##########
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java:
##########
@@ -63,6 +63,17 @@
 
 public class KafkaRecordSupplier implements RecordSupplier<KafkaTopicPartition, Long, KafkaRecordEntity>
 {
+  // by default, we reject all URLs for OAuthBearer authentication
+  // CVE ref: https://www.cve.org/CVERecord?id=CVE-2025-27817
+  // Upgrade kafka dependencies to 4.x to remove the need for this static block
+  static {
+    final String allowedSaslOauthbearerUrlsConfig = "org.apache.kafka.sasl.oauthbearer.allowed.urls";
+    String allowedUrlsProp = System.getProperty(allowedSaslOauthbearerUrlsConfig);
+    if (allowedUrlsProp == null) {
+      System.setProperty(allowedSaslOauthbearerUrlsConfig, "notallowed");
+    }
+  }

Review Comment:
   I wonder if we should just put this in `AuthorizationUtils` even though it doesn't pertain to core Druid and only extensions, just so we can have it in a common place with adequate javadocs.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org