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

[Stuart Marks]

On Wed, 10 Sep 2025 17:33:05 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Allow password hiding even if there is no `System.console`. A manual test is included.
>
> 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

Marked as reviewed by smarks (Reviewer).

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.

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

PR Review: https://git.openjdk.org/jdk/pull/27196#pullrequestreview-3209119549
PR Review Comment: https://git.openjdk.org/jdk/pull/27196#discussion_r2338827872