[Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Pino Toscano]

These two patches add the password and private key authentication
methods to the ssh block driver, using secure objects for
passwords/passphrases.

Pino Toscano (2):
  ssh: implement password authentication
  ssh: implement private key authentication

 block/ssh.c                  | 133 ++++++++++++++++++++++++++++++++++-
 block/trace-events           |   2 +
 docs/qemu-block-drivers.texi |  15 +++-
 qapi/block-core.json         |  13 +++-
 tests/qemu-iotests/207.out   |   2 +-
 5 files changed, 158 insertions(+), 7 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 1/2] ssh: implement password authentication

[Pino Toscano]

Add a 'password-secret' option which represents the name of an object
with the password of the user.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
---
 block/ssh.c                  | 35 ++++++++++++++++++++++++++++++++---
 block/trace-events           |  1 +
 docs/qemu-block-drivers.texi |  7 +++++--
 qapi/block-core.json         |  6 +++++-
 tests/qemu-iotests/207.out   |  2 +-
 5 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 501933b855..04ae223282 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -43,6 +43,7 @@
 #include "qapi/qmp/qstring.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qobject-output-visitor.h"
+#include "crypto/secret.h"
 #include "trace.h"
 
 /*
@@ -499,7 +500,8 @@ static int check_host_key(BDRVSSHState *s, SshHostKeyCheck *hkc, Error **errp)
     return -EINVAL;
 }
 
-static int authenticate(BDRVSSHState *s, Error **errp)
+static int authenticate(BDRVSSHState *s, BlockdevOptionsSsh *opts,
+                        Error **errp)
 {
     int r, ret;
     int method;
@@ -538,9 +540,35 @@ static int authenticate(BDRVSSHState *s, Error **errp)
         }
     }
 
+    /*
+     * Try to authenticate with password, if available.
+     */
+    if (method & SSH_AUTH_METHOD_PASSWORD && opts->has_password_secret) {
+        char *password;
+
+        trace_ssh_option_secret_object(opts->password_secret);
+        password = qcrypto_secret_lookup_as_utf8(opts->password_secret, errp);
+        if (!password) {
+            ret = -EINVAL;
+            goto out;
+        }
+        r = ssh_userauth_password(s->session, NULL, password);
+        g_free(password);
+        if (r == SSH_AUTH_ERROR) {
+            ret = -EINVAL;
+            session_error_setg(errp, s, "failed to authenticate using "
+                                        "password authentication");
+            goto out;
+        } else if (r == SSH_AUTH_SUCCESS) {
+            /* Authenticated! */
+            ret = 0;
+            goto out;
+        }
+    }
+
     ret = -EPERM;
     error_setg(errp, "failed to authenticate using publickey authentication "
-               "and the identities held by your ssh-agent");
+               "and the identities held by your ssh-agent, or using password");
 
  out:
     return ret;
@@ -785,7 +813,7 @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts,
     }
 
     /* Authenticate. */
-    ret = authenticate(s, errp);
+    ret = authenticate(s, opts, errp);
     if (ret < 0) {
         goto err;
     }
@@ -1376,6 +1404,7 @@ static const char *const ssh_strong_runtime_opts[] = {
     "user",
     "host_key_check",
     "server.",
+    "password-secret",
 
     NULL
 };
diff --git a/block/trace-events b/block/trace-events
index d724df0117..391aae03e6 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -186,6 +186,7 @@ ssh_write_return(ssize_t ret, int sftp_err) "sftp_write returned %zd (sftp error
 ssh_seek(int64_t offset) "seeking to offset=%" PRIi64
 ssh_auth_methods(int methods) "auth methods=0x%x"
 ssh_server_status(int status) "server status=%d"
+ssh_option_secret_object(const char *path) "using password from object %s"
 
 # curl.c
 curl_timer_cb(long timeout_ms) "timer callback timeout_ms %ld"
diff --git a/docs/qemu-block-drivers.texi b/docs/qemu-block-drivers.texi
index 91ab0eceae..c77ef2dd69 100644
--- a/docs/qemu-block-drivers.texi
+++ b/docs/qemu-block-drivers.texi
@@ -771,8 +771,11 @@ matches a specific fingerprint:
 (@code{sha1:} can also be used as a prefix, but note that OpenSSH
 tools only use MD5 to print fingerprints).
 
-Currently authentication must be done using ssh-agent.  Other
-authentication methods may be supported in future.
+The optional @var{password-secret} parameter provides the ID of a
+@code{secret} object that contains the password for authenticating.
+
+Currently authentication must be done using ssh-agent, or providing a
+password.  Other authentication methods may be supported in future.
 
 Note: Many ssh servers do not support an @code{fsync}-style operation.
 The ssh driver cannot guarantee that disk flush requests are
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d43d4f37c..1244562c7b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3223,13 +3223,17 @@
 # @host-key-check:      Defines how and what to check the host key against
 #                       (default: known_hosts)
 #
+# @password-secret:     ID of a QCryptoSecret object providing a password
+#                       for authentication (since 4.2)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsSsh',
   'data': { 'server': 'InetSocketAddress',
             'path': 'str',
             '*user': 'str',
-            '*host-key-check': 'SshHostKeyCheck' } }
+            '*host-key-check': 'SshHostKeyCheck',
+            '*password-secret': 'str' } }
 
 
 ##
diff --git a/tests/qemu-iotests/207.out b/tests/qemu-iotests/207.out
index 1239d9d648..5bfdf626b9 100644
--- a/tests/qemu-iotests/207.out
+++ b/tests/qemu-iotests/207.out
@@ -74,7 +74,7 @@ Job failed: failed to open remote file '/this/is/not/an/existing/path': SFTP ser
 
 {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"mode": "none"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}, "user": "invalid user"}, "size": 4194304}}}
 {"return": {}}
-Job failed: failed to authenticate using publickey authentication and the identities held by your ssh-agent
+Job failed: failed to authenticate using publickey authentication and the identities held by your ssh-agent, or using password
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
 
-- 
2.21.0

[Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Pino Toscano]

Add a 'private-key' option which represents the path of a private key
to use for authentication, and 'private-key-secret' as the name of an
object with its passphrase.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
---
 block/ssh.c                  | 98 ++++++++++++++++++++++++++++++++++++
 block/trace-events           |  1 +
 docs/qemu-block-drivers.texi | 12 ++++-
 qapi/block-core.json         |  9 +++-
 4 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 04ae223282..1b7c1f4108 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -500,6 +500,89 @@ static int check_host_key(BDRVSSHState *s, SshHostKeyCheck *hkc, Error **errp)
     return -EINVAL;
 }
 
+static int authenticate_privkey(BDRVSSHState *s, BlockdevOptionsSsh *opts,
+                                Error **errp)
+{
+    int err;
+    int ret;
+    char *pubkey_file = NULL;
+    ssh_key public_key = NULL;
+    ssh_key private_key = NULL;
+    char *passphrase;
+
+    pubkey_file = g_strdup_printf("%s.pub", opts->private_key);
+
+    /* load the private key */
+    trace_ssh_auth_key_passphrase(opts->private_key_secret, opts->private_key);
+    passphrase = qcrypto_secret_lookup_as_utf8(opts->private_key_secret, errp);
+    if (!passphrase) {
+        err = SSH_AUTH_ERROR;
+        goto error;
+    }
+    ret = ssh_pki_import_privkey_file(opts->private_key, passphrase,
+                                      NULL, NULL, &private_key);
+    g_free(passphrase);
+    if (ret == SSH_EOF) {
+        error_setg(errp, "Cannot read private key '%s'", opts->private_key);
+        err = SSH_AUTH_ERROR;
+        goto error;
+    } else if (ret == SSH_ERROR) {
+        error_setg(errp,
+                   "Cannot open private key '%s', maybe the passphrase is "
+                   "wrong",
+                   opts->private_key);
+        err = SSH_AUTH_ERROR;
+        goto error;
+    }
+
+    /* try to open the public part of the private key */
+    ret = ssh_pki_import_pubkey_file(pubkey_file, &public_key);
+    if (ret == SSH_ERROR) {
+        error_setg(errp, "Cannot read public key '%s'", pubkey_file);
+        err = SSH_AUTH_ERROR;
+        goto error;
+    } else if (ret == SSH_EOF) {
+        /* create the public key from the private key */
+        ret = ssh_pki_export_privkey_to_pubkey(private_key, &public_key);
+        if (ret == SSH_ERROR) {
+            error_setg(errp,
+                       "Cannot export the public key from the private key "
+                       "'%s'",
+                       opts->private_key);
+            err = SSH_AUTH_ERROR;
+            goto error;
+        }
+    }
+
+    ret = ssh_userauth_try_publickey(s->session, NULL, public_key);
+    if (ret != SSH_AUTH_SUCCESS) {
+        err = SSH_AUTH_DENIED;
+        goto error;
+    }
+
+    ret = ssh_userauth_publickey(s->session, NULL, private_key);
+    if (ret != SSH_AUTH_SUCCESS) {
+        err = SSH_AUTH_DENIED;
+        goto error;
+    }
+
+    ssh_key_free(private_key);
+    ssh_key_free(public_key);
+    g_free(pubkey_file);
+
+    return SSH_AUTH_SUCCESS;
+
+ error:
+    if (private_key) {
+        ssh_key_free(private_key);
+    }
+    if (public_key) {
+        ssh_key_free(public_key);
+    }
+    g_free(pubkey_file);
+    return err;
+}
+
 static int authenticate(BDRVSSHState *s, BlockdevOptionsSsh *opts,
                         Error **errp)
 {
@@ -538,6 +621,21 @@ static int authenticate(BDRVSSHState *s, BlockdevOptionsSsh *opts,
             ret = 0;
             goto out;
         }
+
+        /*
+         * Try to authenticate with private key, if available.
+         */
+        if (opts->has_private_key && opts->has_private_key_secret) {
+            r = authenticate_privkey(s, opts, errp);
+            if (r == SSH_AUTH_ERROR) {
+                ret = -EINVAL;
+                goto out;
+            } else if (r == SSH_AUTH_SUCCESS) {
+                /* Authenticated! */
+                ret = 0;
+                goto out;
+            }
+        }
     }
 
     /*
diff --git a/block/trace-events b/block/trace-events
index 391aae03e6..ccb51b9992 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -187,6 +187,7 @@ ssh_seek(int64_t offset) "seeking to offset=%" PRIi64
 ssh_auth_methods(int methods) "auth methods=0x%x"
 ssh_server_status(int status) "server status=%d"
 ssh_option_secret_object(const char *path) "using password from object %s"
+ssh_auth_key_passphrase(const char *path, const char *key) "using passphrase from object %s for private key %s"
 
 # curl.c
 curl_timer_cb(long timeout_ms) "timer callback timeout_ms %ld"
diff --git a/docs/qemu-block-drivers.texi b/docs/qemu-block-drivers.texi
index c77ef2dd69..5513bf261c 100644
--- a/docs/qemu-block-drivers.texi
+++ b/docs/qemu-block-drivers.texi
@@ -774,8 +774,16 @@ tools only use MD5 to print fingerprints).
 The optional @var{password-secret} parameter provides the ID of a
 @code{secret} object that contains the password for authenticating.
 
-Currently authentication must be done using ssh-agent, or providing a
-password.  Other authentication methods may be supported in future.
+The optional @var{private-key} parameter provides the path to the
+private key for authenticating.
+
+The optional @var{private-key-secret} parameter provides the ID of a
+@code{secret} object that contains the passphrase of the private key
+specified as @var{private-key} for authenticating.
+
+Currently authentication must be done using ssh-agent, providing a
+private key with its passphrase, or providing a password.
+Other authentication methods may be supported in future.
 
 Note: Many ssh servers do not support an @code{fsync}-style operation.
 The ssh driver cannot guarantee that disk flush requests are
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1244562c7b..e873f8934d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3226,6 +3226,11 @@
 # @password-secret:     ID of a QCryptoSecret object providing a password
 #                       for authentication (since 4.2)
 #
+# @private-key:         path to the private key (since 4.2)
+#
+# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
+#                       for 'private-key' (since 4.2)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsSsh',
@@ -3233,7 +3238,9 @@
             'path': 'str',
             '*user': 'str',
             '*host-key-check': 'SshHostKeyCheck',
-            '*password-secret': 'str' } }
+            '*password-secret': 'str',
+            '*private-key': 'str',
+            '*private-key-secret': 'str' } }
 
 
 ##
-- 
2.21.0

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 2566 bytes --]

On 7/26/19 9:09 AM, Pino Toscano wrote:
> Add a 'private-key' option which represents the path of a private key
> to use for authentication, and 'private-key-secret' as the name of an
> object with its passphrase.
> 
> Signed-off-by: Pino Toscano <ptoscano@redhat.com>

> +++ b/qapi/block-core.json
> @@ -3226,6 +3226,11 @@
>  # @password-secret:     ID of a QCryptoSecret object providing a password
>  #                       for authentication (since 4.2)
>  #
> +# @private-key:         path to the private key (since 4.2)
> +#
> +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
> +#                       for 'private-key' (since 4.2)

Is password-secret intended to be mutually-exclusive with
private-key/private-key-secret?  If so, this should probably utilize an
enum for a discriminator
{ 'enum': 'SshAuth', 'data': ['ssh-agent', 'password', 'private'key'] }

then update BlockdevOptionsSsh to be a union type with an optional
discriminator (defaulting to ssh-agent) for back-compat, where
'auth':'ssh-agent' needs no further fields, 'auth':'password' adds in a
'secret' field for use as password, or where 'auth':'private-key' adds
in both 'key-file' and 'secret' for use as the two pieces needed for
private key use.

Markus may have other suggestions on how best to represent this sort of
union type in QAPI.

> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsSsh',
> @@ -3233,7 +3238,9 @@
>              'path': 'str',
>              '*user': 'str',
>              '*host-key-check': 'SshHostKeyCheck',
> -            '*password-secret': 'str' } }
> +            '*password-secret': 'str',
> +            '*private-key': 'str',
> +            '*private-key-secret': 'str' } }
>  
>  
>  ##
> 

On a different topic, how much of this work overlaps with the nbdkit ssh
plugin? Should we be duplicating efforts with both projects supporting
ssh natively, or is it worth considering getting qemu out of the ssh
business and instead connecting to an nbd device provided by nbdkit
connecting to ssh?  (For comparison, we've already decided that nbdkit
does not plan on writing a qcow2 plugin, because it defers to qemu to be
the expert there; or in the other direction, qemu-nbd has deprecated its
partial support for exposing only a partition of a disk in favor of
qemu-nbd having much more partition support through its filters)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Richard W.M. Jones]

On Fri, Jul 26, 2019 at 09:24:34AM -0500, Eric Blake wrote:
> On a different topic, how much of this work overlaps with the nbdkit ssh
> plugin? Should we be duplicating efforts with both projects supporting
> ssh natively, or is it worth considering getting qemu out of the ssh
> business and instead connecting to an nbd device provided by nbdkit
> connecting to ssh?  (For comparison, we've already decided that nbdkit
> does not plan on writing a qcow2 plugin, because it defers to qemu to be
> the expert there; or in the other direction, qemu-nbd has deprecated its
> partial support for exposing only a partition of a disk in favor of
> qemu-nbd having much more partition support through its filters)

I think it would be good if libvirt could handle this usage, so it
would set up the nbdkit process, set up seccomp or SELinux to confine
it, and kill nbdkit afterwards.

See also:

https://rwmj.wordpress.com/2018/10/30/split-block-drivers-from-qemu-with-nbdkit/

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Pino Toscano]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
> On 7/26/19 9:09 AM, Pino Toscano wrote:
> > Add a 'private-key' option which represents the path of a private key
> > to use for authentication, and 'private-key-secret' as the name of an
> > object with its passphrase.
> > 
> > Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> 
> > +++ b/qapi/block-core.json
> > @@ -3226,6 +3226,11 @@
> >  # @password-secret:     ID of a QCryptoSecret object providing a password
> >  #                       for authentication (since 4.2)
> >  #
> > +# @private-key:         path to the private key (since 4.2)
> > +#
> > +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
> > +#                       for 'private-key' (since 4.2)
> 
> Is password-secret intended to be mutually-exclusive with
> private-key/private-key-secret?

My initial thought was to allow users to specify data for all the
authentication methods possible.  Either ways (all of them, or a single
one) are fine for me.

-- 
Pino Toscano

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Markus Armbruster]

Pino Toscano <ptoscano@redhat.com> writes:

> On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
>> On 7/26/19 9:09 AM, Pino Toscano wrote:
>> > Add a 'private-key' option which represents the path of a private key
>> > to use for authentication, and 'private-key-secret' as the name of an
>> > object with its passphrase.
>> > 
>> > Signed-off-by: Pino Toscano <ptoscano@redhat.com>
>> 
>> > +++ b/qapi/block-core.json
>> > @@ -3226,6 +3226,11 @@
>> >  # @password-secret:     ID of a QCryptoSecret object providing a password
>> >  #                       for authentication (since 4.2)
>> >  #
>> > +# @private-key:         path to the private key (since 4.2)
>> > +#
>> > +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
>> > +#                       for 'private-key' (since 4.2)
>> 
>> Is password-secret intended to be mutually-exclusive with
>> private-key/private-key-secret?
>
> My initial thought was to allow users to specify data for all the
> authentication methods possible.  Either ways (all of them, or a single
> one) are fine for me.

How does this work at the libssh level?  Can you configure multiple
authentication methods, and let negotiation pick the one to be used?

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Pino Toscano]

[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]

On Monday, 29 July 2019 12:57:40 CEST Markus Armbruster wrote:
> Pino Toscano <ptoscano@redhat.com> writes:
> 
> > On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
> >> On 7/26/19 9:09 AM, Pino Toscano wrote:
> >> > Add a 'private-key' option which represents the path of a private key
> >> > to use for authentication, and 'private-key-secret' as the name of an
> >> > object with its passphrase.
> >> > 
> >> > Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> >> 
> >> > +++ b/qapi/block-core.json
> >> > @@ -3226,6 +3226,11 @@
> >> >  # @password-secret:     ID of a QCryptoSecret object providing a password
> >> >  #                       for authentication (since 4.2)
> >> >  #
> >> > +# @private-key:         path to the private key (since 4.2)
> >> > +#
> >> > +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
> >> > +#                       for 'private-key' (since 4.2)
> >> 
> >> Is password-secret intended to be mutually-exclusive with
> >> private-key/private-key-secret?
> >
> > My initial thought was to allow users to specify data for all the
> > authentication methods possible.  Either ways (all of them, or a single
> > one) are fine for me.
> 
> How does this work at the libssh level?  Can you configure multiple
> authentication methods, and let negotiation pick the one to be used?

The remote servers sends all the authentication methods supported, and
the user of libssh can decide which one(s) to attempt.  See for example
the small tutorial in the libssh documentation:
http://api.libssh.org/stable/libssh_tutor_authentication.html

-- 
Pino Toscano

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Markus Armbruster]

Pino Toscano <ptoscano@redhat.com> writes:

> On Monday, 29 July 2019 12:57:40 CEST Markus Armbruster wrote:
>> Pino Toscano <ptoscano@redhat.com> writes:
>> 
>> > On Friday, 26 July 2019 16:24:34 CEST Eric Blake wrote:
>> >> On 7/26/19 9:09 AM, Pino Toscano wrote:
>> >> > Add a 'private-key' option which represents the path of a private key
>> >> > to use for authentication, and 'private-key-secret' as the name of an
>> >> > object with its passphrase.
>> >> > 
>> >> > Signed-off-by: Pino Toscano <ptoscano@redhat.com>
>> >> 
>> >> > +++ b/qapi/block-core.json
>> >> > @@ -3226,6 +3226,11 @@
>> >> >  # @password-secret:     ID of a QCryptoSecret object providing a password
>> >> >  #                       for authentication (since 4.2)
>> >> >  #
>> >> > +# @private-key:         path to the private key (since 4.2)
>> >> > +#
>> >> > +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
>> >> > +#                       for 'private-key' (since 4.2)
>> >> 
>> >> Is password-secret intended to be mutually-exclusive with
>> >> private-key/private-key-secret?
>> >
>> > My initial thought was to allow users to specify data for all the
>> > authentication methods possible.  Either ways (all of them, or a single
>> > one) are fine for me.
>> 
>> How does this work at the libssh level?  Can you configure multiple
>> authentication methods, and let negotiation pick the one to be used?
>
> The remote servers sends all the authentication methods supported, and
> the user of libssh can decide which one(s) to attempt.  See for example
> the small tutorial in the libssh documentation:
> http://api.libssh.org/stable/libssh_tutor_authentication.html

SSH server and client negotiate: the server offers methods, the client
tries offered methods it likes one after the other.

This means we want QMP to let us configure which methods we like, along
with whatever data is necessary to actually try them.

In short, we don't want mutually exclusive here.

Correct?

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

Am 26.07.2019 um 16:24 hat Eric Blake geschrieben:
> On 7/26/19 9:09 AM, Pino Toscano wrote:
> > Add a 'private-key' option which represents the path of a private key
> > to use for authentication, and 'private-key-secret' as the name of an
> > object with its passphrase.
> > 
> > Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> 
> > +++ b/qapi/block-core.json
> > @@ -3226,6 +3226,11 @@
> >  # @password-secret:     ID of a QCryptoSecret object providing a password
> >  #                       for authentication (since 4.2)
> >  #
> > +# @private-key:         path to the private key (since 4.2)
> > +#
> > +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
> > +#                       for 'private-key' (since 4.2)
> 
> Is password-secret intended to be mutually-exclusive with
> private-key/private-key-secret?  If so, this should probably utilize an
> enum for a discriminator
> { 'enum': 'SshAuth', 'data': ['ssh-agent', 'password', 'private'key'] }
> 
> then update BlockdevOptionsSsh to be a union type with an optional
> discriminator (defaulting to ssh-agent) for back-compat, where
> 'auth':'ssh-agent' needs no further fields, 'auth':'password' adds in a
> 'secret' field for use as password, or where 'auth':'private-key' adds
> in both 'key-file' and 'secret' for use as the two pieces needed for
> private key use.

Can we actually support optional discriminators when we don't have
defaults in the QAPI schema yet?

> On a different topic, how much of this work overlaps with the nbdkit ssh
> plugin? Should we be duplicating efforts with both projects supporting
> ssh natively, or is it worth considering getting qemu out of the ssh
> business and instead connecting to an nbd device provided by nbdkit
> connecting to ssh?

ssh behaves essentially like a filesystem whereas NBD behaves like a
block device. This is especially relevant for everything related to the
file size. As far as I know, using an image format like qcow2 that wants
to grow the image file isn't possible over NBD, whereas I expect it to
work with the ssh block driver.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 2440 bytes --]

On 29.07.19 13:08, Kevin Wolf wrote:
> Am 26.07.2019 um 16:24 hat Eric Blake geschrieben:
>> On 7/26/19 9:09 AM, Pino Toscano wrote:
>>> Add a 'private-key' option which represents the path of a private key
>>> to use for authentication, and 'private-key-secret' as the name of an
>>> object with its passphrase.
>>>
>>> Signed-off-by: Pino Toscano <ptoscano@redhat.com>
>>
>>> +++ b/qapi/block-core.json
>>> @@ -3226,6 +3226,11 @@
>>>  # @password-secret:     ID of a QCryptoSecret object providing a password
>>>  #                       for authentication (since 4.2)
>>>  #
>>> +# @private-key:         path to the private key (since 4.2)
>>> +#
>>> +# @private-key-secret:  ID of a QCryptoSecret object providing the passphrase
>>> +#                       for 'private-key' (since 4.2)
>>
>> Is password-secret intended to be mutually-exclusive with
>> private-key/private-key-secret?  If so, this should probably utilize an
>> enum for a discriminator
>> { 'enum': 'SshAuth', 'data': ['ssh-agent', 'password', 'private'key'] }
>>
>> then update BlockdevOptionsSsh to be a union type with an optional
>> discriminator (defaulting to ssh-agent) for back-compat, where
>> 'auth':'ssh-agent' needs no further fields, 'auth':'password' adds in a
>> 'secret' field for use as password, or where 'auth':'private-key' adds
>> in both 'key-file' and 'secret' for use as the two pieces needed for
>> private key use.
> 
> Can we actually support optional discriminators when we don't have
> defaults in the QAPI schema yet?

Just chiming in here, because I wanted to throw in that v4 of my “block:
Try to create well-typed json:{} filenames​” series adds that.

>> On a different topic, how much of this work overlaps with the nbdkit ssh
>> plugin? Should we be duplicating efforts with both projects supporting
>> ssh natively, or is it worth considering getting qemu out of the ssh
>> business and instead connecting to an nbd device provided by nbdkit
>> connecting to ssh?
> 
> ssh behaves essentially like a filesystem whereas NBD behaves like a
> block device. This is especially relevant for everything related to the
> file size. As far as I know, using an image format like qcow2 that wants
> to grow the image file isn't possible over NBD, whereas I expect it to
> work with the ssh block driver.

Just using sshfs and file-posix would seem simpler to me, by the way.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 2/2] ssh: implement private key authentication

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1080 bytes --]

On 7/29/19 6:08 AM, Kevin Wolf wrote:

>> On a different topic, how much of this work overlaps with the nbdkit ssh
>> plugin? Should we be duplicating efforts with both projects supporting
>> ssh natively, or is it worth considering getting qemu out of the ssh
>> business and instead connecting to an nbd device provided by nbdkit
>> connecting to ssh?
> 
> ssh behaves essentially like a filesystem whereas NBD behaves like a
> block device. This is especially relevant for everything related to the
> file size. As far as I know, using an image format like qcow2 that wants
> to grow the image file isn't possible over NBD, whereas I expect it to
> work with the ssh block driver.

Resizing NBD devices isn't available yet, but it is rapidly moving
higher on my list of TODO items to implement as an extension (the ideas
for how it should work have been tossed around, but having code to back
up those ideas is the next step).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Richard W.M. Jones]

On Fri, Jul 26, 2019 at 04:09:52PM +0200, Pino Toscano wrote:
> These two patches add the password and private key authentication
> methods to the ssh block driver, using secure objects for
> passwords/passphrases.

I was attempting to test this but couldn't work out the full command
line to use it (with qemu-img).  I got as far as:

$ ./qemu-img convert -p 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "..." }' /var/tmp/root

I guess the secret should be specified using --object, but at that
point I gave up.

Could do with documentation, even if merely in the commit message or a
blog post.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Pino Toscano]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On Friday, 26 July 2019 16:27:11 CEST Richard W.M. Jones wrote:
> On Fri, Jul 26, 2019 at 04:09:52PM +0200, Pino Toscano wrote:
> > These two patches add the password and private key authentication
> > methods to the ssh block driver, using secure objects for
> > passwords/passphrases.
> 
> I was attempting to test this but couldn't work out the full command
> line to use it (with qemu-img).  I got as far as:
> 
> $ ./qemu-img convert -p 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "..." }' /var/tmp/root
> 
> I guess the secret should be specified using --object, but at that
> point I gave up.

Almost there :) add e.g.
  --object 'secret,id=sec0,file=passwd'
as parameter for the convert command (so after it, not before), and then
set 'sec0' as value for file.password-secret.  Of course 'sec0' is
arbitrary, any other QEMU id will do.

A long helpful comment in include/crypto/secret.h explains the basics
of the crypto objects.

-- 
Pino Toscano

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Richard W.M. Jones]

On Fri, Jul 26, 2019 at 04:45:03PM +0200, Pino Toscano wrote:
> On Friday, 26 July 2019 16:27:11 CEST Richard W.M. Jones wrote:
> > On Fri, Jul 26, 2019 at 04:09:52PM +0200, Pino Toscano wrote:
> > > These two patches add the password and private key authentication
> > > methods to the ssh block driver, using secure objects for
> > > passwords/passphrases.
> > 
> > I was attempting to test this but couldn't work out the full command
> > line to use it (with qemu-img).  I got as far as:
> > 
> > $ ./qemu-img convert -p 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "..." }' /var/tmp/root
> > 
> > I guess the secret should be specified using --object, but at that
> > point I gave up.
> 
> Almost there :) add e.g.
>   --object 'secret,id=sec0,file=passwd'
> as parameter for the convert command (so after it, not before), and then
> set 'sec0' as value for file.password-secret.  Of course 'sec0' is
> arbitrary, any other QEMU id will do.
> 
> A long helpful comment in include/crypto/secret.h explains the basics
> of the crypto objects.

OK, the password part of this patch does work, so:

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

Re: [Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1707 bytes --]

On 7/26/19 9:45 AM, Pino Toscano wrote:
> On Friday, 26 July 2019 16:27:11 CEST Richard W.M. Jones wrote:
>> On Fri, Jul 26, 2019 at 04:09:52PM +0200, Pino Toscano wrote:
>>> These two patches add the password and private key authentication
>>> methods to the ssh block driver, using secure objects for
>>> passwords/passphrases.
>>
>> I was attempting to test this but couldn't work out the full command
>> line to use it (with qemu-img).  I got as far as:
>>
>> $ ./qemu-img convert -p 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "..." }' /var/tmp/root
>>
>> I guess the secret should be specified using --object, but at that
>> point I gave up.
> 
> Almost there :) add e.g.
>   --object 'secret,id=sec0,file=passwd'
> as parameter for the convert command (so after it, not before), and then
> set 'sec0' as value for file.password-secret.  Of course 'sec0' is
> arbitrary, any other QEMU id will do.
> 
> A long helpful comment in include/crypto/secret.h explains the basics
> of the crypto objects.

That is useful information, but even more useful if you amend the commit
message to include a working example command line rather than making
readers chase down the docs :)

Untested, but piecing together what I know from my work on qemu-nbd
encryption, it seems like this should be a starting point for such a
command:

qemu-img convert -p --imageopts --object secret,id=sec0,file=passwd \
  driver=ssh,host=devr7,path=/var/tmp/root,password-secret=sec0 \
  /var/tmp/copy

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Richard W.M. Jones]

On Fri, Jul 26, 2019 at 10:06:43AM -0500, Eric Blake wrote:
> On 7/26/19 9:45 AM, Pino Toscano wrote:
> > On Friday, 26 July 2019 16:27:11 CEST Richard W.M. Jones wrote:
> >> On Fri, Jul 26, 2019 at 04:09:52PM +0200, Pino Toscano wrote:
> >>> These two patches add the password and private key authentication
> >>> methods to the ssh block driver, using secure objects for
> >>> passwords/passphrases.
> >>
> >> I was attempting to test this but couldn't work out the full command
> >> line to use it (with qemu-img).  I got as far as:
> >>
> >> $ ./qemu-img convert -p 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "..." }' /var/tmp/root
> >>
> >> I guess the secret should be specified using --object, but at that
> >> point I gave up.
> > 
> > Almost there :) add e.g.
> >   --object 'secret,id=sec0,file=passwd'
> > as parameter for the convert command (so after it, not before), and then
> > set 'sec0' as value for file.password-secret.  Of course 'sec0' is
> > arbitrary, any other QEMU id will do.
> > 
> > A long helpful comment in include/crypto/secret.h explains the basics
> > of the crypto objects.
> 
> That is useful information, but even more useful if you amend the commit
> message to include a working example command line rather than making
> readers chase down the docs :)
> 
> Untested, but piecing together what I know from my work on qemu-nbd
> encryption, it seems like this should be a starting point for such a
> command:
> 
> qemu-img convert -p --imageopts --object secret,id=sec0,file=passwd \
>   driver=ssh,host=devr7,path=/var/tmp/root,password-secret=sec0 \
>   /var/tmp/copy

--imageopts isn't necessary.  This was the command that worked for me:

unset SSH_AUTH_SOCK; ./qemu-img convert -p --object 'secret,id=sec0,file=/tmp/passwd' 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "sec0" }' /var/tmp/root

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [Qemu-devel] [PATCH 0/2] ssh: add password and privkey auth methods

[Daniel P. Berrangé]

On Fri, Jul 26, 2019 at 04:35:27PM +0100, Richard W.M. Jones wrote:
> On Fri, Jul 26, 2019 at 10:06:43AM -0500, Eric Blake wrote:
> > On 7/26/19 9:45 AM, Pino Toscano wrote:
> > > On Friday, 26 July 2019 16:27:11 CEST Richard W.M. Jones wrote:
> > >> On Fri, Jul 26, 2019 at 04:09:52PM +0200, Pino Toscano wrote:
> > >>> These two patches add the password and private key authentication
> > >>> methods to the ssh block driver, using secure objects for
> > >>> passwords/passphrases.
> > >>
> > >> I was attempting to test this but couldn't work out the full command
> > >> line to use it (with qemu-img).  I got as far as:
> > >>
> > >> $ ./qemu-img convert -p 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "..." }' /var/tmp/root
> > >>
> > >> I guess the secret should be specified using --object, but at that
> > >> point I gave up.
> > > 
> > > Almost there :) add e.g.
> > >   --object 'secret,id=sec0,file=passwd'
> > > as parameter for the convert command (so after it, not before), and then
> > > set 'sec0' as value for file.password-secret.  Of course 'sec0' is
> > > arbitrary, any other QEMU id will do.
> > > 
> > > A long helpful comment in include/crypto/secret.h explains the basics
> > > of the crypto objects.
> > 
> > That is useful information, but even more useful if you amend the commit
> > message to include a working example command line rather than making
> > readers chase down the docs :)
> > 
> > Untested, but piecing together what I know from my work on qemu-nbd
> > encryption, it seems like this should be a starting point for such a
> > command:
> > 
> > qemu-img convert -p --imageopts --object secret,id=sec0,file=passwd \
> >   driver=ssh,host=devr7,path=/var/tmp/root,password-secret=sec0 \
> >   /var/tmp/copy
> 
> --imageopts isn't necessary.  This was the command that worked for me:
> 
> unset SSH_AUTH_SOCK; ./qemu-img convert -p --object 'secret,id=sec0,file=/tmp/passwd' 'json:{ "file.driver": "ssh", "file.host": "devr7", "file.path": "/var/tmp/root", "file.password-secret": "sec0" }' /var/tmp/root

Right you didn't need --imageopts because you used the json filename
syntax.

--imageopts is for telling it to intepret the filename as key,value pairs
as in Eric's example.

json & imageopts syntaxes are equally expressive, so pick which you
prefer.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|