RFR: 8258915: Temporary buffer cleanup [v8]

Weijun Wang weijun at openjdk.java.net
Thu Feb 18 16:43:49 UTC 2021


On Thu, 18 Feb 2021 05:03:58 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   materials
>
> src/java.base/share/classes/com/sun/crypto/provider/TlsKeyMaterialGenerator.java line 97:
> 
>> 95:         } finally {
>> 96:             Arrays.fill(masterSecret, (byte)0);
>> 97:         }
> 
> It seems that for other Tls* classes, the Arrays.fill(...) call is still inside each method instead of being moved up a level. Just curious why this is done differently?

The `engineGenerateKey0` method is quite long and I don't want to wrap everything in a big try-finally block, so I move it a little higher. Now `masterSecret` is still created and cleaned in the same method.

> src/java.base/share/classes/com/sun/crypto/provider/TlsKeyMaterialGenerator.java line 186:
> 
>> 184:             serverMacKey = new SecretKeySpec(tmp, "Mac");
>> 185: 
>> 186:             Arrays.fill(tmp, (byte)0);
> 
> It looks like you can use the SecretKeySpec(byte[], int, int, String) to simplify the code at line 175-186. Essentially, the code block does: 
> clientMacKey = new SecretKeySpec(keyBlock, ofs, macLength, "Mac");
> ofs += macLength;
> serverMacKey = new SecretKeySpec(keyBlock, ofs, macLength, "Mac");

Good idea.

> src/java.base/share/classes/com/sun/crypto/provider/TlsKeyMaterialGenerator.java line 220:
> 
>> 218:                     System.arraycopy(keyBlock, ofs, tmp, 0, ivLength);
>> 219:                     ofs += ivLength;
>> 220:                     serverIv = new IvParameterSpec(tmp);
> 
> Seems easier to just use the IvParameterSpec(byte[], int, int) constructor?

Yes.

> src/java.base/share/classes/com/sun/crypto/provider/TlsKeyMaterialGenerator.java line 251:
> 
>> 249:                         clientIv = new IvParameterSpec(tmp);
>> 250:                         System.arraycopy(block, ivLength, tmp, 0, ivLength);
>> 251:                         serverIv = new IvParameterSpec(tmp);
> 
> Again, consider using IvParameterSpec(byte[], int, int) and get rid of tmp?

Yes.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2070


More information about the security-dev mailing list