| Message ID | 20240830110533.2544070-3-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | crypto: misc pbkdf fixes for testing & algorithm compat | expand |
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>
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
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); }
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(-)