Niels Möller <[email protected]> writes:

>>> My take was that it would be nice to add sntrup761 to Nettle ASAP to
>>> stabilize API and establish support for the algorithm -- we can optimize
>>> or improve the implementation later on (there are many optimized
>>> implementations around for different architectures out there).
>>
>> Makes sense, if it's clear what api and abi should look like (but, e.g.,
>> use of union nettle_block16 does affect the abi, I think).
>
> Having a closer look at the header file defining the api. I see no abi
> subtleties here, only naming nits.
>
>   * sntrup761_keypair: should be sntrup761_generate_keypair, for consistency.

+1

>   * sntrup761_enc, sntrup761_dec: Maybe abbreviate less, is
>     _encapsulate and _decapsulate too much? Or is _enc and _dec really
>     established in the area?

I think _enc and _dec are easy to confuse with _encrypt and _decrypt,
which would be unfortunate.

I like _encapsulate and _decapsulate.

>   * SNTRUP761_PUBLICKEY_SIZE: I think it would be more consistent with
>     an underscore, _PUBLIC_KEY_SIZE.

+1

>   * SNTRUP761_SECRETKEY_SIZE: I prefer SNTRUP761_PRIVATE_KEY_SIZE is
>     more consistent (maybe "private key" is not modern, but it's the
>     terminology used for all other asymmetric algorithms in nettle).

+1

>   * SNTRUP761_CIPHERTEXT_SIZE: Probably right, even though I'm a bit
>     confused by the "ciphertext" terminology when there's no
>     corresponding plaintext.

Yeah... I think this is actually an area that could do more work, since
the output is combined but maybe some consumers could want to split it
up.  This could be fixed later, and there may be good reasons to NOT
expose that internal structure.  The terminology is a bit unclear if the
key is included in ciphertext or not.

>   * SNTRUP761_SIZE: This needs a more specific name, maybe _SECRET_SIZE,
>     _SHARED_SECRET_SIZE, _SESSION_KEY_SIZE, _OUTPUT_SIZE, ...?

I think the acronym 'ss' is widely used for PQC KEM output secrets, so
+1 for SNTRUP761_SHARED_SECRET_SIZE.

The term 'session' confuses KEM's with D-H key agreement, and I think
these are somewhat different animals to warrant different terminology.

> In your docs, I noticed a copy-paste error in the docs for
> SNTRUP761_PUBLICKEY_SIZE.

\o/

> Things I think are desirable to do before merging an initial version:
> Agree on naming. Rename the single-lower-case-letter macros in the .c
> file to macro-like names. Add valgrind-based tests of side-channel silence.

Agreed, let's get those fixed.

/Simon

Attachment: signature.asc
Description: PGP signature

_______________________________________________
nettle-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to