Thanks for the comments Max. Comments inline..
On 01/08/2018 07:59, Weijun Wang wrote:
Some comments:
1. Is it possible to let rename PBEKey::clearKey to PNEKey::destroy?
Sure.
2. I am not sure if the newly added "Arrays.fill(thatEncoded, (byte)0x00)" line
in PBEKey::equals is safe. What if that key does not return a cloned copy when
getEncode() is called? This is possible if the class is only used internally and never
escape.
This is not new code. I just refactored the java.util.Arrays.fill call
to Arrays.fill.
3. You would need to modify the spec of s.s.p.KeyProtector::<init> since
password is of a new type. In fact, is this change necessary? Just to prevent the
duplication of code in convertToBytes()?
Yes, I can add edits to that effect. The class is package private and I
only found 2 usages of it. The main advantage is not having to copy or
create an extra buffer in this code. The calling code now has full
control over when it can null out the bytes in use. That should be a
help here and also means we don't place unnecessary load on GC/Cleaner.
4. I found the last line of PBEKey::<init> might be modified to
CleanerFactory.cleaner().register(this, this::clearKey);
Sure - I can modify that.
regards,
Sean.
I have never been a lambda expert so forgive me if this is not correct.
Thanks
Max
On Aug 1, 2018, at 3:11 AM, Seán Coffey <sean.cof...@oracle.com> wrote:
Looking to improve management of internal buffers in KeyStore. The
com.sun.crypto.provider.KeyProtector class uses the PBEKey class to protect
some keys. Buffers can be freed up earlier if the caller takes responsibility
for lifecycle of PBEKey object. (hence no reliance on Cleaner). Some other
minor improvements made while visiting this area.
Other improvements in sun.security.provider.KeyProtector where I believe the
password buffer can be managed by the caller. I only found 2 instances of
where this class is used.
https://bugs.openjdk.java.net/browse/JDK-8208583
http://cr.openjdk.java.net/~coffeys/webrev.8208583.v1/webrev/index.html
regards,
Sean.
