mbox series

[0/3] UUID validation during migration

Message ID 20190827120221.15725-1-yury-kotov@yandex-team.ru (mailing list archive)
Headers show
Series UUID validation during migration | expand

Message

Yury Kotov Aug. 27, 2019, 12:02 p.m. UTC
Hi,

This series adds an UUID validation at the start of the migration
on the target side. The idea is to identify the source of migration.

Possible case of problem:
1. There are 3 servers: A, B and C
2. Server A has a VM 1, server B has a VM 2
3. VM 1 and VM 2 want to migrate to the server C
4. Target of VM 1 starts on the server C and dies too quickly for some reason
5. Target of VM 2 starts just after that and listen the same tcp port X, which
   the target of VM 1 wanted to use
6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source
7. It's possible that migration might be successful (e.g., devices are the same)
8. So, the target of VM 2 is in undefined state

The series adds a capability to prevent successful (by mistake) migration.

The new capability x-validate-uuid only affects the source so that it sends
its UUID to the target. The target will validate the received UUID and stop
the migration if UUIDs are not equal.

Regards,
Yury

Yury Kotov (3):
  migration: Add x-validate-uuid capability
  tests/libqtest: Allow to set expected exit status
  tests/migration: Add a test for x-validate-uuid capability

 migration/migration.c  |   9 +++
 migration/migration.h  |   1 +
 migration/savevm.c     |  45 +++++++++++++
 qapi/migration.json    |   5 +-
 tests/libqtest.c       |  14 ++++-
 tests/libqtest.h       |   9 +++
 tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++---------
 7 files changed, 189 insertions(+), 34 deletions(-)

Comments

Dr. David Alan Gilbert Sept. 3, 2019, 11:21 a.m. UTC | #1
* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Hi,
> 
> This series adds an UUID validation at the start of the migration
> on the target side. The idea is to identify the source of migration.
> 
> Possible case of problem:
> 1. There are 3 servers: A, B and C
> 2. Server A has a VM 1, server B has a VM 2
> 3. VM 1 and VM 2 want to migrate to the server C
> 4. Target of VM 1 starts on the server C and dies too quickly for some reason
> 5. Target of VM 2 starts just after that and listen the same tcp port X, which
>    the target of VM 1 wanted to use
> 6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source

That shouldn't be possible in practice; you specify the destination tcp
port when you start the destination qemu; so unless the management code
that starts the migration is very broken it should know which port it's
migrating to.
However, if it is very broken then this is a good check.

Dave

> 7. It's possible that migration might be successful (e.g., devices are the same)
> 8. So, the target of VM 2 is in undefined state
> 
> The series adds a capability to prevent successful (by mistake) migration.
> 
> The new capability x-validate-uuid only affects the source so that it sends
> its UUID to the target. The target will validate the received UUID and stop
> the migration if UUIDs are not equal.
> 
> Regards,
> Yury
> 
> Yury Kotov (3):
>   migration: Add x-validate-uuid capability
>   tests/libqtest: Allow to set expected exit status
>   tests/migration: Add a test for x-validate-uuid capability
> 
>  migration/migration.c  |   9 +++
>  migration/migration.h  |   1 +
>  migration/savevm.c     |  45 +++++++++++++
>  qapi/migration.json    |   5 +-
>  tests/libqtest.c       |  14 ++++-
>  tests/libqtest.h       |   9 +++
>  tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++---------
>  7 files changed, 189 insertions(+), 34 deletions(-)
> 
> -- 
> 2.22.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Daniel P. Berrangé Sept. 3, 2019, 11:45 a.m. UTC | #2
On Tue, Sep 03, 2019 at 12:21:40PM +0100, Dr. David Alan Gilbert wrote:
> * Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> > Hi,
> > 
> > This series adds an UUID validation at the start of the migration
> > on the target side. The idea is to identify the source of migration.
> > 
> > Possible case of problem:
> > 1. There are 3 servers: A, B and C
> > 2. Server A has a VM 1, server B has a VM 2
> > 3. VM 1 and VM 2 want to migrate to the server C
> > 4. Target of VM 1 starts on the server C and dies too quickly for some reason
> > 5. Target of VM 2 starts just after that and listen the same tcp port X, which
> >    the target of VM 1 wanted to use
> > 6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source
> 
> That shouldn't be possible in practice; you specify the destination tcp
> port when you start the destination qemu; so unless the management code
> that starts the migration is very broken it should know which port it's
> migrating to.
> However, if it is very broken then this is a good check.

In some, but not all, cases allowing the wrong source QEMU to connect to
a target QEMU could be considered a serious security flaw.

Historically live migration security was pretty minimal, to non-existant,
but we do now have the ability todo much better with TLS.

With the way libvirt currently uses TLS for migration, we're just protecting
against a rogue host connecting, as any host with a valid cert gets allowed.

We could do better by using the new ACLs feature to whitelist just the
particular virt host that we know the source VM is on.

This still allows for an accident if libvirt is migrating 2 VMs on the
same source host at the same time.

What's needed is a unique identity for each individual migration operation.

The use of the UUID here is aiming to serve that role.

Assuming libvirt has protected its TLS certificates on the source host,
then this solution is secure. An attacker would need to become root on
the source host to access libvirt's TLS certs, at which point no other
strategy libvirt used instead of UUID would be secure either.


So I think from a security POV, the use of UUID is acceptable.


An alternative would be to not use TLS certificates, and instead use the
TLS pre-shared  keys credential type, generating a new set of PSK creds
for each migration operation.  In this case, UUID would not be required
at all. I don't see this as a reason to reject the UUID check though.
It is reasonable for mgmt apps to have a choice in which approach they
pick.

Regards,
Daniel