Hi Seán!
Just a minor comment.
I don't know if it's even measurable in this context, but I was under
impression that filling memory with zero *bytes* might be a slightly
more efficient then filling with any other constant.
Maybe it is better to use Arrays.fill(passwd, '\0') instead of
Arrays.fill(passwd, '0') to give the JVM a chance to optimize filling if
it's possible?
With kind regards,
Ivan
On 8/22/18 9:25 AM, Seán Coffey wrote:
Thanks for reviewing. comments inline..
On 22/08/18 15:50, Weijun Wang wrote:
PBES2Core.java:
181 byte[] passwdBytes = key.getEncoded();
182 char[] passwdChars = null;
183 PBEKeySpec pbeSpec;
184 try {
185 if ((passwdBytes == null) ||
186 !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
187 throw new InvalidKeyException("Missing password");
188 }
....
272 } finally {
273 if (passwdChars != null) Arrays.fill(passwdChars, '
');
274 Arrays.fill(passwdBytes, (byte)0x00);
275 }
If passwdBytes == null, line 274 would throw an NPE.
Good catch. Corrected.
PBKDF2KeyImpl.java:
87 char[] passwd = keySpec.getPassword();
88 if (passwd == null) {
89 // Should allow an empty password.
90 this.passwd = new char[0];
91 } else {
92 this.passwd = passwd.clone();
93 }
94 // Convert the password from char[] to byte[]
95 byte[] passwdBytes = getPasswordBytes(this.passwd);
96 // remove local copy
97 Arrays.fill(passwd, '0');
If passwd == null, line 97 would throw an NPE.
Another good catch!
updated webrev :
http://cr.openjdk.java.net/~coffeys/webrev.8209129.v3/webrev/
regards,
Sean.
Otherwise fine.
Thanks
Max
On Aug 17, 2018, at 12:53 AM, Seán Coffey <sean.cof...@oracle.com>
wrote:
Find new webrev here Max :
http://cr.openjdk.java.net/~coffeys/webrev.8209129.v2/webrev/
regards :
--
With kind regards,
Ivan Gerasimov