[Security Manager Replacement] Native Java Agent (dynamic code rewriting, must be low overhead)

reta · reta · commit 1ecaa7a04862 · 2025-04-07T22:04:34.000-04:00
Signed-off-by: Andriy Redko &lt;andriy.redko@aiven.io&gt;
Signed-off-by: Andriy Redko &lt;drreta@gmail.com&gt;
diff --git a/build.gradle b/build.gradle
@@ -433,12 +433,18 @@ gradle.projectsEvaluated {
 
     project.tasks.withType(Test) { task ->
       if (task != null) {
-        if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17) {
+        if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17 && BuildParams.runtimeJavaVersion <= JavaVersion.VERSION_23) {
           task.jvmArgs += ["-Djava.security.manager=allow"]
         }
         if (BuildParams.runtimeJavaVersion >= JavaVersion.VERSION_20) {
           task.jvmArgs += ["--add-modules=jdk.incubator.vector"]
         }
+
+        // Add Java Agent for security sandboxing
+        if (!(project.path in [':build-tools', ":libs:agent-sm:bootstrap", ":libs:agent-sm:agent"])) {
+          dependsOn(project(':libs:agent-sm:agent').prepareAgent)
+          jvmArgs += ["-javaagent:" + project(':libs:agent-sm:agent').jar.archiveFile.get()]
+        }
       }
     }
 
diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle
@@ -110,12 +110,12 @@ dependencies {
   api 'com.netflix.nebula:gradle-info-plugin:12.1.6'
   api 'org.apache.rat:apache-rat:0.15'
   api "commons-io:commons-io:${props.getProperty('commonsio')}"
-  api "net.java.dev.jna:jna:5.14.0"
+  api "net.java.dev.jna:jna:5.16.0"
   api 'com.gradleup.shadow:shadow-gradle-plugin:8.3.5'
   api 'org.jdom:jdom2:2.0.6.1'
   api "org.jetbrains.kotlin:kotlin-stdlib-jdk8:${props.getProperty('kotlin')}"
   api 'de.thetaphi:forbiddenapis:3.8'
-  api 'com.avast.gradle:gradle-docker-compose-plugin:0.17.6'
+  api 'com.avast.gradle:gradle-docker-compose-plugin:0.17.12'
   api "org.yaml:snakeyaml:${props.getProperty('snakeyaml')}"
   api 'org.apache.maven:maven-model:3.9.6'
   api 'com.networknt:json-schema-validator:1.2.0'
diff --git a/buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java b/buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java
@@ -115,7 +115,8 @@ public void execute(Task t) {
                             test.jvmArgs("--illegal-access=warn");
                         }
                     }
-                    if (test.getJavaVersion().compareTo(JavaVersion.VERSION_17) > 0) {
+                    if (test.getJavaVersion().compareTo(JavaVersion.VERSION_17) > 0
+                        && test.getJavaVersion().compareTo(JavaVersion.VERSION_24) < 0) {
                         test.jvmArgs("-Djava.security.manager=allow");
                     }
                 }
diff --git a/buildSrc/src/main/java/org/opensearch/gradle/test/DistroTestPlugin.java b/buildSrc/src/main/java/org/opensearch/gradle/test/DistroTestPlugin.java
@@ -77,9 +77,9 @@
 import java.util.stream.Stream;
 
 public class DistroTestPlugin implements Plugin<Project> {
-    private static final String SYSTEM_JDK_VERSION = "21.0.6+7";
+    private static final String SYSTEM_JDK_VERSION = "23.0.2+7";
     private static final String SYSTEM_JDK_VENDOR = "adoptium";
-    private static final String GRADLE_JDK_VERSION = "21.0.6+7";
+    private static final String GRADLE_JDK_VERSION = "23.0.2+7";
     private static final String GRADLE_JDK_VENDOR = "adoptium";
 
     // all distributions used by distro tests. this is temporary until tests are per distribution
diff --git a/client/rest-high-level/src/test/resources/org/opensearch/bootstrap/test.policy b/client/rest-high-level/src/test/resources/org/opensearch/bootstrap/test.policy
@@ -8,4 +8,5 @@
 
 grant {
   permission java.net.SocketPermission "*", "connect,resolve";
+  permission java.net.NetPermission "accessUnixDomainSocket";
 };
diff --git a/distribution/archives/build.gradle b/distribution/archives/build.gradle
@@ -38,6 +38,9 @@ CopySpec archiveFiles(CopySpec modulesFiles, String distributionType, String pla
       into('lib') {
         with libFiles()
       }
+      into('agent') {
+        with agentFiles()
+      }
       into('config') {
         dirPermissions {
           unix 0750
@@ -226,3 +229,9 @@ subprojects {
 
   group = "org.opensearch.distribution"
 }
+
+tasks.each {
+  if (it.name.startsWith("build")) {
+    it.dependsOn project(':libs:agent-sm:agent').assemble
+  }
+}
diff --git a/distribution/build.gradle b/distribution/build.gradle
@@ -357,6 +357,18 @@ configure(subprojects.findAll { ['archives', 'packages'].contains(it.name) }) {
       }
     }
 
+    agentFiles = {
+      copySpec {
+        from(project(':libs:agent-sm:agent').prepareAgent) {
+          include '**/*.jar'
+          exclude '**/*-javadoc.jar'
+          exclude '**/*-sources.jar'
+          // strip the version since jvm.options is using agent without version
+          rename("opensearch-agent-${project.version}.jar", "opensearch-agent.jar")
+        }
+      }
+    }
+
     modulesFiles = { platform ->
       copySpec {
         eachFile {
diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options
@@ -77,7 +77,7 @@ ${error.file}
 9-:-Xlog:gc*,gc+age=trace,safepoint:file=${loggc}:utctime,pid,tags:filecount=32,filesize=64m
 
 # Explicitly allow security manager (https://bugs.openjdk.java.net/browse/JDK-8270380)
-18-:-Djava.security.manager=allow
+18-23:-Djava.security.manager=allow
 
 # JDK 20+ Incubating Vector Module for SIMD optimizations;
 # disabling may reduce performance on vector optimized lucene
@@ -89,3 +89,6 @@ ${error.file}
 # See please https://bugs.openjdk.org/browse/JDK-8341127 (openjdk/jdk#21283)
 23:-XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.setAsTypeCache
 23:-XX:CompileCommand=dontinline,java/lang/invoke/MethodHandle.asTypeUncached
+
+# It should be JDK-24 (but we cannot bring JDK-24 since Gradle does not support it yet)
+21-:-javaagent:agent/opensearch-agent.jar
diff --git a/distribution/tools/launchers/src/main/java/org/opensearch/tools/launchers/SystemJvmOptions.java b/distribution/tools/launchers/src/main/java/org/opensearch/tools/launchers/SystemJvmOptions.java
@@ -85,7 +85,7 @@ static List<String> systemJvmOptions() {
     }
 
     private static String allowSecurityManagerOption() {
-        if (Runtime.version().feature() > 17) {
+        if (Runtime.version().feature() > 17 && Runtime.version().feature() < 24) {
             return "-Djava.security.manager=allow";
         } else {
             return "";
diff --git a/gradle/ide.gradle b/gradle/ide.gradle
@@ -82,7 +82,7 @@ if (System.getProperty('idea.active') == 'true') {
         runConfigurations {
           defaults(JUnit) {
             vmParameters = '-ea -Djava.locale.providers=SPI,CLDR'
-            if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17) {
+            if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17 && BuildParams.runtimeJavaVersion < JavaVersion.VERSION_24) {
               vmParameters += ' -Djava.security.manager=allow'
             }
           }
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -3,7 +3,7 @@ opensearch        = "3.0.0"
 lucene            = "10.1.0"
 
 bundled_jdk_vendor = "adoptium"
-bundled_jdk = "21.0.6+7"
+bundled_jdk = "23.0.2+7"
 
 # optional dependencies
 spatial4j         = "0.7"
diff --git a/libs/agent-sm/agent/build.gradle b/libs/agent-sm/agent/build.gradle
@@ -75,3 +75,7 @@ tasks.test {
 tasks.check {
   dependsOn test
 }
+
+tasks.named('assemble') {
+  dependsOn prepareAgent
+}
diff --git a/libs/build.gradle b/libs/build.gradle
@@ -41,20 +41,21 @@ subprojects {
    */
   project.afterEvaluate {
     if (!project.path.equals(':libs:agent-sm:agent')) {
-      configurations.all { Configuration conf ->
-        dependencies.matching { it instanceof ProjectDependency }.all { ProjectDependency dep ->
-          Project depProject = project.project(dep.path)
-          if (depProject != null
-            && (false == depProject.path.equals(':libs:opensearch-core') &&
-                false == depProject.path.equals(':libs:opensearch-common'))
-            && depProject.path.startsWith(':libs')) {
-            throw new InvalidUserDataException("projects in :libs "
-              + "may not depend on other projects libs except "
-              + ":libs:opensearch-core or :libs:opensearch-common but "
-              + "${project.path} depends on ${depProject.path}")
+        configurations.all { Configuration conf ->
+          dependencies.matching { it instanceof ProjectDependency }.all { ProjectDependency dep ->
+            Project depProject = project.project(dep.path)
+            if (depProject != null
+              && (false == depProject.path.equals(':libs:opensearch-core') &&
+                  false == depProject.path.equals(':libs:opensearch-common')&&
+                  false == depProject.path.equals(':libs:agent-sm:agent-policy'))
+              && depProject.path.startsWith(':libs')) {
+              throw new InvalidUserDataException("projects in :libs "
+                + "may not depend on other projects libs except "
+                + ":libs:opensearch-core or :libs:opensearch-common but "
+                + "${project.path} depends on ${depProject.path}")
+            }
           }
         }
-      }
     }
   }
 }
diff --git a/libs/nio/src/test/java/org/opensearch/nio/SocketChannelContextTests.java b/libs/nio/src/test/java/org/opensearch/nio/SocketChannelContextTests.java
@@ -125,6 +125,7 @@ public void testSignalWhenPeerClosed() throws IOException {
         assertTrue(context.closeNow());
     }
 
+    @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/pull/16731")
     public void testRegisterInitiatesConnect() throws IOException {
         InetSocketAddress address = mock(InetSocketAddress.class);
         boolean isAccepted = randomBoolean();
@@ -205,6 +206,7 @@ public void testConnectFails() throws IOException {
         assertSame(ioException, exception.get());
     }
 
+    @AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/pull/16731")
     public void testConnectCanSetSocketOptions() throws IOException {
         InetSocketAddress address = mock(InetSocketAddress.class);
         Config.Socket config;
diff --git a/libs/secure-sm/build.gradle b/libs/secure-sm/build.gradle
@@ -31,6 +31,7 @@ apply plugin: 'opensearch.publish'
 
 dependencies {
   // do not add non-test compile dependencies to secure-sm without a good reason to do so
+  api project(":libs:agent-sm:agent-policy")
 
   testImplementation "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
   testImplementation "junit:junit:${versions.junit}"
diff --git a/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java b/libs/secure-sm/src/main/java/org/opensearch/secure_sm/SecureSM.java
@@ -147,7 +147,9 @@ private boolean hasTrustedCallerChain() {
         };
     }
 
-    static final String[] TEST_RUNNER_PACKAGES = new String[] {
+    public static final String[] TEST_RUNNER_PACKAGES = new String[] {
+        // gradle worker
+        "worker\\.org\\.gradle\\.process\\.internal\\.worker\\.GradleWorkerMain*",
         // surefire test runner
         "org\\.apache\\.maven\\.surefire\\.booter\\..*",
         // junit4 test runner
diff --git a/plugins/repository-hdfs/build.gradle b/plugins/repository-hdfs/build.gradle
@@ -146,7 +146,7 @@ for (String fixtureName : ['hdfsFixture', 'haHdfsFixture', 'secureHdfsFixture',
     }
     final List<String> miniHDFSArgs = []
 
-    if (BuildParams.runtimeJavaVersion >= JavaVersion.VERSION_23) {
+    if (BuildParams.runtimeJavaVersion >= JavaVersion.VERSION_23 && BuildParams.runtimeJavaVersion < JavaVersion.VERSION_24) {
       miniHDFSArgs.add('-Djava.security.manager=allow')
     }
 
diff --git a/plugins/repository-hdfs/src/test/resources/org/opensearch/bootstrap/test.policy b/plugins/repository-hdfs/src/test/resources/org/opensearch/bootstrap/test.policy
@@ -0,0 +1,12 @@
+/*
+ * 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.
+ */
+
+grant {
+  permission java.net.NetPermission "accessUnixDomainSocket";
+  permission java.net.SocketPermission "*", "connect,resolve";
+};
diff --git a/plugins/repository-s3/src/internalClusterTest/resources/org/opensearch/bootstrap/test.policy b/plugins/repository-s3/src/internalClusterTest/resources/org/opensearch/bootstrap/test.policy
@@ -0,0 +1,12 @@
+/*
+ * 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.
+ */
+
+grant {
+  permission java.net.NetPermission "accessUnixDomainSocket";
+  permission java.net.SocketPermission "*", "connect,resolve";
+};
diff --git a/server/build.gradle b/server/build.gradle
@@ -71,6 +71,7 @@ dependencies {
   api project(":libs:opensearch-task-commons")
   implementation project(':libs:opensearch-arrow-spi')
 
+  compileOnly project(":libs:agent-sm:bootstrap")
   compileOnly project(':libs:opensearch-plugin-classloader')
   testRuntimeOnly project(':libs:opensearch-plugin-classloader')
 
@@ -378,7 +379,7 @@ tasks.named("licenseHeaders").configure {
 tasks.test {
     environment "node.roles.test", "[]"
     if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_1_8) {
-        jvmArgs += ["--add-opens", "java.base/java.nio.file=ALL-UNNAMED"]
+        jvmArgs += ["--add-opens", "java.base/java.nio.file=ALL-UNNAMED", "-Djdk.attach.allowAttachSelf=true", "-XX:+EnableDynamicAgentLoading" ]
     }
 }
 
diff --git a/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java b/server/src/main/java/org/opensearch/bootstrap/BootstrapChecks.java
@@ -47,6 +47,7 @@
 import org.opensearch.discovery.DiscoveryModule;
 import org.opensearch.env.Environment;
 import org.opensearch.index.IndexModule;
+import org.opensearch.javaagent.bootstrap.AgentPolicy;
 import org.opensearch.monitor.jvm.JvmInfo;
 import org.opensearch.monitor.process.ProcessProbe;
 import org.opensearch.node.NodeRoleSettings;
@@ -57,6 +58,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.AllPermission;
+import java.security.Policy;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -720,10 +722,10 @@ public final BootstrapCheckResult check(BootstrapContext context) {
 
         @SuppressWarnings("removal")
         boolean isAllPermissionGranted() {
-            final SecurityManager sm = System.getSecurityManager();
-            assert sm != null;
+            final Policy policy = AgentPolicy.getPolicy();
+            assert policy != null;
             try {
-                sm.checkPermission(new AllPermission());
+                AgentPolicy.checkPermission(new AllPermission());
             } catch (final SecurityException e) {
                 return false;
             }
diff --git a/server/src/main/java/org/opensearch/bootstrap/OpenSearch.java b/server/src/main/java/org/opensearch/bootstrap/OpenSearch.java
@@ -48,7 +48,6 @@
 
 import java.io.IOException;
 import java.nio.file.Path;
-import java.security.Permission;
 import java.security.Security;
 import java.util.Arrays;
 import java.util.Locale;
@@ -86,19 +85,7 @@ class OpenSearch extends EnvironmentAwareCommand {
     @SuppressWarnings("removal")
     public static void main(final String[] args) throws Exception {
         overrideDnsCachePolicyProperties();
-        /*
-         * We want the JVM to think there is a security manager installed so that if internal policy decisions that would be based on the
-         * presence of a security manager or lack thereof act as if there is a security manager present (e.g., DNS cache policy). This
-         * forces such policies to take effect immediately.
-         */
-        System.setSecurityManager(new SecurityManager() {
-
-            @Override
-            public void checkPermission(Permission perm) {
-                // grant all permissions so that we can later set the security manager to the one that we want
-            }
 
-        });
         LogConfigurator.registerErrorListener();
         final OpenSearch opensearch = new OpenSearch();
         int status = main(args, opensearch, Terminal.DEFAULT);
diff --git a/server/src/main/java/org/opensearch/bootstrap/Security.java b/server/src/main/java/org/opensearch/bootstrap/Security.java
diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/opensearch/common/util/concurrent/ThreadContext.java
diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy
diff --git a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy
diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java
diff --git a/server/src/test/resources/org/opensearch/bootstrap/test.policy b/server/src/test/resources/org/opensearch/bootstrap/test.policy
diff --git a/test/framework/build.gradle b/test/framework/build.gradle
diff --git a/test/framework/src/main/java/org/opensearch/bootstrap/AgentAttach.java b/test/framework/src/main/java/org/opensearch/bootstrap/AgentAttach.java
diff --git a/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/opensearch/bootstrap/BootstrapForTesting.java
diff --git a/test/framework/src/test/resources/org/opensearch/bootstrap/test.policy b/test/framework/src/test/resources/org/opensearch/bootstrap/test.policy

Original file line number	Diff line number	Diff line change
`@@ -433,12 +433,18 @@ gradle.projectsEvaluated {`
`433`	`433`
`434`	`434`	`project.tasks.withType(Test) { task ->`
`435`	`435`	`if (task != null) {`
`436`		`- if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17) {`
	`436`	`+ if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17 && BuildParams.runtimeJavaVersion <= JavaVersion.VERSION_23) {`
`437`	`437`	`task.jvmArgs += ["-Djava.security.manager=allow"]`
`438`	`438`	`}`
`439`	`439`	`if (BuildParams.runtimeJavaVersion >= JavaVersion.VERSION_20) {`
`440`	`440`	`task.jvmArgs += ["--add-modules=jdk.incubator.vector"]`
`441`	`441`	`}`
	`442`	`+`
	`443`	`+ // Add Java Agent for security sandboxing`
	`444`	`+ if (!(project.path in [':build-tools', ":libs:agent-sm:bootstrap", ":libs:agent-sm:agent"])) {`
	`445`	`+ dependsOn(project(':libs:agent-sm:agent').prepareAgent)`
	`446`	`+ jvmArgs += ["-javaagent:" + project(':libs:agent-sm:agent').jar.archiveFile.get()]`
	`447`	`+ }`
`442`	`448`	`}`
`443`	`449`	`}`
`444`	`450`
Original file line number	Diff line number	Diff line change
`@@ -115,7 +115,8 @@ public void execute(Task t) {`
`115`	`115`	`test.jvmArgs("--illegal-access=warn");`
`116`	`116`	`}`
`117`	`117`	`}`
`118`		`- if (test.getJavaVersion().compareTo(JavaVersion.VERSION_17) > 0) {`
	`118`	`+ if (test.getJavaVersion().compareTo(JavaVersion.VERSION_17) > 0`
	`119`	`+ && test.getJavaVersion().compareTo(JavaVersion.VERSION_24) < 0) {`
`119`	`120`	`test.jvmArgs("-Djava.security.manager=allow");`
`120`	`121`	`}`
`121`	`122`	`}`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ static List<String> systemJvmOptions() {`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`private static String allowSecurityManagerOption() {`
`88`		`- if (Runtime.version().feature() > 17) {`
	`88`	`+ if (Runtime.version().feature() > 17 && Runtime.version().feature() < 24) {`
`89`	`89`	`return "-Djava.security.manager=allow";`
`90`	`90`	`} else {`
`91`	`91`	`return "";`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ if (System.getProperty('idea.active') == 'true') {`
`82`	`82`	`runConfigurations {`
`83`	`83`	`defaults(JUnit) {`
`84`	`84`	`vmParameters = '-ea -Djava.locale.providers=SPI,CLDR'`
`85`		`- if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17) {`
	`85`	`+ if (BuildParams.runtimeJavaVersion > JavaVersion.VERSION_17 && BuildParams.runtimeJavaVersion < JavaVersion.VERSION_24) {`
`86`	`86`	`vmParameters += ' -Djava.security.manager=allow'`
`87`	`87`	`}`
`88`	`88`	`}`