Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is -Djava.security.manager=allow needed anymore now that the System.setSecurityManager calls have been removed?

Copy link
Copy Markdown
Contributor Author

@reta reta Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not anymore :-) I keep this pull request as POC to proof check any other work

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrross: @cwperks started taking a look at clean-ups of the -Djava.security.manager across all code bases in core. We think its not a lot of changes and we can do the clean-up (in a day or two) once the final PR from this POC is merged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrross: @cwperks started taking a look at clean-ups of the -Djava.security.manager across all code bases in core. We think its not a lot of changes and we can do the clean-up (in a day or two) once the final PR from this POC is merged.

Thanks @kumargu, I should have the clean pull request to core tomorrow, doing some final tests with JDK-24

}
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()]
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Comment on lines 80 to 83
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On main we have switched back to JDK21 and JDK23 has been deprecated.
Seems like JDK24 has just released yesterday.

Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like JDK24 has just released yesterday.

No worries, I will be testing with JDK-24 asap, thank you


// all distributions used by distro tests. this is temporary until tests are per distribution
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@

grant {
permission java.net.SocketPermission "*", "connect,resolve";
permission java.net.NetPermission "accessUnixDomainSocket";
};
9 changes: 9 additions & 0 deletions distribution/archives/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -226,3 +229,9 @@ subprojects {

group = "org.opensearch.distribution"
}

tasks.each {
if (it.name.startsWith("build")) {
it.dependsOn project(':libs:agent-sm:agent').assemble
}
}
12 changes: 12 additions & 0 deletions distribution/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion distribution/src/config/jvm.options
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Copy link
Copy Markdown
Contributor

@kumargu kumargu Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for Opensearch 3.0, JAVA_21 is indeed the right version, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We bundle JDK 23 but AFAIK our CI images do not have it - they will use JDK 21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you are correct.

21-:-javaagent:agent/opensearch-agent.jar
Original file line number Diff line number Diff line change
Expand Up @@ -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 "";
Expand Down
2 changes: 1 addition & 1 deletion gradle/ide.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
Expand Down
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions libs/agent-sm/agent/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,7 @@ tasks.test {
tasks.check {
dependsOn test
}

tasks.named('assemble') {
dependsOn prepareAgent
}
25 changes: 13 additions & 12 deletions libs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions libs/secure-sm/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion plugins/repository-hdfs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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";
};
Original file line number Diff line number Diff line change
@@ -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";
};
3 changes: 2 additions & 1 deletion server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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" ]
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 0 additions & 13 deletions server/src/main/java/org/opensearch/bootstrap/OpenSearch.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading