jedisct1 / libsodium

A modern, portable, easy to use crypto library.
https://libsodium.org
Other
12.08k stars 1.72k forks source link

Calculate carry on most significant byte in crypto_core_ed25519_scalar_add #1129

Closed johnoliverdriscoll closed 2 years ago

johnoliverdriscoll commented 2 years ago

Not sure if the original behavior was intended, but it seems like a bug. If the sum of two scalars is large enough to overflow into a 33-byte serialization, the output was not correct according to sc_muladd in https://github.com/orlp/ed25519

Changing the len parameter passed into sodium_add to crypto_core_ed25519_NONREDUCEDSCALARBYTES produces the same results as sc_muladd.`

jedisct1 commented 2 years ago

Hi!

The crypto_core_ed25519_scalar_*() functions expect non-reduced scalars to be in the [0..L[ interval, L being the group order ~2^255.

These are low-level functions, and applications are expected to perform a reduction, if necessary, before using these functions.

A sum will never exceed 32 bits. We can process an extra bit in the ed25519_scalar_add() function; it wouldn't cost much. But I'm not even sure that other functions such as sub() and mul() can cope with inputs larger than the group size. That was not the intent and you should assume that this is undefined.

This is also why the scalar_is_canonical() function exists. If your input is untrusted and may be larger than the group order, you should call it prior to performing any arithmetic involving that input.