Re: [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused

[Paolo Bonzini <pbonzini@redhat.com>]

Il 14/03/2014 09:22, Markus Armbruster ha scritto:
> Opening an encrypted image takes an additional step: setting the key.
> Between open and the key set, the image must not be used.
>
> We have some protection against accidental use in place: you can't
> unpause a guest while we're missing keys.  You can, however, hot-plug
> block devices lacking keys into a running guest just fine, or insert
> media lacking keys.  In the latter case, notifying the guest of the
> insert is delayed until the key is set, which may suffice to protect
> at least some guests in common usage.
>
> This patch makes the protection apply in more cases, in a rather
> heavy-handed way: it doesn't let you open encrypted images unless
> we're in a paused state.
>
> It doesn't extend the protection to users other than the guest (block
> jobs?).  Use of runstate_check() from block.c is disgusting.  Best I
> can do right now.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> v2:
> * Don't break -incoming [Paolo]
> * Update qemu-iotests/087 [Fam]

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

>  block.c                    |  9 ++++++++-
>  stubs/Makefile.objs        |  1 +
>  stubs/runstate-check.c     |  6 ++++++
>  tests/qemu-iotests/087     | 17 +++++++++++++++++
>  tests/qemu-iotests/087.out | 11 ++++++++++-
>  5 files changed, 42 insertions(+), 2 deletions(-)
>  create mode 100644 stubs/runstate-check.c
>
> diff --git a/block.c b/block.c
> index fae50c9..53f5b44 100644
> --- a/block.c
> +++ b/block.c
> @@ -1388,12 +1388,19 @@ done:
>          ret = -EINVAL;
>          goto close_and_fail;
>      }
> -    QDECREF(options);
>
>      if (!bdrv_key_required(bs)) {
>          bdrv_dev_change_media_cb(bs, true);
> +    } else if (!runstate_check(RUN_STATE_PRELAUNCH)
> +               && !runstate_check(RUN_STATE_INMIGRATE)
> +               && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
> +        error_setg(errp,
> +                   "Guest must be stopped for opening of encrypted image");
> +        ret = -EBUSY;
> +        goto close_and_fail;
>      }
>
> +    QDECREF(options);
>      *pbs = bs;
>      return 0;
>
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 59c5a54..5ed1d38 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -20,6 +20,7 @@ stub-obj-y += mon-set-error.o
>  stub-obj-y += pci-drive-hot-add.o
>  stub-obj-y += qtest.o
>  stub-obj-y += reset.o
> +stub-obj-y += runstate-check.o
>  stub-obj-y += set-fd-handler.o
>  stub-obj-y += slirp.o
>  stub-obj-y += sysbus.o
> diff --git a/stubs/runstate-check.c b/stubs/runstate-check.c
> new file mode 100644
> index 0000000..bd2e375
> --- /dev/null
> +++ b/stubs/runstate-check.c
> @@ -0,0 +1,6 @@
> +#include "sysemu/sysemu.h"
> +
> +bool runstate_check(RunState state)
> +{
> +    return state == RUN_STATE_PRELAUNCH;
> +}
> diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
> index 53b6c43..a38bb70 100755
> --- a/tests/qemu-iotests/087
> +++ b/tests/qemu-iotests/087
> @@ -99,6 +99,23 @@ echo === Encrypted image ===
>  echo
>
>  _make_test_img -o encryption=on $size
> +run_qemu -S <<EOF
> +{ "execute": "qmp_capabilities" }
> +{ "execute": "blockdev-add",
> +  "arguments": {
> +      "options": {
> +        "driver": "$IMGFMT",
> +        "id": "disk",
> +        "file": {
> +            "driver": "file",
> +            "filename": "$TEST_IMG"
> +        }
> +      }
> +    }
> +  }
> +{ "execute": "quit" }
> +EOF
> +
>  run_qemu <<EOF
>  { "execute": "qmp_capabilities" }
>  { "execute": "blockdev-add",
> diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> index b871032..e65dcdf 100644
> --- a/tests/qemu-iotests/087.out
> +++ b/tests/qemu-iotests/087.out
> @@ -28,7 +28,7 @@ QMP_VERSION
>  === Encrypted image ===
>
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
> -Testing:
> +Testing: -S
>  QMP_VERSION
>  {"return": {}}
>  {"error": {"class": "GenericError", "desc": "blockdev-add doesn't support encrypted devices"}}
> @@ -37,4 +37,13 @@ QMP_VERSION
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
>
> +Testing:
> +QMP_VERSION
> +{"return": {}}
> +{"error": {"class": "GenericError", "desc": "could not open disk image disk: Guest must be stopped for opening of encrypted image"}}
> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
> +
>  *** done
>