[PATCH 0/2] iotests: Fix crypto algorithm failures

[PATCH 0/2] iotests: Fix crypto algorithm failures

[Hanna Reitz]

Hi,

iotests 149, 206, and 210 fail when qemu uses the gnutls crypto backend
(which is the default as of 8bd0931f6) because they try to use
algorithms that this backend does not support.

Have 206 and 210 use different algorithms instead (patch 1), and let 149
be skipped when it encounters an unsupported algorithm (patch 2).


Hanna Reitz (2):
  iotests: Use aes-128-cbc
  iotests/149: Skip on unsupported ciphers

 tests/qemu-iotests/149     | 23 ++++++++++++++++++-----
 tests/qemu-iotests/206     |  4 ++--
 tests/qemu-iotests/206.out |  6 +++---
 tests/qemu-iotests/210     |  4 ++--
 tests/qemu-iotests/210.out |  6 +++---
 5 files changed, 28 insertions(+), 15 deletions(-)

-- 
2.33.1

[PATCH 1/2] iotests: Use aes-128-cbc

[Hanna Reitz]

Our gnutls crypto backend (which is the default as of 8bd0931f6)
supports neither twofish-128 nor the CTR mode.  CBC and aes-128 are
supported by all of our backends (as far as I can tell), so use
aes-128-cbc in our iotests.

(We could also use e.g. aes-256-cbc, but the different key sizes would
lead to different key slot offsets and so change the reference output
more, which is why I went with aes-128.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/206     | 4 ++--
 tests/qemu-iotests/206.out | 6 +++---
 tests/qemu-iotests/210     | 4 ++--
 tests/qemu-iotests/210.out | 6 +++---
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
index c3cdad4ce4..10eff343f7 100755
--- a/tests/qemu-iotests/206
+++ b/tests/qemu-iotests/206
@@ -162,8 +162,8 @@ with iotests.FilePath('t.qcow2') as disk_path, \
                          'encrypt': {
                              'format': 'luks',
                              'key-secret': 'keysec0',
-                             'cipher-alg': 'twofish-128',
-                             'cipher-mode': 'ctr',
+                             'cipher-alg': 'aes-128',
+                             'cipher-mode': 'cbc',
                              'ivgen-alg': 'plain64',
                              'ivgen-hash-alg': 'md5',
                              'hash-alg': 'sha1',
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index 3593e8e9c2..80cd274223 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -97,7 +97,7 @@ Format specific information:
 
 === Successful image creation (encrypted) ===
 
-{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "qcow2", "encrypt": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "format": "luks", "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0"}, "file": {"driver": "file", "filename": "TEST_DIR/PID-t.qcow2"}, "size": 33554432}}}
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
@@ -115,10 +115,10 @@ Format specific information:
     encrypt:
         ivgen alg: plain64
         hash alg: sha1
-        cipher alg: twofish-128
+        cipher alg: aes-128
         uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
         format: luks
-        cipher mode: ctr
+        cipher mode: cbc
         slots:
             [0]:
                 active: true
diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index 5a62ed4dd1..a4dcc5fe59 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -83,8 +83,8 @@ with iotests.FilePath('t.luks') as disk_path, \
                          },
                          'size': size,
                          'key-secret': 'keysec0',
-                         'cipher-alg': 'twofish-128',
-                         'cipher-mode': 'ctr',
+                         'cipher-alg': 'aes-128',
+                         'cipher-mode': 'cbc',
                          'ivgen-alg': 'plain64',
                          'ivgen-hash-alg': 'md5',
                          'hash-alg': 'sha1',
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index 55c0844370..96d9f749dd 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -59,7 +59,7 @@ Format specific information:
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
 
-{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "twofish-128", "cipher-mode": "ctr", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"cipher-alg": "aes-128", "cipher-mode": "cbc", "driver": "luks", "file": {"driver": "file", "filename": "TEST_DIR/PID-t.luks"}, "hash-alg": "sha1", "iter-time": 10, "ivgen-alg": "plain64", "ivgen-hash-alg": "md5", "key-secret": "keysec0", "size": 67108864}}}
 {"return": {}}
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
@@ -71,9 +71,9 @@ encrypted: yes
 Format specific information:
     ivgen alg: plain64
     hash alg: sha1
-    cipher alg: twofish-128
+    cipher alg: aes-128
     uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
-    cipher mode: ctr
+    cipher mode: cbc
     slots:
         [0]:
             active: true
-- 
2.33.1

[PATCH 2/2] iotests/149: Skip on unsupported ciphers

[Hanna Reitz]

Whenever qemu-img or qemu-io report that some cipher is unsupported,
skip the whole test, because that is probably because qemu has been
configured with the gnutls crypto backend.

We could taylor the algorithm list to what gnutls supports, but this is
a test that is run rather rarely anyway (because it requires
password-less sudo), and so it seems better and easier to skip it.  When
this test is intentionally run to check LUKS compatibility, it seems
better not to limit the algorithms but keep the list extensive.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 328fd05a4c..adcef86e88 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -230,6 +230,18 @@ def create_image(config, size_mb):
         fn.truncate(size_mb * 1024 * 1024)
 
 
+def check_cipher_support(output):
+    """Check the output of qemu-img or qemu-io for mention of the respective
+    cipher algorithm being unsupported, and if so, skip this test.
+    (Returns `output` for convenience.)"""
+
+    if 'Unsupported cipher algorithm' in output:
+        iotests.notrun('Unsupported cipher algorithm '
+                       f'{config.cipher}-{config.keylen}-{config.mode}; '
+                       'consider configuring qemu with a different crypto '
+                       'backend')
+    return output
+
 def qemu_img_create(config, size_mb):
     """Create and format a disk image with LUKS using qemu-img"""
 
@@ -253,7 +265,8 @@ def qemu_img_create(config, size_mb):
             "%dM" % size_mb]
 
     iotests.log("qemu-img " + " ".join(args), filters=[iotests.filter_test_dir])
-    iotests.log(iotests.qemu_img_pipe(*args), filters=[iotests.filter_test_dir])
+    iotests.log(check_cipher_support(iotests.qemu_img_pipe(*args)),
+                filters=[iotests.filter_test_dir])
 
 def qemu_io_image_args(config, dev=False):
     """Get the args for access an image or device with qemu-io"""
@@ -279,8 +292,8 @@ def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
     args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
     args.extend(qemu_io_image_args(config, dev))
     iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
-    iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
-                                                 iotests.filter_qemu_io])
+    iotests.log(check_cipher_support(iotests.qemu_io(*args)),
+                filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
 
 
 def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
@@ -291,8 +304,8 @@ def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
     args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
     args.extend(qemu_io_image_args(config, dev))
     iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
-    iotests.log(iotests.qemu_io(*args), filters=[iotests.filter_test_dir,
-                                                 iotests.filter_qemu_io])
+    iotests.log(check_cipher_support(iotests.qemu_io(*args)),
+                filters=[iotests.filter_test_dir, iotests.filter_qemu_io])
 
 
 def test_once(config, qemu_img=False):
-- 
2.33.1

Re: [PATCH 2/2] iotests/149: Skip on unsupported ciphers

[Hanna Reitz]

On 17.11.21 16:01, Hanna Reitz wrote:
> Whenever qemu-img or qemu-io report that some cipher is unsupported,
> skip the whole test, because that is probably because qemu has been
> configured with the gnutls crypto backend.
>
> We could taylor the algorithm list to what gnutls supports, but this is
> a test that is run rather rarely anyway (because it requires
> password-less sudo), and so it seems better and easier to skip it.  When
> this test is intentionally run to check LUKS compatibility, it seems
> better not to limit the algorithms but keep the list extensive.
>
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>   tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
> index 328fd05a4c..adcef86e88 100755
> --- a/tests/qemu-iotests/149
> +++ b/tests/qemu-iotests/149
> @@ -230,6 +230,18 @@ def create_image(config, size_mb):
>           fn.truncate(size_mb * 1024 * 1024)
>   
>   
> +def check_cipher_support(output):
> +    """Check the output of qemu-img or qemu-io for mention of the respective
> +    cipher algorithm being unsupported, and if so, skip this test.
> +    (Returns `output` for convenience.)"""
> +
> +    if 'Unsupported cipher algorithm' in output:
> +        iotests.notrun('Unsupported cipher algorithm '
> +                       f'{config.cipher}-{config.keylen}-{config.mode}; '

Oops.  Just when I sent this I realized that during refactoring (putting 
this code into its own function) I forgot to pass `config` as a parameter.

Didn’t notice that because...  It seems to work just fine despite 
`config` not being defined here?  Python will forever remain a black box 
for me...

Hanna

> +                       'consider configuring qemu with a different crypto '
> +                       'backend')
> +    return output
> +

Re: [PATCH 2/2] iotests/149: Skip on unsupported ciphers

[Kevin Wolf]

Am 17.11.2021 um 16:05 hat Hanna Reitz geschrieben:
> On 17.11.21 16:01, Hanna Reitz wrote:
> > Whenever qemu-img or qemu-io report that some cipher is unsupported,
> > skip the whole test, because that is probably because qemu has been
> > configured with the gnutls crypto backend.
> > 
> > We could taylor the algorithm list to what gnutls supports, but this is
> > a test that is run rather rarely anyway (because it requires
> > password-less sudo), and so it seems better and easier to skip it.  When
> > this test is intentionally run to check LUKS compatibility, it seems
> > better not to limit the algorithms but keep the list extensive.
> > 
> > Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> > ---
> >   tests/qemu-iotests/149 | 23 ++++++++++++++++++-----
> >   1 file changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
> > index 328fd05a4c..adcef86e88 100755
> > --- a/tests/qemu-iotests/149
> > +++ b/tests/qemu-iotests/149
> > @@ -230,6 +230,18 @@ def create_image(config, size_mb):
> >           fn.truncate(size_mb * 1024 * 1024)
> > +def check_cipher_support(output):
> > +    """Check the output of qemu-img or qemu-io for mention of the respective
> > +    cipher algorithm being unsupported, and if so, skip this test.
> > +    (Returns `output` for convenience.)"""
> > +
> > +    if 'Unsupported cipher algorithm' in output:
> > +        iotests.notrun('Unsupported cipher algorithm '
> > +                       f'{config.cipher}-{config.keylen}-{config.mode}; '
> 
> Oops.  Just when I sent this I realized that during refactoring (putting
> this code into its own function) I forgot to pass `config` as a parameter.
> 
> Didn’t notice that because...  It seems to work just fine despite `config`
> not being defined here?  Python will forever remain a black box for me...

This is an old thread by now, but I think that it works is just because
it's defined as a global variable ('for config in configs') before
calling this function.

Kevin