[Pkg-libvirt-maintainers] Bug#637548: Bug#637548: virt-viewer: should not hardcode SSH port 22, but read from ~/.ssh/config
Luca Capello
luca at pca.it
Fri Aug 12 15:30:12 UTC 2011
found 637548 0.4.1-1
tags 637548 + patch
thanks
Hi there!
On Fri, 12 Aug 2011 16:42:39 +0200, Guido Günther wrote:
> On Fri, Aug 12, 2011 at 03:23:17PM +0200, Luca Capello wrote:
>> Package: virt-viewer
>> Version: 0.4.0-1
>> Severity: important
>> Tags: upstream
>> Usertags: pca-virtualization
>>
>> Hi there!
>>
>> 0.4.0-1 is supposed to solve this issue thanks to:
>>
>> <http://anonscm.debian.org/gitweb/?p=pkg-libvirt/virt-viewer.git;a=commitdiff;h=2376c067bd80cf68c425d51982deece3e6129e1a>
>
> This looks like a brown paper bag for me. It has
>
> if (!sshport)
>
> while it should have
>
> if (sshport)
>
> in virt-viewer-app.c:322. Sorry for that. I'll roll out a new version
> but will wait for your patch.
Well, I seem to remember I tested your patch at DebConf11, but maybe it
was not the case, so blame on me as well for that ;-)
The package used to test the patch attached below is available at the
following link, it includes an extra debug patch that shows each value
used to construct the SSH/nc tunnel:
<http://people.debian.org/~gismo/tmp/virt-viewer_0.4.1-2~637548.dsc>
Thx, bye,
Gismo / Luca
--8<---------------cut here---------------start------------->8---
diff -Nru virt-viewer-0.4.1/debian/changelog virt-viewer-0.4.1/debian/changelog
--- virt-viewer-0.4.1/debian/changelog 2011-08-06 12:39:42.000000000 +0200
+++ virt-viewer-0.4.1/debian/changelog 2011-08-12 17:17:58.000000000 +0200
@@ -1,3 +1,11 @@
+virt-viewer (0.4.1-2~637548) UNRELEASED; urgency=low
+
+ * debian/patches/series,
+ debian/patches/src_virt-viewer-app.c_fix-SSH-port-handling.patch:
+ new file (Closes: #637548).
+
+ -- Luca Capello <luca at pca.it> Fri, 12 Aug 2011 17:17:58 +0200
+
virt-viewer (0.4.1-1) unstable; urgency=low
* [09d4f14] Imported Upstream version 0.4.1
diff -Nru virt-viewer-0.4.1/debian/patches/DEBUG_SSH-port.patch virt-viewer-0.4.1/debian/patches/DEBUG_SSH-port.patch
--- virt-viewer-0.4.1/debian/patches/DEBUG_SSH-port.patch 1970-01-01 01:00:00.000000000 +0100
+++ virt-viewer-0.4.1/debian/patches/DEBUG_SSH-port.patch 2011-08-11 17:14:16.000000000 +0200
@@ -0,0 +1,13 @@
+--- virt-viewer.orig/src/virt-viewer-app.c
++++ virt-viewer/src/virt-viewer-app.c
+@@ -339,6 +339,10 @@
+ }
+ cmd[n++] = NULL;
+
++ int j;
++ for(j=0; j<n; j++)
++ printf("open_tunnel_ssh_cmd[%d]:|%s|\n", j, cmd[j]);
++
+ return virt_viewer_app_open_tunnel(cmd);
+ }
+
diff -Nru virt-viewer-0.4.1/debian/patches/series virt-viewer-0.4.1/debian/patches/series
--- virt-viewer-0.4.1/debian/patches/series 1970-01-01 01:00:00.000000000 +0100
+++ virt-viewer-0.4.1/debian/patches/series 2011-08-12 17:16:08.000000000 +0200
@@ -0,0 +1,2 @@
+src_virt-viewer-app.c_fix-SSH-port-handling.patch
+DEBUG_SSH-port.patch
diff -Nru virt-viewer-0.4.1/debian/patches/src_virt-viewer-app.c_fix-SSH-port-handling.patch virt-viewer-0.4.1/debian/patches/src_virt-viewer-app.c_fix-SSH-port-handling.patch
--- virt-viewer-0.4.1/debian/patches/src_virt-viewer-app.c_fix-SSH-port-handling.patch 1970-01-01 01:00:00.000000000 +0100
+++ virt-viewer-0.4.1/debian/patches/src_virt-viewer-app.c_fix-SSH-port-handling.patch 2011-08-12 17:17:25.000000000 +0200
@@ -0,0 +1,102 @@
+Description: src/virt-viewer-app.c: fix SSH port handling
+
+Commit 39439b0b3e531f1eeac44fec229b8cd77bed78a6 ("Don't hardcode SSH
+port to 22") is not enough to deal with ~/.ssh/config settings, see
+Debian bug #637548. This patch addresses the problem once and for
+all, doing different things:
+
+1) bring virt_viewer_app_open_tunnel_ssh back to sanity, consistently
+ using the global variable names.
+
+2) set the ssh -p switch only if port is defined, reflecting
+ src/virt-viewer-app.c:327.
+
+3) in src/virt-viewer-app.c:646, showing the default SSH port when no
+ port is specified in the URI is wrong, given that in this case
+ ~/.ssh/config should be parsed to find the correct port.
+
+ Please note that when no port is specified in the URI 0 is anyway
+ shown, which is a bit unfortunate, since it does not distinguish
+ when no or the real 0 value was assigned to the variable.
+
+4) in src/virt-viewer-app.c:1093, get the port value from the URI, as
+ in src/virt-viewer-util.c:87, instead of always setting it to 0.
+
+Author: Luca Capello <luca at pca.it>
+Bug-Debian: http://bugs.debian.org/637548>
+Last-Update: 2011-08-12
+
+--- virt-viewer.orig/src/virt-viewer-app.c
++++ virt-viewer/src/virt-viewer-app.c
+@@ -307,11 +307,11 @@
+
+
+ static int
+-virt_viewer_app_open_tunnel_ssh(const char *sshhost,
+- int sshport,
+- const char *sshuser,
+- const char *host,
+- const char *port,
++virt_viewer_app_open_tunnel_ssh(const char *host,
++ int port,
++ const char *user,
++ const char *ghost,
++ const char *gport,
+ const char *unixsock)
+ {
+ const char *cmd[10];
+@@ -319,20 +319,20 @@
+ int n = 0;
+
+ cmd[n++] = "ssh";
+- if (!sshport) {
++ if (port) {
+ cmd[n++] = "-p";
+- sprintf(portstr, "%d", sshport);
++ sprintf(portstr, "%d", port);
+ cmd[n++] = portstr;
+ }
+- if (sshuser) {
++ if (user) {
+ cmd[n++] = "-l";
+- cmd[n++] = sshuser;
++ cmd[n++] = user;
+ }
+- cmd[n++] = sshhost;
++ cmd[n++] = host;
+ cmd[n++] = "nc";
+- if (port) {
+- cmd[n++] = host;
+- cmd[n++] = port;
++ if (gport) {
++ cmd[n++] = ghost;
++ cmd[n++] = gport;
+ } else {
+ cmd[n++] = "-U";
+ cmd[n++] = unixsock;
+@@ -643,7 +643,7 @@
+ priv->unixsock);
+ }
+ virt_viewer_app_trace(self, "Setting up SSH tunnel via %s@%s:%d\n",
+- priv->user, priv->host, priv->port ? priv->port : 22);
++ priv->user, priv->host, priv->port);
+
+ if ((fd = virt_viewer_app_open_tunnel_ssh(priv->host, priv->port,
+ priv->user, priv->ghost,
+@@ -1304,6 +1304,7 @@
+ g_free(priv->transport);
+ g_free(priv->unixsock);
+ g_free(priv->user);
++ g_free(priv->port);
+
+ priv->host = g_strdup(host);
+ priv->ghost = g_strdup(ghost);
+@@ -1311,7 +1312,7 @@
+ priv->transport = g_strdup(transport);
+ priv->unixsock = g_strdup(unixsock);
+ priv->user = g_strdup(user);
+- priv->port = 0;
++ priv->port = port;
+
+ virt_viewer_app_update_pretty_address(self);
+ }
--8<---------------cut here---------------end--------------->8---
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-libvirt-maintainers/attachments/20110812/860a52b3/attachment.pgp>
More information about the Pkg-libvirt-maintainers
mailing list