Re: [PATCH 1/2] Crypto: Add SHA-3 hash algorithm

[Raveendra Padasalagi]

Hi Stephan,

Thanks for the review comments. I will address it in the next patch.
Please look at my reply below against each comment.

Regards,
Raveendra

On Wed, Jun 15, 2016 at 5:12 PM, Stephan Mueller <smueller@xxxxxxxxxx> wrote:
> Am Mittwoch, 15. Juni 2016, 15:11:58 schrieb Raveendra Padasalagi:
>
> Hi Raveendra,
>
>> From: Jeff Garzik <jeff@xxxxxxxxxx>
>>
>> This patch adds the implementation of SHA3 algorithm
>> in software and it's based on original implementation
>> pushed in patch https://lwn.net/Articles/518415/ with
>> additional changes to match the padding rules specified
>> in SHA-3 specification.
>>
>> Signed-off-by: Jeff Garzik <jgarzik@xxxxxxxxxx>
>> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@xxxxxxxxxxxx>
>> ---
>>  crypto/Kconfig        |  10 ++
>>  crypto/Makefile       |   1 +
>>  crypto/sha3_generic.c | 296
>> ++++++++++++++++++++++++++++++++++++++++++++++++++ include/crypto/sha3.h |
>> 29 +++++
>>  4 files changed, 336 insertions(+)
>>  create mode 100644 crypto/sha3_generic.c
>>  create mode 100644 include/crypto/sha3.h
>>
>> diff --git a/crypto/Kconfig b/crypto/Kconfig
>> index 1d33beb..83ee8cb 100644
>> --- a/crypto/Kconfig
>> +++ b/crypto/Kconfig
>> @@ -750,6 +750,16 @@ config CRYPTO_SHA512_SPARC64
>>         SHA-512 secure hash standard (DFIPS 180-2) implemented
>>         using sparc64 crypto instructions, when available.
>>
>> +config CRYPTO_SHA3
>> +     tristate "SHA3 digest algorithm"
>> +     select CRYPTO_HASH
>> +     help
>> +       SHA-3 secure hash standard (DFIPS 202). It's based on
>
> Typo DFIPS?

It's not typo, DFIPS mean here Draft FIPS 202.
Do you want me to put it in another way ?

>> +       cryptographic sponge function family called Keccak.
>> +
>> +       References:
>> +       http://keccak.noekeon.org/
>> +
>>  config CRYPTO_TGR192
>>       tristate "Tiger digest algorithms"
>>       select CRYPTO_HASH
>> diff --git a/crypto/Makefile b/crypto/Makefile
>> index 4f4ef7e..0b82c47 100644
>> --- a/crypto/Makefile
>> +++ b/crypto/Makefile
>> @@ -61,6 +61,7 @@ obj-$(CONFIG_CRYPTO_RMD320) += rmd320.o
>>  obj-$(CONFIG_CRYPTO_SHA1) += sha1_generic.o
>>  obj-$(CONFIG_CRYPTO_SHA256) += sha256_generic.o
>>  obj-$(CONFIG_CRYPTO_SHA512) += sha512_generic.o
>> +obj-$(CONFIG_CRYPTO_SHA3) += sha3_generic.o
>>  obj-$(CONFIG_CRYPTO_WP512) += wp512.o
>>  obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
>>  obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
>> diff --git a/crypto/sha3_generic.c b/crypto/sha3_generic.c
>> new file mode 100644
>> index 0000000..162dfc3
>> --- /dev/null
>> +++ b/crypto/sha3_generic.c
>> @@ -0,0 +1,296 @@
>> +/*
>> + * Cryptographic API.
>> + *
>> + * SHA-3, as specified in
>> + * http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.202.pdf
>> + *
>> + * SHA-3 code by Jeff Garzik <jeff@xxxxxxxxxx>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> Free + * Software Foundation; either version 2 of the License, or (at your
>> option)â + * any later version.
>> + *
>> + */
>> +#include <crypto/internal/hash.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/types.h>
>> +#include <crypto/sha3.h>
>> +#include <asm/byteorder.h>
>> +
>> +#define KECCAK_ROUNDS 24
>> +
>> +#define ROTL64(x, y) (((x) << (y)) | ((x) >> (64 - (y))))
>> +
>> +static const u64 keccakf_rndc[24] = {
>> +     0x0000000000000001, 0x0000000000008082, 0x800000000000808a,
>> +     0x8000000080008000, 0x000000000000808b, 0x0000000080000001,
>> +     0x8000000080008081, 0x8000000000008009, 0x000000000000008a,
>> +     0x0000000000000088, 0x0000000080008009, 0x000000008000000a,
>> +     0x000000008000808b, 0x800000000000008b, 0x8000000000008089,
>> +     0x8000000000008003, 0x8000000000008002, 0x8000000000000080,
>> +     0x000000000000800a, 0x800000008000000a, 0x8000000080008081,
>> +     0x8000000000008080, 0x0000000080000001, 0x8000000080008008
>> +};
>> +
>> +static const int keccakf_rotc[24] = {
>> +     1,  3,  6,  10, 15, 21, 28, 36, 45, 55, 2,  14,
>> +     27, 41, 56, 8,  25, 43, 62, 18, 39, 61, 20, 44
>> +};
>> +
>> +static const int keccakf_piln[24] = {
>> +     10, 7,  11, 17, 18, 3, 5,  16, 8,  21, 24, 4,
>> +     15, 23, 19, 13, 12, 2, 20, 14, 22, 9,  6,  1
>> +};
>> +
>> +/* update the state with given number of rounds */
>> +
>> +static void keccakf(u64 st[25])
>> +{
>> +     int i, j, round;
>> +     u64 t, bc[5];
>> +
>> +     for (round = 0; round < KECCAK_ROUNDS; round++) {
>> +
>> +             /* Theta */
>> +             for (i = 0; i < 5; i++)
>> +                     bc[i] = st[i] ^ st[i + 5] ^ st[i + 10] ^ st[i + 15]
>> +                             ^ st[i + 20];
>> +
>> +             for (i = 0; i < 5; i++) {
>> +                     t = bc[(i + 4) % 5] ^ ROTL64(bc[(i + 1) % 5], 1);
>> +                     for (j = 0; j < 25; j += 5)
>> +                             st[j + i] ^= t;
>> +             }
>> +
>> +             /* Rho Pi */
>> +             t = st[1];
>> +             for (i = 0; i < 24; i++) {
>> +                     j = keccakf_piln[i];
>> +                     bc[0] = st[j];
>> +                     st[j] = ROTL64(t, keccakf_rotc[i]);
>> +                     t = bc[0];
>> +             }
>> +
>> +             /* Chi */
>> +             for (j = 0; j < 25; j += 5) {
>> +                     for (i = 0; i < 5; i++)
>> +                             bc[i] = st[j + i];
>> +                     for (i = 0; i < 5; i++)
>> +                             st[j + i] ^= (~bc[(i + 1) % 5]) &
>> +                                          bc[(i + 2) % 5];
>> +             }
>> +
>> +             /* Iota */
>> +             st[0] ^= keccakf_rndc[round];
>> +     }
>> +}
>> +
>> +static void sha3_init(struct sha3_state *sctx, unsigned int digest_sz)
>> +{
>> +     memset(sctx, 0, sizeof(*sctx));
>> +     sctx->md_len = digest_sz;
>> +     sctx->rsiz = 200 - 2 * digest_sz;
>> +     sctx->rsizw = sctx->rsiz / 8;
>> +}
>> +
>> +static int sha3_224_init(struct shash_desc *desc)
>> +{
>> +     struct sha3_state *sctx = shash_desc_ctx(desc);
>> +
>> +     sha3_init(sctx, SHA3_224_DIGEST_SIZE);
>> +     return 0;
>> +}
>> +
>> +static int sha3_256_init(struct shash_desc *desc)
>> +{
>> +     struct sha3_state *sctx = shash_desc_ctx(desc);
>> +
>> +     sha3_init(sctx, SHA3_256_DIGEST_SIZE);
>> +     return 0;
>> +}
>> +
>> +static int sha3_384_init(struct shash_desc *desc)
>> +{
>> +     struct sha3_state *sctx = shash_desc_ctx(desc);
>> +
>> +     sha3_init(sctx, SHA3_384_DIGEST_SIZE);
>> +     return 0;
>> +}
>> +
>> +static int sha3_512_init(struct shash_desc *desc)
>> +{
>> +     struct sha3_state *sctx = shash_desc_ctx(desc);
>> +
>> +     sha3_init(sctx, SHA3_512_DIGEST_SIZE);
>> +     return 0;
>> +}
>> +
>> +static int sha3_update(struct shash_desc *desc, const u8 *data,
>> +                    unsigned int len)
>> +{
>> +     struct sha3_state *sctx = shash_desc_ctx(desc);
>> +     unsigned int done;
>> +     const u8 *src;
>> +
>> +     done = 0;
>> +     src = data;
>> +
>> +     if ((sctx->partial + len) > (sctx->rsiz - 1)) {
>> +             if (sctx->partial) {
>> +                     done = -sctx->partial;
>> +                     memcpy(sctx->buf + sctx->partial, data,
>> +                            done + sctx->rsiz);
>> +                     src = sctx->buf;
>> +             }
>> +
>> +             do {
>> +                     unsigned int i;
>> +
>> +                     for (i = 0; i < sctx->rsizw; i++)
>> +                             sctx->st[i] ^= ((u64 *) src)[i];
>> +                     keccakf(sctx->st);
>> +
>> +                     done += sctx->rsiz;
>> +                     src = data + done;
>> +             } while (done + (sctx->rsiz - 1) < len);
>> +
>> +             sctx->partial = 0;
>> +     }
>> +     memcpy(sctx->buf + sctx->partial, src, len - done);
>> +     sctx->partial += (len - done);
>> +
>> +     return 0;
>> +}
>> +
>> +static int sha3_final(struct shash_desc *desc, u8 *out)
>> +{
>> +     struct sha3_state *sctx = shash_desc_ctx(desc);
>> +     unsigned int i, inlen = sctx->partial;
>> +
>> +     sctx->buf[inlen++] = 0x06;
>> +     memset(sctx->buf + inlen, 0, sctx->rsiz - inlen);
>> +     sctx->buf[sctx->rsiz - 1] |= 0x80;
>> +
>> +     for (i = 0; i < sctx->rsizw; i++)
>> +             sctx->st[i] ^= ((u64 *) sctx->buf)[i];
>> +
>> +     keccakf(sctx->st);
>> +
>> +     for (i = 0; i < sctx->rsizw; i++)
>> +             sctx->st[i] = cpu_to_le64(sctx->st[i]);
>> +
>> +     memcpy(out, sctx->st, sctx->md_len);
>> +
>> +     memset(sctx, 0, sizeof(*sctx));
>> +     return 0;
>> +}
>> +
>> +static struct shash_alg sha3_224 = {
>> +     .digestsize     =       SHA3_224_DIGEST_SIZE,
>> +     .init           =       sha3_224_init,
>> +     .update         =       sha3_update,
>> +     .final          =       sha3_final,
>> +     .descsize       =       sizeof(struct sha3_state),
>> +     .base           =       {
>> +             .cra_name       =       "sha3-224",
>> +             .cra_driver_name =      "sha3-224-generic",
>> +             .cra_flags      =       CRYPTO_ALG_TYPE_SHASH,
>> +             .cra_blocksize  =       SHA3_224_BLOCK_SIZE,
>> +             .cra_module     =       THIS_MODULE,
>> +     }
>> +};
>> +
>> +static struct shash_alg sha3_256 = {
>> +     .digestsize     =       SHA3_256_DIGEST_SIZE,
>> +     .init           =       sha3_256_init,
>> +     .update         =       sha3_update,
>> +     .final          =       sha3_final,
>> +     .descsize       =       sizeof(struct sha3_state),
>> +     .base           =       {
>> +             .cra_name       =       "sha3-256",
>> +             .cra_driver_name =      "sha3-256-generic",
>> +             .cra_flags      =       CRYPTO_ALG_TYPE_SHASH,
>> +             .cra_blocksize  =       SHA3_256_BLOCK_SIZE,
>> +             .cra_module     =       THIS_MODULE,
>> +     }
>> +};
>> +
>> +static struct shash_alg sha3_384 = {
>> +     .digestsize     =       SHA3_384_DIGEST_SIZE,
>> +     .init           =       sha3_384_init,
>> +     .update         =       sha3_update,
>> +     .final          =       sha3_final,
>> +     .descsize       =       sizeof(struct sha3_state),
>> +     .base           =       {
>> +             .cra_name       =       "sha3-384",
>> +             .cra_driver_name =      "sha3-384-generic",
>> +             .cra_flags      =       CRYPTO_ALG_TYPE_SHASH,
>> +             .cra_blocksize  =       SHA3_384_BLOCK_SIZE,
>> +             .cra_module     =       THIS_MODULE,
>> +     }
>> +};
>> +
>> +static struct shash_alg sha3_512 = {
>> +     .digestsize     =       SHA3_512_DIGEST_SIZE,
>> +     .init           =       sha3_512_init,
>> +     .update         =       sha3_update,
>> +     .final          =       sha3_final,
>> +     .descsize       =       sizeof(struct sha3_state),
>> +     .base           =       {
>> +             .cra_name       =       "sha3-512",
>> +             .cra_driver_name =      "sha3-512-generic",
>> +             .cra_flags      =       CRYPTO_ALG_TYPE_SHASH,
>> +             .cra_blocksize  =       SHA3_512_BLOCK_SIZE,
>> +             .cra_module     =       THIS_MODULE,
>> +     }
>> +};
>
> Shouldn't there be a priority here?

Yes, I will fix it in next patch.

>> +
>> +static int __init sha3_generic_mod_init(void)
>> +{
>> +     int ret;
>> +
>> +     ret = crypto_register_shash(&sha3_224);
>> +     if (ret < 0)
>> +             goto err_out;
>> +     ret = crypto_register_shash(&sha3_256);
>> +     if (ret < 0)
>> +             goto err_out_224;
>> +     ret = crypto_register_shash(&sha3_384);
>> +     if (ret < 0)
>> +             goto err_out_256;
>> +     ret = crypto_register_shash(&sha3_512);
>> +     if (ret < 0)
>> +             goto err_out_384;
>> +
>> +     return 0;
>> +
>> +err_out_384:
>> +     crypto_unregister_shash(&sha3_384);
>> +err_out_256:
>> +     crypto_unregister_shash(&sha3_256);
>> +err_out_224:
>> +     crypto_unregister_shash(&sha3_224);
>> +err_out:
>> +     return ret;
>> +}
>> +
>> +static void __exit sha3_generic_mod_fini(void)
>> +{
>> +     crypto_unregister_shash(&sha3_224);
>> +     crypto_unregister_shash(&sha3_256);
>> +     crypto_unregister_shash(&sha3_384);
>> +     crypto_unregister_shash(&sha3_512);
>> +}
>> +
>> +module_init(sha3_generic_mod_init);
>> +module_exit(sha3_generic_mod_fini);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("SHA-3 Secure Hash Algorithm");
>> +
>> +MODULE_ALIAS("sha3-224");
>> +MODULE_ALIAS("sha3-256");
>> +MODULE_ALIAS("sha3-384");
>> +MODULE_ALIAS("sha3-512");
>
> MODULE_ALIAS_CRYPTO?
>
> What about the aliases for cra_driver_name?

Yes, I will fix it in next patch.

>> diff --git a/include/crypto/sha3.h b/include/crypto/sha3.h
>> new file mode 100644
>> index 0000000..f4c9f68
>> --- /dev/null
>> +++ b/include/crypto/sha3.h
>> @@ -0,0 +1,29 @@
>> +/*
>> + * Common values for SHA-3 algorithms
>> + */
>> +#ifndef __CRYPTO_SHA3_H__
>> +#define __CRYPTO_SHA3_H__
>> +
>> +#define SHA3_224_DIGEST_SIZE (224 / 8)
>> +#define SHA3_224_BLOCK_SIZE  (200 - 2 * SHA3_224_DIGEST_SIZE)
>> +
>> +#define SHA3_256_DIGEST_SIZE (256 / 8)
>> +#define SHA3_256_BLOCK_SIZE  (200 - 2 * SHA3_256_DIGEST_SIZE)
>> +
>> +#define SHA3_384_DIGEST_SIZE (384 / 8)
>> +#define SHA3_384_BLOCK_SIZE  (200 - 2 * SHA3_384_DIGEST_SIZE)
>> +
>> +#define SHA3_512_DIGEST_SIZE (512 / 8)
>> +#define SHA3_512_BLOCK_SIZE  (200 - 2 * SHA3_512_DIGEST_SIZE)
>> +
>> +struct sha3_state {
>> +     u64             st[25];
>> +     unsigned int    md_len;
>> +     unsigned int    rsiz;
>> +     unsigned int    rsizw;
>> +
>> +     unsigned int    partial;
>> +     u8              buf[SHA3_224_BLOCK_SIZE];
>> +};
>> +
>> +#endif
>
>
> Ciao
> Stephan