From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGt0-0002jR-Il for qemu-devel@nongnu.org; Mon, 15 Feb 2016 06:00:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVGsw-0005Vw-Pq for qemu-devel@nongnu.org; Mon, 15 Feb 2016 06:00:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33868) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVGsw-0005VI-IP for qemu-devel@nongnu.org; Mon, 15 Feb 2016 06:00:06 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 24C098F4EC for ; Mon, 15 Feb 2016 11:00:06 +0000 (UTC) Date: Mon, 15 Feb 2016 11:00:02 +0000 From: "Daniel P. Berrange" Message-ID: <20160215110002.GC3105@redhat.com> References: <1452599056-27357-1-git-send-email-berrange@redhat.com> <1452599056-27357-21-git-send-email-berrange@redhat.com> <20160212170951.GG2411@work-vm> <20160212172531.GP24725@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160212172531.GP24725@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1 20/22] migration: support TLS encryption with TCP migration backend Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: Amit Shah , qemu-devel@nongnu.org, Juan Quintela On Fri, Feb 12, 2016 at 05:25:31PM +0000, Daniel P. Berrange wrote: > On Fri, Feb 12, 2016 at 05:09:52PM +0000, Dr. David Alan Gilbert wrote: > > * Daniel P. Berrange (berrange@redhat.com) wrote: > > > This extends the TCP migration backend so that it can make use > > > of QIOChannelTLS to provide transparent TLS encryption. To > > > trigger enablement the URI on the incoming and outgoing sides > > > should have 'tls-creds=ID' appended, eg > > > > > > tcp:$HOST:$PORT,tls-creds=ID > > > > > > where ID is the object identifier of a set of TLS credentials > > > previously created using object_add / -object. There is not > > > currently any support for checking the migration client > > > certificate names against ACLs. This is pending a conversion > > > of the ACL code to QOM. > > > > Does that change the option passed or is that just different > > in the way the tls-creds are set up? > > It will mean another new paramter will be added. For example > the above command will become something like this: > > tcp:$HOST:$PORT,tls-creds=ID,auth=ID > > where 'auth=ID' provides the ID of an object implementing > the authentication/authorization check > > > > +typedef struct { > > > + MigrationState *s; > > > + QCryptoTLSCreds *tlscreds; > > > + char *hostname; > > > +} TCPConnectData; > > > + > > > +typedef struct { > > > + MigrationState *s; > > > + QCryptoTLSCreds *tlscreds; > > > +} TCPListenData; > > > + > > > +typedef struct { > > > + MigrationState *s; > > > + QIOChannel *ioc; > > > +} TCPConnectTLSData; > > > + > > > > what makes it TCP specific rather than sharing most of this between transports? i.e. should > > this work for fd migration ? (rdma is probably not reasonable > > giving it's scribbling directly in the other hosts RAM). > > Certainly those types dont really look TCP specific. > > TLS as a protocol doesn't have any limitations, but as part of having > the client validate the x509 certificates, it needs to have a hostname > or IP address to validate against the certificate. This is available > for TCP and RDMA, but there's no user provided hostname for unix, > exec, and fd migration protocols. > > We could extend the syntax for each of those, so that the user could > provide a hostname, and that would then allow us to wire up TLS for > that backend. If we did that, then it would make sense to have the > TLS setup in a separate migration/tls.c file, that we could activate > over all channels. > > > > +static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port, > > > + bool is_listen, > > > + Error **errp) > > > +{ > > > + char *credname = NULL; > > > + Object *creds; > > > + QCryptoTLSCreds *ret; > > > + > > > + credname = tcp_get_opt_str(host_port, "tls-creds"); > > > + if (!credname) { > > > + return NULL; > > > + } > > > > At what point does it get saner just to throw host_port into a qemu_opts > > and let it parse it? > > Possibly quite soon :-) So, having thought about this some more, I think rather than munging parameters onto the end of the URI, it'll make more sense to use the 'migrate-set-parameters' QMP call ie. to enable use of tls migrate-set-parameters tls-creds=tls0 and then to deal with the problem I mention above about not having a hostname available for fd/exec migration, we can also allow migrate-set-parameters tls-hostname=peerhostname which would set the hostname to be used to validate the x509 certificate. This would be quite nice for libvirt, since we can carry on using the fd: migration and establish the connection ourselves, while letting QEMU do the x509 validation. This would in turn motivate moving of the TLS IO Channel creation into a separate file, instead of having it inline in tcp.c. This would in turn let me address the feedback you had previous about possibility of unix: and tcp: code being dealt with in the same file to avoid the code duplication. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|