[PATCH] Add class java.security.StandardMessageDigests

[Florian Weimer]

On 05/06/2014 01:39 PM, Xuelei Fan wrote:
> Another concern of mine is about that the returned MessageDigest object
> heavily depends on the providers configuration when the object get
> instantiated for the 1st time.  If the providers configuration get
> updated during run time, the provider of the returned MessageDigest
> object does not get updated accordingly.  This proposal may be a pretty
> good practice in application layer in some circumstance, but may result
> in unnecessary complexity in JRE layer because we don't know the exact
> context that the application want to use this class, and the exact
> implementation details of the underlying MD service provider.

Hmm.  It would certainly be possible to invalidate the cache if the 
provider configuration is changed.

It's unlikely that a library-based reimplementation could get this right.

> Per the spec, clone() may throw CloneNotSupportedException. It is OK a
> certain provider does not support Cloneable.

The key part is that the behavior has to be consistent across all 
objects.  It's not required that clone() works, but if it works for one 
instance, it works for all of them.

>> The
>> TLS implementation also relies on this behavior (i.e. clone() either
>> consistently fails or consistently succeeds).
>>
> That's true. SunJSSE requires cloneable MD implementation.

Are you sure?  It contains a fairly elaborate workaround for the 
non-cloneable case (construct as many digest objects as needed, and then 
feed them data in parallel so that you can finalize one pseudo-clone, 
but continue hashing using the other one).

> If the performance can get improvement here by define a static cloneable
> object, it might be also can get improvement in the same way in the
> provider implementation, unless the improvement is all about
> implementation service look up among the supported providers.

It's the service lookup.  Well, it was the service lookup in 2010.

>>> BTW! I guess in some situation or some providers, clone() might not be
>>> a lightweight operation.
>>
>> Hmm.  I can see that the state cannot be cloned at all (that's why
>> cloning is optional), but allocating a new state has to happen anyway,
>> no matter how the object is constructed.
>>
> Let's consider a case, every MD object should be bound to a session, and
> the operations should be synchronized in the session. clone() will share
> the session, the operations among different MD objects that share the
> session need to be synchronized.  I think, there is a significant
> performance and scalablity impact if StandardMessageDigests is used for
> such cases in concurrency context.

Why would clone() share the session, but constructing a new digest would 
not?  Because sharing the session would be the only way to share the 
state?  Ugh, yes, you might be right.

>> What I meant is that there might be no place to store the reference to a
>> long-term MessageDigest object which is reused again and again, so the
>> best thing the code can do is to allocate a fresh object when it is needed?
>>
> The object of MessageDigest is not immutable.  IMHO, as we discussed
> above, there are a lot of issues need to be considered carefully to
> support such a long-term object in JRE.

I meant a method like this:

    public static byte[] sha1(byte[] buffer, int offset, int length) {
        MessageDigest md = threadLocalMDs.sha1();
        md.update(buffer, offset, length);
        return md.digest();
    }

The actual digest object would not be exposed through this interface. 
This would cover the use case of hashing short buffers, where the 
instantiation overhead is most visible.


-- 
Florian Weimer / Red Hat Product Security Team