Re: RFC: ssh-copy-id tweaks

Jim Knoble <jmknoble@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

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/li...enssh-unix-dev