RFR: 8284910: Buffer clean in PasswordCallback [v2]

[Sean Mullan]

On Tue, 19 Apr 2022 15:03:05 GMT, Xue-Lei Andrew Fan <xuelei at 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