RFC: ssh-copy-id tweaks

[Nick Dokos]

Jim Knoble <jmknoble at pobox.com> wrote:

> Circa 2008-01-31 13:00 dixit Nick Dokos:
> 
> : I'd like to propose a couple of tweaks to ssh-copy-id:
> : 
> : o Change the default ID_FILE from identity.pub to id_dsa.pub or perhaps
> :   {id_dsa,id_rsa,identity}.pub to cover all the bases, although the
> :   patch below deals only with id_dsa.pub - it would need some more
> :   tweaking to deal with more than one (possibly non-existent) file.
> 
> If this is to be done, i would propose the default be id_rsa.pub, since
> the RSA patent has expired, but that it be changeable via the
> environment.  See below.

IANAL but I thought DSA's patent has been thrown open by NIST for use by
anybody (although I understand that Dr. Schnorr has some claims). In any
case, dealing with more than one (or maybe the most recent as you
mention below) would be preferable imho. I just thought that having the
default being the case that nobody uses (perhaps I should say, that
nobody should use) any more is a little strange.

> 
> : o If the destination authorized_keys file already contains the keys,
> :   they should not be duplicated. I use ssh-copy-id in a regression harness
> :   and I end up adding the same key tens or hundreds of times. I have not
> :   seen any problem but it is somewhat distasteful.
> : 
> :   The method proposed is frankly a hack, but it is simple and I think it
> :   is foolproof and portable. At least initially, it will mess up the
> :   order of the keys, but given that the file is mostly write-only by
> :   humans, that should not make any difference.
> 
> See below for comments.
> 
[patch skipped]

> 
> Even more interesting would be to make ssh-copy-id use the same
> approach as ssh-add, i.e., copying all of the available identities
> that exist (~/.ssh/id_rsa.pub, ~/.ssh/id_dsa.pub, and ~/.ssh/identity).
> (Alternatively, for more paranoai, ssh-copy-id could copy only the first
> found of the three).
> 
> This would be more complex, but might do the right thing for more folks.

I agree. Would you like to propose a patch along these lines? If not, I can
take a whack at it.

> : -{ eval "$GET_ID" ; } | ssh $1 "umask 077; test -d .ssh || mkdir .ssh ; cat >> .ssh/authorized_keys" || exit 1
> : +{ eval "$GET_ID" ; } | ssh $1 "umask 077; test -d .ssh || mkdir .ssh ; cat >> .ssh/authorized_keys && sort -u -o .ssh/authorized_keys .ssh/authorized_keys" || exit 1
> 
> I would propose the following pipeline instead (on multiple lines for
> legibility):
> 
>     { eval "$GET_ID" ; } \
>     |ssh "$1" '
>         umask 077
>         test -d .ssh || mkdir .ssh
>         cd .ssh \
>         && ( test -f authorized_keys || touch authorized_keys ) \
>         && ( cat authorized_keys; cat ) |sort |uniq >authorized_keys.$$ \
>         && mv -f authorized_keys.$$ authorized_keys
>     ' || exit 1
> 
> This does the following:
> 
>     * ensures that the .ssh directory exists and is searchable after the
>       test -d and mkdir
> 
>     * avoids writing to the authorized_keys file until it has been
>       written completely and successfully
> 
>     * avoids depending on 'sort -u', which may not be available on all
>       systems
> 
>     * remains (i believe) compatible with Bourne/ksh, and csh
>       shells and their descendants on the remote host
> 

Again, if this is deemed a better solution, I'm fine with it: I'd just
like to see the duplication go away. On the other hand, I don't think
it's much better than my suggestion: it does deal better (bullet #2)
with corner cases than the original code (and my proposed mod) but I
don't believe that `sort -u' is a portability problem (otoh, I might be
wrong).


> Alternatively, the following would keep the authorized_keys file from
> being reordered, using 'grep -F' to check whether the identity is
> already present:
> 
>     { eval "$GET_ID" ; } \
>     |ssh "$1" '
>         umask 077
>         test -d .ssh || mkdir .ssh
>         cd .ssh || exit 1
>         SSH_IDENTITY="`cat`"
>         test x"${SSH_IDENTITY}" = x"" && exit 1
>         ( test -f authorized_keys || touch authorized_keys ) \
>         grep -F "${SSH_IDENTITY}" authorized_keys >/dev/null 2>&1 \
>         || {
>             ( cat authorized_keys; echo "${SSH_IDENTITY}" )
>             >authorized_keys.$$ \
>             && mv -f authorized_keys.$$ authorized_keys
>         }
>     ' || exit 1
> 
> Unfortunately, the use of the SSH_IDENTITY variable makes this only work
> With Bourne/ksh shells and their descendants.  A shell-independent
> version of this may be able to be written with nawk or gawk, but that
> poses its own portability problems....
> 

Not worth it imho.

Thanks for the comments!

Nick