[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH RFCv2 4/5] libssh2_transport: Use libssh2 driver code in remote driver



On 04.01.2012 00:47, Peter Krempa wrote:
> This patch adds URI options to support libssh2 transport in the remote
> driver.
> 
> A new transport sceme is introduced eg. "qemu+libssh://..." that
> utilizes the libssh2 code added in previous patches.
> 
> The libssh2 code requires the authentication callback to be able to
> perform keyboard-interactive authentication or to ask t passprhases or
> add host keys to known hosts database.
> 
> Added URI components:
> - known_hosts -  path to a knownHosts file in OpenSSH format to check
>                  for known ssh host keys
> - no_verify - this old option is abused to indicate desired behavior,
>               what to do while checking host keys:
>               options: - normal: behave as ssh, ask to add a new key,
>                                  reject unknown
>                        - auto_add: add new keys automaticaly, reject
>                                    unknown
>                        - ignore: don't check host key.
> 
> *src/remote/remote_driver.c: -Clean up whitespace between function name
>                               and parentheses.
>                              - Add libssh2 transport scheme
>                              - Add URI components to configure libssh2
>                                transport
> 
> TODO:
> - Add documentation to web-page documents and man pages regarding new
>   URI options
> - Add support for tunelled cleartext passwords?
> ---
>  src/remote/remote_driver.c |  116 ++++++++++++++++++++++++++++++++------------
>  1 files changed, 84 insertions(+), 32 deletions(-)
> 
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 7580477..0787775 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -304,12 +304,14 @@ enum virDrvOpenRemoteFlags {
>   *   - xxx+tcp:///            -> TCP connection to localhost
>   *   - xxx+unix:///           -> UNIX domain socket
>   *   - xxx:///                -> UNIX domain socket
> + *   - xxx+ssh:///            -> SSH connection (legacy)
> + *   - xxx+libssh:///         -> SSH connection (using libssh2)
>   */
>  static int
> -doRemoteOpen (virConnectPtr conn,
> -              struct private_data *priv,
> -              virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> -              unsigned int flags)
> +doRemoteOpen(virConnectPtr conn,
> +             struct private_data *priv,
> +             virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> +             unsigned int flags)

I'd do these refactorings in a separate patch to make future git bisects
easier.
>  {
>      struct qparam_set *vars = NULL;
>      char *transport_str = NULL;
> @@ -317,6 +319,7 @@ doRemoteOpen (virConnectPtr conn,
>          trans_tls,
>          trans_unix,
>          trans_ssh,
> +        trans_libssh,
>          trans_ext,
>          trans_tcp,
>      } transport;
> @@ -345,9 +348,9 @@ doRemoteOpen (virConnectPtr conn,
>                  else
>                      transport = trans_unix;
>              } else {
> -                if (STRCASEEQ (transport_str, "tls"))
> +                if (STRCASEEQ(transport_str, "tls"))
>                      transport = trans_tls;
> -                else if (STRCASEEQ (transport_str, "unix")) {
> +                else if (STRCASEEQ(transport_str, "unix")) {
>                      if (conn->uri->server) {
>                          remoteError(VIR_ERR_INVALID_ARG,
>                                      _("using unix socket and remote "
> @@ -357,16 +360,18 @@ doRemoteOpen (virConnectPtr conn,
>                      } else {
>                          transport = trans_unix;
>                      }
> -                } else if (STRCASEEQ (transport_str, "ssh"))
> +                } else if (STRCASEEQ(transport_str, "ssh"))
>                      transport = trans_ssh;
> -                else if (STRCASEEQ (transport_str, "ext"))
> +                else if (STRCASEEQ(transport_str, "libssh"))
> +                    transport = trans_libssh;
> +                else if (STRCASEEQ(transport_str, "ext"))
>                      transport = trans_ext;
> -                else if (STRCASEEQ (transport_str, "tcp"))
> +                else if (STRCASEEQ(transport_str, "tcp"))
>                      transport = trans_tcp;
>                  else {
>                      remoteError(VIR_ERR_INVALID_ARG, "%s",
>                                  _("remote_open: transport in URL not recognised "
> -                                  "(should be tls|unix|ssh|ext|tcp)"));
> +                                  "(should be tls|unix|ssh|ext|tcp|libssh)"));
>                      return VIR_DRV_OPEN_ERROR;
>                  }
>              }
> @@ -384,6 +389,8 @@ doRemoteOpen (virConnectPtr conn,
>      bool sanity = true, verify = true, tty ATTRIBUTE_UNUSED = true;
>      char *pkipath = NULL, *keyfile = NULL;
> 
> +    char *knownHostsVerify = NULL,  *knownHosts = NULL;
> +
>      /* Return code from this function, and the private data. */
>      int retcode = VIR_DRV_OPEN_ERROR;
> 
> @@ -430,49 +437,57 @@ doRemoteOpen (virConnectPtr conn,
> 
>          for (i = 0; i < vars->n; i++) {
>              var = &vars->p[i];
> -            if (STRCASEEQ (var->name, "name")) {
> +            if (STRCASEEQ(var->name, "name")) {
>                  VIR_FREE(name);
> -                name = strdup (var->value);
> +                name = strdup(var->value);
>                  if (!name) goto out_of_memory;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "command")) {
> +            } else if (STRCASEEQ(var->name, "command")) {
>                  VIR_FREE(command);
> -                command = strdup (var->value);
> +                command = strdup(var->value);
>                  if (!command) goto out_of_memory;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "socket")) {
> +            } else if (STRCASEEQ(var->name, "socket")) {
>                  VIR_FREE(sockname);
> -                sockname = strdup (var->value);
> +                sockname = strdup(var->value);
>                  if (!sockname) goto out_of_memory;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "auth")) {
> +            } else if (STRCASEEQ(var->name, "auth")) {
>                  VIR_FREE(authtype);
> -                authtype = strdup (var->value);
> +                authtype = strdup(var->value);
>                  if (!authtype) goto out_of_memory;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "netcat")) {
> +            } else if (STRCASEEQ(var->name, "netcat")) {
>                  VIR_FREE(netcat);
> -                netcat = strdup (var->value);
> +                netcat = strdup(var->value);
>                  if (!netcat) goto out_of_memory;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "keyfile")) {
> +            } else if (STRCASEEQ(var->name, "keyfile")) {
>                  VIR_FREE(keyfile);
> -                keyfile = strdup (var->value);
> +                keyfile = strdup(var->value);
>                  if (!keyfile) goto out_of_memory;
> -            } else if (STRCASEEQ (var->name, "no_sanity")) {
> +            } else if (STRCASEEQ(var->name, "no_sanity")) {
>                  sanity = atoi(var->value) == 0;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "no_verify")) {
> -                verify = atoi (var->value) == 0;
> +            } else if (STRCASEEQ(var->name, "no_verify")) {
> +                VIR_FREE(knownHostsVerify);
> +                knownHostsVerify = strdup(var->value);
> +                if (!knownHostsVerify) goto out_of_memory;
> +                verify = atoi(var->value) == 0;
>                  var->ignore = 1;
> -            } else if (STRCASEEQ (var->name, "no_tty")) {
> -                tty = atoi (var->value) == 0;
> +            } else if (STRCASEEQ(var->name, "no_tty")) {
> +                tty = atoi(var->value) == 0;
>                  var->ignore = 1;
>              } else if (STRCASEEQ(var->name, "pkipath")) {
>                  VIR_FREE(pkipath);
>                  pkipath = strdup(var->value);
>                  if (!pkipath) goto out_of_memory;
>                  var->ignore = 1;
> +            } else if (STRCASEEQ(var->name, "known_hosts")) {
> +                VIR_FREE(knownHosts);
> +                knownHosts = strdup(var->value);
> +                if (!knownHosts) goto out_of_memory;
> +                var->ignore = 1;
>              } else {
>                  VIR_DEBUG("passing through variable '%s' ('%s') to remote end",
>                        var->name, var->value);
> @@ -565,6 +580,34 @@ doRemoteOpen (virConnectPtr conn,
> 
>          break;
> 
> +    case trans_libssh:
> +        if (!sockname) {
> +            if (flags & VIR_DRV_OPEN_REMOTE_RO)
> +                sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET_RO);
> +            else
> +                sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET);
> +            if (sockname == NULL)
> +                goto out_of_memory;
> +        }
> +
> +        VIR_DEBUG("Starting LibSSH2 session");
> +
> +        priv->client = virNetClientNewLibSSH(priv->hostname,
> +                                             port,
> +                                             username,
> +                                             NULL,
> +                                             netcat,
> +                                             sockname,
> +                                             knownHosts,
> +                                             knownHostsVerify,
> +                                             keyfile,
> +                                             auth);
> +        if (!priv->client)
> +            goto failed;
> +
> +        priv->is_secure = 1;
> +        break;
> +
>  #ifndef WIN32
>      case trans_unix:
>          if (!sockname) {
> @@ -2569,18 +2612,27 @@ remoteAuthenticate (virConnectPtr conn, struct private_data *priv,
>  {
>      struct remote_auth_list_ret ret;
>      int err, type = REMOTE_AUTH_NONE;
> +    virErrorPtr verr;
> 
>      memset(&ret, 0, sizeof ret);
>      err = call (conn, priv, 0,
>                  REMOTE_PROC_AUTH_LIST,
>                  (xdrproc_t) xdr_void, (char *) NULL,
>                  (xdrproc_t) xdr_remote_auth_list_ret, (char *) &ret);
> +
>      if (err < 0) {
> -        virErrorPtr verr = virGetLastError();
> -        if (verr && verr->code == VIR_ERR_NO_SUPPORT) {
> -            /* Missing RPC - old server - ignore */
> -            virResetLastError();
> -            return 0;
> +        if ((verr = virGetLastError())) {
> +            if (verr->code == VIR_ERR_NO_SUPPORT) {
> +                /* Missing RPC - old server - ignore */
> +                virResetLastError();
> +                return 0;
> +            }
> +
> +            if (verr->code == VIR_ERR_LIBSSH_REMOTE_COMMAND) {
> +                virResetLastError();
> +                remoteError(VIR_ERR_LIBSSH_REMOTE_COMMAND, "%s",
> +                            _("Remote daemon is not running or remote command has failed"));
> +            }
>          }
>          return -1;
>      }

Related to 1st patch in the set:
Some users might be using ssh-agent, however, want to select different
auth mechanisms. I'd suggest to allow users to select which auth
mechanism they want to use. If we don't parse ssh configs, we should let
user to choose if he wants keyboard-interactive or ssh-agent or ...;
Otherwise we end up trying to sign in with keys provided by ssh-agent
which doesn't really must have the right ones.
For example, /me uses ssh-agent for git+ssh://libvirt.org but use public
keys for other machines and even keyboard-interactive :)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]