* [PATCH 01/18] crypto: remove conditional around 3DES crypto test cases
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:27 ` Eric Blake
2021-07-06 9:59 ` [PATCH 02/18] crypto: remove obsolete crypto test condition Daniel P. Berrangé
` (16 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
The main method checks whether the cipher choice is supported
at runtime, so there is no need for compile time conditions.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-crypto-cipher.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tests/unit/test-crypto-cipher.c b/tests/unit/test-crypto-cipher.c
index 280319a223..fd0a8de34c 100644
--- a/tests/unit/test-crypto-cipher.c
+++ b/tests/unit/test-crypto-cipher.c
@@ -165,7 +165,6 @@ static QCryptoCipherTestData test_data[] = {
"ffd29f1bb5596ad94ea2d8e6196b7f09"
"30d8ed0bf2773af36dd82a6280c20926",
},
-#if defined(CONFIG_NETTLE) || defined(CONFIG_GCRYPT)
{
/* Borrowed from linux-kernel crypto/testmgr.h */
.path = "/crypto/cipher/3des-cbc",
@@ -283,7 +282,6 @@ static QCryptoCipherTestData test_data[] = {
"407772c2ea0e3a7846b991b6e73d5142"
"fd51b0c62c6313785ceefccfc4700034",
},
-#endif
{
/* RFC 2144, Appendix B.1 */
.path = "/crypto/cipher/cast5-128",
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 01/18] crypto: remove conditional around 3DES crypto test cases
2021-07-06 9:59 ` [PATCH 01/18] crypto: remove conditional around 3DES crypto test cases Daniel P. Berrangé
@ 2021-07-08 18:27 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:27 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:07AM +0100, Daniel P. Berrangé wrote:
> The main method checks whether the cipher choice is supported
> at runtime, so there is no need for compile time conditions.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-cipher.c | 2 --
> 1 file changed, 2 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 02/18] crypto: remove obsolete crypto test condition
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
2021-07-06 9:59 ` [PATCH 01/18] crypto: remove conditional around 3DES crypto test cases Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:28 ` Eric Blake
2021-07-06 9:59 ` [PATCH 03/18] crypto: skip essiv ivgen tests if AES+ECB isn't available Daniel P. Berrangé
` (15 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
Since we now require gcrypt >= 1.8.0, there is no need
to exclude the pbkdf test case.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-crypto-pbkdf.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tests/unit/test-crypto-pbkdf.c b/tests/unit/test-crypto-pbkdf.c
index c50fd639d2..43c417f6b4 100644
--- a/tests/unit/test-crypto-pbkdf.c
+++ b/tests/unit/test-crypto-pbkdf.c
@@ -229,10 +229,8 @@ static QCryptoPbkdfTestData test_data[] = {
},
/* non-RFC misc test data */
-#ifdef CONFIG_NETTLE
{
- /* empty password test.
- * Broken with libgcrypt <= 1.5.0, hence CONFIG_NETTLE */
+ /* empty password test. */
.path = "/crypto/pbkdf/nonrfc/sha1/iter2",
.hash = QCRYPTO_HASH_ALG_SHA1,
.iterations = 2,
@@ -244,7 +242,6 @@ static QCryptoPbkdfTestData test_data[] = {
"\xbf\x03\xe1\x1c\x71\xca\x79\x4e\x07\x97",
.nout = 20
},
-#endif
{
/* Password exceeds block size test */
.path = "/crypto/pbkdf/nonrfc/sha256/iter1200",
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 02/18] crypto: remove obsolete crypto test condition
2021-07-06 9:59 ` [PATCH 02/18] crypto: remove obsolete crypto test condition Daniel P. Berrangé
@ 2021-07-08 18:28 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:28 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:08AM +0100, Daniel P. Berrangé wrote:
> Since we now require gcrypt >= 1.8.0, there is no need
> to exclude the pbkdf test case.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-pbkdf.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 03/18] crypto: skip essiv ivgen tests if AES+ECB isn't available
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
2021-07-06 9:59 ` [PATCH 01/18] crypto: remove conditional around 3DES crypto test cases Daniel P. Berrangé
2021-07-06 9:59 ` [PATCH 02/18] crypto: remove obsolete crypto test condition Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:29 ` Eric Blake
2021-07-06 9:59 ` [PATCH 04/18] crypto: use &error_fatal in crypto tests Daniel P. Berrangé
` (14 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-crypto-ivgen.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tests/unit/test-crypto-ivgen.c b/tests/unit/test-crypto-ivgen.c
index f581e6aba7..29630ed348 100644
--- a/tests/unit/test-crypto-ivgen.c
+++ b/tests/unit/test-crypto-ivgen.c
@@ -136,8 +136,15 @@ struct QCryptoIVGenTestData {
static void test_ivgen(const void *opaque)
{
const struct QCryptoIVGenTestData *data = opaque;
- uint8_t *iv = g_new0(uint8_t, data->niv);
- QCryptoIVGen *ivgen = qcrypto_ivgen_new(
+ g_autofree uint8_t *iv = g_new0(uint8_t, data->niv);
+ g_autoptr(QCryptoIVGen) ivgen = NULL;
+
+ if (!qcrypto_cipher_supports(data->cipheralg,
+ QCRYPTO_CIPHER_MODE_ECB)) {
+ return;
+ }
+
+ ivgen = qcrypto_ivgen_new(
data->ivalg,
data->cipheralg,
data->hashalg,
@@ -152,9 +159,6 @@ static void test_ivgen(const void *opaque)
&error_abort);
g_assert(memcmp(iv, data->iv, data->niv) == 0);
-
- qcrypto_ivgen_free(ivgen);
- g_free(iv);
}
int main(int argc, char **argv)
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 03/18] crypto: skip essiv ivgen tests if AES+ECB isn't available
2021-07-06 9:59 ` [PATCH 03/18] crypto: skip essiv ivgen tests if AES+ECB isn't available Daniel P. Berrangé
@ 2021-07-08 18:29 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:29 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:09AM +0100, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-ivgen.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 04/18] crypto: use &error_fatal in crypto tests
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (2 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 03/18] crypto: skip essiv ivgen tests if AES+ECB isn't available Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:33 ` Eric Blake
2021-07-06 9:59 ` [PATCH 05/18] crypto: fix gcrypt min version 1.8 regression Daniel P. Berrangé
` (13 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
Using error_fatal provides better diagnostics when tests
failed, than using asserts, because we see the text of
the error message.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-crypto-hash.c | 12 ++++++------
tests/unit/test-crypto-hmac.c | 28 ++++++++--------------------
2 files changed, 14 insertions(+), 26 deletions(-)
diff --git a/tests/unit/test-crypto-hash.c b/tests/unit/test-crypto-hash.c
index ce7d0ab9b5..b50e28f212 100644
--- a/tests/unit/test-crypto-hash.c
+++ b/tests/unit/test-crypto-hash.c
@@ -104,7 +104,7 @@ static void test_hash_alloc(void)
strlen(INPUT_TEXT),
&result,
&resultlen,
- NULL);
+ &error_fatal);
g_assert(ret == 0);
g_assert(resultlen == expected_lens[i]);
@@ -139,7 +139,7 @@ static void test_hash_prealloc(void)
strlen(INPUT_TEXT),
&result,
&resultlen,
- NULL);
+ &error_fatal);
g_assert(ret == 0);
g_assert(resultlen == expected_lens[i]);
@@ -176,7 +176,7 @@ static void test_hash_iov(void)
iov, 3,
&result,
&resultlen,
- NULL);
+ &error_fatal);
g_assert(ret == 0);
g_assert(resultlen == expected_lens[i]);
for (j = 0; j < resultlen; j++) {
@@ -210,7 +210,7 @@ static void test_hash_digest(void)
INPUT_TEXT,
strlen(INPUT_TEXT),
&digest,
- NULL);
+ &error_fatal);
g_assert(ret == 0);
g_assert_cmpstr(digest, ==, expected_outputs[i]);
g_free(digest);
@@ -234,7 +234,7 @@ static void test_hash_base64(void)
INPUT_TEXT,
strlen(INPUT_TEXT),
&digest,
- NULL);
+ &error_fatal);
g_assert(ret == 0);
g_assert_cmpstr(digest, ==, expected_outputs_b64[i]);
g_free(digest);
@@ -243,7 +243,7 @@ static void test_hash_base64(void)
int main(int argc, char **argv)
{
- g_assert(qcrypto_init(NULL) == 0);
+ g_assert(qcrypto_init(&error_fatal) == 0);
g_test_init(&argc, &argv, NULL);
g_test_add_func("/crypto/hash/iov", test_hash_iov);
diff --git a/tests/unit/test-crypto-hmac.c b/tests/unit/test-crypto-hmac.c
index ee55382a3c..23eb724d94 100644
--- a/tests/unit/test-crypto-hmac.c
+++ b/tests/unit/test-crypto-hmac.c
@@ -89,7 +89,6 @@ static void test_hmac_alloc(void)
QCryptoHmac *hmac = NULL;
uint8_t *result = NULL;
size_t resultlen = 0;
- Error *err = NULL;
const char *exp_output = NULL;
int ret;
size_t j;
@@ -101,14 +100,12 @@ static void test_hmac_alloc(void)
exp_output = data->hex_digest;
hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
- strlen(KEY), &err);
- g_assert(err == NULL);
+ strlen(KEY), &error_fatal);
g_assert(hmac != NULL);
ret = qcrypto_hmac_bytes(hmac, (const char *)INPUT_TEXT,
strlen(INPUT_TEXT), &result,
- &resultlen, &err);
- g_assert(err == NULL);
+ &resultlen, &error_fatal);
g_assert(ret == 0);
for (j = 0; j < resultlen; j++) {
@@ -131,7 +128,6 @@ static void test_hmac_prealloc(void)
QCryptoHmac *hmac = NULL;
uint8_t *result = NULL;
size_t resultlen = 0;
- Error *err = NULL;
const char *exp_output = NULL;
int ret;
size_t j;
@@ -146,14 +142,12 @@ static void test_hmac_prealloc(void)
result = g_new0(uint8_t, resultlen);
hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
- strlen(KEY), &err);
- g_assert(err == NULL);
+ strlen(KEY), &error_fatal);
g_assert(hmac != NULL);
ret = qcrypto_hmac_bytes(hmac, (const char *)INPUT_TEXT,
strlen(INPUT_TEXT), &result,
- &resultlen, &err);
- g_assert(err == NULL);
+ &resultlen, &error_fatal);
g_assert(ret == 0);
exp_output = data->hex_digest;
@@ -177,7 +171,6 @@ static void test_hmac_iov(void)
QCryptoHmac *hmac = NULL;
uint8_t *result = NULL;
size_t resultlen = 0;
- Error *err = NULL;
const char *exp_output = NULL;
int ret;
size_t j;
@@ -194,13 +187,11 @@ static void test_hmac_iov(void)
exp_output = data->hex_digest;
hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
- strlen(KEY), &err);
- g_assert(err == NULL);
+ strlen(KEY), &error_fatal);
g_assert(hmac != NULL);
ret = qcrypto_hmac_bytesv(hmac, iov, 3, &result,
- &resultlen, &err);
- g_assert(err == NULL);
+ &resultlen, &error_fatal);
g_assert(ret == 0);
for (j = 0; j < resultlen; j++) {
@@ -222,7 +213,6 @@ static void test_hmac_digest(void)
QCryptoHmacTestData *data = &test_data[i];
QCryptoHmac *hmac = NULL;
uint8_t *result = NULL;
- Error *err = NULL;
const char *exp_output = NULL;
int ret;
@@ -233,14 +223,12 @@ static void test_hmac_digest(void)
exp_output = data->hex_digest;
hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
- strlen(KEY), &err);
- g_assert(err == NULL);
+ strlen(KEY), &error_fatal);
g_assert(hmac != NULL);
ret = qcrypto_hmac_digest(hmac, (const char *)INPUT_TEXT,
strlen(INPUT_TEXT), (char **)&result,
- &err);
- g_assert(err == NULL);
+ &error_fatal);
g_assert(ret == 0);
g_assert_cmpstr((const char *)result, ==, exp_output);
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 04/18] crypto: use &error_fatal in crypto tests
2021-07-06 9:59 ` [PATCH 04/18] crypto: use &error_fatal in crypto tests Daniel P. Berrangé
@ 2021-07-08 18:33 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:33 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:10AM +0100, Daniel P. Berrangé wrote:
> Using error_fatal provides better diagnostics when tests
> failed, than using asserts, because we see the text of
> the error message.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-hash.c | 12 ++++++------
> tests/unit/test-crypto-hmac.c | 28 ++++++++--------------------
> 2 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/tests/unit/test-crypto-hash.c b/tests/unit/test-crypto-hash.c
> index ce7d0ab9b5..b50e28f212 100644
> --- a/tests/unit/test-crypto-hash.c
> @@ -243,7 +243,7 @@ static void test_hash_base64(void)
>
> int main(int argc, char **argv)
> {
> - g_assert(qcrypto_init(NULL) == 0);
> + g_assert(qcrypto_init(&error_fatal) == 0);
This is a side effect inside a g_assert(). It might be worth cleaning
it up while you are touching here. But since it is pre-existing, it
doesn't affect my:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 05/18] crypto: fix gcrypt min version 1.8 regression
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (3 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 04/18] crypto: use &error_fatal in crypto tests Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:34 ` Eric Blake
2021-07-06 9:59 ` [PATCH 06/18] crypto: drop gcrypt thread initialization code Daniel P. Berrangé
` (12 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
The min gcrypt was bumped:
commit b33a84632a3759c00320fd80923aa963c11207fc
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Fri May 14 13:04:08 2021 +0100
crypto: bump min gcrypt to 1.8.0, dropping RHEL-7 support
but this was accidentally lost in conflict resolution for
commit 5761251138cb69c310e9df7dfc82c4c6fd2444e4
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Thu Jun 3 11:15:26 2021 +0200
configure, meson: convert crypto detection to meson
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index db6789af9c..945ae9c81d 100644
--- a/meson.build
+++ b/meson.build
@@ -834,7 +834,7 @@ elif (not get_option('nettle').auto() or have_system) and not get_option('gcrypt
endif
endif
if (not get_option('gcrypt').auto() or have_system) and not nettle.found()
- gcrypt = dependency('libgcrypt', version: '>=1.5',
+ gcrypt = dependency('libgcrypt', version: '>=1.8',
method: 'config-tool',
required: get_option('gcrypt'),
kwargs: static_kwargs)
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 05/18] crypto: fix gcrypt min version 1.8 regression
2021-07-06 9:59 ` [PATCH 05/18] crypto: fix gcrypt min version 1.8 regression Daniel P. Berrangé
@ 2021-07-08 18:34 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:34 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:11AM +0100, Daniel P. Berrangé wrote:
> The min gcrypt was bumped:
>
> commit b33a84632a3759c00320fd80923aa963c11207fc
> Author: Daniel P. Berrangé <berrange@redhat.com>
> Date: Fri May 14 13:04:08 2021 +0100
>
> crypto: bump min gcrypt to 1.8.0, dropping RHEL-7 support
>
> but this was accidentally lost in conflict resolution for
>
> commit 5761251138cb69c310e9df7dfc82c4c6fd2444e4
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date: Thu Jun 3 11:15:26 2021 +0200
>
> configure, meson: convert crypto detection to meson
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 06/18] crypto: drop gcrypt thread initialization code
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (4 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 05/18] crypto: fix gcrypt min version 1.8 regression Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:36 ` Eric Blake
2021-07-06 9:59 ` [PATCH 07/18] crypto: drop custom XTS support in gcrypt driver Daniel P. Berrangé
` (11 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
This is only required on gcrypt < 1.6.0, and is thus obsolete
since
commit b33a84632a3759c00320fd80923aa963c11207fc
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Fri May 14 13:04:08 2021 +0100
crypto: bump min gcrypt to 1.8.0, dropping RHEL-7 support
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/init.c | 62 ---------------------------------------------------
1 file changed, 62 deletions(-)
diff --git a/crypto/init.c b/crypto/init.c
index ea233b9192..fb7f1bff10 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -35,21 +35,6 @@
#include "crypto/random.h"
/* #define DEBUG_GNUTLS */
-
-/*
- * We need to init gcrypt threading if
- *
- * - gcrypt < 1.6.0
- *
- */
-
-#if (defined(CONFIG_GCRYPT) && \
- (GCRYPT_VERSION_NUMBER < 0x010600))
-#define QCRYPTO_INIT_GCRYPT_THREADS
-#else
-#undef QCRYPTO_INIT_GCRYPT_THREADS
-#endif
-
#ifdef DEBUG_GNUTLS
static void qcrypto_gnutls_log(int level, const char *str)
{
@@ -57,55 +42,8 @@ static void qcrypto_gnutls_log(int level, const char *str)
}
#endif
-#ifdef QCRYPTO_INIT_GCRYPT_THREADS
-static int qcrypto_gcrypt_mutex_init(void **priv)
-{ \
- QemuMutex *lock = NULL;
- lock = g_new0(QemuMutex, 1);
- qemu_mutex_init(lock);
- *priv = lock;
- return 0;
-}
-
-static int qcrypto_gcrypt_mutex_destroy(void **priv)
-{
- QemuMutex *lock = *priv;
- qemu_mutex_destroy(lock);
- g_free(lock);
- return 0;
-}
-
-static int qcrypto_gcrypt_mutex_lock(void **priv)
-{
- QemuMutex *lock = *priv;
- qemu_mutex_lock(lock);
- return 0;
-}
-
-static int qcrypto_gcrypt_mutex_unlock(void **priv)
-{
- QemuMutex *lock = *priv;
- qemu_mutex_unlock(lock);
- return 0;
-}
-
-static struct gcry_thread_cbs qcrypto_gcrypt_thread_impl = {
- (GCRY_THREAD_OPTION_PTHREAD | (GCRY_THREAD_OPTION_VERSION << 8)),
- NULL,
- qcrypto_gcrypt_mutex_init,
- qcrypto_gcrypt_mutex_destroy,
- qcrypto_gcrypt_mutex_lock,
- qcrypto_gcrypt_mutex_unlock,
- NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
-};
-#endif /* QCRYPTO_INIT_GCRYPT */
-
int qcrypto_init(Error **errp)
{
-#ifdef QCRYPTO_INIT_GCRYPT_THREADS
- gcry_control(GCRYCTL_SET_THREAD_CBS, &qcrypto_gcrypt_thread_impl);
-#endif /* QCRYPTO_INIT_GCRYPT_THREADS */
-
#ifdef CONFIG_GNUTLS
int ret;
ret = gnutls_global_init();
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 06/18] crypto: drop gcrypt thread initialization code
2021-07-06 9:59 ` [PATCH 06/18] crypto: drop gcrypt thread initialization code Daniel P. Berrangé
@ 2021-07-08 18:36 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:36 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:12AM +0100, Daniel P. Berrangé wrote:
> This is only required on gcrypt < 1.6.0, and is thus obsolete
> since
>
> commit b33a84632a3759c00320fd80923aa963c11207fc
> Author: Daniel P. Berrangé <berrange@redhat.com>
> Date: Fri May 14 13:04:08 2021 +0100
>
> crypto: bump min gcrypt to 1.8.0, dropping RHEL-7 support
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/init.c | 62 ---------------------------------------------------
> 1 file changed, 62 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 07/18] crypto: drop custom XTS support in gcrypt driver
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (5 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 06/18] crypto: drop gcrypt thread initialization code Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:40 ` Eric Blake
2021-07-06 9:59 ` [PATCH 08/18] crypto: add crypto tests for single block DES-ECB and DES-CBC Daniel P. Berrangé
` (10 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
The XTS cipher mode was introduced in gcrypt 1.8.0, which
matches QEMU's current minimum version.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/cipher-gcrypt.c.inc | 127 -------------------------------------
meson.build | 14 +---
2 files changed, 1 insertion(+), 140 deletions(-)
diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
index 42d4137534..3aab08a1a9 100644
--- a/crypto/cipher-gcrypt.c.inc
+++ b/crypto/cipher-gcrypt.c.inc
@@ -18,10 +18,6 @@
*
*/
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-#include "crypto/xts.h"
-#endif
-
#include <gcrypt.h>
bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
@@ -59,10 +55,6 @@ typedef struct QCryptoCipherGcrypt {
QCryptoCipher base;
gcry_cipher_hd_t handle;
size_t blocksize;
-#ifdef CONFIG_QEMU_PRIVATE_XTS
- gcry_cipher_hd_t tweakhandle;
- uint8_t iv[XTS_BLOCK_SIZE];
-#endif
} QCryptoCipherGcrypt;
@@ -178,90 +170,6 @@ static const struct QCryptoCipherDriver qcrypto_gcrypt_ctr_driver = {
.cipher_free = qcrypto_gcrypt_ctx_free,
};
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-static void qcrypto_gcrypt_xts_ctx_free(QCryptoCipher *cipher)
-{
- QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-
- gcry_cipher_close(ctx->tweakhandle);
- qcrypto_gcrypt_ctx_free(cipher);
-}
-
-static void qcrypto_gcrypt_xts_wrape(const void *ctx, size_t length,
- uint8_t *dst, const uint8_t *src)
-{
- gcry_error_t err;
- err = gcry_cipher_encrypt((gcry_cipher_hd_t)ctx, dst, length, src, length);
- g_assert(err == 0);
-}
-
-static void qcrypto_gcrypt_xts_wrapd(const void *ctx, size_t length,
- uint8_t *dst, const uint8_t *src)
-{
- gcry_error_t err;
- err = gcry_cipher_decrypt((gcry_cipher_hd_t)ctx, dst, length, src, length);
- g_assert(err == 0);
-}
-
-static int qcrypto_gcrypt_xts_encrypt(QCryptoCipher *cipher, const void *in,
- void *out, size_t len, Error **errp)
-{
- QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-
- if (len & (ctx->blocksize - 1)) {
- error_setg(errp, "Length %zu must be a multiple of block size %zu",
- len, ctx->blocksize);
- return -1;
- }
-
- xts_encrypt(ctx->handle, ctx->tweakhandle,
- qcrypto_gcrypt_xts_wrape, qcrypto_gcrypt_xts_wrapd,
- ctx->iv, len, out, in);
- return 0;
-}
-
-static int qcrypto_gcrypt_xts_decrypt(QCryptoCipher *cipher, const void *in,
- void *out, size_t len, Error **errp)
-{
- QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-
- if (len & (ctx->blocksize - 1)) {
- error_setg(errp, "Length %zu must be a multiple of block size %zu",
- len, ctx->blocksize);
- return -1;
- }
-
- xts_decrypt(ctx->handle, ctx->tweakhandle,
- qcrypto_gcrypt_xts_wrape, qcrypto_gcrypt_xts_wrapd,
- ctx->iv, len, out, in);
- return 0;
-}
-
-static int qcrypto_gcrypt_xts_setiv(QCryptoCipher *cipher,
- const uint8_t *iv, size_t niv,
- Error **errp)
-{
- QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-
- if (niv != ctx->blocksize) {
- error_setg(errp, "Expected IV size %zu not %zu",
- ctx->blocksize, niv);
- return -1;
- }
-
- memcpy(ctx->iv, iv, niv);
- return 0;
-}
-
-static const struct QCryptoCipherDriver qcrypto_gcrypt_xts_driver = {
- .cipher_encrypt = qcrypto_gcrypt_xts_encrypt,
- .cipher_decrypt = qcrypto_gcrypt_xts_decrypt,
- .cipher_setiv = qcrypto_gcrypt_xts_setiv,
- .cipher_free = qcrypto_gcrypt_xts_ctx_free,
-};
-#endif /* CONFIG_QEMU_PRIVATE_XTS */
-
-
static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode,
const uint8_t *key,
@@ -323,12 +231,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
gcrymode = GCRY_CIPHER_MODE_ECB;
break;
case QCRYPTO_CIPHER_MODE_XTS:
-#ifdef CONFIG_QEMU_PRIVATE_XTS
- drv = &qcrypto_gcrypt_xts_driver;
- gcrymode = GCRY_CIPHER_MODE_ECB;
-#else
gcrymode = GCRY_CIPHER_MODE_XTS;
-#endif
break;
case QCRYPTO_CIPHER_MODE_CBC:
gcrymode = GCRY_CIPHER_MODE_CBC;
@@ -354,23 +257,6 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
}
ctx->blocksize = gcry_cipher_get_algo_blklen(gcryalg);
-#ifdef CONFIG_QEMU_PRIVATE_XTS
- if (mode == QCRYPTO_CIPHER_MODE_XTS) {
- if (ctx->blocksize != XTS_BLOCK_SIZE) {
- error_setg(errp,
- "Cipher block size %zu must equal XTS block size %d",
- ctx->blocksize, XTS_BLOCK_SIZE);
- goto error;
- }
- err = gcry_cipher_open(&ctx->tweakhandle, gcryalg, gcrymode, 0);
- if (err != 0) {
- error_setg(errp, "Cannot initialize cipher: %s",
- gcry_strerror(err));
- goto error;
- }
- }
-#endif
-
if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
/* We're using standard DES cipher from gcrypt, so we need
* to munge the key so that the results are the same as the
@@ -380,16 +266,6 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
err = gcry_cipher_setkey(ctx->handle, rfbkey, nkey);
g_free(rfbkey);
} else {
-#ifdef CONFIG_QEMU_PRIVATE_XTS
- if (mode == QCRYPTO_CIPHER_MODE_XTS) {
- nkey /= 2;
- err = gcry_cipher_setkey(ctx->tweakhandle, key + nkey, nkey);
- if (err != 0) {
- error_setg(errp, "Cannot set key: %s", gcry_strerror(err));
- goto error;
- }
- }
-#endif
err = gcry_cipher_setkey(ctx->handle, key, nkey);
}
if (err != 0) {
@@ -400,9 +276,6 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
return &ctx->base;
error:
-#ifdef CONFIG_QEMU_PRIVATE_XTS
- gcry_cipher_close(ctx->tweakhandle);
-#endif
gcry_cipher_close(ctx->handle);
g_free(ctx);
return NULL;
diff --git a/meson.build b/meson.build
index 945ae9c81d..2821edc0f5 100644
--- a/meson.build
+++ b/meson.build
@@ -838,16 +838,7 @@ if (not get_option('gcrypt').auto() or have_system) and not nettle.found()
method: 'config-tool',
required: get_option('gcrypt'),
kwargs: static_kwargs)
- if gcrypt.found() and cc.compiles('''
- #include <gcrypt.h>
- int main(void) {
- gcry_cipher_hd_t handle;
- gcry_cipher_open(&handle, GCRY_CIPHER_AES, GCRY_CIPHER_MODE_XTS, 0);
- return 0;
- }
- ''', dependencies: gcrypt)
- xts = 'gcrypt'
- endif
+ xts = 'gcrypt'
# Debian has removed -lgpg-error from libgcrypt-config
# as it "spreads unnecessary dependencies" which in
# turn breaks static builds...
@@ -2731,9 +2722,6 @@ summary_info += {'TLS priority': config_host['CONFIG_TLS_PRIORITY']}
summary_info += {'GNUTLS support': gnutls.found()}
# TODO: add back version
summary_info += {'libgcrypt': gcrypt.found()}
-if gcrypt.found()
- summary_info += {' XTS': xts != 'private'}
-endif
# TODO: add back version
summary_info += {'nettle': nettle.found()}
if nettle.found()
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 07/18] crypto: drop custom XTS support in gcrypt driver
2021-07-06 9:59 ` [PATCH 07/18] crypto: drop custom XTS support in gcrypt driver Daniel P. Berrangé
@ 2021-07-08 18:40 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:40 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:13AM +0100, Daniel P. Berrangé wrote:
> The XTS cipher mode was introduced in gcrypt 1.8.0, which
> matches QEMU's current minimum version.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/cipher-gcrypt.c.inc | 127 -------------------------------------
> meson.build | 14 +---
> 2 files changed, 1 insertion(+), 140 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 08/18] crypto: add crypto tests for single block DES-ECB and DES-CBC
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (6 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 07/18] crypto: drop custom XTS support in gcrypt driver Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:50 ` Eric Blake
2021-07-06 9:59 ` [PATCH 09/18] crypto: delete built-in DES implementation Daniel P. Berrangé
` (9 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
The GNUTLS crypto provider doesn't support DES-ECB, only DES-CBC.
We can use the latter to simulate the former, if we encrypt only
1 block (8 bytes) of data at a time, using a all-zeros IV. This
is a very inefficient way to use the QCryptoCipher APIs, but
since the VNC authentication challenge is only 16 bytes, this
is acceptable. No other part of QEMU should be using DES. This
test case demonstrates the equivalence of ECB and CBC for the
single-block case.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-crypto-cipher.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tests/unit/test-crypto-cipher.c b/tests/unit/test-crypto-cipher.c
index fd0a8de34c..7dca7b26e4 100644
--- a/tests/unit/test-crypto-cipher.c
+++ b/tests/unit/test-crypto-cipher.c
@@ -149,6 +149,29 @@ static QCryptoCipherTestData test_data[] = {
"39f23369a9d9bacfa530e26304231461"
"b2eb05e2c39be9fcda6c19078c6a9d1b",
},
+ {
+ /*
+ * Testing 'password' as plaintext fits
+ * in single AES block, and gives identical
+ * ciphertext in ECB and CBC modes
+ */
+ .path = "/crypto/cipher/des-rfb-ecb-56-one-block",
+ .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
+ .mode = QCRYPTO_CIPHER_MODE_ECB,
+ .key = "0123456789abcdef",
+ .plaintext = "70617373776f7264",
+ .ciphertext = "73fa80b66134e403",
+ },
+ {
+ /* See previous comment */
+ .path = "/crypto/cipher/des-rfb-cbc-56-one-block",
+ .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
+ .mode = QCRYPTO_CIPHER_MODE_CBC,
+ .key = "0123456789abcdef",
+ .iv = "0000000000000000",
+ .plaintext = "70617373776f7264",
+ .ciphertext = "73fa80b66134e403",
+ },
{
.path = "/crypto/cipher/des-rfb-ecb-56",
.alg = QCRYPTO_CIPHER_ALG_DES_RFB,
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 08/18] crypto: add crypto tests for single block DES-ECB and DES-CBC
2021-07-06 9:59 ` [PATCH 08/18] crypto: add crypto tests for single block DES-ECB and DES-CBC Daniel P. Berrangé
@ 2021-07-08 18:50 ` Eric Blake
2021-07-09 13:53 ` Daniel P. Berrangé
0 siblings, 1 reply; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:50 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:14AM +0100, Daniel P. Berrangé wrote:
> The GNUTLS crypto provider doesn't support DES-ECB, only DES-CBC.
I had to go research these terms; DES-ECB is weaker (each block
encrypted on its own), DES-CBC is stronger (the encryption of later
blocks depend on the earlier text). Makes sense that GNUTLS has
dropped support for the weaker form.
> We can use the latter to simulate the former, if we encrypt only
> 1 block (8 bytes) of data at a time, using a all-zeros IV. This
using an all-zeros
> is a very inefficient way to use the QCryptoCipher APIs, but
> since the VNC authentication challenge is only 16 bytes, this
> is acceptable. No other part of QEMU should be using DES. This
> test case demonstrates the equivalence of ECB and CBC for the
> single-block case.
Agreed - both on the inefficiency (we're throwing away all the work
spent on chaining the later blocks - thankfully there is only one such
block in our 16-byte challenge), and on the fact that DES should be
avoided where possible (our sole use is due to VNC's less-than-stellar
"security").
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-cipher.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/18] crypto: add crypto tests for single block DES-ECB and DES-CBC
2021-07-08 18:50 ` Eric Blake
@ 2021-07-09 13:53 ` Daniel P. Berrangé
0 siblings, 0 replies; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-09 13:53 UTC (permalink / raw)
To: Eric Blake; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Thu, Jul 08, 2021 at 01:50:54PM -0500, Eric Blake wrote:
> On Tue, Jul 06, 2021 at 10:59:14AM +0100, Daniel P. Berrangé wrote:
> > The GNUTLS crypto provider doesn't support DES-ECB, only DES-CBC.
>
> I had to go research these terms; DES-ECB is weaker (each block
> encrypted on its own), DES-CBC is stronger (the encryption of later
> blocks depend on the earlier text). Makes sense that GNUTLS has
> dropped support for the weaker form.
>
> > We can use the latter to simulate the former, if we encrypt only
> > 1 block (8 bytes) of data at a time, using a all-zeros IV. This
>
> using an all-zeros
>
> > is a very inefficient way to use the QCryptoCipher APIs, but
> > since the VNC authentication challenge is only 16 bytes, this
> > is acceptable. No other part of QEMU should be using DES. This
> > test case demonstrates the equivalence of ECB and CBC for the
> > single-block case.
>
> Agreed - both on the inefficiency (we're throwing away all the work
> spent on chaining the later blocks - thankfully there is only one such
> block in our 16-byte challenge), and on the fact that DES should be
> avoided where possible (our sole use is due to VNC's less-than-stellar
> "security").
Actually there isn't any work wasted chaining blocks, because we're
only writing one block of data.
The inefficiency is because we have to constantly re-create the
cipher context object after every 8 bytes. This massively dominates
over the cipher speed.
>
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > tests/unit/test-crypto-cipher.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 09/18] crypto: delete built-in DES implementation
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (7 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 08/18] crypto: add crypto tests for single block DES-ECB and DES-CBC Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:54 ` Eric Blake
2021-07-06 9:59 ` [PATCH 10/18] crypto: delete built-in XTS cipher mode support Daniel P. Berrangé
` (8 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
The built-in DES implementation is used for the VNC server password
authentication scheme. When building system emulators it is reasonable
to expect that an external crypto library is being used. It is thus
not worth keeping a home grown DES implementation in tree.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/cipher-builtin.c.inc | 72 -------
crypto/desrfb.c | 416 ------------------------------------
crypto/meson.build | 1 -
3 files changed, 489 deletions(-)
delete mode 100644 crypto/desrfb.c
diff --git a/crypto/cipher-builtin.c.inc b/crypto/cipher-builtin.c.inc
index 7597cf4a10..70743f253c 100644
--- a/crypto/cipher-builtin.c.inc
+++ b/crypto/cipher-builtin.c.inc
@@ -19,7 +19,6 @@
*/
#include "crypto/aes.h"
-#include "crypto/desrfb.h"
#include "crypto/xts.h"
typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
@@ -265,69 +264,10 @@ static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_xts = {
};
-typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;
-struct QCryptoCipherBuiltinDESRFB {
- QCryptoCipher base;
-
- /* C.f. alg_key_len[QCRYPTO_CIPHER_ALG_DES_RFB] */
- uint8_t key[8];
-};
-
-static int qcrypto_cipher_encrypt_des_rfb(QCryptoCipher *cipher,
- const void *in, void *out,
- size_t len, Error **errp)
-{
- QCryptoCipherBuiltinDESRFB *ctx
- = container_of(cipher, QCryptoCipherBuiltinDESRFB, base);
- size_t i;
-
- if (!qcrypto_length_check(len, 8, errp)) {
- return -1;
- }
-
- deskey(ctx->key, EN0);
-
- for (i = 0; i < len; i += 8) {
- des((void *)in + i, out + i);
- }
-
- return 0;
-}
-
-static int qcrypto_cipher_decrypt_des_rfb(QCryptoCipher *cipher,
- const void *in, void *out,
- size_t len, Error **errp)
-{
- QCryptoCipherBuiltinDESRFB *ctx
- = container_of(cipher, QCryptoCipherBuiltinDESRFB, base);
- size_t i;
-
- if (!qcrypto_length_check(len, 8, errp)) {
- return -1;
- }
-
- deskey(ctx->key, DE1);
-
- for (i = 0; i < len; i += 8) {
- des((void *)in + i, out + i);
- }
-
- return 0;
-}
-
-static const struct QCryptoCipherDriver qcrypto_cipher_des_rfb_driver = {
- .cipher_encrypt = qcrypto_cipher_encrypt_des_rfb,
- .cipher_decrypt = qcrypto_cipher_decrypt_des_rfb,
- .cipher_setiv = qcrypto_cipher_no_setiv,
- .cipher_free = qcrypto_cipher_ctx_free,
-};
-
bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode)
{
switch (alg) {
- case QCRYPTO_CIPHER_ALG_DES_RFB:
- return mode == QCRYPTO_CIPHER_MODE_ECB;
case QCRYPTO_CIPHER_ALG_AES_128:
case QCRYPTO_CIPHER_ALG_AES_192:
case QCRYPTO_CIPHER_ALG_AES_256:
@@ -356,18 +296,6 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
}
switch (alg) {
- case QCRYPTO_CIPHER_ALG_DES_RFB:
- if (mode == QCRYPTO_CIPHER_MODE_ECB) {
- QCryptoCipherBuiltinDESRFB *ctx;
-
- ctx = g_new0(QCryptoCipherBuiltinDESRFB, 1);
- ctx->base.driver = &qcrypto_cipher_des_rfb_driver;
- memcpy(ctx->key, key, sizeof(ctx->key));
-
- return &ctx->base;
- }
- goto bad_mode;
-
case QCRYPTO_CIPHER_ALG_AES_128:
case QCRYPTO_CIPHER_ALG_AES_192:
case QCRYPTO_CIPHER_ALG_AES_256:
diff --git a/crypto/desrfb.c b/crypto/desrfb.c
deleted file mode 100644
index b2a105ebbc..0000000000
--- a/crypto/desrfb.c
+++ /dev/null
@@ -1,416 +0,0 @@
-/*
- * This is D3DES (V5.09) by Richard Outerbridge with the double and
- * triple-length support removed for use in VNC. Also the bytebit[] array
- * has been reversed so that the most significant bit in each byte of the
- * key is ignored, not the least significant.
- *
- * These changes are:
- * Copyright (C) 1999 AT&T Laboratories Cambridge. All Rights Reserved.
- *
- * This software 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.
- */
-
-/* D3DES (V5.09) -
- *
- * A portable, public domain, version of the Data Encryption Standard.
- *
- * Written with Symantec's THINK (Lightspeed) C by Richard Outerbridge.
- * Thanks to: Dan Hoey for his excellent Initial and Inverse permutation
- * code; Jim Gillogly & Phil Karn for the DES key schedule code; Dennis
- * Ferguson, Eric Young and Dana How for comparing notes; and Ray Lau,
- * for humouring me on.
- *
- * Copyright (c) 1988,1989,1990,1991,1992 by Richard Outerbridge.
- * (GEnie : OUTER; CIS : [71755,204]) Graven Imagery, 1992.
- */
-
-#include "qemu/osdep.h"
-#include "crypto/desrfb.h"
-
-static void scrunch(unsigned char *, unsigned long *);
-static void unscrun(unsigned long *, unsigned char *);
-static void desfunc(unsigned long *, unsigned long *);
-static void cookey(unsigned long *);
-
-static unsigned long KnL[32] = { 0L };
-
-static const unsigned short bytebit[8] = {
- 01, 02, 04, 010, 020, 040, 0100, 0200 };
-
-static const unsigned long bigbyte[24] = {
- 0x800000L, 0x400000L, 0x200000L, 0x100000L,
- 0x80000L, 0x40000L, 0x20000L, 0x10000L,
- 0x8000L, 0x4000L, 0x2000L, 0x1000L,
- 0x800L, 0x400L, 0x200L, 0x100L,
- 0x80L, 0x40L, 0x20L, 0x10L,
- 0x8L, 0x4L, 0x2L, 0x1L };
-
-/* Use the key schedule specified in the Standard (ANSI X3.92-1981). */
-
-static const unsigned char pc1[56] = {
- 56, 48, 40, 32, 24, 16, 8, 0, 57, 49, 41, 33, 25, 17,
- 9, 1, 58, 50, 42, 34, 26, 18, 10, 2, 59, 51, 43, 35,
- 62, 54, 46, 38, 30, 22, 14, 6, 61, 53, 45, 37, 29, 21,
- 13, 5, 60, 52, 44, 36, 28, 20, 12, 4, 27, 19, 11, 3 };
-
-static const unsigned char totrot[16] = {
- 1, 2, 4, 6, 8, 10, 12, 14, 15, 17, 19, 21, 23, 25, 27, 28 };
-
-static const unsigned char pc2[48] = {
- 13, 16, 10, 23, 0, 4, 2, 27, 14, 5, 20, 9,
- 22, 18, 11, 3, 25, 7, 15, 6, 26, 19, 12, 1,
- 40, 51, 30, 36, 46, 54, 29, 39, 50, 44, 32, 47,
- 43, 48, 38, 55, 33, 52, 45, 41, 49, 35, 28, 31 };
-
-/* Thanks to James Gillogly & Phil Karn! */
-void deskey(unsigned char *key, int edf)
-{
- register int i, j, l, m, n;
- unsigned char pc1m[56], pcr[56];
- unsigned long kn[32];
-
- for ( j = 0; j < 56; j++ ) {
- l = pc1[j];
- m = l & 07;
- pc1m[j] = (key[l >> 3] & bytebit[m]) ? 1 : 0;
- }
- for( i = 0; i < 16; i++ ) {
- if( edf == DE1 ) m = (15 - i) << 1;
- else m = i << 1;
- n = m + 1;
- kn[m] = kn[n] = 0L;
- for( j = 0; j < 28; j++ ) {
- l = j + totrot[i];
- if( l < 28 ) pcr[j] = pc1m[l];
- else pcr[j] = pc1m[l - 28];
- }
- for( j = 28; j < 56; j++ ) {
- l = j + totrot[i];
- if( l < 56 ) pcr[j] = pc1m[l];
- else pcr[j] = pc1m[l - 28];
- }
- for( j = 0; j < 24; j++ ) {
- if( pcr[pc2[j]] ) kn[m] |= bigbyte[j];
- if( pcr[pc2[j + 24]] ) kn[n] |= bigbyte[j];
- }
- }
- cookey(kn);
- return;
- }
-
-static void cookey(register unsigned long *raw1)
-{
- register unsigned long *cook, *raw0;
- unsigned long dough[32];
- register int i;
-
- cook = dough;
- for( i = 0; i < 16; i++, raw1++ ) {
- raw0 = raw1++;
- *cook = (*raw0 & 0x00fc0000L) << 6;
- *cook |= (*raw0 & 0x00000fc0L) << 10;
- *cook |= (*raw1 & 0x00fc0000L) >> 10;
- *cook++ |= (*raw1 & 0x00000fc0L) >> 6;
- *cook = (*raw0 & 0x0003f000L) << 12;
- *cook |= (*raw0 & 0x0000003fL) << 16;
- *cook |= (*raw1 & 0x0003f000L) >> 4;
- *cook++ |= (*raw1 & 0x0000003fL);
- }
- usekey(dough);
- return;
- }
-
-void usekey(register unsigned long *from)
-{
- register unsigned long *to, *endp;
-
- to = KnL, endp = &KnL[32];
- while( to < endp ) *to++ = *from++;
- return;
- }
-
-void des(unsigned char *inblock, unsigned char *outblock)
-{
- unsigned long work[2];
-
- scrunch(inblock, work);
- desfunc(work, KnL);
- unscrun(work, outblock);
- return;
- }
-
-static void scrunch(register unsigned char *outof, register unsigned long *into)
-{
- *into = (*outof++ & 0xffL) << 24;
- *into |= (*outof++ & 0xffL) << 16;
- *into |= (*outof++ & 0xffL) << 8;
- *into++ |= (*outof++ & 0xffL);
- *into = (*outof++ & 0xffL) << 24;
- *into |= (*outof++ & 0xffL) << 16;
- *into |= (*outof++ & 0xffL) << 8;
- *into |= (*outof & 0xffL);
- return;
- }
-
-static void unscrun(register unsigned long *outof, register unsigned char *into)
-{
- *into++ = (unsigned char)((*outof >> 24) & 0xffL);
- *into++ = (unsigned char)((*outof >> 16) & 0xffL);
- *into++ = (unsigned char)((*outof >> 8) & 0xffL);
- *into++ = (unsigned char)(*outof++ & 0xffL);
- *into++ = (unsigned char)((*outof >> 24) & 0xffL);
- *into++ = (unsigned char)((*outof >> 16) & 0xffL);
- *into++ = (unsigned char)((*outof >> 8) & 0xffL);
- *into = (unsigned char)(*outof & 0xffL);
- return;
- }
-
-static const unsigned long SP1[64] = {
- 0x01010400L, 0x00000000L, 0x00010000L, 0x01010404L,
- 0x01010004L, 0x00010404L, 0x00000004L, 0x00010000L,
- 0x00000400L, 0x01010400L, 0x01010404L, 0x00000400L,
- 0x01000404L, 0x01010004L, 0x01000000L, 0x00000004L,
- 0x00000404L, 0x01000400L, 0x01000400L, 0x00010400L,
- 0x00010400L, 0x01010000L, 0x01010000L, 0x01000404L,
- 0x00010004L, 0x01000004L, 0x01000004L, 0x00010004L,
- 0x00000000L, 0x00000404L, 0x00010404L, 0x01000000L,
- 0x00010000L, 0x01010404L, 0x00000004L, 0x01010000L,
- 0x01010400L, 0x01000000L, 0x01000000L, 0x00000400L,
- 0x01010004L, 0x00010000L, 0x00010400L, 0x01000004L,
- 0x00000400L, 0x00000004L, 0x01000404L, 0x00010404L,
- 0x01010404L, 0x00010004L, 0x01010000L, 0x01000404L,
- 0x01000004L, 0x00000404L, 0x00010404L, 0x01010400L,
- 0x00000404L, 0x01000400L, 0x01000400L, 0x00000000L,
- 0x00010004L, 0x00010400L, 0x00000000L, 0x01010004L };
-
-static const unsigned long SP2[64] = {
- 0x80108020L, 0x80008000L, 0x00008000L, 0x00108020L,
- 0x00100000L, 0x00000020L, 0x80100020L, 0x80008020L,
- 0x80000020L, 0x80108020L, 0x80108000L, 0x80000000L,
- 0x80008000L, 0x00100000L, 0x00000020L, 0x80100020L,
- 0x00108000L, 0x00100020L, 0x80008020L, 0x00000000L,
- 0x80000000L, 0x00008000L, 0x00108020L, 0x80100000L,
- 0x00100020L, 0x80000020L, 0x00000000L, 0x00108000L,
- 0x00008020L, 0x80108000L, 0x80100000L, 0x00008020L,
- 0x00000000L, 0x00108020L, 0x80100020L, 0x00100000L,
- 0x80008020L, 0x80100000L, 0x80108000L, 0x00008000L,
- 0x80100000L, 0x80008000L, 0x00000020L, 0x80108020L,
- 0x00108020L, 0x00000020L, 0x00008000L, 0x80000000L,
- 0x00008020L, 0x80108000L, 0x00100000L, 0x80000020L,
- 0x00100020L, 0x80008020L, 0x80000020L, 0x00100020L,
- 0x00108000L, 0x00000000L, 0x80008000L, 0x00008020L,
- 0x80000000L, 0x80100020L, 0x80108020L, 0x00108000L };
-
-static const unsigned long SP3[64] = {
- 0x00000208L, 0x08020200L, 0x00000000L, 0x08020008L,
- 0x08000200L, 0x00000000L, 0x00020208L, 0x08000200L,
- 0x00020008L, 0x08000008L, 0x08000008L, 0x00020000L,
- 0x08020208L, 0x00020008L, 0x08020000L, 0x00000208L,
- 0x08000000L, 0x00000008L, 0x08020200L, 0x00000200L,
- 0x00020200L, 0x08020000L, 0x08020008L, 0x00020208L,
- 0x08000208L, 0x00020200L, 0x00020000L, 0x08000208L,
- 0x00000008L, 0x08020208L, 0x00000200L, 0x08000000L,
- 0x08020200L, 0x08000000L, 0x00020008L, 0x00000208L,
- 0x00020000L, 0x08020200L, 0x08000200L, 0x00000000L,
- 0x00000200L, 0x00020008L, 0x08020208L, 0x08000200L,
- 0x08000008L, 0x00000200L, 0x00000000L, 0x08020008L,
- 0x08000208L, 0x00020000L, 0x08000000L, 0x08020208L,
- 0x00000008L, 0x00020208L, 0x00020200L, 0x08000008L,
- 0x08020000L, 0x08000208L, 0x00000208L, 0x08020000L,
- 0x00020208L, 0x00000008L, 0x08020008L, 0x00020200L };
-
-static const unsigned long SP4[64] = {
- 0x00802001L, 0x00002081L, 0x00002081L, 0x00000080L,
- 0x00802080L, 0x00800081L, 0x00800001L, 0x00002001L,
- 0x00000000L, 0x00802000L, 0x00802000L, 0x00802081L,
- 0x00000081L, 0x00000000L, 0x00800080L, 0x00800001L,
- 0x00000001L, 0x00002000L, 0x00800000L, 0x00802001L,
- 0x00000080L, 0x00800000L, 0x00002001L, 0x00002080L,
- 0x00800081L, 0x00000001L, 0x00002080L, 0x00800080L,
- 0x00002000L, 0x00802080L, 0x00802081L, 0x00000081L,
- 0x00800080L, 0x00800001L, 0x00802000L, 0x00802081L,
- 0x00000081L, 0x00000000L, 0x00000000L, 0x00802000L,
- 0x00002080L, 0x00800080L, 0x00800081L, 0x00000001L,
- 0x00802001L, 0x00002081L, 0x00002081L, 0x00000080L,
- 0x00802081L, 0x00000081L, 0x00000001L, 0x00002000L,
- 0x00800001L, 0x00002001L, 0x00802080L, 0x00800081L,
- 0x00002001L, 0x00002080L, 0x00800000L, 0x00802001L,
- 0x00000080L, 0x00800000L, 0x00002000L, 0x00802080L };
-
-static const unsigned long SP5[64] = {
- 0x00000100L, 0x02080100L, 0x02080000L, 0x42000100L,
- 0x00080000L, 0x00000100L, 0x40000000L, 0x02080000L,
- 0x40080100L, 0x00080000L, 0x02000100L, 0x40080100L,
- 0x42000100L, 0x42080000L, 0x00080100L, 0x40000000L,
- 0x02000000L, 0x40080000L, 0x40080000L, 0x00000000L,
- 0x40000100L, 0x42080100L, 0x42080100L, 0x02000100L,
- 0x42080000L, 0x40000100L, 0x00000000L, 0x42000000L,
- 0x02080100L, 0x02000000L, 0x42000000L, 0x00080100L,
- 0x00080000L, 0x42000100L, 0x00000100L, 0x02000000L,
- 0x40000000L, 0x02080000L, 0x42000100L, 0x40080100L,
- 0x02000100L, 0x40000000L, 0x42080000L, 0x02080100L,
- 0x40080100L, 0x00000100L, 0x02000000L, 0x42080000L,
- 0x42080100L, 0x00080100L, 0x42000000L, 0x42080100L,
- 0x02080000L, 0x00000000L, 0x40080000L, 0x42000000L,
- 0x00080100L, 0x02000100L, 0x40000100L, 0x00080000L,
- 0x00000000L, 0x40080000L, 0x02080100L, 0x40000100L };
-
-static const unsigned long SP6[64] = {
- 0x20000010L, 0x20400000L, 0x00004000L, 0x20404010L,
- 0x20400000L, 0x00000010L, 0x20404010L, 0x00400000L,
- 0x20004000L, 0x00404010L, 0x00400000L, 0x20000010L,
- 0x00400010L, 0x20004000L, 0x20000000L, 0x00004010L,
- 0x00000000L, 0x00400010L, 0x20004010L, 0x00004000L,
- 0x00404000L, 0x20004010L, 0x00000010L, 0x20400010L,
- 0x20400010L, 0x00000000L, 0x00404010L, 0x20404000L,
- 0x00004010L, 0x00404000L, 0x20404000L, 0x20000000L,
- 0x20004000L, 0x00000010L, 0x20400010L, 0x00404000L,
- 0x20404010L, 0x00400000L, 0x00004010L, 0x20000010L,
- 0x00400000L, 0x20004000L, 0x20000000L, 0x00004010L,
- 0x20000010L, 0x20404010L, 0x00404000L, 0x20400000L,
- 0x00404010L, 0x20404000L, 0x00000000L, 0x20400010L,
- 0x00000010L, 0x00004000L, 0x20400000L, 0x00404010L,
- 0x00004000L, 0x00400010L, 0x20004010L, 0x00000000L,
- 0x20404000L, 0x20000000L, 0x00400010L, 0x20004010L };
-
-static const unsigned long SP7[64] = {
- 0x00200000L, 0x04200002L, 0x04000802L, 0x00000000L,
- 0x00000800L, 0x04000802L, 0x00200802L, 0x04200800L,
- 0x04200802L, 0x00200000L, 0x00000000L, 0x04000002L,
- 0x00000002L, 0x04000000L, 0x04200002L, 0x00000802L,
- 0x04000800L, 0x00200802L, 0x00200002L, 0x04000800L,
- 0x04000002L, 0x04200000L, 0x04200800L, 0x00200002L,
- 0x04200000L, 0x00000800L, 0x00000802L, 0x04200802L,
- 0x00200800L, 0x00000002L, 0x04000000L, 0x00200800L,
- 0x04000000L, 0x00200800L, 0x00200000L, 0x04000802L,
- 0x04000802L, 0x04200002L, 0x04200002L, 0x00000002L,
- 0x00200002L, 0x04000000L, 0x04000800L, 0x00200000L,
- 0x04200800L, 0x00000802L, 0x00200802L, 0x04200800L,
- 0x00000802L, 0x04000002L, 0x04200802L, 0x04200000L,
- 0x00200800L, 0x00000000L, 0x00000002L, 0x04200802L,
- 0x00000000L, 0x00200802L, 0x04200000L, 0x00000800L,
- 0x04000002L, 0x04000800L, 0x00000800L, 0x00200002L };
-
-static const unsigned long SP8[64] = {
- 0x10001040L, 0x00001000L, 0x00040000L, 0x10041040L,
- 0x10000000L, 0x10001040L, 0x00000040L, 0x10000000L,
- 0x00040040L, 0x10040000L, 0x10041040L, 0x00041000L,
- 0x10041000L, 0x00041040L, 0x00001000L, 0x00000040L,
- 0x10040000L, 0x10000040L, 0x10001000L, 0x00001040L,
- 0x00041000L, 0x00040040L, 0x10040040L, 0x10041000L,
- 0x00001040L, 0x00000000L, 0x00000000L, 0x10040040L,
- 0x10000040L, 0x10001000L, 0x00041040L, 0x00040000L,
- 0x00041040L, 0x00040000L, 0x10041000L, 0x00001000L,
- 0x00000040L, 0x10040040L, 0x00001000L, 0x00041040L,
- 0x10001000L, 0x00000040L, 0x10000040L, 0x10040000L,
- 0x10040040L, 0x10000000L, 0x00040000L, 0x10001040L,
- 0x00000000L, 0x10041040L, 0x00040040L, 0x10000040L,
- 0x10040000L, 0x10001000L, 0x10001040L, 0x00000000L,
- 0x10041040L, 0x00041000L, 0x00041000L, 0x00001040L,
- 0x00001040L, 0x00040040L, 0x10000000L, 0x10041000L };
-
-static void desfunc(register unsigned long *block, register unsigned long *keys)
-{
- register unsigned long fval, work, right, leftt;
- register int round;
-
- leftt = block[0];
- right = block[1];
- work = ((leftt >> 4) ^ right) & 0x0f0f0f0fL;
- right ^= work;
- leftt ^= (work << 4);
- work = ((leftt >> 16) ^ right) & 0x0000ffffL;
- right ^= work;
- leftt ^= (work << 16);
- work = ((right >> 2) ^ leftt) & 0x33333333L;
- leftt ^= work;
- right ^= (work << 2);
- work = ((right >> 8) ^ leftt) & 0x00ff00ffL;
- leftt ^= work;
- right ^= (work << 8);
- right = ((right << 1) | ((right >> 31) & 1L)) & 0xffffffffL;
- work = (leftt ^ right) & 0xaaaaaaaaL;
- leftt ^= work;
- right ^= work;
- leftt = ((leftt << 1) | ((leftt >> 31) & 1L)) & 0xffffffffL;
-
- for( round = 0; round < 8; round++ ) {
- work = (right << 28) | (right >> 4);
- work ^= *keys++;
- fval = SP7[ work & 0x3fL];
- fval |= SP5[(work >> 8) & 0x3fL];
- fval |= SP3[(work >> 16) & 0x3fL];
- fval |= SP1[(work >> 24) & 0x3fL];
- work = right ^ *keys++;
- fval |= SP8[ work & 0x3fL];
- fval |= SP6[(work >> 8) & 0x3fL];
- fval |= SP4[(work >> 16) & 0x3fL];
- fval |= SP2[(work >> 24) & 0x3fL];
- leftt ^= fval;
- work = (leftt << 28) | (leftt >> 4);
- work ^= *keys++;
- fval = SP7[ work & 0x3fL];
- fval |= SP5[(work >> 8) & 0x3fL];
- fval |= SP3[(work >> 16) & 0x3fL];
- fval |= SP1[(work >> 24) & 0x3fL];
- work = leftt ^ *keys++;
- fval |= SP8[ work & 0x3fL];
- fval |= SP6[(work >> 8) & 0x3fL];
- fval |= SP4[(work >> 16) & 0x3fL];
- fval |= SP2[(work >> 24) & 0x3fL];
- right ^= fval;
- }
-
- right = (right << 31) | (right >> 1);
- work = (leftt ^ right) & 0xaaaaaaaaL;
- leftt ^= work;
- right ^= work;
- leftt = (leftt << 31) | (leftt >> 1);
- work = ((leftt >> 8) ^ right) & 0x00ff00ffL;
- right ^= work;
- leftt ^= (work << 8);
- work = ((leftt >> 2) ^ right) & 0x33333333L;
- right ^= work;
- leftt ^= (work << 2);
- work = ((right >> 16) ^ leftt) & 0x0000ffffL;
- leftt ^= work;
- right ^= (work << 16);
- work = ((right >> 4) ^ leftt) & 0x0f0f0f0fL;
- leftt ^= work;
- right ^= (work << 4);
- *block++ = right;
- *block = leftt;
- return;
- }
-
-/* Validation sets:
- *
- * Single-length key, single-length plaintext -
- * Key : 0123 4567 89ab cdef
- * Plain : 0123 4567 89ab cde7
- * Cipher : c957 4425 6a5e d31d
- *
- * Double-length key, single-length plaintext -
- * Key : 0123 4567 89ab cdef fedc ba98 7654 3210
- * Plain : 0123 4567 89ab cde7
- * Cipher : 7f1d 0a77 826b 8aff
- *
- * Double-length key, double-length plaintext -
- * Key : 0123 4567 89ab cdef fedc ba98 7654 3210
- * Plain : 0123 4567 89ab cdef 0123 4567 89ab cdff
- * Cipher : 27a0 8440 406a df60 278f 47cf 42d6 15d7
- *
- * Triple-length key, single-length plaintext -
- * Key : 0123 4567 89ab cdef fedc ba98 7654 3210 89ab cdef 0123 4567
- * Plain : 0123 4567 89ab cde7
- * Cipher : de0b 7c06 ae5e 0ed5
- *
- * Triple-length key, double-length plaintext -
- * Key : 0123 4567 89ab cdef fedc ba98 7654 3210 89ab cdef 0123 4567
- * Plain : 0123 4567 89ab cdef 0123 4567 89ab cdff
- * Cipher : ad0d 1b30 ac17 cf07 0ed1 1c63 81e4 4de5
- *
- * d3des V5.0a rwo 9208.07 18:44 Graven Imagery
- **********************************************************************/
diff --git a/crypto/meson.build b/crypto/meson.build
index 7cbf1a6ba7..b384ca8b57 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -5,7 +5,6 @@ crypto_ss.add(files(
'block-qcow.c',
'block.c',
'cipher.c',
- 'desrfb.c',
'hash.c',
'hmac.c',
'ivgen-essiv.c',
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 09/18] crypto: delete built-in DES implementation
2021-07-06 9:59 ` [PATCH 09/18] crypto: delete built-in DES implementation Daniel P. Berrangé
@ 2021-07-08 18:54 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:54 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:15AM +0100, Daniel P. Berrangé wrote:
> The built-in DES implementation is used for the VNC server password
> authentication scheme. When building system emulators it is reasonable
> to expect that an external crypto library is being used. It is thus
> not worth keeping a home grown DES implementation in tree.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/cipher-builtin.c.inc | 72 -------
> crypto/desrfb.c | 416 ------------------------------------
> crypto/meson.build | 1 -
> 3 files changed, 489 deletions(-)
> delete mode 100644 crypto/desrfb.c
Nice diffstat!
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 10/18] crypto: delete built-in XTS cipher mode support
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (8 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 09/18] crypto: delete built-in DES implementation Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:56 ` Eric Blake
2021-07-06 9:59 ` [PATCH 11/18] crypto: rename des-rfb cipher to just des Daniel P. Berrangé
` (7 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
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(-)
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...
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 10/18] crypto: delete built-in XTS cipher mode support
2021-07-06 9:59 ` [PATCH 10/18] crypto: delete built-in XTS cipher mode support Daniel P. Berrangé
@ 2021-07-08 18:56 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:56 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
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>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 11/18] crypto: rename des-rfb cipher to just des
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (9 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 10/18] crypto: delete built-in XTS cipher mode support Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-07 12:47 ` Markus Armbruster
2021-07-08 19:50 ` Eric Blake
2021-07-06 9:59 ` [PATCH 12/18] crypto: flip priority of backends to prefer gcrypt Daniel P. Berrangé
` (6 subsequent siblings)
17 siblings, 2 replies; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
Currently the crypto layer exposes support for a 'des-rfb'
algorithm which is just normal single-DES, with the bits
in each key byte reversed. This special key munging is
required by the RFB protocol password authentication
mechanism.
Since the crypto layer is generic shared code, it makes
more sense to do the key byte munging in the VNC server
code, and expose normal single-DES support.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/cipher-gcrypt.c.inc | 16 +++-------------
crypto/cipher-nettle.c.inc | 26 +++++++++++---------------
crypto/cipher.c | 28 +++++-----------------------
qapi/crypto.json | 4 ++--
tests/unit/test-crypto-cipher.c | 18 +++++++++---------
ui/vnc.c | 20 +++++++++++++++++---
6 files changed, 47 insertions(+), 65 deletions(-)
diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
index 3aab08a1a9..a6a0117717 100644
--- a/crypto/cipher-gcrypt.c.inc
+++ b/crypto/cipher-gcrypt.c.inc
@@ -24,7 +24,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode)
{
switch (alg) {
- case QCRYPTO_CIPHER_ALG_DES_RFB:
+ case QCRYPTO_CIPHER_ALG_DES:
case QCRYPTO_CIPHER_ALG_3DES:
case QCRYPTO_CIPHER_ALG_AES_128:
case QCRYPTO_CIPHER_ALG_AES_192:
@@ -186,7 +186,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
}
switch (alg) {
- case QCRYPTO_CIPHER_ALG_DES_RFB:
+ case QCRYPTO_CIPHER_ALG_DES:
gcryalg = GCRY_CIPHER_DES;
break;
case QCRYPTO_CIPHER_ALG_3DES:
@@ -257,17 +257,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
}
ctx->blocksize = gcry_cipher_get_algo_blklen(gcryalg);
- if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
- /* We're using standard DES cipher from gcrypt, so we need
- * to munge the key so that the results are the same as the
- * bizarre RFB variant of DES :-)
- */
- uint8_t *rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
- err = gcry_cipher_setkey(ctx->handle, rfbkey, nkey);
- g_free(rfbkey);
- } else {
- err = gcry_cipher_setkey(ctx->handle, key, nkey);
- }
+ err = gcry_cipher_setkey(ctx->handle, key, nkey);
if (err != 0) {
error_setg(errp, "Cannot set key: %s", gcry_strerror(err));
goto error;
diff --git a/crypto/cipher-nettle.c.inc b/crypto/cipher-nettle.c.inc
index fc6f40c026..24cc61f87b 100644
--- a/crypto/cipher-nettle.c.inc
+++ b/crypto/cipher-nettle.c.inc
@@ -235,11 +235,11 @@ static const struct QCryptoCipherDriver NAME##_driver_xts = { \
DEFINE_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)
-typedef struct QCryptoNettleDESRFB {
+typedef struct QCryptoNettleDES {
QCryptoCipher base;
struct des_ctx key;
uint8_t iv[DES_BLOCK_SIZE];
-} QCryptoNettleDESRFB;
+} QCryptoNettleDES;
static void des_encrypt_native(const void *ctx, size_t length,
uint8_t *dst, const uint8_t *src)
@@ -253,7 +253,7 @@ static void des_decrypt_native(const void *ctx, size_t length,
des_decrypt(ctx, length, dst, src);
}
-DEFINE_ECB_CBC_CTR(qcrypto_nettle_des_rfb, QCryptoNettleDESRFB,
+DEFINE_ECB_CBC_CTR(qcrypto_nettle_des, QCryptoNettleDES,
DES_BLOCK_SIZE, des_encrypt_native, des_decrypt_native)
@@ -431,7 +431,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
QCryptoCipherMode mode)
{
switch (alg) {
- case QCRYPTO_CIPHER_ALG_DES_RFB:
+ case QCRYPTO_CIPHER_ALG_DES:
case QCRYPTO_CIPHER_ALG_3DES:
case QCRYPTO_CIPHER_ALG_AES_128:
case QCRYPTO_CIPHER_ALG_AES_192:
@@ -480,32 +480,28 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
}
switch (alg) {
- case QCRYPTO_CIPHER_ALG_DES_RFB:
+ case QCRYPTO_CIPHER_ALG_DES:
{
- QCryptoNettleDESRFB *ctx;
+ QCryptoNettleDES *ctx;
const QCryptoCipherDriver *drv;
- uint8_t *rfbkey;
switch (mode) {
case QCRYPTO_CIPHER_MODE_ECB:
- drv = &qcrypto_nettle_des_rfb_driver_ecb;
+ drv = &qcrypto_nettle_des_driver_ecb;
break;
case QCRYPTO_CIPHER_MODE_CBC:
- drv = &qcrypto_nettle_des_rfb_driver_cbc;
+ drv = &qcrypto_nettle_des_driver_cbc;
break;
case QCRYPTO_CIPHER_MODE_CTR:
- drv = &qcrypto_nettle_des_rfb_driver_ctr;
+ drv = &qcrypto_nettle_des_driver_ctr;
break;
default:
goto bad_cipher_mode;
}
- ctx = g_new0(QCryptoNettleDESRFB, 1);
+ ctx = g_new0(QCryptoNettleDES, 1);
ctx->base.driver = drv;
-
- rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
- des_set_key(&ctx->key, rfbkey);
- g_free(rfbkey);
+ des_set_key(&ctx->key, key);
return &ctx->base;
}
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 068b2fb867..1f5528be49 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -29,7 +29,7 @@ static const size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
[QCRYPTO_CIPHER_ALG_AES_128] = 16,
[QCRYPTO_CIPHER_ALG_AES_192] = 24,
[QCRYPTO_CIPHER_ALG_AES_256] = 32,
- [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
+ [QCRYPTO_CIPHER_ALG_DES] = 8,
[QCRYPTO_CIPHER_ALG_3DES] = 24,
[QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
[QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
@@ -44,7 +44,7 @@ static const size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
[QCRYPTO_CIPHER_ALG_AES_128] = 16,
[QCRYPTO_CIPHER_ALG_AES_192] = 16,
[QCRYPTO_CIPHER_ALG_AES_256] = 16,
- [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
+ [QCRYPTO_CIPHER_ALG_DES] = 8,
[QCRYPTO_CIPHER_ALG_3DES] = 8,
[QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
[QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
@@ -107,9 +107,9 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
}
if (mode == QCRYPTO_CIPHER_MODE_XTS) {
- if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
- || alg == QCRYPTO_CIPHER_ALG_3DES) {
- error_setg(errp, "XTS mode not compatible with DES-RFB/3DES");
+ if (alg == QCRYPTO_CIPHER_ALG_DES ||
+ alg == QCRYPTO_CIPHER_ALG_3DES) {
+ error_setg(errp, "XTS mode not compatible with DES/3DES");
return false;
}
if (nkey % 2) {
@@ -132,24 +132,6 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
return true;
}
-#if defined(CONFIG_GCRYPT) || defined(CONFIG_NETTLE)
-static uint8_t *
-qcrypto_cipher_munge_des_rfb_key(const uint8_t *key,
- size_t nkey)
-{
- uint8_t *ret = g_new0(uint8_t, nkey);
- size_t i;
- for (i = 0; i < nkey; i++) {
- uint8_t r = key[i];
- r = (r & 0xf0) >> 4 | (r & 0x0f) << 4;
- r = (r & 0xcc) >> 2 | (r & 0x33) << 2;
- r = (r & 0xaa) >> 1 | (r & 0x55) << 1;
- ret[i] = r;
- }
- return ret;
-}
-#endif /* CONFIG_GCRYPT || CONFIG_NETTLE */
-
#ifdef CONFIG_GCRYPT
#include "cipher-gcrypt.c.inc"
#elif defined CONFIG_NETTLE
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 7116ae9a46..6b3fadabac 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -66,7 +66,7 @@
# @aes-128: AES with 128 bit / 16 byte keys
# @aes-192: AES with 192 bit / 24 byte keys
# @aes-256: AES with 256 bit / 32 byte keys
-# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
+# @des: DES with 56 bit / 8 byte keys. Do not use except in VNC.
# @3des: 3DES(EDE) with 192 bit / 24 byte keys (since 2.9)
# @cast5-128: Cast5 with 128 bit / 16 byte keys
# @serpent-128: Serpent with 128 bit / 16 byte keys
@@ -80,7 +80,7 @@
{ 'enum': 'QCryptoCipherAlgorithm',
'prefix': 'QCRYPTO_CIPHER_ALG',
'data': ['aes-128', 'aes-192', 'aes-256',
- 'des-rfb', '3des',
+ 'des', '3des',
'cast5-128',
'serpent-128', 'serpent-192', 'serpent-256',
'twofish-128', 'twofish-192', 'twofish-256']}
diff --git a/tests/unit/test-crypto-cipher.c b/tests/unit/test-crypto-cipher.c
index 7dca7b26e4..d9d9d078ff 100644
--- a/tests/unit/test-crypto-cipher.c
+++ b/tests/unit/test-crypto-cipher.c
@@ -155,28 +155,28 @@ static QCryptoCipherTestData test_data[] = {
* in single AES block, and gives identical
* ciphertext in ECB and CBC modes
*/
- .path = "/crypto/cipher/des-rfb-ecb-56-one-block",
- .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
+ .path = "/crypto/cipher/des-ecb-56-one-block",
+ .alg = QCRYPTO_CIPHER_ALG_DES,
.mode = QCRYPTO_CIPHER_MODE_ECB,
- .key = "0123456789abcdef",
+ .key = "80c4a2e691d5b3f7",
.plaintext = "70617373776f7264",
.ciphertext = "73fa80b66134e403",
},
{
/* See previous comment */
- .path = "/crypto/cipher/des-rfb-cbc-56-one-block",
- .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
+ .path = "/crypto/cipher/des-cbc-56-one-block",
+ .alg = QCRYPTO_CIPHER_ALG_DES,
.mode = QCRYPTO_CIPHER_MODE_CBC,
- .key = "0123456789abcdef",
+ .key = "80c4a2e691d5b3f7",
.iv = "0000000000000000",
.plaintext = "70617373776f7264",
.ciphertext = "73fa80b66134e403",
},
{
- .path = "/crypto/cipher/des-rfb-ecb-56",
- .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
+ .path = "/crypto/cipher/des-ecb-56",
+ .alg = QCRYPTO_CIPHER_ALG_DES,
.mode = QCRYPTO_CIPHER_MODE_ECB,
- .key = "0123456789abcdef",
+ .key = "80c4a2e691d5b3f7",
.plaintext =
"6bc1bee22e409f96e93d7e117393172a"
"ae2d8a571e03ac9c9eb76fac45af8e51"
diff --git a/ui/vnc.c b/ui/vnc.c
index 0e5fcb278f..af02522e84 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2733,6 +2733,19 @@ static void authentication_failed(VncState *vs)
vnc_client_error(vs);
}
+static void
+vnc_munge_des_rfb_key(unsigned char *key, size_t nkey)
+{
+ size_t i;
+ for (i = 0; i < nkey; i++) {
+ uint8_t r = key[i];
+ r = (r & 0xf0) >> 4 | (r & 0x0f) << 4;
+ r = (r & 0xcc) >> 2 | (r & 0x33) << 2;
+ r = (r & 0xaa) >> 1 | (r & 0x55) << 1;
+ key[i] = r;
+ }
+}
+
static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
{
unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
@@ -2757,9 +2770,10 @@ static int protocol_client_auth_vnc(VncState *vs, uint8_t *data, size_t len)
pwlen = strlen(vs->vd->password);
for (i=0; i<sizeof(key); i++)
key[i] = i<pwlen ? vs->vd->password[i] : 0;
+ vnc_munge_des_rfb_key(key, sizeof(key));
cipher = qcrypto_cipher_new(
- QCRYPTO_CIPHER_ALG_DES_RFB,
+ QCRYPTO_CIPHER_ALG_DES,
QCRYPTO_CIPHER_MODE_ECB,
key, G_N_ELEMENTS(key),
&err);
@@ -4045,9 +4059,9 @@ void vnc_display_open(const char *id, Error **errp)
goto fail;
}
if (!qcrypto_cipher_supports(
- QCRYPTO_CIPHER_ALG_DES_RFB, QCRYPTO_CIPHER_MODE_ECB)) {
+ QCRYPTO_CIPHER_ALG_DES, QCRYPTO_CIPHER_MODE_ECB)) {
error_setg(errp,
- "Cipher backend does not support DES RFB algorithm");
+ "Cipher backend does not support DES algorithm");
goto fail;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 11/18] crypto: rename des-rfb cipher to just des
2021-07-06 9:59 ` [PATCH 11/18] crypto: rename des-rfb cipher to just des Daniel P. Berrangé
@ 2021-07-07 12:47 ` Markus Armbruster
2021-07-07 13:48 ` Daniel P. Berrangé
2021-07-08 19:50 ` Eric Blake
1 sibling, 1 reply; 43+ messages in thread
From: Markus Armbruster @ 2021-07-07 12:47 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Markus Armbruster, Eric Blake, qemu-devel, Gerd Hoffmann
Daniel P. Berrangé <berrange@redhat.com> writes:
> Currently the crypto layer exposes support for a 'des-rfb'
> algorithm which is just normal single-DES, with the bits
> in each key byte reversed. This special key munging is
> required by the RFB protocol password authentication
> mechanism.
>
> Since the crypto layer is generic shared code, it makes
> more sense to do the key byte munging in the VNC server
> code, and expose normal single-DES support.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
[...]
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 7116ae9a46..6b3fadabac 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -66,7 +66,7 @@
> # @aes-128: AES with 128 bit / 16 byte keys
> # @aes-192: AES with 192 bit / 24 byte keys
> # @aes-256: AES with 256 bit / 32 byte keys
> -# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
> +# @des: DES with 56 bit / 8 byte keys. Do not use except in VNC.
> # @3des: 3DES(EDE) with 192 bit / 24 byte keys (since 2.9)
> # @cast5-128: Cast5 with 128 bit / 16 byte keys
> # @serpent-128: Serpent with 128 bit / 16 byte keys
> @@ -80,7 +80,7 @@
> { 'enum': 'QCryptoCipherAlgorithm',
> 'prefix': 'QCRYPTO_CIPHER_ALG',
> 'data': ['aes-128', 'aes-192', 'aes-256',
> - 'des-rfb', '3des',
> + 'des', '3des',
> 'cast5-128',
> 'serpent-128', 'serpent-192', 'serpent-256',
> 'twofish-128', 'twofish-192', 'twofish-256']}
Is enum value "des-rfb" part of any external interface?
[...]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 11/18] crypto: rename des-rfb cipher to just des
2021-07-07 12:47 ` Markus Armbruster
@ 2021-07-07 13:48 ` Daniel P. Berrangé
2021-07-08 14:41 ` Markus Armbruster
0 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-07 13:48 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Eric Blake, qemu-devel, Gerd Hoffmann
On Wed, Jul 07, 2021 at 02:47:15PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > Currently the crypto layer exposes support for a 'des-rfb'
> > algorithm which is just normal single-DES, with the bits
> > in each key byte reversed. This special key munging is
> > required by the RFB protocol password authentication
> > mechanism.
> >
> > Since the crypto layer is generic shared code, it makes
> > more sense to do the key byte munging in the VNC server
> > code, and expose normal single-DES support.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
>
> [...]
>
> > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > index 7116ae9a46..6b3fadabac 100644
> > --- a/qapi/crypto.json
> > +++ b/qapi/crypto.json
> > @@ -66,7 +66,7 @@
> > # @aes-128: AES with 128 bit / 16 byte keys
> > # @aes-192: AES with 192 bit / 24 byte keys
> > # @aes-256: AES with 256 bit / 32 byte keys
> > -# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
> > +# @des: DES with 56 bit / 8 byte keys. Do not use except in VNC.
> > # @3des: 3DES(EDE) with 192 bit / 24 byte keys (since 2.9)
> > # @cast5-128: Cast5 with 128 bit / 16 byte keys
> > # @serpent-128: Serpent with 128 bit / 16 byte keys
> > @@ -80,7 +80,7 @@
> > { 'enum': 'QCryptoCipherAlgorithm',
> > 'prefix': 'QCRYPTO_CIPHER_ALG',
> > 'data': ['aes-128', 'aes-192', 'aes-256',
> > - 'des-rfb', '3des',
> > + 'des', '3des',
> > 'cast5-128',
> > 'serpent-128', 'serpent-192', 'serpent-256',
> > 'twofish-128', 'twofish-192', 'twofish-256']}
>
> Is enum value "des-rfb" part of any external interface?
Strictly speaking, yes, but in reality it doesn't matter.
The only place in QEMU that actually uses DES-RFB is the
VNC server code. That is an indirect usage when the user
sets the "password" option flag in QemuOpts. The fact that
it uses DES-RFB is an internal impl detail.
The one place that does publically expose ability to set a
field using the QCryptoCipherAlgorithm enum type is the
LUKS support in the block layer:
{ 'struct': 'QCryptoBlockCreateOptionsLUKS',
'base': 'QCryptoBlockOptionsLUKS',
'data': { '*cipher-alg': 'QCryptoCipherAlgorithm',
'*cipher-mode': 'QCryptoCipherMode',
'*ivgen-alg': 'QCryptoIVGenAlgorithm',
'*ivgen-hash-alg': 'QCryptoHashAlgorithm',
'*hash-alg': 'QCryptoHashAlgorithm',
'*iter-time': 'int'}}
eg exposed on CLI as:
$ qemu-img create -f luks -o cipher-alg=NNN foo.luks 1G
or equivalant with QMP blockdev-create
While the QMP schema allows any valid QCryptoCipherAlgorithm
string to be set, the actual implementation does not.
The crypto/block-luks.c code has a map between cipher algs
and LUKS format algoritm names:
static const QCryptoBlockLUKSCipherNameMap
qcrypto_block_luks_cipher_name_map[] = {
{ "aes", qcrypto_block_luks_cipher_size_map_aes },
{ "cast5", qcrypto_block_luks_cipher_size_map_cast5 },
{ "serpent", qcrypto_block_luks_cipher_size_map_serpent },
{ "twofish", qcrypto_block_luks_cipher_size_map_twofish },
};
If it isn't in that table, it can't be used. IOW, the only
scenario we're affecting in this rename is one which would
already result in an error condition
Original behaviour:
$ qemu-img create -f luks --object secret,id=sec0,data=123 -o cipher-alg=des-rfb,key-secret=sec0 demo.luks 1G
Formatting 'demo.luks', fmt=luks size=1073741824 key-secret=sec0 cipher-alg=des-rfb
qemu-img: demo.luks: Algorithm 'des-rfb' not supported
New behaviour:
$ qemu-img create -f luks --object secret,id=sec0,data=123 -o cipher-alg=des-rfb,key-secret=sec0 demo.luks 1G
Formatting 'demo.luks', fmt=luks size=1073741824 key-secret=sec0 cipher-alg=des-fish
qemu-img: demo.luks: Invalid parameter 'des-rfb'
I considered this incompatibility to be acceptable, and thus
not worth going through a deprecation dance.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 11/18] crypto: rename des-rfb cipher to just des
2021-07-07 13:48 ` Daniel P. Berrangé
@ 2021-07-08 14:41 ` Markus Armbruster
2021-07-09 13:59 ` Daniel P. Berrangé
0 siblings, 1 reply; 43+ messages in thread
From: Markus Armbruster @ 2021-07-08 14:41 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Eric Blake, qemu-devel, Gerd Hoffmann
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Wed, Jul 07, 2021 at 02:47:15PM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > Currently the crypto layer exposes support for a 'des-rfb'
>> > algorithm which is just normal single-DES, with the bits
>> > in each key byte reversed. This special key munging is
>> > required by the RFB protocol password authentication
>> > mechanism.
>> >
>> > Since the crypto layer is generic shared code, it makes
>> > more sense to do the key byte munging in the VNC server
>> > code, and expose normal single-DES support.
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>>
>> [...]
>>
>> > diff --git a/qapi/crypto.json b/qapi/crypto.json
>> > index 7116ae9a46..6b3fadabac 100644
>> > --- a/qapi/crypto.json
>> > +++ b/qapi/crypto.json
>> > @@ -66,7 +66,7 @@
>> > # @aes-128: AES with 128 bit / 16 byte keys
>> > # @aes-192: AES with 192 bit / 24 byte keys
>> > # @aes-256: AES with 256 bit / 32 byte keys
>> > -# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
>> > +# @des: DES with 56 bit / 8 byte keys. Do not use except in VNC.
>> > # @3des: 3DES(EDE) with 192 bit / 24 byte keys (since 2.9)
>> > # @cast5-128: Cast5 with 128 bit / 16 byte keys
>> > # @serpent-128: Serpent with 128 bit / 16 byte keys
>> > @@ -80,7 +80,7 @@
>> > { 'enum': 'QCryptoCipherAlgorithm',
>> > 'prefix': 'QCRYPTO_CIPHER_ALG',
>> > 'data': ['aes-128', 'aes-192', 'aes-256',
>> > - 'des-rfb', '3des',
>> > + 'des', '3des',
>> > 'cast5-128',
>> > 'serpent-128', 'serpent-192', 'serpent-256',
>> > 'twofish-128', 'twofish-192', 'twofish-256']}
>>
>> Is enum value "des-rfb" part of any external interface?
>
> Strictly speaking, yes, but in reality it doesn't matter.
>
>
> The only place in QEMU that actually uses DES-RFB is the
> VNC server code. That is an indirect usage when the user
> sets the "password" option flag in QemuOpts. The fact that
> it uses DES-RFB is an internal impl detail.
>
> The one place that does publically expose ability to set a
> field using the QCryptoCipherAlgorithm enum type is the
> LUKS support in the block layer:
>
> { 'struct': 'QCryptoBlockCreateOptionsLUKS',
> 'base': 'QCryptoBlockOptionsLUKS',
> 'data': { '*cipher-alg': 'QCryptoCipherAlgorithm',
> '*cipher-mode': 'QCryptoCipherMode',
> '*ivgen-alg': 'QCryptoIVGenAlgorithm',
> '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> '*hash-alg': 'QCryptoHashAlgorithm',
> '*iter-time': 'int'}}
>
> eg exposed on CLI as:
>
> $ qemu-img create -f luks -o cipher-alg=NNN foo.luks 1G
>
> or equivalant with QMP blockdev-create
>
> While the QMP schema allows any valid QCryptoCipherAlgorithm
> string to be set, the actual implementation does not.
>
> The crypto/block-luks.c code has a map between cipher algs
> and LUKS format algoritm names:
>
>
> static const QCryptoBlockLUKSCipherNameMap
> qcrypto_block_luks_cipher_name_map[] = {
> { "aes", qcrypto_block_luks_cipher_size_map_aes },
> { "cast5", qcrypto_block_luks_cipher_size_map_cast5 },
> { "serpent", qcrypto_block_luks_cipher_size_map_serpent },
> { "twofish", qcrypto_block_luks_cipher_size_map_twofish },
> };
>
> If it isn't in that table, it can't be used. IOW, the only
> scenario we're affecting in this rename is one which would
> already result in an error condition
>
> Original behaviour:
>
> $ qemu-img create -f luks --object secret,id=sec0,data=123 -o cipher-alg=des-rfb,key-secret=sec0 demo.luks 1G
> Formatting 'demo.luks', fmt=luks size=1073741824 key-secret=sec0 cipher-alg=des-rfb
> qemu-img: demo.luks: Algorithm 'des-rfb' not supported
>
> New behaviour:
>
> $ qemu-img create -f luks --object secret,id=sec0,data=123 -o cipher-alg=des-rfb,key-secret=sec0 demo.luks 1G
> Formatting 'demo.luks', fmt=luks size=1073741824 key-secret=sec0 cipher-alg=des-fish
> qemu-img: demo.luks: Invalid parameter 'des-rfb'
>
> I considered this incompatibility to be acceptable, and thus
> not worth going through a deprecation dance.
Thanks for the explanation. I agree the deprecation dance is not
necessary here.
Please consider explaining this in your commit message. Suggest to
append a variation of the tail of your explanation:
Replacing cipher 'des-rfb' by 'des' looks like an incompatible
interface change, but it doesn't matter. While the QMP schema allows
...
qemu-img: demo.luks: Invalid parameter 'des-rfb'
Also consider tweaking the title to
crypto: Replace 'des-rfb' cipher by 'des'
because it's not actually just a rename.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 11/18] crypto: rename des-rfb cipher to just des
2021-07-08 14:41 ` Markus Armbruster
@ 2021-07-09 13:59 ` Daniel P. Berrangé
0 siblings, 0 replies; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-09 13:59 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Eric Blake, qemu-devel, Gerd Hoffmann
On Thu, Jul 08, 2021 at 04:41:28PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Wed, Jul 07, 2021 at 02:47:15PM +0200, Markus Armbruster wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >>
> >> > Currently the crypto layer exposes support for a 'des-rfb'
> >> > algorithm which is just normal single-DES, with the bits
> >> > in each key byte reversed. This special key munging is
> >> > required by the RFB protocol password authentication
> >> > mechanism.
> >> >
> >> > Since the crypto layer is generic shared code, it makes
> >> > more sense to do the key byte munging in the VNC server
> >> > code, and expose normal single-DES support.
> >> >
> >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >> > ---
> >>
> >> [...]
> >>
> >> > diff --git a/qapi/crypto.json b/qapi/crypto.json
> >> > index 7116ae9a46..6b3fadabac 100644
> >> > --- a/qapi/crypto.json
> >> > +++ b/qapi/crypto.json
> >> > @@ -66,7 +66,7 @@
> >> > # @aes-128: AES with 128 bit / 16 byte keys
> >> > # @aes-192: AES with 192 bit / 24 byte keys
> >> > # @aes-256: AES with 256 bit / 32 byte keys
> >> > -# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
> >> > +# @des: DES with 56 bit / 8 byte keys. Do not use except in VNC.
> >> > # @3des: 3DES(EDE) with 192 bit / 24 byte keys (since 2.9)
> >> > # @cast5-128: Cast5 with 128 bit / 16 byte keys
> >> > # @serpent-128: Serpent with 128 bit / 16 byte keys
> >> > @@ -80,7 +80,7 @@
> >> > { 'enum': 'QCryptoCipherAlgorithm',
> >> > 'prefix': 'QCRYPTO_CIPHER_ALG',
> >> > 'data': ['aes-128', 'aes-192', 'aes-256',
> >> > - 'des-rfb', '3des',
> >> > + 'des', '3des',
> >> > 'cast5-128',
> >> > 'serpent-128', 'serpent-192', 'serpent-256',
> >> > 'twofish-128', 'twofish-192', 'twofish-256']}
> >>
> >> Is enum value "des-rfb" part of any external interface?
> >
> > Strictly speaking, yes, but in reality it doesn't matter.
> >
> >
> > The only place in QEMU that actually uses DES-RFB is the
> > VNC server code. That is an indirect usage when the user
> > sets the "password" option flag in QemuOpts. The fact that
> > it uses DES-RFB is an internal impl detail.
> >
> > The one place that does publically expose ability to set a
> > field using the QCryptoCipherAlgorithm enum type is the
> > LUKS support in the block layer:
> >
> > { 'struct': 'QCryptoBlockCreateOptionsLUKS',
> > 'base': 'QCryptoBlockOptionsLUKS',
> > 'data': { '*cipher-alg': 'QCryptoCipherAlgorithm',
> > '*cipher-mode': 'QCryptoCipherMode',
> > '*ivgen-alg': 'QCryptoIVGenAlgorithm',
> > '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> > '*hash-alg': 'QCryptoHashAlgorithm',
> > '*iter-time': 'int'}}
> >
> > eg exposed on CLI as:
> >
> > $ qemu-img create -f luks -o cipher-alg=NNN foo.luks 1G
> >
> > or equivalant with QMP blockdev-create
> >
> > While the QMP schema allows any valid QCryptoCipherAlgorithm
> > string to be set, the actual implementation does not.
> >
> > The crypto/block-luks.c code has a map between cipher algs
> > and LUKS format algoritm names:
> >
> >
> > static const QCryptoBlockLUKSCipherNameMap
> > qcrypto_block_luks_cipher_name_map[] = {
> > { "aes", qcrypto_block_luks_cipher_size_map_aes },
> > { "cast5", qcrypto_block_luks_cipher_size_map_cast5 },
> > { "serpent", qcrypto_block_luks_cipher_size_map_serpent },
> > { "twofish", qcrypto_block_luks_cipher_size_map_twofish },
> > };
> >
> > If it isn't in that table, it can't be used. IOW, the only
> > scenario we're affecting in this rename is one which would
> > already result in an error condition
> >
> > Original behaviour:
> >
> > $ qemu-img create -f luks --object secret,id=sec0,data=123 -o cipher-alg=des-rfb,key-secret=sec0 demo.luks 1G
> > Formatting 'demo.luks', fmt=luks size=1073741824 key-secret=sec0 cipher-alg=des-rfb
> > qemu-img: demo.luks: Algorithm 'des-rfb' not supported
> >
> > New behaviour:
> >
> > $ qemu-img create -f luks --object secret,id=sec0,data=123 -o cipher-alg=des-rfb,key-secret=sec0 demo.luks 1G
> > Formatting 'demo.luks', fmt=luks size=1073741824 key-secret=sec0 cipher-alg=des-fish
> > qemu-img: demo.luks: Invalid parameter 'des-rfb'
> >
> > I considered this incompatibility to be acceptable, and thus
> > not worth going through a deprecation dance.
>
> Thanks for the explanation. I agree the deprecation dance is not
> necessary here.
>
> Please consider explaining this in your commit message. Suggest to
> append a variation of the tail of your explanation:
>
> Replacing cipher 'des-rfb' by 'des' looks like an incompatible
> interface change, but it doesn't matter. While the QMP schema allows
> ...
> qemu-img: demo.luks: Invalid parameter 'des-rfb'
>
> Also consider tweaking the title to
>
> crypto: Replace 'des-rfb' cipher by 'des'
>
> because it's not actually just a rename.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks, I've updated the commit msg as suggested
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 11/18] crypto: rename des-rfb cipher to just des
2021-07-06 9:59 ` [PATCH 11/18] crypto: rename des-rfb cipher to just des Daniel P. Berrangé
2021-07-07 12:47 ` Markus Armbruster
@ 2021-07-08 19:50 ` Eric Blake
1 sibling, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:50 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:17AM +0100, Daniel P. Berrangé wrote:
> Currently the crypto layer exposes support for a 'des-rfb'
> algorithm which is just normal single-DES, with the bits
> in each key byte reversed. This special key munging is
> required by the RFB protocol password authentication
> mechanism.
>
> Since the crypto layer is generic shared code, it makes
> more sense to do the key byte munging in the VNC server
> code, and expose normal single-DES support.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
I agree with Markus' suggestion to enhance the commit message.
> +++ b/tests/unit/test-crypto-cipher.c
> @@ -155,28 +155,28 @@ static QCryptoCipherTestData test_data[] = {
> * in single AES block, and gives identical
> * ciphertext in ECB and CBC modes
> */
> - .path = "/crypto/cipher/des-rfb-ecb-56-one-block",
> - .alg = QCRYPTO_CIPHER_ALG_DES_RFB,
> + .path = "/crypto/cipher/des-ecb-56-one-block",
> + .alg = QCRYPTO_CIPHER_ALG_DES,
> .mode = QCRYPTO_CIPHER_MODE_ECB,
> - .key = "0123456789abcdef",
> + .key = "80c4a2e691d5b3f7",
> .plaintext = "70617373776f7264",
> .ciphertext = "73fa80b66134e403",
> },
This is a rather cute way to avoid recomputing the canonical
.ciphertext due to the change in bit ordering.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 12/18] crypto: flip priority of backends to prefer gcrypt
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (10 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 11/18] crypto: rename des-rfb cipher to just des Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 18:59 ` Eric Blake
2021-07-06 9:59 ` [PATCH 13/18] crypto: introduce build system for gnutls crypto backend Daniel P. Berrangé
` (5 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
Originally we preferred to use nettle, over gcrypt because
gnutls already links to nettle and thus it minimizes the
dependencies. In retrospect this was the wrong criteria to
optimize for.
Currently shipping versions of gcrypt have cipher impls that
are massively faster than those in nettle and this is way
more important. The nettle library is also not capable of
enforcing FIPS compliance, since it considers that out of
scope. It merely aims to provide general purpose impls of
algorithms, and usage policy is left upto the layer above,
such as GNUTTLS.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/meson.build b/meson.build
index 07b4e7f950..51b8f4ab75 100644
--- a/meson.build
+++ b/meson.build
@@ -818,22 +818,13 @@ if not get_option('gnutls').auto() or have_system
kwargs: static_kwargs)
endif
-# Nettle has priority over gcrypt
+# Gcrypt has priority over nettle
gcrypt = not_found
nettle = not_found
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()
- nettle = dependency('nettle', version: '>=3.4',
- method: 'pkg-config',
- required: get_option('nettle'),
- kwargs: static_kwargs)
- 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()
+elif (not get_option('gcrypt').auto() or have_system) and not get_option('nettle').enabled()
gcrypt = dependency('libgcrypt', version: '>=1.8',
method: 'config-tool',
required: get_option('gcrypt'),
@@ -847,6 +838,15 @@ if (not get_option('gcrypt').auto() or have_system) and not nettle.found()
cc.find_library('gpg-error', required: true, kwargs: static_kwargs)])
endif
endif
+if (not get_option('nettle').auto() or have_system) and not gcrypt.found()
+ nettle = dependency('nettle', version: '>=3.4',
+ method: 'pkg-config',
+ required: get_option('nettle'),
+ kwargs: static_kwargs)
+ if nettle.found() and not cc.has_header('nettle/xts.h', dependencies: nettle)
+ xts = 'private'
+ endif
+endif
gtk = not_found
gtkx11 = not_found
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 12/18] crypto: flip priority of backends to prefer gcrypt
2021-07-06 9:59 ` [PATCH 12/18] crypto: flip priority of backends to prefer gcrypt Daniel P. Berrangé
@ 2021-07-08 18:59 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 18:59 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:18AM +0100, Daniel P. Berrangé wrote:
> Originally we preferred to use nettle, over gcrypt because
s/nettle, over gcrypt/nettle over gcrypt,/
> gnutls already links to nettle and thus it minimizes the
> dependencies. In retrospect this was the wrong criteria to
> optimize for.
>
> Currently shipping versions of gcrypt have cipher impls that
> are massively faster than those in nettle and this is way
> more important. The nettle library is also not capable of
> enforcing FIPS compliance, since it considers that out of
> scope. It merely aims to provide general purpose impls of
> algorithms, and usage policy is left upto the layer above,
> such as GNUTTLS.
GNUTLS
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> meson.build | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
My meson skills are weak, but the change looks reasonable, and the
proof is in building things.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 13/18] crypto: introduce build system for gnutls crypto backend
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (11 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 12/18] crypto: flip priority of backends to prefer gcrypt Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 19:03 ` Eric Blake
2021-07-06 9:59 ` [PATCH 14/18] crypto: add gnutls cipher provider Daniel P. Berrangé
` (4 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
This introduces the build logic needed to decide whether we can
use gnutls as a crypto driver backend. The actual implementations
will be introduced in following patches. We only wish to use
gnutls if it has version 3.6.14 or newer, because that is what
finally brings HW accelerated AES-XTS mode for x86_64.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 36 ++++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build
index 51b8f4ab75..6031f4f0b1 100644
--- a/meson.build
+++ b/meson.build
@@ -811,11 +811,34 @@ if 'CONFIG_OPENGL' in config_host
endif
gnutls = not_found
+gnutls_crypto = not_found
if not get_option('gnutls').auto() or have_system
- gnutls = dependency('gnutls', version: '>=3.5.18',
- method: 'pkg-config',
- required: get_option('gnutls'),
- kwargs: static_kwargs)
+ # For general TLS support our min gnutls matches
+ # that implied by our platform support matrix
+ #
+ # For the crypto backends, we look for a newer
+ # gnutls:
+ #
+ # Version 3.6.8 is needed to get XTS
+ # Version 3.6.13 is needed to get PBKDF
+ # Version 3.6.14 is needed to get HW accelerated XTS
+ #
+ # If newer enough gnutls isn't available, we can
+ # still use a different crypto backend to satisfy
+ # the platform support requirements
+ gnutls_crypto = dependency('gnutls', version: '>=3.6.14',
+ method: 'pkg-config',
+ required: get_option('gnutls'),
+ kwargs: static_kwargs)
+ if gnutls_crypto.found()
+ gnutls = gnutls_crypto
+ else
+ # Our min version if all we need is TLS
+ gnutls = dependency('gnutls', version: '>=3.5.18',
+ method: 'pkg-config',
+ required: get_option('gnutls'),
+ kwargs: static_kwargs)
+ endif
endif
# Gcrypt has priority over nettle
@@ -847,6 +870,9 @@ if (not get_option('nettle').auto() or have_system) and not gcrypt.found()
xts = 'private'
endif
endif
+if gcrypt.found() or nettle.found()
+ gnutls_crypto = not_found
+endif
gtk = not_found
gtkx11 = not_found
@@ -1219,6 +1245,7 @@ config_host_data.set('CONFIG_XKBCOMMON', xkbcommon.found())
config_host_data.set('CONFIG_KEYUTILS', keyutils.found())
config_host_data.set('CONFIG_GETTID', has_gettid)
config_host_data.set('CONFIG_GNUTLS', gnutls.found())
+config_host_data.set('CONFIG_GNUTLS_CRYPTO', gnutls_crypto.found())
config_host_data.set('CONFIG_GCRYPT', gcrypt.found())
config_host_data.set('CONFIG_NETTLE', nettle.found())
config_host_data.set('CONFIG_QEMU_PRIVATE_XTS', xts == 'private')
@@ -2719,6 +2746,7 @@ summary(summary_info, bool_yn: true, section: 'Block layer support')
summary_info = {}
summary_info += {'TLS priority': config_host['CONFIG_TLS_PRIORITY']}
summary_info += {'GNUTLS support': gnutls.found()}
+summary_info += {'GNUTLS crypto': gnutls_crypto.found()}
# TODO: add back version
summary_info += {'libgcrypt': gcrypt.found()}
# TODO: add back version
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 13/18] crypto: introduce build system for gnutls crypto backend
2021-07-06 9:59 ` [PATCH 13/18] crypto: introduce build system for gnutls crypto backend Daniel P. Berrangé
@ 2021-07-08 19:03 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:03 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:19AM +0100, Daniel P. Berrangé wrote:
> This introduces the build logic needed to decide whether we can
> use gnutls as a crypto driver backend. The actual implementations
> will be introduced in following patches. We only wish to use
> gnutls if it has version 3.6.14 or newer, because that is what
> finally brings HW accelerated AES-XTS mode for x86_64.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> meson.build | 36 ++++++++++++++++++++++++++++++++----
> 1 file changed, 32 insertions(+), 4 deletions(-)
Again, take this with a grain of salt, since my meson skills are near
zero. But the comments do a good job, and it looks sane.
> diff --git a/meson.build b/meson.build
> index 51b8f4ab75..6031f4f0b1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -811,11 +811,34 @@ if 'CONFIG_OPENGL' in config_host
> endif
>
> gnutls = not_found
> +gnutls_crypto = not_found
> if not get_option('gnutls').auto() or have_system
> - gnutls = dependency('gnutls', version: '>=3.5.18',
> - method: 'pkg-config',
> - required: get_option('gnutls'),
> - kwargs: static_kwargs)
> + # For general TLS support our min gnutls matches
> + # that implied by our platform support matrix
> + #
> + # For the crypto backends, we look for a newer
> + # gnutls:
> + #
> + # Version 3.6.8 is needed to get XTS
> + # Version 3.6.13 is needed to get PBKDF
> + # Version 3.6.14 is needed to get HW accelerated XTS
> + #
> + # If newer enough gnutls isn't available, we can
> + # still use a different crypto backend to satisfy
> + # the platform support requirements
> + gnutls_crypto = dependency('gnutls', version: '>=3.6.14',
> + method: 'pkg-config',
> + required: get_option('gnutls'),
> + kwargs: static_kwargs)
> + if gnutls_crypto.found()
> + gnutls = gnutls_crypto
> + else
> + # Our min version if all we need is TLS
> + gnutls = dependency('gnutls', version: '>=3.5.18',
> + method: 'pkg-config',
> + required: get_option('gnutls'),
> + kwargs: static_kwargs)
TAB damage.
With that fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 14/18] crypto: add gnutls cipher provider
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (12 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 13/18] crypto: introduce build system for gnutls crypto backend Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 19:13 ` Eric Blake
2021-07-06 9:59 ` [PATCH 15/18] crypto: add gnutls hash provider Daniel P. Berrangé
` (3 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
Add an implementation of the QEMU cipher APIs to the gnutls
crypto backend. XTS support is only available for gnutls
version >= 3.6.8. Since ECB mode is not exposed by gnutls
APIs, we can't use the private XTS code for compatibility.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/cipher-gnutls.c.inc | 325 +++++++++++++++++++++++++++++++++++++
crypto/cipher.c | 2 +
2 files changed, 327 insertions(+)
create mode 100644 crypto/cipher-gnutls.c.inc
diff --git a/crypto/cipher-gnutls.c.inc b/crypto/cipher-gnutls.c.inc
new file mode 100644
index 0000000000..eb6eb49546
--- /dev/null
+++ b/crypto/cipher-gnutls.c.inc
@@ -0,0 +1,325 @@
+/*
+ * QEMU Crypto cipher gnutls algorithms
+ *
+ * Copyright (c) 2021 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 "qemu/osdep.h"
+#include "cipherpriv.h"
+
+#include <gnutls/crypto.h>
+
+#if GNUTLS_VERSION_NUMBER >= 0x030608
+#define QEMU_GNUTLS_XTS
+#endif
+
+bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode)
+{
+
+ switch (mode) {
+ case QCRYPTO_CIPHER_MODE_ECB:
+ case QCRYPTO_CIPHER_MODE_CBC:
+ switch (alg) {
+ case QCRYPTO_CIPHER_ALG_AES_128:
+ case QCRYPTO_CIPHER_ALG_AES_192:
+ case QCRYPTO_CIPHER_ALG_AES_256:
+ case QCRYPTO_CIPHER_ALG_DES:
+ case QCRYPTO_CIPHER_ALG_3DES:
+ return true;
+ default:
+ return false;
+ }
+#ifdef QEMU_GNUTLS_XTS
+ case QCRYPTO_CIPHER_MODE_XTS:
+ switch (alg) {
+ case QCRYPTO_CIPHER_ALG_AES_128:
+ case QCRYPTO_CIPHER_ALG_AES_256:
+ return true;
+ default:
+ return false;
+ }
+ return true;
+#endif
+ default:
+ return false;
+ }
+}
+
+typedef struct QCryptoCipherGnutls QCryptoCipherGnutls;
+struct QCryptoCipherGnutls {
+ QCryptoCipher base;
+ gnutls_cipher_hd_t handle; /* XTS & CBC mode */
+ gnutls_cipher_algorithm_t galg; /* ECB mode */
+ guint8 *key; /* ECB mode */
+ size_t nkey; /* ECB mode */
+ size_t blocksize;
+};
+
+
+static void
+qcrypto_gnutls_cipher_free(QCryptoCipher *cipher)
+{
+ QCryptoCipherGnutls *ctx = container_of(cipher, QCryptoCipherGnutls, base);
+
+ g_free(ctx->key);
+ if (ctx->handle) {
+ gnutls_cipher_deinit(ctx->handle);
+ }
+ g_free(ctx);
+}
+
+
+static int
+qcrypto_gnutls_cipher_encrypt(QCryptoCipher *cipher,
+ const void *in,
+ void *out,
+ size_t len,
+ Error **errp)
+{
+ QCryptoCipherGnutls *ctx = container_of(cipher, QCryptoCipherGnutls, base);
+ int err;
+
+ if (len % ctx->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctx->blocksize);
+ return -1;
+ }
+
+ if (ctx->handle) { /* CBC / XTS mode */
+ err = gnutls_cipher_encrypt2(ctx->handle,
+ in, len,
+ out, len);
+ if (err != 0) {
+ error_setg(errp, "Cannot encrypt data: %s",
+ gnutls_strerror(err));
+ return -1;
+ }
+ } else { /* ECB mode very inefficiently faked with CBC */
+ g_autofree unsigned char *iv = g_new0(unsigned char, ctx->blocksize);
+ while (len) {
+ gnutls_cipher_hd_t handle;
+ gnutls_datum_t gkey = { (unsigned char *)ctx->key, ctx->nkey };
+ int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
+ if (err != 0) {
+ error_setg(errp, "Cannot initialize cipher: %s",
+ gnutls_strerror(err));
+ return -1;
+ }
+
+ gnutls_cipher_set_iv(handle, iv, ctx->blocksize);
+
+ err = gnutls_cipher_encrypt2(handle,
+ in, ctx->blocksize,
+ out, ctx->blocksize);
+ if (err != 0) {
+ gnutls_cipher_deinit(handle);
+ error_setg(errp, "Cannot encrypt data: %s",
+ gnutls_strerror(err));
+ return -1;
+ }
+ gnutls_cipher_deinit(handle);
+
+ len -= ctx->blocksize;
+ in += ctx->blocksize;
+ out += ctx->blocksize;
+ }
+ }
+
+ return 0;
+}
+
+
+static int
+qcrypto_gnutls_cipher_decrypt(QCryptoCipher *cipher,
+ const void *in,
+ void *out,
+ size_t len,
+ Error **errp)
+{
+ QCryptoCipherGnutls *ctx = container_of(cipher, QCryptoCipherGnutls, base);
+ int err;
+
+ if (len % ctx->blocksize) {
+ error_setg(errp, "Length %zu must be a multiple of block size %zu",
+ len, ctx->blocksize);
+ return -1;
+ }
+
+ if (ctx->handle) { /* CBC / XTS mode */
+ err = gnutls_cipher_decrypt2(ctx->handle,
+ in, len,
+ out, len);
+
+ if (err != 0) {
+ error_setg(errp, "Cannot decrypt data: %s",
+ gnutls_strerror(err));
+ return -1;
+ }
+ } else { /* ECB mode very inefficiently faked with CBC */
+ g_autofree unsigned char *iv = g_new0(unsigned char, ctx->blocksize);
+ while (len) {
+ gnutls_cipher_hd_t handle;
+ gnutls_datum_t gkey = { (unsigned char *)ctx->key, ctx->nkey };
+ int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
+ if (err != 0) {
+ error_setg(errp, "Cannot initialize cipher: %s",
+ gnutls_strerror(err));
+ return -1;
+ }
+
+ gnutls_cipher_set_iv(handle, iv, ctx->blocksize);
+
+ err = gnutls_cipher_decrypt2(handle,
+ in, ctx->blocksize,
+ out, ctx->blocksize);
+ if (err != 0) {
+ gnutls_cipher_deinit(handle);
+ error_setg(errp, "Cannot encrypt data: %s",
+ gnutls_strerror(err));
+ return -1;
+ }
+ gnutls_cipher_deinit(handle);
+
+ len -= ctx->blocksize;
+ in += ctx->blocksize;
+ out += ctx->blocksize;
+ }
+ }
+
+ return 0;
+}
+
+static int
+qcrypto_gnutls_cipher_setiv(QCryptoCipher *cipher,
+ const uint8_t *iv, size_t niv,
+ Error **errp)
+{
+ QCryptoCipherGnutls *ctx = container_of(cipher, QCryptoCipherGnutls, base);
+
+ if (niv != ctx->blocksize) {
+ error_setg(errp, "Expected IV size %zu not %zu",
+ ctx->blocksize, niv);
+ return -1;
+ }
+
+ gnutls_cipher_set_iv(ctx->handle, (unsigned char *)iv, niv);
+
+ return 0;
+}
+
+
+static struct QCryptoCipherDriver gnutls_driver = {
+ .cipher_encrypt = qcrypto_gnutls_cipher_encrypt,
+ .cipher_decrypt = qcrypto_gnutls_cipher_decrypt,
+ .cipher_setiv = qcrypto_gnutls_cipher_setiv,
+ .cipher_free = qcrypto_gnutls_cipher_free,
+};
+
+static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode,
+ const uint8_t *key,
+ size_t nkey,
+ Error **errp)
+{
+ QCryptoCipherGnutls *ctx;
+ gnutls_datum_t gkey = { (unsigned char *)key, nkey };
+ gnutls_cipher_algorithm_t galg = GNUTLS_CIPHER_UNKNOWN;
+ int err;
+
+ switch (mode) {
+#ifdef QEMU_GNUTLS_XTS
+ case QCRYPTO_CIPHER_MODE_XTS:
+ switch (alg) {
+ case QCRYPTO_CIPHER_ALG_AES_128:
+ galg = GNUTLS_CIPHER_AES_128_XTS;
+ break;
+ case QCRYPTO_CIPHER_ALG_AES_256:
+ galg = GNUTLS_CIPHER_AES_256_XTS;
+ break;
+ default:
+ break;
+ }
+ break;
+#endif
+
+ case QCRYPTO_CIPHER_MODE_ECB:
+ case QCRYPTO_CIPHER_MODE_CBC:
+ switch (alg) {
+ case QCRYPTO_CIPHER_ALG_AES_128:
+ galg = GNUTLS_CIPHER_AES_128_CBC;
+ break;
+ case QCRYPTO_CIPHER_ALG_AES_192:
+ galg = GNUTLS_CIPHER_AES_192_CBC;
+ break;
+ case QCRYPTO_CIPHER_ALG_AES_256:
+ galg = GNUTLS_CIPHER_AES_256_CBC;
+ break;
+ case QCRYPTO_CIPHER_ALG_DES:
+ galg = GNUTLS_CIPHER_DES_CBC;
+ break;
+ case QCRYPTO_CIPHER_ALG_3DES:
+ galg = GNUTLS_CIPHER_3DES_CBC;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+
+ if (galg == GNUTLS_CIPHER_UNKNOWN) {
+ error_setg(errp, "Unsupported cipher algorithm %s with %s mode",
+ QCryptoCipherAlgorithm_str(alg),
+ QCryptoCipherMode_str(mode));
+ return NULL;
+ }
+
+ if (!qcrypto_cipher_validate_key_length(alg, mode, nkey, errp)) {
+ return NULL;
+ }
+
+ ctx = g_new0(QCryptoCipherGnutls, 1);
+ ctx->base.driver = &gnutls_driver;
+
+ if (mode == QCRYPTO_CIPHER_MODE_ECB) {
+ ctx->key = g_new0(guint8, nkey);
+ memcpy(ctx->key, key, nkey);
+ ctx->nkey = nkey;
+ ctx->galg = galg;
+ } else {
+ err = gnutls_cipher_init(&ctx->handle, galg, &gkey, NULL);
+ if (err != 0) {
+ error_setg(errp, "Cannot initialize cipher: %s",
+ gnutls_strerror(err));
+ goto error;
+ }
+ }
+
+ if (alg == QCRYPTO_CIPHER_ALG_DES ||
+ alg == QCRYPTO_CIPHER_ALG_3DES)
+ ctx->blocksize = 8;
+ else
+ ctx->blocksize = 16;
+
+ return &ctx->base;
+
+ error:
+ qcrypto_gnutls_cipher_free(&ctx->base);
+ return NULL;
+}
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 1f5528be49..74b09a5b26 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -136,6 +136,8 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
#include "cipher-gcrypt.c.inc"
#elif defined CONFIG_NETTLE
#include "cipher-nettle.c.inc"
+#elif defined CONFIG_GNUTLS_CRYPTO
+#include "cipher-gnutls.c.inc"
#else
#include "cipher-builtin.c.inc"
#endif
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 14/18] crypto: add gnutls cipher provider
2021-07-06 9:59 ` [PATCH 14/18] crypto: add gnutls cipher provider Daniel P. Berrangé
@ 2021-07-08 19:13 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:13 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:20AM +0100, Daniel P. Berrangé wrote:
> Add an implementation of the QEMU cipher APIs to the gnutls
> crypto backend. XTS support is only available for gnutls
> version >= 3.6.8. Since ECB mode is not exposed by gnutls
> APIs, we can't use the private XTS code for compatibility.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/cipher-gnutls.c.inc | 325 +++++++++++++++++++++++++++++++++++++
> crypto/cipher.c | 2 +
> 2 files changed, 327 insertions(+)
> create mode 100644 crypto/cipher-gnutls.c.inc
>
> diff --git a/crypto/cipher-gnutls.c.inc b/crypto/cipher-gnutls.c.inc
> new file mode 100644
> index 0000000000..eb6eb49546
> --- /dev/null
> +++ b/crypto/cipher-gnutls.c.inc
> +
> +bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
> + QCryptoCipherMode mode)
> +{
> +
> + switch (mode) {
> + case QCRYPTO_CIPHER_MODE_ECB:
> + case QCRYPTO_CIPHER_MODE_CBC:
> + switch (alg) {
> + case QCRYPTO_CIPHER_ALG_AES_128:
> + case QCRYPTO_CIPHER_ALG_AES_192:
> + case QCRYPTO_CIPHER_ALG_AES_256:
> + case QCRYPTO_CIPHER_ALG_DES:
> + case QCRYPTO_CIPHER_ALG_3DES:
> + return true;
> + default:
> + return false;
> + }
> +#ifdef QEMU_GNUTLS_XTS
> + case QCRYPTO_CIPHER_MODE_XTS:
> + switch (alg) {
> + case QCRYPTO_CIPHER_ALG_AES_128:
> + case QCRYPTO_CIPHER_ALG_AES_256:
> + return true;
> + default:
> + return false;
> + }
> + return true;
This line is dead code.
> +#endif
> + default:
> + return false;
> + }
> +}
> +
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 15/18] crypto: add gnutls hash provider
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (13 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 14/18] crypto: add gnutls cipher provider Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 19:29 ` Eric Blake
2021-07-06 9:59 ` [PATCH 16/18] crypto: add gnutls hmac provider Daniel P. Berrangé
` (2 subsequent siblings)
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
This adds support for using gnutls as a provider of the crypto
hash APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/hash-gnutls.c | 104 +++++++++++++++++++++++++++++++++++++++++++
crypto/meson.build | 2 +
2 files changed, 106 insertions(+)
create mode 100644 crypto/hash-gnutls.c
diff --git a/crypto/hash-gnutls.c b/crypto/hash-gnutls.c
new file mode 100644
index 0000000000..f88db71f00
--- /dev/null
+++ b/crypto/hash-gnutls.c
@@ -0,0 +1,104 @@
+/*
+ * QEMU Crypto hash algorithms
+ *
+ * Copyright (c) 2016 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 "qemu/osdep.h"
+#include <gnutls/crypto.h>
+#include "qapi/error.h"
+#include "crypto/hash.h"
+#include "hashpriv.h"
+
+
+static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG__MAX] = {
+ [QCRYPTO_HASH_ALG_MD5] = GNUTLS_DIG_MD5,
+ [QCRYPTO_HASH_ALG_SHA1] = GNUTLS_DIG_SHA1,
+ [QCRYPTO_HASH_ALG_SHA224] = GNUTLS_DIG_SHA224,
+ [QCRYPTO_HASH_ALG_SHA256] = GNUTLS_DIG_SHA256,
+ [QCRYPTO_HASH_ALG_SHA384] = GNUTLS_DIG_SHA384,
+ [QCRYPTO_HASH_ALG_SHA512] = GNUTLS_DIG_SHA512,
+ [QCRYPTO_HASH_ALG_RIPEMD160] = GNUTLS_DIG_RMD160,
+};
+
+gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
+{
+ size_t i;
+ const gnutls_digest_algorithm_t *algs;
+ if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_map) ||
+ qcrypto_hash_alg_map[alg] == GNUTLS_DIG_UNKNOWN) {
+ return false;
+ }
+ algs = gnutls_digest_list();
+ for (i = 0; algs[i] != GNUTLS_DIG_UNKNOWN; i++) {
+ if (algs[i] == qcrypto_hash_alg_map[alg]) {
+ return true;
+ }
+ }
+ return false;
+}
+
+
+static int
+qcrypto_gnutls_hash_bytesv(QCryptoHashAlgorithm alg,
+ const struct iovec *iov,
+ size_t niov,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+ int i, ret;
+ gnutls_hash_hd_t hash;
+
+ if (!qcrypto_hash_supports(alg)) {
+ error_setg(errp,
+ "Unknown hash algorithm %d",
+ alg);
+ return -1;
+ }
+
+ ret = gnutls_hash_get_len(qcrypto_hash_alg_map[alg]);
+ if (*resultlen == 0) {
+ *resultlen = ret;
+ *result = g_new0(uint8_t, *resultlen);
+ } else if (*resultlen != ret) {
+ error_setg(errp,
+ "Result buffer size %zu is smaller than hash %d",
+ *resultlen, ret);
+ return -1;
+ }
+
+ ret = gnutls_hash_init(&hash, qcrypto_hash_alg_map[alg]);
+ if (ret < 0) {
+ error_setg(errp,
+ "Unable to initialize hash algorithm: %s",
+ gnutls_strerror(ret));
+ return -1;
+ }
+
+ for (i = 0; i < niov; i++) {
+ gnutls_hash(hash, iov[i].iov_base, iov[i].iov_len);
+ }
+
+ gnutls_hash_deinit(hash, *result);
+ return 0;
+}
+
+
+QCryptoHashDriver qcrypto_hash_lib_driver = {
+ .hash_bytesv = qcrypto_gnutls_hash_bytesv,
+};
diff --git a/crypto/meson.build b/crypto/meson.build
index fc8de287e1..d6df83f2ab 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -28,6 +28,8 @@ if nettle.found()
endif
elif gcrypt.found()
crypto_ss.add(gcrypt, files('hash-gcrypt.c', 'hmac-gcrypt.c', 'pbkdf-gcrypt.c'))
+elif gnutls_crypto.found()
+ crypto_ss.add(gnutls, files('hash-gnutls.c', 'hmac-glib.c', 'pbkdf-stub.c')
else
crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
endif
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 15/18] crypto: add gnutls hash provider
2021-07-06 9:59 ` [PATCH 15/18] crypto: add gnutls hash provider Daniel P. Berrangé
@ 2021-07-08 19:29 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:29 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:21AM +0100, Daniel P. Berrangé wrote:
> This adds support for using gnutls as a provider of the crypto
> hash APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/hash-gnutls.c | 104 +++++++++++++++++++++++++++++++++++++++++++
> crypto/meson.build | 2 +
> 2 files changed, 106 insertions(+)
> create mode 100644 crypto/hash-gnutls.c
>
> diff --git a/crypto/hash-gnutls.c b/crypto/hash-gnutls.c
> new file mode 100644
> index 0000000000..f88db71f00
> --- /dev/null
> +++ b/crypto/hash-gnutls.c
> @@ -0,0 +1,104 @@
> +/*
> + * QEMU Crypto hash algorithms
> + *
> + * Copyright (c) 2016 Red Hat, Inc.
Want to add 2021 here?
> +static int
> +qcrypto_gnutls_hash_bytesv(QCryptoHashAlgorithm alg,
> + const struct iovec *iov,
> + size_t niov,
> + uint8_t **result,
> + size_t *resultlen,
> + Error **errp)
> +{
> +
> + ret = gnutls_hash_init(&hash, qcrypto_hash_alg_map[alg]);
> + if (ret < 0) {
> + error_setg(errp,
> + "Unable to initialize hash algorithm: %s",
> + gnutls_strerror(ret));
> + return -1;
> + }
> +
> + for (i = 0; i < niov; i++) {
> + gnutls_hash(hash, iov[i].iov_base, iov[i].iov_len);
> + }
> +
> + gnutls_hash_deinit(hash, *result);
Is there any speed penalty for creating a new gnutls_hash object for
each call to qcrypto_gnutls_hash_bytesv(), compared to creating a hash
object just once and using gnutls_hash_output() to grab results and
reset it for reuse? But that can be a later patch (if it even works -
I may have mis-read gnutls docs), so it doesn't affect review of this
patch.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 16/18] crypto: add gnutls hmac provider
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (14 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 15/18] crypto: add gnutls hash provider Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 19:35 ` Eric Blake
2021-07-06 9:59 ` [PATCH 17/18] crypto: add gnutls pbkdf provider Daniel P. Berrangé
2021-07-06 9:59 ` [PATCH 18/18] crypto: prefer gnutls as the crypto backend if new enough Daniel P. Berrangé
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
This adds support for using gnutls as a provider of the crypto
hmac APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/hmac-gnutls.c | 136 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 136 insertions(+)
create mode 100644 crypto/hmac-gnutls.c
diff --git a/crypto/hmac-gnutls.c b/crypto/hmac-gnutls.c
new file mode 100644
index 0000000000..ea33b5753b
--- /dev/null
+++ b/crypto/hmac-gnutls.c
@@ -0,0 +1,136 @@
+/*
+ * QEMU Crypto hmac algorithms
+ *
+ * Derived from hmac-gcrypt.c:
+ *
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version. See the COPYING file in the
+ * top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "crypto/hmac.h"
+#include "hmacpriv.h"
+#include <gnutls/crypto.h>
+
+static int qcrypto_hmac_alg_map[QCRYPTO_HASH_ALG__MAX] = {
+ [QCRYPTO_HASH_ALG_MD5] = GNUTLS_MAC_MD5,
+ [QCRYPTO_HASH_ALG_SHA1] = GNUTLS_MAC_SHA1,
+ [QCRYPTO_HASH_ALG_SHA224] = GNUTLS_MAC_SHA224,
+ [QCRYPTO_HASH_ALG_SHA256] = GNUTLS_MAC_SHA256,
+ [QCRYPTO_HASH_ALG_SHA384] = GNUTLS_MAC_SHA384,
+ [QCRYPTO_HASH_ALG_SHA512] = GNUTLS_MAC_SHA512,
+ [QCRYPTO_HASH_ALG_RIPEMD160] = GNUTLS_MAC_RMD160,
+};
+
+typedef struct QCryptoHmacGnutls QCryptoHmacGnutls;
+struct QCryptoHmacGnutls {
+ gnutls_hmac_hd_t handle;
+};
+
+bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
+{
+ size_t i;
+ const gnutls_digest_algorithm_t *algs;
+ if (alg >= G_N_ELEMENTS(qcrypto_hmac_alg_map) ||
+ qcrypto_hmac_alg_map[alg] == GNUTLS_DIG_UNKNOWN) {
+ return false;
+ }
+ algs = gnutls_digest_list();
+ for (i = 0; algs[i] != GNUTLS_DIG_UNKNOWN; i++) {
+ if (algs[i] == qcrypto_hmac_alg_map[alg]) {
+ return true;
+ }
+ }
+ return false;
+}
+
+void *qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
+ const uint8_t *key, size_t nkey,
+ Error **errp)
+{
+ QCryptoHmacGnutls *ctx;
+ int err;
+
+ if (!qcrypto_hmac_supports(alg)) {
+ error_setg(errp, "Unsupported hmac algorithm %s",
+ QCryptoHashAlgorithm_str(alg));
+ return NULL;
+ }
+
+ ctx = g_new0(QCryptoHmacGnutls, 1);
+
+ err = gnutls_hmac_init(&ctx->handle,
+ qcrypto_hmac_alg_map[alg],
+ (const void *)key, nkey);
+ if (err != 0) {
+ error_setg(errp, "Cannot initialize hmac: %s",
+ gnutls_strerror(err));
+ goto error;
+ }
+
+ return ctx;
+
+error:
+ g_free(ctx);
+ return NULL;
+}
+
+static void
+qcrypto_gnutls_hmac_ctx_free(QCryptoHmac *hmac)
+{
+ QCryptoHmacGnutls *ctx;
+
+ ctx = hmac->opaque;
+ gnutls_hmac_deinit(ctx->handle, NULL);
+
+ g_free(ctx);
+}
+
+static int
+qcrypto_gnutls_hmac_bytesv(QCryptoHmac *hmac,
+ const struct iovec *iov,
+ size_t niov,
+ uint8_t **result,
+ size_t *resultlen,
+ Error **errp)
+{
+ QCryptoHmacGnutls *ctx;
+ uint32_t ret;
+ int i;
+
+ ctx = hmac->opaque;
+
+ for (i = 0; i < niov; i++) {
+ gnutls_hmac(ctx->handle, iov[i].iov_base, iov[i].iov_len);
+ }
+
+ ret = gnutls_hmac_get_len(qcrypto_hmac_alg_map[hmac->alg]);
+ if (ret <= 0) {
+ error_setg(errp, "Unable to get hmac length: %s",
+ gnutls_strerror(ret));
+ return -1;
+ }
+
+ if (*resultlen == 0) {
+ *resultlen = ret;
+ *result = g_new0(uint8_t, *resultlen);
+ } else if (*resultlen != ret) {
+ error_setg(errp, "Result buffer size %zu is smaller than hmac %d",
+ *resultlen, ret);
+ return -1;
+ }
+
+ gnutls_hmac_output(ctx->handle, *result);
+
+ return 0;
+}
+
+QCryptoHmacDriver qcrypto_hmac_lib_driver = {
+ .hmac_bytesv = qcrypto_gnutls_hmac_bytesv,
+ .hmac_free = qcrypto_gnutls_hmac_ctx_free,
+};
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 16/18] crypto: add gnutls hmac provider
2021-07-06 9:59 ` [PATCH 16/18] crypto: add gnutls hmac provider Daniel P. Berrangé
@ 2021-07-08 19:35 ` Eric Blake
2021-07-09 14:03 ` Daniel P. Berrangé
0 siblings, 1 reply; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:35 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:22AM +0100, Daniel P. Berrangé wrote:
> This adds support for using gnutls as a provider of the crypto
> hmac APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/hmac-gnutls.c | 136 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 136 insertions(+)
> create mode 100644 crypto/hmac-gnutls.c
>
> diff --git a/crypto/hmac-gnutls.c b/crypto/hmac-gnutls.c
> new file mode 100644
> index 0000000000..ea33b5753b
> --- /dev/null
> +++ b/crypto/hmac-gnutls.c
> @@ -0,0 +1,136 @@
> +/*
> + * QEMU Crypto hmac algorithms
> + *
> + * Derived from hmac-gcrypt.c:
> + *
> + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
Is this line correct?
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version. See the COPYING file in the
> + * top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "crypto/hmac.h"
> +#include "hmacpriv.h"
> +#include <gnutls/crypto.h>
Should you favor floating this to be right after "qemu/osdep.h"?
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 16/18] crypto: add gnutls hmac provider
2021-07-08 19:35 ` Eric Blake
@ 2021-07-09 14:03 ` Daniel P. Berrangé
0 siblings, 0 replies; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-09 14:03 UTC (permalink / raw)
To: Eric Blake; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Thu, Jul 08, 2021 at 02:35:19PM -0500, Eric Blake wrote:
> On Tue, Jul 06, 2021 at 10:59:22AM +0100, Daniel P. Berrangé wrote:
> > This adds support for using gnutls as a provider of the crypto
> > hmac APIs.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > crypto/hmac-gnutls.c | 136 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 136 insertions(+)
> > create mode 100644 crypto/hmac-gnutls.c
> >
> > diff --git a/crypto/hmac-gnutls.c b/crypto/hmac-gnutls.c
> > new file mode 100644
> > index 0000000000..ea33b5753b
> > --- /dev/null
> > +++ b/crypto/hmac-gnutls.c
> > @@ -0,0 +1,136 @@
> > +/*
> > + * QEMU Crypto hmac algorithms
> > + *
> > + * Derived from hmac-gcrypt.c:
> > + *
> > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
>
> Is this line correct?
This is referring to the statement above "Derived from..."
I should have /also/ added a Red Hat copyright before though
>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * (at your option) any later version. See the COPYING file in the
> > + * top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "crypto/hmac.h"
> > +#include "hmacpriv.h"
> > +#include <gnutls/crypto.h>
>
> Should you favor floating this to be right after "qemu/osdep.h"?
yeah good idea
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 17/18] crypto: add gnutls pbkdf provider
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (15 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 16/18] crypto: add gnutls hmac provider Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 19:43 ` Eric Blake
2021-07-06 9:59 ` [PATCH 18/18] crypto: prefer gnutls as the crypto backend if new enough Daniel P. Berrangé
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
This adds support for using gnutls as a provider of the crypto
pbkdf APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/meson.build | 2 +-
crypto/pbkdf-gnutls.c | 90 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 crypto/pbkdf-gnutls.c
diff --git a/crypto/meson.build b/crypto/meson.build
index d6df83f2ab..5a1464bc69 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -29,7 +29,7 @@ if nettle.found()
elif gcrypt.found()
crypto_ss.add(gcrypt, files('hash-gcrypt.c', 'hmac-gcrypt.c', 'pbkdf-gcrypt.c'))
elif gnutls_crypto.found()
- crypto_ss.add(gnutls, files('hash-gnutls.c', 'hmac-glib.c', 'pbkdf-stub.c')
+ crypto_ss.add(gnutls, files('hash-gnutls.c', 'hmac-gnutls.c', 'pbkdf-gnutls.c'))
else
crypto_ss.add(files('hash-glib.c', 'hmac-glib.c', 'pbkdf-stub.c'))
endif
diff --git a/crypto/pbkdf-gnutls.c b/crypto/pbkdf-gnutls.c
new file mode 100644
index 0000000000..8462596a90
--- /dev/null
+++ b/crypto/pbkdf-gnutls.c
@@ -0,0 +1,90 @@
+/*
+ * QEMU Crypto PBKDF support (Password-Based Key Derivation Function)
+ *
+ * Copyright (c) 2015-2016 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 "qemu/osdep.h"
+#include <gnutls/crypto.h>
+#include "qapi/error.h"
+#include "crypto/pbkdf.h"
+
+bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
+{
+ switch (hash) {
+ case QCRYPTO_HASH_ALG_MD5:
+ case QCRYPTO_HASH_ALG_SHA1:
+ case QCRYPTO_HASH_ALG_SHA224:
+ case QCRYPTO_HASH_ALG_SHA256:
+ case QCRYPTO_HASH_ALG_SHA384:
+ case QCRYPTO_HASH_ALG_SHA512:
+ case QCRYPTO_HASH_ALG_RIPEMD160:
+ return true;
+ default:
+ return false;
+ }
+}
+
+int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
+ const uint8_t *key, size_t nkey,
+ const uint8_t *salt, size_t nsalt,
+ uint64_t iterations,
+ uint8_t *out, size_t nout,
+ Error **errp)
+{
+ static const int hash_map[QCRYPTO_HASH_ALG__MAX] = {
+ [QCRYPTO_HASH_ALG_MD5] = GNUTLS_DIG_MD5,
+ [QCRYPTO_HASH_ALG_SHA1] = GNUTLS_DIG_SHA1,
+ [QCRYPTO_HASH_ALG_SHA224] = GNUTLS_DIG_SHA224,
+ [QCRYPTO_HASH_ALG_SHA256] = GNUTLS_DIG_SHA256,
+ [QCRYPTO_HASH_ALG_SHA384] = GNUTLS_DIG_SHA384,
+ [QCRYPTO_HASH_ALG_SHA512] = GNUTLS_DIG_SHA512,
+ [QCRYPTO_HASH_ALG_RIPEMD160] = GNUTLS_DIG_RMD160,
+ };
+ int ret;
+ const gnutls_datum_t gkey = { (unsigned char *)key, nkey };
+ const gnutls_datum_t gsalt = { (unsigned char *)salt, nsalt };
+
+ if (iterations > ULONG_MAX) {
+ error_setg_errno(errp, ERANGE,
+ "PBKDF iterations %llu must be less than %lu",
+ (long long unsigned)iterations, ULONG_MAX);
+ return -1;
+ }
+
+ if (hash >= G_N_ELEMENTS(hash_map) ||
+ hash_map[hash] == GNUTLS_DIG_UNKNOWN) {
+ error_setg_errno(errp, ENOSYS,
+ "PBKDF does not support hash algorithm %s",
+ QCryptoHashAlgorithm_str(hash));
+ return -1;
+ }
+
+ ret = gnutls_pbkdf2(hash_map[hash],
+ &gkey,
+ &gsalt,
+ iterations,
+ out,
+ nout);
+ if (ret != 0) {
+ error_setg(errp, "Cannot derive password: %s",
+ gnutls_strerror(ret));
+ return -1;
+ }
+
+ return 0;
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 17/18] crypto: add gnutls pbkdf provider
2021-07-06 9:59 ` [PATCH 17/18] crypto: add gnutls pbkdf provider Daniel P. Berrangé
@ 2021-07-08 19:43 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:43 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:23AM +0100, Daniel P. Berrangé wrote:
> This adds support for using gnutls as a provider of the crypto
> pbkdf APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/meson.build | 2 +-
> crypto/pbkdf-gnutls.c | 90 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 91 insertions(+), 1 deletion(-)
> create mode 100644 crypto/pbkdf-gnutls.c
>
> +++ b/crypto/pbkdf-gnutls.c
> @@ -0,0 +1,90 @@
> +/*
> + * QEMU Crypto PBKDF support (Password-Based Key Derivation Function)
> + *
> + * Copyright (c) 2015-2016 Red Hat, Inc.
Want to add 2021?
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 18/18] crypto: prefer gnutls as the crypto backend if new enough
2021-07-06 9:59 [PATCH 00/18] crypto: misc cleanup and introduce gnutls backend driver Daniel P. Berrangé
` (16 preceding siblings ...)
2021-07-06 9:59 ` [PATCH 17/18] crypto: add gnutls pbkdf provider Daniel P. Berrangé
@ 2021-07-06 9:59 ` Daniel P. Berrangé
2021-07-08 19:52 ` Eric Blake
17 siblings, 1 reply; 43+ messages in thread
From: Daniel P. Berrangé @ 2021-07-06 9:59 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Daniel P. Berrangé, Gerd Hoffmann, Markus Armbruster
If we have gnutls >= 3.6.13, then it has enough functionality
and performance that we can use it as the preferred crypto
backend.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 59 +++++++++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/meson.build b/meson.build
index 6031f4f0b1..0bec6f7e40 100644
--- a/meson.build
+++ b/meson.build
@@ -841,39 +841,50 @@ if not get_option('gnutls').auto() or have_system
endif
endif
-# Gcrypt has priority over nettle
+# We prefer use of gnutls for crypto, unless the options
+# explicitly asked for nettle or gcrypt.
+#
+# If gnutls isn't available for crypto, then we'll prefer
+# gcrypt over nettle for performance reasons.
gcrypt = not_found
nettle = not_found
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('gcrypt').auto() or have_system) and not get_option('nettle').enabled()
- gcrypt = dependency('libgcrypt', version: '>=1.8',
- method: 'config-tool',
- required: get_option('gcrypt'),
- kwargs: static_kwargs)
- # Debian has removed -lgpg-error from libgcrypt-config
- # as it "spreads unnecessary dependencies" which in
- # turn breaks static builds...
- if gcrypt.found() and enable_static
- gcrypt = declare_dependency(dependencies: [
- gcrypt,
- cc.find_library('gpg-error', required: true, kwargs: static_kwargs)])
- endif
-endif
-if (not get_option('nettle').auto() or have_system) and not gcrypt.found()
- nettle = dependency('nettle', version: '>=3.4',
- method: 'pkg-config',
- required: get_option('nettle'),
- kwargs: static_kwargs)
- if nettle.found() and not cc.has_header('nettle/xts.h', dependencies: nettle)
- xts = 'private'
- endif
endif
-if gcrypt.found() or nettle.found()
+
+# Explicit nettle/gcrypt request, so ignore gnutls for crypto
+if get_option('nettle').enabled() or get_option('gcrypt').enabled()
gnutls_crypto = not_found
endif
+if not gnutls_crypto.found()
+ if (not get_option('gcrypt').auto() or have_system) and not get_option('nettle').enabled()
+ gcrypt = dependency('libgcrypt', version: '>=1.8',
+ method: 'config-tool',
+ required: get_option('gcrypt'),
+ kwargs: static_kwargs)
+ # Debian has removed -lgpg-error from libgcrypt-config
+ # as it "spreads unnecessary dependencies" which in
+ # turn breaks static builds...
+ if gcrypt.found() and enable_static
+ gcrypt = declare_dependency(dependencies: [
+ gcrypt,
+ cc.find_library('gpg-error', required: true, kwargs: static_kwargs)])
+ endif
+ endif
+ if (not get_option('nettle').auto() or have_system) and not gcrypt.found()
+ nettle = dependency('nettle', version: '>=3.4',
+ method: 'pkg-config',
+ required: get_option('nettle'),
+ kwargs: static_kwargs)
+ if nettle.found() and not cc.has_header('nettle/xts.h', dependencies: nettle)
+ xts = 'private'
+ endif
+ endif
+endif
+
gtk = not_found
gtkx11 = not_found
if not get_option('gtk').auto() or (have_system and not cocoa.found())
--
2.31.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 18/18] crypto: prefer gnutls as the crypto backend if new enough
2021-07-06 9:59 ` [PATCH 18/18] crypto: prefer gnutls as the crypto backend if new enough Daniel P. Berrangé
@ 2021-07-08 19:52 ` Eric Blake
0 siblings, 0 replies; 43+ messages in thread
From: Eric Blake @ 2021-07-08 19:52 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Markus Armbruster, qemu-devel, Gerd Hoffmann
On Tue, Jul 06, 2021 at 10:59:24AM +0100, Daniel P. Berrangé wrote:
> If we have gnutls >= 3.6.13, then it has enough functionality
> and performance that we can use it as the preferred crypto
> backend.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> meson.build | 59 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 24 deletions(-)
Once again, take my meson review with a grain of salt.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 43+ messages in thread