Re: [libvirt] [PATCH] remote: Forbid default "/session" connections when using ssh transport

On 06/12/13 21:34, Laine Stump wrote: > On 06/10/2013 08:44 AM, Peter Krempa wrote: > > > Can you include in the commit log a link to the BZ describing this > problem? It helps *immensely* when trying to trace things months/years > later. > There isn't any publicly available BZ describing this problem so I chose not to put the private link in the commit. I can do it if you insist but having private links in a public repo doesn't seem right to me.

> >> Without the socket path explicitly specified, the remote driver >> tried to >> connect to the "/system" instance socket even if "/session" was >> specified in the uri. With this patch this configuration now >> produces an >> error. >> >> It is still possible to initiate a session connection with specifying >> the path to the socket manually and also manually starting the session >> daemon. This was also possible prior to this patch, >> >> This is a minimal fix. We may decide to support remote session >> connections using ssh but this will require changes to the remote >> driver >> code so this fix shouldn't cause regressions in the case we decide >> to do >> that. >> --- >> src/remote/remote_driver.c | 40 >> ++++++++++++++++++++++++++++++---------- >> 1 file changed, 30 insertions(+), 10 deletions(-) >> >> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c >> index fcf45d3..bd5646a 100644 >> --- a/src/remote/remote_driver.c >> +++ b/src/remote/remote_driver.c >> @@ -631,11 +631,21 @@ doRemoteOpen(virConnectPtr conn, >> break; >> >> case trans_libssh2: >> - if (!sockname && >> - VIR_STRDUP(sockname, >> - flags & VIR_DRV_OPEN_REMOTE_RO ? >> - LIBVIRTD_PRIV_UNIX_SOCKET_RO : >> LIBVIRTD_PRIV_UNIX_SOCKET) < 0) >> - goto failed; >> + if (!sockname) { >> + /* Right now we don't support default session >> connections */ >> + if (STREQ(conn->uri->path, "/session")) { >> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", >> + _("Connecting to session instance >> without " >> + "socket path is not supported by >> the libssh2 " >> + "connection driver")); >> + goto failed; >> + } > > A totally naive question: do we want to only allow "/system"? or > 'anything except "/session"'? We want to forbid only "/session" with the default socket path which won't work right now. A user is able to start a session daemon and successfully connect to it even with this patch. The user has to manually specify a socket path. Rejecting everything except "/system" would break drivers that use different path.