* [Qemu-devel] [PATCH 01/10] qcrypto: add suport for amend options
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
@ 2019-08-30 20:55 ` Maxim Levitsky
2019-09-06 13:40 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management Maxim Levitsky
` (8 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
This adds the qcrypto_amend_options and corresponding
crypto driver callbacks for the for encrypted
key managedment
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block.c | 31 +++++++++++++++++++++++++++++++
crypto/blockpriv.h | 8 ++++++++
include/crypto/block.h | 22 ++++++++++++++++++++++
3 files changed, 61 insertions(+)
diff --git a/crypto/block.c b/crypto/block.c
index 325752871c..14b684de7f 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -115,6 +115,37 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
}
+int qcrypto_block_amend_options(QCryptoBlock *block,
+ QCryptoBlockReadFunc readfunc,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ QCryptoBlockCreateOptions *options,
+ bool force,
+ Error **errp)
+{
+ if (options->format != block->format) {
+ error_setg(errp,
+ "Its not possible to change encryption format with amend interface");
+ return -1;
+ }
+
+ if (!block->driver->amend) {
+ error_setg(errp,
+ "Crypto format %s doesn't support format options amendment",
+ QCryptoBlockFormat_str(block->format));
+ return -1;
+ }
+
+ return block->driver->amend(block,
+ readfunc,
+ writefunc,
+ opaque,
+ options,
+ force,
+ errp);
+}
+
+
QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
Error **errp)
{
diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 71c59cb542..c18a4e0b43 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -62,6 +62,14 @@ struct QCryptoBlockDriver {
void *opaque,
Error **errp);
+ int (*amend)(QCryptoBlock *block,
+ QCryptoBlockReadFunc readfunc,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ QCryptoBlockCreateOptions *options,
+ bool force,
+ Error **errp);
+
int (*get_info)(QCryptoBlock *block,
QCryptoBlockInfo *info,
Error **errp);
diff --git a/include/crypto/block.h b/include/crypto/block.h
index d49d2c2da9..777fd51ebe 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -144,6 +144,28 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
void *opaque,
Error **errp);
+/**
+ * qcrypto_block_amend_options:
+ * @block: the block encryption object
+ *
+ * @readfunc: callback for reading data from the volume header
+ * @writefunc: callback for writing data to the volume header
+ * @opaque: data to pass to @readfunc and @writefunc
+ * @options: the new/amended encryption options
+ * @force: hint for the driver to allow unsafe operation
+ * @errp: error pointer
+ *
+ * Changes the crypto options of the encryption format
+ *
+ */
+int qcrypto_block_amend_options(QCryptoBlock *block,
+ QCryptoBlockReadFunc readfunc,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ QCryptoBlockCreateOptions *options,
+ bool force,
+ Error **errp);
+
/**
* qcrypto_block_get_info:
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 01/10] qcrypto: add suport for amend options
2019-08-30 20:55 ` [Qemu-devel] [PATCH 01/10] qcrypto: add suport for amend options Maxim Levitsky
@ 2019-09-06 13:40 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 13:40 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:55:59PM +0300, Maxim Levitsky wrote:
> This adds the qcrypto_amend_options and corresponding
> crypto driver callbacks for the for encrypted
> key managedment
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block.c | 31 +++++++++++++++++++++++++++++++
> crypto/blockpriv.h | 8 ++++++++
> include/crypto/block.h | 22 ++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
2019-08-30 20:55 ` [Qemu-devel] [PATCH 01/10] qcrypto: add suport for amend options Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 13:49 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the " Maxim Levitsky
` (7 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Now you can specify which slot to put the encryption key to
Plus add 'active' option which will let user erase the key secret
instead of adding it.
Check that it is true for creation
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
block/crypto.c | 2 ++
block/crypto.h | 16 +++++++++++
block/qcow2.c | 2 ++
crypto/block-luks.c | 26 +++++++++++++++---
qapi/crypto.json | 19 ++++++++++++++
tests/qemu-iotests/082.out | 54 ++++++++++++++++++++++++++++++++++++++
6 files changed, 115 insertions(+), 4 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index 6e822c6e50..a6a3e1f1d8 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -144,6 +144,8 @@ static QemuOptsList block_crypto_create_opts_luks = {
BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
+ BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(""),
+ BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(""),
{ /* end of list */ }
},
};
diff --git a/block/crypto.h b/block/crypto.h
index b935695e79..05cc43d9bc 100644
--- a/block/crypto.h
+++ b/block/crypto.h
@@ -35,12 +35,14 @@
"ID of the secret that provides the AES encryption key")
#define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
+#define BLOCK_CRYPTO_OPT_LUKS_SLOT "slot"
#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg"
#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
#define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
#define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
+#define BLOCK_CRYPTO_OPT_LUKS_ACTIVE "active"
#define BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(prefix) \
BLOCK_CRYPTO_OPT_DEF_KEY_SECRET(prefix, \
@@ -88,6 +90,20 @@
.help = "Time to spend in PBKDF in milliseconds", \
}
+#define BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(prefix) \
+ { \
+ .name = prefix BLOCK_CRYPTO_OPT_LUKS_SLOT, \
+ .type = QEMU_OPT_NUMBER, \
+ .help = "Controls the slot where the secret is added/erased", \
+ }
+
+#define BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(prefix) \
+ { \
+ .name = prefix BLOCK_CRYPTO_OPT_LUKS_ACTIVE, \
+ .type = QEMU_OPT_BOOL, \
+ .help = "Controls if the added secret is added or erased", \
+ }
+
QCryptoBlockCreateOptions *
block_crypto_create_opts_init(QDict *opts, Error **errp);
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c5a4859f7..be4a5063e5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5167,6 +5167,8 @@ static QemuOptsList qcow2_create_opts = {
BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."),
BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."),
BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."),
+ BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT("encrypt."),
+ BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE("encrypt."),
{
.name = BLOCK_OPT_CLUSTER_SIZE,
.type = QEMU_OPT_SIZE,
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 3af137e364..ba20d55246 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1230,6 +1230,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
const char *hash_alg;
g_autofree char *cipher_mode_spec = NULL;
uint64_t iters;
+ unsigned int slot_idx = 0;
memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
if (!luks_opts.has_iter_time) {
@@ -1263,12 +1264,30 @@ qcrypto_block_luks_create(QCryptoBlock *block,
luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg;
luks->hash_alg = luks_opts.hash_alg;
+ if (luks_opts.has_active && !luks_opts.active) {
+ error_setg(errp,
+ "For image creation, the added secret must be active!");
+ goto error;
+
+ }
+
+ if (luks_opts.has_slot) {
+ if (luks_opts.slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ||
+ luks_opts.slot < 0) {
+ error_setg(errp,
+ "Invalid slot %" PRId64 " is specified",
+ luks_opts.slot);
+ goto error;
+ }
+ slot_idx = (unsigned int)luks_opts.slot;
+ }
+
/* Note we're allowing ivgen_hash_alg to be set even for
* non-essiv iv generators that don't need a hash. It will
* be silently ignored, for compatibility with dm-crypt */
- if (!options->u.luks.key_secret) {
+ if (!luks_opts.has_key_secret) {
error_setg(errp, "Parameter '%skey-secret' is required for cipher",
optprefix ? optprefix : "");
goto error;
@@ -1473,11 +1492,10 @@ qcrypto_block_luks_create(QCryptoBlock *block,
goto error;
}
-
- /* populate the slot 0 with the password encrypted master key*/
+ /* populate one of the slots with the password encrypted master key*/
/* This will also store the header */
if (qcrypto_block_luks_store_key(block,
- 0,
+ slot_idx,
password,
masterkey,
luks_opts.iter_time,
diff --git a/qapi/crypto.json b/qapi/crypto.json
index b2a4cff683..9b83a70634 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -190,6 +190,20 @@
# Currently defaults to 'sha256'
# @hash-alg: the master key hash algorithm
# Currently defaults to 'sha256'
+#
+# @active: Should the new secret be added (true) or erased (false)
+# (amend only, since 4.2)
+#
+# @slot: The slot in which to put/erase the secret
+# if not given, will select first free slot for secret addtion
+# and erase all matching keyslots for erase. except last one
+# (optional, since 4.2)
+#
+# @unlock-secret: The secret to use to unlock the image
+# If not given, will use the secret that was used
+# when opening the image.
+# (optional, for amend only, since 4.2)
+#
# @iter-time: number of milliseconds to spend in
# PBKDF passphrase processing. Currently defaults
# to 2000. (since 2.8)
@@ -201,7 +215,12 @@
'*cipher-mode': 'QCryptoCipherMode',
'*ivgen-alg': 'QCryptoIVGenAlgorithm',
'*ivgen-hash-alg': 'QCryptoHashAlgorithm',
+
'*hash-alg': 'QCryptoHashAlgorithm',
+ '*active' : 'bool',
+ '*slot': 'int',
+ '*unlock-secret': 'str',
+
'*iter-time': 'int'}}
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 9d4ed4dc9d..5651a0b953 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -50,6 +50,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -58,6 +59,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -73,6 +75,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -81,6 +84,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -96,6 +100,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -104,6 +109,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -119,6 +125,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -127,6 +134,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -142,6 +150,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -150,6 +159,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -165,6 +175,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -173,6 +184,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -188,6 +200,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -196,6 +209,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -211,6 +225,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -219,6 +234,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -249,6 +265,7 @@ Supported qcow2 options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -257,6 +274,7 @@ Supported qcow2 options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -330,6 +348,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -338,6 +357,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -353,6 +373,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -361,6 +382,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -376,6 +398,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -384,6 +407,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -399,6 +423,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -407,6 +432,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -422,6 +448,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -430,6 +457,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -445,6 +473,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -453,6 +482,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -468,6 +498,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -476,6 +507,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -491,6 +523,7 @@ Supported options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -499,6 +532,7 @@ Supported options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
@@ -529,6 +563,7 @@ Supported qcow2 options:
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -537,6 +572,7 @@ Supported qcow2 options:
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -621,6 +657,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -629,6 +666,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -645,6 +683,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -653,6 +692,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -669,6 +709,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -677,6 +718,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -693,6 +735,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -701,6 +744,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -717,6 +761,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -725,6 +770,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -741,6 +787,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -749,6 +796,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -765,6 +813,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -773,6 +822,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -789,6 +839,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -797,6 +848,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
@@ -830,6 +882,7 @@ Creation options for 'qcow2':
compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
data_file=<str> - File name of an external data file
data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
+ encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
encrypt.cipher-mode=<str> - Name of encryption cipher mode
encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
@@ -838,6 +891,7 @@ Creation options for 'qcow2':
encrypt.ivgen-alg=<str> - Name of IV generator algorithm
encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
+ encrypt.slot=<num> - Controls the slot where the secret is added/erased
encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
lazy_refcounts=<bool (on/off)> - Postpone refcount updates
preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management
2019-08-30 20:56 ` [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management Maxim Levitsky
@ 2019-09-06 13:49 ` Daniel P. Berrangé
2019-09-06 13:57 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 13:49 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:00PM +0300, Maxim Levitsky wrote:
> Now you can specify which slot to put the encryption key to
> Plus add 'active' option which will let user erase the key secret
> instead of adding it.
> Check that it is true for creation
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> block/crypto.c | 2 ++
> block/crypto.h | 16 +++++++++++
> block/qcow2.c | 2 ++
> crypto/block-luks.c | 26 +++++++++++++++---
> qapi/crypto.json | 19 ++++++++++++++
> tests/qemu-iotests/082.out | 54 ++++++++++++++++++++++++++++++++++++++
> 6 files changed, 115 insertions(+), 4 deletions(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index 6e822c6e50..a6a3e1f1d8 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -144,6 +144,8 @@ static QemuOptsList block_crypto_create_opts_luks = {
> BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
> BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
> BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
> + BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(""),
> + BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(""),
> { /* end of list */ }
> },
> };
> diff --git a/block/crypto.h b/block/crypto.h
> index b935695e79..05cc43d9bc 100644
> --- a/block/crypto.h
> +++ b/block/crypto.h
> @@ -35,12 +35,14 @@
> "ID of the secret that provides the AES encryption key")
>
> #define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
> +#define BLOCK_CRYPTO_OPT_LUKS_SLOT "slot"
> #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
> #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
> #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg"
> #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
> #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
> #define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
> +#define BLOCK_CRYPTO_OPT_LUKS_ACTIVE "active"
>
> #define BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(prefix) \
> BLOCK_CRYPTO_OPT_DEF_KEY_SECRET(prefix, \
> @@ -88,6 +90,20 @@
> .help = "Time to spend in PBKDF in milliseconds", \
> }
>
> +#define BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(prefix) \
> + { \
> + .name = prefix BLOCK_CRYPTO_OPT_LUKS_SLOT, \
> + .type = QEMU_OPT_NUMBER, \
> + .help = "Controls the slot where the secret is added/erased", \
> + }
> +
> +#define BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(prefix) \
> + { \
> + .name = prefix BLOCK_CRYPTO_OPT_LUKS_ACTIVE, \
> + .type = QEMU_OPT_BOOL, \
> + .help = "Controls if the added secret is added or erased", \
> + }
Do we actually need the "active" property for initial
creation. I think its only needed for amend, so perhaps
we shuold not register this at all ?
> +
> QCryptoBlockCreateOptions *
> block_crypto_create_opts_init(QDict *opts, Error **errp);
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7c5a4859f7..be4a5063e5 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -5167,6 +5167,8 @@ static QemuOptsList qcow2_create_opts = {
> BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."),
> BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."),
> BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."),
> + BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT("encrypt."),
> + BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE("encrypt."),
> {
> .name = BLOCK_OPT_CLUSTER_SIZE,
> .type = QEMU_OPT_SIZE,
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 3af137e364..ba20d55246 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -1230,6 +1230,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> const char *hash_alg;
> g_autofree char *cipher_mode_spec = NULL;
> uint64_t iters;
> + unsigned int slot_idx = 0;
>
> memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
> if (!luks_opts.has_iter_time) {
> @@ -1263,12 +1264,30 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> luks->ivgen_hash_alg = luks_opts.ivgen_hash_alg;
> luks->hash_alg = luks_opts.hash_alg;
>
> + if (luks_opts.has_active && !luks_opts.active) {
> + error_setg(errp,
> + "For image creation, the added secret must be active!");
> + goto error;
> +
> + }
> +
> + if (luks_opts.has_slot) {
> + if (luks_opts.slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ||
> + luks_opts.slot < 0) {
> + error_setg(errp,
> + "Invalid slot %" PRId64 " is specified",
> + luks_opts.slot);
> + goto error;
> + }
> + slot_idx = (unsigned int)luks_opts.slot;
> + }
> +
>
> /* Note we're allowing ivgen_hash_alg to be set even for
> * non-essiv iv generators that don't need a hash. It will
> * be silently ignored, for compatibility with dm-crypt */
>
> - if (!options->u.luks.key_secret) {
> + if (!luks_opts.has_key_secret) {
> error_setg(errp, "Parameter '%skey-secret' is required for cipher",
> optprefix ? optprefix : "");
> goto error;
> @@ -1473,11 +1492,10 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> goto error;
> }
>
> -
> - /* populate the slot 0 with the password encrypted master key*/
> + /* populate one of the slots with the password encrypted master key*/
> /* This will also store the header */
> if (qcrypto_block_luks_store_key(block,
> - 0,
> + slot_idx,
> password,
> masterkey,
> luks_opts.iter_time,
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index b2a4cff683..9b83a70634 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -190,6 +190,20 @@
> # Currently defaults to 'sha256'
> # @hash-alg: the master key hash algorithm
> # Currently defaults to 'sha256'
> +#
> +# @active: Should the new secret be added (true) or erased (false)
> +# (amend only, since 4.2)
> +#
> +# @slot: The slot in which to put/erase the secret
> +# if not given, will select first free slot for secret addtion
> +# and erase all matching keyslots for erase. except last one
> +# (optional, since 4.2)
> +#
> +# @unlock-secret: The secret to use to unlock the image
> +# If not given, will use the secret that was used
> +# when opening the image.
> +# (optional, for amend only, since 4.2)
> +#
> # @iter-time: number of milliseconds to spend in
> # PBKDF passphrase processing. Currently defaults
> # to 2000. (since 2.8)
> @@ -201,7 +215,12 @@
> '*cipher-mode': 'QCryptoCipherMode',
> '*ivgen-alg': 'QCryptoIVGenAlgorithm',
> '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> +
> '*hash-alg': 'QCryptoHashAlgorithm',
> + '*active' : 'bool',
> + '*slot': 'int',
> + '*unlock-secret': 'str',
> +
> '*iter-time': 'int'}}
>
>
> diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
> index 9d4ed4dc9d..5651a0b953 100644
> --- a/tests/qemu-iotests/082.out
> +++ b/tests/qemu-iotests/082.out
> @@ -50,6 +50,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -58,6 +59,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -73,6 +75,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -81,6 +84,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -96,6 +100,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -104,6 +109,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -119,6 +125,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -127,6 +134,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -142,6 +150,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -150,6 +159,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -165,6 +175,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -173,6 +184,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -188,6 +200,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -196,6 +209,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -211,6 +225,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -219,6 +234,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -249,6 +265,7 @@ Supported qcow2 options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -257,6 +274,7 @@ Supported qcow2 options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -330,6 +348,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -338,6 +357,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -353,6 +373,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -361,6 +382,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -376,6 +398,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -384,6 +407,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -399,6 +423,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -407,6 +432,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -422,6 +448,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -430,6 +457,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -445,6 +473,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -453,6 +482,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -468,6 +498,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -476,6 +507,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -491,6 +523,7 @@ Supported options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -499,6 +532,7 @@ Supported options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> nocow=<bool (on/off)> - Turn off copy-on-write (valid only on btrfs)
> @@ -529,6 +563,7 @@ Supported qcow2 options:
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -537,6 +572,7 @@ Supported qcow2 options:
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -621,6 +657,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -629,6 +666,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -645,6 +683,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -653,6 +692,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -669,6 +709,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -677,6 +718,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -693,6 +735,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -701,6 +744,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -717,6 +761,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -725,6 +770,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -741,6 +787,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -749,6 +796,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -765,6 +813,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -773,6 +822,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -789,6 +839,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -797,6 +848,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> @@ -830,6 +882,7 @@ Creation options for 'qcow2':
> compat=<str> - Compatibility level (v2 [0.10] or v3 [1.1])
> data_file=<str> - File name of an external data file
> data_file_raw=<bool (on/off)> - The external data file must stay valid as a raw image
> + encrypt.active=<bool (on/off)> - Controls if the added secret is added or erased
> encrypt.cipher-alg=<str> - Name of encryption cipher algorithm
> encrypt.cipher-mode=<str> - Name of encryption cipher mode
> encrypt.format=<str> - Encrypt the image, format choices: 'aes', 'luks'
> @@ -838,6 +891,7 @@ Creation options for 'qcow2':
> encrypt.ivgen-alg=<str> - Name of IV generator algorithm
> encrypt.ivgen-hash-alg=<str> - Name of IV generator hash algorithm
> encrypt.key-secret=<str> - ID of secret providing qcow AES key or LUKS passphrase
> + encrypt.slot=<num> - Controls the slot where the secret is added/erased
> encryption=<bool (on/off)> - Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)
> lazy_refcounts=<bool (on/off)> - Postpone refcount updates
> preallocation=<str> - Preallocation mode (allowed values: off, metadata, falloc, full)
> --
> 2.17.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management
2019-09-06 13:49 ` Daniel P. Berrangé
@ 2019-09-06 13:57 ` Maxim Levitsky
2019-09-06 14:15 ` Daniel P. Berrangé
0 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-06 13:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, 2019-09-06 at 14:49 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:00PM +0300, Maxim Levitsky wrote:
> > Now you can specify which slot to put the encryption key to
> > Plus add 'active' option which will let user erase the key secret
> > instead of adding it.
> > Check that it is true for creation
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > block/crypto.c | 2 ++
> > block/crypto.h | 16 +++++++++++
> > block/qcow2.c | 2 ++
> > crypto/block-luks.c | 26 +++++++++++++++---
> > qapi/crypto.json | 19 ++++++++++++++
> > tests/qemu-iotests/082.out | 54 ++++++++++++++++++++++++++++++++++++++
> > 6 files changed, 115 insertions(+), 4 deletions(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 6e822c6e50..a6a3e1f1d8 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -144,6 +144,8 @@ static QemuOptsList block_crypto_create_opts_luks = {
> > BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
> > BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
> > BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
> > + BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(""),
> > + BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(""),
> > { /* end of list */ }
> > },
> > };
> > diff --git a/block/crypto.h b/block/crypto.h
> > index b935695e79..05cc43d9bc 100644
> > --- a/block/crypto.h
> > +++ b/block/crypto.h
> > @@ -35,12 +35,14 @@
> > "ID of the secret that provides the AES encryption key")
> >
> > #define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
> > +#define BLOCK_CRYPTO_OPT_LUKS_SLOT "slot"
> > #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
> > #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
> > #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg"
> > #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
> > #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
> > #define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
> > +#define BLOCK_CRYPTO_OPT_LUKS_ACTIVE "active"
> >
> > #define BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(prefix) \
> > BLOCK_CRYPTO_OPT_DEF_KEY_SECRET(prefix, \
> > @@ -88,6 +90,20 @@
> > .help = "Time to spend in PBKDF in milliseconds", \
> > }
> >
> > +#define BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(prefix) \
> > + { \
> > + .name = prefix BLOCK_CRYPTO_OPT_LUKS_SLOT, \
> > + .type = QEMU_OPT_NUMBER, \
> > + .help = "Controls the slot where the secret is added/erased", \
> > + }
> > +
> > +#define BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(prefix) \
> > + { \
> > + .name = prefix BLOCK_CRYPTO_OPT_LUKS_ACTIVE, \
> > + .type = QEMU_OPT_BOOL, \
> > + .help = "Controls if the added secret is added or erased", \
> > + }
>
> Do we actually need the "active" property for initial
> creation. I think its only needed for amend, so perhaps
> we shuold not register this at all ?
Sadly we kind of do, since both amend and create use the same option list currently.
I tried to duplicate it, and it is possible, but then you end up
with significant code duplication in qcow2 with its huge create option list.
I am now thinking that we could have had , 'create only' option list, 'amend only' option list,
and 'common' option list.
What do you think?
[...]
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management
2019-09-06 13:57 ` Maxim Levitsky
@ 2019-09-06 14:15 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:15 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Sep 06, 2019 at 04:57:22PM +0300, Maxim Levitsky wrote:
> On Fri, 2019-09-06 at 14:49 +0100, Daniel P. Berrangé wrote:
> > On Fri, Aug 30, 2019 at 11:56:00PM +0300, Maxim Levitsky wrote:
> > > Now you can specify which slot to put the encryption key to
> > > Plus add 'active' option which will let user erase the key secret
> > > instead of adding it.
> > > Check that it is true for creation
> > >
> > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > ---
> > > block/crypto.c | 2 ++
> > > block/crypto.h | 16 +++++++++++
> > > block/qcow2.c | 2 ++
> > > crypto/block-luks.c | 26 +++++++++++++++---
> > > qapi/crypto.json | 19 ++++++++++++++
> > > tests/qemu-iotests/082.out | 54 ++++++++++++++++++++++++++++++++++++++
> > > 6 files changed, 115 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/block/crypto.c b/block/crypto.c
> > > index 6e822c6e50..a6a3e1f1d8 100644
> > > --- a/block/crypto.c
> > > +++ b/block/crypto.c
> > > @@ -144,6 +144,8 @@ static QemuOptsList block_crypto_create_opts_luks = {
> > > BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
> > > BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
> > > BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
> > > + BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(""),
> > > + BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(""),
> > > { /* end of list */ }
> > > },
> > > };
> > > diff --git a/block/crypto.h b/block/crypto.h
> > > index b935695e79..05cc43d9bc 100644
> > > --- a/block/crypto.h
> > > +++ b/block/crypto.h
> > > @@ -35,12 +35,14 @@
> > > "ID of the secret that provides the AES encryption key")
> > >
> > > #define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
> > > +#define BLOCK_CRYPTO_OPT_LUKS_SLOT "slot"
> > > #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
> > > #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
> > > +#define BLOCK_CRYPTO_OPT_LUKS_ACTIVE "active"
> > >
> > > #define BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(prefix) \
> > > BLOCK_CRYPTO_OPT_DEF_KEY_SECRET(prefix, \
> > > @@ -88,6 +90,20 @@
> > > .help = "Time to spend in PBKDF in milliseconds", \
> > > }
> > >
> > > +#define BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(prefix) \
> > > + { \
> > > + .name = prefix BLOCK_CRYPTO_OPT_LUKS_SLOT, \
> > > + .type = QEMU_OPT_NUMBER, \
> > > + .help = "Controls the slot where the secret is added/erased", \
> > > + }
> > > +
> > > +#define BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(prefix) \
> > > + { \
> > > + .name = prefix BLOCK_CRYPTO_OPT_LUKS_ACTIVE, \
> > > + .type = QEMU_OPT_BOOL, \
> > > + .help = "Controls if the added secret is added or erased", \
> > > + }
> >
> > Do we actually need the "active" property for initial
> > creation. I think its only needed for amend, so perhaps
> > we shuold not register this at all ?
>
> Sadly we kind of do, since both amend and create use the same option list currently.
> I tried to duplicate it, and it is possible, but then you end up
> with significant code duplication in qcow2 with its huge create option list.
Ah, I see now.
> I am now thinking that we could have had , 'create only' option list, 'amend only' option list,
> and 'common' option list.
> What do you think?
I'm not too fussed - either way is fine with me.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the encryption key management
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
2019-08-30 20:55 ` [Qemu-devel] [PATCH 01/10] qcrypto: add suport for amend options Maxim Levitsky
2019-08-30 20:56 ` [Qemu-devel] [PATCH 02/10] qcrypto-luks: extend the create options for upcoming encryption key management Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 13:55 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 04/10] block: amend: add 'force' option Maxim Levitsky
` (6 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
crypto/block-luks.c | 366 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 364 insertions(+), 2 deletions(-)
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index ba20d55246..21325fbc79 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -70,6 +70,9 @@ typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot;
#define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL
+#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 2000
+#define QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS 40
+
static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = {
'L', 'U', 'K', 'S', 0xBA, 0xBE
};
@@ -219,6 +222,9 @@ struct QCryptoBlockLUKS {
/* Hash algorithm used in pbkdf2 function */
QCryptoHashAlgorithm hash_alg;
+
+ /* Name of the secret that was used to open the image */
+ char *secret;
};
@@ -1089,6 +1095,175 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
}
+
+/*
+ * Returns true if a slot i is marked as active
+ * (contains encrypted copy of the master key)
+ */
+
+static bool
+qcrypto_block_luks_slot_active(const QCryptoBlockLUKS *luks,
+ unsigned int slot_idx)
+{
+ uint32_t val = luks->header.key_slots[slot_idx].active;
+ return val == QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
+}
+
+/*
+ * Returns the number of slots that are marked as active
+ * (contains encrypted copy of the master key)
+ */
+
+static unsigned int
+qcrypto_block_luks_count_active_slots(const QCryptoBlockLUKS *luks)
+{
+ size_t i = 0;
+ unsigned int ret = 0;
+
+ for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+ if (qcrypto_block_luks_slot_active(luks, i)) {
+ ret++;
+ }
+ }
+ return ret;
+}
+
+
+/*
+ * Finds first key slot which is not active
+ * Returns the key slot index, or -1 if doesn't exist
+ */
+
+static int
+qcrypto_block_luks_find_free_keyslot(const QCryptoBlockLUKS *luks)
+{
+ size_t i;
+
+ for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+ if (!qcrypto_block_luks_slot_active(luks, i)) {
+ return i;
+ }
+ }
+ return -1;
+
+}
+
+/*
+ * Erases an keyslot given its index
+ * Returns:
+ * 0 if the keyslot was erased successfully
+ * -1 if a error occurred while erasing the keyslot
+ *
+ */
+
+static int
+qcrypto_block_luks_erase_key(QCryptoBlock *block,
+ unsigned int slot_idx,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ Error **errp)
+{
+ QCryptoBlockLUKS *luks = block->opaque;
+ QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
+ g_autofree uint8_t *garbagesplitkey = NULL;
+ size_t splitkeylen = luks->header.master_key_len * slot->stripes;
+ size_t i;
+
+ assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
+ assert(splitkeylen > 0);
+
+ garbagesplitkey = g_malloc0(splitkeylen);
+
+ /* Reset the key slot header */
+ memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN);
+ slot->iterations = 0;
+ slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
+
+ qcrypto_block_luks_store_header(block, writefunc, opaque, errp);
+
+ /*
+ * Now try to erase the key material, even if the header
+ * update failed
+ */
+
+ for (i = 0 ; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS ; i++) {
+ if (qcrypto_random_bytes(garbagesplitkey, splitkeylen, errp) < 0) {
+ /*
+ * If we failed to get the random data, still write
+ * at least zeros to the key slot at least once
+ */
+
+ if (i > 0) {
+ return -1;
+ }
+ }
+
+ if (writefunc(block,
+ slot->key_offset_sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+ garbagesplitkey,
+ splitkeylen,
+ opaque,
+ errp) != splitkeylen) {
+ return -1;
+ }
+ }
+ return 0;
+}
+
+
+/*
+ * Erase all the keys that match the given password
+ * Will stop when only one keyslot is remaining
+ * Returns number of slots that were erased or -1 on failure
+ */
+
+static int
+qcrypto_block_luks_erase_matching_keys(QCryptoBlock *block,
+ const char *password,
+ QCryptoBlockReadFunc readfunc,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ bool force,
+ Error **errp)
+{
+ QCryptoBlockLUKS *luks = block->opaque;
+ size_t i;
+ int rv;
+ g_autofree uint8_t *masterkey = NULL;
+ unsigned int erased_cnt = 0;
+ unsigned int active_slot_cnt = qcrypto_block_luks_count_active_slots(luks);
+
+ masterkey = g_new0(uint8_t, luks->header.master_key_len);
+
+ for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+
+ /* refuse to erase last key if not forced */
+ if (!force && active_slot_cnt == 1) {
+ break;
+ }
+
+ rv = qcrypto_block_luks_load_key(block, i, password, masterkey,
+ readfunc, opaque, errp);
+ if (rv < 0) {
+ return -1;
+ }
+ if (rv == 0) {
+ continue;
+ }
+
+ rv = qcrypto_block_luks_erase_key(block, i, writefunc, opaque, errp);
+ if (rv < 0) {
+ return -1;
+ }
+
+ erased_cnt++;
+ active_slot_cnt--;
+ }
+
+ return erased_cnt;
+}
+
+
static int
qcrypto_block_luks_open(QCryptoBlock *block,
QCryptoBlockOpenOptions *options,
@@ -1119,6 +1294,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks = g_new0(QCryptoBlockLUKS, 1);
block->opaque = luks;
+ luks->secret = g_strdup(options->u.luks.key_secret);
ret = qcrypto_block_luks_load_header(block, readfunc, opaque, errp);
if (ret) {
@@ -1234,7 +1410,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
if (!luks_opts.has_iter_time) {
- luks_opts.iter_time = 2000;
+ luks_opts.iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME;
}
if (!luks_opts.has_cipher_alg) {
luks_opts.cipher_alg = QCRYPTO_CIPHER_ALG_AES_256;
@@ -1292,6 +1468,8 @@ qcrypto_block_luks_create(QCryptoBlock *block,
optprefix ? optprefix : "");
goto error;
}
+ luks->secret = g_strdup(options->u.luks.key_secret);
+
password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp);
if (!password) {
goto error;
@@ -1522,6 +1700,187 @@ qcrypto_block_luks_create(QCryptoBlock *block,
}
+#define CHECK_NON_AMEND_OPTION(luks, luks_opts, name) \
+ if (luks_opts.has_##name && luks_opts.name != luks->name) { \
+ error_setg(errp, "Option \"" #name "\" can't be amended"); \
+ goto cleanup; \
+ }
+
+static int
+qcrypto_block_luks_amend_options(QCryptoBlock *block,
+ QCryptoBlockReadFunc readfunc,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ QCryptoBlockCreateOptions *options,
+ bool force,
+ Error **errp)
+{
+ QCryptoBlockLUKS *luks = block->opaque;
+ QCryptoBlockCreateOptionsLUKS luks_opts;
+ g_autofree char *old_password = NULL;
+ g_autofree char *password = NULL;
+ const char *unlock_secret = luks->secret;
+ g_autofree uint8_t *masterkey = NULL;
+ int slot = -1;
+ int ret = -1;
+ bool active = true;
+ int64_t iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME;
+
+ memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
+
+ CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_alg);
+ CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_mode);
+ CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_alg);
+ CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_hash_alg);
+ CHECK_NON_AMEND_OPTION(luks, luks_opts, hash_alg);
+
+ /* Read given slot and check it */
+ if (luks_opts.has_slot) {
+ slot = luks_opts.slot;
+ if (slot < 0 || slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) {
+ error_setg(errp,
+ "Given key slot %i is not supported by LUKS", slot);
+ goto cleanup;
+ }
+ }
+
+ if (luks_opts.has_iter_time) {
+ iter_time = luks_opts.iter_time;
+ }
+
+ if (luks_opts.has_active && luks_opts.active == false) {
+ active = false;
+ }
+
+ if (active) {
+
+ /* Check that we are not overwriting an active slot */
+ if (!force && slot != -1 &&
+ qcrypto_block_luks_slot_active(luks, slot)) {
+
+ error_setg(errp, "Can't update an active key slot %i",
+ slot);
+ goto cleanup;
+ }
+
+ /* check that we have the passwords*/
+ if (!luks_opts.has_key_secret) {
+ error_setg(errp, "Can't add a key slot without a password");
+ goto cleanup;
+ }
+
+ if (luks_opts.has_unlock_secret) {
+ unlock_secret = luks_opts.unlock_secret;
+ }
+
+ /* Read the old password */
+ old_password = qcrypto_secret_lookup_as_utf8(unlock_secret, errp);
+ if (!old_password) {
+ goto cleanup;
+ }
+
+ masterkey = g_new0(uint8_t, luks->header.master_key_len);
+
+ /* Retrieve the master key*/
+ if (qcrypto_block_luks_find_key(block, old_password, masterkey,
+ readfunc, opaque,
+ errp) < 0) {
+ error_append_hint(errp,
+ "unlock secret, doesn't unlock the image");
+ goto cleanup;
+ }
+
+ /* Read the new password*/
+ password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp);
+ if (!password) {
+ goto cleanup;
+ }
+
+ /* Find the new slot to write to */
+ if (slot == -1) {
+ slot = qcrypto_block_luks_find_free_keyslot(luks);
+
+ if (slot == -1) {
+ error_setg(errp,
+ "Can't add a keyslot - all key slots are in use");
+ goto cleanup;
+
+ }
+ }
+
+ /* Store the master key to the new slot */
+ if (qcrypto_block_luks_store_key(block, slot, password, masterkey,
+ iter_time, writefunc, opaque,
+ errp)) {
+
+ error_append_hint(errp, "Failed to store the keyslot %i", slot);
+ goto cleanup;
+ }
+
+ } else {
+
+ /* Check that we are not erasing last key slot */
+ if (qcrypto_block_luks_count_active_slots(luks) <= 1) {
+
+ if (!force) {
+ error_setg(errp, "Only one slot active - can't erase");
+ goto cleanup;
+ }
+ }
+
+ if (slot != -1) {
+ /* Check that we are not erasing an inactive slot */
+ if (!qcrypto_block_luks_slot_active(luks, luks_opts.slot)) {
+ if (!force) {
+ error_setg(errp, "Can't erase an inactive key slot %i",
+ slot);
+ goto cleanup;
+ }
+ }
+
+ /* Erase the given slot */
+ if (qcrypto_block_luks_erase_key(block, slot,
+ writefunc, opaque, errp)) {
+ goto cleanup;
+ }
+
+ } else {
+ if (!luks_opts.has_key_secret) {
+ error_setg(errp,
+ "To erase a keyslot you have to specify either the"
+ "slot index or a password "
+ "(to erase all slots that match it)");
+ goto cleanup;
+ }
+
+ password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret,
+ errp);
+ if (!password) {
+ goto cleanup;
+ }
+
+ ret = qcrypto_block_luks_erase_matching_keys(block, password,
+ readfunc, writefunc,
+ opaque, force, errp);
+ if (ret == 0) {
+ error_setg(errp,
+ "Didn't erase a keyslot, because no keyslots match"
+ " the given password");
+ ret = -EINVAL;
+ goto cleanup;
+ }
+
+ if (ret < 0) {
+ goto cleanup;
+ }
+ }
+ }
+ ret = 0;
+cleanup:
+ return ret;
+}
+
+
static int qcrypto_block_luks_get_info(QCryptoBlock *block,
QCryptoBlockInfo *info,
Error **errp)
@@ -1569,7 +1928,9 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
static void qcrypto_block_luks_cleanup(QCryptoBlock *block)
{
- g_free(block->opaque);
+ QCryptoBlockLUKS *luks = block->opaque;
+ g_free(luks->secret);
+ g_free(luks);
}
@@ -1606,6 +1967,7 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block,
const QCryptoBlockDriver qcrypto_block_driver_luks = {
.open = qcrypto_block_luks_open,
.create = qcrypto_block_luks_create,
+ .amend = qcrypto_block_luks_amend_options,
.get_info = qcrypto_block_luks_get_info,
.cleanup = qcrypto_block_luks_cleanup,
.decrypt = qcrypto_block_luks_decrypt,
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the encryption key management
2019-08-30 20:56 ` [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the " Maxim Levitsky
@ 2019-09-06 13:55 ` Daniel P. Berrangé
2019-09-12 9:48 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 13:55 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:01PM +0300, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> crypto/block-luks.c | 366 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 364 insertions(+), 2 deletions(-)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index ba20d55246..21325fbc79 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -70,6 +70,9 @@ typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot;
>
> #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL
>
> +#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 2000
Perhaps use ITER_TIME_MS to make it clear it is millisecs
> +#define QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS 40
> +
> static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = {
> 'L', 'U', 'K', 'S', 0xBA, 0xBE
> };
> @@ -219,6 +222,9 @@ struct QCryptoBlockLUKS {
>
> /* Hash algorithm used in pbkdf2 function */
> QCryptoHashAlgorithm hash_alg;
> +
> + /* Name of the secret that was used to open the image */
> + char *secret;
> +
> +/*
> + * Returns true if a slot i is marked as active
> + * (contains encrypted copy of the master key)
> + */
> +
> +static bool
No blank line is wanted between the comment & function.
Likewise for the rest of this patch series
> +qcrypto_block_luks_slot_active(const QCryptoBlockLUKS *luks,
> + unsigned int slot_idx)
> +{
> + uint32_t val = luks->header.key_slots[slot_idx].active;
> + return val == QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
> +}
> +static int
> +qcrypto_block_luks_erase_key(QCryptoBlock *block,
> + unsigned int slot_idx,
> + QCryptoBlockWriteFunc writefunc,
> + void *opaque,
> + Error **errp)
> +{
> + QCryptoBlockLUKS *luks = block->opaque;
> + QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
> + g_autofree uint8_t *garbagesplitkey = NULL;
> + size_t splitkeylen = luks->header.master_key_len * slot->stripes;
> + size_t i;
> +
> + assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> + assert(splitkeylen > 0);
> +
> + garbagesplitkey = g_malloc0(splitkeylen);
I'd prefer g_new0(uint8_t, splitkeylen)
> +
> + /* Reset the key slot header */
> + memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN);
> + slot->iterations = 0;
> + slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> +
> @@ -1522,6 +1700,187 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> }
>
>
> +#define CHECK_NON_AMEND_OPTION(luks, luks_opts, name) \
> + if (luks_opts.has_##name && luks_opts.name != luks->name) { \
> + error_setg(errp, "Option \"" #name "\" can't be amended"); \
> + goto cleanup; \
> + }
> +
> +static int
> +qcrypto_block_luks_amend_options(QCryptoBlock *block,
> + QCryptoBlockReadFunc readfunc,
> + QCryptoBlockWriteFunc writefunc,
> + void *opaque,
> + QCryptoBlockCreateOptions *options,
> + bool force,
> + Error **errp)
> +{
> + QCryptoBlockLUKS *luks = block->opaque;
> + QCryptoBlockCreateOptionsLUKS luks_opts;
> + g_autofree char *old_password = NULL;
> + g_autofree char *password = NULL;
> + const char *unlock_secret = luks->secret;
> + g_autofree uint8_t *masterkey = NULL;
> + int slot = -1;
> + int ret = -1;
> + bool active = true;
> + int64_t iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME;
> +
> + memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
> +
> + CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_alg);
> + CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_mode);
> + CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_alg);
> + CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_hash_alg);
> + CHECK_NON_AMEND_OPTION(luks, luks_opts, hash_alg);
> +
> + /* Read given slot and check it */
> + if (luks_opts.has_slot) {
> + slot = luks_opts.slot;
> + if (slot < 0 || slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) {
> + error_setg(errp,
> + "Given key slot %i is not supported by LUKS", slot);
> + goto cleanup;
Off by 1 one indent
> + }
> + }
> +
> + if (luks_opts.has_iter_time) {
> + iter_time = luks_opts.iter_time;
> + }
> +
> + if (luks_opts.has_active && luks_opts.active == false) {
> + active = false;
> + }
> +
> + if (active) {
> +
> + /* Check that we are not overwriting an active slot */
> + if (!force && slot != -1 &&
> + qcrypto_block_luks_slot_active(luks, slot)) {
> +
> + error_setg(errp, "Can't update an active key slot %i",
> + slot);
> + goto cleanup;
> + }
> +
> + /* check that we have the passwords*/
> + if (!luks_opts.has_key_secret) {
> + error_setg(errp, "Can't add a key slot without a password");
> + goto cleanup;
> + }
> +
> + if (luks_opts.has_unlock_secret) {
> + unlock_secret = luks_opts.unlock_secret;
> + }
> +
> + /* Read the old password */
> + old_password = qcrypto_secret_lookup_as_utf8(unlock_secret, errp);
> + if (!old_password) {
> + goto cleanup;
> + }
> +
> + masterkey = g_new0(uint8_t, luks->header.master_key_len);
> +
> + /* Retrieve the master key*/
> + if (qcrypto_block_luks_find_key(block, old_password, masterkey,
> + readfunc, opaque,
> + errp) < 0) {
> + error_append_hint(errp,
> + "unlock secret, doesn't unlock the image");
No need for the ',' in this msg I think.
> + goto cleanup;
> + }
> +
> + /* Read the new password*/
> + password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp);
> + if (!password) {
> + goto cleanup;
> + }
> +
> + /* Find the new slot to write to */
> + if (slot == -1) {
> + slot = qcrypto_block_luks_find_free_keyslot(luks);
> +
> + if (slot == -1) {
> + error_setg(errp,
> + "Can't add a keyslot - all key slots are in use");
> + goto cleanup;
> +
> + }
Extra blank line
> + }
> +
> + /* Store the master key to the new slot */
> + if (qcrypto_block_luks_store_key(block, slot, password, masterkey,
> + iter_time, writefunc, opaque,
> + errp)) {
> +
Extra blank line
> + error_append_hint(errp, "Failed to store the keyslot %i", slot);
> + goto cleanup;
> + }
> +
> + } else {
> +
> + /* Check that we are not erasing last key slot */
> + if (qcrypto_block_luks_count_active_slots(luks) <= 1) {
> +
Extra blank line(s)
> + if (!force) {
> + error_setg(errp, "Only one slot active - can't erase");
> + goto cleanup;
> + }
> + }
> +
> + if (slot != -1) {
> + /* Check that we are not erasing an inactive slot */
> + if (!qcrypto_block_luks_slot_active(luks, luks_opts.slot)) {
> + if (!force) {
> + error_setg(errp, "Can't erase an inactive key slot %i",
> + slot);
> + goto cleanup;
> + }
> + }
> +
> + /* Erase the given slot */
> + if (qcrypto_block_luks_erase_key(block, slot,
> + writefunc, opaque, errp)) {
> + goto cleanup;
> + }
> +
> + } else {
> + if (!luks_opts.has_key_secret) {
> + error_setg(errp,
> + "To erase a keyslot you have to specify either the"
> + "slot index or a password "
> + "(to erase all slots that match it)");
> + goto cleanup;
> + }
> +
> + password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret,
> + errp);
> + if (!password) {
> + goto cleanup;
> + }
> +
> + ret = qcrypto_block_luks_erase_matching_keys(block, password,
> + readfunc, writefunc,
> + opaque, force, errp);
> + if (ret == 0) {
> + error_setg(errp,
> + "Didn't erase a keyslot, because no keyslots match"
> + " the given password");
> + ret = -EINVAL;
> + goto cleanup;
> + }
> +
> + if (ret < 0) {
> + goto cleanup;
> + }
> + }
> + }
> + ret = 0;
> +cleanup:
> + return ret;
> +}
> +
> +
> static int qcrypto_block_luks_get_info(QCryptoBlock *block,
> QCryptoBlockInfo *info,
> Error **errp)
> @@ -1569,7 +1928,9 @@ static int qcrypto_block_luks_get_info(QCryptoBlock *block,
>
> static void qcrypto_block_luks_cleanup(QCryptoBlock *block)
> {
> - g_free(block->opaque);
> + QCryptoBlockLUKS *luks = block->opaque;
> + g_free(luks->secret);
> + g_free(luks);
> }
>
>
> @@ -1606,6 +1967,7 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block,
> const QCryptoBlockDriver qcrypto_block_driver_luks = {
> .open = qcrypto_block_luks_open,
> .create = qcrypto_block_luks_create,
> + .amend = qcrypto_block_luks_amend_options,
> .get_info = qcrypto_block_luks_get_info,
> .cleanup = qcrypto_block_luks_cleanup,
> .decrypt = qcrypto_block_luks_decrypt,
> --
> 2.17.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the encryption key management
2019-09-06 13:55 ` Daniel P. Berrangé
@ 2019-09-12 9:48 ` Maxim Levitsky
0 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:48 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Max Reitz,
John Snow
On Fri, 2019-09-06 at 14:55 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:01PM +0300, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > crypto/block-luks.c | 366 +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 364 insertions(+), 2 deletions(-)
> >
> > diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> > index ba20d55246..21325fbc79 100644
> > --- a/crypto/block-luks.c
> > +++ b/crypto/block-luks.c
> > @@ -70,6 +70,9 @@ typedef struct QCryptoBlockLUKSKeySlot QCryptoBlockLUKSKeySlot;
> >
> > #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL
> >
> > +#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME 2000
>
> Perhaps use ITER_TIME_MS to make it clear it is millisecs
Why not... done.
>
> > +#define QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS 40
> > +
> > static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = {
> > 'L', 'U', 'K', 'S', 0xBA, 0xBE
> > };
> > @@ -219,6 +222,9 @@ struct QCryptoBlockLUKS {
> >
> > /* Hash algorithm used in pbkdf2 function */
> > QCryptoHashAlgorithm hash_alg;
> > +
> > + /* Name of the secret that was used to open the image */
> > + char *secret;
> > +
> > +/*
> > + * Returns true if a slot i is marked as active
> > + * (contains encrypted copy of the master key)
> > + */
> > +
> > +static bool
>
> No blank line is wanted between the comment & function.
> Likewise for the rest of this patch series
No problem!
>
> > +qcrypto_block_luks_slot_active(const QCryptoBlockLUKS *luks,
> > + unsigned int slot_idx)
> > +{
> > + uint32_t val = luks->header.key_slots[slot_idx].active;
> > + return val == QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
> > +}
>
>
>
> > +static int
> > +qcrypto_block_luks_erase_key(QCryptoBlock *block,
> > + unsigned int slot_idx,
> > + QCryptoBlockWriteFunc writefunc,
> > + void *opaque,
> > + Error **errp)
> > +{
> > + QCryptoBlockLUKS *luks = block->opaque;
> > + QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
> > + g_autofree uint8_t *garbagesplitkey = NULL;
> > + size_t splitkeylen = luks->header.master_key_len * slot->stripes;
> > + size_t i;
> > +
> > + assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> > + assert(splitkeylen > 0);
> > +
> > + garbagesplitkey = g_malloc0(splitkeylen);
>
> I'd prefer g_new0(uint8_t, splitkeylen)
Done.
>
> > +
> > + /* Reset the key slot header */
> > + memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN);
> > + slot->iterations = 0;
> > + slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
> > +
>
>
> > @@ -1522,6 +1700,187 @@ qcrypto_block_luks_create(QCryptoBlock *block,
> > }
> >
> >
> > +#define CHECK_NON_AMEND_OPTION(luks, luks_opts, name) \
> > + if (luks_opts.has_##name && luks_opts.name != luks->name) { \
> > + error_setg(errp, "Option \"" #name "\" can't be amended"); \
> > + goto cleanup; \
> > + }
> > +
> > +static int
> > +qcrypto_block_luks_amend_options(QCryptoBlock *block,
> > + QCryptoBlockReadFunc readfunc,
> > + QCryptoBlockWriteFunc writefunc,
> > + void *opaque,
> > + QCryptoBlockCreateOptions *options,
> > + bool force,
> > + Error **errp)
> > +{
> > + QCryptoBlockLUKS *luks = block->opaque;
> > + QCryptoBlockCreateOptionsLUKS luks_opts;
> > + g_autofree char *old_password = NULL;
> > + g_autofree char *password = NULL;
> > + const char *unlock_secret = luks->secret;
> > + g_autofree uint8_t *masterkey = NULL;
> > + int slot = -1;
> > + int ret = -1;
> > + bool active = true;
> > + int64_t iter_time = QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME;
> > +
> > + memcpy(&luks_opts, &options->u.luks, sizeof(luks_opts));
> > +
> > + CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_alg);
> > + CHECK_NON_AMEND_OPTION(luks, luks_opts, cipher_mode);
> > + CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_alg);
> > + CHECK_NON_AMEND_OPTION(luks, luks_opts, ivgen_hash_alg);
> > + CHECK_NON_AMEND_OPTION(luks, luks_opts, hash_alg);
> > +
> > + /* Read given slot and check it */
> > + if (luks_opts.has_slot) {
> > + slot = luks_opts.slot;
> > + if (slot < 0 || slot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) {
> > + error_setg(errp,
> > + "Given key slot %i is not supported by LUKS", slot);
> > + goto cleanup;
>
> Off by 1 one indent
Sorry about that.
>
> > + }
> > + }
> > +
> > + if (luks_opts.has_iter_time) {
> > + iter_time = luks_opts.iter_time;
> > + }
> > +
> > + if (luks_opts.has_active && luks_opts.active == false) {
> > + active = false;
> > + }
> > +
> > + if (active) {
> > +
> > + /* Check that we are not overwriting an active slot */
> > + if (!force && slot != -1 &&
> > + qcrypto_block_luks_slot_active(luks, slot)) {
> > +
> > + error_setg(errp, "Can't update an active key slot %i",
> > + slot);
> > + goto cleanup;
> > + }
> > +
> > + /* check that we have the passwords*/
> > + if (!luks_opts.has_key_secret) {
> > + error_setg(errp, "Can't add a key slot without a password");
> > + goto cleanup;
> > + }
> > +
> > + if (luks_opts.has_unlock_secret) {
> > + unlock_secret = luks_opts.unlock_secret;
> > + }
> > +
> > + /* Read the old password */
> > + old_password = qcrypto_secret_lookup_as_utf8(unlock_secret, errp);
> > + if (!old_password) {
> > + goto cleanup;
> > + }
> > +
> > + masterkey = g_new0(uint8_t, luks->header.master_key_len);
> > +
> > + /* Retrieve the master key*/
> > + if (qcrypto_block_luks_find_key(block, old_password, masterkey,
> > + readfunc, opaque,
> > + errp) < 0) {
> > + error_append_hint(errp,
> > + "unlock secret, doesn't unlock the image");
>
> No need for the ',' in this msg I think.
Fixed and reworded a bit.
>
> > + goto cleanup;
> > + }
> > +
> > + /* Read the new password*/
> > + password = qcrypto_secret_lookup_as_utf8(luks_opts.key_secret, errp);
> > + if (!password) {
> > + goto cleanup;
> > + }
> > +
> > + /* Find the new slot to write to */
> > + if (slot == -1) {
> > + slot = qcrypto_block_luks_find_free_keyslot(luks);
> > +
> > + if (slot == -1) {
> > + error_setg(errp,
> > + "Can't add a keyslot - all key slots are in use");
> > + goto cleanup;
> > +
> > + }
>
> Extra blank line
Agree
>
> > + }
> > +
> > + /* Store the master key to the new slot */
> > + if (qcrypto_block_luks_store_key(block, slot, password, masterkey,
> > + iter_time, writefunc, opaque,
> > + errp)) {
> > +
>
>
> Extra blank line
>
> > + error_append_hint(errp, "Failed to store the keyslot %i", slot);
> > + goto cleanup;
> > + }
> > +
> > + } else {
> > +
> > + /* Check that we are not erasing last key slot */
> > + if (qcrypto_block_luks_count_active_slots(luks) <= 1) {
> > +
>
> Extra blank line(s)
I hope I cleaned all blank lines, sorry about that.
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 04/10] block: amend: add 'force' option
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (2 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 03/10] qcrypto-luks: implement the " Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 13:59 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 05/10] block/crypto: implement the encryption key management Maxim Levitsky
` (5 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
block.c | 4 +++-
block/qcow2.c | 1 +
include/block/block.h | 1 +
include/block/block_int.h | 1 +
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 8 +++++++-
qemu-img.texi | 6 +++++-
7 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 874a29a983..df6707677a 100644
--- a/block.c
+++ b/block.c
@@ -6142,6 +6142,7 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs,
int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+ bool force,
Error **errp)
{
if (!bs->drv) {
@@ -6153,7 +6154,8 @@ int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
bs->drv->format_name);
return -ENOTSUP;
}
- return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque, errp);
+ return bs->drv->bdrv_amend_options(bs, opts, status_cb,
+ cb_opaque, force, errp);
}
/* This function will be called by the bdrv_recurse_is_first_non_filter method
diff --git a/block/qcow2.c b/block/qcow2.c
index be4a5063e5..376bb416fd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4823,6 +4823,7 @@ static void qcow2_amend_helper_cb(BlockDriverState *bs,
static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
void *cb_opaque,
+ bool force,
Error **errp)
{
BDRVQcow2State *s = bs->opaque;
diff --git a/include/block/block.h b/include/block/block.h
index 124ad40809..6bc89c7667 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -400,6 +400,7 @@ typedef void BlockDriverAmendStatusCB(BlockDriverState *bs, int64_t offset,
int64_t total_work_size, void *opaque);
int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+ bool force,
Error **errp);
/* external snapshots */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ceec8c2f56..c6aa05214f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -402,6 +402,7 @@ struct BlockDriver {
int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
void *cb_opaque,
+ bool force,
Error **errp);
void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1c93e6d185..323ea10ad0 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -14,9 +14,9 @@ STEXI
ETEXI
DEF("amend", img_amend,
- "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
+ "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] [--force] -o options filename")
STEXI
-@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [--force] -o @var{options} @var{filename}
ETEXI
DEF("bench", img_bench,
diff --git a/qemu-img.c b/qemu-img.c
index 7daa05e51a..4533a44c1d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -70,6 +70,7 @@ enum {
OPTION_PREALLOCATION = 265,
OPTION_SHRINK = 266,
OPTION_SALVAGE = 267,
+ OPTION_FORCE = 268,
};
typedef enum OutputFormat {
@@ -3915,6 +3916,7 @@ static int img_amend(int argc, char **argv)
BlockBackend *blk = NULL;
BlockDriverState *bs = NULL;
bool image_opts = false;
+ bool force = false;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
@@ -3922,6 +3924,7 @@ static int img_amend(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"force", no_argument, 0, OPTION_FORCE},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, ":ho:f:t:pq",
@@ -3977,6 +3980,9 @@ static int img_amend(int argc, char **argv)
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
+ case OPTION_FORCE:
+ force = true;
+ break;
}
}
@@ -4054,7 +4060,7 @@ static int img_amend(int argc, char **argv)
/* In case the driver does not call amend_status_cb() */
qemu_progress_print(0.f, 0);
- ret = bdrv_amend_options(bs, opts, &amend_status_cb, NULL, &err);
+ ret = bdrv_amend_options(bs, opts, &amend_status_cb, NULL, force, &err);
qemu_progress_print(100.f, 0);
if (ret < 0) {
error_report_err(err);
diff --git a/qemu-img.texi b/qemu-img.texi
index b5156d6316..b6ed4357e8 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -201,11 +201,15 @@ Command description:
@table @option
-@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [--force] -o @var{options} @var{filename}
Amends the image format specific @var{options} for the image file
@var{filename}. Not all file formats support this operation.
+--force allows some unsafe operations. Currently for -f luks,
+it allows to erase last encryption key, and to overwrite an active
+encryption key.
+
@item bench [-c @var{count}] [-d @var{depth}] [-f @var{fmt}] [--flush-interval=@var{flush_interval}] [-n] [--no-drain] [-o @var{offset}] [--pattern=@var{pattern}] [-q] [-s @var{buffer_size}] [-S @var{step_size}] [-t @var{cache}] [-w] [-U] @var{filename}
Run a simple sequential I/O benchmark on the specified image. If @code{-w} is
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 04/10] block: amend: add 'force' option
2019-08-30 20:56 ` [Qemu-devel] [PATCH 04/10] block: amend: add 'force' option Maxim Levitsky
@ 2019-09-06 13:59 ` Daniel P. Berrangé
2019-09-12 9:53 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 13:59 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:02PM +0300, Maxim Levitsky wrote:
This could do with some text to explain what this will be
used for.
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> block.c | 4 +++-
> block/qcow2.c | 1 +
> include/block/block.h | 1 +
> include/block/block_int.h | 1 +
> qemu-img-cmds.hx | 4 ++--
> qemu-img.c | 8 +++++++-
> qemu-img.texi | 6 +++++-
> 7 files changed, 20 insertions(+), 5 deletions(-)
For the code
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 04/10] block: amend: add 'force' option
2019-09-06 13:59 ` Daniel P. Berrangé
@ 2019-09-12 9:53 ` Maxim Levitsky
0 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-12 9:53 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Max Reitz,
John Snow
On Fri, 2019-09-06 at 14:59 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:02PM +0300, Maxim Levitsky wrote:
>
> This could do with some text to explain what this will be
> used for.
I actually added an explanation to the man page
"
+--force allows some unsafe operations. Currently for -f luks,
+it allows to erase last encryption key, and to overwrite an active
+encryption key.
+
"
You probably mean adding few words in the commit message as well,
and I'll do that anyway.
>
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > block.c | 4 +++-
> > block/qcow2.c | 1 +
> > include/block/block.h | 1 +
> > include/block/block_int.h | 1 +
> > qemu-img-cmds.hx | 4 ++--
> > qemu-img.c | 8 +++++++-
> > qemu-img.texi | 6 +++++-
> > 7 files changed, 20 insertions(+), 5 deletions(-)
>
> For the code
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> Regards,
> Daniel
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 05/10] block/crypto: implement the encryption key management
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (3 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 04/10] block: amend: add 'force' option Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 14:04 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 06/10] qcow2: implement crypto amend options Maxim Levitsky
` (4 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
This implements the encryption key management
using the generic code in qcrypto layer
(currently only for qemu-img amend)
This code adds another 'write_func' because the initialization
write_func works directly on the underlying file,
because during the creation, there is no open instance
of the luks driver, but during regular use, we have it,
and should use it instead.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
block/crypto.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 103 insertions(+), 3 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index a6a3e1f1d8..dbd95a99ba 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -36,6 +36,7 @@ typedef struct BlockCrypto BlockCrypto;
struct BlockCrypto {
QCryptoBlock *block;
+ bool updating_keys;
};
@@ -70,6 +71,24 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
return ret;
}
+static ssize_t block_crypto_write_func(QCryptoBlock *block,
+ size_t offset,
+ const uint8_t *buf,
+ size_t buflen,
+ void *opaque,
+ Error **errp)
+{
+ BlockDriverState *bs = opaque;
+ ssize_t ret;
+
+ ret = bdrv_pwrite(bs->file, offset, buf, buflen);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not write encryption header");
+ return ret;
+ }
+ return ret;
+}
+
struct BlockCryptoCreateData {
BlockBackend *blk;
@@ -647,6 +666,88 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
return spec_info;
}
+
+static int
+block_crypto_amend_options(BlockDriverState *bs,
+ QemuOpts *opts,
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque,
+ bool force,
+ Error **errp)
+{
+ BlockCrypto *crypto = bs->opaque;
+ QDict *cryptoopts = NULL;
+ QCryptoBlockCreateOptions *amend_options = NULL;
+ int ret;
+
+ assert(crypto);
+ assert(crypto->block);
+
+ crypto->updating_keys = true;
+
+ ret = bdrv_child_refresh_perms(bs, bs->file, errp);
+ if (ret) {
+ goto cleanup;
+ }
+
+ cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
+ &block_crypto_create_opts_luks,
+ true);
+
+ qdict_put_str(cryptoopts, "format", "luks");
+ amend_options = block_crypto_create_opts_init(cryptoopts, errp);
+ if (!amend_options) {
+ ret = -EINVAL;
+ goto cleanup;
+ }
+
+ ret = qcrypto_block_amend_options(crypto->block,
+ block_crypto_read_func,
+ block_crypto_write_func,
+ bs,
+ amend_options,
+ force,
+ errp);
+cleanup:
+ crypto->updating_keys = false;
+ bdrv_child_refresh_perms(bs, bs->file, errp);
+ qapi_free_QCryptoBlockCreateOptions(amend_options);
+ qobject_unref(cryptoopts);
+ return ret;
+}
+
+
+static void
+block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
+ const BdrvChildRole *role,
+ BlockReopenQueue *reopen_queue,
+ uint64_t perm, uint64_t shared,
+ uint64_t *nperm, uint64_t *nshared)
+{
+
+ BlockCrypto *crypto = bs->opaque;
+
+ /*
+ * This driver doesn't modify LUKS metadata except
+ * when updating the encryption slots.
+ * Allow share-rw=on as a special case.
+ *
+ * Encryption update will set the crypto->updating_keys
+ * during that period and refresh permissions
+ *
+ * */
+
+ if (crypto->updating_keys) {
+ /*need exclusive write access for header update */
+ perm |= BLK_PERM_WRITE;
+ shared &= ~BLK_PERM_WRITE;
+ }
+
+ bdrv_filter_default_perms(bs, c, role, reopen_queue,
+ perm, shared, nperm, nshared);
+}
+
+
static const char *const block_crypto_strong_runtime_opts[] = {
BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET,
@@ -659,9 +760,7 @@ static BlockDriver bdrv_crypto_luks = {
.bdrv_probe = block_crypto_probe_luks,
.bdrv_open = block_crypto_open_luks,
.bdrv_close = block_crypto_close,
- /* This driver doesn't modify LUKS metadata except when creating image.
- * Allow share-rw=on as a special case. */
- .bdrv_child_perm = bdrv_filter_default_perms,
+ .bdrv_child_perm = block_crypto_child_perms,
.bdrv_co_create = block_crypto_co_create_luks,
.bdrv_co_create_opts = block_crypto_co_create_opts_luks,
.bdrv_co_truncate = block_crypto_co_truncate,
@@ -674,6 +773,7 @@ static BlockDriver bdrv_crypto_luks = {
.bdrv_getlength = block_crypto_getlength,
.bdrv_get_info = block_crypto_get_info_luks,
.bdrv_get_specific_info = block_crypto_get_specific_info_luks,
+ .bdrv_amend_options = block_crypto_amend_options,
.strong_runtime_opts = block_crypto_strong_runtime_opts,
};
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 05/10] block/crypto: implement the encryption key management
2019-08-30 20:56 ` [Qemu-devel] [PATCH 05/10] block/crypto: implement the encryption key management Maxim Levitsky
@ 2019-09-06 14:04 ` Daniel P. Berrangé
2019-09-12 10:08 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:04 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:03PM +0300, Maxim Levitsky wrote:
> This implements the encryption key management
> using the generic code in qcrypto layer
> (currently only for qemu-img amend)
>
> This code adds another 'write_func' because the initialization
> write_func works directly on the underlying file,
> because during the creation, there is no open instance
> of the luks driver, but during regular use, we have it,
> and should use it instead.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> block/crypto.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 103 insertions(+), 3 deletions(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index a6a3e1f1d8..dbd95a99ba 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -36,6 +36,7 @@ typedef struct BlockCrypto BlockCrypto;
>
> struct BlockCrypto {
> QCryptoBlock *block;
> + bool updating_keys;
> };
>
>
> @@ -70,6 +71,24 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> return ret;
> }
>
> +static ssize_t block_crypto_write_func(QCryptoBlock *block,
> + size_t offset,
> + const uint8_t *buf,
> + size_t buflen,
> + void *opaque,
> + Error **errp)
Indent off-by-1 - align with param on the first line
> +{
> + BlockDriverState *bs = opaque;
> + ssize_t ret;
> +
> + ret = bdrv_pwrite(bs->file, offset, buf, buflen);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not write encryption header");
> + return ret;
> + }
> + return ret;
> +}
> +
>
> struct BlockCryptoCreateData {
> BlockBackend *blk;
> @@ -647,6 +666,88 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
> return spec_info;
> }
>
> +
> +static int
> +block_crypto_amend_options(BlockDriverState *bs,
> + QemuOpts *opts,
> + BlockDriverAmendStatusCB *status_cb,
> + void *cb_opaque,
> + bool force,
> + Error **errp)
> +{
> + BlockCrypto *crypto = bs->opaque;
> + QDict *cryptoopts = NULL;
> + QCryptoBlockCreateOptions *amend_options = NULL;
> + int ret;
> +
> + assert(crypto);
> + assert(crypto->block);
> +
> + crypto->updating_keys = true;
> +
> + ret = bdrv_child_refresh_perms(bs, bs->file, errp);
> + if (ret) {
I can;'t remember - does this need to be "ret < 0" or
does refresh_perms return positive errnos ?
> + goto cleanup;
> + }
> +
> + cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> + &block_crypto_create_opts_luks,
> + true);
> +
> + qdict_put_str(cryptoopts, "format", "luks");
> + amend_options = block_crypto_create_opts_init(cryptoopts, errp);
> + if (!amend_options) {
> + ret = -EINVAL;
> + goto cleanup;
> + }
> +
> + ret = qcrypto_block_amend_options(crypto->block,
> + block_crypto_read_func,
> + block_crypto_write_func,
> + bs,
> + amend_options,
> + force,
> + errp);
> +cleanup:
> + crypto->updating_keys = false;
> + bdrv_child_refresh_perms(bs, bs->file, errp);
> + qapi_free_QCryptoBlockCreateOptions(amend_options);
> + qobject_unref(cryptoopts);
> + return ret;
> +}
> +
> +
> +static void
> +block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
> + const BdrvChildRole *role,
> + BlockReopenQueue *reopen_queue,
> + uint64_t perm, uint64_t shared,
> + uint64_t *nperm, uint64_t *nshared)
> +{
> +
> + BlockCrypto *crypto = bs->opaque;
> +
> + /*
> + * This driver doesn't modify LUKS metadata except
> + * when updating the encryption slots.
> + * Allow share-rw=on as a special case.
> + *
> + * Encryption update will set the crypto->updating_keys
> + * during that period and refresh permissions
> + *
> + * */
> +
> + if (crypto->updating_keys) {
> + /*need exclusive write access for header update */
> + perm |= BLK_PERM_WRITE;
> + shared &= ~BLK_PERM_WRITE;
> + }
> +
> + bdrv_filter_default_perms(bs, c, role, reopen_queue,
> + perm, shared, nperm, nshared);
> +}
> +
> +
> static const char *const block_crypto_strong_runtime_opts[] = {
> BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET,
>
> @@ -659,9 +760,7 @@ static BlockDriver bdrv_crypto_luks = {
> .bdrv_probe = block_crypto_probe_luks,
> .bdrv_open = block_crypto_open_luks,
> .bdrv_close = block_crypto_close,
> - /* This driver doesn't modify LUKS metadata except when creating image.
> - * Allow share-rw=on as a special case. */
> - .bdrv_child_perm = bdrv_filter_default_perms,
> + .bdrv_child_perm = block_crypto_child_perms,
> .bdrv_co_create = block_crypto_co_create_luks,
> .bdrv_co_create_opts = block_crypto_co_create_opts_luks,
> .bdrv_co_truncate = block_crypto_co_truncate,
> @@ -674,6 +773,7 @@ static BlockDriver bdrv_crypto_luks = {
> .bdrv_getlength = block_crypto_getlength,
> .bdrv_get_info = block_crypto_get_info_luks,
> .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
> + .bdrv_amend_options = block_crypto_amend_options,
>
> .strong_runtime_opts = block_crypto_strong_runtime_opts,
> };
> --
> 2.17.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 05/10] block/crypto: implement the encryption key management
2019-09-06 14:04 ` Daniel P. Berrangé
@ 2019-09-12 10:08 ` Maxim Levitsky
0 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-12 10:08 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Max Reitz,
John Snow
On Fri, 2019-09-06 at 15:04 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:03PM +0300, Maxim Levitsky wrote:
> > This implements the encryption key management
> > using the generic code in qcrypto layer
> > (currently only for qemu-img amend)
> >
> > This code adds another 'write_func' because the initialization
> > write_func works directly on the underlying file,
> > because during the creation, there is no open instance
> > of the luks driver, but during regular use, we have it,
> > and should use it instead.
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > block/crypto.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 103 insertions(+), 3 deletions(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index a6a3e1f1d8..dbd95a99ba 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -36,6 +36,7 @@ typedef struct BlockCrypto BlockCrypto;
> >
> > struct BlockCrypto {
> > QCryptoBlock *block;
> > + bool updating_keys;
> > };
> >
> >
> > @@ -70,6 +71,24 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
> > return ret;
> > }
> >
> > +static ssize_t block_crypto_write_func(QCryptoBlock *block,
> > + size_t offset,
> > + const uint8_t *buf,
> > + size_t buflen,
> > + void *opaque,
> > + Error **errp)
>
> Indent off-by-1 - align with param on the first line
I hope you won't hate me after all these indent bugs.
I'll learn to notice, I promise :-)
>
> > +{
> > + BlockDriverState *bs = opaque;
> > + ssize_t ret;
> > +
> > + ret = bdrv_pwrite(bs->file, offset, buf, buflen);
> > + if (ret < 0) {
> > + error_setg_errno(errp, -ret, "Could not write encryption header");
> > + return ret;
> > + }
> > + return ret;
> > +}
> > +
> >
> > struct BlockCryptoCreateData {
> > BlockBackend *blk;
> > @@ -647,6 +666,88 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
> > return spec_info;
> > }
> >
> > +
> > +static int
> > +block_crypto_amend_options(BlockDriverState *bs,
> > + QemuOpts *opts,
> > + BlockDriverAmendStatusCB *status_cb,
> > + void *cb_opaque,
> > + bool force,
> > + Error **errp)
> > +{
> > + BlockCrypto *crypto = bs->opaque;
> > + QDict *cryptoopts = NULL;
> > + QCryptoBlockCreateOptions *amend_options = NULL;
> > + int ret;
> > +
> > + assert(crypto);
> > + assert(crypto->block);
> > +
> > + crypto->updating_keys = true;
> > +
> > + ret = bdrv_child_refresh_perms(bs, bs->file, errp);
> > + if (ret) {
>
> I can;'t remember - does this need to be "ret < 0" or
> does refresh_perms return positive errnos ?
I don't really know but looking at the source the
bdrv_child_refresh_perms calls the bdrv_child_try_set_perm
which seems to forward only negative error codes,
so I'll do this here as well.
Also an iotest for this is a must, now I remember.
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 06/10] qcow2: implement crypto amend options
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (4 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 05/10] block/crypto: implement the encryption key management Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 14:06 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 07/10] block: add x-blockdev-amend qmp command Maxim Levitsky
` (3 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
---
block/qcow2.c | 79 ++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 63 insertions(+), 16 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 376bb416fd..8dff4c6b5f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -171,6 +171,25 @@ static ssize_t qcow2_crypto_hdr_write_func(QCryptoBlock *block, size_t offset,
return ret;
}
+static QCryptoBlockCreateOptions*
+qcow2_extract_crypto_create_opts(QemuOpts *opts, const char *fmt, Error **errp)
+{
+ QDict *cryptoopts_qdict;
+ QCryptoBlockCreateOptions *cryptoopts;
+ QDict *opts_qdict;
+
+ /* Extract "encrypt." options into a qdict */
+ opts_qdict = qemu_opts_to_qdict(opts, NULL);
+ qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
+ qobject_unref(opts_qdict);
+
+ /* Build QCryptoBlockCreateOptions object from qdict */
+ qdict_put_str(cryptoopts_qdict, "format", "luks");
+ cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);
+ qobject_unref(cryptoopts_qdict);
+ return cryptoopts;
+}
+
/*
* read qcow2 extension and fill bs
@@ -4366,20 +4385,10 @@ static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
static bool qcow2_measure_luks_headerlen(QemuOpts *opts, size_t *len,
Error **errp)
{
- QDict *opts_qdict;
- QDict *cryptoopts_qdict;
QCryptoBlockCreateOptions *cryptoopts;
QCryptoBlock *crypto;
- /* Extract "encrypt." options into a qdict */
- opts_qdict = qemu_opts_to_qdict(opts, NULL);
- qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
- qobject_unref(opts_qdict);
-
- /* Build QCryptoBlockCreateOptions object from qdict */
- qdict_put_str(cryptoopts_qdict, "format", "luks");
- cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);
- qobject_unref(cryptoopts_qdict);
+ cryptoopts = qcow2_extract_crypto_create_opts(opts, "luks", errp);
if (!cryptoopts) {
return false;
}
@@ -4756,6 +4765,7 @@ typedef enum Qcow2AmendOperation {
* invocation from an operation change */
QCOW2_NO_OPERATION = 0,
+ QCOW2_UPDATING_ENCRYPTION,
QCOW2_CHANGING_REFCOUNT_ORDER,
QCOW2_DOWNGRADING,
} Qcow2AmendOperation;
@@ -4840,6 +4850,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
int ret;
QemuOptDesc *desc = opts->list->desc;
Qcow2AmendHelperCBInfo helper_cb_info;
+ bool encryption_update = false;
while (desc && desc->name) {
if (!qemu_opt_find(opts, desc->name)) {
@@ -4888,9 +4899,22 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
return -ENOTSUP;
}
} else if (g_str_has_prefix(desc->name, "encrypt.")) {
- error_setg(errp,
- "Changing the encryption parameters is not supported");
- return -ENOTSUP;
+
+ if (!s->crypto) {
+ error_setg(errp,
+ "Can't amend encryption options - encryption not supported");
+ return -ENOTSUP;
+
+ }
+
+ if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
+ error_setg(errp,
+ "Only LUKS encryption options can be amended");
+ return -ENOTSUP;
+ }
+
+ encryption_update = true;
+
} else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) {
cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
cluster_size);
@@ -4927,7 +4951,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
"images");
return -EINVAL;
}
- } else {
+ } else {
/* if this point is reached, this probably means a new option was
* added without having it covered here */
abort();
@@ -4940,7 +4964,8 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
.original_status_cb = status_cb,
.original_cb_opaque = cb_opaque,
.total_operations = (new_version < old_version)
- + (s->refcount_bits != refcount_bits)
+ + (s->refcount_bits != refcount_bits) +
+ (encryption_update == true)
};
/* Upgrade first (some features may require compat=1.1) */
@@ -4954,6 +4979,28 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
}
}
+ if (encryption_update) {
+
+ QCryptoBlockCreateOptions *cryptoopts;
+
+ cryptoopts = qcow2_extract_crypto_create_opts(opts, "luks", errp);
+ if (!cryptoopts)
+ return -EINVAL;
+
+ helper_cb_info.current_operation = QCOW2_UPDATING_ENCRYPTION;
+
+ ret = qcrypto_block_amend_options(s->crypto,
+ qcow2_crypto_hdr_read_func,
+ qcow2_crypto_hdr_write_func,
+ bs,
+ cryptoopts,
+ force,
+ errp);
+ if (ret) {
+ return ret;
+ }
+ }
+
if (s->refcount_bits != refcount_bits) {
int refcount_order = ctz32(refcount_bits);
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 06/10] qcow2: implement crypto amend options
2019-08-30 20:56 ` [Qemu-devel] [PATCH 06/10] qcow2: implement crypto amend options Maxim Levitsky
@ 2019-09-06 14:06 ` Daniel P. Berrangé
2019-09-12 19:11 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:06 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:04PM +0300, Maxim Levitsky wrote:
> ---
> block/qcow2.c | 79 ++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 63 insertions(+), 16 deletions(-)
>
> @@ -4888,9 +4899,22 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> return -ENOTSUP;
> }
> } else if (g_str_has_prefix(desc->name, "encrypt.")) {
> - error_setg(errp,
> - "Changing the encryption parameters is not supported");
> - return -ENOTSUP;
> +
> + if (!s->crypto) {
> + error_setg(errp,
> + "Can't amend encryption options - encryption not supported");
> + return -ENOTSUP;
> +
> + }
Perhaps ' - encryption not present', and -EINVAL
> +
> + if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
> + error_setg(errp,
> + "Only LUKS encryption options can be amended");
> + return -ENOTSUP;
> + }
> +
> + encryption_update = true;
> +
> } else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) {
> cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
> cluster_size);
> @@ -4927,7 +4951,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> "images");
> return -EINVAL;
> }
> - } else {
> + } else {
Accidental change
> /* if this point is reached, this probably means a new option was
> * added without having it covered here */
> abort();
> @@ -4940,7 +4964,8 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> .original_status_cb = status_cb,
> .original_cb_opaque = cb_opaque,
> .total_operations = (new_version < old_version)
> - + (s->refcount_bits != refcount_bits)
> + + (s->refcount_bits != refcount_bits) +
> + (encryption_update == true)
> };
>
> /* Upgrade first (some features may require compat=1.1) */
> @@ -4954,6 +4979,28 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> }
> }
>
> + if (encryption_update) {
> +
Redundant blank line
> + QCryptoBlockCreateOptions *cryptoopts;
> +
> + cryptoopts = qcow2_extract_crypto_create_opts(opts, "luks", errp);
> + if (!cryptoopts)
> + return -EINVAL;
> +
> + helper_cb_info.current_operation = QCOW2_UPDATING_ENCRYPTION;
> +
> + ret = qcrypto_block_amend_options(s->crypto,
> + qcow2_crypto_hdr_read_func,
> + qcow2_crypto_hdr_write_func,
> + bs,
> + cryptoopts,
> + force,
> + errp);
> + if (ret) {
Check ret < 0
> + return ret;
> + }
> + }
> +
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 06/10] qcow2: implement crypto amend options
2019-09-06 14:06 ` Daniel P. Berrangé
@ 2019-09-12 19:11 ` Maxim Levitsky
0 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-12 19:11 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Max Reitz,
John Snow
On Fri, 2019-09-06 at 15:06 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:04PM +0300, Maxim Levitsky wrote:
> > ---
> > block/qcow2.c | 79 ++++++++++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 63 insertions(+), 16 deletions(-)
> >
> > @@ -4888,9 +4899,22 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> > return -ENOTSUP;
> > }
> > } else if (g_str_has_prefix(desc->name, "encrypt.")) {
> > - error_setg(errp,
> > - "Changing the encryption parameters is not supported");
> > - return -ENOTSUP;
> > +
> > + if (!s->crypto) {
> > + error_setg(errp,
> > + "Can't amend encryption options - encryption not supported");
> > + return -ENOTSUP;
> > +
> > + }
>
> Perhaps ' - encryption not present', and -EINVAL
Agree. Fixed.
>
> > +
> > + if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
> > + error_setg(errp,
> > + "Only LUKS encryption options can be amended");
> > + return -ENOTSUP;
> > + }
> > +
> > + encryption_update = true;
> > +
> > } else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) {
> > cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
> > cluster_size);
> > @@ -4927,7 +4951,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> > "images");
> > return -EINVAL;
> > }
> > - } else {
> > + } else {
>
> Accidental change
Fixed.
>
> > /* if this point is reached, this probably means a new option was
> > * added without having it covered here */
> > abort();
> > @@ -4940,7 +4964,8 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> > .original_status_cb = status_cb,
> > .original_cb_opaque = cb_opaque,
> > .total_operations = (new_version < old_version)
> > - + (s->refcount_bits != refcount_bits)
> > + + (s->refcount_bits != refcount_bits) +
> > + (encryption_update == true)
> > };
> >
> > /* Upgrade first (some features may require compat=1.1) */
> > @@ -4954,6 +4979,28 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> > }
> > }
> >
> > + if (encryption_update) {
> > +
>
> Redundant blank line
Fixed.
>
> > + QCryptoBlockCreateOptions *cryptoopts;
> > +
> > + cryptoopts = qcow2_extract_crypto_create_opts(opts, "luks", errp);
> > + if (!cryptoopts)
> > + return -EINVAL;
> > +
> > + helper_cb_info.current_operation = QCOW2_UPDATING_ENCRYPTION;
> > +
> > + ret = qcrypto_block_amend_options(s->crypto,
> > + qcow2_crypto_hdr_read_func,
> > + qcow2_crypto_hdr_write_func,
> > + bs,
> > + cryptoopts,
> > + force,
> > + errp);
> > + if (ret) {
>
> Check ret < 0
Fixed.
>
> > + return ret;
> > + }
> > + }
> > +
>
> Regards,
> Daniel
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 07/10] block: add x-blockdev-amend qmp command
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (5 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 06/10] qcow2: implement crypto amend options Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-08-30 20:56 ` [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend Maxim Levitsky
` (2 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
block/Makefile.objs | 2 +-
block/amend.c | 116 ++++++++++++++++++++++++++++++++++++++
include/block/block_int.h | 23 ++++++--
qapi/block-core.json | 26 +++++++++
qapi/job.json | 4 +-
5 files changed, 163 insertions(+), 8 deletions(-)
create mode 100644 block/amend.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 35f3bca4d9..10d0308792 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -18,7 +18,7 @@ block-obj-y += block-backend.o snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += file-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
-block-obj-y += null.o mirror.o commit.o io.o create.o
+block-obj-y += null.o mirror.o commit.o io.o create.o amend.o
block-obj-y += throttle-groups.o
block-obj-$(CONFIG_LINUX) += nvme.o
diff --git a/block/amend.c b/block/amend.c
new file mode 100644
index 0000000000..9bd28e08e7
--- /dev/null
+++ b/block/amend.c
@@ -0,0 +1,116 @@
+/*
+ * Block layer code related to image options amend
+ *
+ * Copyright (c) 2018 Kevin Wolf <kwolf@redhat.com>
+ * Copyright (c) 2019 Maxim Levitsky <mlevitsk@redhat.com>
+ *
+ * Heavily based on create.c
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "qemu/job.h"
+#include "qemu/main-loop.h"
+#include "qapi/qapi-commands-block-core.h"
+#include "qapi/qapi-visit-block-core.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/error.h"
+
+typedef struct BlockdevAmendJob {
+ Job common;
+ BlockdevCreateOptions *opts;
+ BlockDriverState *bs;
+ bool force;
+} BlockdevAmendJob;
+
+static int coroutine_fn blockdev_amend_run(Job *job, Error **errp)
+{
+ BlockdevAmendJob *s = container_of(job, BlockdevAmendJob, common);
+ int ret;
+
+ job_progress_set_remaining(&s->common, 1);
+ ret = s->bs->drv->bdrv_co_amend(s->bs, s->opts, s->force, errp);
+ job_progress_update(&s->common, 1);
+
+ qapi_free_BlockdevCreateOptions(s->opts);
+
+ return ret;
+}
+
+static const JobDriver blockdev_amend_job_driver = {
+ .instance_size = sizeof(BlockdevAmendJob),
+ .job_type = JOB_TYPE_AMEND,
+ .run = blockdev_amend_run,
+};
+
+void qmp_x_blockdev_amend(const char *job_id,
+ const char *node_name,
+ BlockdevCreateOptions *options,
+ bool has_force,
+ bool force,
+ Error **errp)
+{
+ BlockdevAmendJob *s;
+ const char *fmt = BlockdevDriver_str(options->driver);
+ BlockDriver *drv = bdrv_find_format(fmt);
+ BlockDriverState *bs = bdrv_find_node(node_name);
+
+ /*
+ * If the driver is in the schema, we know that it exists. But it may not
+ * be whitelisted.
+ */
+ assert(drv);
+ if (bdrv_uses_whitelist() && !bdrv_is_whitelisted(drv, false)) {
+ error_setg(errp, "Driver is not whitelisted");
+ return;
+ }
+
+ if (bs->drv != drv) {
+ error_setg(errp,
+ "x-blockdev-amend doesn't support changing the block driver");
+ return;
+
+ }
+
+ /* Error out if the driver doesn't support .bdrv_co_amend */
+ if (!drv->bdrv_co_amend) {
+ error_setg(errp, "Driver does not support x-blockdev-amend");
+ return;
+ }
+
+ /*
+ * Create the block job
+ * TODO Running in the main context. Block drivers need to error out or add
+ * locking when they use a BDS in a different AioContext.
+ */
+ s = job_create(job_id, &blockdev_amend_job_driver, NULL,
+ qemu_get_aio_context(), JOB_DEFAULT | JOB_MANUAL_DISMISS,
+ NULL, NULL, errp);
+ if (!s) {
+ return;
+ }
+
+ s->bs = bs,
+ s->opts = QAPI_CLONE(BlockdevCreateOptions, options),
+ s->force = has_force ? force : false;
+
+ job_start(&s->common);
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c6aa05214f..546c84957c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -133,11 +133,28 @@ struct BlockDriver {
int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
+
+
int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
Error **errp);
int coroutine_fn (*bdrv_co_create_opts)(const char *filename,
QemuOpts *opts,
Error **errp);
+
+
+ int coroutine_fn (*bdrv_co_amend)(BlockDriverState *bs,
+ BlockdevCreateOptions *opts,
+ bool force,
+ Error **errp);
+
+ int (*bdrv_amend_options)(BlockDriverState *bs,
+ QemuOpts *opts,
+ BlockDriverAmendStatusCB *status_cb,
+ void *cb_opaque,
+ bool force,
+ Error **errp);
+
+
int (*bdrv_make_empty)(BlockDriverState *bs);
/*
@@ -399,12 +416,6 @@ struct BlockDriver {
BdrvCheckResult *result,
BdrvCheckMode fix);
- int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb,
- void *cb_opaque,
- bool force,
- Error **errp);
-
void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
/* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e6edd641f1..7900914506 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4650,6 +4650,32 @@
'data': { 'job-id': 'str',
'options': 'BlockdevCreateOptions' } }
+##
+# @x-blockdev-amend:
+#
+# Starts a job to amend format specific options of an existing open block device.
+# The job is automatically finalized, but a manual job-dismiss is required.
+#
+# @job-id: Identifier for the newly created job.
+#
+# @node-name: Name of the block node to work on
+#
+# @options: Options (same as for image creation)
+#
+# @force: Allow unsafe operations, format specific
+# For luks that allows erase of the last active keyslot
+# (permanent loss of data),
+# and replacement of an active keyslot
+# (possible loss of data if IO error happens)
+#
+# Since: 4.2
+##
+{ 'command': 'x-blockdev-amend',
+ 'data': { 'job-id': 'str',
+ 'node-name': 'str',
+ 'options': 'BlockdevCreateOptions',
+ '*force': 'bool' } }
+
##
# @blockdev-open-tray:
#
diff --git a/qapi/job.json b/qapi/job.json
index a121b615fb..342d29a7aa 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -19,10 +19,12 @@
#
# @create: image creation job type, see "blockdev-create" (since 3.0)
#
+# @amend: image options amend job type, see "x-blockdev-amend" (since 4.2)
+#
# Since: 1.7
##
{ 'enum': 'JobType',
- 'data': ['commit', 'stream', 'mirror', 'backup', 'create'] }
+ 'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend'] }
##
# @JobStatus:
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (6 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 07/10] block: add x-blockdev-amend qmp command Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 14:10 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 09/10] block/qcow2: " Maxim Levitsky
2019-08-30 20:56 ` [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management Maxim Levitsky
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
block/crypto.c | 86 +++++++++++++++++++++++++++++++++-----------
qapi/block-core.json | 4 +--
2 files changed, 68 insertions(+), 22 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index dbd95a99ba..9cb668ff0e 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -534,6 +534,17 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
luks_opts = &create_options->u.luks;
+ if (!luks_opts->has_size) {
+ error_setg(errp, "'size' is manadatory for image creation");
+ return -EINVAL;
+ }
+
+ if (!luks_opts->has_file) {
+ error_setg(errp, "'file' is manadatory for image creation");
+ return -EINVAL;
+ }
+
+
bs = bdrv_open_blockdev_ref(luks_opts->file, errp);
if (bs == NULL) {
return -EIO;
@@ -667,6 +678,39 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
}
+static int
+block_crypto_amend_options_generic(BlockDriverState *bs,
+ QCryptoBlockCreateOptions *amend_options,
+ bool force,
+ Error **errp)
+{
+ BlockCrypto *crypto = bs->opaque;
+ int ret = -1;
+
+ assert(crypto);
+ assert(crypto->block);
+
+ /* apply for exclusive write permissions to the underlying file*/
+ crypto->updating_keys = true;
+ ret = bdrv_child_refresh_perms(bs, bs->file, errp);
+ if (ret) {
+ goto cleanup;
+ }
+
+ ret = qcrypto_block_amend_options(crypto->block,
+ block_crypto_read_func,
+ block_crypto_write_func,
+ bs,
+ amend_options,
+ force,
+ errp);
+cleanup:
+ /* release exclusive write permissions to the underlying file*/
+ crypto->updating_keys = false;
+ bdrv_child_refresh_perms(bs, bs->file, errp);
+ return ret;
+}
+
static int
block_crypto_amend_options(BlockDriverState *bs,
QemuOpts *opts,
@@ -678,44 +722,45 @@ block_crypto_amend_options(BlockDriverState *bs,
BlockCrypto *crypto = bs->opaque;
QDict *cryptoopts = NULL;
QCryptoBlockCreateOptions *amend_options = NULL;
- int ret;
+ int ret= -EINVAL;
assert(crypto);
assert(crypto->block);
- crypto->updating_keys = true;
-
- ret = bdrv_child_refresh_perms(bs, bs->file, errp);
- if (ret) {
- goto cleanup;
- }
-
cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
&block_crypto_create_opts_luks,
true);
qdict_put_str(cryptoopts, "format", "luks");
amend_options = block_crypto_create_opts_init(cryptoopts, errp);
+
if (!amend_options) {
- ret = -EINVAL;
- goto cleanup;
+ goto out;
}
- ret = qcrypto_block_amend_options(crypto->block,
- block_crypto_read_func,
- block_crypto_write_func,
- bs,
- amend_options,
- force,
- errp);
-cleanup:
- crypto->updating_keys = false;
- bdrv_child_refresh_perms(bs, bs->file, errp);
+ ret = block_crypto_amend_options_generic(bs, amend_options, force, errp);
+out:
qapi_free_QCryptoBlockCreateOptions(amend_options);
qobject_unref(cryptoopts);
return ret;
}
+static int
+coroutine_fn block_crypto_co_amend(BlockDriverState *bs,
+ BlockdevCreateOptions *opts,
+ bool force,
+ Error **errp)
+{
+ QCryptoBlockCreateOptions amend_opts;
+
+ amend_opts = (QCryptoBlockCreateOptions) {
+ .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+ .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(&opts->u.luks),
+ };
+
+ return block_crypto_amend_options_generic(bs, &amend_opts, force, errp);
+}
+
static void
block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
@@ -774,6 +819,7 @@ static BlockDriver bdrv_crypto_luks = {
.bdrv_get_info = block_crypto_get_info_luks,
.bdrv_get_specific_info = block_crypto_get_specific_info_luks,
.bdrv_amend_options = block_crypto_amend_options,
+ .bdrv_co_amend = block_crypto_co_amend,
.strong_runtime_opts = block_crypto_strong_runtime_opts,
};
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7900914506..02375fb59a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4220,8 +4220,8 @@
##
{ 'struct': 'BlockdevCreateOptionsLUKS',
'base': 'QCryptoBlockCreateOptionsLUKS',
- 'data': { 'file': 'BlockdevRef',
- 'size': 'size',
+ 'data': { '*file': 'BlockdevRef',
+ '*size': 'size',
'*preallocation': 'PreallocMode' } }
##
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend
2019-08-30 20:56 ` [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend Maxim Levitsky
@ 2019-09-06 14:10 ` Daniel P. Berrangé
2019-09-12 19:18 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:10 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:06PM +0300, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> block/crypto.c | 86 +++++++++++++++++++++++++++++++++-----------
> qapi/block-core.json | 4 +--
> 2 files changed, 68 insertions(+), 22 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> static int
> block_crypto_amend_options(BlockDriverState *bs,
> QemuOpts *opts,
> @@ -678,44 +722,45 @@ block_crypto_amend_options(BlockDriverState *bs,
> BlockCrypto *crypto = bs->opaque;
> QDict *cryptoopts = NULL;
> QCryptoBlockCreateOptions *amend_options = NULL;
> - int ret;
> + int ret= -EINVAL;
nitpick - space before '='
>
> assert(crypto);
> assert(crypto->block);
>
> - crypto->updating_keys = true;
> -
> - ret = bdrv_child_refresh_perms(bs, bs->file, errp);
> - if (ret) {
> - goto cleanup;
> - }
> -
> cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> &block_crypto_create_opts_luks,
> true);
>
> qdict_put_str(cryptoopts, "format", "luks");
> amend_options = block_crypto_create_opts_init(cryptoopts, errp);
> +
> if (!amend_options) {
> - ret = -EINVAL;
> - goto cleanup;
> + goto out;
> }
>
> - ret = qcrypto_block_amend_options(crypto->block,
> - block_crypto_read_func,
> - block_crypto_write_func,
> - bs,
> - amend_options,
> - force,
> - errp);
> -cleanup:
> - crypto->updating_keys = false;
> - bdrv_child_refresh_perms(bs, bs->file, errp);
> + ret = block_crypto_amend_options_generic(bs, amend_options, force, errp);
> +out:
No need to rename the "cleanup" label to "out"
> qapi_free_QCryptoBlockCreateOptions(amend_options);
> qobject_unref(cryptoopts);
> return ret;
> }
>
> +static int
> +coroutine_fn block_crypto_co_amend(BlockDriverState *bs,
> + BlockdevCreateOptions *opts,
> + bool force,
> + Error **errp)
> +{
> + QCryptoBlockCreateOptions amend_opts;
> +
> + amend_opts = (QCryptoBlockCreateOptions) {
> + .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
> + .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(&opts->u.luks),
> + };
> +
> + return block_crypto_amend_options_generic(bs, &amend_opts, force, errp);
> +}
> +
>
> static void
> block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
> @@ -774,6 +819,7 @@ static BlockDriver bdrv_crypto_luks = {
> .bdrv_get_info = block_crypto_get_info_luks,
> .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
> .bdrv_amend_options = block_crypto_amend_options,
> + .bdrv_co_amend = block_crypto_co_amend,
>
> .strong_runtime_opts = block_crypto_strong_runtime_opts,
> };
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7900914506..02375fb59a 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4220,8 +4220,8 @@
> ##
> { 'struct': 'BlockdevCreateOptionsLUKS',
> 'base': 'QCryptoBlockCreateOptionsLUKS',
> - 'data': { 'file': 'BlockdevRef',
> - 'size': 'size',
> + 'data': { '*file': 'BlockdevRef',
> + '*size': 'size',
Docs comment to explain they are mandatory for create
> '*preallocation': 'PreallocMode' } }
>
> ##
> --
> 2.17.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend
2019-09-06 14:10 ` Daniel P. Berrangé
@ 2019-09-12 19:18 ` Maxim Levitsky
0 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-12 19:18 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Max Reitz,
John Snow
On Fri, 2019-09-06 at 15:10 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:06PM +0300, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > block/crypto.c | 86 +++++++++++++++++++++++++++++++++-----------
> > qapi/block-core.json | 4 +--
> > 2 files changed, 68 insertions(+), 22 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> > static int
> > block_crypto_amend_options(BlockDriverState *bs,
> > QemuOpts *opts,
> > @@ -678,44 +722,45 @@ block_crypto_amend_options(BlockDriverState *bs,
> > BlockCrypto *crypto = bs->opaque;
> > QDict *cryptoopts = NULL;
> > QCryptoBlockCreateOptions *amend_options = NULL;
> > - int ret;
> > + int ret= -EINVAL;
>
> nitpick - space before '='
Done. This is one of the few errors that checkpatch.pl does catch,
but apparently I forgot to run it on this patch.
>
> >
> > assert(crypto);
> > assert(crypto->block);
> >
> > - crypto->updating_keys = true;
> > -
> > - ret = bdrv_child_refresh_perms(bs, bs->file, errp);
> > - if (ret) {
> > - goto cleanup;
> > - }
> > -
> > cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> > &block_crypto_create_opts_luks,
> > true);
> >
> > qdict_put_str(cryptoopts, "format", "luks");
> > amend_options = block_crypto_create_opts_init(cryptoopts, errp);
> > +
> > if (!amend_options) {
> > - ret = -EINVAL;
> > - goto cleanup;
> > + goto out;
> > }
> >
> > - ret = qcrypto_block_amend_options(crypto->block,
> > - block_crypto_read_func,
> > - block_crypto_write_func,
> > - bs,
> > - amend_options,
> > - force,
> > - errp);
> > -cleanup:
> > - crypto->updating_keys = false;
> > - bdrv_child_refresh_perms(bs, bs->file, errp);
> > + ret = block_crypto_amend_options_generic(bs, amend_options, force, errp);
> > +out:
>
> No need to rename the "cleanup" label to "out"
All right.
>
> > qapi_free_QCryptoBlockCreateOptions(amend_options);
> > qobject_unref(cryptoopts);
> > return ret;
> > }
> >
> > +static int
> > +coroutine_fn block_crypto_co_amend(BlockDriverState *bs,
> > + BlockdevCreateOptions *opts,
> > + bool force,
> > + Error **errp)
> > +{
> > + QCryptoBlockCreateOptions amend_opts;
> > +
> > + amend_opts = (QCryptoBlockCreateOptions) {
> > + .format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
> > + .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(&opts->u.luks),
> > + };
> > +
> > + return block_crypto_amend_options_generic(bs, &amend_opts, force, errp);
> > +}
> > +
> >
> > static void
> > block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
> > @@ -774,6 +819,7 @@ static BlockDriver bdrv_crypto_luks = {
> > .bdrv_get_info = block_crypto_get_info_luks,
> > .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
> > .bdrv_amend_options = block_crypto_amend_options,
> > + .bdrv_co_amend = block_crypto_co_amend,
> >
> > .strong_runtime_opts = block_crypto_strong_runtime_opts,
> > };
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 7900914506..02375fb59a 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4220,8 +4220,8 @@
> > ##
> > { 'struct': 'BlockdevCreateOptionsLUKS',
> > 'base': 'QCryptoBlockCreateOptionsLUKS',
> > - 'data': { 'file': 'BlockdevRef',
> > - 'size': 'size',
> > + 'data': { '*file': 'BlockdevRef',
> > + '*size': 'size',
>
> Docs comment to explain they are mandatory for create
Done
>
> > '*preallocation': 'PreallocMode' } }
> >
> > ##
> > --
> > 2.17.2
> >
>
> Regards,
> Daniel
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 09/10] block/qcow2: implement blockdev-amend
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (7 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 14:12 ` Daniel P. Berrangé
2019-08-30 20:56 ` [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management Maxim Levitsky
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Currently only for changing crypto parameters
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
block/qcow2.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
qapi/block-core.json | 4 +--
2 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 8dff4c6b5f..327d2afd9f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3082,6 +3082,18 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
assert(create_options->driver == BLOCKDEV_DRIVER_QCOW2);
qcow2_opts = &create_options->u.qcow2;
+ if (!qcow2_opts->has_size) {
+ error_setg(errp, "Size is manadatory for image creation");
+ return -EINVAL;
+
+ }
+
+ if (!qcow2_opts->has_file) {
+ error_setg(errp, "'file' is manadatory for image creation");
+ return -EINVAL;
+
+ }
+
bs = bdrv_open_blockdev_ref(qcow2_opts->file, errp);
if (bs == NULL) {
return -EIO;
@@ -5112,6 +5124,64 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
return 0;
}
+
+static int coroutine_fn qcow2_co_amend(BlockDriverState *bs,
+ BlockdevCreateOptions *opts,
+ bool force,
+ Error **errp)
+{
+ BlockdevCreateOptionsQcow2 *qopts = &opts->u.qcow2;
+ BDRVQcow2State *s = bs->opaque;
+ int ret;
+
+ /*
+ * This is ugly as hell, in later versions of this patch
+ * something has to be done about this
+ */
+ if (qopts->has_file || qopts->has_size || qopts->has_data_file ||
+ qopts->has_data_file_raw || qopts->has_version ||
+ qopts->has_backing_file || qopts->has_backing_fmt ||
+ qopts->has_cluster_size || qopts->has_preallocation ||
+ qopts->has_lazy_refcounts || qopts->has_refcount_bits) {
+
+ error_setg(errp,
+ "Only LUKS encryption options can be amended for qcow2 with blockdev-amend");
+ return -EOPNOTSUPP;
+
+ }
+
+ if (qopts->has_encrypt) {
+ if (!s->crypto) {
+ error_setg(errp, "QCOW2 image is not encrypted, can't amend");
+ return -EOPNOTSUPP;
+ }
+
+ if (qopts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
+ error_setg(errp,
+ "Amend can't be used to change the qcow2 encryption format");
+ return -EOPNOTSUPP;
+ }
+
+ if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
+ error_setg(errp,
+ "Only LUKS encryption options can be amended for qcow2 with blockdev-amend");
+ return -EOPNOTSUPP;
+ }
+
+ ret = qcrypto_block_amend_options(s->crypto,
+ qcow2_crypto_hdr_read_func,
+ qcow2_crypto_hdr_write_func,
+ bs,
+ qopts->encrypt,
+ force,
+ errp);
+ if (ret) {
+ return ret;
+ }
+ }
+ return 0;
+}
+
/*
* If offset or size are negative, respectively, they will not be included in
* the BLOCK_IMAGE_CORRUPTED event emitted.
@@ -5304,6 +5374,7 @@ BlockDriver bdrv_qcow2 = {
.mutable_opts = mutable_opts,
.bdrv_co_check = qcow2_co_check,
.bdrv_amend_options = qcow2_amend_options,
+ .bdrv_co_amend = qcow2_co_amend,
.bdrv_detach_aio_context = qcow2_detach_aio_context,
.bdrv_attach_aio_context = qcow2_attach_aio_context,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 02375fb59a..ba41744427 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4312,10 +4312,10 @@
# Since: 2.12
##
{ 'struct': 'BlockdevCreateOptionsQcow2',
- 'data': { 'file': 'BlockdevRef',
+ 'data': { '*file': 'BlockdevRef',
'*data-file': 'BlockdevRef',
'*data-file-raw': 'bool',
- 'size': 'size',
+ '*size': 'size',
'*version': 'BlockdevQcow2Version',
'*backing-file': 'str',
'*backing-fmt': 'BlockdevDriver',
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 09/10] block/qcow2: implement blockdev-amend
2019-08-30 20:56 ` [Qemu-devel] [PATCH 09/10] block/qcow2: " Maxim Levitsky
@ 2019-09-06 14:12 ` Daniel P. Berrangé
2019-09-12 19:22 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:12 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:07PM +0300, Maxim Levitsky wrote:
> Currently only for changing crypto parameters
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> block/qcow2.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
> qapi/block-core.json | 4 +--
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8dff4c6b5f..327d2afd9f 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3082,6 +3082,18 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> assert(create_options->driver == BLOCKDEV_DRIVER_QCOW2);
> qcow2_opts = &create_options->u.qcow2;
>
> + if (!qcow2_opts->has_size) {
> + error_setg(errp, "Size is manadatory for image creation");
> + return -EINVAL;
> +
> + }
> +
> + if (!qcow2_opts->has_file) {
> + error_setg(errp, "'file' is manadatory for image creation");
> + return -EINVAL;
> +
> + }
> +
> bs = bdrv_open_blockdev_ref(qcow2_opts->file, errp);
> if (bs == NULL) {
> return -EIO;
> @@ -5112,6 +5124,64 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> return 0;
> }
>
> +
> +static int coroutine_fn qcow2_co_amend(BlockDriverState *bs,
> + BlockdevCreateOptions *opts,
> + bool force,
> + Error **errp)
> +{
> + BlockdevCreateOptionsQcow2 *qopts = &opts->u.qcow2;
> + BDRVQcow2State *s = bs->opaque;
> + int ret;
> +
> + /*
> + * This is ugly as hell, in later versions of this patch
> + * something has to be done about this
> + */
> + if (qopts->has_file || qopts->has_size || qopts->has_data_file ||
> + qopts->has_data_file_raw || qopts->has_version ||
> + qopts->has_backing_file || qopts->has_backing_fmt ||
> + qopts->has_cluster_size || qopts->has_preallocation ||
> + qopts->has_lazy_refcounts || qopts->has_refcount_bits) {
> +
> + error_setg(errp,
> + "Only LUKS encryption options can be amended for qcow2 with blockdev-amend");
> + return -EOPNOTSUPP;
> +
> + }
> +
> + if (qopts->has_encrypt) {
> + if (!s->crypto) {
> + error_setg(errp, "QCOW2 image is not encrypted, can't amend");
> + return -EOPNOTSUPP;
> + }
> +
> + if (qopts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
> + error_setg(errp,
> + "Amend can't be used to change the qcow2 encryption format");
> + return -EOPNOTSUPP;
> + }
> +
> + if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
> + error_setg(errp,
> + "Only LUKS encryption options can be amended for qcow2 with blockdev-amend");
> + return -EOPNOTSUPP;
> + }
> +
> + ret = qcrypto_block_amend_options(s->crypto,
> + qcow2_crypto_hdr_read_func,
> + qcow2_crypto_hdr_write_func,
> + bs,
> + qopts->encrypt,
> + force,
> + errp);
> + if (ret) {
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> /*
> * If offset or size are negative, respectively, they will not be included in
> * the BLOCK_IMAGE_CORRUPTED event emitted.
> @@ -5304,6 +5374,7 @@ BlockDriver bdrv_qcow2 = {
> .mutable_opts = mutable_opts,
> .bdrv_co_check = qcow2_co_check,
> .bdrv_amend_options = qcow2_amend_options,
> + .bdrv_co_amend = qcow2_co_amend,
>
> .bdrv_detach_aio_context = qcow2_detach_aio_context,
> .bdrv_attach_aio_context = qcow2_attach_aio_context,
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 02375fb59a..ba41744427 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4312,10 +4312,10 @@
> # Since: 2.12
> ##
> { 'struct': 'BlockdevCreateOptionsQcow2',
> - 'data': { 'file': 'BlockdevRef',
> + 'data': { '*file': 'BlockdevRef',
> '*data-file': 'BlockdevRef',
> '*data-file-raw': 'bool',
> - 'size': 'size',
> + '*size': 'size',
> '*version': 'BlockdevQcow2Version',
> '*backing-file': 'str',
> '*backing-fmt': 'BlockdevDriver',
Docs comment to say they are mandatory for creation.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 09/10] block/qcow2: implement blockdev-amend
2019-09-06 14:12 ` Daniel P. Berrangé
@ 2019-09-12 19:22 ` Maxim Levitsky
0 siblings, 0 replies; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-12 19:22 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, qemu-devel, Markus Armbruster, Max Reitz,
John Snow
On Fri, 2019-09-06 at 15:12 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:07PM +0300, Maxim Levitsky wrote:
> > Currently only for changing crypto parameters
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > block/qcow2.c | 71 ++++++++++++++++++++++++++++++++++++++++++++
> > qapi/block-core.json | 4 +--
> > 2 files changed, 73 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 8dff4c6b5f..327d2afd9f 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -3082,6 +3082,18 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> > assert(create_options->driver == BLOCKDEV_DRIVER_QCOW2);
> > qcow2_opts = &create_options->u.qcow2;
> >
> > + if (!qcow2_opts->has_size) {
> > + error_setg(errp, "Size is manadatory for image creation");
> > + return -EINVAL;
> > +
> > + }
> > +
> > + if (!qcow2_opts->has_file) {
> > + error_setg(errp, "'file' is manadatory for image creation");
> > + return -EINVAL;
> > +
> > + }
> > +
> > bs = bdrv_open_blockdev_ref(qcow2_opts->file, errp);
> > if (bs == NULL) {
> > return -EIO;
> > @@ -5112,6 +5124,64 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> > return 0;
> > }
> >
> > +
> > +static int coroutine_fn qcow2_co_amend(BlockDriverState *bs,
> > + BlockdevCreateOptions *opts,
> > + bool force,
> > + Error **errp)
> > +{
> > + BlockdevCreateOptionsQcow2 *qopts = &opts->u.qcow2;
> > + BDRVQcow2State *s = bs->opaque;
> > + int ret;
> > +
> > + /*
> > + * This is ugly as hell, in later versions of this patch
> > + * something has to be done about this
> > + */
> > + if (qopts->has_file || qopts->has_size || qopts->has_data_file ||
> > + qopts->has_data_file_raw || qopts->has_version ||
> > + qopts->has_backing_file || qopts->has_backing_fmt ||
> > + qopts->has_cluster_size || qopts->has_preallocation ||
> > + qopts->has_lazy_refcounts || qopts->has_refcount_bits) {
> > +
> > + error_setg(errp,
> > + "Only LUKS encryption options can be amended for qcow2 with blockdev-amend");
> > + return -EOPNOTSUPP;
> > +
> > + }
> > +
> > + if (qopts->has_encrypt) {
> > + if (!s->crypto) {
> > + error_setg(errp, "QCOW2 image is not encrypted, can't amend");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (qopts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
> > + error_setg(errp,
> > + "Amend can't be used to change the qcow2 encryption format");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
> > + error_setg(errp,
> > + "Only LUKS encryption options can be amended for qcow2 with blockdev-amend");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + ret = qcrypto_block_amend_options(s->crypto,
> > + qcow2_crypto_hdr_read_func,
> > + qcow2_crypto_hdr_write_func,
> > + bs,
> > + qopts->encrypt,
> > + force,
> > + errp);
> > + if (ret) {
> > + return ret;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > /*
> > * If offset or size are negative, respectively, they will not be included in
> > * the BLOCK_IMAGE_CORRUPTED event emitted.
> > @@ -5304,6 +5374,7 @@ BlockDriver bdrv_qcow2 = {
> > .mutable_opts = mutable_opts,
> > .bdrv_co_check = qcow2_co_check,
> > .bdrv_amend_options = qcow2_amend_options,
> > + .bdrv_co_amend = qcow2_co_amend,
> >
> > .bdrv_detach_aio_context = qcow2_detach_aio_context,
> > .bdrv_attach_aio_context = qcow2_attach_aio_context,
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 02375fb59a..ba41744427 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4312,10 +4312,10 @@
> > # Since: 2.12
> > ##
> > { 'struct': 'BlockdevCreateOptionsQcow2',
> > - 'data': { 'file': 'BlockdevRef',
> > + 'data': { '*file': 'BlockdevRef',
> > '*data-file': 'BlockdevRef',
> > '*data-file-raw': 'bool',
> > - 'size': 'size',
> > + '*size': 'size',
> > '*version': 'BlockdevQcow2Version',
> > '*backing-file': 'str',
> > '*backing-fmt': 'BlockdevDriver',
>
> Docs comment to say they are mandatory for creation.
Done
>
>
> Regards,
> Daniel
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management
2019-08-30 20:55 [Qemu-devel] [PATCH 00/10] RFC crypto/luks: encryption key managment using amend interface Maxim Levitsky
` (8 preceding siblings ...)
2019-08-30 20:56 ` [Qemu-devel] [PATCH 09/10] block/qcow2: " Maxim Levitsky
@ 2019-08-30 20:56 ` Maxim Levitsky
2019-09-06 14:14 ` Daniel P. Berrangé
9 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-08-30 20:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé,
qemu-block, Markus Armbruster, Max Reitz, Maxim Levitsky,
John Snow
Note that currently I add tests 300-302, which are
placeholders to ease the rebase. In final version
of these patches I will update these.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
tests/qemu-iotests/087.out | 6 +-
tests/qemu-iotests/134.out | 2 +-
tests/qemu-iotests/158.out | 4 +-
tests/qemu-iotests/188.out | 2 +-
tests/qemu-iotests/189.out | 4 +-
tests/qemu-iotests/198.out | 4 +-
tests/qemu-iotests/300 | 202 +++++++++++++++++++++++++
tests/qemu-iotests/300.out | 98 ++++++++++++
tests/qemu-iotests/301 | 90 +++++++++++
tests/qemu-iotests/301.out | 30 ++++
tests/qemu-iotests/302 | 247 +++++++++++++++++++++++++++++++
tests/qemu-iotests/302.out | 18 +++
tests/qemu-iotests/common.filter | 6 +-
tests/qemu-iotests/group | 8 +
14 files changed, 708 insertions(+), 13 deletions(-)
create mode 100755 tests/qemu-iotests/300
create mode 100644 tests/qemu-iotests/300.out
create mode 100755 tests/qemu-iotests/301
create mode 100644 tests/qemu-iotests/301.out
create mode 100644 tests/qemu-iotests/302
create mode 100644 tests/qemu-iotests/302.out
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 2d92ea847b..b61ba638af 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -34,7 +34,7 @@ QMP_VERSION
=== Encrypted image QCow ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
Testing:
QMP_VERSION
{"return": {}}
@@ -46,7 +46,7 @@ QMP_VERSION
=== Encrypted image LUKS ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encrypt.format=luks encrypt.key-secret=sec0
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
Testing:
QMP_VERSION
{"return": {}}
@@ -58,7 +58,7 @@ QMP_VERSION
=== Missing driver ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
Testing: -S
QMP_VERSION
{"return": {}}
diff --git a/tests/qemu-iotests/134.out b/tests/qemu-iotests/134.out
index 09d46f6b17..4abc5b5f7d 100644
--- a/tests/qemu-iotests/134.out
+++ b/tests/qemu-iotests/134.out
@@ -1,5 +1,5 @@
QA output created by 134
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
== reading whole image ==
read 134217728/134217728 bytes at offset 0
diff --git a/tests/qemu-iotests/158.out b/tests/qemu-iotests/158.out
index 6def216e55..f28a17626b 100644
--- a/tests/qemu-iotests/158.out
+++ b/tests/qemu-iotests/158.out
@@ -1,6 +1,6 @@
QA output created by 158
== create base ==
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 encryption=on
== writing whole image ==
wrote 134217728/134217728 bytes at offset 0
@@ -10,7 +10,7 @@ wrote 134217728/134217728 bytes at offset 0
read 134217728/134217728 bytes at offset 0
128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== create overlay ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base encryption=on encrypt.key-secret=sec0
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base encryption=on
== writing part of a cluster ==
wrote 1024/1024 bytes at offset 0
diff --git a/tests/qemu-iotests/188.out b/tests/qemu-iotests/188.out
index c568ef3701..5426861b18 100644
--- a/tests/qemu-iotests/188.out
+++ b/tests/qemu-iotests/188.out
@@ -1,5 +1,5 @@
QA output created by 188
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216
== reading whole image ==
read 16777216/16777216 bytes at offset 0
diff --git a/tests/qemu-iotests/189.out b/tests/qemu-iotests/189.out
index a0b7c9c24c..bc213cbe14 100644
--- a/tests/qemu-iotests/189.out
+++ b/tests/qemu-iotests/189.out
@@ -1,6 +1,6 @@
QA output created by 189
== create base ==
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216
== writing whole image ==
wrote 16777216/16777216 bytes at offset 0
@@ -10,7 +10,7 @@ wrote 16777216/16777216 bytes at offset 0
read 16777216/16777216 bytes at offset 0
16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== create overlay ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base encrypt.format=luks encrypt.key-secret=sec1 encrypt.iter-time=10
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base
== writing part of a cluster ==
wrote 1024/1024 bytes at offset 0
diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index e86b175e39..2eff420795 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -1,12 +1,12 @@
QA output created by 198
== create base ==
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216
== writing whole image base ==
wrote 16777216/16777216 bytes at offset 0
16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== create overlay ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base encrypt.format=luks encrypt.key-secret=sec1 encrypt.iter-time=10
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base
== writing whole image layer ==
wrote 16777216/16777216 bytes at offset 0
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
new file mode 100755
index 0000000000..5b65ef95de
--- /dev/null
+++ b/tests/qemu-iotests/300
@@ -0,0 +1,202 @@
+#!/usr/bin/env bash
+#
+# Test encryption key management with luks
+# Based on 134
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mlevitsk@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2 luks
+_supported_proto file #TODO
+
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
+
+if [ "$IMGFMT" = "qcow2" ] ; then
+ PR="encrypt."
+ EXTRA_IMG_ARGS="-o encrypt.format=luks"
+fi
+
+
+# secrets: you are supposed to see the password as *******, see :-)
+S0="--object secret,id=sec0,data=hunter0"
+S1="--object secret,id=sec1,data=hunter1"
+S2="--object secret,id=sec2,data=hunter2"
+S3="--object secret,id=sec3,data=hunter3"
+S4="--object secret,id=sec4,data=hunter4"
+SECRETS="$S0 $S1 $S2 $S3 $S4"
+
+# image with given secret
+IMGS0="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec0"
+IMGS1="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec1"
+IMGS2="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec2"
+IMGS3="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec3"
+IMGS4="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec4"
+
+
+echo "== creating a test image =="
+_make_test_img $S4 $EXTRA_IMG_ARGS -o ${PR}key-secret=sec4,${PR}iter-time=10,${PR}slot=4 32M
+
+echo
+echo "== test that key 4 opens the image =="
+$QEMU_IO $S4 -c "read 0 4096" $IMGS4 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== adding a password to slot 0 =="
+$QEMU_IMG amend $SECRETS $IMGS4 -o ${PR}key-secret=sec0,${PR}iter-time=10
+echo "== adding a password to slot 1 =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}key-secret=sec1,${PR}iter-time=10
+echo "== adding a password to slot 3 =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}key-secret=sec3,${PR}iter-time=10,${PR}slot=3
+echo "== adding a password to slot 2 =="
+$QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec2,${PR}iter-time=10
+
+
+echo "== erase slot 4 =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=4 | _filter_img_create
+
+
+echo
+echo "== all secrets should work =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+ $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
+done
+
+echo
+echo "== erase slot 0 and try it =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec0 | _filter_img_create
+$QEMU_IO $SECRETS -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== erase slot 2 and try it =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=2| _filter_img_create
+$QEMU_IO $SECRETS -c "read 0 4096" $IMGS2 | _filter_qemu_io | _filter_testdir
+
+
+# at this point slots 1 and 3 should be active
+
+echo
+echo "== filling 4 slots with secret 2 =="
+for i in $(seq 0 3) ; do
+ $QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec2,${PR}iter-time=10
+done
+
+echo
+echo "== adding secret 0 =="
+ $QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec0,${PR}iter-time=10
+
+echo
+echo "== adding secret 3 (last slot) =="
+ $QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec3,${PR}iter-time=10
+
+echo
+echo "== trying to add another slot (should fail) =="
+$QEMU_IMG amend $SECRETS $IMGS2 -o ${PR}key-secret=sec3,${PR}iter-time=10
+
+echo
+echo "== all secrets should work again =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+ $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
+done
+
+echo
+echo "== erase all keys of secret 2=="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec2
+echo "== erase all keys of secret 1=="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec1
+echo "== erase all keys of secret 0=="
+$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}active=off,${PR}key-secret=sec0
+echo "== erase all keys of secret 3, except a remaining key =="
+$QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}active=off,${PR}key-secret=sec3
+
+
+echo
+echo "== only secret3 should work now =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+ $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
+done
+
+echo
+echo "== add secret0 =="
+$QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec0,${PR}iter-time=10
+
+echo "== erase secret3 =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}active=off,${PR}key-secret=sec3
+
+echo
+echo "== only secret0 should work now =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+ $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
+done
+
+echo
+echo "== replace secret0 with secret1 (should fail) =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}key-secret=sec1,${PR}iter-time=10,${PR}slot=0
+
+echo
+echo "== replace secret0 with secret1 with force (should work) =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}key-secret=sec1,${PR}iter-time=10,${PR}slot=0 --force
+
+echo
+echo "== only secret1 should work now =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+ $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
+done
+
+
+echo
+echo "== erase last secret (should fail) =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=0
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec1
+
+
+echo "== erase non existing secrets (should fail) =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec5 --force
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec0 --force
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=1 --force
+
+echo
+echo "== erase last secret with force by slot (should work) =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=0 --force
+
+echo
+echo "== we have no secrets now, data is lost forever =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+ $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
+
diff --git a/tests/qemu-iotests/300.out b/tests/qemu-iotests/300.out
new file mode 100644
index 0000000000..d809595381
--- /dev/null
+++ b/tests/qemu-iotests/300.out
@@ -0,0 +1,98 @@
+QA output created by 300
+== creating a test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432
+
+== test that key 4 opens the image ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== adding a password to slot 0 ==
+== adding a password to slot 1 ==
+== adding a password to slot 3 ==
+== adding a password to slot 2 ==
+== erase slot 4 ==
+
+== all secrets should work ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== erase slot 0 and try it ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+
+== erase slot 2 and try it ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+
+== filling 4 slots with secret 2 ==
+
+== adding secret 0 ==
+
+== adding secret 3 (last slot) ==
+
+== trying to add another slot (should fail) ==
+qemu-img: Can't add a keyslot - all key slots are in use
+
+== all secrets should work again ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== erase all keys of secret 2==
+== erase all keys of secret 1==
+== erase all keys of secret 0==
+== erase all keys of secret 3, except a remaining key ==
+
+== only secret3 should work now ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== add secret0 ==
+== erase secret3 ==
+
+== only secret0 should work now ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+
+== replace secret0 with secret1 (should fail) ==
+qemu-img: Can't update an active key slot 0
+
+== replace secret0 with secret1 with force (should work) ==
+
+== only secret1 should work now ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+
+== erase last secret (should fail) ==
+qemu-img: Only one slot active - can't erase
+qemu-img: Only one slot active - can't erase
+== erase non existing secrets (should fail) ==
+qemu-img: No secret with id 'sec5'
+qemu-img: Didn't erase a keyslot, because no keyslots match the given password
+
+== erase last secret with force by slot (should work) ==
+
+== we have no secrets now, data is lost forever ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+*** done
diff --git a/tests/qemu-iotests/301 b/tests/qemu-iotests/301
new file mode 100755
index 0000000000..ba29bf10a0
--- /dev/null
+++ b/tests/qemu-iotests/301
@@ -0,0 +1,90 @@
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mlevitsk@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt luks
+_supported_proto file #TODO
+
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
+
+# you are supposed to see the password as *******, see :-)
+S0="--object secret,id=sec0,data=hunter0"
+S1="--object secret,id=sec1,data=hunter1"
+SECRETS="$S0 $S1"
+
+
+IMGS0="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,key-secret=sec0"
+IMGS1="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,key-secret=sec1"
+
+echo "== creating a test image =="
+_make_test_img $S0 -o "key-secret=sec0,iter-time=10" 32M
+
+echo
+echo "== test that key 0 opens the image =="
+$QEMU_IO $S0 -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== adding a password to slot 1 =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o active=on,key-secret=sec1,slot=1,iter-time=10
+
+echo
+echo "== 'backup' the image header =="
+dd if=$TEST_IMG_FILE of=${TEST_IMG_FILE}.bk bs=4K skip=0 count=1
+
+echo
+echo "== erase slot 0 =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o active=off,slot=0 | _filter_img_create
+
+echo
+echo "== test that key 0 doesn't open the image =="
+$QEMU_IO $S0 -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== 'restore' the image header =="
+dd if=${TEST_IMG_FILE}.bk of=${TEST_IMG_FILE} bs=4K skip=0 count=1 conv=notrunc
+
+echo
+echo "== test that key 0 still doesn't open the image (key material is erased) =="
+$QEMU_IO $SECRETS -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== test that key 1 still works =="
+$QEMU_IO $SECRETS -c "read 0 4096" $IMGS1 | _filter_qemu_io | _filter_testdir
+
+echo "*** done"
+rm -f $seq.full
+status=0
+
+
+exit 0
diff --git a/tests/qemu-iotests/301.out b/tests/qemu-iotests/301.out
new file mode 100644
index 0000000000..e653c30330
--- /dev/null
+++ b/tests/qemu-iotests/301.out
@@ -0,0 +1,30 @@
+QA output created by 301
+== creating a test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432
+
+== test that key 0 opens the image ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== adding a password to slot 1 ==
+
+== 'backup' the image header ==
+1+0 records in
+1+0 records out
+
+== erase slot 0 ==
+
+== test that key 0 doesn't open the image ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+
+== 'restore' the image header ==
+1+0 records in
+1+0 records out
+
+== test that key 0 still doesn't open the image (key material is erased) ==
+qemu-io: can't open: Invalid password, cannot unlock any keyslot
+
+== test that key 1 still works ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
new file mode 100644
index 0000000000..ef33d6fff5
--- /dev/null
+++ b/tests/qemu-iotests/302
@@ -0,0 +1,247 @@
+#!/usr/bin/env python
+#
+# Test case QMP's encrypted key management
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+import os
+import time
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class Secret:
+ def __init__(self, index):
+ self._id = "keysec" + str(index)
+ # you are not supposed to see the password...
+ self._secret = "hunter" + str(index)
+
+ def id(self):
+ return self._id
+
+ def secret(self):
+ return self._secret
+
+ def to_cmdline_object(self):
+ return [ "secret,id=" + self._id + ",data=" + self._secret]
+
+ def to_qmp_object(self):
+ return { "qom_type" : "secret", "id": self.id(),
+ "props": { "data": self.secret() } }
+
+################################################################################
+
+class EncryptionSetupTestCase(iotests.QMPTestCase):
+
+ # test case startup
+ def setUp(self):
+ # start the VM
+ self.vm = iotests.VM()
+ self.vm.launch()
+
+ # create the secrets and load 'em into the VM
+ self.secrets = [ Secret(i) for i in range(0, 4) ]
+ for secret in self.secrets:
+ result = self.vm.qmp("object-add", **secret.to_qmp_object())
+ self.assert_qmp(result, 'return', {})
+
+ if iotests.imgfmt == "qcow2":
+ self.pfx="encrypt."
+ self.img_opts = [ '-o', "encrypt.format=luks" ]
+ else:
+ self.pfx=""
+ self.img_opts = []
+
+
+ # test case shutdown
+ def tearDown(self):
+ # stop the VM
+ self.vm.shutdown()
+
+ ###########################################################################
+ # create the encrypted block device
+ def createImg(self, file, secret):
+
+ iotests.qemu_img(
+ 'create',
+ '--object', *secret.to_cmdline_object(),
+ '-f', iotests.imgfmt,
+ '-o', self.pfx+'key-secret=' + secret.id(),
+ '-o', self.pfx+'iter-time=10',
+ *self.img_opts,
+ file,
+ '1M')
+
+ ###########################################################################
+ # open an encrypted block device
+ def openLUKS(self, id, file, secret):
+ if iotests.imgfmt == "qcow2":
+ encrypt_options = {
+ 'encrypt': {
+ 'format':'luks',
+ 'key-secret' : secret.id()}
+ }
+ else:
+ encrypt_options = {
+ 'key-secret' : secret.id()
+ }
+
+ result = self.vm.qmp('blockdev-add', **
+ {
+ 'driver': iotests.imgfmt,
+ 'node-name': id,
+
+ **encrypt_options,
+
+ 'file': {
+ 'driver': 'file',
+ 'filename': test_img,
+ }
+ }
+ )
+ self.assert_qmp(result, 'return', {})
+
+ # close the encrypted block device
+ def closeLUKS(self, id):
+ result = self.vm.qmp('blockdev-del', **{ 'node-name': id })
+ self.assert_qmp(result, 'return', {})
+
+ ###########################################################################
+
+ # add a key to an encrypted block device
+ def addKey(self, id, secret, unlock_secret = None, slot = None, force = False):
+
+ crypt_options = {
+ 'active': True,
+ 'key-secret' : secret.id(),
+ 'iter-time' : 10
+ }
+
+ if slot != None:
+ crypt_options['slot'] = slot
+ if unlock_secret != None:
+ crypt_options['unlock-secret'] = unlock_secret.id()
+
+ if iotests.imgfmt == "qcow2":
+ crypt_options['format'] = 'luks'
+ crypt_options = {
+ 'encrypt': crypt_options
+ }
+
+ args = {
+ 'node-name': id,
+ 'job-id' : 'job0',
+ 'options' : {
+ 'driver' : iotests.imgfmt,
+ **crypt_options
+ },
+ }
+
+ if force == True:
+ args['force'] = True
+
+ #TODO: check what jobs return
+ result = self.vm.qmp('x-blockdev-amend', **args)
+ assert result['return'] == {}
+ self.vm.run_job('job0')
+
+ # erase a key from an encrypted block device
+ def eraseKey(self, id, secret = None, slot = None, force = False):
+
+ crypt_options = {
+ 'active': False,
+ 'iter-time' : 10
+ }
+
+ if slot != None:
+ crypt_options['slot'] = slot
+ if secret != None:
+ crypt_options['key-secret'] = secret.id()
+
+ if iotests.imgfmt == "qcow2":
+ crypt_options['format'] = 'luks'
+ crypt_options = {
+ 'encrypt': crypt_options
+ }
+
+ args = {
+ 'node-name': id,
+ 'job-id' : 'job1',
+ 'options' : {
+ 'driver' : iotests.imgfmt,
+ **crypt_options
+ },
+ }
+
+ if force == True:
+ args['force'] = True
+
+ result = self.vm.qmp('x-blockdev-amend', **args)
+ assert result['return'] == {}
+ self.vm.run_job('job1')
+
+ ###########################################################################
+ # create image, and change its key
+ def testChangeKey(self):
+
+ # create the image with secret0 and open it
+ self.createImg(test_img, self.secrets[0]);
+ self.openLUKS("testdev", test_img, self.secrets[0])
+
+ # add key to slot 1
+ self.addKey("testdev", secret=self.secrets[1])
+
+
+ # erase key from slot 0
+ self.eraseKey("testdev", secret=self.secrets[0])
+
+ #reopen the image with secret1
+ self.closeLUKS("testdev")
+ self.openLUKS("testdev", test_img, self.secrets[1])
+
+ # close and erase the image for good
+ self.closeLUKS("testdev")
+ os.remove(test_img)
+
+ # test that if we erase the old password,
+ # we can still change the encryption keys using 'old-secret'
+ def testOldPassword(self):
+
+ # create the image with secret0 and open it
+ self.createImg(test_img, self.secrets[0]);
+ self.openLUKS("testdev", test_img, self.secrets[0])
+
+ # add key to slot 1
+ self.addKey("testdev", secret=self.secrets[1])
+
+ # erase key from slot 0
+ self.eraseKey("testdev", secret=self.secrets[0])
+
+ # this will fail as the old password is no longer valid
+ self.addKey("testdev", secret=self.secrets[2])
+
+ # this will work
+ self.addKey("testdev", secret=self.secrets[2], unlock_secret=self.secrets[1])
+
+ # close and erase the image for good
+ self.closeLUKS("testdev")
+ os.remove(test_img)
+
+
+if __name__ == '__main__':
+ # Encrypted formats support
+ iotests.main(iotests.main(supported_fmts=['qcow2','luks']))
diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out
new file mode 100644
index 0000000000..78bfc0864c
--- /dev/null
+++ b/tests/qemu-iotests/302.out
@@ -0,0 +1,18 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job1"}}
+{"return": {}}
+Job failed: Invalid password, cannot unlock any keyslot
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 8e9235d6fe..a93808cac2 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -138,8 +138,10 @@ _filter_img_create()
-e "s# block_state_zero=\\(on\\|off\\)##g" \
-e "s# log_size=[0-9]\\+##g" \
-e "s# refcount_bits=[0-9]\\+##g" \
- -e "s# key-secret=[a-zA-Z0-9]\\+##g" \
- -e "s# iter-time=[0-9]\\+##g" \
+ -e "s# \\(encrypt\\.\\)\\?key-secret=[a-zA-Z0-9]\\+##g" \
+ -e "s# \\(encrypt\\.\\)\\?slot=[0-9]\\+##g" \
+ -e "s# \\(encrypt\\.\\)\\?iter-time=[0-9]\\+##g" \
+ -e "s# encrypt\\.format=[a-zA-Z0-9]\\+##g" \
-e "s# force_size=\\(on\\|off\\)##g"
}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index d95d556414..cc2c824b7d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -274,3 +274,11 @@
257 rw
258 rw quick
262 rw quick migration
+
+
+
+
+
+300 rw auto
+301 rw auto quick
+302 rw auto
--
2.17.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management
2019-08-30 20:56 ` [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management Maxim Levitsky
@ 2019-09-06 14:14 ` Daniel P. Berrangé
2019-09-06 14:26 ` Maxim Levitsky
0 siblings, 1 reply; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:14 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Aug 30, 2019 at 11:56:08PM +0300, Maxim Levitsky wrote:
> Note that currently I add tests 300-302, which are
> placeholders to ease the rebase. In final version
> of these patches I will update these.
>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> tests/qemu-iotests/087.out | 6 +-
> tests/qemu-iotests/134.out | 2 +-
> tests/qemu-iotests/158.out | 4 +-
> tests/qemu-iotests/188.out | 2 +-
> tests/qemu-iotests/189.out | 4 +-
> tests/qemu-iotests/198.out | 4 +-
> tests/qemu-iotests/300 | 202 +++++++++++++++++++++++++
> tests/qemu-iotests/300.out | 98 ++++++++++++
> tests/qemu-iotests/301 | 90 +++++++++++
> tests/qemu-iotests/301.out | 30 ++++
> tests/qemu-iotests/302 | 247 +++++++++++++++++++++++++++++++
> tests/qemu-iotests/302.out | 18 +++
> tests/qemu-iotests/common.filter | 6 +-
> tests/qemu-iotests/group | 8 +
> 14 files changed, 708 insertions(+), 13 deletions(-)
> create mode 100755 tests/qemu-iotests/300
> create mode 100644 tests/qemu-iotests/300.out
> create mode 100755 tests/qemu-iotests/301
> create mode 100644 tests/qemu-iotests/301.out
> create mode 100644 tests/qemu-iotests/302
> create mode 100644 tests/qemu-iotests/302.out
>
> diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> index 2d92ea847b..b61ba638af 100644
> --- a/tests/qemu-iotests/087.out
> +++ b/tests/qemu-iotests/087.out
> @@ -34,7 +34,7 @@ QMP_VERSION
>
> === Encrypted image QCow ===
>
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
Why is the output format changing ? There's no code changes in
this patch. If the change is due to an earlier patch, then this
patch chunk should be put in the earlier patch that causes it.
Likewise for the changed output to other files in this patch.
> Testing:
> QMP_VERSION
> {"return": {}}
> @@ -46,7 +46,7 @@ QMP_VERSION
>
> === Encrypted image LUKS ===
>
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encrypt.format=luks encrypt.key-secret=sec0
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
> Testing:
> QMP_VERSION
> {"return": {}}
> @@ -58,7 +58,7 @@ QMP_VERSION
>
> === Missing driver ===
>
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
> Testing: -S
> QMP_VERSION
> {"return": {}}
> diff --git a/tests/qemu-iotests/134.out b/tests/qemu-iotests/134.out
> index 09d46f6b17..4abc5b5f7d 100644
> --- a/tests/qemu-iotests/134.out
> +++ b/tests/qemu-iotests/134.out
> @@ -1,5 +1,5 @@
> QA output created by 134
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
>
> == reading whole image ==
> read 134217728/134217728 bytes at offset 0
> diff --git a/tests/qemu-iotests/158.out b/tests/qemu-iotests/158.out
> index 6def216e55..f28a17626b 100644
> --- a/tests/qemu-iotests/158.out
> +++ b/tests/qemu-iotests/158.out
> @@ -1,6 +1,6 @@
> QA output created by 158
> == create base ==
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 encryption=on
>
> == writing whole image ==
> wrote 134217728/134217728 bytes at offset 0
> @@ -10,7 +10,7 @@ wrote 134217728/134217728 bytes at offset 0
> read 134217728/134217728 bytes at offset 0
> 128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> == create overlay ==
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base encryption=on encrypt.key-secret=sec0
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base encryption=on
>
> == writing part of a cluster ==
> wrote 1024/1024 bytes at offset 0
> diff --git a/tests/qemu-iotests/188.out b/tests/qemu-iotests/188.out
> index c568ef3701..5426861b18 100644
> --- a/tests/qemu-iotests/188.out
> +++ b/tests/qemu-iotests/188.out
> @@ -1,5 +1,5 @@
> QA output created by 188
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216
>
> == reading whole image ==
> read 16777216/16777216 bytes at offset 0
> diff --git a/tests/qemu-iotests/189.out b/tests/qemu-iotests/189.out
> index a0b7c9c24c..bc213cbe14 100644
> --- a/tests/qemu-iotests/189.out
> +++ b/tests/qemu-iotests/189.out
> @@ -1,6 +1,6 @@
> QA output created by 189
> == create base ==
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216
>
> == writing whole image ==
> wrote 16777216/16777216 bytes at offset 0
> @@ -10,7 +10,7 @@ wrote 16777216/16777216 bytes at offset 0
> read 16777216/16777216 bytes at offset 0
> 16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> == create overlay ==
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base encrypt.format=luks encrypt.key-secret=sec1 encrypt.iter-time=10
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base
>
> == writing part of a cluster ==
> wrote 1024/1024 bytes at offset 0
> diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
> index e86b175e39..2eff420795 100644
> --- a/tests/qemu-iotests/198.out
> +++ b/tests/qemu-iotests/198.out
> @@ -1,12 +1,12 @@
> QA output created by 198
> == create base ==
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216 encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216
>
> == writing whole image base ==
> wrote 16777216/16777216 bytes at offset 0
> 16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> == create overlay ==
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base encrypt.format=luks encrypt.key-secret=sec1 encrypt.iter-time=10
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 backing_file=TEST_DIR/t.IMGFMT.base
>
> == writing whole image layer ==
> wrote 16777216/16777216 bytes at offset 0
> diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> new file mode 100755
> index 0000000000..5b65ef95de
> --- /dev/null
> +++ b/tests/qemu-iotests/300
> @@ -0,0 +1,202 @@
> +#!/usr/bin/env bash
> +#
> +# Test encryption key management with luks
> +# Based on 134
> +#
> +# Copyright (C) 2019 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +# creator
> +owner=mlevitsk@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt qcow2 luks
> +_supported_proto file #TODO
> +
> +QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
> +
> +if [ "$IMGFMT" = "qcow2" ] ; then
> + PR="encrypt."
> + EXTRA_IMG_ARGS="-o encrypt.format=luks"
> +fi
> +
> +
> +# secrets: you are supposed to see the password as *******, see :-)
> +S0="--object secret,id=sec0,data=hunter0"
> +S1="--object secret,id=sec1,data=hunter1"
> +S2="--object secret,id=sec2,data=hunter2"
> +S3="--object secret,id=sec3,data=hunter3"
> +S4="--object secret,id=sec4,data=hunter4"
> +SECRETS="$S0 $S1 $S2 $S3 $S4"
> +
> +# image with given secret
> +IMGS0="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec0"
> +IMGS1="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec1"
> +IMGS2="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec2"
> +IMGS3="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec3"
> +IMGS4="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec4"
> +
> +
> +echo "== creating a test image =="
> +_make_test_img $S4 $EXTRA_IMG_ARGS -o ${PR}key-secret=sec4,${PR}iter-time=10,${PR}slot=4 32M
> +
> +echo
> +echo "== test that key 4 opens the image =="
> +$QEMU_IO $S4 -c "read 0 4096" $IMGS4 | _filter_qemu_io | _filter_testdir
> +
> +echo
> +echo "== adding a password to slot 0 =="
> +$QEMU_IMG amend $SECRETS $IMGS4 -o ${PR}key-secret=sec0,${PR}iter-time=10
> +echo "== adding a password to slot 1 =="
> +$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}key-secret=sec1,${PR}iter-time=10
> +echo "== adding a password to slot 3 =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}key-secret=sec3,${PR}iter-time=10,${PR}slot=3
> +echo "== adding a password to slot 2 =="
> +$QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec2,${PR}iter-time=10
> +
> +
> +echo "== erase slot 4 =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=4 | _filter_img_create
> +
> +
> +echo
> +echo "== all secrets should work =="
> +for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
> + $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
> +done
> +
> +echo
> +echo "== erase slot 0 and try it =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec0 | _filter_img_create
> +$QEMU_IO $SECRETS -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
> +
> +echo
> +echo "== erase slot 2 and try it =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=2| _filter_img_create
> +$QEMU_IO $SECRETS -c "read 0 4096" $IMGS2 | _filter_qemu_io | _filter_testdir
> +
> +
> +# at this point slots 1 and 3 should be active
> +
> +echo
> +echo "== filling 4 slots with secret 2 =="
> +for i in $(seq 0 3) ; do
> + $QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec2,${PR}iter-time=10
> +done
> +
> +echo
> +echo "== adding secret 0 =="
> + $QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec0,${PR}iter-time=10
> +
> +echo
> +echo "== adding secret 3 (last slot) =="
> + $QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec3,${PR}iter-time=10
> +
> +echo
> +echo "== trying to add another slot (should fail) =="
> +$QEMU_IMG amend $SECRETS $IMGS2 -o ${PR}key-secret=sec3,${PR}iter-time=10
> +
> +echo
> +echo "== all secrets should work again =="
> +for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
> + $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
> +done
> +
> +echo
> +echo "== erase all keys of secret 2=="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec2
> +echo "== erase all keys of secret 1=="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec1
> +echo "== erase all keys of secret 0=="
> +$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}active=off,${PR}key-secret=sec0
> +echo "== erase all keys of secret 3, except a remaining key =="
> +$QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}active=off,${PR}key-secret=sec3
> +
> +
> +echo
> +echo "== only secret3 should work now =="
> +for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
> + $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
> +done
> +
> +echo
> +echo "== add secret0 =="
> +$QEMU_IMG amend $SECRETS $IMGS3 -o ${PR}key-secret=sec0,${PR}iter-time=10
> +
> +echo "== erase secret3 =="
> +$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}active=off,${PR}key-secret=sec3
> +
> +echo
> +echo "== only secret0 should work now =="
> +for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
> + $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
> +done
> +
> +echo
> +echo "== replace secret0 with secret1 (should fail) =="
> +$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}key-secret=sec1,${PR}iter-time=10,${PR}slot=0
> +
> +echo
> +echo "== replace secret0 with secret1 with force (should work) =="
> +$QEMU_IMG amend $SECRETS $IMGS0 -o ${PR}key-secret=sec1,${PR}iter-time=10,${PR}slot=0 --force
> +
> +echo
> +echo "== only secret1 should work now =="
> +for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
> + $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
> +done
> +
> +
> +echo
> +echo "== erase last secret (should fail) =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=0
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec1
> +
> +
> +echo "== erase non existing secrets (should fail) =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec5 --force
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}key-secret=sec0 --force
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=1 --force
> +
> +echo
> +echo "== erase last secret with force by slot (should work) =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}active=off,${PR}slot=0 --force
> +
> +echo
> +echo "== we have no secrets now, data is lost forever =="
> +for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
> + $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | _filter_testdir
> +done
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> +
> diff --git a/tests/qemu-iotests/300.out b/tests/qemu-iotests/300.out
> new file mode 100644
> index 0000000000..d809595381
> --- /dev/null
> +++ b/tests/qemu-iotests/300.out
> @@ -0,0 +1,98 @@
> +QA output created by 300
> +== creating a test image ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432
> +
> +== test that key 4 opens the image ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== adding a password to slot 0 ==
> +== adding a password to slot 1 ==
> +== adding a password to slot 3 ==
> +== adding a password to slot 2 ==
> +== erase slot 4 ==
> +
> +== all secrets should work ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== erase slot 0 and try it ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +
> +== erase slot 2 and try it ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +
> +== filling 4 slots with secret 2 ==
> +
> +== adding secret 0 ==
> +
> +== adding secret 3 (last slot) ==
> +
> +== trying to add another slot (should fail) ==
> +qemu-img: Can't add a keyslot - all key slots are in use
> +
> +== all secrets should work again ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== erase all keys of secret 2==
> +== erase all keys of secret 1==
> +== erase all keys of secret 0==
> +== erase all keys of secret 3, except a remaining key ==
> +
> +== only secret3 should work now ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== add secret0 ==
> +== erase secret3 ==
> +
> +== only secret0 should work now ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +
> +== replace secret0 with secret1 (should fail) ==
> +qemu-img: Can't update an active key slot 0
> +
> +== replace secret0 with secret1 with force (should work) ==
> +
> +== only secret1 should work now ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +
> +== erase last secret (should fail) ==
> +qemu-img: Only one slot active - can't erase
> +qemu-img: Only one slot active - can't erase
> +== erase non existing secrets (should fail) ==
> +qemu-img: No secret with id 'sec5'
> +qemu-img: Didn't erase a keyslot, because no keyslots match the given password
> +
> +== erase last secret with force by slot (should work) ==
> +
> +== we have no secrets now, data is lost forever ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +*** done
> diff --git a/tests/qemu-iotests/301 b/tests/qemu-iotests/301
> new file mode 100755
> index 0000000000..ba29bf10a0
> --- /dev/null
> +++ b/tests/qemu-iotests/301
> @@ -0,0 +1,90 @@
> +#
> +# Copyright (C) 2019 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +# creator
> +owner=mlevitsk@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt luks
> +_supported_proto file #TODO
> +
> +QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
> +
> +# you are supposed to see the password as *******, see :-)
> +S0="--object secret,id=sec0,data=hunter0"
> +S1="--object secret,id=sec1,data=hunter1"
> +SECRETS="$S0 $S1"
> +
> +
> +IMGS0="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,key-secret=sec0"
> +IMGS1="--image-opts driver=$IMGFMT,file.filename=$TEST_IMG,key-secret=sec1"
> +
> +echo "== creating a test image =="
> +_make_test_img $S0 -o "key-secret=sec0,iter-time=10" 32M
> +
> +echo
> +echo "== test that key 0 opens the image =="
> +$QEMU_IO $S0 -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
> +
> +echo
> +echo "== adding a password to slot 1 =="
> +$QEMU_IMG amend $SECRETS $IMGS0 -o active=on,key-secret=sec1,slot=1,iter-time=10
> +
> +echo
> +echo "== 'backup' the image header =="
> +dd if=$TEST_IMG_FILE of=${TEST_IMG_FILE}.bk bs=4K skip=0 count=1
> +
> +echo
> +echo "== erase slot 0 =="
> +$QEMU_IMG amend $SECRETS $IMGS1 -o active=off,slot=0 | _filter_img_create
> +
> +echo
> +echo "== test that key 0 doesn't open the image =="
> +$QEMU_IO $S0 -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
> +
> +echo
> +echo "== 'restore' the image header =="
> +dd if=${TEST_IMG_FILE}.bk of=${TEST_IMG_FILE} bs=4K skip=0 count=1 conv=notrunc
> +
> +echo
> +echo "== test that key 0 still doesn't open the image (key material is erased) =="
> +$QEMU_IO $SECRETS -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
> +
> +echo
> +echo "== test that key 1 still works =="
> +$QEMU_IO $SECRETS -c "read 0 4096" $IMGS1 | _filter_qemu_io | _filter_testdir
> +
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> +
> +
> +exit 0
> diff --git a/tests/qemu-iotests/301.out b/tests/qemu-iotests/301.out
> new file mode 100644
> index 0000000000..e653c30330
> --- /dev/null
> +++ b/tests/qemu-iotests/301.out
> @@ -0,0 +1,30 @@
> +QA output created by 301
> +== creating a test image ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432
> +
> +== test that key 0 opens the image ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== adding a password to slot 1 ==
> +
> +== 'backup' the image header ==
> +1+0 records in
> +1+0 records out
> +
> +== erase slot 0 ==
> +
> +== test that key 0 doesn't open the image ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +
> +== 'restore' the image header ==
> +1+0 records in
> +1+0 records out
> +
> +== test that key 0 still doesn't open the image (key material is erased) ==
> +qemu-io: can't open: Invalid password, cannot unlock any keyslot
> +
> +== test that key 1 still works ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +*** done
> diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
> new file mode 100644
> index 0000000000..ef33d6fff5
> --- /dev/null
> +++ b/tests/qemu-iotests/302
> @@ -0,0 +1,247 @@
> +#!/usr/bin/env python
> +#
> +# Test case QMP's encrypted key management
> +#
> +# Copyright (C) 2019 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import iotests
> +import os
> +import time
> +
> +test_img = os.path.join(iotests.test_dir, 'test.img')
> +
> +class Secret:
> + def __init__(self, index):
> + self._id = "keysec" + str(index)
> + # you are not supposed to see the password...
> + self._secret = "hunter" + str(index)
> +
> + def id(self):
> + return self._id
> +
> + def secret(self):
> + return self._secret
> +
> + def to_cmdline_object(self):
> + return [ "secret,id=" + self._id + ",data=" + self._secret]
> +
> + def to_qmp_object(self):
> + return { "qom_type" : "secret", "id": self.id(),
> + "props": { "data": self.secret() } }
> +
> +################################################################################
> +
> +class EncryptionSetupTestCase(iotests.QMPTestCase):
> +
> + # test case startup
> + def setUp(self):
> + # start the VM
> + self.vm = iotests.VM()
> + self.vm.launch()
> +
> + # create the secrets and load 'em into the VM
> + self.secrets = [ Secret(i) for i in range(0, 4) ]
> + for secret in self.secrets:
> + result = self.vm.qmp("object-add", **secret.to_qmp_object())
> + self.assert_qmp(result, 'return', {})
> +
> + if iotests.imgfmt == "qcow2":
> + self.pfx="encrypt."
> + self.img_opts = [ '-o', "encrypt.format=luks" ]
> + else:
> + self.pfx=""
> + self.img_opts = []
> +
> +
> + # test case shutdown
> + def tearDown(self):
> + # stop the VM
> + self.vm.shutdown()
> +
> + ###########################################################################
> + # create the encrypted block device
> + def createImg(self, file, secret):
> +
> + iotests.qemu_img(
> + 'create',
> + '--object', *secret.to_cmdline_object(),
> + '-f', iotests.imgfmt,
> + '-o', self.pfx+'key-secret=' + secret.id(),
> + '-o', self.pfx+'iter-time=10',
> + *self.img_opts,
> + file,
> + '1M')
> +
> + ###########################################################################
> + # open an encrypted block device
> + def openLUKS(self, id, file, secret):
> + if iotests.imgfmt == "qcow2":
> + encrypt_options = {
> + 'encrypt': {
> + 'format':'luks',
> + 'key-secret' : secret.id()}
> + }
> + else:
> + encrypt_options = {
> + 'key-secret' : secret.id()
> + }
> +
> + result = self.vm.qmp('blockdev-add', **
> + {
> + 'driver': iotests.imgfmt,
> + 'node-name': id,
> +
> + **encrypt_options,
> +
> + 'file': {
> + 'driver': 'file',
> + 'filename': test_img,
> + }
> + }
> + )
> + self.assert_qmp(result, 'return', {})
> +
> + # close the encrypted block device
> + def closeLUKS(self, id):
> + result = self.vm.qmp('blockdev-del', **{ 'node-name': id })
> + self.assert_qmp(result, 'return', {})
> +
> + ###########################################################################
> +
> + # add a key to an encrypted block device
> + def addKey(self, id, secret, unlock_secret = None, slot = None, force = False):
> +
> + crypt_options = {
> + 'active': True,
> + 'key-secret' : secret.id(),
> + 'iter-time' : 10
> + }
> +
> + if slot != None:
> + crypt_options['slot'] = slot
> + if unlock_secret != None:
> + crypt_options['unlock-secret'] = unlock_secret.id()
> +
> + if iotests.imgfmt == "qcow2":
> + crypt_options['format'] = 'luks'
> + crypt_options = {
> + 'encrypt': crypt_options
> + }
> +
> + args = {
> + 'node-name': id,
> + 'job-id' : 'job0',
> + 'options' : {
> + 'driver' : iotests.imgfmt,
> + **crypt_options
> + },
> + }
> +
> + if force == True:
> + args['force'] = True
> +
> + #TODO: check what jobs return
> + result = self.vm.qmp('x-blockdev-amend', **args)
> + assert result['return'] == {}
> + self.vm.run_job('job0')
> +
> + # erase a key from an encrypted block device
> + def eraseKey(self, id, secret = None, slot = None, force = False):
> +
> + crypt_options = {
> + 'active': False,
> + 'iter-time' : 10
> + }
> +
> + if slot != None:
> + crypt_options['slot'] = slot
> + if secret != None:
> + crypt_options['key-secret'] = secret.id()
> +
> + if iotests.imgfmt == "qcow2":
> + crypt_options['format'] = 'luks'
> + crypt_options = {
> + 'encrypt': crypt_options
> + }
> +
> + args = {
> + 'node-name': id,
> + 'job-id' : 'job1',
> + 'options' : {
> + 'driver' : iotests.imgfmt,
> + **crypt_options
> + },
> + }
> +
> + if force == True:
> + args['force'] = True
> +
> + result = self.vm.qmp('x-blockdev-amend', **args)
> + assert result['return'] == {}
> + self.vm.run_job('job1')
> +
> + ###########################################################################
> + # create image, and change its key
> + def testChangeKey(self):
> +
> + # create the image with secret0 and open it
> + self.createImg(test_img, self.secrets[0]);
> + self.openLUKS("testdev", test_img, self.secrets[0])
> +
> + # add key to slot 1
> + self.addKey("testdev", secret=self.secrets[1])
> +
> +
> + # erase key from slot 0
> + self.eraseKey("testdev", secret=self.secrets[0])
> +
> + #reopen the image with secret1
> + self.closeLUKS("testdev")
> + self.openLUKS("testdev", test_img, self.secrets[1])
> +
> + # close and erase the image for good
> + self.closeLUKS("testdev")
> + os.remove(test_img)
> +
> + # test that if we erase the old password,
> + # we can still change the encryption keys using 'old-secret'
> + def testOldPassword(self):
> +
> + # create the image with secret0 and open it
> + self.createImg(test_img, self.secrets[0]);
> + self.openLUKS("testdev", test_img, self.secrets[0])
> +
> + # add key to slot 1
> + self.addKey("testdev", secret=self.secrets[1])
> +
> + # erase key from slot 0
> + self.eraseKey("testdev", secret=self.secrets[0])
> +
> + # this will fail as the old password is no longer valid
> + self.addKey("testdev", secret=self.secrets[2])
> +
> + # this will work
> + self.addKey("testdev", secret=self.secrets[2], unlock_secret=self.secrets[1])
> +
> + # close and erase the image for good
> + self.closeLUKS("testdev")
> + os.remove(test_img)
> +
> +
> +if __name__ == '__main__':
> + # Encrypted formats support
> + iotests.main(iotests.main(supported_fmts=['qcow2','luks']))
> diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out
> new file mode 100644
> index 0000000000..78bfc0864c
> --- /dev/null
> +++ b/tests/qemu-iotests/302.out
> @@ -0,0 +1,18 @@
> +..
> +----------------------------------------------------------------------
> +Ran 2 tests
> +
> +OK
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job1"}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job1"}}
> +{"return": {}}
> +Job failed: Invalid password, cannot unlock any keyslot
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> index 8e9235d6fe..a93808cac2 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -138,8 +138,10 @@ _filter_img_create()
> -e "s# block_state_zero=\\(on\\|off\\)##g" \
> -e "s# log_size=[0-9]\\+##g" \
> -e "s# refcount_bits=[0-9]\\+##g" \
> - -e "s# key-secret=[a-zA-Z0-9]\\+##g" \
> - -e "s# iter-time=[0-9]\\+##g" \
> + -e "s# \\(encrypt\\.\\)\\?key-secret=[a-zA-Z0-9]\\+##g" \
> + -e "s# \\(encrypt\\.\\)\\?slot=[0-9]\\+##g" \
> + -e "s# \\(encrypt\\.\\)\\?iter-time=[0-9]\\+##g" \
> + -e "s# encrypt\\.format=[a-zA-Z0-9]\\+##g" \
> -e "s# force_size=\\(on\\|off\\)##g"
> }
>
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index d95d556414..cc2c824b7d 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -274,3 +274,11 @@
> 257 rw
> 258 rw quick
> 262 rw quick migration
> +
> +
> +
> +
> +
> +300 rw auto
> +301 rw auto quick
> +302 rw auto
> --
> 2.17.2
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management
2019-09-06 14:14 ` Daniel P. Berrangé
@ 2019-09-06 14:26 ` Maxim Levitsky
2019-09-06 14:27 ` Daniel P. Berrangé
0 siblings, 1 reply; 30+ messages in thread
From: Maxim Levitsky @ 2019-09-06 14:26 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, 2019-09-06 at 15:14 +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 11:56:08PM +0300, Maxim Levitsky wrote:
> > Note that currently I add tests 300-302, which are
> > placeholders to ease the rebase. In final version
> > of these patches I will update these.
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > tests/qemu-iotests/087.out | 6 +-
> > tests/qemu-iotests/134.out | 2 +-
> > tests/qemu-iotests/158.out | 4 +-
> > tests/qemu-iotests/188.out | 2 +-
> > tests/qemu-iotests/189.out | 4 +-
> > tests/qemu-iotests/198.out | 4 +-
> > tests/qemu-iotests/300 | 202 +++++++++++++++++++++++++
> > tests/qemu-iotests/300.out | 98 ++++++++++++
> > tests/qemu-iotests/301 | 90 +++++++++++
> > tests/qemu-iotests/301.out | 30 ++++
> > tests/qemu-iotests/302 | 247 +++++++++++++++++++++++++++++++
> > tests/qemu-iotests/302.out | 18 +++
> > tests/qemu-iotests/common.filter | 6 +-
> > tests/qemu-iotests/group | 8 +
> > 14 files changed, 708 insertions(+), 13 deletions(-)
> > create mode 100755 tests/qemu-iotests/300
> > create mode 100644 tests/qemu-iotests/300.out
> > create mode 100755 tests/qemu-iotests/301
> > create mode 100644 tests/qemu-iotests/301.out
> > create mode 100644 tests/qemu-iotests/302
> > create mode 100644 tests/qemu-iotests/302.out
> >
> > diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> > index 2d92ea847b..b61ba638af 100644
> > --- a/tests/qemu-iotests/087.out
> > +++ b/tests/qemu-iotests/087.out
> > @@ -34,7 +34,7 @@ QMP_VERSION
> >
> > === Encrypted image QCow ===
> >
> > -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
> > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
>
> Why is the output format changing ? There's no code changes in
> this patch. If the change is due to an earlier patch, then this
> patch chunk should be put in the earlier patch that causes it.
>
> Likewise for the changed output to other files in this patch.
>
> >
I tweaked the common.filter to filter more luks specific create options
so that a test could have same output for both qcow2 and plain raw luks encryption.
(due to the "encrypt.*" prefix)
I can move this in a separate patch if you think this is worth it.
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH 10/10] iotests : add tests for encryption key management
2019-09-06 14:26 ` Maxim Levitsky
@ 2019-09-06 14:27 ` Daniel P. Berrangé
0 siblings, 0 replies; 30+ messages in thread
From: Daniel P. Berrangé @ 2019-09-06 14:27 UTC (permalink / raw)
To: Maxim Levitsky
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
John Snow
On Fri, Sep 06, 2019 at 05:26:09PM +0300, Maxim Levitsky wrote:
> On Fri, 2019-09-06 at 15:14 +0100, Daniel P. Berrangé wrote:
> > On Fri, Aug 30, 2019 at 11:56:08PM +0300, Maxim Levitsky wrote:
> > > Note that currently I add tests 300-302, which are
> > > placeholders to ease the rebase. In final version
> > > of these patches I will update these.
> > >
> > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > ---
> > > tests/qemu-iotests/087.out | 6 +-
> > > tests/qemu-iotests/134.out | 2 +-
> > > tests/qemu-iotests/158.out | 4 +-
> > > tests/qemu-iotests/188.out | 2 +-
> > > tests/qemu-iotests/189.out | 4 +-
> > > tests/qemu-iotests/198.out | 4 +-
> > > tests/qemu-iotests/300 | 202 +++++++++++++++++++++++++
> > > tests/qemu-iotests/300.out | 98 ++++++++++++
> > > tests/qemu-iotests/301 | 90 +++++++++++
> > > tests/qemu-iotests/301.out | 30 ++++
> > > tests/qemu-iotests/302 | 247 +++++++++++++++++++++++++++++++
> > > tests/qemu-iotests/302.out | 18 +++
> > > tests/qemu-iotests/common.filter | 6 +-
> > > tests/qemu-iotests/group | 8 +
> > > 14 files changed, 708 insertions(+), 13 deletions(-)
> > > create mode 100755 tests/qemu-iotests/300
> > > create mode 100644 tests/qemu-iotests/300.out
> > > create mode 100755 tests/qemu-iotests/301
> > > create mode 100644 tests/qemu-iotests/301.out
> > > create mode 100644 tests/qemu-iotests/302
> > > create mode 100644 tests/qemu-iotests/302.out
> > >
> > > diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
> > > index 2d92ea847b..b61ba638af 100644
> > > --- a/tests/qemu-iotests/087.out
> > > +++ b/tests/qemu-iotests/087.out
> > > @@ -34,7 +34,7 @@ QMP_VERSION
> > >
> > > === Encrypted image QCow ===
> > >
> > > -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on encrypt.key-secret=sec0
> > > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
> >
> > Why is the output format changing ? There's no code changes in
> > this patch. If the change is due to an earlier patch, then this
> > patch chunk should be put in the earlier patch that causes it.
> >
> > Likewise for the changed output to other files in this patch.
> >
> > >
>
> I tweaked the common.filter to filter more luks specific create options
> so that a test could have same output for both qcow2 and plain raw luks encryption.
> (due to the "encrypt.*" prefix)
Oh i see, I was not looking carefully enough.
> I can move this in a separate patch if you think this is worth it.
Yeah that's a good idea.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 30+ messages in thread