On Fri, 18 Oct 2024 19:03:30 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> This is the implementation of JEP 486: Permanently Disable the Security >> Manager. See [JEP 486](https://openjdk.org/jeps/486) for more details. The >> [CSR](https://bugs.openjdk.org/browse/JDK-8338412) 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](https://bugs.openjdk.org/browse/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, ... > > Sean Mullan has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 97 commits: > > - Merge remote-tracking branch 'jdk-sandbox/jep486' into JDK-8338411 > - Change apiNote to deprecated annotation on checkAccess methods. Change > method dedescription to "Does nothing". > - Sanitize the class descriptions of DelegationPermission and > ServicePermission > by removing text that refers to granting permissions, but avoid changes > that > affect the API specification, such as the description and format of input > parameters. > - Restored methods in RMIConnection to throw SecurityExceptions again but > with adjusted text that avoids the word "permission". > - Add text to class description of MBeanServer stating that implementations > may throw SecurityException if authorization doesn't allow access to > resource. > - Restore text about needing permissions from the desktop environment in the > getPixelColor and createScreenCapture methods. > - Add api note to getClassContext to use StackWalker instead and > add DROP_METHOD_INFO option to StackWalker. > - Change checkAccess() methods to be no-ops, rather than throwing > SecurityException. > - Merge > - Merge > - ... and 87 more: https://git.openjdk.org/jdk/compare/f50bd0d9...f89d9d09 Reviewed test/jdk/java/lang/** and test/jdk/sun/reflect/* tests. test/jdk/java/lang/Class/getDeclaredField/ClassDeclaredFieldsTest.java line 31: > 29: * @summary test that all fields returned by getDeclaredFields() can be > 30: * set accessible if the right permission is granted; this test > 31: * also verifies that Class.classLoader final private field is "if the right permission is granted" can be replaced with "package java.lang is open to unnamed module". test/jdk/java/lang/Class/getDeclaredField/FieldSetAccessibleTest.java line 338: > 336: > 337: public static enum TestCase { > 338: UNSECURE; Better to drop this enum entirely. Simply call `FieldSetAccessibleTest::run` as it's the only test case. test/jdk/java/lang/StackWalker/CallerSensitiveMethod/csm/jdk/test/CallerSensitiveTest.java line 45: > 43: > 44: public static void main(String... args) throws Throwable { > 45: System.err.println("Test without security manager."); Security manager is not relevant any more. Suggest to drop this println. test/jdk/java/lang/invoke/RevealDirectTest.java line 33: > 31: * @test > 32: * @summary verify Lookup.revealDirect on a variety of input handles, > with security manager > 33: * @run > main/othervm/policy=jtreg.security.policy/secure=java.lang.SecurityManager > -ea -esa test.java.lang.invoke.RevealDirectTest line 36 can also be removed. * $ $JAVA8X_HOME/bin/java -cp $JUNIT4_JAR:../../../.. -ea -esa -Djava.security.manager test.java.lang.invoke.RevealDirectTest test/jdk/java/lang/invoke/callerSensitive/csm/jdk/test/MethodInvokeTest.java line 77: > 75: @Override > 76: public boolean implies(ProtectionDomain domain, Permission p) > { > 77: return perms.implies(p) || DEFAULT_POLICY.implies(domain, > p); static variable `DEFAULT_POLICY` (line 48) can be removed. test/jdk/java/lang/reflect/Proxy/nonPublicProxy/NonPublicProxyClass.java line 83: > 81: } > 82: > 83: private static final String NEW_PROXY_IN_PKG = "newProxyInPackage."; This constant is no longer needed. ------------- PR Review: https://git.openjdk.org/jdk/pull/21498#pullrequestreview-2383401067 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1809627796 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1809631220 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1811445313 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1811458290 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1811462419 PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1809602637
