qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Eduardo Otubo" <otubo@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Juan Quintela" <quintela@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Jiri Pirko" <jiri@resnulli.us>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: [PULL 10/26] crypto: delete built-in XTS cipher mode support
Date: Wed, 14 Jul 2021 15:08:42 +0100	[thread overview]
Message-ID: <20210714140858.2247409-11-berrange@redhat.com> (raw)
In-Reply-To: <20210714140858.2247409-1-berrange@redhat.com>

The built-in AES+XTS implementation is used for the LUKS encryption
When building system emulators it is reasonable to expect that an
external crypto library is being used instead. The performance of the
builtin XTS implementation is terrible as it has no CPU acceleration
support. It is thus not worth keeping a home grown XTS implementation
for the built-in cipher backend.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/cipher-builtin.c.inc | 60 -------------------------------------
 crypto/meson.build          |  6 ++--
 meson.build                 |  7 ++---
 3 files changed, 6 insertions(+), 67 deletions(-)

diff --git a/crypto/cipher-builtin.c.inc b/crypto/cipher-builtin.c.inc
index 70743f253c..b409089095 100644
--- a/crypto/cipher-builtin.c.inc
+++ b/crypto/cipher-builtin.c.inc
@@ -19,7 +19,6 @@
  */
 
 #include "crypto/aes.h"
-#include "crypto/xts.h"
 
 typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
 struct QCryptoCipherBuiltinAESContext {
@@ -31,7 +30,6 @@ typedef struct QCryptoCipherBuiltinAES QCryptoCipherBuiltinAES;
 struct QCryptoCipherBuiltinAES {
     QCryptoCipher base;
     QCryptoCipherBuiltinAESContext key;
-    QCryptoCipherBuiltinAESContext key_tweak;
     uint8_t iv[AES_BLOCK_SIZE];
 };
 
@@ -193,39 +191,6 @@ static int qcrypto_cipher_aes_decrypt_cbc(QCryptoCipher *cipher,
     return 0;
 }
 
-static int qcrypto_cipher_aes_encrypt_xts(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;
-    }
-    xts_encrypt(&ctx->key, &ctx->key_tweak,
-                do_aes_encrypt_ecb, do_aes_decrypt_ecb,
-                ctx->iv, len, out, in);
-    return 0;
-}
-
-static int qcrypto_cipher_aes_decrypt_xts(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;
-    }
-    xts_decrypt(&ctx->key, &ctx->key_tweak,
-                do_aes_encrypt_ecb, do_aes_decrypt_ecb,
-                ctx->iv, len, out, in);
-    return 0;
-}
-
-
 static int qcrypto_cipher_aes_setiv(QCryptoCipher *cipher, const uint8_t *iv,
                              size_t niv, Error **errp)
 {
@@ -256,14 +221,6 @@ static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_cbc = {
     .cipher_free = qcrypto_cipher_ctx_free,
 };
 
-static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_xts = {
-    .cipher_encrypt = qcrypto_cipher_aes_encrypt_xts,
-    .cipher_decrypt = qcrypto_cipher_aes_decrypt_xts,
-    .cipher_setiv = qcrypto_cipher_aes_setiv,
-    .cipher_free = qcrypto_cipher_ctx_free,
-};
-
-
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
 {
@@ -274,7 +231,6 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
         switch (mode) {
         case QCRYPTO_CIPHER_MODE_ECB:
         case QCRYPTO_CIPHER_MODE_CBC:
-        case QCRYPTO_CIPHER_MODE_XTS:
             return true;
         default:
             return false;
@@ -310,9 +266,6 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
             case QCRYPTO_CIPHER_MODE_CBC:
                 drv = &qcrypto_cipher_aes_driver_cbc;
                 break;
-            case QCRYPTO_CIPHER_MODE_XTS:
-                drv = &qcrypto_cipher_aes_driver_xts;
-                break;
             default:
                 goto bad_mode;
             }
@@ -320,19 +273,6 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
             ctx = g_new0(QCryptoCipherBuiltinAES, 1);
             ctx->base.driver = drv;
 
-            if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-                nkey /= 2;
-                if (AES_set_encrypt_key(key + nkey, nkey * 8,
-                                        &ctx->key_tweak.enc)) {
-                    error_setg(errp, "Failed to set encryption key");
-                    goto error;
-                }
-                if (AES_set_decrypt_key(key + nkey, nkey * 8,
-                                        &ctx->key_tweak.dec)) {
-                    error_setg(errp, "Failed to set decryption key");
-                    goto error;
-                }
-            }
             if (AES_set_encrypt_key(key, nkey * 8, &ctx->key.enc)) {
                 error_setg(errp, "Failed to set encryption key");
                 goto error;
diff --git a/crypto/meson.build b/crypto/meson.build
index b384ca8b57..fc8de287e1 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -23,14 +23,14 @@ crypto_ss.add(files(
 
 if nettle.found()
   crypto_ss.add(nettle, files('hash-nettle.c', 'hmac-nettle.c', 'pbkdf-nettle.c'))
+  if xts == 'private'
+    crypto_ss.add(files('xts.c'))
+  endif
 elif gcrypt.found()
   crypto_ss.add(gcrypt, files('hash-gcrypt.c', 'hmac-gcrypt.c', 'pbkdf-gcrypt.c'))
 else
   crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
 endif
-if xts == 'private'
-  crypto_ss.add(files('xts.c'))
-endif
 
 crypto_ss.add(when: 'CONFIG_SECRET_KEYRING', if_true: files('secret_keyring.c'))
 crypto_ss.add(when: 'CONFIG_AF_ALG', if_true: files('afalg.c', 'cipher-afalg.c', 'hash-afalg.c'))
diff --git a/meson.build b/meson.build
index a96c8b858c..8f899e1e9b 100644
--- a/meson.build
+++ b/meson.build
@@ -826,7 +826,7 @@ endif
 # Nettle has priority over gcrypt
 gcrypt = not_found
 nettle = not_found
-xts = 'private'
+xts = 'none'
 if get_option('nettle').enabled() and get_option('gcrypt').enabled()
   error('Only one of gcrypt & nettle can be enabled')
 elif (not get_option('nettle').auto() or have_system) and not get_option('gcrypt').enabled()
@@ -834,8 +834,8 @@ elif (not get_option('nettle').auto() or have_system) and not get_option('gcrypt
                       method: 'pkg-config',
                       required: get_option('nettle'),
                       kwargs: static_kwargs)
-  if nettle.found() and cc.has_header('nettle/xts.h', dependencies: nettle)
-    xts = 'nettle'
+  if nettle.found() and not cc.has_header('nettle/xts.h', dependencies: nettle)
+    xts = 'private'
   endif
 endif
 if (not get_option('gcrypt').auto() or have_system) and not nettle.found()
@@ -843,7 +843,6 @@ if (not get_option('gcrypt').auto() or have_system) and not nettle.found()
                          method: 'config-tool',
                          required: get_option('gcrypt'),
                          kwargs: static_kwargs)
-  xts = 'gcrypt'
   # Debian has removed -lgpg-error from libgcrypt-config
   # as it "spreads unnecessary dependencies" which in
   # turn breaks static builds...
-- 
2.31.1



  parent reply	other threads:[~2021-07-14 14:17 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14 14:08 [PULL v2 00/26] Crypto and more patches Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 01/26] crypto: remove conditional around 3DES crypto test cases Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 02/26] crypto: remove obsolete crypto test condition Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 03/26] crypto: skip essiv ivgen tests if AES+ECB isn't available Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 04/26] crypto: use &error_fatal in crypto tests Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 05/26] crypto: fix gcrypt min version 1.8 regression Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 06/26] crypto: drop gcrypt thread initialization code Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 07/26] crypto: drop custom XTS support in gcrypt driver Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 08/26] crypto: add crypto tests for single block DES-ECB and DES-CBC Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 09/26] crypto: delete built-in DES implementation Daniel P. Berrangé
2021-07-14 14:08 ` Daniel P. Berrangé [this message]
2021-07-14 14:08 ` [PULL 11/26] crypto: replace 'des-rfb' cipher with 'des' Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 12/26] crypto: flip priority of backends to prefer gcrypt Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 13/26] crypto: introduce build system for gnutls crypto backend Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 14/26] crypto: add gnutls cipher provider Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 15/26] crypto: add gnutls hash provider Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 16/26] crypto: add gnutls hmac provider Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 17/26] crypto: add gnutls pbkdf provider Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 18/26] crypto: prefer gnutls as the crypto backend if new enough Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 19/26] net/rocker: use GDateTime for formatting timestamp in debug messages Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 20/26] io: use GDateTime for formatting timestamp for websock headers Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 21/26] seccomp: don't block getters for resource control syscalls Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 22/26] tests/migration: fix unix socket migration Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 23/26] docs: fix typo s/Intel/AMD/ in CPU model notes Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 24/26] qemu-options: re-arrange CPU topology options Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 25/26] qemu-options: tweak to show that CPU count is optional Daniel P. Berrangé
2021-07-14 14:08 ` [PULL 26/26] qemu-options: rewrite help for -smp options Daniel P. Berrangé
2021-07-15 20:38 ` [PULL v2 00/26] Crypto and more patches Peter Maydell

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=20210714140858.2247409-11-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=kraxel@redhat.com \
    --cc=otubo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=richard.henderson@linaro.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).