Messages in this thread | | | Date | Mon, 22 Mar 2021 17:51:06 -0700 | From | Eric Biggers <> | Subject | 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 <arnd@kernel.org> wrote: > > > > 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 <ebiggers@google.com>
- Eric
| |