On Thu, Dec 12, 2019 at 06:32:56PM +0100, Jason A. Donenfeld wrote:
> diff --git a/include/crypto/internal/poly1305.h b/include/crypto/internal/poly1305.h
> index 479b0cab2a1a..ad97819711eb 100644
> --- a/include/crypto/internal/poly1305.h
> +++ b/include/crypto/internal/poly1305.h
> @@ -11,11 +11,12 @@
> #include <crypto/poly1305.h>
>
> /*
> - * Poly1305 core functions. These implement the ε-almost-∆-universal hash
> + * Poly1305 core functions. These can implement the ε-almost-∆-universal hash
> * function underlying the Poly1305 MAC, i.e. they don't add an encrypted nonce
> - * ("s key") at the end. They also only support block-aligned inputs.
> + * ("s key") at the end, by passing NULL to nonce. They also only support
> + * block-aligned inputs.
> */
This comment is now ambiguous because the "i.e." clause wasn't updated to match
the change "implement" => "can implement".
This comment also wasn't updated when the 'hibit' argument was added earlier
anyway. So how about just rewriting it like:
/*
* Poly1305 core functions. These only accept whole blocks; the caller must
* handle any needed block buffering and padding. 'hibit' must be 1 for any
* full blocks, or 0 for the final block if it had to be padded. If 'nonce' is
* non-NULL, then it's added at the end to compute the Poly1305 MAC. Otherwise,
* only the ε-almost-∆-universal hash function (not the full MAC) is computed.
*/
> diff --git a/include/crypto/nhpoly1305.h b/include/crypto/nhpoly1305.h
> index 53c04423c582..a2f4e37b5107 100644
> --- a/include/crypto/nhpoly1305.h
> +++ b/include/crypto/nhpoly1305.h
> @@ -33,7 +33,7 @@
> #define NHPOLY1305_KEY_SIZE (POLY1305_BLOCK_SIZE + NH_KEY_BYTES)
>
> struct nhpoly1305_key {
> - struct poly1305_key poly_key;
> + struct poly1305_key poly_key[2];
> u32 nh_key[NH_KEY_WORDS];
> };
I still feel that this makes the code worse. Before, poly1305_key was an opaque
type that represented a Poly1305 key. After, users would need to know that an
array of 2 "keys" is needed, despite there actually being only one key.
Given that this even caused an actual bug in v1 of this series, how about going
with a less error-prone approach?
> +void poly1305_core_blocks(struct poly1305_state *state,
> + const struct poly1305_key *key, const void *src,
> + unsigned int nblocks, u32 hibit)
> +{
> + const u8 *input = src;
> + u32 r0, r1, r2, r3, r4;
> + u32 s1, s2, s3, s4;
> + u32 h0, h1, h2, h3, h4;
> + u64 d0, d1, d2, d3, d4;
> + u32 c;
> +
> + if (!nblocks)
> + return;
> +
> + hibit <<= 24;
> +
> + r0 = key[0].r[0];
> + r1 = key[0].r[1];
> + r2 = key[0].r[2];
> + r3 = key[0].r[3];
> + r4 = key[0].r[4];
> +
> + s1 = key[1].r[0];
> + s2 = key[1].r[1];
> + s3 = key[1].r[2];
> + s4 = key[1].r[3];
And some of the function prototypes, like this one, still give no indication
that 2 "keys" are needed...
- Eric
