Re: [PATCH crypto 2/2] lib/crypto: blake2s: move hmac construction into wireguard

[Ard Biesheuvel <ardb@kernel.org>]

On Tue, 11 Jan 2022 at 14:49, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Basically nobody should use blake2s in an HMAC construction; it already
> has a keyed variant. But for unfortunately historical reasons, Noise,

-ly

> used by WireGuard, uses HKDF quite strictly, which means we have to use
> this. Because this really shouldn't be used by others, this commit moves
> it into wireguard's noise.c locally, so that kernels that aren't using
> WireGuard don't get this superfluous code baked in. On m68k systems,
> this shaves off ~314 bytes.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: netdev@vger.kernel.org
> Cc: wireguard@lists.zx2c4.com
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  drivers/net/wireguard/noise.c | 45 ++++++++++++++++++++++++++++++-----
>  include/crypto/blake2s.h      |  3 ---
>  lib/crypto/blake2s-selftest.c | 31 ------------------------
>  lib/crypto/blake2s.c          | 37 ----------------------------
>  4 files changed, 39 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c
> index c0cfd9b36c0b..720952b92e78 100644
> --- a/drivers/net/wireguard/noise.c
> +++ b/drivers/net/wireguard/noise.c
> @@ -302,6 +302,41 @@ void wg_noise_set_static_identity_private_key(
>                 static_identity->static_public, private_key);
>  }
>
> +static void hmac(u8 *out, const u8 *in, const u8 *key, const size_t inlen, const size_t keylen)
> +{
> +       struct blake2s_state state;
> +       u8 x_key[BLAKE2S_BLOCK_SIZE] __aligned(__alignof__(u32)) = { 0 };
> +       u8 i_hash[BLAKE2S_HASH_SIZE] __aligned(__alignof__(u32));
> +       int i;
> +
> +       if (keylen > BLAKE2S_BLOCK_SIZE) {
> +               blake2s_init(&state, BLAKE2S_HASH_SIZE);
> +               blake2s_update(&state, key, keylen);
> +               blake2s_final(&state, x_key);
> +       } else
> +               memcpy(x_key, key, keylen);
> +
> +       for (i = 0; i < BLAKE2S_BLOCK_SIZE; ++i)
> +               x_key[i] ^= 0x36;
> +
> +       blake2s_init(&state, BLAKE2S_HASH_SIZE);
> +       blake2s_update(&state, x_key, BLAKE2S_BLOCK_SIZE);
> +       blake2s_update(&state, in, inlen);
> +       blake2s_final(&state, i_hash);
> +
> +       for (i = 0; i < BLAKE2S_BLOCK_SIZE; ++i)
> +               x_key[i] ^= 0x5c ^ 0x36;
> +
> +       blake2s_init(&state, BLAKE2S_HASH_SIZE);
> +       blake2s_update(&state, x_key, BLAKE2S_BLOCK_SIZE);
> +       blake2s_update(&state, i_hash, BLAKE2S_HASH_SIZE);
> +       blake2s_final(&state, i_hash);
> +
> +       memcpy(out, i_hash, BLAKE2S_HASH_SIZE);
> +       memzero_explicit(x_key, BLAKE2S_BLOCK_SIZE);
> +       memzero_explicit(i_hash, BLAKE2S_HASH_SIZE);
> +}
> +
>  /* This is Hugo Krawczyk's HKDF:
>   *  - https://eprint.iacr.org/2010/264.pdf
>   *  - https://tools.ietf.org/html/rfc5869
> @@ -322,14 +357,14 @@ static void kdf(u8 *first_dst, u8 *second_dst, u8 *third_dst, const u8 *data,
>                  ((third_len || third_dst) && (!second_len || !second_dst))));
>
>         /* Extract entropy from data into secret */
> -       blake2s256_hmac(secret, data, chaining_key, data_len, NOISE_HASH_LEN);
> +       hmac(secret, data, chaining_key, data_len, NOISE_HASH_LEN);
>
>         if (!first_dst || !first_len)
>                 goto out;
>
>         /* Expand first key: key = secret, data = 0x1 */
>         output[0] = 1;
> -       blake2s256_hmac(output, output, secret, 1, BLAKE2S_HASH_SIZE);
> +       hmac(output, output, secret, 1, BLAKE2S_HASH_SIZE);
>         memcpy(first_dst, output, first_len);
>
>         if (!second_dst || !second_len)
> @@ -337,8 +372,7 @@ static void kdf(u8 *first_dst, u8 *second_dst, u8 *third_dst, const u8 *data,
>
>         /* Expand second key: key = secret, data = first-key || 0x2 */
>         output[BLAKE2S_HASH_SIZE] = 2;
> -       blake2s256_hmac(output, output, secret, BLAKE2S_HASH_SIZE + 1,
> -                       BLAKE2S_HASH_SIZE);
> +       hmac(output, output, secret, BLAKE2S_HASH_SIZE + 1, BLAKE2S_HASH_SIZE);
>         memcpy(second_dst, output, second_len);
>
>         if (!third_dst || !third_len)
> @@ -346,8 +380,7 @@ static void kdf(u8 *first_dst, u8 *second_dst, u8 *third_dst, const u8 *data,
>
>         /* Expand third key: key = secret, data = second-key || 0x3 */
>         output[BLAKE2S_HASH_SIZE] = 3;
> -       blake2s256_hmac(output, output, secret, BLAKE2S_HASH_SIZE + 1,
> -                       BLAKE2S_HASH_SIZE);
> +       hmac(output, output, secret, BLAKE2S_HASH_SIZE + 1, BLAKE2S_HASH_SIZE);
>         memcpy(third_dst, output, third_len);
>
>  out:
> diff --git a/include/crypto/blake2s.h b/include/crypto/blake2s.h
> index bc3fb59442ce..4e30e1799e61 100644
> --- a/include/crypto/blake2s.h
> +++ b/include/crypto/blake2s.h
> @@ -101,7 +101,4 @@ static inline void blake2s(u8 *out, const u8 *in, const u8 *key,
>         blake2s_final(&state, out);
>  }
>
> -void blake2s256_hmac(u8 *out, const u8 *in, const u8 *key, const size_t inlen,
> -                    const size_t keylen);
> -
>  #endif /* _CRYPTO_BLAKE2S_H */
> diff --git a/lib/crypto/blake2s-selftest.c b/lib/crypto/blake2s-selftest.c
> index 5d9ea53be973..409e4b728770 100644
> --- a/lib/crypto/blake2s-selftest.c
> +++ b/lib/crypto/blake2s-selftest.c
> @@ -15,7 +15,6 @@
>   * #include <stdio.h>
>   *
>   * #include <openssl/evp.h>
> - * #include <openssl/hmac.h>
>   *
>   * #define BLAKE2S_TESTVEC_COUNT       256
>   *
> @@ -58,16 +57,6 @@
>   *     }
>   *     printf("};\n\n");
>   *
> - *     printf("static const u8 blake2s_hmac_testvecs[][BLAKE2S_HASH_SIZE] __initconst = {\n");
> - *
> - *     HMAC(EVP_blake2s256(), key, sizeof(key), buf, sizeof(buf), hash, NULL);
> - *     print_vec(hash, BLAKE2S_OUTBYTES);
> - *
> - *     HMAC(EVP_blake2s256(), buf, sizeof(buf), key, sizeof(key), hash, NULL);
> - *     print_vec(hash, BLAKE2S_OUTBYTES);
> - *
> - *     printf("};\n");
> - *
>   *     return 0;
>   *}
>   */
> @@ -554,15 +543,6 @@ static const u8 blake2s_testvecs[][BLAKE2S_HASH_SIZE] __initconst = {
>      0xd6, 0x98, 0x6b, 0x07, 0x10, 0x65, 0x52, 0x65, },
>  };
>
> -static const u8 blake2s_hmac_testvecs[][BLAKE2S_HASH_SIZE] __initconst = {
> -  { 0xce, 0xe1, 0x57, 0x69, 0x82, 0xdc, 0xbf, 0x43, 0xad, 0x56, 0x4c, 0x70,
> -    0xed, 0x68, 0x16, 0x96, 0xcf, 0xa4, 0x73, 0xe8, 0xe8, 0xfc, 0x32, 0x79,
> -    0x08, 0x0a, 0x75, 0x82, 0xda, 0x3f, 0x05, 0x11, },
> -  { 0x77, 0x2f, 0x0c, 0x71, 0x41, 0xf4, 0x4b, 0x2b, 0xb3, 0xc6, 0xb6, 0xf9,
> -    0x60, 0xde, 0xe4, 0x52, 0x38, 0x66, 0xe8, 0xbf, 0x9b, 0x96, 0xc4, 0x9f,
> -    0x60, 0xd9, 0x24, 0x37, 0x99, 0xd6, 0xec, 0x31, },
> -};
> -
>  bool __init blake2s_selftest(void)
>  {
>         u8 key[BLAKE2S_KEY_SIZE];
> @@ -607,16 +587,5 @@ bool __init blake2s_selftest(void)
>                 }
>         }
>
> -       if (success) {
> -               blake2s256_hmac(hash, buf, key, sizeof(buf), sizeof(key));
> -               success &= !memcmp(hash, blake2s_hmac_testvecs[0], BLAKE2S_HASH_SIZE);
> -
> -               blake2s256_hmac(hash, key, buf, sizeof(key), sizeof(buf));
> -               success &= !memcmp(hash, blake2s_hmac_testvecs[1], BLAKE2S_HASH_SIZE);
> -
> -               if (!success)
> -                       pr_err("blake2s256_hmac self-test: FAIL\n");
> -       }
> -
>         return success;
>  }
> diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
> index 93f2ae051370..9364f79937b8 100644
> --- a/lib/crypto/blake2s.c
> +++ b/lib/crypto/blake2s.c
> @@ -30,43 +30,6 @@ void blake2s_final(struct blake2s_state *state, u8 *out)
>  }
>  EXPORT_SYMBOL(blake2s_final);
>
> -void blake2s256_hmac(u8 *out, const u8 *in, const u8 *key, const size_t inlen,
> -                    const size_t keylen)
> -{
> -       struct blake2s_state state;
> -       u8 x_key[BLAKE2S_BLOCK_SIZE] __aligned(__alignof__(u32)) = { 0 };
> -       u8 i_hash[BLAKE2S_HASH_SIZE] __aligned(__alignof__(u32));
> -       int i;
> -
> -       if (keylen > BLAKE2S_BLOCK_SIZE) {
> -               blake2s_init(&state, BLAKE2S_HASH_SIZE);
> -               blake2s_update(&state, key, keylen);
> -               blake2s_final(&state, x_key);
> -       } else
> -               memcpy(x_key, key, keylen);
> -
> -       for (i = 0; i < BLAKE2S_BLOCK_SIZE; ++i)
> -               x_key[i] ^= 0x36;
> -
> -       blake2s_init(&state, BLAKE2S_HASH_SIZE);
> -       blake2s_update(&state, x_key, BLAKE2S_BLOCK_SIZE);
> -       blake2s_update(&state, in, inlen);
> -       blake2s_final(&state, i_hash);
> -
> -       for (i = 0; i < BLAKE2S_BLOCK_SIZE; ++i)
> -               x_key[i] ^= 0x5c ^ 0x36;
> -
> -       blake2s_init(&state, BLAKE2S_HASH_SIZE);
> -       blake2s_update(&state, x_key, BLAKE2S_BLOCK_SIZE);
> -       blake2s_update(&state, i_hash, BLAKE2S_HASH_SIZE);
> -       blake2s_final(&state, i_hash);
> -
> -       memcpy(out, i_hash, BLAKE2S_HASH_SIZE);
> -       memzero_explicit(x_key, BLAKE2S_BLOCK_SIZE);
> -       memzero_explicit(i_hash, BLAKE2S_HASH_SIZE);
> -}
> -EXPORT_SYMBOL(blake2s256_hmac);
> -
>  static int __init blake2s_mod_init(void)
>  {
>         if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) &&
> --
> 2.34.1
>