On Fri, 15 Nov 2024 17:03:50 GMT, Aleksei Efimov <aefi...@openjdk.org> wrote:
> This PR permanently disable remote code downloading in JNDI/LDAP and JNDI/RMI > JDK providers, and contains the following changes: > - The following two properties are removed: > - `com.sun.jndi.ldap.object.trustURLCodebase` > - `com.sun.jndi.rmi.object.trustURLCodebase` > - JNDIs object factories logic has been altered to make it possible to > reconstruct object factories from remote locations when a custom > [ObjectFactoryBuilder](https://docs.oracle.com/en/java/javase/23/docs/api/java.naming/javax/naming/spi/ObjectFactoryBuilder.html) > is assigned via the > [NamingManager#setObjectFactoryBuilder](https://docs.oracle.com/en/java/javase/23/docs/api/java.naming/javax/naming/spi/NamingManager.html#setObjectFactoryBuilder(javax.naming.spi.ObjectFactoryBuilder)) > API. > - The `NamingManager` class-level documentation is edited to remove > references to the `SecurityManager`. It was also revised to clarify a > reconstruction mechanism of object factories from remote references in the > presence of a custom `ObjectFactoriesBuilder`. > - Also, the modified classes have been cleaned-up from `SecurityManager`, > `doPrivildged`, and `AccessController` usages. > > These changes require a CSR that will be submitted soon. > > ### Testing > - Added a new test to check if NamingManager#setObjectFactoryBuilder can be > used to implement remote code downloading: > `test/jdk/com/sun/jndi/rmi/registry/objects/ObjectFactoryBuilderCodebaseTest.java` > - `jdk-tier1` to `jdk-tier3` and other JNDI LDAP/RMI tests show no issue with > the proposed changes. src/java.naming/share/classes/com/sun/naming/internal/NamingManagerHelper.java line 205: > 203: if (clas == null && > 204: ref.getFactoryClassLocation() != null) { > 205: return null; This is what I suggest: at line 197 above (CNFE) just do: } catch (ClassNotFoundException e) { return null; } Then you can remove the rest of the code that follows and simply do assert clas != null @SuppressWarnings("deprecation") // Class.newInstance ObjectFactory result = clas.newInstance(); return result; } src/java.naming/share/classes/javax/naming/spi/NamingManager.java line 129: > 127: * is not supported, unless a custom {@link ObjectFactoryBuilder} > 128: * {@linkplain #setObjectFactoryBuilder(ObjectFactoryBuilder) is > set} > 129: * to determine object factories load policy. Maybe change to something like: Downloading a factory class from a location specified in the reference is not supported out of the box. A custom {@link ObjectFactoryBuilder} {@linkplain #setObjectFactoryBuilder(ObjectFactoryBuilder) may be used} if a different policy is desired. src/jdk.naming.rmi/share/classes/com/sun/jndi/rmi/registry/RegistryContext.java line 472: > 470: * Classes may only be loaded from an arbitrary URL codebase > when > 471: * a custom ObjectFactoryBuilder() is setup, otherwise load > from > 472: * arbitrary URL codebase is disabled. Maybe reuse the text above: Downloading a factory class from a location specified in the reference is not supported out of the box. A custom {@link ObjectFactoryBuilder} {@linkplain #setObjectFactoryBuilder(ObjectFactoryBuilder) may be used} if a different policy is desired. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22154#discussion_r1847105861 PR Review Comment: https://git.openjdk.org/jdk/pull/22154#discussion_r1847123637 PR Review Comment: https://git.openjdk.org/jdk/pull/22154#discussion_r1847129710
