[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new bloc
From: |
ashish mittal |
Subject: |
Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs" |
Date: |
Fri, 24 Feb 2017 15:30:21 -0800 |
Thanks!
I hope the following is in line with what you suggested -
We will error out in case either of username, secret-id, or password
are missing.
Good case, passing password via a file -
$ ./qemu-io --trace enable=vxhs* --object
secret,id=xvxhspasswd,file=/tmp/some/file/path -c 'read 66000 128k'
'json:{"server.host": "127.0.0.1", "server.port": "9999", "vdisk-id":
"/test.raw", "driver": "vxhs", "user": "ashish", "password-secret":
"xvxhspasswd"}'
address@hidden:vxhs_open_vdiskid Opening vdisk-id /test.raw
address@hidden:vxhs_get_creds User ashish, SecretID
xvxhspasswd, Password address@hidden <=== **** NOTE WILL NOT PRINT
PASSWORD IN FINAL CODE ****
address@hidden:vxhs_open_hostinfo Adding host 127.0.0.1:9999
to BDRVVXHSState
address@hidden:vxhs_get_vdisk_stat vDisk /test.raw stat ioctl
returned size 196616
read 131072/131072 bytes at offset 66000
128 KiB, 1 ops; 0.0012 sec (99.049 MiB/sec and 792.3930 ops/sec)
address@hidden:vxhs_close Closing vdisk /test.raw
Bad case, missing user -
$ ./qemu-io --trace enable=vxhs* --object
secret,id=xvxhspasswd,data=/tmp/some/file/path -c 'read 66000 128k'
'json:{"server.host": "127.0.0.1", "server.port": "9999", "vdisk-id":
"/test.raw", "driver": "vxhs"}'
address@hidden:vxhs_open_vdiskid Opening vdisk-id /test.raw
can't open device json:{"server.host": "127.0.0.1", "server.port":
"9999", "vdisk-id": "/test.raw", "driver": "vxhs"}: please specify the
user for authenticating to target
diff --git a/block/vxhs.c b/block/vxhs.c
index 4f0633e..9b60ddf 100644
--- a/block/vxhs.c
+++ b/block/vxhs.c
@@ -17,12 +17,16 @@
#include "qemu/uri.h"
#include "qapi/error.h"
#include "qemu/uuid.h"
+#include "crypto/secret.h"
#define VXHS_OPT_FILENAME "filename"
#define VXHS_OPT_VDISK_ID "vdisk-id"
#define VXHS_OPT_SERVER "server"
#define VXHS_OPT_HOST "host"
#define VXHS_OPT_PORT "port"
+#define VXHS_OPT_USER "user"
+#define VXHS_OPT_PASSWORD "password"
+#define VXHS_OPT_SECRETID "password-secret"
#define VXHS_UUID_DEF "12345678-1234-1234-1234-123456789012"
QemuUUID qemu_uuid __attribute__ ((weak));
@@ -136,6 +140,22 @@ static QemuOptsList runtime_opts = {
.type = QEMU_OPT_STRING,
.help = "UUID of the VxHS vdisk",
},
+ {
+ .name = VXHS_OPT_USER,
+ .type = QEMU_OPT_STRING,
+ .help = "username for authentication to target",
+ },
+ {
+ .name = VXHS_OPT_PASSWORD,
+ .type = QEMU_OPT_STRING,
+ .help = "password for authentication to target",
+ },
+ {
+ .name = VXHS_OPT_SECRETID,
+ .type = QEMU_OPT_STRING,
+ .help = "ID of the secret providing password for"
+ "authentication to target",
+ },
{ /* end of list */ }
},
};
@@ -257,6 +277,9 @@ static int vxhs_open(BlockDriverState *bs, QDict *options,
const char *server_host_opt;
char *str = NULL;
int ret = 0;
+ const char *user = NULL;
+ const char *secretid = NULL;
+ const char *password = NULL;
ret = vxhs_init_and_ref();
if (ret < 0) {
@@ -320,6 +343,35 @@ static int vxhs_open(BlockDriverState *bs, QDict *options,
goto out;
}
+ /* check if we got username and secretid via the options */
+ user = qemu_opt_get(opts, VXHS_OPT_USER);
+ if (!user) {
+ error_setg(&local_err, "please specify the user for authenticating to "
+ "target");
+ qdict_del(backing_options, str);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ secretid = qemu_opt_get(opts, VXHS_OPT_SECRETID);
+ if (!secretid) {
+ error_setg(&local_err, "please specify the ID of the secret to be "
+ "used for authenticating to target");
+ qdict_del(backing_options, str);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* check if we got password via the --object argument */
+ password = qcrypto_secret_lookup_as_utf8(secretid, &local_err);
+ if (local_err != NULL) {
+ trace_vxhs_get_creds(user, secretid, password);
+ qdict_del(backing_options, str);
+ ret = -EINVAL;
+ goto out;
+ }
+ trace_vxhs_get_creds(user, secretid, password);
+
s->vdisk_hostinfo.host = g_strdup(server_host_opt);
s->vdisk_hostinfo.port = g_ascii_strtoll(qemu_opt_get(tcp_opts,
Regards,
Ashish
On Fri, Feb 24, 2017 at 1:19 AM, Daniel P. Berrange <address@hidden> wrote:
> On Thu, Feb 23, 2017 at 08:19:29PM -0800, ashish mittal wrote:
>> Hi,
>>
>> Just want to check if the following mechanism for accepting the secret
>> password looks OK?
>>
>> We have yet to internally discuss the semantics of how we plan to use
>> the user-ID/password for authentication. This diff is just to
>> understand how I am expected to accept the secret from the command
>> line.
>>
>> Example specifying the username and password:
>>
>> $ ./qemu-io --trace enable=vxhs* --object
>> secret,id=ashish,data=asd888d989s -c 'read 66000 128k'
>> 'json:{"server.host": "127.0.0.1", "server.port": "9999", "vdisk-id":
>> "/test.raw", "driver": "vxhs", "user": "ashish"}'
>> address@hidden:vxhs_open_vdiskid Opening vdisk-id /test.raw
>> address@hidden:vxhs_get_creds SecretID ashish, Password
>> asd888d989s <==== NOTE!!!!!
>> address@hidden:vxhs_open_hostinfo Adding host 127.0.0.1:9999
>> to BDRVVXHSState
>> address@hidden:vxhs_get_vdisk_stat vDisk /test.raw stat ioctl
>> returned size 196616
>> read 131072/131072 bytes at offset 66000
>> 128 KiB, 1 ops; 0.0017 sec (72.844 MiB/sec and 582.7506 ops/sec)
>> address@hidden:vxhs_close Closing vdisk /test.raw
>> address@hidden qemu] 2017-02-23 20:01:45$
>>
>> Example where username and password are missing:
>>
>> address@hidden qemu] 2017-02-23 19:30:16$ ./qemu-io
>> --trace enable=vxhs* -c 'read 66000 128k' 'json:{"server.host":
>> "127.0.0.1", "server.port": "9999", "vdisk-id": "/test.raw", "driver":
>> "vxhs"}'
>> address@hidden:vxhs_open_vdiskid Opening vdisk-id /test.raw
>> address@hidden:vxhs_get_creds SecretID user, Password (null)
>> can't open device json:{"server.host": "127.0.0.1", "server.port":
>> "9999", "vdisk-id": "/test.raw", "driver": "vxhs"}: No secret with id
>> 'user' <==== NOTE!!!!!
>> address@hidden qemu] 2017-02-23 19:30:25$
>
> It is close but not quite correct approach. You're overloading a
> single property to provide two different things - the username
> and as a key to lookup the password secret - you want different
> properties.
>
>
> The 'secret' object only needs to be used for data that must be
> kept private. By convention the block drivers try to use a property
> 'password-secret' to reference the ID of the secret object.
>
> So, as an example, if you had a username "fred" and passwd "letmein"
> then a suitable syntax would be
>
> $ ./qemu-io \
> --object secret,id=vxhspasswd,data=letmein
> -c 'read 66000 128k'
> 'json:{"server.host": "127.0.0.1", "server.port": "9999",
> "vdisk-id": "/test.raw", "driver": "vxhs",
> "user": "fred", "password-secret": "xvxhspasswd"}'
>
> (obviously in real world, we'd not send across the password
> in clear text in the data= parameter of the secret - we'd
> use the file= parameter instead, but its fine for dev testing.
>
>> diff --git a/block/vxhs.c b/block/vxhs.c
>> index 4f0633e..f3e70ce 100644
>> --- a/block/vxhs.c
>> +++ b/block/vxhs.c
>> @@ -17,6 +17,7 @@
>> #include "qemu/uri.h"
>> #include "qapi/error.h"
>> #include "qemu/uuid.h"
>> +#include "crypto/secret.h"
>>
>> #define VXHS_OPT_FILENAME "filename"
>> #define VXHS_OPT_VDISK_ID "vdisk-id"
>> @@ -136,6 +137,14 @@ static QemuOptsList runtime_opts = {
>> .type = QEMU_OPT_STRING,
>> .help = "UUID of the VxHS vdisk",
>> },
>> + {
>> + .name = "user",
>> + .type = QEMU_OPT_STRING,
>> + },
>> + {
>> + .name = "password",
>> + .type = QEMU_OPT_STRING,
>> + },
>> { /* end of list */ }
>> },
>> };
>> @@ -257,6 +266,8 @@ static int vxhs_open(BlockDriverState *bs, QDict
>> *options,
>> const char *server_host_opt;
>> char *str = NULL;
>> int ret = 0;
>> + const char *user = NULL;
>> + const char *password = NULL;
>>
>> ret = vxhs_init_and_ref();
>> if (ret < 0) {
>> @@ -320,6 +331,23 @@ static int vxhs_open(BlockDriverState *bs, QDict
>> *options,
>> goto out;
>> }
>>
>> + /* check if we got username via the options */
>> + user = qemu_opt_get(opts, "user");
>> + if (!user) {
>> + //trace_vxhs_get_creds(user, password);
>> + user = "user";
>
> We should not default any login credentials - if no username was
> provided we should simply not use any username - if the server
> mandates a username this obviously means connection would fail
> and that's ok.
>
>> + //return;
>> + }
>> +
>> + password = qcrypto_secret_lookup_as_utf8(user, &local_err);
>
> Instead of passing 'user' into this method, you need to call
> qemu_opt_get(opts, "password-secret") and use that result
>
>> + if (local_err != NULL) {
>> + trace_vxhs_get_creds(user, password);
>> + qdict_del(backing_options, str);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + trace_vxhs_get_creds(user, password);
>> +
>
> Regards,
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", (continued)
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/20
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Stefan Hajnoczi, 2017/02/20
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", ashish mittal, 2017/02/20
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Stefan Hajnoczi, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Stefan Hajnoczi, 2017/02/22
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/22
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Jeff Cody, 2017/02/22
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", ashish mittal, 2017/02/23
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/24
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs",
ashish mittal <=
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/27
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", ashish mittal, 2017/02/28
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Ketan Nilangekar, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Jeff Cody, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Jeff Cody, 2017/02/21
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/22
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Stefan Hajnoczi, 2017/02/22
- Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs", Daniel P. Berrange, 2017/02/20