The DEFINITIVE thread on secure password entry (console, gui, code reviews)
Feb 10, 2004 1:37 PM
Like many of you, I have an application that reads in passwords to do encryption/decryption. (This is part of a project to do automated backup, and I want those files that are written out of my encrypted NTFS partitions--such as ones written to my optical drive or ftped to a friend's server--to be encrypted. If you want to see code samples from this larger project I will do a separate post.)
I have spent some time looking at this forum as well as Sun's code, and I would like to have one place where definitive information on this subject can be found. The bulk of this post is going to be my code reviews of code samples that others have put out, along with my current thinking about how to do things. I would love to hear your feedback.
The first topic is how to read chars off a stream securely. Sun suggests the following code (see http://java.sun.com/j2se/1.4.2/docs/guide/security/jce/JCERefGuide.html#PBEEx), so I suspect that it is used by tons of people:
/**
* Reads user password from given input stream.
*/
publicchar[] readPasswd(InputStream in) throws IOException {
char[] lineBuffer;
char[] buf;
int i;
buf = lineBuffer = newchar[128];
int room = buf.length;
int offset = 0;
int c;
loop: while (true) {
switch (c = in.read()) {
case -1:
case '\n':
break loop;
case '\r':
int c2 = in.read();
if ((c2 != '\n') && (c2 != -1)) {
if (!(in instanceof PushbackInputStream)) {
in = new PushbackInputStream(in);
}
((PushbackInputStream)in).unread(c2);
} elsebreak loop;
default:
if (--room < 0) {
buf = newchar[offset + 128];
room = buf.length - offset - 1;
System.arraycopy(lineBuffer, 0, buf, 0, offset);
Arrays.fill(lineBuffer, ' ');
lineBuffer = buf;
}
buf[offset++] = (char) c;
break;
}
}
if (offset == 0) {
returnnull;
}
char[] ret = newchar[offset];
System.arraycopy(buf, 0, ret, 0, offset);
Arrays.fill(buf, ' ');
return ret;
}
While I am grateful to this document, especially for pointing out that char[]s should be used for top security, I claim that this code suffers from at least 2 bugs:
bug #1: fails to handle Mac streams (which use a simple '\r' as line ends); in the code above, a \r (that is not followed by \n) falls thru to the default case, and so will be part of the result
bug #2: fails to guarantee that all char[]s get blanked out (need finally clause if exception occurs)
In addition, albeit much less critical, are some poor programming practices:
--can return null (better is to return a zero-length array, so that the caller need never do null checks)
--too terse of names
--variable declaration not sufficiently localized (best practice is to always declare them at as low a level as possible, not all of them at the beginning of method like C requires)
--lack of modularity (fine grained methods that do one simple function are best both for code clarity as well as reuse), .
Here is what I suggest as a replacement:
/**
* Reads chars from in until an end-of-line sequence (EOL) or end-of-file (EOF) is encountered,
* and then returns the data as a char[].
* <p>
* The EOL sequence may be any of the standard formats: '\n' (unix), '\r' (mac), "\r\n" (dos).
* The EOL sequence is always completely read off the stream but is never included in the result.
* <i>Note:</i> this means that the result will never contain the chars '\n' or '\r'.
* In order to guarantee reading thru but not beyond the EOL sequence for all formats (unix, mac, dos),
* this method requires that a PushbackInputStream and not a more general InputStream be supplied.
* <p>
* The code is secure: no Strings are used, only char arrays,
* and all such arrays other than the result are guaranteed to be blanked out after last use to ensure privacy.
* Thus, this method is suitable for reading in sensitive information such as passwords.
* <p>
* This method never returns null; if no data before the EOL or EOF is read, a zero-length char[] is returned.
* <p>
* @throws IllegalArgumentException if in == null
* @throws IOException if an I/O problem occurs
* @see <a href="http://java.sun.com/j2se/1.4.2/docs/guide/security/jce/JCERefGuide.html#PBEEx">Password based encryption code examples from JCE documentation</a>
*/
publicstaticfinalchar[] readLineSecure(PushbackInputStream in) throws IllegalArgumentException, IOException {
if (in == null) thrownew IllegalArgumentException("in == null");
char[] buffer = null;
try {
buffer = newchar[128];
int offset = 0;
loop: while (true) {
int c = in.read();
switch (c) {
case -1:
case '\n':
break loop;
case '\r':
int c2 = in.read();
if ((c2 != '\n') && (c2 != -1)) in.unread(c2); // guarantees that mac & dos line end sequences are completely read thru but not beyond
break loop;
default:
buffer = checkBuffer(buffer, offset);
buffer[offset++] = (char) c;
break;
}
}
char[] result = newchar[offset];
System.arraycopy(buffer, 0, result, 0, offset);
return result;
}
finally {
eraseChars(buffer);
}
}
/**
* Checks if buffer is sufficiently large to store an element at an index == offset.
* If it is, then buffer is simply returned.
* If it is not, then a new char[] of more than sufficient size is created and initialized with buffer's current elements and returned;
* the original supplied buffer is guaranteed to be blanked out upon method return in this case.
* <p>
* @throws IllegalArgumentException if buffer == null; offset < 0
*/
publicstaticfinalchar[] checkBuffer(char[] buffer, int offset) throws IllegalArgumentException {
if (buffer == null) thrownew IllegalArgumentException("buffer == null");
if (offset < 0) thrownew IllegalArgumentException("offset = " + offset + " is < 0");
if (offset < buffer.length)
return buffer;
else {
try {
char[] bufferNew = newchar[offset + 128];
System.arraycopy(buffer, 0, bufferNew, 0, buffer.length);
return bufferNew;
}
finally {
eraseChars(buffer);
}
}
}
/** If buffer is not null, fills buffer with space (' ') chars. */
publicstaticfinalvoid eraseChars(char[] buffer) {
if (buffer != null) Arrays.fill(buffer, ' ');
}
Next, for my app, I have no intrinsic reason to bring up a gui, so I would prefer to just be able to read the password from the console (the default System.in).
This naturally brings up how you can mask the keyboard input so that the password chars do not appear for anyone walking by to read (or the NSA with their CRT reading devices; see http://www.eskimo.com/~joelm/tempest.html or http://www.theregister.co.uk/content/4/15743.html).
Tons of others have already asked this question; the first few obtained from a forum search are:
http://forum.java.sun.com/thread.jsp?forum=17&thread=26514
http://forum.java.sun.com/thread.jsp?forum=1&thread=165885
http://forum.java.sun.com/thread.jsp?forum=54&thread=230935
http://forum.java.sun.com/thread.jsp?forum=31&thread=141432
http://forum.java.sun.com/thread.jsp?forum=31&thread=483248
http://forum.java.sun.com/thread.jsp?forum=31&thread=438848
The summary of the above is either have to use JNI to access some low level platform specific stuff, or use a line erasing thread as a masking hack.
Sun's often referenced and (possibly often used) code which illustrates how the latter technique works is (see http://java.sun.com/features/2002/09/pword_mask.html):
Code Sample 1: MaskingThread.java
/**
* This class attempts to erase characters echoed to the console.
*/
class MaskingThread extends Thread {
privateboolean stop = false;
privateint index;
private String prompt;
/**
*@param prompt The prompt displayed to the user
*/
public MaskingThread(String prompt) {
this.prompt = prompt;
}
/**
* Begin masking until asked to stop.
*/
publicvoid run() {
while(!stop) {
try {
// attempt masking at this rate
this.sleep(1);
}catch (InterruptedException iex) {
iex.printStackTrace();
}
if (!stop) {
System.out.print("\r" + prompt + " \r" + prompt);
}
System.out.flush();
}
}
/**
* Instruct the thread to stop masking.
*/
publicvoid stopMasking() {
this.stop = true;
}
}
which can be used along with this to form a complete program:
Code Sample 2: PasswordField.java
import java.io.*;
/**
* This class prompts the user for a password and attempts to mask input with ""
*/
publicclass PasswordField {
/**
*@param prompt The prompt to display to the user.
*@return The password as entered by the user.
*/
String getPassword(String prompt) throws IOException {
// password holder
String password = "";
MaskingThread maskingthread = new MaskingThread(prompt);
Thread thread = new Thread(maskingthread);
thread.start();
// block until enter is pressed
while (true) {
char c = (char)System.in.read();
// assume enter pressed, stop masking
maskingthread.stopMasking();
if (c == '\r') {
c = (char)System.in.read();
if (c == '\n') {
break;
} else {
continue;
}
} elseif (c == '\n') {
break;
} else {
// store the password
password += c;
}
}
return password;
}
}
Again, while I am grateful to this document for pointing out such a clever hack, I claim that this code suffers from at least 3 bugs:
bug #1: fails to handle Mac streams (which use a simple '\r' as line ends); in the code above, a \r (that is not followed by \n) will end up part of the result
bug #2: the stop field (in MaskingThread) is not volatile, which you really need in order to guarantee visibility across threads (especially on multi cpu boxes)
bug #3: it assumes that only 1 char has been typed beyond the prompt since the last mask; it is possibile, however, that several could have
In addition, it has some other poor programming practices:
--ignores interrupts, always throwing away the InterruptedException (extremely bad practice)
--fails to boost priority of the masking thread during the time that it masks, which is critical in helping ensure that masking always takes place when system under heavy load
--the run method, altho it flushes out, never checks for errors; turns out that you can very conveniently do both operations with a simple call to checkError
--the main password reading thread fails to use join to ensure that the masking thread is dead before continuing; this is probably a good idea
Here is code that I suggest as a replacement for the above:
/**
* Reads and returns some sensitive piece of information (e.g. a password)
* from the console (i.e. System.in and System.out) in a secure manner.
* <p>
* For top security, all console input is masked out while the user types in the password.
* Once the user presses enter, the password is read via a call to {@link #readLineSecure readLineSecure(in)},
* using a PushbackInputStream that wraps System.in.
* <p>
* This method never returns null.
* <p>
* @throws IOException if an I/O problem occurs
* @throws InterruptedException if the calling thread is interrupted while it is waiting at some point
* @see <a href="http://java.sun.com/features/2002/09/pword_mask.html">Password masking in console</a>
*/
publicstaticfinalchar[] readConsoleSecure(String prompt) throws IOException, InterruptedException {
System.out.println();
// start a separate thread which will mask out all chars typed on System.in by overwriting them using System.out:
StreamMasker masker = new StreamMasker(System.out, prompt);
Thread threadMasking = new Thread(masker);
threadMasking.start();
// Goal: block this current thread (allowing masker to mask all user input)
// while the user is in the middle of typing the password.
// This may be achieved by trying to read just the first byte from System.in,
// since reading from System.in blocks until it detects that an enter has been pressed.
// Wrap System.in with a PushbackInputStream because this byte will be unread below.
PushbackInputStream pin = new PushbackInputStream(System.in);
int b = pin.read();
// When current thread gets here, the block on reading System.in is over (e.g. the user pressed enter, or some error occurred?)
// signal threadMasking to stop and wait till it is dead:
masker.stop();
threadMasking.join();
// check for stream errors:
if (b == -1) thrownew IOException("end-of-file was detected in System.in without any data being read");
if (System.out.checkError()) thrownew IOException("an I/O problem was detected in System.out");
// pushback the first byte and supply the now unaltered stream to readLineSecure which will return the complete password:
pin.unread(b);
return readLineSecure(pin); // this is the same method earlier in this post
}
/**
* Masks an InputStream by overwriting blank chars to the PrintStream corresponding to its output.
* A typical application is for password input masking.
* <p>
* @see <a href="http://java.sun.com/features/2002/09/pword_mask.html">Password masking in console</a>
*/
publicstaticclass StreamMasker implements Runnable {
privatestaticfinal String TEN_BLANKS = " ";
privatefinal PrintStream out;
privatefinal String promptOverwrite;
privatevolatileboolean doMasking; // MUST be volatile to ensure update by one thread is instantly visible to other threads
/**
* Constructor.
* <p>
* @throws IllegalArgumentException if out == null; prompt == null; prompt contains the char '\r' or '\n'
*/
public StreamMasker(PrintStream out, String prompt) throws IllegalArgumentException {
if (out == null) thrownew IllegalArgumentException("out == null");
if (prompt == null) thrownew IllegalArgumentException("prompt == null");
if (prompt.indexOf('\r') != -1) thrownew IllegalArgumentException("prompt contains the char '\\r'");
if (prompt.indexOf('\n') != -1) thrownew IllegalArgumentException("prompt contains the char '\\n'");
this.out = out;
this.promptOverwrite =
"\r" + // sets cursor back to beginning of line:
prompt + // writes prompt (critical: this reduces visual flicker in the prompt text that otherwise occurs if simply write blanks here)
TEN_BLANKS + // writes 10 blanks beyond the prompt to mask out any input; go 10, not 1, spaces beyond end of prompt to handle the (hopefully rare) case that input occurred at a rapid rate
"\r" + // sets cursor back to beginning of line:
prompt; // writes prompt again; the cursor will now be positioned immediately after prompt (critical: overwriting only works if all input text starts here)
}
/**
* Repeatedly overwrites the current line of out with prompt followed by blanks.
* This effectively masks any chars coming on out, as long as the masking occurs fast enough.
* <p>
* To help ensure that masking occurs when system is in heavy use, the calling thread will have its priority
* boosted to the max for the duration of the call (with its original priority restored upon return).
* Interrupting the calling thread will eventually result in an exit from this method,
* and the interrupted status of the calling thread will be set to true.
* <p>
* @throws RuntimeException if an error in the masking process is detected
*/
publicvoid run() throws RuntimeException {
int priorityOriginal = Thread.currentThread().getPriority();
Thread.currentThread().setPriority(Thread.MAX_PRIORITY);
try {
doMasking = true; // do this assignment here and NOT at variable declaration line to allow this instance to be restarted if desired
while (doMasking) {
out.print(promptOverwrite);
// call checkError, which first flushes out, and then lets us confirm that everything was written correctly:
if (out.checkError()) thrownew RuntimeException("an I/O problem was detected in out"); // should be an IOException, but that would break method contract
// limit the masking rate to fairly share the cpu; interruption breaks the loop
try {
Thread.currentThread().sleep(1); // have experimentally found that sometimes see chars for a brief bit unless set this to its min value
}
catch (InterruptedException ie) {
Thread.currentThread().interrupt(); // resets the interrupted status, which is typically lost when an InterruptedException is thrown, as per our method contract; see Lea, "Concurrent Programming in Java Second Edition", p. 171
break;
}
}
}
finally {
Thread.currentThread().setPriority(priorityOriginal);
}
}
/** Signals any thread executing run to stop masking and exit run. */
publicvoid stop() { doMasking = false; }
}
I have extensively tested the above code. On my (win xp pro sp1) box, it mostly works. But a few times I saw glitches. For instance, with my current program, the console should look like this
Enter password:
Crypto start: 2004/02/09 10:58:08:484
Crypto end: 2004/02/09 10:58:08:484
but there are a few times that I have seen it look like this:
Enter password:
Enter password: Crypto start: 2004/02/09 10:57:56:218
Crypto end: 2004/02/09 10:57:56:218
Assuming that it takes the masking thread on the order of 1-100 microseconds to execute the print, the masking thread actually spends the bulk of its life sleeping. This is fine, since if the user presses enter during this time and the block in the main thread gets lifted, then the masking thread will detect the stop flag when it awakes.
But what apparently happens on occaision is that the timing is such that the user has pressed enter, which causes the console to advance to the next line, but the unbuffering of its input and unblocking of the main thread does not occur soon enough to prevent a final mask, which now occurs on the second line.
This thread communication/timing bug is bound to occur with the original code too, so I conclude that the pure java console masking hack that has been proposed and much mentioned in the forums is actually unreliable; it should be regarded as broken and useless.
In fact, to be sure that the bug is found in the original code, I added the following main to it
publicstaticvoid main(String[] args) throws Exception {
PasswordField field = new PasswordField();
System.out.println( "the password just entered is " + field.getPassword("Enter password: ") );
}
and sure enough, after executing it several times, finally got a glitch line like this:
Enter password:
Enter password: the password just entered is sdvsdvsd
Am I the first one in these forums to point out that this technique is unreliable? I am surprised that no one else seems to have, given how often it is mentioned.
Another cosmetic problem with it is that there is annoying flicker in the prompt text.
If there was sufficient cursor control available with System.out, as in merely supporting ANSI control sequences, then I think that the following code would work great (would be immune from both the above thread timing bug as well as flicker in prompt):
/**
* Masks an InputStream by overwriting blank chars to the PrintStream corresponding to its output.
* A typical application is for password input.
* <p>
* @see <a href="http://java.sun.com/features/2002/09/pword_mask.html">Password masking in console</a>
*/
publicstaticclass StreamMasker implements Runnable {
// For more on ansi codes:
// http://www.computerhope.com/ansisys.htm#00
// http://www.evergreen.edu/biophysics/technotes/program/ansi_esc.htm
// http://forum.java.sun.com/thread.jsp?forum=31&thread=429395
// Ansi code support on dos/windoze systems:
// requires having the ansi driver called ansi.sys installed
// for instructions see section titled "Enabling ANSI.SYS" of http://www.evergreen.edu/biophysics/technotes/program/ansi_esc.htm
// Java curses packages:
// http://www.nongnu.org/jcurzez/
privatestaticfinal String CURSOR_SAVE = "\033[s";
privatestaticfinal String CURSOR_RESTORE = "\033[u";
privatestaticfinal String ERASE_TO_LINE_END = "\033[K";
privatefinal PrintStream out;
privatefinal String prompt;
privatevolatileboolean doMasking; // MUST be volatile to ensure update by one thread is instantly visible to other threads
/** Constructor.
* <p>
* @throws IllegalArgumentException if out == null; prompt == null; prompt contains the char '\r' or '\n'
*/
public StreamMasker(PrintStream out, String prompt) throws IllegalArgumentException {
if (out == null) thrownew IllegalArgumentException("out == null");
if (prompt == null) thrownew IllegalArgumentException("prompt == null");
if (prompt.indexOf('\r') != -1) thrownew IllegalArgumentException("prompt contains the char '\\r'");
if (prompt.indexOf('\n') != -1) thrownew IllegalArgumentException("prompt contains the char '\\n'");
this.out = out;
this.prompt = prompt;
}
/** Constantly overwrites out with prompt.
* This effectively masks any chars coming on out, as long as the masking occurs fast enough.
* <p>
* To help ensure that masking occurs when system is in heavy use, the calling thread will have its priority
* boosted to the max for the duration of the call (but its original priority will be restored upon return).
* Interrupting the calling thread will eventually result in an exit from this method,
* and the interrupted status of the calling thread will be set to true.
* <p>
* @throws RuntimeException if an error in the masking process is detected
*/
publicvoid run() throws RuntimeException {
int priorityOriginal = Thread.currentThread().getPriority();
Thread.currentThread().setPriority(Thread.MAX_PRIORITY);
try {
out.print(prompt);
out.print(CURSOR_SAVE); // save cursor position just after prompt
doMasking = true; // do this assignment here and NOT at variable declaration line to allow this instance to be restarted if desired
while (doMasking) {
out.print(CURSOR_RESTORE); // restore cursor position to just after prompt
out.print(ERASE_TO_LINE_END); // erase anything else on line
// call checkError, which first flushes out, and then lets us confirm that everything was written correctly:
if (out.checkError()) thrownew RuntimeException("an I/O problem was detected in out"); // should be an IOException, but that would break method contract
// limit the masking rate to fairly share the cpu; interruption breaks the loop
try {
Thread.currentThread().sleep(1); // have experimentally found that sometimes see chars for a brief bit unless set this to its min value
}
catch (InterruptedException ie) {
Thread.currentThread().interrupt(); // resets the interrupted status, which is typically lost when an InterruptedException is thrown, as per our method contract; see Lea, "Concurrent Programming in Java Second Edition", p. 171
break;
}
}
// once have exited loop, do one last mask followed by a newline to guarantee that the cursor is left in a consistent position:
out.print(CURSOR_RESTORE); // restore cursor position to just after prompt
out.print(ERASE_TO_LINE_END); // erase anything else on line
out.println();
}
finally {
Thread.currentThread().setPriority(priorityOriginal);
}
}
/** Signals any thread executing run to stop masking and exit run. */
publicvoid stop() { doMasking = false; }
}
Unfortunately, The Beast appears to have totally dropped support for ansi controls for some reason from all versions of their OS from NT onwards, so I have not tested the above. (Yes, I know that you can hack it in so that command.com uses it, but command.com is horrible and I need a way to get ansi stuff to work in cmd.exe, which is the shell that is used by the wonderful Command Prompt Here utility.) If anyone with a decent unix terminal would test the code above, I would be interested to know if it works.
Any chance that we could all agree to jointly lobby Sun to finally offer a decent console? It is pathetic that they never have...
Given that secure console input using pure java is apparently impossible, the only remaining choice is to use a gui. I offer the following code which uses a JPasswordField:
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
/**
* Subclass of JDialog which is used to read in some sensitive piece of information.
* <p>
* As with all Java GUI (i.e. AWT, Swing) code, this class expects to only operate in
* a single threaded environment, and so is not multithread safe.
*/
publicclass DialogSecure extends JDialog {
privatefinal String prompt;
privatefinalint columns;
privatefinal JPasswordField secureField = new JPasswordField();
// -------------------- getInputSecure --------------------
/**
* Reads and returns some sensitive piece of information (e.g. a password) from a DialogSecure instance.
*/
publicstaticfinalchar[] getInputSecure(Frame parent, String title, boolean modal, String prompt, int columns) {
returnnew DialogSecure(parent, title, modal, prompt, columns).getInput();
}
// -------------------- Constructor --------------------
/**
* Constructs a new DialogSecure instance.
* <p>
* @param parent the parent Frame that this JDialog will be attached to; may be null
* @param title the dialog's title
* @param modal specifies whether or not this JDialog is a modal dialog
* @param prompt text displayed next to the secure text entry field
*/
public DialogSecure(Frame parent, String title, boolean modal, String prompt, int columns) {
super(parent, title, modal);
this.prompt = prompt;
this.columns = columns;
setDefaultCloseOperation( DISPOSE_ON_CLOSE );
setLocationRelativeTo(parent);
getContentPane().add( buildGui() );
pack();
show();
}
// -------------------- accessors and mutators --------------------
publicchar[] getInput() { return secureField.getPassword(); }
// -------------------- build methods --------------------
protected JComponent buildGui() {
JPanel jpanel = new JPanel( LineLayout.makeVertical() );
jpanel.setBorder( BorderFactory.createEmptyBorder(20, 20, 20, 20) );
jpanel.add( buildSecureEntry() );
jpanel.add( Box.createVerticalStrut(20) );
jpanel.add( buildWarning() );
jpanel.add( Box.createVerticalStrut(20) );
jpanel.add( buildOKAndCancelButtons() );
return jpanel;
}
protected JComponent buildSecureEntry() {
secureField.setColumns(columns);
secureField.setEchoChar('\r'); // using carriage return as the echo char means that the cursor is always repositioned at the start, which means that the password length is never revealed for highest security (note: also tried \b, but that does not work for some reason...)
JPanel jpanel = new JPanel( LineLayout.makeHorizontal() );
jpanel.add( new JLabel(prompt) );
jpanel.add( Box.createHorizontalStrut(10) );
jpanel.add( secureField );
return jpanel;
}
protected JComponent buildWarning() {
JPanel jpanel = new JPanel( LineLayout.makeHorizontal() );
jpanel.add( new JLabel("<html><i>Note: for top security, no text is echoed when type in field above</i></html>") );
return jpanel;
}
protected JComponent buildOKAndCancelButtons() {
JPanel jpanel = new JPanel( LineLayout.makeHorizontal() );
JButton okButton = new JButton("OK");
okButton.addActionListener( new ActionListener() {
publicvoid actionPerformed(ActionEvent ae) {
DialogSecure.this.dispose();
}
} );
this.getRootPane().setDefaultButton(okButton);
jpanel.add( okButton );
jpanel.add( Box.createHorizontalStrut(20) );
JButton cancelButton = new JButton("Cancel");
cancelButton.addActionListener( new ActionListener() {
publicvoid actionPerformed(ActionEvent ae) {
secureField.setText(""); // delete everything that may have been typed before disposing
DialogSecure.this.dispose();
}
} );
jpanel.add( cancelButton );
return jpanel;
}
}
This code seems to work great. The only issue that I am concerned about is that for top security, you are supposed to blank out all passwords after use. While JPasswordField's getPassword method does return a char[] that can be blanked out, what is not clear to me is how it stores data internally. There are all kinds of complex underlying document models etc in Swing components like JPasswordField, and I am not sure whether they have fully thought security thru there. For instance, JPasswordField's setText method, which is non-deprecated, still takes a String. Also, they offer no method which says that it will securley blank out all chars held in the model. If anyone knows more here, please post.
It turns out that you CAN handle the timing bug in the console masking thread technique by simply always doing a final rewrite of the current line with spaces in the masking thread, since you know that doMasking will only be set to false once the user has hit enter and so the console has advanced to the next line. Thus, if you are overwriting anything with these spaces, it will just be that spurious prompt.
Rewrite the run method of the StreamMasker class as:
publicvoid run() throws RuntimeException {
int priorityOriginal = Thread.currentThread().getPriority();
Thread.currentThread().setPriority(Thread.MAX_PRIORITY);
try {
doMasking = true; // do this assignment here and NOT at variable declaration line to allow this instance to be restarted if desired
while (doMasking) {
out.print(promptOverwrite);
// call checkError, which first flushes out, and then lets us confirm that everything was written correctly:
if (out.checkError()) thrownew RuntimeException("an I/O problem was detected in out"); // should be an IOException, but that would break method contract
// limit the masking rate to fairly share the cpu; interruption breaks the loop
try {
Thread.currentThread().sleep(1); // have experimentally found that sometimes see chars for a brief bit unless set this to its min value
}
catch (InterruptedException ie) {
Thread.currentThread().interrupt(); // resets the interrupted status, which is typically lost when an InterruptedException is thrown, as per our method contract; see Lea, "Concurrent Programming in Java Second Edition", p. 171
return; // return, NOT break, since now want to skip the lines below where write bunch of blanks since typically the user will not have pressed enter yet
}
}
// erase any prompt that may have been spuriously written on the NEXT line after the user pressed enter
out.print('\r');
for (int i = 0; i < promptOverwrite.length(); i++) out.print(' ');
out.print('\r');
}
finally {
Thread.currentThread().setPriority(priorityOriginal);
}
}
I have extensively tested this and it appears to work OK. (Well, you are still left with some visual flicker in the prompt text; also, the first letter of the password that you are typing also sometimes momentarily flashes on the screen before being blanked out.)
But sun should still offer decent console support so that this stuff just works...
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Feb 12, 2004 1:50 PM
(reply 2
of 13) (In reply to
#1 )
Yet another correction. Since I harshed on Sun and others for making the assumption that \r leaves you on the same line (it is actually OS dependent; on Macs, for instance, it acts exactly like a \n on unix), I should not have used that char for masking the console line. The backspace char \b is hopefully a bit more OS independent. Thus, the first part of StreamMasker class should probably look like:
publicstaticclass StreamMasker implements Runnable {
privatestaticfinal String TEN_BLANKS = StringUtil.repeatChars(' ', 10);
privatefinal PrintStream out;
privatefinal String promptOverwrite;
privatevolatileboolean doMasking; // MUST be volatile to ensure update by one thread is instantly visible to other threads
/**
* Constructor.
* <p>
* @throws IllegalArgumentException if out == null; prompt == null; prompt contains the char '\r' or '\n'
*/
public StreamMasker(PrintStream out, String prompt) throws IllegalArgumentException {
if (out == null) thrownew IllegalArgumentException("out == null");
if (prompt == null) thrownew IllegalArgumentException("prompt == null");
if (prompt.indexOf('\r') != -1) thrownew IllegalArgumentException("prompt contains the char '\\r'");
if (prompt.indexOf('\n') != -1) thrownew IllegalArgumentException("prompt contains the char '\\n'");
this.out = out;
String setCursorToStart = StringUtil.repeatChars('\b', prompt.length() + TEN_BLANKS.length());
this.promptOverwrite =
setCursorToStart + // sets cursor back to beginning of line:
prompt + // writes prompt (critical: this reduces visual flicker in the prompt text that otherwise occurs if simply write blanks here)
TEN_BLANKS + // writes 10 blanks beyond the prompt to mask out any input; go 10, not 1, spaces beyond end of prompt to handle the (hopefully rare) case that input occurred at a rapid rate
setCursorToStart + // sets cursor back to beginning of line:
prompt; // writes prompt again; the cursor will now be positioned immediately after prompt (critical: overwriting only works if all input text starts here)
}
where the repeatChars method has the obvious implementation:
/**
* Returns a String of the specified length which consists of entirely of the char c.
* <p>
* @throws IllegalArgumentException if length < 0
*/
publicstaticfinal String repeatChars(char c, int length) throws IllegalArgumentException {
if (length < 0) thrownew IllegalArgumentException("length = " + length + " is < 0");
StringBuffer sb = new StringBuffer(length);
for (int i = 0; i < length; i++) {
sb.append(c);
}
return sb.toString();
}
Re: The DEFINITIVE thread on secure password entry (console, gui, code reviews)
Feb 2, 2005 3:14 PM
(reply 3
of 13) (In reply to
original post )
Since buffer in readLineSecure() is allocated through new, is there a security
vulnerability if the heap compactor runs and moves it? Same question for
bufferNew in checkBuffer()?
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Feb 12, 2005 7:31 AM
(reply 4
of 13) (In reply to
#3 )
Wow, only a year after I published all this code, and finally someone responds--thanks Baboo!
Since buffer in readLineSecure() is allocated through new, is there a security
vulnerability if the heap compactor runs and moves it? Same question for
bufferNew in checkBuffer()?
Thats a great question. If I understand your concern, you are worried that if objects are moved around in memory by the JVM, that the formerly used memory cells still retain their old data until they get overwritten with new data, which would allow some malicious code in the meanwhile to read those old memory cells and get the data that way?
I do not know enough about the low level workings of the JVM to answer that question. Maybe the JVM explicitly overwrites all previously used memory with zeroes, or maybe it ensures that no untrusted code can ever examine its memory. If anyone does know more details, I would love to see a posting on this.
But, as far as my java code is concerned, there is NO other way to work with objects other then to instantiate them with new, so if the vulnerability that you describe exists, it exists for all java code.
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Feb 13, 2005 7:43 PM
(reply 5
of 13) (In reply to
#4 )
Wow, only a year after I published all this code, and
finally someone responds--thanks Baboo!
Welcome :-)
Since buffer in readLineSecure() is allocated
through new, is there a security
vulnerability if the heap compactor runs and moves
it? Same question for
bufferNew in checkBuffer()?
Thats a great question. If I understand your
concern, you are worried that if objects are moved
around in memory by the JVM, that the formerly used
memory cells still retain their old data until they
get overwritten with new data, which would allow some
malicious code in the meanwhile to read those old
memory cells and get the data that way?
Yes. This is my concern. Further, my understanding is the JVM preallocates heap
memory, and then manages it from then on. If the memory is moved, and the
section of the heap in which the sensitive date resided is in a section of
memory that becomes rarely used, the OS may consider that region as a candidate
for swapping to disk, thus, increasing the exploit window. I realize this may be
an "edge" case.
I do not know enough about the low level workings of
the JVM to answer that question. Maybe the JVM
explicitly overwrites all previously used memory with
zeroes, or maybe it ensures that no untrusted code
can ever examine its memory. If anyone does know
more details, I would love to see a posting on this.
But, as far as my java code is concerned, there is NO
other way to work with objects other then to
instantiate them with new, so if the vulnerability
that you describe exists, it exists for all java code.
Agreed. It seems to me that there may be two ways to decrease the possibility of
sensitive data leaks. One, less desirable way, is to write native code through
JNI for all of the platforms on which one wishes to run to handle securing the
memory. The second would be an additional API in the JDK that handles this. It
would be nice to know for sure that the Swing and any other GUI toolkits handled
this in a secure fashion as well.
btw... thanks for a great post. I've only recently come to Java after 4 years of
C/C++ (with some dabbling in Python and PERL) and needed info on handling
sensitive data in Java. Your post has been quite helpful to me in considering
the issues.
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Feb 15, 2005 9:02 AM
(reply 6
of 13) (In reply to
#5 )
I have actually updated some of the code from my first posting.
I believe that there are some defect in JPasswordField that prevent it from achieving top security; the code below fixes all the issues I believe.
Baboo (or anyone else looking at this), I would appreciate a code review and any feedback that you could give me, since I am thinking of reporting this as a bug or feature request to Sun.
The 3 new/revised top-level classes are:
import java.awt.*;
import java.awt.event.*;
import java.io.*;
import javax.swing.*;
import bb.util.StringUtil;
/**
* Subclass of {@link JDialog JDialog} which lets the user input some highly sensitive piece of text (e.g. a password).
* <p>
* The text is entered in a {@link TextFieldSecure TextFieldSecure} instance inside the dialog.
* This class allows optional header text above the TextFieldSecure (e.g. for instructions, a password hint, etc),
* as well as requires prompt text on the left to label the TextFieldSecure.
* It automatically puts a label on the right which specifies the maximum number of chars that may be entered.
* <p>
* The only public member is the static {@link #getInputSecure getInputSecure} method, which is all that normal users will use.
* For subclassing purposes, the other members of this class have protected access.
* <p>
* Like all java gui code, this class is not multithread safe (it expects to only be called by the AWT Event dispatching thread).
*/
publicclass DialogInputSecure extends JDialog {
// -------------------- constants --------------------
privatestaticfinallong serialVersionUID = 1;
// -------------------- instance fields --------------------
/**
* A {@link TextFieldSecure TextFieldSecure} instance that is used by this class to input highly sensitive text.
* <p>
* @serial
*/
privatefinal TextFieldSecure textFieldSecure;
// -------------------- getInputSecure --------------------
/**
* Reads and returns some sensitive piece of information (e.g. a password) from a new DialogInputSecure instance.
* The result is a char[], not a String, so that it can be zeroed out after use.
* <p>
* See the {@link #DialogInputSecure constructor} for documentation of all the params and throws.
*/
publicstaticfinalchar[] getInputSecure(Frame parent, String title, boolean modal, String header, String prompt, int numberCharsMax) throws IllegalArgumentException {
DialogInputSecure dialog = null;
try {
dialog = new DialogInputSecure(parent, title, modal, header, prompt, numberCharsMax);
return dialog.textFieldSecure.getInput();
}
finally {
if (dialog != null) dialog.textFieldSecure.zeroOutInput();
}
}
// -------------------- constructor --------------------
/**
* Constructs a new DialogInputSecure instance.
* <p>
* @param parent the parent Frame that this JDialog will be attached to; may be null
* @param title the dialog's title; may be null
* @param modal specifies whether or not this JDialog is a modal dialog
* @param header optional text (e.g. instructions, a password hint) displayed above the TextFieldSecure;
* this text will be put into a JLabel, so it may be html formatted; may be null
* @param prompt mandatory text displayed next to the TextFieldSecure;
* this text will be put into a JLabel, so it may be html formatted; must be non-blank
* @param numberCharsMax the maximum number of chars that may be typed in the TextFieldSecure
* as well as the number of columns in the TextFieldSecure (this affects its displayed width); must be > 0
* @throws IllegalArgumentException if prompt is blank; numberCharsMax <= 0
*/
protected DialogInputSecure(Frame parent, String title, boolean modal, String header, String prompt, int numberCharsMax) throws IllegalArgumentException {
super(parent, title, modal);
if (StringUtil.isBlank(prompt)) thrownew IllegalArgumentException("prompt is blank");
if (numberCharsMax <= 0) thrownew IllegalArgumentException("numberCharsMax = " + numberCharsMax + " <= 0");
textFieldSecure = new TextFieldSecure(numberCharsMax);
setDefaultCloseOperation( DISPOSE_ON_CLOSE );
getContentPane().add( buildGui(header, prompt, numberCharsMax) );
pack();
setLocationRelativeTo(parent); // MUST do this call after have properly sized the dialog with pack
setVisible(true);
}
// -------------------- build methods --------------------
protected JComponent buildGui(String header, String prompt, int numberCharsMax) {
JPanel jpanel = new JPanel( LineLayout.makeVertical() );
jpanel.setBorder( BorderFactory.createEmptyBorder(20, 20, 20, 20) );
if (header != null) {
jpanel.add( new JLabel(header) );
jpanel.add( Box.createVerticalStrut(20) );
}
jpanel.add( buildSecureEntry(prompt, numberCharsMax) );
jpanel.add( Box.createVerticalStrut(20) );
jpanel.add( buildButtons() );
return jpanel;
}
protected JComponent buildSecureEntry(String prompt, int numberCharsMax) {
JPanel jpanel = new JPanel( LineLayout.makeHorizontal() );
jpanel.add( new JLabel(prompt) );
jpanel.add( Box.createHorizontalStrut(10) );
jpanel.add( textFieldSecure );
jpanel.add( Box.createHorizontalStrut(10) );
jpanel.add( new JLabel("(" + numberCharsMax + " chars max)") );
return jpanel;
}
protected JComponent buildButtons() {
JPanel jpanel = new JPanel( LineLayout.makeHorizontal() );
JButton helpButton = new JButton("Help");
helpButton.addActionListener( new ActionListener() {
publicvoid actionPerformed(ActionEvent ae) {
String helpText =
"This dialog allows you to enter text (e.g. a password) in a very secure manner." + "\n" +
"\n" +
"For top security, when you type in this dialog's text field, no visible characters are echoed." + "\n" +
"Unlike some other interfaces, which at least echo some generic character like an asterisk ('*')," + "\n" +
"this dialog echoes nothing, since that would give away the length of the text." + "\n" +
"Furthermore, there is no caret (vertical line indicating text position) in the text field either," + "\n" +
"since that too might give away the length of the text." + "\n" +
"\n" +
"Because of the above, it is important to give the user some visual cue when the text field has keyboard focus." + "\n" +
"(Keyboard focus means that if you type on the keyboard, the text is entered in the component" + "\n" +
" which has the focus, as opposed to some other text component which might be present in the interface.)" + "\n" +
"This dialog indicates when its text field has keyboard focus by setting the field's background color to white," + "\n" +
"and it indicates when it lacks keyboard focus by setting its background color to light red." + "\n" +
"\n" +
"If you type in the text field and then click on the OK button" + "\n" +
"(or type the enter/return key on some platforms), then this dialog disappears" + "\n" +
"but the text that you typed in (except for any possible enter/return key)" + "\n" +
"is retained (and should be read by some other part of the program)." + "\n" +
"\n" +
"Else, if you type in the text field and then click on the Cancel button," + "\n" +
"then this dialog disappears and the text that you typed in is lost.";
JOptionPane.showMessageDialog(DialogInputSecure.this, helpText, "DialogInputSecure Help", JOptionPane.INFORMATION_MESSAGE);
}
} );
jpanel.add( helpButton );
jpanel.add( Box.createHorizontalStrut(20) );
JButton okButton = new JButton("OK");
okButton.addActionListener( new ActionListener() {
publicvoid actionPerformed(ActionEvent ae) {
dispose();
}
} );
this.getRootPane().setDefaultButton(okButton);
jpanel.add( okButton );
jpanel.add( Box.createHorizontalStrut(20) );
JButton cancelButton = new JButton("Cancel");
cancelButton.addActionListener( new ActionListener() {
publicvoid actionPerformed(ActionEvent ae) {
textFieldSecure.zeroOutInput();
textFieldSecure.setText(""); // need to call this as well, since the above does not affect the position indices or anything
dispose();
}
} );
jpanel.add( cancelButton );
return jpanel;
}
// -------------------- Test (inner class) --------------------
/**
* An inner class that consists solely of test code for the parent class.
* <p>
* Putting all the test code in this inner class (rather than a <code>main</code> method of the parent class) has the following benefits:
* <ol>
* <li>test code is cleanly separated from working code</li>
* <li>any <code>main</code> method in the parent class is now reserved for a true program entry point</li>
* <li>test code may be easily excluded from the shipping product by removing all the Test class files (e.g. on Windoze, delete all files that end with <code>$Test.class</code>)</li>
* </ol>
* Putting all the test code in this inner class (rather than a shadow external class) has the following benefits:
* <ol>
* <li>non-public members may be accessed</li>
* <li>the test code lives very close to (so is easy to find) yet is distinct from the working code</li>
* <li>there is no need to set up a test package structure</li>
* </ol>
*/
publicstaticclass Test {
publicstaticvoid main(String[] args) throws Exception {
test_getInputSecure();
// test_serialization();
// +++ the method above fails with the message
// Exception in thread "main" java.io.NotSerializableException: javax.swing.plaf.basic.BasicTextUI$UpdateHandler
// This appears to be a known bug that sun will be fixing in 1.5_02 which is not out yet; see
// http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6190713
}
privatestaticvoid test_getInputSecure() {
String instructions =
"<html>" +
"<p>" +
"Type <i>anything</i> that you wish in the text field below." + "<br>" +
"When you hit the OK button, this dialog will disappear" + "<br>" +
"and the text that you typed will be printed to System.out." +
"</p>" +
"</html>";
char[] chars = DialogInputSecure.getInputSecure(null, "Test Of DialogInputSecure.getInputSecure", true, instructions, "Text input:", 5);
String inputText = new String( chars );
System.out.println("The text that was input to the DialogInputSecure is:");
System.out.println(inputText);
}
// +++ what am hoping to confirm in this method is that the deserialized dialog contains no text initially
// (i.e. that the serialization features promised in TextFieldSecure/ContentSecure work as described)
// and that the deserialized dialog otherwise works as before (that it has an invisible caret etc)
privatestaticvoid test_serialization() throws Exception {
DialogInputSecure dialog = new DialogInputSecure(null, "Test Of DialogInputSecure serialization", true, null, "Text input:", 5);
String text = new String( dialog.textFieldSecure.getPassword() );
System.out.println("text entered in the unserialized dialog: " + text);
dialog.dispose();
ByteArrayOutputStream baos = new ByteArrayOutputStream();
ObjectOutputStream oos = new ObjectOutputStream( baos );
oos.writeObject( dialog );
ObjectInputStream ois = new ObjectInputStream( new ByteArrayInputStream( baos.toByteArray() ) );
DialogInputSecure dialogDeserialized = (DialogInputSecure) ois.readObject();
dialogDeserialized.setTitle("This is the deserialized dialog");
dialogDeserialized.setVisible(true);
String textDeserialized = new String( dialogDeserialized.textFieldSecure.getPassword() );
System.out.println("text present inside the deserialized dialog: " + textDeserialized);
dialogDeserialized.dispose();
}
}
}
import java.awt.*;
import java.awt.event.*;
import java.io.*;
import javax.swing.*;
import javax.swing.text.*;
/**
* Subclass of {@link JPasswordField JPasswordField} which fixes defects in its superclass to offer superior security.
* <p>
* For top security, it is critical that:
* <ol>
* <li>
* the text entered in the text field (or even its length) never gets revealed.
* This class achieves this by setting the echo char to an ordinary whitespace char
* and the caret to a {@link CaretSecure CaretSecure} instance.
* </li>
* <li>
* a char[] be used as the underlying text storage medium (as opposed to, say, a String)
* and that the char[] be zeroed out when use is over.
* This class uses the {@link DocumentSecure DocumentSecure} and {@link ContentSecure ContentSecure} classes to achieve this.
* </li>
* <li>
* serialization never writes out the sensitive text, where it would be exposed to the world.
* This class uses the {@link ContentSecure ContentSecure} classes to prevent this.
* </li>
* </ol>
* <p>
* Like all java gui code, this class is not multithread safe (it expects to only be called by the AWT Event dispatching thread).
*/
publicclass TextFieldSecure extends JPasswordField {
// -------------------- constants --------------------
privatestaticfinallong serialVersionUID = 1;
// -------------------- constructor --------------------
/**
* Constructs a new TextFieldSecure instance.
* <p>
* @param numberCharsMax the maximum number of chars that may be typed in this instance
* as well as its number of columns (this affects its displayed width); must be > 0
* @throws IllegalArgumentException if numberCharsMax <= 0
*/
public TextFieldSecure(int numberCharsMax) throws IllegalArgumentException {
// numberCharsMax checked by the DocumentSecure constructor below
setColumns(numberCharsMax);
setEchoChar(' ');
setCaret( new CaretSecure() );
addFocusListener( CaretSecure.makeFocusListener(this) );
setDocument( new DocumentSecure(numberCharsMax) );
}
// -------------------- accessors and mutators --------------------
/** Returns the Document used by this instance, which is always a {@link DocumentSecure DocumentSecure} instance. */
public DocumentSecure getDocumentSecure() { return (DocumentSecure) getDocument(); }
/** Returns the maximum number of chars that may be typed in this instance. */
publicint getNumberCharsMax() { return getDocumentSecure().getNumberCharsMax(); }
/** Returns a char[] of the chars currently in this instance. Is just a renamed version of {@link #getPassword getPassword}. */
publicchar[] getInput() { return getPassword(); }
/** Calls <code>{@link #getDocumentSecure getDocumentSecure}.{@link DocumentSecure#zeroOutContent zeroOutContent()}</code>. */
publicvoid zeroOutInput() {
getDocumentSecure().zeroOutContent();
}
// -------------------- serialization --------------------
/**
* Customizes deserialization.
* This method manually sets the caret field to a CaretSecure instance, since Sun wrongly declared that field transient.
* (If you look in the source code of JTextComponent, there is the comment "This should be serializable",
* so confirm in a later release of the JVM whether or not you need this method--may be able to eliminate.)
* <p>
* @throws ClassNotFoundException if the class of a serialized object could not be found
* @throws IOException if an I/O problem occurs
* @throws NotActiveException if the stream is not currently reading objects
*/
privatevoid readObject(ObjectInputStream ois) throws ClassNotFoundException, IOException, NotActiveException {
ois.defaultReadObject();
setCaret( new CaretSecure() );
}
// -------------------- DocumentSecure (static inner class) --------------------
/**
* Subclass of {@link DocumentLimitedLength DocumentLimitedLength} which is designed for high security.
* This class always uses a {@link ContentSecure ContentSecure} instance to hold its data, for the reasons described there.
* It also offers a {@link #zeroOutContent zeroOutContent} method which hooks into the corresponding ContentSecure method.
*/
publicstaticclass DocumentSecure extends DocumentLimitedLength {
privatestaticfinallong serialVersionUID = 1;
/** Constructor. */
public DocumentSecure(int numberCharsMax) {
super( new ContentSecure(numberCharsMax), numberCharsMax );
}
public ContentSecure getContentSecure() { return (ContentSecure) getContent(); }
/** Calls <code>{@link #getContentSecure getContentSecure}.{@link ContentSecure#zeroOutContent zeroOutContent()}</code>. */
publicvoid zeroOutContent() { getContentSecure().zeroOutContent(); }
}
// -------------------- ContentSecure (static inner class) --------------------
/**
* Subclass of {@link GapContent GapContent} which is designed for high security.
* This class subclasses GapContent, since GapContent uses a single underlying char[] for its storage
* which {@link #zeroOutContent can be zeroed out} when use is done.
* This class also takes care to {@link #writeObject never write out the sensitive text content} during serialization.
*/
publicstaticclass ContentSecure extends GapContent {
privatestaticfinallong serialVersionUID = 1;
/**
* Multiple calls can be made to the {@link #allocateArray allocateArray} method.
* And this is true even tho intelligence was put into the length initially allocated by the constructor below.
* For instance, the user could attempt to paste a big string of text into the enclosing TextFieldSecure,
* which will cause a large reallocation request.
* In order to be sure that all buffers that were ever allocated all get zeroed out at some point,
* the {@link #zeroOutContent zeroOutContent} method needs to access them all, so this field stores them.
* <p>
* <i>Note: would not need to do this if Sun would simply have made the resize method of GapVector protected
* so that we could override it and zero out each old array just before it is discarded...</i>
* <p>
* <b>Warning:</b> storing all the buffers like this makes this class unsuited for handling large size documents
* because of all the wasted memory.
*/
privatetransient java.util.List<char[]> buffers; // has to be lazy initialized inside allocateArray because of
/** Constructor. */
public ContentSecure(int lengthInitial) {
super( lengthInitial + 3 ); // add 3 to give room for the implied break, the gap, plus 1 extra overtyped char
}
/** Calls {@link #zeroOutContent zeroOutContent}, thus guaranteeing that the content is zeroed out before garbage collection. */
protectedvoid finalize() {
zeroOutContent();
}
/** Returns the superclass result, but before returning it, stores it inside {@link #buffers buffers}. */
protected Object allocateArray(int length) {
char[] arrayNew = (char[]) super.allocateArray(length);
if (buffers == null) buffers = new java.util.ArrayList<char[]>();
buffers.add(arrayNew);
return arrayNew;
}
/**
* Writes zeroes to the underlying char[] which holds this instance's text contents.
* (Actually, this method zeroes out every buffer that was ever created by the {@link #allocateArray allocateArray} method,
* not just the current one.)
*/
publicvoid zeroOutContent() {
for (char[] buffer: buffers) {
for (int i = 0; i < buffer.length; i++) {
buffer[i] = '0';
}
}
}
/**
* The default serialization behavior would write out the complete current state of this instance,
* <i>including the highly sensitive underlying char[] that stores this instance's text content</i>.
* This constitutes a major security breach.
* <p>
* To prevent this catastrophy, this method ensures that only a zeroed out char[] is ever written.
* The underlying char[] is first copied to a local variable, then zeroed out,
* then serialization is performed, and then it is restored from the local copy before method return.
* <p>
* So, users of this class need to beware that the serialized object will lose all the text state.
*/
privatevoid writeObject(ObjectOutputStream oos) throws IOException {
// copy the underlying char[] to a local variable and zero out the underlying char[] as well:
char[] chars = (char[]) getArray();
char[] charsTemp = newchar[ chars.length ];
for (int i = 0; i < chars.length; i++) {
charsTemp[i] = chars[i];
chars[i] = '0';
}
// do normal serialization (which will write the zeroed out array):
oos.defaultWriteObject();
// restore the underlying char[] to a local variable and zero out the local variable as well:
for (int i = 0; i < chars.length; i++) {
chars[i] = charsTemp[i];
charsTemp[i] = '0';
}
}
}
// -------------------- CaretSecure (static inner class) --------------------
/**
* Subclass of {@link DefaultCaret DefaultCaret} which is designed for high security.
* This class has a {@link #paint permanently invisible caret}, so it never indicates the length of text being entered.
*/
publicstaticclass CaretSecure extends DefaultCaret {
privatestaticfinallong serialVersionUID = 1;
/** Constructor. */
public CaretSecure() {
//setUpdatePolicy( DefaultCaret.NEVER_UPDATE ); // do NOT do this: it will cause all the text that is typed to be entered in reverse order (since the caret would then always be at the beginning of the JPasswordField, that is where new chars are inserted)
}
/** Draws nothing, effectively making the caret invisible. */
publicvoid paint(Graphics g) {}
/**
* Because this instance's caret is invisible, it is important to give the user some visual cue
* when the JTextComponent it is part of has keyboard focus.
* This method creates a FocusListener which
* indicates when textComponent has keyboard focus by setting its background color to white,
* and setting its background color to light red when it lacks keyboard focus.
*/
publicstatic FocusListener makeFocusListener(final JTextComponent textComponent) {
returnnew FocusListener() {
publicvoid focusGained(FocusEvent fe) { textComponent.setBackground( Color.WHITE ); }
publicvoid focusLost(FocusEvent fe) { textComponent.setBackground( new Color(255, 200, 200) ); } // is a light red
};
}
}
// -------------------- Test (inner class) --------------------
// None: is tested by DialogInputSecure
}
import javax.swing.UIManager;
import javax.swing.text.*;
import bb.util.ThrowableUtil;
/**
* Subclass of {@link PlainDocument PlainDocument} which limits the number of chars that it will contain.
* <p>
* One use of this class is to serve as the model of a JTextComponent like JTextField
* when the text needs to be constrained to a maximum length.
* <p>
* Like all java gui code, this class is not multithread safe (it expects to only be called by the AWT Event dispatching thread).
* <p>
* @see <a href="http://forum.java.sun.com/thread.jspa?forumID=57&threadID=404834">Java forum posting #1</a>
* @see <a href="http://forum.java.sun.com/thread.jspa?forumID=57&threadID=340706">Java forum posting #2</a>
*/
publicclass DocumentLimitedLength extends PlainDocument {
// -------------------- constants --------------------
privatestaticfinallong serialVersionUID = 1;
// -------------------- fields --------------------
/**
* Records the maximum number of chars that this instance will hold.
* <p>
* Contract: is > 0.
* <p>
* @serial
*/
privatefinalint numberCharsMax;
// -------------------- constructor --------------------
/**
* Constructs a new DocumentLimitedLength instance.
* <p>
* @param numberCharsMax specifies the maximum number of chars that this instance will hold; must be > 0
* @throws IllegalArgumentException if numberCharsMax <= 0
*/
public DocumentLimitedLength(int numberCharsMax) throws IllegalArgumentException {
this( new GapContent(), numberCharsMax );
}
/**
* Constructs a new DocumentLimitedLength instance.
* <p>
* @param content a {@link javax.swing.AbstractDocument.Content} instance that contains initial text content
* @param numberCharsMax specifies the maximum number of chars that this instance will hold; must be > 0
* @throws IllegalArgumentException if content == null; numberCharsMax <= 0; content.length() > numberCharsMax
*/
public DocumentLimitedLength(AbstractDocument.Content content, int numberCharsMax) throws IllegalArgumentException {
super(content);
if (content == null) thrownew IllegalArgumentException("content == null");
if (numberCharsMax <= 0) thrownew IllegalArgumentException("numberCharsMax = " + numberCharsMax + " <= 0");
if (content.length() > numberCharsMax) thrownew IllegalArgumentException("content.length() = " + content.length() + " > numberCharsMax = " + numberCharsMax);
this.numberCharsMax = numberCharsMax;
}
// -------------------- accessors and mutators --------------------
/** Returns the maximum number of chars that may be typed in this instance. */
publicint getNumberCharsMax() { return numberCharsMax; }
/** Returns all the text contained in this instance. */
public String getText() {
try {
return getText(0, getLength());
}
catch (Exception e) {
throw ThrowableUtil.toRuntimeException(e); // this line should never get called, but need the catch block to suppress the compiler from forcing a throws BadLocationException declaration
}
}
// +++ why does Document lack this method? must have been an oversight by Sun...
// -------------------- Document methods --------------------
/**
* First calls <code>super.insertString(offset, s, attributeSet)</code>.
* Then checks to see if the new length exceeds {@link #numberCharsMax numberCharsMax}, and if it does,
* sounds a beep and then truncates the final chars so that the length ends up equaling numberCharsMax.
*/
publicvoid insertString(int offset, String s, AttributeSet attributeSet) throws BadLocationException {
// all args checked by the call to super.insertString below
super.insertString(offset, s, attributeSet);
if (getLength() > numberCharsMax) {
UIManager.getLookAndFeel().provideErrorFeedback(null);
remove(numberCharsMax, getLength() - numberCharsMax);
}
}
// -------------------- Test (inner class) --------------------
/**
* An inner class that consists solely of test code for the parent class.
* <p>
* Putting all the test code in this inner class (rather than a <code>main</code> method of the parent class) has the following benefits:
* <ol>
* <li>test code is cleanly separated from working code</li>
* <li>any <code>main</code> method in the parent class is now reserved for a true program entry point</li>
* <li>test code may be easily excluded from the shipping product by removing all the Test class files (e.g. on Windoze, delete all files that end with <code>$Test.class</code>)</li>
* </ol>
* Putting all the test code in this inner class (rather than a shadow external class) has the following benefits:
* <ol>
* <li>non-public members may be accessed</li>
* <li>the test code lives very close to (so is easy to find) yet is distinct from the working code</li>
* <li>there is no need to set up a test package structure</li>
* </ol>
*/
publicstaticclass Test {
publicstaticvoid main(String[] args) throws Exception {
DocumentLimitedLength doc = new DocumentLimitedLength(10);
doc.insertString(0, "abcdefghij", null); // add first 10 chars of the alphabet
System.out.println("doc after step #1: " + doc.getText());
doc.insertString(5, "123", null); // add first 3 natural numbers starting at index 5
System.out.println("doc after step #2: " + doc.getText());
System.out.println("did insertString work as expected: " + doc.getText().equals("abcde123fg") );
}
}
}
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Feb 18, 2005 9:59 AM
(reply 7
of 13) (In reply to
#6 )
I will be happy to review. FYI though, I'm currently heavy into a dev cycle and may be a little slow in getting to it. I want to give it a thorough review, and can't commit the time at the moment.
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Oct 2, 2005 10:17 AM
(reply 8
of 13) (In reply to
#7 )
Hi guys !
I would like to ask a question, perhaps stupid but I try.
I don't understand all that you write on your tutorial, who's the only i found after several researches.
I just want to know how i can do not to write in java's console what the user's writting, like password, but without any security, just to learn how to do in a first step !! After i'll try to understand the second stage with security but now, it's not the case.
So, can you help me on what can i do not to write in java's console what the user's writting.
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Jun 6, 2006 4:14 PM
(reply 9
of 13) (In reply to
#8 )
So, can you help me on what can i do not to write in java's console what the
user's writting.
This doesn't help much for previous releases, but JDK 6 introduces the java.io.Console class, plus readPassword(), which turns off echoing during Console reads. You can get weekly snapshots at http://mustand.dev.java.net if you're not already doing so.
Bug in StreamMasker.run: huge race condition which results in infinite loop when password is piped to the program (ie. already available at stdin when run starts). The infinite loop happens when stopMasking is called before run initializes doMasking. This happens always/often when a script feeds a password to the program, rather than a user interactively entering the password.
1) Simple fix: move "doMasking=true" from run() to the constructor.
Other small (optional) tweaks I made to somewhat improve behavior of this class:
2) Do "out.print(promptOverwrite)" in the constructor so that a prompt will always occur when a script feeds a password to the program.
3) Count the number of promptOverwrite written (including in constructor), and in method run after the while loop
(but still inside the "try") do only if the count is greater than 1, "out.print(setCursorToStart)".
This completely fixes the problem of an extra blank line (or prompt) appearing after the password has been
entered interactively (an extra prompt may still be written when the password is piped to the program).
This initialization should appear in the constructor:
// '\010' is backspace -- not sure if '\b' works on all platforms
setCursorToStart = StreamMasker.repeatChars('\010', prompt.length() TEN_BLANKS.length());
After implementing bbatman's most excellent algorithm (with tweaks 2 and 3) and after the code was in production use for 1 year, someone discovered the trivial fix to the showstopper bug of the infinite loop, so I cannot take credit for finding the solution to this somewhat subtle but gapingly huge race condition (I had put sleep calls in our scripts to workaround it, which was a very poor workaround).
For what it's worth,
sergei kucherov
setCursorToStart = StreamMasker.repeatChars('\010', prompt.length() TEN_BLANKS.length()); // '\010'
Note that I also do this:
// "\r" may be as good or better than setCursorToStart on the next line -- need to experiment
promptOverwrite = setCursorToStart prompt TEN_BLANKS setCursorToStart prompt;
After my implementing bbatman's most excellent algorithm (with added tweaks 2 and 3) and after the code being in production use for 1 year, someone discovered the trivial fix to the showstopper bug of the infinite loop, so I cannot take credit for finding the solution to this somewhat subtle but gapingly huge race condition (I had put sleep calls in our scripts to workaround it, which was very poor engineering).
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Oct 20, 2008 11:41 AM
(reply 12
of 13) (In reply to
#10 )
This obvious solution is considered by Sun to be insecure since the password is stored as a String
(even if just momentarily). However, it does perfectly mask an entered password on a unix system, so it may
be useful for "non-secure" apps that are constrained to use a Java version prior to Java 6
(which has the Console class).
Re: The DEFINITIVE thread on secure password entry (console, gui, code revi
Oct 21, 2008 9:41 PM
(reply 13
of 13) (In reply to
#11 )
Sergei: many thanks for reporting the bug in my code. I never thought about passwords being piped in, but I should have seen that race condition.
I decided that flag variables are bad in this context, and went directly to using interruption, as that is a conventional way to communicate across threads. I am less than thrilled about doing some of your stuff in the constructor, since that means that the StreamMasker cannot be reused more than once, and there is no current check that it is not being reused, and i gets slightly complicated if you want really robust code.
Here is my latest implementation of StreamMasker which I think addresses the concerns:
/**
* Masks an InputStream by overwriting blank chars to the PrintStream corresponding to its output.
* A typical application is for password input masking.
* <p>
* This class is not multithread safe: just a single thread at a time should call {@link #run run}.
* <p>
* @see <a href="http://forums.sun.com/thread.jspa?threadID=490728">My forum posting on password entry</a>
* @see <a href="http://java.sun.com/features/2002/09/pword_mask.html">Password masking in console</a>
*/
privatestaticclass StreamMasker implements Runnable {
privatestaticfinal String TEN_BLANKS = StringUtil.repeatChars(' ', 10);
privatefinal PrintStream out;
privatefinal String setCursorToStart;
privatefinal String promptOverwrite;
/**
* Constructor.
* <p>
* @throws IllegalArgumentException if out == null; prompt == null; prompt contains the char '\r' or '\n'
*/
private StreamMasker(PrintStream out, String prompt) throws IllegalArgumentException {
if (out == null) thrownew IllegalArgumentException("out == null");
if (prompt == null) thrownew IllegalArgumentException("prompt == null");
if (prompt.contains("\n")) thrownew IllegalArgumentException("prompt contains the new line ('\\n') char");
if (prompt.contains("\r")) thrownew IllegalArgumentException("prompt contains the carriage return ('\\r') char");
this.out = out;
//setCursorToStart = StringUtil.repeatChars('\b', prompt.length() + TEN_BLANKS.length()); // use backspaces, as \r has OS dependent meaning
setCursorToStart = "\r"; // decided to go back with this, since is not only shorter, but it works on unix whereas the above may back up into the line above (this is a difference between windows and unix consoles)
promptOverwrite =
setCursorToStart + // sets cursor back to beginning of line:
prompt + // writes prompt (critical: this reduces visual flicker in the prompt text that otherwise occurs if simply write blanks here)
TEN_BLANKS + // writes 10 blanks beyond the prompt to mask out any input; go 10, not 1, spaces beyond end of prompt to handle the (hopefully rare) case that input occurred at a rapid rate
setCursorToStart + // sets cursor back to beginning of line:
prompt; // writes prompt again; the cursor will now be positioned immediately after prompt (critical: overwriting only works if all input text starts here)
out.print(promptOverwrite); // ensure that is written at least once
}
privatevoid printPromptOverwrite() {
out.print(promptOverwrite);
}
/**
* Repeatedly overwrites the current line of out with prompt followed by blanks.
* This effectively masks any chars coming on out, as long as the masking occurs fast enough.
* <p>
* To help ensure that masking occurs when system is in heavy use, the calling thread will have its priority
* boosted to the max for the duration of the call (with its original priority restored upon return).
* Interrupting the calling thread will eventually result in an exit from this method,
* and the interrupted status of the calling thread will be set to true.
* <p>
* @throws RuntimeException (or some subclass) if any error occurs; this may merely wrap some other underlying Throwable
*/
publicvoid run() throws RuntimeException {
int priorityOriginal = Thread.currentThread().getPriority();
try {
Thread.currentThread().setPriority(Thread.MAX_PRIORITY);
while (!Thread.currentThread().isInterrupted()) {
out.print(promptOverwrite);
// call checkError, which first flushes out, and then lets us confirm that everything was written correctly:
if (out.checkError()) thrownew RuntimeException("an I/O problem was detected in out"); // should be an IOException, but that would break method contract
// limit the masking rate to fairly share the cpu; interruption breaks the loop
Thread.sleep(1); // have experimentally found that sometimes see chars for a brief bit unless set this to its min value
}
}
catch (InterruptedException ie) {
Thread.currentThread().interrupt(); // resets the interrupted status, which is typically lost when an InterruptedException is thrown, as per our method contract; see Doug Lea "Concurrent Programming in Java Second Edition" p. 171; in general should never swallow this; see http://www-128.ibm.com/developerworks/java/library/j-jtp05236.html
}
finally {
// ensure that any prompt that may have been written is erased:
out.print(setCursorToStart);
for (int i = 0; i < promptOverwrite.length(); i++) out.print(' ');
out.print(setCursorToStart);
Thread.currentThread().setPriority(priorityOriginal);
}
}
}
And here is my code which uses it:
/**
* Reads and returns some sensitive piece of information (e.g. a password)
* from the console (i.e. System.in and System.out) in a secure manner.
* <p>
* For top security, all console input is masked out while the user types in the password.
* Once the user presses enter, the password is read via a call to {@link #readLineSecure readLineSecure(in)},
* using a PushbackInputStream that wraps System.in.
* <p>
* This method never returns null.
* <p>
* @throws IOException if an I/O problem occurs
* @throws InterruptedException if the calling thread is interrupted while it is waiting at some point
* @see <a href="http://java.sun.com/features/2002/09/pword_mask.html">Password masking in console</a>
*/
publicstaticchar[] readConsoleSecure(String prompt) throws IOException, InterruptedException {
// start a separate thread which will mask out all chars typed on System.in by overwriting them using System.out:
StreamMasker masker = new StreamMasker(System.out, prompt);
Thread threadMasking = new Thread( masker, "EncryptUtil_StreamMasker" );
threadMasking.setPriority( Thread.NORM_PRIORITY );
threadMasking.start();
masker.printPromptOverwrite(); // ensure that prompt is written at least once (e.g. needed if password has been piped into the program instead of being typed by user)
// Goal: block this current thread (allowing masker to mask all user input)
// while the user is in the middle of typing the password.
// This may be achieved by trying to read just the first byte from System.in,
// since reading from System.in blocks until it detects that an enter has been pressed.
// Wrap System.in with a PushbackInputStream because this byte will be unread below.
PushbackInputStream pin = new PushbackInputStream(System.in);
int b = pin.read();
// When current thread gets here, the block on reading System.in is over (e.g. the user pressed enter, or some error occurred?)
// interrupt threadMasking and wait till it is dead:
threadMasking.interrupt();
threadMasking.join();
// check for stream errors:
if (b == -1) thrownew IOException("end-of-file was detected in System.in without any data being read");
if (System.out.checkError()) thrownew IOException("an I/O problem was detected in System.out");
// pushback the first byte and supply the now unaltered stream to readLineSecure which will return the complete password:
pin.unread(b);
return readLineSecure(pin);
}
/**
* Reads chars from in until an end-of-line sequence (EOL) or end-of-file (EOF) is encountered,
* and then returns the data as a char[].
* <p>
* The EOL sequence may be any of the standard formats: '\n' (unix), '\r' (mac), "\r\n" (dos).
* The EOL sequence is always completely read off the stream but is never included in the result.
* <i>Note:</i> this means that the result will never contain the chars '\n' or '\r'.
* In order to guarantee reading thru but not beyond the EOL sequence for all formats (unix, mac, dos),
* this method requires that a PushbackInputStream and not a more general InputStream be supplied.
* <p>
* The code is secure: no Strings are used, only char arrays,
* and all such arrays other than the result are guaranteed to be blanked out after last use to ensure privacy.
* Thus, this method is suitable for reading in sensitive information such as passwords.
* <p>
* This method never returns null; if no data before the EOL or EOF is read, a zero-length char[] is returned.
* <p>
* @throws IllegalArgumentException if in == null
* @throws IOException if an I/O problem occurs
* @see <a href="http://java.sun.com/j2se/1.4.2/docs/guide/security/jce/JCERefGuide.html#PBEEx">Password based encryption code examples from JCE documentation</a>
*/
publicstaticchar[] readLineSecure(PushbackInputStream in) throws IllegalArgumentException, IOException {
if (in == null) thrownew IllegalArgumentException("in == null");
char[] buffer = null;
try {
buffer = newchar[128];
int offset = 0;
loop: while (true) {
int c = in.read();
switch (c) {
case -1:
case '\n':
break loop;
case '\r':
int c2 = in.read();
if ((c2 != '\n') && (c2 != -1)) in.unread(c2); // guarantees that mac & dos line end sequences are completely read thru but not beyond
break loop;
default:
buffer = checkBuffer(buffer, offset);
buffer[offset++] = (char) c;
break;
}
}
char[] result = newchar[offset];
System.arraycopy(buffer, 0, result, 0, offset);
return result;
}
finally {
eraseChars(buffer);
}
}
/**
* Checks if buffer is sufficiently large to store an element at an index == offset.
* If it is, then buffer is simply returned.
* If it is not, then a new char[] of more than sufficient size is created and initialized with buffer's current elements and returned;
* the original supplied buffer is guaranteed to be blanked out upon method return in this case.
* <p>
* @throws IllegalArgumentException if buffer == null; offset < 0
*/
publicstaticchar[] checkBuffer(char[] buffer, int offset) throws IllegalArgumentException {
if (buffer == null) thrownew IllegalArgumentException("buffer == null");
if (offset < 0) thrownew IllegalArgumentException("offset = " + offset + " is < 0");
if (offset < buffer.length)
return buffer;
else {
try {
char[] bufferNew = newchar[offset + 128];
System.arraycopy(buffer, 0, bufferNew, 0, buffer.length);
return bufferNew;
}
finally {
eraseChars(buffer);
}
}
}
/** If buffer is not null, fills buffer with space (' ') chars. */
publicstaticvoid eraseChars(char[] buffer) {
if (buffer != null) Arrays.fill(buffer, ' ');
}
I would be thrilled to see your feedback on this.
I need to update my class which does all this stuff to use jdk 1.6's Console functionality, as it should solve a lot of this crap at least for passwords.
Full command line interfaces are another matter: Sun, if you are listening, we desperately need a good Java full console capability, since CLIs are so much easier to whip up than GUIs.