Fix crypto testmgr reference to rng self tests.
It seems the ansi_cprng self tests get skipped currently. It appears this is
because the test name is "ansi_cprng", but the test is looked up by the cra_name
value of the algorithm in question, and ansi_cprng registers its cra_name as
stdrng. This patch brings the two into line.
Signed-off-by: Neil Horman <[email protected]>
testmgr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bea9d..3315a38 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1480,7 +1480,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
/* Please keep this list sorted by algorithm name. */
static const struct alg_test_desc alg_test_descs[] = {
{
- .alg = "ansi_cprng",
+ .alg = "stdrng",
.test = alg_test_cprng,
.fips_allowed = 1,
.suite = {
* Neil Horman | 2009-07-01 11:52:19 [-0400]:
>Fix crypto testmgr reference to rng self tests.
>
>It seems the ansi_cprng self tests get skipped currently. It appears this is
>because the test name is "ansi_cprng", but the test is looked up by the cra_name
>value of the algorithm in question, and ansi_cprng registers its cra_name as
>stdrng. This patch brings the two into line.
>
>Signed-off-by: Neil Horman <[email protected]>
>
>
> testmgr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/crypto/testmgr.c b/crypto/testmgr.c
>index f9bea9d..3315a38 100644
>--- a/crypto/testmgr.c
>+++ b/crypto/testmgr.c
>@@ -1480,7 +1480,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
> /* Please keep this list sorted by algorithm name. */
> static const struct alg_test_desc alg_test_descs[] = {
> {
>- .alg = "ansi_cprng",
>+ .alg = "stdrng",
> .test = alg_test_cprng,
> .fips_allowed = 1,
> .suite = {
Don't you use this vector for _all_ stdrngs now? krng for instance will
fail.
Sebastian
* Sebastian Andrzej Siewior | 2009-07-02 08:02:55 [+0200]:
>* Neil Horman | 2009-07-01 11:52:19 [-0400]:
>
>>index f9bea9d..3315a38 100644
>>--- a/crypto/testmgr.c
>>+++ b/crypto/testmgr.c
>>@@ -1480,7 +1480,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
>> /* Please keep this list sorted by algorithm name. */
>> static const struct alg_test_desc alg_test_descs[] = {
>> {
>>- .alg = "ansi_cprng",
>>+ .alg = "stdrng",
>> .test = alg_test_cprng,
>> .fips_allowed = 1,
>> .suite = {
>
>Don't you use this vector for _all_ stdrngs now? krng for instance will
>fail.
This is wrong anyway because the alg field has to be sorted
alphabetically due to the nature of the search function.
Sebastian
On Thu, Jul 02, 2009 at 08:02:55AM +0200, Sebastian Andrzej Siewior wrote:
>
> >@@ -1480,7 +1480,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
> > /* Please keep this list sorted by algorithm name. */
> > static const struct alg_test_desc alg_test_descs[] = {
> > {
> >- .alg = "ansi_cprng",
> >+ .alg = "stdrng",
> > .test = alg_test_cprng,
> > .fips_allowed = 1,
> > .suite = {
>
> Don't you use this vector for _all_ stdrngs now? krng for instance will
> fail.
Good cath!
What this really should do is test a specific implementation of
stdrng. Like this,
commit a68f6610d4f1ebe61818f5926fa8fa9e75d06a95
Author: Herbert Xu <[email protected]>
Date: Thu Jul 2 16:32:12 2009 +0800
crypto: testmgr - Allow implementation-specific tests
This patch adds the support for testing specific implementations.
This should only be used in very specific situations. Right now
this means specific implementations of random number generators.
Signed-off-by: Herbert Xu <[email protected]>
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index f9bea9d..29b228d 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -2344,6 +2344,7 @@ static int alg_find_test(const char *alg)
int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
{
int i;
+ int j;
int rc;
if ((type & CRYPTO_ALG_TYPE_MASK) == CRYPTO_ALG_TYPE_CIPHER) {
@@ -2365,14 +2366,22 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
}
i = alg_find_test(alg);
- if (i < 0)
+ j = alg_find_test(driver);
+ if (i < 0 && j < 0)
goto notest;
- if (fips_enabled && !alg_test_descs[i].fips_allowed)
+ if (fips_enabled && ((i >= 0 && !alg_test_descs[i].fips_allowed) ||
+ (j >= 0 && !alg_test_descs[j].fips_allowed)))
goto non_fips_alg;
- rc = alg_test_descs[i].test(alg_test_descs + i, driver,
- type, mask);
+ rc = 0;
+ if (i >= 0)
+ rc |= alg_test_descs[i].test(alg_test_descs + i, driver,
+ type, mask);
+ if (j >= 0)
+ rc |= alg_test_descs[j].test(alg_test_descs + j, driver,
+ type, mask);
+
test_done:
if (fips_enabled && rc)
panic("%s: %s alg self test failed in fips mode!\n", driver, alg);
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Jul 02, 2009 at 04:32:59PM +0800, Herbert Xu wrote:
> On Thu, Jul 02, 2009 at 08:02:55AM +0200, Sebastian Andrzej Siewior wrote:
> >
> > >@@ -1480,7 +1480,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
> > > /* Please keep this list sorted by algorithm name. */
> > > static const struct alg_test_desc alg_test_descs[] = {
> > > {
> > >- .alg = "ansi_cprng",
> > >+ .alg = "stdrng",
> > > .test = alg_test_cprng,
> > > .fips_allowed = 1,
> > > .suite = {
> >
> > Don't you use this vector for _all_ stdrngs now? krng for instance will
> > fail.
>
> Good cath!
>
Indeed, I didn't even think of that.
> What this really should do is test a specific implementation of
> stdrng. Like this,
>
Yeah, that looks right
Acked-by: Neil Horman <[email protected]>
* Herbert Xu | 2009-07-02 16:32:59 [+0800]:
>--- a/crypto/testmgr.c
>+++ b/crypto/testmgr.c
>@@ -2365,14 +2366,22 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
> }
>
> i = alg_find_test(alg);
>- if (i < 0)
>+ j = alg_find_test(driver);
>+ if (i < 0 && j < 0)
> goto notest;
>
>- if (fips_enabled && !alg_test_descs[i].fips_allowed)
>+ if (fips_enabled && ((i >= 0 && !alg_test_descs[i].fips_allowed) ||
>+ (j >= 0 && !alg_test_descs[j].fips_allowed)))
> goto non_fips_alg;
>
>- rc = alg_test_descs[i].test(alg_test_descs + i, driver,
>- type, mask);
>+ rc = 0;
>+ if (i >= 0)
>+ rc |= alg_test_descs[i].test(alg_test_descs + i, driver,
>+ type, mask);
>+ if (j >= 0)
>+ rc |= alg_test_descs[j].test(alg_test_descs + j, driver,
>+ type, mask);
Do you execute test2 if test1 failed on purpose?
If not what about:
if (!rc && j >= 0)
rc = alg_test_descs[j].test(alg_test_descs + j, driver,
type, mask);
so there is return code mixup.
Sebastian
On Fri, Jul 03, 2009 at 10:59:09PM +0200, Sebastian Andrzej Siewior wrote:
>
> >- rc = alg_test_descs[i].test(alg_test_descs + i, driver,
> >- type, mask);
> >+ rc = 0;
> >+ if (i >= 0)
> >+ rc |= alg_test_descs[i].test(alg_test_descs + i, driver,
> >+ type, mask);
> >+ if (j >= 0)
> >+ rc |= alg_test_descs[j].test(alg_test_descs + j, driver,
> >+ type, mask);
>
> Do you execute test2 if test1 failed on purpose?
Yes that was intentional. Not that it makes much of a differnce.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt