As you can see here: http://git.hands.com/ssh-copy-id and as mentioned here: http://lists.mindrot.org/pipermail/openssh-unix-dev/2010-December/029123.html I've produced an updated version of ssh-copy-id I hope you don't consider that I'm being pushy by also submitting a bug, but the mail to the list provoked no response whatsoever, and it seems a shame that this is not getting used upstream. I note that there are probably bug reports that have been reported against the old version, that may well also apply to this (i.e. I just noticed the bug report about SELinux incompatibility). I'll cheerfully fix such issues if they are shown to exist, but I'm unlikely to put the work in unless there's some reasonable prospect of the work being put to use. Cheers, Phil.
Created attachment 2133 [details] patch showing differences from current A bug is our preferred method of getting our attention since we don't always (often?) have spare cycles :-) I've attached the diff comparing the two for reviewing purposes.
Comment on attachment 2133 [details] patch showing differences from current It's a big diff so it's hard to evaluate it. some random comments: > USER_HOST=${1%:} is that a bashism? it doesn't work with solaris' /bin/sh >but it is much better to instead use >+.Xr ssh 1 Ns 's >+.Ar ProxyCommand >+option with >+.Xr netcat 1 you can use ssh -W insted of ssh+netcat as a proxycommand which behaves the same but doesn't require anything on the intermediate host except tcp port forwarding.
[Hmm, seems bugzilla ignored my reply, so I'll paste it in here] > https://bugzilla.mindrot.org/show_bug.cgi?id=1980 > > --- Comment #2 from Darren Tucker <dtucker@zip.com.au> 2012-02-24 11:50:27 EST --- [ 2 more citation lines. Click/Enter to show. ] > Comment on attachment 2133 [details] > --> https://bugzilla.mindrot.org/attachment.cgi?id=2133 > patch showing differences from current > > It's a big diff so it's hard to evaluate it. Well, it's more an almost complete rewrite that a diff, so perhaps better to evaluate it from that point of view. > some random comments: > > > USER_HOST=${1%:} > > is that a bashism? it doesn't work with solaris' /bin/sh Really? It works in dash, so how out of date is solaris's version of posix? =-=-=-=- phil@poker:~/src/ssh-copy-id$ dash $ X=foo: $ echo $X foo: $ echo ${X%:} foo $ =-=-=-=- So, any suggestions for a portable implementation (given that I'd rather assumed that if dash does it, most other shell implementations would be capable too, but apparently not -- of course I've been writing shell stuff since the early '80s so it's a little difficult to remember what counts as obsolete these days). > >but it is much better to instead use > >+.Xr ssh 1 Ns 's [ 4 more citation lines. Click/Enter to show. ] > >+.Ar ProxyCommand > >+option with > >+.Xr netcat 1 > > you can use ssh -W insted of ssh+netcat as a proxycommand which behaves > the same but doesn't require anything on the intermediate host except > tcp port forwarding. Ah, good point, I'd not noticed that option, but that nicely side-steps the problem with there being two nc implementations with varying options, so I'll definitely be using that in future -- thanks. Cheers, Phil.
(In reply to comment #3) > Really? It works in dash, so how out of date is solaris's version of > posix? Sadly, Solaris isn't even the usual lowest common denominator (that honour usually goes to one of the SCOs unixes). It looks like I did the equivalent with sed: http://anoncvs.mindrot.org/index.cgi/openssh/contrib/ssh-copy-id?r1=1.9&r2=1.10 It'd probably be worth checking if any of the other changes in our history apply to your version. http://anoncvs.mindrot.org/index.cgi/openssh/contrib/ssh-copy-id?view=log
Sorry for the delay. I've finally added your suggestion, and grabbed a few things out of the change log, also as suggested. What do you think?
The ' openssh-unix-dev@mindrot.org' mailing list thread "ssh-copy-id enhancement 2013-01-01" covers another request for this; the version behind this bug appears to be well beyond that request so anytime this can be pushed into the main codebase would be great. Sorry I didn't find this sooner.
I'd like to get this in shortly. Some comments on the revised script: 35 DEFAULT_PUB_ID_FILE=$(ls -t ${HOME}/.ssh/*.pub | head -n 1) The man page says that the default behaviour is to copy id*.pub. I think copying id* is a better idea too. You also need to exclude *-cert.pub as these don't have any place in authorized_keys. 67 GETOPT_PARSED=$(getopt --options 'i::p:nh?' --name "$0" --quiet -- "$@") Please consider passing through all -o options directly to the ssh commandline. 131 populate_new_ids() { The old ssh-copy-id script didn't do this and I can't say that I'm thrilled with the extra complexity it requires. It also has the potential to be quite slow when a number of key are to be copied. authorized_keys doesn't care if the IDs already exist, but I guess it would be worthwhile to ensure that an ID with key options isn't clobbered by one that lacks them. IMO it would be better to do everything in one ssh run: connect, grep for the keys in authorized_keys and add them if they aren't already there. If this leads to too long a command-line then you might need to consider piping in a script to "ssh user@host sh". 182 if [ $? = 255 ] ; then 183 echo "$0: WARNING: NetScreen only supports dsa keys" >&2 IMO it would be better to grep for "ssh-dsa" in the key strings rather than sending them to the remote host. 193 # Assuming default being OpenSSH I think it would be a good idea to verify this assumption. e.g. by doing a "test -x ssh-keygen || exit 1" early in the commandline.
As you can see from the code, the man page is in need of an update. The new default is to use the most recent (in ls -t terms) *.pub file. This allows one to touch the .pub that you most want sent and make that the default. id* strikes me as rather too likely to send the private key to a host that you might not trust. I can certainly strip out the -cert.pub's from the list though. We could add an option to disabled the populate_new_ids behaviour for people that might be in a hurry, and that don't want those checks, but it seems quite nice for the occasional user to have that default. I guess the multiple runs was a attempt to assume as little about the far end as possible -- I'll look at that. As for assuming that the far end is OpenSSH -- really that's just falling back to the assumptions that everyone has been using in the current ssh-copy-id. Do we really care if the far end is non-free ssh, as long as it works with the same format of authorized_keys? I suppose the comment should actually say something along those lines instead though. I'll have a look at it later today if I have chance on the trains I'll be on.
(In reply to comment #8) ... > I'll have a look at it later today if I have chance on the trains > I'll be on. I did that last week -- new version in my git repo: http://git.hands.com/ssh-copy-id
Thanks for making the changes - it's pretty close now. A couple more comments: > 35 DEFAULT_PUB_ID_FILE=$(ls -t ${HOME}/.ssh/*.pub | grep -v -- '-cert.pub$' | head -n 1) Could you make this id*.pub? I worry about people accidentally exporting special-use keys instead of the usual default auth keys by mistake. > 67 GETOPT_PARSED=$(getopt --options 'i::p:nh?' --name "$0" --quiet -- "$@") Would it be possible to pass -o [arg] though to ssh? Quite a few people have requested this over the years. > 178 NetScreen*) > 179 populate_new_ids 1 > 180 for KEY in $(echo "$NEW_IDS"| cut -d' ' -f2) ; do I'd add: echo "$KEY" | grep -q ssh-dss || continue to skip non-DSA keys here if that's all the Netscreens support. I think populate_new_ids() might need a umask call too.
(In reply to comment #10) > Thanks for making the changes - it's pretty close now. No problem. > A couple more comments: > > > 35 DEFAULT_PUB_ID_FILE=$(ls -t ${HOME}/.ssh/*.pub | grep -v -- '-cert.pub$' | head -n 1) > > Could you make this id*.pub? I worry about people accidentally > exporting special-use keys instead of the usual default auth keys by > mistake. Done. > > 67 GETOPT_PARSED=$(getopt --options 'i::p:nh?' --name "$0" --quiet -- "$@") > > Would it be possible to pass -o [arg] though to ssh? Quite a few > people have requested this over the years. I presume that will need to be properly quoted in order to be passed through, and that they may want to specify more than one -o option. That seems to mean that I'll have to do some sort of nasty quoting, and then eval the ssh command in order to unwrap the quoting, or am I making things more complicated than they need to be? > > 178 NetScreen*) > > 179 populate_new_ids 1 > > 180 for KEY in $(echo "$NEW_IDS"| cut -d' ' -f2) ; do > > I'd add: > > echo "$KEY" | grep -q ssh-dss || continue > > to skip non-DSA keys here if that's all the Netscreens support. Well, I've added a warning, and made the error messages a bit more useful (hopefully), but in effect -- Done. > I think populate_new_ids() might need a umask call too. Do you mean 0022 in case they have something silly set, or 0177 or some such for reasons of paranoia?
Created attachment 2210 [details] Patch to restore contexts on authorized_keys Would it be possible to make the ssh-copy-id selinux-aware? I proposed a small patch in the attachments that adds the possibility. It modifies more things, like readability, but the important line is the one with 'restorecon'. I also found out few other things about the script, so if you're willing to consider that, I'll be glad to provide a feedback (even with patches).
(In reply to comment #12) > Created attachment 2210 [details] > Patch to restore contexts on authorized_keys > > Would it be possible to make the ssh-copy-id selinux-aware? Seems like a reasonable idea, and as long as it's not too radical a change, shouldn't delay the inclusion in the next ssh release, but: The ret=$? in the patch seems a bit pointless, given the preceding ... || exit 1 I'd think that something like this should do the trick for the first half of the patch: mkdir -p .ssh && cat >> ~/.ssh/authorized_keys || exit 1 but the next bit looks like two different versions of the patch incorrectly glued together, so perhaps you will be kind enough to have another look and write what you meant instead. Also, please include a space in front of ;'s to match the style of the rest of the script. > I also found out few other things about the script, so if you're > willing to consider that, I'll be glad to provide a feedback (even > with patches). Feel free to mail me about it at phil@hands.com Cheers, Phil.
Comment on attachment 2210 [details] Patch to restore contexts on authorized_keys >+which restorecon >/dev/null 2>&1 && restorecon -F .ssh .ssh/authorized_keys I don't think "which" is in POSIX, but "type" is.
(In reply to comment #14) > Comment on attachment 2210 [details] > Patch to restore contexts on authorized_keys > > >+which restorecon >/dev/null 2>&1 && restorecon -F .ssh .ssh/authorized_keys > > I don't think "which" is in POSIX, but "type" is. Yes, sorry for that. I've sent the fixed version with some other details to Phil, so he's in charge of publishing it (either in the git or here) now.
OK, Martin's stuff partly incorporated, partly re-implemented. Also, I've added a check for multiple -i options (which won't work, so might as well throw an error) and added a message to distinguish between the testing stage, and the key installation phase.
Oh BTW, while looking for portability tips for old shells, I note that: http://ftp.gnu.org/old-gnu/Manuals/autoconf-2.57/html_chapter/autoconf_10.html#SEC119 suggests that old BSD shells, such as Ultrix sh, choke on ${ :- } which I presume means that they also fail with ${ := }, which I just used. That is a document from 2002 though, so hopefully it's OK to ignore what was already old then? For instance, that document recommends against $(...) too, which I'm also using, so I presume we're not trying to support stone-age shells (or did you just miss that in the earlier comments, and should I revert to using `...` ?)
Roll in some other ssh-copy-id bugs. I think the update fixes some of these already.
> > > 67 GETOPT_PARSED=$(getopt --options 'i::p:nh?' --name "$0" --quiet -- "$@") > > > > Would it be possible to pass -o [arg] though to ssh? Quite a few > > people have requested this over the years. > > I presume that will need to be properly quoted in order to be passed > through, and that they may want to specify more than one -o option. I don't think so, you just need to retain -o's argument and pass the whole mass through quoted. E.g. -o) PORTOPTION="\"-o$2"\ " shift 2 ;; should do it. > > I think populate_new_ids() might need a umask call too. > > Do you mean 0022 in case they have something silly set, or 0177 or > some > such for reasons of paranoia? paranoia ;)
(In reply to comment #19) > > > > 67 GETOPT_PARSED=$(getopt --options 'i::p:nh?' --name "$0" --quiet -- "$@") > > > > > > Would it be possible to pass -o [arg] though to ssh? Quite a few > > > people have requested this over the years. > > > > I presume that will need to be properly quoted in order to be passed > > through, and that they may want to specify more than one -o option. > > I don't think so, you just need to retain -o's argument and pass the > whole mass through quoted. E.g. > > -o) > PORTOPTION="\"-o$2"\ " > shift 2 > ;; > > should do it. I'm presuming that was meant to be: PORTOPTION="\"-o$2\" " or in fact, probably SSH_O_OPTS=... Anyway, I cannot get that quoting to work -- it seems to leave the quotes in and then use it as a hostname -- I've tried a few variations with no luck so far. Also, I'd say (assuming the quoting worked) that it should actually be something like the following, in order to allow -o to be specified more than once: SSH_OPTS="$SSH_OPTS \"-o$2\"" with the -p option similarly doing: SSH_OPTS="$SSH_OPTS -p $2" and replacing PORTOPTION with SSH_OPTS throughout. I'm also mildly concerned about how appropriate the -o options that people specify are going to be for the diagnostic probes, and other ssh calls. I'm assuming that they'll be harmless, and perhaps essential, so should be added to every ssh call in the script. anyway, here's the failed attempt at that patch -- suggestions as to how to make the quoting work properly would be welcome: http://git.hands.com/ssh-copy-id?p=ssh-copy-id.git;a=commitdiff;h=79ea2824f05cf3c1491231e9acf5faa3cc415221 > > > I think populate_new_ids() might need a umask call too. > > > > Do you mean 0022 in case they have something silly set, or 0177 or > > some > > such for reasons of paranoia? > > paranoia ;) Fair enough, I've added that. Cheers, Phil.t
(In reply to comment #20) > (In reply to comment #19) > anyway, here's the failed attempt at that patch -- suggestions as to > how to make the quoting work properly would be welcome: > > http://git.hands.com/ssh-copy-id?p=ssh-copy-id.git;a=commitdiff; > h=79ea2824f05cf3c1491231e9acf5faa3cc415221 > The quoting doesn't seem to work as expected no matter what I'm trying to do. The only working approach seems to be using 'set' for SSH_OPTS and then "$@" for the proper use of the parameters, but that makes lots of places in the code pretty unhappy. Other working approach (that might be the only solution) seems to be eval'ing the expanded string. I 've gotten this tip after I've started shouting and closed the editor, so I ended up with no attemp on this at all. I'll have a look at it later, but in case somebody wants to have a look, this info might help a bit: # Eliminating the problem to the smallest part possible ... $ l="a 'b c' d" $ for i in $l; do echo $i; done a 'b c' d # ... shows the problem I've been dealing with # Using eval on this command in this case ... $ eval "$(echo "for i in $l; do echo \$i; done")" a b c d # ... makes it work properly I've also realized that assigning it to a variable makes it sad again. Hope this helps someone. It could be also fixed using arrays, but those won't be very compatible, I guess. Martin
OK, I was hoping to be able to avoid the eval set ... trick, but it seems we need it, so I've done that, and while I was about it also decided to get rid of all the echos in favour of printfs to avoid any chance of getting bitten by echo's crappy portability. latest version on my git repo. Cheers, Phil.
Updated ssh-copy-id script committed for openssh-6.2
mark bugs closed by openssh-6.2 release as CLOSED