From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVeqg-0002GU-Ql for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:30:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVeqd-0000gP-HT for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:30:50 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:34063) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVeqc-0000gA-5d for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:30:47 -0400 Message-ID: <55002751.5080401@huawei.com> Date: Wed, 11 Mar 2015 19:30:25 +0800 From: Gonglei MIME-Version: 1.0 References: <1426004854-4869-1-git-send-email-berrange@redhat.com> <54FF9EFE.8000706@huawei.com> <20150311094529.GB22609@redhat.com> <55002205.2040509@huawei.com> <20150311111057.GF22609@redhat.com> <5500260A.10203@huawei.com> <20150311112707.GG22609@redhat.com> In-Reply-To: <20150311112707.GG22609@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] ui: fix regression in x509verify parameter for VNC server List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, Gerd Hoffmann On 2015/3/11 19:27, Daniel P. Berrange wrote: > On Wed, Mar 11, 2015 at 07:24:58PM +0800, Gonglei wrote: >> On 2015/3/11 19:10, Daniel P. Berrange wrote: >>> On Wed, Mar 11, 2015 at 07:07:49PM +0800, Gonglei wrote: >>>> On 2015/3/11 17:45, Daniel P. Berrange wrote: >>>>> On Wed, Mar 11, 2015 at 09:48:46AM +0800, Gonglei wrote: >>>>>> On 2015/3/11 0:27, Daniel P. Berrange wrote: >>>>>>> The 'x509verify' parameter is documented as taking a path to the >>>>>>> x509 certificates, ie the same syntax as the 'x509' parameter. >>>>>>> >>>>>>> commit 4db14629c38611061fc19ec6927405923de84f08 >>>>>>> Author: Gerd Hoffmann >>>>>>> Date: Tue Sep 16 12:33:03 2014 +0200 >>>>>>> >>>>>>> vnc: switch to QemuOpts, allow multiple servers >>>>>>> >>>>>>> caused a regression by turning 'x509verify' into a boolean >>>>>>> parameter instead. This breaks setup from libvirt and is not >>>>>>> consistent with the docs. >>>>>>> >>>>>>> Signed-off-by: Daniel P. Berrange >>>>>>> --- >>>>>>> ui/vnc.c | 9 +++++++-- >>>>>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/ui/vnc.c b/ui/vnc.c >>>>>>> index 10a2724..37290e7 100644 >>>>>>> --- a/ui/vnc.c >>>>>>> +++ b/ui/vnc.c >>>>>>> @@ -3304,7 +3304,7 @@ static QemuOptsList qemu_vnc_opts = { >>>>>>> .type = QEMU_OPT_BOOL, >>>>>>> },{ >>>>>>> .name = "x509verify", >>>>>>> - .type = QEMU_OPT_BOOL, >>>>>>> + .type = QEMU_OPT_STRING, >>>>>>> },{ >>>>>>> .name = "acl", >>>>>>> .type = QEMU_OPT_BOOL, >>>>>>> @@ -3391,9 +3391,14 @@ void vnc_display_open(const char *id, Error **errp) >>>>>>> #ifdef CONFIG_VNC_TLS >>>>>>> tls = qemu_opt_get_bool(opts, "tls", false); >>>>>>> path = qemu_opt_get(opts, "x509"); >>>>>>> + if (!path) { >>>>>>> + path = qemu_opt_get(opts, "x509verify"); >>>>>>> + if (path) { >>>>>>> + vs->tls.x509verify = true; >>>>>>> + } >>>>>>> + } >>>>>>> if (path) { >>>>>>> x509 = true; >>>>>>> - vs->tls.x509verify = qemu_opt_get_bool(opts, "x509verify", false); >>>>>> >>>>>> We still need to get the x509verify value, while both 'x509' and 'x509verify' >>>>>> are configured, isn't it? >>>>> >>>>> Err, the code 7 lines earlier gets the x509verify value. The x509 and x509verify >>>>> parameters are never to be specified at the same time - either one or the other >>>>> is used. >>>>> >>>> I noticed that there are several places that check x509verify (or vs->tls.x509verify) >>>> value: >>>> >>>> ./ui/vnc-auth-vencrypt.c:85: if (vs->vd->tls.x509verify) { >>>> ./ui/vnc-tls.c:260: if (vs->vd->tls.x509verify) { >>>> ./ui/vnc-tls.c:388: if (vs->vd->tls.x509verify) { >>>> ./ui/vnc.c:3212: vs->tls.x509verify = 0; >>>> ./ui/vnc.c:3306: .name = "x509verify", >>>> ./ui/vnc.c:3396: vs->tls.x509verify = qemu_opt_get_bool(opts, "x509verify", false); >>>> ./ui/vnc.c:3456: if (acl && x509 && vs->tls.x509verify) { >>>> >>>> If only 'x509' parameter is specified, can vnc-tls work after applying this patch? >>>> Am I missing something? >>> >>> x509 only says that the server must provide x509 certs to the client. The >>> x509verify says that the server must also verify the client's certificate. >>> So there are obviously extra codepaths for the latter. >>> >> Okay, thanks for your explanation. :) >> >> But since their different functions, Qemu should support the scenario that >> those two parameters are specified at the same time (by Qemu command >> line) IMHO because they are not mutually exclusive, isn't it? > > It does not make sense to supply both at the same time, as that would > mean you were giving QEMU two sets of x509 certificates for the same > server. Also since x509verify is a superset of x509, there is no reason > to need to specify both at once. > > Regards, > Daniel > Get it, thanks. Regards, -Gonglei