RFR 8051408: JEP 273: DRBG-Based SecureRandom Implementations

[Xuelei Fan]

On 4/21/2016 11:10 PM, Wang Weijun wrote:
> 
>> On Apr 21, 2016, at 9:44 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>
>>>> public MyCertStore extends CertStoreSpi {
>>>>
>>>>   public MyCertStore() {
>>>>       // whatever
>>>>       // ;-) Don't ask me why this construct is necessary.
>>>>   }
>>>>
>>>>   public MyCertStore(XXX params) {
>>>>       // throws NoSuchMethodException
>>>>       // ;-) Don't ask me why throw this exception.
>>>>   }
>>>> }
>>>>
>>>> newInstanceUtil(MyCertStore, ...)
>>>>
>>>> The MyCertStore() would get called, unexpectly.  Am I missing something?
>>>
>>> Probably not, unless you call getInstance(arg, null). I am not sure this null will trigger some other exception along the way.
>>>
>>> OK, I admit there is a side effect here: If you design getInstance(alg,params) but params is always null, then you can only implement a constructor with no params.
>>>
>>> This is stupid and useless, but not really harmful.
>>>
>> Can you explain more here?
> 
> The code change looks like this
> 
>      private static Object newInstanceUtil(final Class<?> clazz,
>          final Class<?> ctrParamClz, final Object ctorParamObj)
>          throws Exception {
>          if (ctrParamClz == null) {
>              Constructor<?> con = clazz.getConstructor();
>              return con.newInstance();
>          } else {
> -            Constructor<?> con = clazz.getConstructor(ctrParamClz);
> -            return con.newInstance(ctorParamObj);
> +            try {
> +                Constructor<?> con = clazz.getConstructor(ctrParamClz);
> +                return con.newInstance(ctorParamObj);
> +            } catch (NoSuchMethodException nsme) {
> +                if (ctorParamObj == null) {
> +                    try {
> +                        Constructor<?> con = clazz.getConstructor();
> +                        return con.newInstance();
> +                    } catch (NoSuchMethodException nsme2) {
> +                        nsme.addSuppressed(nsme2);
> +                        throw nsme;
> +                    }
> +                } else {
> +                    throw nsme;
> +                }
> +            }
>          }
>      }
> 
> So in order for the arg-less constructor to be called, you will need
> 
> 1. ctrParamClz != null, i.e. there is a getInstance(arg,params) API.
> 
> 2. ctorParamObj == null, i.e. someone calls it with getInstance(arg) or getInstance(arg,null).
> 
> 3. nsme caught, i.e. the implementation has not provided a constructor with args
> 
> This matches the "otherwise" part of what I described in the @implSpec of SecureRandomSpi, which I don't suggest new implementation doing it, and is not what all non-SecureRandom implementations are doing now (they always have a constructor with args).
> 
Got it.

Constructor-Fall-back is not common in general.  I think this is only
apply to SecureRandom.  No problem for known engines.

In the new update, there are a few description about SecureRandom, but
no control code related to SecureRandom explicitly.  A reader like me
would have to ask a lot questions before I can understand the logic.
For safe, what do you think about adding a check to make sure it is for
SecureRandom only (instanceof, etc), or a note so that we don't forgot
to revise if thing get changed in the future?

A simple update may be:

-    // For other primitives using params, ctorParamObj should not
+    // For other known primitives using params, ctorParamObj should not
     // be null and nsme is thrown, just like before.

Thanks,
Xuelei