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

Reply via email to