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

Stuart Marks smarks at openjdk.org
Fri Sep 12 03:40:18 UTC 2025


On Thu, 11 Sep 2025 17:50:10 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> 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.

Thanks for the updates. Re-approving in case it's necessary.

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

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


More information about the client-libs-dev mailing list