RFR: 8338536: Permanently disable remote code downloading in JNDI

[Daniel Fuchs]

On Fri, 15 Nov 2024 17:03:50 GMT, Aleksei Efimov <aefimov at 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