ssh-copy-id fails when run with colon (:)

Bug #249706 reported by KarlGoetz on 2008-07-18
2
Affects Status Importance Assigned to Milestone
portable OpenSSH
Confirmed
Unknown
openssh (Debian)
Fix Released
Unknown
openssh (Ubuntu)
Wishlist
Colin Watson

Bug Description

For example:

kgoetz@leia:~$ ssh-copy-id -i .ssh/id_rsa.pub example.com:
15
ssh: example.com:: Name or service not known

Since : is required when using scp, its something (i at least) keep adding to my domains with ssh-copy-id.
It would be great if ssh-copy-id could silently remove the : from the end of the line before trying.

Related branches

Colin Watson (cjwatson) on 2008-08-19
Changed in openssh:
importance: Undecided → Wishlist
status: New → Triaged
Changed in openssh:
status: Unknown → New
KarlGoetz (kgoetz) wrote :

This is an update to the patch i posted in Debians BTS - it diffs "the correct way", rather then improving functionality at all. If it works for other people, I'll try and do up a proper using-patch-system debdiff.

Colin Watson (cjwatson) wrote :

I don't use a patch system for openssh, and would prefer you not to introduce one.

Colin Watson (cjwatson) wrote :

As I said in the Debian BTS, perhaps ssh itself should do this, rather than ssh-copy-id? You apparently ignored this comment. That said, that seems to be fiddly and maybe isn't worth it.

Your patch introduces 'set -ex'. -e doesn't make any difference in this code, and -x is noisy.

Your patch doesn't follow the indentation convention in that file. You should always follow the prevailing convention.

It would be better just to strip colons at the end, rather than throughout (which could produce very confusing results!). For that, just replacing $1 with ${1%:} would be entirely sufficient; you don't need to introduce another shell variable. (And why export HOSTNAME? Nothing outside this script needs it.)

In future, consider writing this instead of introducing an unnecessary call to the '[' program:

-if [ `echo "$1" |grep -c \: -` ]; then
+if echo "$1" | grep -q :; then

In the end, I've applied the attached patch to my Debian CVS repository. Thanks for nudging this along.

Changed in openssh:
assignee: nobody → kamion
status: Triaged → In Progress
Changed in openssh:
status: Unknown → Confirmed
Changed in openssh:
status: New → Fix Committed

On Thu, October 9, 2008 21:25, Colin Watson wrote:
> As I said in the Debian BTS, perhaps ssh itself should do this, rather
> than ssh-copy-id? You apparently ignored this comment.

I have no C, which i assume is what ssh proper is written in.

That said, that
> seems to be fiddly and maybe isn't worth it.
>
> Your patch introduces 'set -ex'. -e doesn't make any difference in this
> code, and -x is noisy.

Sorry, I should have cleaned up after testing.

> Your patch doesn't follow the indentation convention in that file. You
> should always follow the prevailing convention.
>
> It would be better just to strip colons at the end, rather than
> throughout (which could produce very confusing results!). For that, just
> replacing $1 with ${1%:} would be entirely sufficient; you don't need to
> introduce another shell variable. (And why export HOSTNAME? Nothing
> outside this script needs it.)

>
> In future, consider writing this instead of introducing an unnecessary
> call to the '[' program:
>
> -if [ `echo "$1" |grep -c \: -` ]; then
> +if echo "$1" | grep -q :; then

I didnt know this was posible, so thanks for letting me know. I'll try and
familiarise myself with it.

>
> In the end, I've applied the attached patch to my Debian CVS repository.
> Thanks for nudging this along.

Thanks for fixing it up.
kk

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package openssh - 1:5.1p1-4ubuntu1

---------------
openssh (1:5.1p1-4ubuntu1) jaunty; urgency=low

  * Resynchronise with Debian. Remaining changes:
    - Add support for registering ConsoleKit sessions on login.
    - Drop openssh-blacklist and openssh-blacklist-extra to Suggests; they
      take up a lot of CD space, and I suspect that rolling them out in
      security updates has covered most affected systems now.
    - Add ufw integration.

openssh (1:5.1p1-4) unstable; urgency=low

  * ssh-copy-id: Strip trailing colons from hostname (closes: #226172,
    LP: #249706; thanks to Karl Goetz for nudging this along; forwarded
    upstream as https://bugzilla.mindrot.org/show_bug.cgi?id=1530).
  * Backport from upstream CVS (Markus Friedl):
    - Only send eow and no-more-sessions requests to openssh 5 and newer;
      fixes interop problems with broken ssh v2 implementations (closes:
      #495917).
  * Fix double-free when failing to parse a forwarding specification given
    using ~C (closes: #505330; forwarded upstream as
    https://bugzilla.mindrot.org/show_bug.cgi?id=1539).

 -- Colin Watson <email address hidden> Sun, 23 Nov 2008 14:55:17 +0000

Changed in openssh:
status: In Progress → Fix Released
Changed in openssh:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.