[Qemu-devel] The unholy encrypted image key mess

[Markus Armbruster]

The encrypted images Saturday afternoon hack is a gift that keeps on
giving.  I wish we could rip it out and bury it deep.  Or that I could
continue to ignore it.  Unfortunately, it looks like I need to touch it
to clean up error propagation in the monitor.  So here goes.

A naive user might expect QEMU to require the password right when he
asks QEMU to open an encrypted image.  But that's not how it works.

Two bits in BlockDriverState apply: encrypted and valid_key.

Both are unset when BlockDriverState has no image, i.e. after bdrv_new()
and bdrv_close().

Opening an image sets encrypted iff the image is encrypted, in driver
method bdrv_open().  valid_key remains unset.  Two states: NOKEY and
NEEDKEY.  NEEDKEY is the troublemaker.

bdrv_set_key() sets a key.  Fails unless encrypted is set, i.e. it
requires state NEEDKEY.  Good.  Also fails if the block driver rejects
the password, but no driver does that.  Otherwise, it sets valid_key.
Call this state GOTKEY.

The user can open an image via command line, HMP commands change,
usb_add (legacy), drive_add, and QMP command blockdev-add.  They all
create BlockDriverStates in states NOKEY or NEEDKEY.

The user can set a key with HMP / QMP command block_passwd.  If it
succeeds, state goes from NEEDKEY to GOTKEY.

You can't unpause a guest while a BlockDriverState is in state NEEDKEY:

* QMP command cont fails if any BlockDriverState is in state NEEDKEY.

* HMP command cont tries to be cute then: it picks a BlockDriverState in
  state NEEDKEY and prompts for its key.  If this is the only one in
  state NEEDKEY, it unpauses the guest as ordered.  Else it silently
  doesn't.  The user is expected to know to rerun cont for the next key
  prompt.

This might make you think that the guest is protected from ever having a
block device backed by a BlockDriverState in state NEEDKEY.  Not true!
Simply open an image while the guest runs, and hot-plug a device backed
by it.  The guest will happily read encrypted garbage from it, and if it
writes anything to it, it's not so encrypted anymore.  To add insult to
injury, the "protection" kicks in next time you pause the guest.

Changing media has its wrinkles, too:

* HMP command change first closes the old image, then opens the new
  image, and if it's encrypted, it asks for a key.  Okay.

* QMP command change first opens the image, then errors out if it's
  encrypted and the password argument is missing, or it's not encrypted
  and the password argument is present.  These two errors aren't really
  failures; the change succeeds just fine.

  Clients can detect the former non-failure error (it's class
  "DeviceEncrypted"), and use block_passwd to go to state GOTKEY.

  Clients can't reliably distinguish the latter non-failure error from
  real errors, because it's class "GenericError".

In any case, callback bdrv_dev_change_media_cb() is delayed for
encrypted images until we enter state GOTKEY.  As afar as I can tell,
nothing stops the device model from reading or writing before it gets
the callback, but that would be a device model bug.

The final funny is device model usb-storage (another one that
desperately needs to be buried deep).  Its init() callback
usb_msd_initfn_storage() tries to be cute when it detects state NEEDKEY.

If it's running in monitor context (say in HMP/QMP command device_add),
it attempts to ask for a key.  In HMP context, it unplugs itself when
this fails (I think).  In QMP context, it behaves similar to change: it
works, but you get a "DeviceEncrypted" error, and the backend remains in
state NEEDKEY.

If it's not running in monitor context, it clears autostart.  No idea
why it does that, and I'm not sure it has any effect.  Opening an
encrypted image clears autostart already, in blockdev_init().

Thankfully, usb-storage is the only device model that messes with keys.

Questions:

1. Should we protect guests from state NEEDKEY?

2. If yes, how?

   Pause the guest when something enters state NEEDKEY?  I'd hate that.

   Fail device_add in state NEEDKEY?  Takes care of hot-plug, and
   cold-plug is already protected by cont.

Other bright ideas?