Disable custom serialization after 2.13

Signed-off-by: Peter Nied <petern@amazon.com>
peternied · Apr 17, 2024 · 51e9d69 · 51e9d69
1 parent 0bfe765
commit 51e9d69
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 8 deletions.
diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java
@@ -333,9 +333,6 @@ public enum RolesMappingResolution {
     public static final String TENANCY_GLOBAL_TENANT_NAME = "global";
     public static final String TENANCY_GLOBAL_TENANT_DEFAULT_NAME = "";
 
-    public static final String USE_JDK_SERIALIZATION = "plugins.security.use_jdk_serialization";
-    public static final Version FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION = Version.V_2_11_0;
-
     // On-behalf-of endpoints settings
     // CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings
     public static final String EXTENSIONS_BWC_PLUGIN_MODE = "bwcPluginMode";

diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java
@@ -38,7 +38,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-
+import org.opensearch.Version;
 import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsAction;
 import org.opensearch.action.admin.cluster.shards.ClusterSearchShardsResponse;
 import org.opensearch.action.get.GetRequest;
@@ -150,7 +150,8 @@ public <T extends TransportResponse> void sendRequestDecorate(
         final String origCCSTransientMf = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_CCS);
 
         final boolean isDebugEnabled = log.isDebugEnabled();
-        final boolean useJDKSerialization = connection.getVersion().before(ConfigConstants.FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION);
+
+        final var serializationFormat = shouldUseJdkSerialization(connection);
         final boolean isSameNodeRequest = localNode != null && localNode.equals(connection.getNode());
 
         try (ThreadContext.StoredContext stashedContext = getThreadContext().stashContext()) {
@@ -228,7 +229,7 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROL
                 );
             }
 
-            if (useJDKSerialization) {
+            if (serializationFormat == SerializationFormat.JDK) {
                 Map<String, String> jdkSerializedHeaders = new HashMap<>();
                 HeaderHelper.getAllSerializedHeaderNames()
                     .stream()
@@ -246,7 +247,7 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROL
                 injectedUserString,
                 injectedRolesString,
                 isSameNodeRequest,
-                useJDKSerialization
+                serializationFormat
             );
 
             if (actionTraceEnabled.get()) {
@@ -268,14 +269,31 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROL
         }
     }
 
+    private static final String USE_JDK_SERIALIZATION = "plugins.security.use_jdk_serialization";
+    private static final Version FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION = Version.V_2_11_0;
+    private static final Version CUSTOM_SERIALIZATION_NO_LONGER_SUPPORTED_OS_VERSION = Version.V_2_14_0;
+    private SerializationFormat shouldUseJdkSerialization(final Connection connection) {
+        var version = connection.getVersion();
+        if (version.after(FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION)
+        && version.before(CUSTOM_SERIALIZATION_NO_LONGER_SUPPORTED_OS_VERSION)) {
+            return SerializationFormat.CustomSerializer_2_11;
+        }
+        return SerializationFormat.JDK;
+    }
+
+    private enum SerializationFormat {
+        JDK,
+        CustomSerializer_2_11
+    }
+
     private void ensureCorrectHeaders(
         final Object remoteAdr,
         final User origUser,
         final String origin,
         final String injectedUserString,
         final String injectedRolesString,
         final boolean isSameNodeRequest,
-        final boolean useJDKSerialization
+        final SerializationFormat format
     ) {
         // keep original address
 
@@ -313,6 +331,7 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADE
                 getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUserString);
             }
         } else {
+            final var useJDKSerialization = format == SerializationFormat.JDK;
             if (transportAddress != null) {
                 getThreadContext().putHeader(
                     ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS_HEADER,

diff --git a/src/test/java/org/opensearch/security/support/Base64HelperTest.java b/src/test/java/org/opensearch/security/support/Base64HelperTest.java
@@ -11,12 +11,16 @@
 package org.opensearch.security.support;
 
 import java.io.Serializable;
+import java.util.HashMap;
+import java.util.stream.IntStream;
 
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.junit.Assert.assertThat;
 import static org.opensearch.security.support.Base64Helper.deserializeObject;
 import static org.opensearch.security.support.Base64Helper.serializeObject;
+import static org.hamcrest.Matchers.equalTo;
 
 public class Base64HelperTest {
 
@@ -48,4 +52,23 @@ public void testEnsureJDKSerialized() {
         Assert.assertEquals(jdkSerialized, Base64Helper.ensureJDKSerialized(jdkSerialized));
         Assert.assertEquals(jdkSerialized, Base64Helper.ensureJDKSerialized(customSerialized));
     }
+
+    @Test
+    public void testDifference() {
+        var largeObject = new HashMap<String, Object>();
+        var hm = new HashMap<>();
+        IntStream.range(0, 100).forEach(i -> {
+            hm.put("c" + i, "cvalue" + i);
+        });
+        IntStream.range(0, 100).forEach(i -> {
+            largeObject.put("a" + i, "value");
+            largeObject.put("a", "value" + 1);
+            largeObject.put("b" + i, hm);
+        });
+
+        String jdkSerialized = Base64Helper.serializeObject(largeObject, true);
+        String customSerialized = Base64Helper.serializeObject(largeObject, false);
+
+        assertThat(jdkSerialized.length(), equalTo(customSerialized.length()));
+    }
 }