diff mbox series

[10/18] crypto: delete built-in XTS cipher mode support

Message ID 20210706095924.764117-11-berrange@redhat.com
State New
Headers show
Series crypto: misc cleanup and introduce gnutls backend driver | expand

Commit Message

Daniel P. Berrangé July 6, 2021, 9:59 a.m. UTC
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.

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(-)

Comments

Eric Blake July 8, 2021, 6:56 p.m. UTC | #1
On Tue, Jul 06, 2021 at 10:59:16AM +0100, Daniel P. Berrangé wrote:
> 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.
> 
> 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(-)

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

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 2821edc0f5..07b4e7f950 100644
--- a/meson.build
+++ b/meson.build
@@ -821,7 +821,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()
@@ -829,8 +829,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()
@@ -838,7 +838,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...