* [PATCH 0/2] crypto: misc pbkdf fixes for testing & algorithm compat
@ 2024-08-30 11:05 Daniel P. Berrangé
2024-08-30 11:05 ` [PATCH 1/2] crypto: check gnutls & gcrypt support the requested pbkdf hash Daniel P. Berrangé
2024-08-30 11:05 ` [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test Daniel P. Berrangé
0 siblings, 2 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2024-08-30 11:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
Daniel P. Berrangé (2):
crypto: check gnutls & gcrypt support the requested pbkdf hash
tests/unit: always build the pbkdf crypto unit test
crypto/pbkdf-gcrypt.c | 2 +-
crypto/pbkdf-gnutls.c | 2 +-
tests/unit/meson.build | 4 +---
tests/unit/test-crypto-pbkdf.c | 9 ++++++---
4 files changed, 9 insertions(+), 8 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] crypto: check gnutls & gcrypt support the requested pbkdf hash
2024-08-30 11:05 [PATCH 0/2] crypto: misc pbkdf fixes for testing & algorithm compat Daniel P. Berrangé
@ 2024-08-30 11:05 ` Daniel P. Berrangé
2024-09-02 19:43 ` Philippe Mathieu-Daudé
2024-08-30 11:05 ` [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test Daniel P. Berrangé
1 sibling, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2024-08-30 11:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
Both gnutls and gcrypt can be configured to exclude support for certain
algorithms via a runtime check against system crypto policies. Thus it
is not sufficient to have a compile time test for hash support in their
pbkdf implementations.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/pbkdf-gcrypt.c | 2 +-
crypto/pbkdf-gnutls.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
index a8d8e64f4d..bc0719c831 100644
--- a/crypto/pbkdf-gcrypt.c
+++ b/crypto/pbkdf-gcrypt.c
@@ -33,7 +33,7 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
case QCRYPTO_HASH_ALG_SHA384:
case QCRYPTO_HASH_ALG_SHA512:
case QCRYPTO_HASH_ALG_RIPEMD160:
- return true;
+ return qcrypto_hash_supports(hash);
default:
return false;
}
diff --git a/crypto/pbkdf-gnutls.c b/crypto/pbkdf-gnutls.c
index 2dfbbd382c..911b565bea 100644
--- a/crypto/pbkdf-gnutls.c
+++ b/crypto/pbkdf-gnutls.c
@@ -33,7 +33,7 @@ bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
case QCRYPTO_HASH_ALG_SHA384:
case QCRYPTO_HASH_ALG_SHA512:
case QCRYPTO_HASH_ALG_RIPEMD160:
- return true;
+ return qcrypto_hash_supports(hash);
default:
return false;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test
2024-08-30 11:05 [PATCH 0/2] crypto: misc pbkdf fixes for testing & algorithm compat Daniel P. Berrangé
2024-08-30 11:05 ` [PATCH 1/2] crypto: check gnutls & gcrypt support the requested pbkdf hash Daniel P. Berrangé
@ 2024-08-30 11:05 ` Daniel P. Berrangé
2024-09-02 19:45 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2024-08-30 11:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
The meson rules were excluding the pbkdf crypto test when gnutls was the
crypto backend. It was then excluded again in #if statements in the test
file.
Rather than update these conditions, remove them all, and use the result
of the qcrypto_pbkdf_supports() function to determine whether to skip
test registration.
Also add CONFIG_DARWIN to the remaining condition, since we have a way
to measure CPU time on this platform since commit bf98afc75efedf1.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/meson.build | 4 +---
tests/unit/test-crypto-pbkdf.c | 9 ++++++---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 490ab8182d..972d792883 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -121,9 +121,7 @@ if have_block
if config_host_data.get('CONFIG_REPLICATION')
tests += {'test-replication': [testblock]}
endif
- if nettle.found() or gcrypt.found()
- tests += {'test-crypto-pbkdf': [io]}
- endif
+ tests += {'test-crypto-pbkdf': [io]}
endif
if have_system
diff --git a/tests/unit/test-crypto-pbkdf.c b/tests/unit/test-crypto-pbkdf.c
index 43c417f6b4..034bb02422 100644
--- a/tests/unit/test-crypto-pbkdf.c
+++ b/tests/unit/test-crypto-pbkdf.c
@@ -25,8 +25,7 @@
#include <sys/resource.h>
#endif
-#if ((defined(CONFIG_NETTLE) || defined(CONFIG_GCRYPT)) && \
- (defined(_WIN32) || defined(RUSAGE_THREAD)))
+#if defined(_WIN32) || defined(RUSAGE_THREAD) || defined(CONFIG_DARWIN)
#include "crypto/pbkdf.h"
typedef struct QCryptoPbkdfTestData QCryptoPbkdfTestData;
@@ -422,13 +421,17 @@ int main(int argc, char **argv)
g_assert(qcrypto_init(NULL) == 0);
for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
+ if (!qcrypto_pbkdf2_supports(test_data[i].hash)) {
+ continue;
+ }
+
if (!test_data[i].slow ||
g_test_slow()) {
g_test_add_data_func(test_data[i].path, &test_data[i], test_pbkdf);
}
}
- if (g_test_slow()) {
+ if (g_test_slow() && qcrypto_pbkdf2_supports(QCRYPTO_HASH_ALG_SHA256)) {
g_test_add_func("/crypt0/pbkdf/timing", test_pbkdf_timing);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] crypto: check gnutls & gcrypt support the requested pbkdf hash
2024-08-30 11:05 ` [PATCH 1/2] crypto: check gnutls & gcrypt support the requested pbkdf hash Daniel P. Berrangé
@ 2024-09-02 19:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-02 19:43 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
On 30/8/24 13:05, Daniel P. Berrangé wrote:
> Both gnutls and gcrypt can be configured to exclude support for certain
> algorithms via a runtime check against system crypto policies. Thus it
> is not sufficient to have a compile time test for hash support in their
> pbkdf implementations.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/pbkdf-gcrypt.c | 2 +-
> crypto/pbkdf-gnutls.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test
2024-08-30 11:05 ` [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test Daniel P. Berrangé
@ 2024-09-02 19:45 ` Philippe Mathieu-Daudé
2024-09-04 9:00 ` Daniel P. Berrangé
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-09-02 19:45 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
On 30/8/24 13:05, Daniel P. Berrangé wrote:
> The meson rules were excluding the pbkdf crypto test when gnutls was the
> crypto backend. It was then excluded again in #if statements in the test
> file.
>
> Rather than update these conditions, remove them all, and use the result
> of the qcrypto_pbkdf_supports() function to determine whether to skip
> test registration.
>
> Also add CONFIG_DARWIN to the remaining condition, since we have a way
> to measure CPU time on this platform since commit bf98afc75efedf1.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/meson.build | 4 +---
> tests/unit/test-crypto-pbkdf.c | 9 ++++++---
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 490ab8182d..972d792883 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -121,9 +121,7 @@ if have_block
> if config_host_data.get('CONFIG_REPLICATION')
> tests += {'test-replication': [testblock]}
> endif
> - if nettle.found() or gcrypt.found()
> - tests += {'test-crypto-pbkdf': [io]}
> - endif
> + tests += {'test-crypto-pbkdf': [io]}
> endif
>
> if have_system
> diff --git a/tests/unit/test-crypto-pbkdf.c b/tests/unit/test-crypto-pbkdf.c
> index 43c417f6b4..034bb02422 100644
> --- a/tests/unit/test-crypto-pbkdf.c
> +++ b/tests/unit/test-crypto-pbkdf.c
> @@ -25,8 +25,7 @@
> #include <sys/resource.h>
> #endif
>
> -#if ((defined(CONFIG_NETTLE) || defined(CONFIG_GCRYPT)) && \
> - (defined(_WIN32) || defined(RUSAGE_THREAD)))
> +#if defined(_WIN32) || defined(RUSAGE_THREAD) || defined(CONFIG_DARWIN)
Add CONFIG_DARWIN in a subsequent commit?
> #include "crypto/pbkdf.h"
>
> typedef struct QCryptoPbkdfTestData QCryptoPbkdfTestData;
> @@ -422,13 +421,17 @@ int main(int argc, char **argv)
> g_assert(qcrypto_init(NULL) == 0);
>
> for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
> + if (!qcrypto_pbkdf2_supports(test_data[i].hash)) {
> + continue;
> + }
> +
> if (!test_data[i].slow ||
> g_test_slow()) {
> g_test_add_data_func(test_data[i].path, &test_data[i], test_pbkdf);
> }
> }
>
> - if (g_test_slow()) {
> + if (g_test_slow() && qcrypto_pbkdf2_supports(QCRYPTO_HASH_ALG_SHA256)) {
> g_test_add_func("/crypt0/pbkdf/timing", test_pbkdf_timing);
While here, rename test_pbkdf_timing -> test_pbkdf_sha256_timing?
> }
>
Anyway,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test
2024-09-02 19:45 ` Philippe Mathieu-Daudé
@ 2024-09-04 9:00 ` Daniel P. Berrangé
0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2024-09-04 9:00 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel
On Mon, Sep 02, 2024 at 09:45:04PM +0200, Philippe Mathieu-Daudé wrote:
> On 30/8/24 13:05, Daniel P. Berrangé wrote:
> > The meson rules were excluding the pbkdf crypto test when gnutls was the
> > crypto backend. It was then excluded again in #if statements in the test
> > file.
> >
> > Rather than update these conditions, remove them all, and use the result
> > of the qcrypto_pbkdf_supports() function to determine whether to skip
> > test registration.
> >
> > Also add CONFIG_DARWIN to the remaining condition, since we have a way
> > to measure CPU time on this platform since commit bf98afc75efedf1.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > tests/unit/meson.build | 4 +---
> > tests/unit/test-crypto-pbkdf.c | 9 ++++++---
> > 2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> > index 490ab8182d..972d792883 100644
> > --- a/tests/unit/meson.build
> > +++ b/tests/unit/meson.build
> > @@ -121,9 +121,7 @@ if have_block
> > if config_host_data.get('CONFIG_REPLICATION')
> > tests += {'test-replication': [testblock]}
> > endif
> > - if nettle.found() or gcrypt.found()
> > - tests += {'test-crypto-pbkdf': [io]}
> > - endif
> > + tests += {'test-crypto-pbkdf': [io]}
> > endif
> > if have_system
> > diff --git a/tests/unit/test-crypto-pbkdf.c b/tests/unit/test-crypto-pbkdf.c
> > index 43c417f6b4..034bb02422 100644
> > --- a/tests/unit/test-crypto-pbkdf.c
> > +++ b/tests/unit/test-crypto-pbkdf.c
> > @@ -25,8 +25,7 @@
> > #include <sys/resource.h>
> > #endif
> > -#if ((defined(CONFIG_NETTLE) || defined(CONFIG_GCRYPT)) && \
> > - (defined(_WIN32) || defined(RUSAGE_THREAD)))
> > +#if defined(_WIN32) || defined(RUSAGE_THREAD) || defined(CONFIG_DARWIN)
>
> Add CONFIG_DARWIN in a subsequent commit?
Yes, classic trap. If you have "Also ..." in a commit message then you've
just told yourself it should have been a separate commit :-)
>
> > #include "crypto/pbkdf.h"
> > typedef struct QCryptoPbkdfTestData QCryptoPbkdfTestData;
> > @@ -422,13 +421,17 @@ int main(int argc, char **argv)
> > g_assert(qcrypto_init(NULL) == 0);
> > for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
> > + if (!qcrypto_pbkdf2_supports(test_data[i].hash)) {
> > + continue;
> > + }
> > +
> > if (!test_data[i].slow ||
> > g_test_slow()) {
> > g_test_add_data_func(test_data[i].path, &test_data[i], test_pbkdf);
> > }
> > }
> > - if (g_test_slow()) {
> > + if (g_test_slow() && qcrypto_pbkdf2_supports(QCRYPTO_HASH_ALG_SHA256)) {
> > g_test_add_func("/crypt0/pbkdf/timing", test_pbkdf_timing);
>
> While here, rename test_pbkdf_timing -> test_pbkdf_sha256_timing?
Will do
>
> > }
>
> Anyway,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
With 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] 6+ messages in thread
end of thread, other threads:[~2024-09-04 9:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-30 11:05 [PATCH 0/2] crypto: misc pbkdf fixes for testing & algorithm compat Daniel P. Berrangé
2024-08-30 11:05 ` [PATCH 1/2] crypto: check gnutls & gcrypt support the requested pbkdf hash Daniel P. Berrangé
2024-09-02 19:43 ` Philippe Mathieu-Daudé
2024-08-30 11:05 ` [PATCH 2/2] tests/unit: always build the pbkdf crypto unit test Daniel P. Berrangé
2024-09-02 19:45 ` Philippe Mathieu-Daudé
2024-09-04 9:00 ` Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).