Re: [PATCH] crypto: poly1305: fix poly1305_core_setkey() declaration

On Mon, Mar 22, 2021 at 07:51:47PM +0100, Ard Biesheuvel wrote:

On Mon, 22 Mar 2021 at 18:05, Arnd Bergmann [off-list ref] wrote:

quoted

From: Arnd Bergmann <arnd@arndb.de>

gcc-11 points out a mismatch between the declaration and the definition
of poly1305_core_setkey():

lib/crypto/poly1305-donna32.c:13:67: error: argument 2 of type ‘const u8[16]’ {aka ‘const unsigned char[16]’} with mismatched bound [-Werror=array-parameter=]
   13 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 raw_key[16])
      |                                                          ~~~~~~~~~^~~~~~~~~~~
In file included from lib/crypto/poly1305-donna32.c:11:
include/crypto/internal/poly1305.h:21:68: note: previously declared as ‘const u8 *’ {aka ‘const unsigned char *’}
   21 | void poly1305_core_setkey(struct poly1305_core_key *key, const u8 *raw_key);

This is harmless in principle, as the calling conventions are the same,
but the more specific prototype allows better type checking in the
caller.

Change the declaration to match the actual function definition.
The poly1305_simd_init() is a bit suspicious here, as it previously
had a 32-byte argument type, but looks like it needs to take the
16-byte POLY1305_BLOCK_SIZE array instead.

This looks ok to me. For historical reasons, the Poly1305 integration
is based on an unkeyed shash, and both the Poly1305 key and nonce are
passed as ordinary input, prepended to the actual data.
poly1305_simd_init() takes only the key but not the nonce, so it
should only be passed 16 bytes.

Well to be more precise, there are two conventions for using Poly1305.  One
where it is invoked many times with the same 16-byte key and different 16-byte
nonces.  And one where every invocation uses a unique key *and* nonce,
interpreted as a 32-byte "one-time key".

So that's why there's a mix of 16 and 32 byte "keys".

The naming "POLY1305_KEY_SIZE" assumes the second convention, which is a bit
confusing; it really should be called something like POLY1305_ONETIME_KEY_SIZE.
I guess the idea was that the one-time key convention is the more common one.

Anyway, the patch seems to be fine, as it uses the correct length in each
location.  You can add:

Reviewed-by: Eric Biggers <redacted>

- Eric