From patchwork Fri Feb 24 23:30:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ashish Mittal X-Patchwork-Id: 732306 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vVS6N37WDz9s7R for ; Sat, 25 Feb 2017 10:31:24 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="iRn9PWP7"; dkim-atps=neutral Received: from localhost ([::1]:40456 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1chPKc-0005HY-0k for incoming@patchwork.ozlabs.org; Fri, 24 Feb 2017 18:31:22 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1chPJh-0004jN-GM for qemu-devel@nongnu.org; Fri, 24 Feb 2017 18:30:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1chPJf-0004Mu-Os for qemu-devel@nongnu.org; Fri, 24 Feb 2017 18:30:25 -0500 Received: from mail-io0-x241.google.com ([2607:f8b0:4001:c06::241]:34867) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1chPJf-0004Mi-JG for qemu-devel@nongnu.org; Fri, 24 Feb 2017 18:30:23 -0500 Received: by mail-io0-x241.google.com with SMTP id v13so517253iov.2 for ; Fri, 24 Feb 2017 15:30:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=1UOctsFPFBRmlAe8hKf6tf1HOreGWEz4M9rWh/VQHSQ=; b=iRn9PWP7QC+tlwPduukzaQbKzLlYf8OkhfdJfsoNHWbFyTRp9XVk6qEKbIQaYz1v/I jwNuzlm2WbJ2V6HBUfnUzJVW/QFy3mlDmlD86BcmalU3Wh8Yydy40/aUnJsTRp3VL22c +gNMw3d5iGgWZ4NRp92hy+ypt2urTxOoT37MtJCNpBH59zOGRDwfEVymIe/uT1pBIcZ8 129awB1UODTyB/vMzls1/kvCptT2o47SSJVtgoF5yBxvc/tQjQ7NhaXEl02e0D0pBMYQ /+yG2YNUCRC7J717lg+wW64NA3ejiD3lZCK2inpvmbU+XNbh/4/a5RXqgba7yqLMNAr/ D42w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=1UOctsFPFBRmlAe8hKf6tf1HOreGWEz4M9rWh/VQHSQ=; b=FUp34SfgGrx61fvu75qdkD3y0b4NHtlK9uJfPAw1RpEXPFaYfqPgLrHOP1n5lKhM42 /WO56Mo3vyCfXIZj5VwoCaXtUN+a/nrH5bl7MFbeoPh4Sx4u2gbUvua8KhSlpWx2VQOf KgtjOdTTUp4iSi92HG9dgNL3KHIXPeejI7Gnj5v+Sk+NwD4Xd1fmWRwFINdovQROm/io OWPXD3lMRla2d8AGO+tuwAwIrc0cn8ZTOWgmc7K+xpnrEqf/lcnxlKOF2igqUqsVLja3 oqtKKKhkglQpZr1Mv5AD3rVgdcvIZ9B6aJr2DmtaHi0kJZqTFblNMRid23vmDdgpNU90 ptYQ== X-Gm-Message-State: AMke39lKrOGgMxEuhfWuwwDxjwRbVRVzKxdpwjSgQ8ECC/sLV0sHXvdaM2f5ISPh93sctpAK7TY13aZMPtLSOw== X-Received: by 10.107.19.77 with SMTP id b74mr4942336ioj.77.1487979022815; Fri, 24 Feb 2017 15:30:22 -0800 (PST) MIME-Version: 1.0 Received: by 10.79.13.17 with HTTP; Fri, 24 Feb 2017 15:30:21 -0800 (PST) In-Reply-To: <20170224091916.GD3702@redhat.com> References: <20170217214215.GK19045@localhost.localdomain> <20170220110235.GD21255@stefanha-x1.localdomain> <20170221105918.GA22731@stefanha-x1.localdomain> <20170221113353.GC17041@redhat.com> <20170222140920.GA10201@stefanha-x1.localdomain> <20170222142230.GR28937@redhat.com> <20170222144407.GS19045@localhost.localdomain> <20170224091916.GD3702@redhat.com> From: ashish mittal Date: Fri, 24 Feb 2017 15:30:21 -0800 Message-ID: To: "Daniel P. Berrange" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:4001:c06::241 Subject: Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs" X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Peter Maydell , "Venkatesha M.G." , Ashish Mittal , Fam Zheng , Nitin Jerath , Stefan Hajnoczi , Jeff Cody , qemu-devel , Rakesh Ranjan , Markus Armbruster , Suraj Singh , Ketan Nilangekar , Abhijit Dey , John Ferlan , Paolo Bonzini , Buddhi Madhav Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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"}' 1132@1487977829.151064:vxhs_open_vdiskid Opening vdisk-id /test.raw 1132@1487977829.151141:vxhs_get_creds User ashish, SecretID xvxhspasswd, Password Str0ngP@ssw0rd <=== **** NOTE WILL NOT PRINT PASSWORD IN FINAL CODE **** 1132@1487977829.151168:vxhs_open_hostinfo Adding host 127.0.0.1:9999 to BDRVVXHSState 1132@1487977829.173062: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) 1132@1487977829.175141: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"}' 1310@1487978547.771234: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 Regards, Ashish On Fri, Feb 24, 2017 at 1:19 AM, Daniel P. Berrange 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"}' >> 17396@1487908905.546890:vxhs_open_vdiskid Opening vdisk-id /test.raw >> 17396@1487908905.546969:vxhs_get_creds SecretID ashish, Password >> asd888d989s <==== NOTE!!!!! >> 17396@1487908905.546994:vxhs_open_hostinfo Adding host 127.0.0.1:9999 >> to BDRVVXHSState >> 17396@1487908905.568370: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) >> 17396@1487908905.570917:vxhs_close Closing vdisk /test.raw >> [root@rhel72ga-build-upstream qemu] 2017-02-23 20:01:45$ >> >> Example where username and password are missing: >> >> [root@rhel72ga-build-upstream 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"}' >> 16120@1487907025.251167:vxhs_open_vdiskid Opening vdisk-id /test.raw >> 16120@1487907025.251245: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!!!!! >> [root@rhel72ga-build-upstream 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/ :| 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,