qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: [PATCH v2 4/4] crypto: fully drop built-in cipher provider
Date: Mon, 12 May 2025 15:24:39 +0100	[thread overview]
Message-ID: <20250512142439.1101159-5-berrange@redhat.com> (raw)
In-Reply-To: <20250512142439.1101159-1-berrange@redhat.com>

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



  parent reply	other threads:[~2025-05-12 14:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-12 14:24 [PATCH v2 0/4] crypto: fully drop built-in cipher provider Daniel P. Berrangé
2025-05-12 14:24 ` [PATCH v2 1/4] tests: skip encrypted secret tests if AES is not available Daniel P. Berrangé
2025-05-12 14:24 ` [PATCH v2 2/4] tests: skip legacy qcow2 encryption test " Daniel P. Berrangé
2025-05-12 14:24 ` [PATCH v2 3/4] tests: fix skipping cipher tests when " Daniel P. Berrangé
2025-05-12 14:24 ` Daniel P. Berrangé [this message]
2025-05-13 10:05 ` [PATCH v2 0/4] crypto: fully drop built-in cipher provider Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250512142439.1101159-5-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).