On Wed, 24 May 2023 23:35:55 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Martin Balao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8301553: Support Password-Based Cryptography in SunPKCS11 (iteration #3) >> >> Co-authored-by: Francisco Ferrari <fferr...@redhat.com> >> Co-authored-by: Martin Balao <mba...@redhat.com> > > src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Mac.java line > 210: > >> 208: // because this is a PBE Mac service. In addition to >> checking >> 209: // the key, check that params (if passed) are >> consistent. >> 210: PBEUtil.checkKeyAndParams(key, params, algorithm); > > After this call, shouldn't you store key internally as p11Key? Not sure if > this is being taken care of by the P11SecretKeyFactory.convertKey(...) call > on line 248. The key is stored as in the internal field p11Key a bit later. The reason is because we need P11SecretKeyFactory::convertKey to check if the P11 key is from a different token and, eventually, re-derive it. The path taken is the same as for non-PBE Mac services. It's only in the PBE Mac case in which we had to derive the PBE key that we have absolute guarantees that calling P11SecretKeyFactory::convertKey would be useless, and we cut it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1206027982