| Message ID | cover.1570812094.git.dsterba@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | BLAKE2b generic implementation | expand |
On Fri, Oct 11, 2019 at 06:52:03PM +0200, David Sterba wrote: > 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.
On Fri, Oct 11, 2019 at 06:52:03PM +0200, David Sterba wrote: > 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) 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 > > 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 Can you please rebase this onto cryptodev/master? - Eric
On Fri, Oct 11, 2019 at 10:57:40AM -0700, Eric Biggers wrote: > 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. > 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 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. > > 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 > > Can you please rebase this onto cryptodev/master? Will do. Thanks for the comments.
On Sun, Oct 13, 2019 at 09:50:52PM +0200, David Sterba wrote: > On Fri, Oct 11, 2019 at 10:57:40AM -0700, Eric Biggers wrote: > > 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. > > > 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 > > 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
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/cover.1569849051.git.dsterba@suse.com/ 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