RFR: JDK-8209129 :Further improvements to cipher buffer management

Weijun Wang weijun.wang at oracle.com
Fri Aug 10 10:53:16 UTC 2018


HmacPKCS12PBESHA1.java:

  76             byte[] passwdBytes = key.getEncoded();
  77             if ((passwdBytes == null) ||
  78                 !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
  79                 throw new InvalidKeyException("Missing password");
  80             }

 Please also cleanup passwdBytes. Maybe divide the if check above into 2 to avoid passwdBytes being assigned but algorithm is not PBE.

 132         Arrays.fill(passwdChars, '0');

 In a finally block?

PBES1Core.java:

 250         Arrays.fill(passwdBytes, (byte) 0x00);

 In a finally block?

PBES2Core.java:

 273         Arrays.fill(passwdChars, ' ');
 274         Arrays.fill(passwdBytes, (byte)0x00);

 In a finally block?

PBKDF2KeyImpl.java:

  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');

 Strange, why passwd.clone()?

 129         Arrays.fill(passwdBytes, (byte)0x00);

 In a finally block?

PBMAC1Core.java:

 111             byte[] passwdBytes = key.getEncoded();
 112             if ((passwdBytes == null) ||
 113                 !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
 114                 throw new InvalidKeyException("Missing password");
 115             }

 170         Arrays.fill(passwdChars, ' ');

 Same comments as for HmacPKCS12PBESHA1.java.

PKCS12PBECipherCore.java:

 262         char[] passwdChars = null;

 Deal with the same way as in PBMAC1Core.java?

MD4.java:

 110     void implReset() {

 Add @Override.

Thanks
Max


> On Aug 9, 2018, at 6:42 PM, Seán Coffey <sean.coffey at oracle.com> wrote:
> 
> Adding RFR to title..
> 
> On 09/08/2018 11:37, Seán Coffey wrote:
>> I've been looking further at how private/temporary buffers are used in cipher/keystore management and identified some more areas that could benefit with a more aggressive nulling out of contents.
>> 
>> I've been testing through use of stepping through debugging sessions while setting/getting keys and capturing process memory via tooling like gcore.
>> 
>> JBS report : https://bugs.openjdk.java.net/browse/JDK-8209129
>> 
>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8209129.v1/webrev/index.html
>> 
>> TCK and regression tests are green.
>> 
>> regards,
>> Sean.
>> 
> 




More information about the security-dev mailing list