Fix ClassCastException for otherName SAN entries during inter-cluster handshake

src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java

@@ -31,7 +31,6 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -124,34 +123,33 @@ public boolean isInterClusterRequest(
 
                 for (final List<?> ian : ianList) {
 
-                    if (ian == null) {
+                    if (ian == null || ian.size() < 2) {
                         continue;
                     }
 
-                    for (@SuppressWarnings("rawtypes")
-                    final Iterator iterator = ian.iterator(); iterator.hasNext();) {
-                        final int id = (int) iterator.next();
-                        if (id == 8) { // id 8 = OID, id 1 = name (as string or
-                                       // ASN.1 encoded byte[])
-                            Object value = iterator.next();
-
-                            if (value == null) {
-                                continue;
-                            }
-
-                            if (value instanceof String) {
-                                sb.append(id + "::" + value);
-                            } else if (value instanceof byte[]) {
-                                log.error(
-                                    "Unable to handle OID san {} with value {} of type byte[] (ASN.1 DER not supported here)",
-                                    id,
-                                    Arrays.toString((byte[]) value)
-                                );
-                            } else {
-                                log.error("Unable to handle OID san {} with value {} of type {}", id, value, value.getClass());
-                            }
+                    final Object typeId = ian.get(0);
+                    if (!(typeId instanceof Integer)) {
+                        continue;
+                    }
+                    final int id = (Integer) typeId;
+                    if (id == 8) { // id 8 = OID, id 1 = name (as string or
+                                   // ASN.1 encoded byte[])
+                        Object value = ian.get(1);
+
+                        if (value == null) {
+                            continue;
+                        }
+
+                        if (value instanceof String) {
+                            sb.append(id + "::" + value);
+                        } else if (value instanceof byte[]) {
+                            log.error(
+                                "Unable to handle OID san {} with value {} of type byte[] (ASN.1 DER not supported here)",
+                                id,
+                                Arrays.toString((byte[]) value)
+                            );
                         } else {
-                            iterator.next();
+                            log.error("Unable to handle OID san {} with value {} of type {}", id, value, value.getClass());
                         }
                     }
                 }

src/test/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluatorTest.java

@@ -0,0 +1,110 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.security.transport;
+
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Test;
+
+import org.opensearch.common.settings.Settings;
+import org.opensearch.security.support.ConfigConstants;
+import org.opensearch.transport.TransportRequest;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class DefaultInterClusterRequestEvaluatorTest {
+
+    private static final String NODE_OID = "1.2.3.4.5.5";
+
+    private DefaultInterClusterRequestEvaluator newEvaluator() {
+        Settings settings = Settings.builder().put(ConfigConstants.SECURITY_CERT_OID, NODE_OID).build();
+        return new DefaultInterClusterRequestEvaluator(settings);
+    }
+
+    @Test
+    public void testIsInterClusterRequest_upnOtherNameInSan_doesNotThrow() throws Exception {
+        X509Certificate cert = mock(X509Certificate.class);
+        List<List<?>> sanList = new ArrayList<>();
+        sanList.add(Arrays.asList(0, "user@example.com", "1.3.6.1.4.1.311.20.2.3"));
+        when(cert.getSubjectAlternativeNames()).thenReturn(sanList);
+
+        DefaultInterClusterRequestEvaluator evaluator = newEvaluator();
+        boolean result = evaluator.isInterClusterRequest(
+            mock(TransportRequest.class),
+            new X509Certificate[] { cert },
+            new X509Certificate[] { cert },
+            "CN=foo"
+        );
+
+        assertFalse(result);
+    }
+
+    @Test
+    public void testIsInterClusterRequest_oidSanMatchesNodeOid_returnsTrue() throws Exception {
+        X509Certificate cert = mock(X509Certificate.class);
+        List<List<?>> sanList = new ArrayList<>();
+        sanList.add(Arrays.asList(8, NODE_OID));
+        when(cert.getSubjectAlternativeNames()).thenReturn(sanList);
+
+        DefaultInterClusterRequestEvaluator evaluator = newEvaluator();
+        boolean result = evaluator.isInterClusterRequest(
+            mock(TransportRequest.class),
+            new X509Certificate[] { cert },
+            new X509Certificate[] { cert },
+            "CN=foo"
+        );
+
+        assertTrue(result);
+    }
+
+    @Test
+    public void testIsInterClusterRequest_oidSanAlongsideUpn_returnsTrue() throws Exception {
+        X509Certificate cert = mock(X509Certificate.class);
+        List<List<?>> sanList = new ArrayList<>();
+        sanList.add(Arrays.asList(0, "user@example.com", "1.3.6.1.4.1.311.20.2.3"));
+        sanList.add(Arrays.asList(2, "node1.example.com"));
+        sanList.add(Arrays.asList(8, NODE_OID));
+        when(cert.getSubjectAlternativeNames()).thenReturn(sanList);
+
+        DefaultInterClusterRequestEvaluator evaluator = newEvaluator();
+        boolean result = evaluator.isInterClusterRequest(
+            mock(TransportRequest.class),
+            new X509Certificate[] { cert },
+            new X509Certificate[] { cert },
+            "CN=foo"
+        );
+
+        assertTrue(result);
+    }
+
+    @Test
+    public void testIsInterClusterRequest_nullSan_returnsFalse() throws Exception {
+        X509Certificate cert = mock(X509Certificate.class);
+        when(cert.getSubjectAlternativeNames()).thenReturn(null);
+
+        DefaultInterClusterRequestEvaluator evaluator = newEvaluator();
+        boolean result = evaluator.isInterClusterRequest(
+            mock(TransportRequest.class),
+            new X509Certificate[] { cert },
+            new X509Certificate[] { cert },
+            "CN=foo"
+        );
+
+        assertFalse(result);
+    }
+}