[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 00/14] Add TLS support for migration



On Tue, Feb 28, 2017 at 02:48:19PM +0100, Jiri Denemark wrote:
> On Thu, Feb 23, 2017 at 13:42:02 -0500, John Ferlan wrote:
> > AFAIU - removal of the objects would remove the migration tls-creds,
> > tls-hostname settings.
> 
> This would be a very strange behavior.
> 
> > I left the cfg->migrateTLS in for now - it's very simple to remove, but
> > there would still need to be a key on something to ensure the migrateTLS
> > environment has been properly configured since that would mean the default
> > environment would need to be used/configured. Setting up the default
> > environment is keyed off having the migrateTLS defined. That's all part
> > of the qemu_conf reading logic.
> 
> I still don't see the value in keeping migrate_tls option in qemu.conf.
> If we want to check the environment is setup correctly we should check
> the certificates are in place rather than relying on the option. The
> error reported when migrate_tls is set, but the environment is not
> prepared is:
> 
>     internal error: unable to execute QEMU command 'object-add': Unable
>     to access credentials /etc/pki/qemu/ca-cert.pem: No such file or
>     directory
> 
> Not ideal but at least it names the file which is missing.
> 
> > John Ferlan (14):
> >   qemu: Introduce qemuDomainSecretInfoNew
> >   qemu: Introduce qemuDomainSecretMigratePrepare
> >   qemu: Move exit monitor calls in failure paths
> >   qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
> >   qemu: Refactor qemuDomainGetChardevTLSObjects to converge code
> >   qemu: Move qemuDomainSecretChardevPrepare call
> >   qemu: Move qemuDomainPrepareChardevSourceTLS call
> >   qemu: Introduce qemuDomainGetTLSObjects
> 
> The eight patches above are mostly OK except for some small issues.
> 
> But the rest of the series needs more work...
> 
> >   qemu: Add TLS params to _qemuMonitorMigrationParams
> >   Add new migration flag VIR_MIGRATE_TLS
> >   qemu: Create #define for TLS configuration setup.
> >   conf: Introduce migrate_tls_x509_cert_dir
> >   qemu: Set up the migrate TLS objects for target
> >   qemu: Set up the migration TLS objects for source
> 
> The code should check whether QEMU actually supports TLS migration,
> otherwise you will get
> 
>     internal error: unable to execute QEMU command
>     'migrate-set-parameters': Invalid parameter 'tls-creds'
> 
> As I mentioned in my v1 review, we should always set the parameters if
> QEMU supports them to make sure they don't contain any leftovers from a
> previous migration. I also said we should properly clean the TLS stuff
> somewhere inside qemuProcessRecoverMigration*. And if we do it in
> addition to properly cleaning up everything in the Finish and Confirm
> phases, we should not need to store migrateTLS in the status XML. We can
> just always do the cleanup if QEMU supports TLS migration.
> 
> 
> Anyway, TLS migration doesn't work even if it is properly configured:
> 
>     operation failed: migration job: unexpectedly failed
> 
> And the destination QEMU log contains:
> 
>     qemu-system-x86_64: No TLS credentials with id 'objmigrate_tls0'
> 
> The reason is an interesting flow of QMP commands issued by libvirt
> during the Prepare phase:
> 
>     {
>         "execute": "object-add",
>         "arguments": {
>             "qom-type": "tls-creds-x509",
>             "id": "objmigrate_tls0",
>             "props": {
>                 "dir": "/etc/pki/libvirt",
>                 "endpoint": "server",
>                 "verify-peer": false
>             }
>         },
>         "id": "libvirt-21"
>     }
> 
>     {
>         "execute": "migrate-set-parameters",
>         "arguments": {
>             "tls-creds": "objmigrate_tls0"
>         },
>         "id": "libvirt-26"
>     }
> 
>     {
>         "execute": "migrate-incoming",
>         "arguments": {
>             "uri": "tcp:[::]:49152"
>         },
>         "id": "libvirt-27"
>     }
> 
>     {
>         "execute": "object-del",
>         "arguments": {
>             "id": "objmigrate_tls0"
>         },
>         "id": "libvirt-28"
>     }
> 
> The error reported after you deleted the objmigrate_tls0 object doesn't
> seem to confirm your idea about migration parameters begin unset when
> the object is removed.

The problem here is that 'migrate-incoming' merely sets up a TCP
listener, it doesn't actually initiate a connection or TLS handshake.
The object-del thus deletes the TLS creds before the src QEMU has had
a chance to even connect.

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/ :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]