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

[Laine Stump]

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.


> 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"'?


> +
> +            if (VIR_STRDUP(sockname,
> +                           flags & VIR_DRV_OPEN_REMOTE_RO ?
> +                           LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> +                goto failed;
> +        }
>
>          VIR_DEBUG("Starting LibSSH2 session");
>
> @@ -698,11 +708,21 @@ doRemoteOpen(virConnectPtr conn,
>          if (!command && VIR_STRDUP(command, "ssh") < 0)
>              goto failed;
>
> -        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 ssh "
> +                                 "connection driver"));
> +                goto failed;
> +            }
> +
> +            if (VIR_STRDUP(sockname,
> +                           flags & VIR_DRV_OPEN_REMOTE_RO ?
> +                           LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> +                goto failed;
> +        }
>
>          if (!(priv->client = virNetClientNewSSH(priv->hostname,
>                                                  port,

ACK once the BZ link is included in the commit log (and pending my
question about whether we want to check for == "/session" or != "/system").