* [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused
@ 2014-03-14 8:22 Markus Armbruster
2014-03-14 12:28 ` Eric Blake
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Markus Armbruster @ 2014-03-14 8:22 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, famz, stefanha
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]
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
--
1.8.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused
2014-03-14 8:22 [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused Markus Armbruster
@ 2014-03-14 12:28 ` Eric Blake
2014-03-14 12:51 ` Paolo Bonzini
2014-03-14 15:24 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Eric Blake @ 2014-03-14 12:28 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kwolf, pbonzini, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]
On 03/14/2014 02:22 AM, Markus Armbruster wrote:
> 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: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused
2014-03-14 8:22 [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused Markus Armbruster
2014-03-14 12:28 ` Eric Blake
@ 2014-03-14 12:51 ` Paolo Bonzini
2014-03-14 15:24 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-03-14 12:51 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kwolf, famz, stefanha
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused
2014-03-14 8:22 [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused Markus Armbruster
2014-03-14 12:28 ` Eric Blake
2014-03-14 12:51 ` Paolo Bonzini
@ 2014-03-14 15:24 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2014-03-14 15:24 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, pbonzini, famz, qemu-devel
On Fri, Mar 14, 2014 at 09:22:48AM +0100, Markus Armbruster wrote:
> 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]
>
> 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
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-14 15:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-14 8:22 [Qemu-devel] [PATCH v2 for-2.0] blockdev: Refuse to open encrypted image unless paused Markus Armbruster
2014-03-14 12:28 ` Eric Blake
2014-03-14 12:51 ` Paolo Bonzini
2014-03-14 15:24 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.