We want to reject small order points for two reasons:
- Those give rise to side channels attacks where the underlying field
arithmetic is not constant time (which our bignum isn't so far),
see the "May the Fourth" paper mentioned in ecp.
- Those force the shared secret to be all-bytes zero, which is bad for
protocols that might require contributory behaviour.
Honest peers will never send low-order points anyway.
While at it, test that we are clipping the input to 255 bits (ie, that
we are unsetting bit 256) as required by the RFC.
Again, honest peers will never send strings with bit 256 set. But the
RFC says to accept such strings anyway, so that's what we do.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Those no longer allocate, so they can no longer return that error. This
was missed in the commit that removed heap allocations.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Add comments explaining why 3 passes are enough (except for 224 where 1 is enough).
For p256 and p384, this is a bit different
from some other functions as the carry is signed here.
Rm FIPS reference from the documentation, it's about the implementation.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
On principle:
- If the code is written with a loop, the compiler can easily unroll the
loop at high optimisation levels; if we unroll the loop in the source,
the compiler is unlikely to roll it up at -Os/-Oz. So loops are more
adaptable.
- All the other functions are using loops, and there's nothing special
about those functions that would justify a different treatement.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The comments already explained why the result is in the desired range.
However 2^P521_WIDTH didn't make sense as P521_WIDTH is a number of
limbs no bits, so fix that typo.
Remove the FIPS reference as we're not following it and it doesn't bring
anything useful.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The test `if (X_Limbs > P255_WIDTH)` was useless: we know that X_limbs =
2 * P255_WIDTH.
The implementation already had an explicit argument why the result is
in range at the end.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The documentation used to say upon return X < N but that's wrong: if we
comment out the final conditional subtract in the test function
ecp_mod_p_generic_raw() then we have p448 cases that fail.
This was probably a typo. The actual guarantee is X < 2^448 < 2 * N.
Slightly optimize the code in the loop: we know that A1 = B0 is 1 limb,
so we don't need to memset() or memcpy() so many limbs.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The function-specific comments promised in the last line will follow.
FAST_QUASI_REDUCTION and NIST_QUASI_REDUCTION will be referenced by
specific functions. They're a single word to simplify looking them up.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
That's one 32-byte buffer and one 40-byte buffer (at most, when the
256-bit "Koblitz" curve is enabled).
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
With 64-bit limbs, we're using two 64-byte arrays on the stack.
With 32-bit limbs, that's two 60-byte arrays.
This is entirely acceptable, and much better than allocating all the
time in a performance-critical function.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The "shortcut" implementation was using the fact that this function is
called in a single place where we know that the top limb of A is zero.
But the function's documentation was not stating that as a requirement.
Align the documentation and "non-shortcut" implementation on the
"shortcut" implementation. This should be a tiny bit better for
performance: using the fact that A's top limb is zero means we have one
limb less to load from memory. (Copying is load + store, while setting
to 0 is only a store.)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The functions being tested are now used regardless of whether
MBEDTLS_ECP_WITH_MPI_UINT is defined, so they should also be tested
regardless of that option.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The comment about optimizing compiler's likely behaviour has been
checked with GCC and Clang on x86-64, 32-bit Arm and 64-bit Arm.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Using memcpy / memmove works only when:
- the size being copied is a multiple of the limb size -> 224 is a
multiple of 32 but not of 64;
- or the endianness is the same as the order we store limbs, which is
little-endian (that is, X[0] is the least significant limb).
Said otherwise, it breaks on big-endian 64-bit platforms.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The size checks were wrong, and the removed comment was misleading:
this is not about tests, it's just the normal way to wrap a "new bignum"
(aka "raw") function, that expects a fixed number of limbs, to a "legacy
bignum" function, that accepts any size (and allocates).
Also, this aligns ecp_mod_p448() and mbedtls_ecp_mod_p448_raw() with all
other ecp_mod_pXXX() and mbedtls_ecp_mod_pXXX_raw() functions.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is not really necessary for 1.0+, but for the 3.6 branch where the
structure and these constants are still public, it is cleaner.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The old functions only guaranteed an output in the range [-a * P, b * P]
for a and b "small enough".
The new functions have a stricter contract, allowing the final reduction
to be done efficiently in constant time.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Currently there's only one file that has two variants, and we are going
to merge it back, so variant checking no longer really makes sense.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Since __packed is a reserved keyword for IAR compilers, and
Zephyr defines it to attribute(__packed__), some typedef constructs
in mbedtls does not work with attribute(packed), only with the
keyword packed.
This fix temporary undefs the macro and restores it after the typedefs.
Signed-off-by: Lars-Ove Karlsson <lars-ove.karlsson@iar.com>
