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

8338411: Implement JEP 486: Permanently Disable the Security Manager #21498

Closed
wants to merge 230 commits into from

Conversation

seanjmullan
Copy link
Member

@seanjmullan seanjmullan commented Oct 14, 2024

This is the implementation of JEP 486: Permanently Disable the Security Manager. See JEP 486 for more details. The CSR describes in detail the main changes in the JEP and also includes an apidiff of the specification changes.

NOTE: the majority (~95%) of the changes in this PR are test updates (removal/modifications) and API specification changes, the latter mostly to remove @throws SecurityException. The remaining changes are primarily the removal of the SecurityManager, Policy, AccessController and other Security Manager API implementations. There is very little new code.

The code changes can be broken down into roughly the following categories:

  1. Degrading the behavior of Security Manager APIs to either throw Exceptions by default or provide an execution environment that disallows access to all resources by default.
  2. Changing hundreds of methods and constructors to no longer throw a SecurityException if a Security Manager was enabled. They will operate as they did in JDK 23 with no Security Manager enabled.
  3. Changing the java command to exit with a fatal error if a Security Manager is enabled.
  4. Removing the hotspot native code for the privileged stack walk and the inherited access control context. The remaining hotspot code and tests related to the Security Manager will be removed immediately after integration - see JDK-8341916.
  5. Removing or modifying hundreds of tests. Many tests that tested Security Manager behavior are no longer relevant and thus have been removed or modified.

There are a handful of Security Manager related tests that are failing and are at the end of the test/jdk/ProblemList.txt, test/langtools/ProblemList.txt and test/hotspot/jtreg/ProblemList.txt files - these will be removed or separate bugs will be filed before integrating this PR.

Inside the JDK, we have retained calls to SecurityManager::getSecurityManager and AccessController::doPrivileged for now, as these methods have been degraded to behave the same as they did in JDK 23 with no Security Manager enabled. After we integrate this JEP, those calls will be removed in each area (client-libs, core-libs, security, etc).

I don't expect each reviewer to review all the code changes in this JEP. Rather, I advise that you only focus on the changes for the area (client-libs, core-libs, net, security, etc) that you are most familiar with.


Progress

  • Change must not contain extraneous whitespace
  • Change requires CSR request JDK-8338412 to be approved
  • Commit message must refer to an issue
  • Change requires a JEP request to be targeted
  • Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

  • JDK-8338411: Implement JEP 486: Permanently Disable the Security Manager (Enhancement - P3)
  • JDK-8338625: JEP 486: Permanently Disable the Security Manager (JEP)
  • JDK-8338412: Implement JEP 486: Permanently Disable the Security Manager (CSR)

Reviewers

Reviewers without OpenJDK IDs

Contributors

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21498/head:pull/21498
$ git checkout pull/21498

Update a local copy of the PR:
$ git checkout pull/21498
$ git pull https://git.openjdk.org/jdk.git pull/21498/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21498

View PR using the GUI difftool:
$ git pr show -t 21498

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21498.diff

Using Webrev

Link to Webrev Comment

AlanBateman and others added 30 commits September 30, 2024 18:02
Co-authored-by: Sean Mullan <[email protected]>
Co-authored-by: Alan Bateman <[email protected]>
Co-authored-by: Weijun Wang <[email protected]>
Co-authored-by: Aleksei Efimov <[email protected]>
Co-authored-by: Brian Burkhalter <[email protected]>
Co-authored-by: Daniel Fuchs <[email protected]>
Co-authored-by: Harshitha Onkar <[email protected]>
Co-authored-by: Joe Wang <[email protected]>
Co-authored-by: Jorn Vernee <[email protected]>
Co-authored-by: Justin Lu <[email protected]>
Co-authored-by: Kevin Walls <[email protected]>>
Co-authored-by: Lance Andersen <[email protected]>
Co-authored-by: Naoto Sato <[email protected]>
Co-authored-by: Roger Riggs <[email protected]>
Co-authored-by: Brent Christian <[email protected]>
setInitialContextFactoryBuilder and setObjectFactoryBuilder methods in
javax.naming.spi.NamingManager.
permission cannot be used anymore to control access.
…sion

checks of the Class.getNestHost and getNestMembers methods, which no
longer apply.
Copy link
Contributor

@RogerRiggs RogerRiggs left a comment

Choose a reason for hiding this comment

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

Good to go!

@openjdk openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Nov 12, 2024
@seanjmullan
Copy link
Member Author

/integrate

@seanjmullan
Copy link
Member Author

/reviewers 2

@openjdk openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Nov 12, 2024
@openjdk
Copy link

openjdk bot commented Nov 12, 2024

@seanjmullan This pull request has not yet been marked as ready for integration.

@openjdk
Copy link

openjdk bot commented Nov 12, 2024

@seanjmullan
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

Copy link
Contributor

@LanceAndersen LanceAndersen left a comment

Choose a reason for hiding this comment

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

Thank you for all of your hard work on this JEP Sean.

@seanjmullan
Copy link
Member Author

/integrate

@seanjmullan
Copy link
Member Author

/integrate

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Nov 12, 2024
@seanjmullan
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Nov 12, 2024

Going to push as commit db85090.
Since your change was applied there have been 5 commits pushed to the master branch:

  • c12b386: 8338007: [JVMCI] ResolvedJavaMethod.reprofile can crash ciMethodData
  • 81752c4: 8338565: Test crashed: assert(is_path_empty()) failed: invariant
  • e5eaa7f: 8343946: JFR: Wildcard should only work with COUNT for 'jfr view'
  • 2989d87: 8343805: RISC-V: JVM crashes on startup when disabling compressed instructions
  • 78b8015: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Nov 12, 2024
@openjdk openjdk bot closed this Nov 12, 2024
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 12, 2024
@openjdk
Copy link

openjdk bot commented Nov 12, 2024

@seanjmullan Pushed as commit db85090.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@openjdk
Copy link

openjdk bot commented Nov 12, 2024

@seanjmullan The command integrate can only be used in open pull requests.

@openjdk
Copy link

openjdk bot commented Nov 12, 2024

@seanjmullan The command integrate can only be used in open pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.