Feat: more defensive handling of malformed LeaveGroup requests (kroxylicious#2788)

* Special case handling of badly formed LeaveGroup requests

Signed-off-by: Sam Barker <[email protected]>

---------

Signed-off-by: Sam Barker <[email protected]>

Author: SamBarker

kroxylicious-runtime/src/main/java/io/kroxylicious/proxy/internal/KafkaProxyExceptionMapper.java

@@ -6,6 +6,8 @@
 
 package io.kroxylicious.proxy.internal;
 
+import java.util.List;
+
 import org.apache.kafka.common.ElectionType;
 import org.apache.kafka.common.IsolationLevel;
 import org.apache.kafka.common.TopicPartition;
@@ -290,9 +292,8 @@ private static AbstractRequest errorResponse(ApiKeys apiKey, ApiMessage reqBody,
                         .build(apiVersion);
                 break;
             case LEAVE_GROUP:
-                LeaveGroupRequestData data = (LeaveGroupRequestData) reqBody;
-                req = new LeaveGroupRequest.Builder(data.groupId(), data.members())
-                        .build(apiVersion);
+                LeaveGroupRequest.Builder builder = toLeaveGroupBuilder((LeaveGroupRequestData) reqBody);
+                req = builder.build(apiVersion);
                 break;
             case SYNC_GROUP:
                 req = new SyncGroupRequest((SyncGroupRequestData) reqBody, apiVersion);
@@ -574,4 +575,16 @@ private static AbstractRequest errorResponse(ApiKeys apiKey, ApiMessage reqBody,
         }
         return req;
     }
+
+    private static LeaveGroupRequest.Builder toLeaveGroupBuilder(LeaveGroupRequestData reqBody) {
+        LeaveGroupRequest.Builder builder;
+        if (!reqBody.members().isEmpty()) {
+            builder = new LeaveGroupRequest.Builder(reqBody.groupId(), reqBody.members());
+        }
+        else {
+            // This should never happen with a legitimate client but as a edge service we should handle malicious ones too
+            builder = new LeaveGroupRequest.Builder(reqBody.groupId(), List.of(new LeaveGroupRequestData.MemberIdentity()));
+        }
+        return builder;
+    }
 }

kroxylicious-runtime/src/test/java/io/kroxylicious/proxy/internal/filter/RequestFilterResultBuilderTest.java

@@ -12,6 +12,7 @@
 import org.apache.kafka.common.errors.UnknownServerException;
 import org.apache.kafka.common.message.FetchRequestData;
 import org.apache.kafka.common.message.FetchResponseData;
+import org.apache.kafka.common.message.LeaveGroupRequestData;
 import org.apache.kafka.common.message.RequestHeaderData;
 import org.apache.kafka.common.message.ResponseHeaderData;
 import org.apache.kafka.common.protocol.ApiKeys;
@@ -164,6 +165,30 @@ void shouldBuildErrorResponse(RequestFactory.ApiMessageVersion versionedMessage)
                 });
     }
 
+    @Test
+    void shouldBuildErrorResponseForIllegitimateLeaveGroupRequest() {
+        // Given
+        final ApiKeys apiKey = ApiKeys.LEAVE_GROUP;
+        final Class<? extends ApiMessage> responseMessageClass = apiKey.messageType.newResponse().getClass();
+        var header = new RequestHeaderData();
+        header.setRequestApiKey(apiKey.id);
+        header.setRequestApiVersion(apiKey.latestVersion());
+        header.setCorrelationId(23456);
+
+        // When
+        var future = builder.errorResponse(header, new LeaveGroupRequestData(), FILTER_RUNTIME_EXCEPTION).completed();
+
+        // Then
+        assertThat(future)
+                .succeedsWithin(Duration.ofSeconds(10))
+                .satisfies(result -> {
+                    assertThat(result.header())
+                            .satisfies(headerMessage -> assertThat(headerMessage).asInstanceOf(InstanceOfAssertFactories.type(ResponseHeaderData.class))
+                                    .satisfies(responseHeaderData -> assertThat(responseHeaderData.correlationId()).isEqualTo(header.correlationId())));
+                    assertThat(result.message()).satisfies(actualResponse -> assertThat(actualResponse).isExactlyInstanceOf(responseMessageClass));
+                });
+    }
+
     @Test
     void shouldErrorResponseShouldNotInvokeRemainingFilterChain() {
         // Given