[PATCH v2 0/4] crypto: fully drop built-in cipher provider

[PATCH v2 0/4] crypto: fully drop built-in cipher provider

[Daniel P. Berrangé]

Changed in v2:

 - Addressed various unit tests which blindly assume AES was always
   available

Daniel P. Berrangé (4):
  tests: skip encrypted secret tests if AES is not available
  tests: skip legacy qcow2 encryption test if AES is not available
  tests: fix skipping cipher tests when AES is not available
  crypto: fully drop built-in cipher provider

 crypto/cipher-builtin.c.inc     | 303 --------------------------------
 crypto/cipher-stub.c.inc        |  41 +++++
 crypto/cipher.c                 |   2 +-
 tests/unit/test-crypto-block.c  |   7 +
 tests/unit/test-crypto-cipher.c |  13 +-
 tests/unit/test-crypto-secret.c |  28 +--
 6 files changed, 74 insertions(+), 320 deletions(-)
 delete mode 100644 crypto/cipher-builtin.c.inc
 create mode 100644 crypto/cipher-stub.c.inc

-- 
2.49.0

[PATCH v2 1/4] tests: skip encrypted secret tests if AES is not available

[Daniel P. Berrangé]

From: Daniel P. Berrangé <berrange@redhat.com>

This avoids test breakage when we drop support for using the
built-in AES impl as a fallback for missing crypto libraries.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/unit/test-crypto-secret.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/tests/unit/test-crypto-secret.c b/tests/unit/test-crypto-secret.c
index ffd13ff70e..fc32a01747 100644
--- a/tests/unit/test-crypto-secret.c
+++ b/tests/unit/test-crypto-secret.c
@@ -22,6 +22,7 @@
 
 #include "crypto/init.h"
 #include "crypto/secret.h"
+#include "crypto/cipher.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
 #if defined(CONFIG_KEYUTILS) && defined(CONFIG_SECRET_KEYRING)
@@ -597,18 +598,21 @@ int main(int argc, char **argv)
     g_test_add_func("/crypto/secret/conv/utf8/base64",
                     test_secret_conv_utf8_base64);
 
-    g_test_add_func("/crypto/secret/crypt/raw",
-                    test_secret_crypt_raw);
-    g_test_add_func("/crypto/secret/crypt/base64",
-                    test_secret_crypt_base64);
-    g_test_add_func("/crypto/secret/crypt/shortkey",
-                    test_secret_crypt_short_key);
-    g_test_add_func("/crypto/secret/crypt/shortiv",
-                    test_secret_crypt_short_iv);
-    g_test_add_func("/crypto/secret/crypt/missingiv",
-                    test_secret_crypt_missing_iv);
-    g_test_add_func("/crypto/secret/crypt/badiv",
-                    test_secret_crypt_bad_iv);
+    if (qcrypto_cipher_supports(QCRYPTO_CIPHER_ALGO_AES_128,
+                                QCRYPTO_CIPHER_MODE_CBC)) {
+        g_test_add_func("/crypto/secret/crypt/raw",
+                        test_secret_crypt_raw);
+        g_test_add_func("/crypto/secret/crypt/base64",
+                        test_secret_crypt_base64);
+        g_test_add_func("/crypto/secret/crypt/shortkey",
+                        test_secret_crypt_short_key);
+        g_test_add_func("/crypto/secret/crypt/shortiv",
+                        test_secret_crypt_short_iv);
+        g_test_add_func("/crypto/secret/crypt/missingiv",
+                        test_secret_crypt_missing_iv);
+        g_test_add_func("/crypto/secret/crypt/badiv",
+                        test_secret_crypt_bad_iv);
+    }
 
     return g_test_run();
 }
-- 
2.49.0

[PATCH v2 2/4] tests: skip legacy qcow2 encryption test if AES is not available

[Daniel P. Berrangé]

From: Daniel P. Berrangé <berrange@redhat.com>

This avoids test breakage when we drop support for using the
built-in AES impl as a fallback for missing crypto libraries.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/unit/test-crypto-block.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/unit/test-crypto-block.c b/tests/unit/test-crypto-block.c
index 9217b9a2ef..3ac7f17b2a 100644
--- a/tests/unit/test-crypto-block.c
+++ b/tests/unit/test-crypto-block.c
@@ -574,6 +574,13 @@ int main(int argc, char **argv)
     for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
         if (test_data[i].open_opts->format == QCRYPTO_BLOCK_FORMAT_LUKS &&
             !qcrypto_hash_supports(test_data[i].hash_alg)) {
+            g_printerr("# skip unsupported %s\n",
+                       QCryptoHashAlgo_str(test_data[i].hash_alg));
+            continue;
+        }
+        if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALGO_AES_128,
+                                     QCRYPTO_CIPHER_MODE_CBC)) {
+            g_printerr("# skip unsupported aes-128:cbc\n");
             continue;
         }
         if (!test_data[i].slow ||
-- 
2.49.0

[PATCH v2 3/4] tests: fix skipping cipher tests when AES is not available

[Daniel P. Berrangé]

From: Daniel P. Berrangé <berrange@redhat.com>

This avoid tests breakage when we drop support for using the
built-in AES impl as a fallback for missing crypto libraries.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/unit/test-crypto-cipher.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test-crypto-cipher.c b/tests/unit/test-crypto-cipher.c
index b328b482e1..1331d558cf 100644
--- a/tests/unit/test-crypto-cipher.c
+++ b/tests/unit/test-crypto-cipher.c
@@ -828,11 +828,16 @@ int main(int argc, char **argv)
         }
     }
 
-    g_test_add_func("/crypto/cipher/null-iv",
-                    test_cipher_null_iv);
+    if (qcrypto_cipher_supports(QCRYPTO_CIPHER_ALGO_AES_256,
+                                QCRYPTO_CIPHER_MODE_CBC)) {
+        g_test_add_func("/crypto/cipher/null-iv",
+                        test_cipher_null_iv);
 
-    g_test_add_func("/crypto/cipher/short-plaintext",
-                    test_cipher_short_plaintext);
+        g_test_add_func("/crypto/cipher/short-plaintext",
+                        test_cipher_short_plaintext);
+    } else {
+        g_printerr("# skip unsupported aes-256:cbc\n");
+    }
 
     return g_test_run();
 }
-- 
2.49.0

[PATCH v2 4/4] crypto: fully drop built-in cipher provider

[Daniel P. Berrangé]

From: Daniel P. Berrangé <berrange@redhat.com>

When originally creating the internal crypto cipher APIs, they were
wired up to use the built-in D3DES and AES implementations, as a way
to gracefully transition to the new APIs without introducing an
immediate hard dep on any external crypto libraries for the VNC
password auth (D3DES) or the qcow2 encryption (AES).

In the 6.1.0 release we dropped the built-in D3DES impl, and also
the XTS mode for the AES impl, leaving only AES with ECB/CBC modes.
The rational was that with the system emulators, it is expected that
3rd party crypto libraries will be available.

The qcow2 LUKS impl is preferred to the legacy raw AES impl, and by
default that requires AES in XTS mode, limiting the usefulness of
the built-in cipher provider.

The built-in AES impl has known timing attacks and is only suitable
for use cases where a security boundary is already not expected to
be provided (TCG).

Providing a built-in cipher impl thus potentially misleads users,
should they configure a QEMU without any crypto library, and try
to use it with the LUKS backend, even if that requires a non-default
configuration choice.

Complete what we started in 6.1.0 and purge the remaining AES
support.

Use of either gnutls, nettle, or libcrypt is now mandatory for any
cipher support, except for TCG impls.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/cipher-builtin.c.inc | 303 ------------------------------------
 crypto/cipher-stub.c.inc    |  41 +++++
 crypto/cipher.c             |   2 +-
 3 files changed, 42 insertions(+), 304 deletions(-)
 delete mode 100644 crypto/cipher-builtin.c.inc
 create mode 100644 crypto/cipher-stub.c.inc

diff --git a/crypto/cipher-builtin.c.inc b/crypto/cipher-builtin.c.inc
deleted file mode 100644
index da5fcbd9a3..0000000000
--- a/crypto/cipher-builtin.c.inc
+++ /dev/null
@@ -1,303 +0,0 @@
-/*
- * QEMU Crypto cipher built-in algorithms
- *
- * Copyright (c) 2015 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library 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
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-#include "crypto/aes.h"
-
-typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
-struct QCryptoCipherBuiltinAESContext {
-    AES_KEY enc;
-    AES_KEY dec;
-};
-
-typedef struct QCryptoCipherBuiltinAES QCryptoCipherBuiltinAES;
-struct QCryptoCipherBuiltinAES {
-    QCryptoCipher base;
-    QCryptoCipherBuiltinAESContext key;
-    uint8_t iv[AES_BLOCK_SIZE];
-};
-
-
-static inline bool qcrypto_length_check(size_t len, size_t blocksize,
-                                        Error **errp)
-{
-    if (unlikely(len & (blocksize - 1))) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, blocksize);
-        return false;
-    }
-    return true;
-}
-
-static void qcrypto_cipher_ctx_free(QCryptoCipher *cipher)
-{
-    g_free(cipher);
-}
-
-static int qcrypto_cipher_no_setiv(QCryptoCipher *cipher,
-                                   const uint8_t *iv, size_t niv,
-                                   Error **errp)
-{
-    error_setg(errp, "Setting IV is not supported");
-    return -1;
-}
-
-static void do_aes_encrypt_ecb(const void *vctx,
-                               size_t len,
-                               uint8_t *out,
-                               const uint8_t *in)
-{
-    const QCryptoCipherBuiltinAESContext *ctx = vctx;
-
-    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
-    while (len) {
-        AES_encrypt(in, out, &ctx->enc);
-        in += AES_BLOCK_SIZE;
-        out += AES_BLOCK_SIZE;
-        len -= AES_BLOCK_SIZE;
-    }
-}
-
-static void do_aes_decrypt_ecb(const void *vctx,
-                               size_t len,
-                               uint8_t *out,
-                               const uint8_t *in)
-{
-    const QCryptoCipherBuiltinAESContext *ctx = vctx;
-
-    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
-    while (len) {
-        AES_decrypt(in, out, &ctx->dec);
-        in += AES_BLOCK_SIZE;
-        out += AES_BLOCK_SIZE;
-        len -= AES_BLOCK_SIZE;
-    }
-}
-
-static void do_aes_encrypt_cbc(const AES_KEY *key,
-                               size_t len,
-                               uint8_t *out,
-                               const uint8_t *in,
-                               uint8_t *ivec)
-{
-    uint8_t tmp[AES_BLOCK_SIZE];
-    size_t n;
-
-    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
-    while (len) {
-        for (n = 0; n < AES_BLOCK_SIZE; ++n) {
-            tmp[n] = in[n] ^ ivec[n];
-        }
-        AES_encrypt(tmp, out, key);
-        memcpy(ivec, out, AES_BLOCK_SIZE);
-        len -= AES_BLOCK_SIZE;
-        in += AES_BLOCK_SIZE;
-        out += AES_BLOCK_SIZE;
-    }
-}
-
-static void do_aes_decrypt_cbc(const AES_KEY *key,
-                               size_t len,
-                               uint8_t *out,
-                               const uint8_t *in,
-                               uint8_t *ivec)
-{
-    uint8_t tmp[AES_BLOCK_SIZE];
-    size_t n;
-
-    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
-    while (len) {
-        memcpy(tmp, in, AES_BLOCK_SIZE);
-        AES_decrypt(in, out, key);
-        for (n = 0; n < AES_BLOCK_SIZE; ++n) {
-            out[n] ^= ivec[n];
-        }
-        memcpy(ivec, tmp, AES_BLOCK_SIZE);
-        len -= AES_BLOCK_SIZE;
-        in += AES_BLOCK_SIZE;
-        out += AES_BLOCK_SIZE;
-    }
-}
-
-static int qcrypto_cipher_aes_encrypt_ecb(QCryptoCipher *cipher,
-                                          const void *in, void *out,
-                                          size_t len, Error **errp)
-{
-    QCryptoCipherBuiltinAES *ctx
-        = container_of(cipher, QCryptoCipherBuiltinAES, base);
-
-    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
-        return -1;
-    }
-    do_aes_encrypt_ecb(&ctx->key, len, out, in);
-    return 0;
-}
-
-static int qcrypto_cipher_aes_decrypt_ecb(QCryptoCipher *cipher,
-                                          const void *in, void *out,
-                                          size_t len, Error **errp)
-{
-    QCryptoCipherBuiltinAES *ctx
-        = container_of(cipher, QCryptoCipherBuiltinAES, base);
-
-    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
-        return -1;
-    }
-    do_aes_decrypt_ecb(&ctx->key, len, out, in);
-    return 0;
-}
-
-static int qcrypto_cipher_aes_encrypt_cbc(QCryptoCipher *cipher,
-                                          const void *in, void *out,
-                                          size_t len, Error **errp)
-{
-    QCryptoCipherBuiltinAES *ctx
-        = container_of(cipher, QCryptoCipherBuiltinAES, base);
-
-    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
-        return -1;
-    }
-    do_aes_encrypt_cbc(&ctx->key.enc, len, out, in, ctx->iv);
-    return 0;
-}
-
-static int qcrypto_cipher_aes_decrypt_cbc(QCryptoCipher *cipher,
-                                          const void *in, void *out,
-                                          size_t len, Error **errp)
-{
-    QCryptoCipherBuiltinAES *ctx
-        = container_of(cipher, QCryptoCipherBuiltinAES, base);
-
-    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
-        return -1;
-    }
-    do_aes_decrypt_cbc(&ctx->key.dec, len, out, in, ctx->iv);
-    return 0;
-}
-
-static int qcrypto_cipher_aes_setiv(QCryptoCipher *cipher, const uint8_t *iv,
-                             size_t niv, Error **errp)
-{
-    QCryptoCipherBuiltinAES *ctx
-        = container_of(cipher, QCryptoCipherBuiltinAES, base);
-
-    if (niv != AES_BLOCK_SIZE) {
-        error_setg(errp, "IV must be %d bytes not %zu",
-                   AES_BLOCK_SIZE, niv);
-        return -1;
-    }
-
-    memcpy(ctx->iv, iv, AES_BLOCK_SIZE);
-    return 0;
-}
-
-static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_ecb = {
-    .cipher_encrypt = qcrypto_cipher_aes_encrypt_ecb,
-    .cipher_decrypt = qcrypto_cipher_aes_decrypt_ecb,
-    .cipher_setiv = qcrypto_cipher_no_setiv,
-    .cipher_free = qcrypto_cipher_ctx_free,
-};
-
-static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_cbc = {
-    .cipher_encrypt = qcrypto_cipher_aes_encrypt_cbc,
-    .cipher_decrypt = qcrypto_cipher_aes_decrypt_cbc,
-    .cipher_setiv = qcrypto_cipher_aes_setiv,
-    .cipher_free = qcrypto_cipher_ctx_free,
-};
-
-bool qcrypto_cipher_supports(QCryptoCipherAlgo alg,
-                             QCryptoCipherMode mode)
-{
-    switch (alg) {
-    case QCRYPTO_CIPHER_ALGO_AES_128:
-    case QCRYPTO_CIPHER_ALGO_AES_192:
-    case QCRYPTO_CIPHER_ALGO_AES_256:
-        switch (mode) {
-        case QCRYPTO_CIPHER_MODE_ECB:
-        case QCRYPTO_CIPHER_MODE_CBC:
-            return true;
-        default:
-            return false;
-        }
-        break;
-    default:
-        return false;
-    }
-}
-
-static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
-                                             QCryptoCipherMode mode,
-                                             const uint8_t *key,
-                                             size_t nkey,
-                                             Error **errp)
-{
-    if (!qcrypto_cipher_validate_key_length(alg, mode, nkey, errp)) {
-        return NULL;
-    }
-
-    switch (alg) {
-    case QCRYPTO_CIPHER_ALGO_AES_128:
-    case QCRYPTO_CIPHER_ALGO_AES_192:
-    case QCRYPTO_CIPHER_ALGO_AES_256:
-        {
-            QCryptoCipherBuiltinAES *ctx;
-            const QCryptoCipherDriver *drv;
-
-            switch (mode) {
-            case QCRYPTO_CIPHER_MODE_ECB:
-                drv = &qcrypto_cipher_aes_driver_ecb;
-                break;
-            case QCRYPTO_CIPHER_MODE_CBC:
-                drv = &qcrypto_cipher_aes_driver_cbc;
-                break;
-            default:
-                goto bad_mode;
-            }
-
-            ctx = g_new0(QCryptoCipherBuiltinAES, 1);
-            ctx->base.driver = drv;
-
-            if (AES_set_encrypt_key(key, nkey * 8, &ctx->key.enc)) {
-                error_setg(errp, "Failed to set encryption key");
-                goto error;
-            }
-            if (AES_set_decrypt_key(key, nkey * 8, &ctx->key.dec)) {
-                error_setg(errp, "Failed to set decryption key");
-                goto error;
-            }
-
-            return &ctx->base;
-
-        error:
-            g_free(ctx);
-            return NULL;
-        }
-
-    default:
-        error_setg(errp,
-                   "Unsupported cipher algorithm %s",
-                   QCryptoCipherAlgo_str(alg));
-        return NULL;
-    }
-
- bad_mode:
-    error_setg(errp, "Unsupported cipher mode %s",
-               QCryptoCipherMode_str(mode));
-    return NULL;
-}
diff --git a/crypto/cipher-stub.c.inc b/crypto/cipher-stub.c.inc
new file mode 100644
index 0000000000..2574882d89
--- /dev/null
+++ b/crypto/cipher-stub.c.inc
@@ -0,0 +1,41 @@
+/*
+ * QEMU Crypto cipher built-in algorithms
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+bool qcrypto_cipher_supports(QCryptoCipherAlgo alg,
+                             QCryptoCipherMode mode)
+{
+    return false;
+}
+
+static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgo alg,
+                                             QCryptoCipherMode mode,
+                                             const uint8_t *key,
+                                             size_t nkey,
+                                             Error **errp)
+{
+    if (!qcrypto_cipher_validate_key_length(alg, mode, nkey, errp)) {
+        return NULL;
+    }
+
+    error_setg(errp,
+               "Unsupported cipher algorithm %s, no crypto library enabled in build",
+               QCryptoCipherAlgo_str(alg));
+    return NULL;
+}
diff --git a/crypto/cipher.c b/crypto/cipher.c
index c14a8b8a11..229710f76b 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -145,7 +145,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgo alg,
 #elif defined CONFIG_GNUTLS_CRYPTO
 #include "cipher-gnutls.c.inc"
 #else
-#include "cipher-builtin.c.inc"
+#include "cipher-stub.c.inc"
 #endif
 
 QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgo alg,
-- 
2.49.0

Re: [PATCH v2 0/4] crypto: fully drop built-in cipher provider

[Richard Henderson]

On 5/12/25 07:24, Daniel P. Berrangé wrote:
> Daniel P. Berrangé (4):
>    tests: skip encrypted secret tests if AES is not available
>    tests: skip legacy qcow2 encryption test if AES is not available
>    tests: fix skipping cipher tests when AES is not available
>    crypto: fully drop built-in cipher provider

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~