On Fri, 26 May 2023 22:17:01 GMT, Martin Balao <mba...@openjdk.org> wrote:
>> Good point. As I see it, the problem is not in the random source itself but >> in the values. There are a couple of P11PBECipher::engineInit paths in which >> P11PBECipher initialization succeeds but the pbes2Params does not have the >> salt, iCount and ivSpec in use. These paths are those in which the P11 key >> was already derived (it's a P11PBEKey): we check consistency but record >> nothing for future P11PBECipher::engineGetParameters calls. I think that we >> can get the right values from the P11PBEKey and PBEParameterSpec. Notice >> that if the ivSpec is not passed, it's value could be randomly generated in >> the underlying Cipher. @franferrax what do you think? > > We can confirm that the problem was indeed as in my previous comment. As part > of fixing this issue, we did some internal refactoring to PBEUtil.PBES2Params > which have simplified the code. In particular, PBEUtil.PBES2Params has a > method called "initialize" which consolidate all possible state > initialization and is called from P11PBECipher::engineGetParameters, > PBEUtil.PBES2Params::getPBEKeySpec and > PBEUtil.PBES2Params::getAlgorithmParameters. One more comment. You will see JCAUtil.getSecureRandom still passed BUT this is used only if the PBEUtil.PBES2Params instance was not initialized before from engineInit. Notice that if it was initialized before, a random passed to engineInit was used to generate the IV and salt values. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1207427324