Re: There is unnecessary resource consumption in `SecureRandom.getInstance()`.

[Bernd]

Good finding,

 but just a related comment: it has been customary to request sha1prng, but that practice should be re-evaluated since all new RNG mode features (including the parameters) are absent in that implementation amd it might sooner or later create compliance issues by the use of outdated sha1.

(Having said that I also have some places to change and cannot fully judge if the performance gets worse or the blocking problems re-occur. Maybe that would be a good candidate for a blog/statement. Especially given newer Linux kernel versions with changed /dev/random semantics, as well).

Gruss

Bernd

--

http://bernd.eckenfels.net

 

---

Von: core-libs-dev <core-libs-dev-r...@openjdk.org> im Auftrag von 구태진 <koo.tae...@gmail.com>
Gesendet: Freitag, April 28, 2023 8:06 AM
An: core-libs-dev@openjdkorg <core-libs-dev@openjdk.org>
Betreff: There is unnecessary resource consumption in `SecureRandom.getInstance()`.

 

Hello, this is my first e-mail  at this mailng-list.

It is an honor for me to write an e-mail here. :)
If I misunderstood and wrote an email, please forgive me.

- reproduction code

  tested on oracle-openjdk-20.0.1 and temurin-17.0.3

```

SecureRandom.getInstance("SHA1PRNG")

```

The reason it's slow is because exception is created during this operation. (result is ok.)

```

java.lang.NoSuchMethodException.<init>()
        at java.lang.Class.getConstructor0(Class.java:3585)
        at java.lang.Class.getConstructor(Class.java:2271)
        at java.security.Provider$Service.newInstanceUtil(Provider.java:1896)
        at java.security.Provider$Service.newInstance(Provider.java:1861)
        at sun.security.jca.GetInstance.getInstance(GetInstance.java:236)
        at sun.security.jca.GetInstance.getInstance(GetInstance.java:164)                                  
        at java.security.SecureRandom.getInstance(SecureRandom.java:387

```

I looked up why the exception was being created.

https://github.com/openjdk/jdk/blob/0deb648985b018653ccdaf193dc13b3cf21c088a/src/java.base/share/classes/java/security/Provider.java#L1587

// addEngine("SecureRandom", false, "java.security.SecureRandomParameters");

It looks that is specifying a constructor for SecureRandom at this point.

But SecureRandom looks it doesn't have that constructor (It looks like SecureRandomSpi has that constructor .).

For this reason, the initial creation fails and an exception is created.
As an alternative, it seems that created using a constructor without arguments.

```

// java.security.Provider.Service#newInstanceUtil

private Object newInstanceUtil(Class<?> ctrParamClz, Object ctorParamObj)
                throws Exception
        {
            if (ctrParamClz == null) {
                return newInstanceOf();
            } else {
                // Looking for the constructor with a params first and fallback
                // to one without if not found. This is to support the enhanced
                // SecureRandom where both styles of constructors are supported.
                // Before jdk9, there was no params support (only getInstance(alg))
                // and an impl only had the params-less constructor. Since jdk9,
                // there is getInstance(alg,params) and an impl can contain
                // an Impl(params) constructor.
                try {
                    Constructor<?> con = getImplClass().getConstructor(ctrParamClz);
                    return con.newInstance(ctorParamObj);
                } catch (NoSuchMethodException nsme) {
                    // For pre-jdk9 SecureRandom implementations, they only
                    // have params-less constructors which still works when
                    // the input ctorParamObj is null.
                    //
                    // For other primitives using params, ctorParamObj should not
                    // be null and nsme is thrown, just like before.
                    if (ctorParamObj == null) {
                        try {
                            return newInstanceOf();
                        } catch (NoSuchMethodException nsme2) {
                            nsme.addSuppressed(nsme2);
                            throw nsme;
                        }
                    } else {
                        throw nsme;
                    }
                }
            }
        }

```

There are two solutions I think.

1. Create a constructor for SecureRandom.
2. Compare using getConstructors instead of getConstrctor.

Thanks for reading. :)