* [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
* 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
* [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 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).