On Tue, 19 Apr 2022 15:03:05 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
> > I am not quite seeing the rationale for this change. > > There were a lot of effort to clean up the buffering password in JDK. In some > circumstances, the PasswordCallback would be called for further using of the > password. However, because the call to PasswordCallback, the password cleanup > effort was void as PasswordCallback will have a copy of the password in the > memory. > > For example, in the P11KeyStore implementation, there is an effort to clean > up the password while finalizing. ` Arrays.fill(password, ' ');` However, the > password has been set to the PasswordCallback, and where a copy is placed in > the memory. > > ``` > PasswordCallback pc = (PasswordCallback)callbacks[0]; > pc.setPassword(password); // this clones the password if not null > ``` Ok, but the [SunPKCS11 code clears the cloned password](https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java#L1513) as soon as it retrieves it from the Callback: pin = pcall.getPassword(); pcall.clearPassword(); > > Are you trying to protect against callers forgetting to call clearPassword? > > Is that really our responsibility? > > Yes, the clearPassword() method may be not called as expected. It may be not > our responsibility, but it would be good to collect the password even if the > clearPassword() method is not called. It is just something like to clean up > the socket if socket.close() is not called. I may file another PR later, > where password cleanup/destroy is should be called, but not actually. Ok, I can see how this can be a good DiD strategy. However, I think that we need to carefully check the interactions between cleaners and methods that explicitly allow the contents to be cleared so that there are not unexpected results. ------------- PR: https://git.openjdk.java.net/jdk/pull/8272
