https://bugzilla.mindrot.org/show_bug.cgi?id=1980
dajoker at gmail.com changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dajoker at gmail.com
--- Comment #6 from dajoker at gmail.com ---
The ' openssh-unix-dev at 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.
--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980
--- Comment #7 from Damien Miller <djm at mindrot.org> ---
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 at 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.
--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #8 from Philip Hands <phil at hands.com> --- 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. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #9 from Philip Hands <phil at hands.com> --- (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 -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #10 from Damien Miller <djm at mindrot.org> --- 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) ; doI'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. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #11 from Philip Hands <phil at hands.com> --- (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? -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #12 from Martin Kletzander <mkletzan at redhat.com> --- Created attachment 2210 --> https://bugzilla.mindrot.org/attachment.cgi?id=2210&action=edit 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). -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #13 from Philip Hands <phil at hands.com> --- (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 at hands.com Cheers, Phil. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #14 from Damien Miller <djm at mindrot.org> --- Comment on attachment 2210 --> https://bugzilla.mindrot.org/attachment.cgi?id=2210 Patch to restore contexts on authorized_keys>+which restorecon >/dev/null 2>&1 && restorecon -F .ssh .ssh/authorized_keysI don't think "which" is in POSIX, but "type" is. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980
Martin Kletzander <mkletzan at redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mkletzan at redhat.com
--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #15 from Martin Kletzander <mkletzan at redhat.com> --- (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. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980 --- Comment #16 from Philip Hands <phil at hands.com> --- 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. -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug.
https://bugzilla.mindrot.org/show_bug.cgi?id=1980
--- Comment #17 from Philip Hands <phil at hands.com> ---
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 `...` ?)
--
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.