Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17641: Disable the Security Manager for Java 24+ #3153

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

HoustonPutman
Copy link
Contributor

@github-actions github-actions bot added documentation Improvements or additions to documentation tool:build start-scripts labels Jan 31, 2025
Copy link
Contributor

@uschindler uschindler left a comment

Choose a reason for hiding this comment

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

I haven't tested the Windows code, but looks fine in general.

I will update Policeman Jenkins to latest Java 24 tomorrow. We may need changes in Lucene, too.

@epugh
Copy link
Contributor

epugh commented Feb 1, 2025

It's great that you made the disabling conditional on Java 24+. I wonder though if we are better served just removing it completely from Solr 10, regardless of version of Java?

Copy link
Contributor

@malliaridis malliaridis left a comment

Choose a reason for hiding this comment

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

I've tested the Windows script schanges, works fine.

Can't build the project though with JDK 24 (ea).

Co-authored-by: Christos Malliaridis <[email protected]>
@madrob
Copy link
Contributor

madrob commented Feb 3, 2025

Agree with @epugh - isn't Security Manager all no-ops even with 21 (which IIRC is the minimum Java for Solr 10)

@HoustonPutman
Copy link
Contributor Author

Agree with @epugh - isn't Security Manager all no-ops even with 21 (which IIRC is the minimum Java for Solr 10)

I've never heard that. Do you have a link for that?

@uschindler
Copy link
Contributor

uschindler commented Feb 5, 2025

Agree with @epugh - isn't Security Manager all no-ops even with 21 (which IIRC is the minimum Java for Solr 10)

I've never heard that. Do you have a link for that?

No, it isn't. It's fully working in 21. It gets a noop in 24 (actually Access controller). In addition, you can't configure it anymore.

@@ -204,6 +204,10 @@ allprojects {
if (project.ext.has("useSecurityManager")) {
useSecurityManager = project.ext.get("useSecurityManager")
}
// Solr does not run with the Security Mangager for Java 24+
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Solr does not run with the Security Mangager for Java 24+
// There is no longer a Security Manager in Java 24

@@ -35,6 +35,8 @@ plugins {
}

// Declare default Java versions for the entire project and for SolrJ separately
rootProject.ext.currentJavaVersion = JavaVersion.current()
rootProject.ext.minJavaVersionSolrJ = JavaVersion.toVersion(libs.versions.java.solrj.get())
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicates line 41

@@ -1155,6 +1155,11 @@ else
REMOTE_JMX_OPTS=()
fi

# Do not use the java security manager when running Java 24+
if (( JAVA_VER_NUM >= 24 )) ; then
Copy link
Contributor

Choose a reason for hiding this comment

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

could just combine this condition into line 1164. Ideally this changes the "true" in the default interpretation of that env var to a "false". But it's fine.

Comment on lines +72 to +73
Java removed support for the Security Manager starting with Java 24, therefore Solr will disable the feature when run with Java 24 or later.

Copy link
Contributor

Choose a reason for hiding this comment

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

minor: Okay but arguably you didn't have to bother to say this. There's no action for the user to take. I don't think it's up to Solr to tell people what's coming/going/happening in the Java ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation start-scripts tool:build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants