diff --git a/src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java b/src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java index 54dbd31f3f..93cafb8556 100644 --- a/src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java +++ b/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()); } } } diff --git a/src/test/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluatorTest.java b/src/test/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluatorTest.java new file mode 100644 index 0000000000..226232a957 --- /dev/null +++ b/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> 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> 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> 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); + } +}