Max,
some updates.
1.
I wasn't able to rename to destroy since that method is reserved for the
Destroyable interface. I've gone with destroyKey.
3.
As per offline communication, I've removed the ASCII reference in the
s.s.p.KeyProtector constructor comments. I don't believe ASCII
requirement was ever enforced (nor needed). We use the 16 bits if
presented. If password standards need review, that work should be
carried out in another bug report.
4.
I wasn't able to use your suggested method reference syntax. The
reference to this would prevent the Object from being cleaned. The
PBEKeyCleanupTest testcase seems to confirm that.
http://cr.openjdk.java.net/~coffeys/webrev.8208583.v2/webrev/index.html
Regards,
Sean.
On 01/08/18 09:17, Seán Coffey wrote:
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.
