On Wed, 23 Oct 2019 at 06:51, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Thu, Oct 17, 2019 at 09:09:19PM +0200, Ard Biesheuvel wrote:
> > diff --git a/lib/crypto/blake2s-selftest.c b/lib/crypto/blake2s-selftest.c
> > new file mode 100644
> > index 000000000000..7ba00fcc6b60
> > --- /dev/null
> > +++ b/lib/crypto/blake2s-selftest.c
> > @@ -0,0 +1,2093 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > +/*
> > + * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@xxxxxxxxx>. All Rights Reserved.
> > + */
> > +
> > +#include <crypto/blake2s.h>
> > +#include <linux/string.h>
> > +
> > +static const u8 blake2s_testvecs[][BLAKE2S_HASH_SIZE] __initconst = {
> [...]
> > +bool __init blake2s_selftest(void)
> > +{
> > + u8 key[BLAKE2S_KEY_SIZE];
> > + u8 buf[ARRAY_SIZE(blake2s_testvecs)];
> > + u8 hash[BLAKE2S_HASH_SIZE];
> > + size_t i;
> > + bool success = true;
> > +
> > + for (i = 0; i < BLAKE2S_KEY_SIZE; ++i)
> > + key[i] = (u8)i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(blake2s_testvecs); ++i)
> > + buf[i] = (u8)i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(blake2s_keyed_testvecs); ++i) {
> > + blake2s(hash, buf, key, BLAKE2S_HASH_SIZE, i, BLAKE2S_KEY_SIZE);
> > + if (memcmp(hash, blake2s_keyed_testvecs[i], BLAKE2S_HASH_SIZE)) {
> > + pr_err("blake2s keyed self-test %zu: FAIL\n", i + 1);
> > + success = false;
> > + }
> > + }
> > +
> > + for (i = 0; i < ARRAY_SIZE(blake2s_testvecs); ++i) {
> > + blake2s(hash, buf, NULL, BLAKE2S_HASH_SIZE, i, 0);
> > + if (memcmp(hash, blake2s_testvecs[i], BLAKE2S_HASH_SIZE)) {
> > + pr_err("blake2s unkeyed self-test %zu: FAIL\n", i + i);
> > + success = false;
> > + }
> > + }
> > + return success;
> > +}
>
> The only tests here are for blake2s(), with 0 and 32-byte keys. There's no
> tests that incremental blake2s_update()s work correctly, nor any other key
> sizes. And these don't get tested properly by the blake2s-generic shash tests
> either, because blake2s-generic has a separate implementation of the boilerplate
> and calls blake2s_compress_generic() directly. Did you consider implementing
> blake2s-generic on top of blake2s_init/update/final instead?
>
That would make blake2s-generic use the accelerated implementations as
well, which I tried to avoid.
I will add some test cases here instead.
> Also, blake2s_hmac() needs tests.
>
Ack
