BLAKE2b generic implementation

PATCH Superseded David Sterba 3 participants 13 messages Oct 11, 2019 – Oct 14, 2019

Subsystems (2)

Crypto API Arm STM32

lore

Share & tools

lore

David Sterba

author

2019-10-11 16:52 UTC #0 lore raw

v4 cover BLAKE2b generic implementation

The patchset adds blake2b refrerence implementation and test vectors.

V4:

Code changes:

- removed .finup
- removed .cra_init
- dropped redundant sanity checks (key length, output size length)
- switch blake2b_param from a 1 element array to plain struct
- direct assignment in blake2b_init, instead of put_unaligned*
- removed blake2b_is_lastblock
- removed useless error cases in the blake * helpers
- replace digest_desc_ctx with blake2b_state
- use __le32 in blake2b_param

Added testmgr vectors:

- all digests covered: 160, 256, 384, 512
- 4 different keys:
- empty
- short (1 byte, 'B', 0x42)
- half of the default key (32 bytes, sequence 00..1f)
- default key (64 bytes, sequence 00..3f)
- plaintext values:
- subsequences of 0..15 and 247..255
- the full range 0..255 add up to 4MiB of .h, for all digests and key sizes, so this is not very practical for the in-kernel testsuite
- official blake2 provided test vectors are only for empty and default key for digest size 512
- the remaining combinations were obtained from b2sum utility (enhanced to accept a key)

Testing performed:

- compiled with SLUB_DEBUG and KASAN, plus crypto selftests CONFIG_CRYPTO_MANAGER2=y CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
- module loaded, no errors reported from the tessuite
- (un)intentionally broken test values were detected

The test values were produced by b2sum, compiled from the reference implementation. The generated values were cross-checked by pyblake2 based script (ie. not the same sources, built by distro).

The .h portion of testmgr is completely generated, so in case somebody feels like reducing it in size, adding more keys, changing the formatting, it's easy to do.

In case the patches don't make it to the mailinglist, it's in git

git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git dev/blake2b-v4

V1: https://lore.kernel.org/linux-crypto/[email protected]/
V2: https://lore.kernel.org/linux-crypto/e31c2030fcfa7f409b2c81adf8f179a8a55a584a.1570184333.git.dsterba@suse.com/
V3: https://lore.kernel.org/linux-crypto/e7f46def436c2c705c0b2cac3324f817efa4717d.1570715842.git.dsterba@suse.com/

David Sterba (5):
crypto: add blake2b generic implementation
crypto: add test vectors for blake2b-160
crypto: add test vectors for blake2b-256
crypto: add test vectors for blake2b-384
crypto: add test vectors for blake2b-512

 crypto/Kconfig           |    17 +
 crypto/Makefile          |     1 +
 crypto/blake2b_generic.c |   418 ++
 crypto/testmgr.c         |    28 +
 crypto/testmgr.h         | 10561 +++++++++++++++++++++++++++++++++++++
 include/crypto/blake2b.h |    48 +
 6 files changed, 11073 insertions(+)
 create mode 100644 crypto/blake2b_generic.c
 create mode 100644 include/crypto/blake2b.h

Signature · 1 line

2.23.0

David Sterba

author

2019-10-11 16:52 UTC #1 lore raw

PATCH v4 1/5 crypto: add blake2b generic implementation

The patch brings support of several BLAKE2 variants (2b with various digest lengths). The keyed digest is supported, using tfm->setkey call. The in-tree user will be btrfs (for checksumming), we're going to use the BLAKE2b-256 variant.

The code is reference implementation taken from the official sources and modified in terms of kernel coding style (whitespace, comments, uintXX_t -> uXX types, removed unused prototypes and #ifdefs, removed testing code, changed secure_zero_memory -> memzero_explicit, used own helpers for unaligned reads/writes and rotations).

Further changes removed sanity checks of key length or output size, these values are verified in the crypto API callbacks or hardcoded in shash_alg and not exposed to users.

Signed-off-by David Sterba <[email protected]>

---
 crypto/Kconfig           |  17 ++
 crypto/Makefile          |   1 +
 crypto/blake2b_generic.c | 418 +++++++++++++++++++++++++++++++++++++++
 include/crypto/blake2b.h |  48 +++++
 4 files changed, 484 insertions(+)
 create mode 100644 crypto/blake2b_generic.c
 create mode 100644 include/crypto/blake2b.h

crypto/Kconfig +17 -0

index e801450bcb1c..192cbb824928 100644
@@ -691,6 +691,23 @@ config CRYPTO_XXHASH
 	  xxHash non-cryptographic hash algorithm. Extremely fast, working at
 	  speeds close to RAM limits.
 
+config CRYPTO_BLAKE2B
+	tristate "BLAKE2b digest algorithm"
+	select CRYPTO_HASH
+	help
+	  Implementation of cryptographic hash function BLAKE2b (or just BLAKE2),
+	  optimized for 64bit platforms and can produce digests of any size
+	  between 1 to 64.  The keyed hash is also implemented.
+
+	  This module provides the following algorithms:
+
+	  - blake2b-160
+	  - blake2b-256
+	  - blake2b-384
+	  - blake2b-512
+
+	  See https://blake2.net for further information.
+
 config CRYPTO_CRCT10DIF
 	tristate "CRCT10DIF algorithm"
 	select CRYPTO_HASH

crypto/Makefile +1 -0

index 9479e1a45d8c..2318420d3e71 100644
@@ -74,6 +74,7 @@ obj-$(CONFIG_CRYPTO_STREEBOG) += streebog_generic.o
 obj-$(CONFIG_CRYPTO_WP512) += wp512.o
 CFLAGS_wp512.o := $(call cc-option,-fno-schedule-insns)  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149
 obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
+obj-$(CONFIG_CRYPTO_BLAKE2B) += blake2b_generic.o
 obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
 obj-$(CONFIG_CRYPTO_ECB) += ecb.o
 obj-$(CONFIG_CRYPTO_CBC) += cbc.o

crypto/blake2b_generic.c new file +418 -0

418 lines not on this page · raw message

include/crypto/blake2b.h new file +48 -0

48 lines not on this page · raw message

Signature · 1 line

2.23.0

Eric Biggers

2019-10-11 18:04 UTC #2 lore raw

Re: [PATCH v4 1/5] crypto: add blake2b generic implementation

Quoted 34 lines · David Sterba

crypto/Makefile +1 -0

index 9479e1a45d8c..2318420d3e71 100644
@@ -74,6 +74,7 @@ obj-$(CONFIG_CRYPTO_STREEBOG) += streebog_generic.o
 obj-$(CONFIG_CRYPTO_WP512) += wp512.o
 CFLAGS_wp512.o := $(call cc-option,-fno-schedule-insns)  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149
 obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
+obj-$(CONFIG_CRYPTO_BLAKE2B) += blake2b_generic.o
 obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
 obj-$(CONFIG_CRYPTO_ECB) += ecb.o
 obj-$(CONFIG_CRYPTO_CBC) += cbc.o

crypto/blake2b_generic.c new file +16 -0

new file mode 100644
index 000000000000..e31fb669383b
@@ -0,0 +1,418 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR Apache-2.0)
+/*
+ * BLAKE2b reference source code package - reference C implementations
+ *
+ * Copyright 2012, Samuel Neves <[email protected]>.  You may use this under the
+ * terms of the CC0, the OpenSSL Licence, or the Apache Public License 2.0, at
+ * your option.  The terms of these licenses can be found at:
+ *
+ * - CC0 1.0 Universal : http://creativecommons.org/publicdomain/zero/1.0
+ * - OpenSSL license   : https://www.openssl.org/source/license.html
+ * - Apache 2.0        : http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * More information about the BLAKE2 hash function can be found at
+ * https://blake2.net.
+ */
+

crypto/blake2b_generic.c line 26

Quoted 10 lines

crypto/blake2b_generic.c new file +10 -0

+#include <asm/unaligned.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <crypto/internal/hash.h>
+#include <crypto/blake2b.h>
+
+struct blake2b_param
+{

It should be 'struct blake2b_param {'

checkpatch.pl should warn about this. Can you fix the checkpatch warnings that make sense to fix?

Quoted 6 lines

crypto/blake2b_generic.c new file +6 -0

+/* init xors IV with input parameter block */
+static int blake2b_init_param(struct blake2b_state *S,
+			      const struct blake2b_param *P)
+{
+	const u8 *p = (const u8 *)(P);
+	size_t i;

crypto/blake2b_generic.c line 111

Quoted 10 lines

crypto/blake2b_generic.c new file +10 -0

+
+	blake2b_init0(S);
+
+	/* IV XOR ParamBlock */
+	for (i = 0; i < 8; ++i)
+		S->h[i] ^= get_unaligned_le64(p + sizeof(S->h[i]) * i);
+
+	S->outlen = P->digest_length;
+	return 0;
+}

No need for this to have a return value anymore. Same with:

	blake2b_init_param()
	blake2b_update()
	blake2b_init()
	blake2b_init_key()
	blake2b_final()

The code would be more readable if they returned void, since otherwise it gives the impression that errors can occur.

crypto/blake2b_generic.c line 231

Quoted 3 lines

crypto/blake2b_generic.c new file +3 -0

+static int blake2b_update(struct blake2b_state *S, const void *pin, size_t inlen)
+{
+	const unsigned char *in = (const unsigned char *)pin;

Convention is to use 'u8', not 'unsigned char'.

crypto/blake2b_generic.c line 410

Quoted 2 lines

crypto/blake2b_generic.c new file +2 -0
+MODULE_ALIAS_CRYPTO("blake2b");
+MODULE_ALIAS_CRYPTO("blake2b-generic");

Should remove these module aliases now that the "blake2b" algorithm was removed.

- Eric

David Sterba

2019-10-13 17:44 UTC #3 lore raw

Re: [PATCH v4 1/5] crypto: add blake2b generic implementation

Quoted 7 lines · Eric Biggers

Review
Quoted 2 lines
(quoted diff) +2 -0
+struct blake2b_param
+{
It should be 'struct blake2b_param {'
checkpatch.pl should warn about this. Can you fix the checkpatch warnings that make sense to fix?

I fixed all the { at the end of line of struct definitions and left all of type

ERROR: space prohibited after that open square bracket '[' #276: FILE: crypto/blake2b_generic.c:200:
+ v[ 9] = blake2b_IV[1];

and

WARNING: line over 80 characters
#304: FILE: crypto/blake2b_generic.c:228:
+static void blake2b_update(struct blake2b_state *S, const void *pin, size_t inlen)

ie. where the ) or ); is beyond 80 but does not otherwise hurt readability unlike forced newline and parameters on another line. This is my prefrence in code I otherwise work on but I'll follow what's common practice in crypto/.

Quoted 19 lines

Quoted 6 lines
crypto/blake2b_generic.c new file +6 -0
+/* init xors IV with 
+static int blake2b_init_param(struct blake2b_state *S,
+			      const struct blake2b_param *P)
+{
+	const u8 *p = (const u8 *)(P);
+	size_t i;
crypto/blake2b_generic.c line 111
Quoted 10 lines
crypto/blake2b_generic.c new file +10 -0
+
+	blake2b_init0(S);
+
+	/* IV XOR ParamBlock */
+	for (i = 0; i < 8; ++i)
+		S->h[i] ^= get_unaligned_le64(p + sizeof(S->h[i]) * i);
+
+	S->outlen = P->digest_length;
+	return 0;
+}
No need for this to have a return value anymore. Same with:

Quoted 8 lines

	blake2b_init_param()
	blake2b_update()
	blake2b_init()
	blake2b_init_key()
	blake2b_final()
The code would be more readable if they returned void, since otherwise it gives the impression that errors can occur.

Make sense.

Quoted 5 lines

crypto/blake2b_generic.c line 231
Quoted 3 lines
crypto/blake2b_generic.c new file +3 -0
+static int blake2b_update(struct blake2b_state *S, const void *pin, size_t inlen)
+{
+	const unsigned char *in = (const unsigned char *)pin;
Convention is to use 'u8', not 'unsigned char'.

Fixed.

Quoted 4 lines

crypto/blake2b_generic.c line 410
Quoted 2 lines
crypto/blake2b_generic.c new file +2 -0
+MODULE_ALIAS_CRYPTO("blake2b");
+MODULE_ALIAS_CRYPTO("blake2b-generic");
Should remove these module aliases now that the "blake2b" algorithm was removed.

A bit more of typing 'modprobe blake2-512' but I have checked if other modules use a bare algo name as an alias and found none. So for consistency I'll remove the 2 lines.

Eric Biggers

2019-10-11 18:11 UTC #4 lore raw

Re: [PATCH v4 1/5] crypto: add blake2b generic implementation

In reply to David Sterba

Quoted 76 lines · David Sterba

The patch brings support of several BLAKE2 variants (2b with various digest lengths). The keyed digest is supported, using tfm->setkey call. The in-tree user will be btrfs (for checksumming), we're going to use the BLAKE2b-256 variant.
The code is reference implementation taken from the official sources and modified in terms of kernel coding style (whitespace, comments, uintXX_t -> uXX types, removed unused prototypes and #ifdefs, removed testing code, changed secure_zero_memory -> memzero_explicit, used own helpers for unaligned reads/writes and rotations).
Further changes removed sanity checks of key length or output size, these values are verified in the crypto API callbacks or hardcoded in shash_alg and not exposed to users.
Signed-off-by David Sterba <[email protected]>
---
 crypto/Kconfig           |  17 ++
 crypto/Makefile          |   1 +
 crypto/blake2b_generic.c | 418 +++++++++++++++++++++++++++++++++++++++
 include/crypto/blake2b.h |  48 +++++
 4 files changed, 484 insertions(+)
 create mode 100644 crypto/blake2b_generic.c
 create mode 100644 include/crypto/blake2b.h
crypto/Kconfig +17 -0
index e801450bcb1c..192cbb824928 100644
@@ -691,6 +691,23 @@ config CRYPTO_XXHASH
 	  xxHash non-cryptographic hash algorithm. Extremely fast, working at
 	  speeds close to RAM limits.
 
+config CRYPTO_BLAKE2B
+	tristate "BLAKE2b digest algorithm"
+	select CRYPTO_HASH
+	help
+	  Implementation of cryptographic hash function BLAKE2b (or just BLAKE2),
+	  optimized for 64bit platforms and can produce digests of any size
+	  between 1 to 64.  The keyed hash is also implemented.
+
+	  This module provides the following algorithms:
+
+	  - blake2b-160
+	  - blake2b-256
+	  - blake2b-384
+	  - blake2b-512
+
+	  See https://blake2.net for further information.
+
 config CRYPTO_CRCT10DIF
 	tristate "CRCT10DIF algorithm"
 	select CRYPTO_HASH
crypto/Makefile +1 -0
index 9479e1a45d8c..2318420d3e71 100644
@@ -74,6 +74,7 @@ obj-$(CONFIG_CRYPTO_STREEBOG) += streebog_generic.o
 obj-$(CONFIG_CRYPTO_WP512) += wp512.o
 CFLAGS_wp512.o := $(call cc-option,-fno-schedule-insns)  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149
 obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
+obj-$(CONFIG_CRYPTO_BLAKE2B) += blake2b_generic.o
 obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
 obj-$(CONFIG_CRYPTO_ECB) += ecb.o
 obj-$(CONFIG_CRYPTO_CBC) += cbc.o
crypto/blake2b_generic.c new file +5 -0
new file mode 100644
index 000000000000..e31fb669383b
@@ -0,0 +1,418 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR Apache-2.0)
+/*
+ * BLAKE2b reference source code package - reference C implementations
+ *
+ * Copyright 2012, Samuel Neves <[email protected]>.  You may use this under the

crypto/blake2b_generic.c line 15

Quoted 10 lines

crypto/blake2b_generic.c new file +10 -0

+ * terms of the CC0, the OpenSSL Licence, or the Apache Public License 2.0, at
+ * your option.  The terms of these licenses can be found at:
+ *
+ * - CC0 1.0 Universal : http://creativecommons.org/publicdomain/zero/1.0
+ * - OpenSSL license   : https://www.openssl.org/source/license.html
+ * - Apache 2.0        : http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * More information about the BLAKE2 hash function can be found at
+ * https://blake2.net.
+ */

Can you also adjust this comment to make it clear that this isn't the reference implementation verbatim, but rather it's been modified for inclusion in the kernel?

Thanks!

- Eric

David Sterba

author

2019-10-11 16:52 UTC #5 lore raw

PATCH v4 2/5 crypto: add test vectors for blake2b-160

In reply to David Sterba

Add test vectors for blake2b, with 160 bit digest size. The tested variants are with the following:

- empty key
- 1 byte key
- first 32 bytes of the default key
- default key (64 bytes)

Tested plain text are sequences of bytes of lengths 0..15 and 247..255.

Signed-off-by David Sterba <[email protected]>

---
 crypto/testmgr.c |    7 +
 crypto/testmgr.h | 2416 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 2423 insertions(+)

crypto/testmgr.c +7 -0

index d0b5b33806a6..f92879f00df6 100644
@@ -4034,6 +4034,13 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.alg = "authenc(hmac(sha512),rfc3686(ctr(aes)))",
 		.test = alg_test_null,
 		.fips_allowed = 1,
+	}, {
+		.alg = "blake2b-160",
+		.test = alg_test_hash,
+		.fips_allowed = 0,
+		.suite = {
+			.hash = __VECS(blake2b_160_tv_template)
+		}
 	}, {
 		.alg = "cbc(aes)",
 		.test = alg_test_skcipher,

crypto/testmgr.h +2416 -0

2,420 lines not on this page · raw message

Signature · 1 line

2.23.0

David Sterba

author

2019-10-11 16:52 UTC #6 lore raw

PATCH v4 3/5 crypto: add test vectors for blake2b-256

In reply to David Sterba

Add test vectors for blake2b, with 256 bit digest size. The tested variants are with the following:

- empty key
- 1 byte key
- first 32 bytes of the default key
- default key (64 bytes)

Tested plain text are sequences of bytes of lengths 0..15 and 247..255.

Signed-off-by David Sterba <[email protected]>

---
 crypto/testmgr.c |    7 +
 crypto/testmgr.h | 2515 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 2522 insertions(+)

crypto/testmgr.c +7 -0

index f92879f00df6..3cedda2b5d85 100644
@@ -4041,6 +4041,13 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.hash = __VECS(blake2b_160_tv_template)
 		}
+	}, {
+		.alg = "blake2b-256",
+		.test = alg_test_hash,
+		.fips_allowed = 0,
+		.suite = {
+			.hash = __VECS(blake2b_256_tv_template)
+		}
 	}, {
 		.alg = "cbc(aes)",
 		.test = alg_test_skcipher,

crypto/testmgr.h +2515 -0

2,519 lines not on this page · raw message

Signature · 1 line

2.23.0

David Sterba

author

2019-10-11 16:52 UTC #7 lore raw

PATCH v4 4/5 crypto: add test vectors for blake2b-384

In reply to David Sterba

Add test vectors for blake2b, with 384 bit digest size. The tested variants are with the following:

- empty key
- 1 byte key
- first 32 bytes of the default key
- default key (64 bytes)

Tested plain text are sequences of bytes of lengths 0..15 and 247..255.

Signed-off-by David Sterba <[email protected]>

---
 crypto/testmgr.c |    7 +
 crypto/testmgr.h | 2715 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 2722 insertions(+)

crypto/testmgr.c +7 -0

index 3cedda2b5d85..f0e6d5f7982a 100644
@@ -4048,6 +4048,13 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.hash = __VECS(blake2b_256_tv_template)
 		}
+	}, {
+		.alg = "blake2b-384",
+		.test = alg_test_hash,
+		.fips_allowed = 0,
+		.suite = {
+			.hash = __VECS(blake2b_384_tv_template)
+		}
 	}, {
 		.alg = "cbc(aes)",
 		.test = alg_test_skcipher,

crypto/testmgr.h +2715 -0

2,719 lines not on this page · raw message

Signature · 1 line

2.23.0

David Sterba

author

2019-10-11 16:52 UTC #8 lore raw

PATCH v4 5/5 crypto: add test vectors for blake2b-512

In reply to David Sterba

Add test vectors for blake2b, with 512 bit digest size. This is also the default digest size for blake2b. The tested variants are with the following:

- empty key
- 1 byte key
- first 32 bytes of the default key
- default key (64 bytes)

Tested plain text are sequences of bytes of lengths 0..15 and 247..255.

Signed-off-by David Sterba <[email protected]>

---
 crypto/testmgr.c |    7 +
 crypto/testmgr.h | 2915 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 2922 insertions(+)

crypto/testmgr.c +7 -0

index f0e6d5f7982a..8a879226afcc 100644
@@ -4055,6 +4055,13 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.hash = __VECS(blake2b_384_tv_template)
 		}
+	}, {
+		.alg = "blake2b-512",
+		.test = alg_test_hash,
+		.fips_allowed = 0,
+		.suite = {
+			.hash = __VECS(blake2b_512_tv_template)
+		}
 	}, {
 		.alg = "cbc(aes)",
 		.test = alg_test_skcipher,

crypto/testmgr.h +2915 -0

2,919 lines not on this page · raw message

Signature · 1 line

2.23.0

David Sterba

2019-10-11 17:15 UTC #9 lore raw

In reply to David Sterba

Quoted 7 lines · David Sterba

Testing performed:
- compiled with SLUB_DEBUG and KASAN, plus crypto selftests CONFIG_CRYPTO_MANAGER2=y CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
- module loaded, no errors reported from the tessuite

I forgot to add that this was on x86_64 only, I don't have big-endian test setup. Ard, you offered to do the tests on my behalf, thanks.

Eric Biggers

2019-10-11 17:57 UTC #10 lore raw

In reply to David Sterba

Quoted 18 lines · David Sterba

The patchset adds blake2b refrerence implementation and test vectors.
V4:
Code changes:
- removed .finup
- removed .cra_init
- dropped redundant sanity checks (key length, output size length)
- switch blake2b_param from a 1 element array to plain struct
- direct assignment in blake2b_init, instead of put_unaligned*
- removed blake2b_is_lastblock
- removed useless error cases in the blake * helpers
- replace digest_desc_ctx with blake2b_state
- use __le32 in blake2b_param
Added testmgr vectors:

Quoted 14 lines

- all digests covered: 160, 256, 384, 512
- 4 different keys:
- empty
- short (1 byte, 'B', 0x42)
- half of the default key (32 bytes, sequence 00..1f)
- default key (64 bytes, sequence 00..3f)
- plaintext values:
- subsequences of 0..15 and 247..255
- the full range 0..255 add up to 4MiB of .h, for all digests and key sizes, so this is not very practical for the in-kernel testsuite
- official blake2 provided test vectors are only for empty and default key for digest size 512
- the remaining combinations were obtained from b2sum utility (enhanced to accept a key)

The choice of data lengths seems a bit unusual, as they include every length in two ranges but nothing in between. Also, none of the lengths except 0 is a multiple of the blake2b block size. Instead, maybe use something like [0, 1, 7, 15, 64, 247, 256]?

Also, since the 4 variants share nearly all their code, it seems the tests would be just as effective in practice if we cut the test vectors down by 4x by distributing the key lengths among each variant. Like:

          blake2b-160  blake2b-256  blake2b-384  blake2b-512
         ---------------------------------------------------
len=0   | klen=0       klen=1       klen=16      klen=32
len=1   | klen=16      klen=32      klen=0       klen=1
len=7   | klen=32      klen=0       klen=1       klen=16
len=15  | klen=1       klen=16      klen=32      klen=0
len=64  | klen=0       klen=1       klen=16      klen=32
len=247 | klen=16      klen=32      klen=0       klen=1
len=256 | klen=32      klen=0       klen=1       klen=16

Quoted 14 lines

Testing performed:
- compiled with SLUB_DEBUG and KASAN, plus crypto selftests CONFIG_CRYPTO_MANAGER2=y CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
- module loaded, no errors reported from the tessuite
- (un)intentionally broken test values were detected
The test values were produced by b2sum, compiled from the reference implementation. The generated values were cross-checked by pyblake2 based script (ie. not the same sources, built by distro).

Quoted 8 lines

The .h portion of testmgr is completely generated, so in case somebody feels like reducing it in size, adding more keys, changing the formatting, it's easy to do.
In case the patches don't make it to the mailinglist, it's in git
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git dev/blake2b-v4

Can you please rebase this onto cryptodev/master?

- Eric

David Sterba

2019-10-13 19:50 UTC #11 lore raw

Quoted 4 lines · Eric Biggers

The choice of data lengths seems a bit unusual, as they include every length in two ranges but nothing in between. Also, none of the lengths except 0 is a multiple of the blake2b block size. Instead, maybe use something like [0, 1, 7, 15, 64, 247, 256]?

Just to clarify, do you mean the block size defined by BLAKE2B_BLOCKBYTES? That's 128, so that makes 0 and 256 the multiples.

Quoted 4 lines

Also, since the 4 variants share nearly all their code, it seems the tests would be just as effective in practice if we cut the test vectors down by 4x by distributing the key lengths among each variant. Like:

Quoted 9 lines

          blake2b-160  blake2b-256  blake2b-384  blake2b-512
         ---------------------------------------------------
len=0   | klen=0       klen=1       klen=16      klen=32
len=1   | klen=16      klen=32      klen=0       klen=1
len=7   | klen=32      klen=0       klen=1       klen=16
len=15  | klen=1       klen=16      klen=32      klen=0
len=64  | klen=0       klen=1       klen=16      klen=32
len=247 | klen=16      klen=32      klen=0       klen=1
len=256 | klen=32      klen=0       klen=1       klen=16

That's clever. I assume the 32 key length refers to the default key, right? That's 64 bytes (BLAKE2B_KEYBYTES), so I'll use that value.

Quoted 18 lines

Quoted 18 lines
Testing performed:
- compiled with SLUB_DEBUG and KASAN, plus crypto selftests CONFIG_CRYPTO_MANAGER2=y CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
- module loaded, no errors reported from the tessuite
- (un)intentionally broken test values were detected
The test values were produced by b2sum, compiled from the reference implementation. The generated values were cross-checked by pyblake2 based script (ie. not the same sources, built by distro).
The .h portion of testmgr is completely generated, so in case somebody feels like reducing it in size, adding more keys, changing the formatting, it's easy to do.

Quoted 5 lines

Quoted 3 lines
In case the patches don't make it to the mailinglist, it's in git
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git dev/blake2b-v4
Can you please rebase this onto cryptodev/master?

Will do.

Thanks for the comments.

Eric Biggers

2019-10-14 02:54 UTC #12 lore raw

Quoted 8 lines · David Sterba

Quoted 4 lines · Eric Biggers
The choice of data lengths seems a bit unusual, as they include every length in two ranges but nothing in between. Also, none of the lengths except 0 is a multiple of the blake2b block size. Instead, maybe use something like [0, 1, 7, 15, 64, 247, 256]?
Just to clarify, do you mean the block size defined by BLAKE2B_BLOCKBYTES? That's 128, so that makes 0 and 256 the multiples.

Yes.

Quoted 15 lines

Quoted 5 lines
Also, since the 4 variants share nearly all their code, it seems the tests would be just as effective in practice if we cut the test vectors down by 4x by distributing the key lengths among each variant. Like:
Quoted 9 lines
          blake2b-160  blake2b-256  blake2b-384  blake2b-512
         ---------------------------------------------------
len=0   | klen=0       klen=1       klen=16      klen=32
len=1   | klen=16      klen=32      klen=0       klen=1
len=7   | klen=32      klen=0       klen=1       klen=16
len=15  | klen=1       klen=16      klen=32      klen=0
len=64  | klen=0       klen=1       klen=16      klen=32
len=247 | klen=16      klen=32      klen=0       klen=1
len=256 | klen=32      klen=0       klen=1       klen=16

Quoted 3 lines

That's clever. I assume the 32 key length refers to the default key, right? That's 64 bytes (BLAKE2B_KEYBYTES), so I'll use that value.

Yes, I meant key lengths [0, 1, 32, 64]. I forgot that BLAKE2b has a max key length of 64 bytes rather than 32.

- Eric