Bug#66740: ssh-askpass-gnome: The first password is always bad (was: OpenSSH 3.0)

[Charles C. Fu]

   Background: Many Debian users have reported a problem to Debian
   with ssh-askpass-gnome always rejecting the first passphrase.
   <URL:http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=66740&repeatmerged=yes>
   shows the thread for one of these reports.

OK, I have traced through the logic and believe I have identified the
causes of this bug.

1. ssh-askpass-gnome uses puts() to print the passphrase.  The puts()
   call writes the passphrase and a line terminator to stdout.
   However, multiple write() calls may be invoked by puts() to write
   the passphrase and terminator.

2. In ssh-add, readpass.c(ssh_askpass) uses a single read() call to
   read the passphrase from the SSH_ASKPASS program.  If the
   SSH_ASKPASS program used multiple calls to write(), depending upon
   timing, read() may not return the characters written by write()
   calls after the first.  With Debian/Linux, the result is that only
   the passphrase is read and not the LF.

3. In ssh-add, readpass.c(ssh_askpass) did not correctly check if the
   characters read ended with a line terminator.  If they did not,
   the function would not null-terminate the string correctly, and
   ssh-add could report a bad passphrase (depending upon what the
   contents of buf happened to be before the read()).  As reported on
   the openssh-unix-dev mailing list (see, for example, the message
   <20011107.124748.92055508.gotoh at taiyo.co.jp> on 06 Nov 2001), this
   problem could also result in worse problems.

   The second time the passphrase is requested, the buffer into
   which the passphrase is read is pre-zeroed with memset, thereby
   null-terminating the string (normally) and avoiding the bug.

Fixing any of these three issues should eliminate this problem.  The
third issue was already fixed in the CVS repository on Nov 8, 2001.  I
think the second issue should be addressed as well.

1. As Mattia Monga suggested, replacing puts() in ssh-askpass-gnome
   with a function that does a single call to write() works around the
   bug.  Of course, it does not fix the underlying problems in
   readpass.c, so other SSH_ASKPASS programs could continue to exhibit
   this bug.

2. In readpass.c(ssh_askpass), replacing the single read() call with a
   loop to be sure all characters are read (up to the size permitted
   by the buf buffer) would be a good fix to the problem.

   Moving the waitpid() call up before the read() call (so that the
   passphrase is not read from the pipe until the SSH_ASKPASS program
   has exited) also works for ordinary cases.  However, deadlock could
   then result if the SSH_ASKPASS program tries to write a passphrase
   exceeding the OS buffer for pipes.

3. Revision 1.23 of readpass.c, as a pleasant side effect, by ensuring
   that the characters read are always null terminated, works around
   the ssh-askpass-gnome problem in addition to fixing the buffer
   overflow problem for which it was originally submitted.  Simply
   picking up this recent revision in Debian (it was incorporated into
   OpenSSH 3.0.1) should have fixed the apparent ssh-askpass-gnome
   bug, so it and all the duplicate reports of this problem can now be
   closed in the Debian bug tracking system.  I can confirm that one
   of my Debian boxes exhibited this bug in versions up to and
   including 2.9p2-6 and that it has now finally gone away with the
   installation of 3.0.1p1-1.2.

   Again, theoretical problems with other SSH_ASKPASS programs which
   use multiple write() calls to write the passphrase will remain
   until issue 2 above is resolved.

Finally, since readpass.c(ssh_askpass) does not use the stdio library
to read the passphrase, it might be a good idea to recommend
SSH_ASKPASS programs not use stdio routines to write the passphrase.
This would avoid line terminator mismatches.  However, with the
changes in readpass.c revision 1.23, the line terminator has become
optional, and I think it would be an even better idea to phase out
writing it in SSH_ASKPASS programs.

-ccwf
-- 
                 ,--         Charles C. Fu          ccwf at bacchus.com
___  __ __. . ,-/--         Vice President            310-455-2396
    (_,(_,|/|/ /             Bacchus, Inc.    http://www.bacchus.com/~ccwf/
            --'