openssh unix dev - Nov 2007 - [PATCH] one-time ssh-agent confirmation password

The patch (against 4.7p1) modifies gnome-ssh-askpass to optionally 
generate a one-time
password and transmits it to the user via an out-of-band communication 
channel. If you can
read the password and enter it back into the gnome-ssh-askpass dialog, 
ssh-agent is allowed
to continue with the authentication process.

There are two ways to use the modified gnome-ssh-askpass. The first 
incrementally increases
the security provided by the traditional ssh-agent/gnome-ssh-askpass 
combination. The second
allows you to create two fully separated authentication factors - the 
private key and one-time
password - without using a specialized hardware token. Both are 
described in the attached
README.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gnome-ssh-askpass2.c.patch
Type: text/x-patch
Size: 3886 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071121/58c326ea/attachment.bin
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: README
Url:
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071121/58c326ea/attachment.ksh
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ssh-otac-fifo
Url:
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071121/58c326ea/attachment-0001.ksh

On Thu 2007-11-22 00:59:46 -0500, paul sery wrote:
> The patch (against 4.7p1) modifies gnome-ssh-askpass to optionally
> generate a one-time password and transmits it to the user via an
> out-of-band communication channel. If you can read the password and
> enter it back into the gnome-ssh-askpass dialog, ssh-agent is
> allowed to continue with the authentication process.
This is an interesting idea.  Thanks for publishing!  I haven't had
time to digest it enough to know if the general framework is something
i want, but here's a couple quick notes about the diff:
> --- gnome-ssh-askpass2.c.orig	2003-11-21 05:48:56.000000000 -0700
> +++ gnome-ssh-askpass2.c	2007-11-01 10:54:11.000000000 -0600
> @@ -38,6 +38,9 @@
>  
>  #define GRAB_TRIES	16
>  #define GRAB_WAIT	250 /* milliseconds */
> +#define OTAC_LEN         4  /* number of characters in otac passphrase */
> +#define OTAC_FIFO_LEN	128	/* number of characters in otac passphrase */
Both constants have the same comment, but have different values.  I
suspect one of the comments is wrong.  
> +
Why add the blank line?
> -passphrase_dialog(char *message)
> +passphrase_dialog(char *message, char *otac_passphrase, char *otac_fifo)
Your additions to this function (and elsewhere) seem to use spaces for
indentation.  Previous code in this file uses tabs for indentation.
Sticking to the coding convention will make your patch more appealing
to the original author.
> +/* generate the one-time agent confirm password */
> +char *
> +gen_otac_passphrase(int otac_length)
> +{
spaces v. tabs in this function, as well.
> +        int i,ran,nchars=52;
> +        char
cpool[52]="abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +        char *otac_passphrase;
> +
> +        /* use random # to select characters
> +           to build one-time passphrase with them */ 
> +
> +        srandom(time(0));
Seeding with the time (in seconds since the UNIX epoch) means that
every one of these one-time-passwords that happens in a given second
is going to use the same random password.  So that password will be
predictable -- probably not a property you intend the one time
passwords to have.
> +        otac_passphrase=malloc(otac_length+1);
> +        for (i=0;i<otac_length;i++) {
> +                ran = random();
> +                otac_passphrase[i]=cpool[ran%nchars];
> +        }
> +	strncat(otac_passphrase,"",1);
Why not just:

 otac_passphrase[otac_length] = 0;
> +
> +        return(otac_passphrase);
> +}
There's some memory management weirdness going on here.  You allocate
the otac_passphrase buffer in one function, pass it around through at
least two others, and rely on the others to free it.  While it looks
like you've cleared this one up properly, it's probably not best
practices.
>  int
>  main(int argc, char **argv)
>  {
> -	char *message;
> -	int result;
> +        char *message,*passphrase,*otac_passphrase,*otac_fifo;
> +        int result,otac_length=OTAC_LEN;
> +
> +	otac_fifo=malloc(otac_length);
Does this ever get freed?
> +        otac_fifo=getenv("SSH_OTAC_FIFO");
This looks like it overwrites the pointer to the buffer allocated
immediately above.
> +        if (otac_fifo) {
> +                otac_passphrase=malloc(otac_length+1);
> +                otac_passphrase=gen_otac_passphrase(otac_length);
> +                write_to_fifo(otac_passphrase,otac_fifo);
> +        }
>  
>  	gtk_init(&argc, &argv);
>  
> @@ -213,8 +275,7 @@
>  	}
>  
>  	setvbuf(stdout, 0, _IONBF, 0);
> -	result = passphrase_dialog(message);
> +	result = passphrase_dialog(message,otac_passphrase,otac_fifo);
if (!otac_fifo), then what is otac_passphrase set to when you pass it
in this function?  Passing uninitialized pointers is dangerous.
>  	g_free(message);
> -
Why make a whitespace change here?

Thanks again for publishing this idea.  For patches that you want
people to consider against OpenSSH, you probably want to post them to
the OpenSSH bugzilla (not just this mailing list):

 https://bugzilla.mindrot.org/

That makes your work easier to find for people looking for it later.

Regards,

        --dkg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 826 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071122/226294ae/attachment-0001.bin

Daniel Kahn Gillmor <dkg-openssh.com> wrote:
>Paul Sery (pgsery-swcp.com> wrote:
>> The patch (against 4.7p1) modifies gnome-ssh-askpass to optionally
>> generate a one-time password and transmits it to the user via an
>> out-of-band communication channel. If you can read the password and
>> enter it back into the gnome-ssh-askpass dialog, ssh-agent is
>> allowed to continue with the authentication process.
>This is an interesting idea.  Thanks for publishing!  I haven't had
>time to digest it enough to know if the general framework is something
>i want, but here's a couple quick notes about the diff:
I've cleaned up most of the clutter and tightened it up in general.
>Seeding with the time (in seconds since the UNIX epoch) means that
>every one of these one-time-passwords that happens in a given second
>is going to use the same random password.  So that password will be
>predictable -- probably not a property you intend the one time
>passwords to have.
Yes, my current implementation is a place-holder. I'd like guidance 
on whether to use arc4random_stir or something else. 
>Thanks again for publishing this idea.  For patches that you want
>people to consider against OpenSSH, you probably want to post them to
>the OpenSSH bugzilla (not just this mailing list):
> https://bugzilla.mindrot.org/
> That makes your work easier to find for people looking for it later.
Bug 1393.

Thanks for the advice and help! I hope it proves useful.

-Paul

-------------- next part --------------
A non-text attachment was scrubbed...
Name: gnome-ssh-askpass2.c.patch
Type: text/x-patch
Size: 3158 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071124/3702c644/attachment-0002.bin
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: README
Url:
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071124/3702c644/attachment-0001.ksh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ssh-otac-fifo.pl
Type: application/x-perl
Size: 2756 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20071124/3702c644/attachment-0003.bin