RFR: 8354469: Keytool exposes the password in plain text when command is piped using | grep [v3]

[Weijun Wang]

On Thu, 11 Sep 2025 06:26:37 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Weijun Wang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>> 
>>  - Merge branch 'master' into 8354469
>>  - decouple PassFailJFrame.java change; simplify code flow
>>  - the fix
>
> src/java.base/share/classes/sun/security/util/Password.java line 193:
> 
>> 191:                 ba[bb.position()] = '\n';
>> 192:             }
>> 193:             return ba;
> 
> Some random comments. This is all internal to this class, and it's all vaguely cleanup, so it's up to you whether to take any action on any of these points.
> 
> It might be worth having a comment on this method that explains why it's doing this. The "obvious" way of encoding the characters is to use String.getBytes(), but you probably don't want to create a String, because (I think) you want to be able to erase the arrays afterwards. Worth explaining, if true.
> 
> At lines 190-191 the mixture of buffer and array stuff is kind of confusing. I think you can check whether bb.remaining() > 0 and use bb.put().
> 
> There are actually three possibilities for what's in the returned byte array: 1) it's completely filled with encoded bytes from the input; 2) it contains the encoded input followed by an NL byte; or 3) it contains the encoded input, an NL byte, followed by an arbitrary number of zero bytes. The way that the returned array is processed by the caller handles this, but it seems a bit brittle. One possibility is always to return an array of the exact correct length, including an NL byte. Of course this entails an extra copy (and an extra array to erase) but there are fewer cases for the caller to handle.

Thanks. I'll add some comments and do not mix buffer and array. The `\n` trick as a stop sign will be kept to avoid too many changes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27196#discussion_r2341920574