60f7f2de5de917c2bee32a4cd79cc3818b6a94a0 Don't assume that ALIGNMENT > 1 in tests (Tim Ruffing)
ada6361dece4265823478e0019a8c196e9285a26 Use ROUND_TO_ALIGN in scratch_create (Jonas Nick)
8ecc6ce50ead28a0b8bab2f1fb18a58ee5204a13 Add check preventing rounding to alignment from wrapping around in scratch_alloc (Jonas Nick)
4edaf06fb02a9ac9cd115e0c967bb0ef35cae01d Add check preventing integer multiplication wrapping around in scratch_max_allocation (Jonas Nick)
Pull request description:
This PR increases the general robustness of scratch spaces. It does not fix an existing vulnerability because scratch spaces aren't used anywhere in master. Additionally, it must be prevented anyway that an attacker has (indirect) control over the arguments touched in this PR.
ACKs for top commit:
sipa:
ACK 60f7f2de5de917c2bee32a4cd79cc3818b6a94a0
Tree-SHA512: ecdd794b55a01d1d6d24098f3abff34cb8bb6f33737ec4ec93714aa631c9d397b213cc3603a916ad79f4b09d6b2f8973bf87fc07b81b25a530cc72c4dbafaba9
0dccf98a21beb245f6cd9ed76fb7368529df09c7 Use preprocessor macros instead of autoconf to detect endianness (Tim Ruffing)
Pull request description:
This does not fix any particular issue but it's preferable to not
rely on autoconf. This avoids endianness mess for users on BE hosts
if they use their build without autoconf.
The macros are carefully written to err on the side of the caution,
e.g., we #error if the user manually configures a different endianness
than what we detect.
Supersedes #770 .
ACKs for top commit:
sipa:
ACK 0dccf98a21beb245f6cd9ed76fb7368529df09c7
gmaxwell:
ACK 0dccf98a21beb245f6cd9ed76fb7368529df09c7
Tree-SHA512: 6779458de5cb6eaef2ac37f9d4b8fa6c9b299f58f6e5b72f2b0d7e36c12ea06074e483acfb85085a147e0f4b51cd67d897f61a67250ec1cea284a0f7680eb2e8
This does not fix any particular issue but it's preferable to not
rely on autoconf. This avoids endianness mess for users on BE hosts
if they use their build without autoconf.
The macros are carefully written to err on the side of the caution,
e.g., we #error if the user manually configures a different endianness
than what we detect.
Instead of supporting configuration of the field and scalar size independently,
both are now controlled by the availability of a 64x64->128 bit multiplication
(currently only through __int128). This is autodetected from the C code through
__SIZEOF_INT128__, but can be overridden using configure's
--with-test-override-wide-multiply, or by defining
USE_FORCE_WIDEMUL_{INT64,INT128} manually.
So far this has not been needed, as it's only used by the static precomputation
which always builds with 32-bit fields.
This prepares for the ability to have __int128 detected on the C side, breaking
that restriction.
67a429f31fd3d1b37c5365cc58b70588b8645d62 Suppress a harmless variable-time optimization by clang in _int_cmov (Tim Ruffing)
5b196338f0c8dc07bf0eece37b46d8686c4da3ce Remove redundant "? 1 : 0" after comparisons in scalar code (Tim Ruffing)
Pull request description:
Attempt at resolving #771 .
This surprisingly seems to improve the situation at least for the compilers available on godbolt.
ACKs for top commit:
gmaxwell:
ACK 67a429f31fd3d1b37c5365cc58b70588b8645d62
elichai:
tACK 67a429f31fd3d1b37c5365cc58b70588b8645d62
Tree-SHA512: ee8b0c86831ec8c3d5a9abcad773ed8a0f267e5c47012e4e1423b10a64c26b4cf6e3c466c3df765ba7e636787a3fe134d633926d67b599287f12c51be924f478
37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0 Remove unnecessary sign variable from wnaf_const (Jonas Nick)
6bb0b77e158fc2f9e56e4b65b08bcb660d4c588b Fix test_constant_wnaf for -1 and add a test for it. (Jonas Nick)
Pull request description:
There currently is a single branch in the `ecmul_const` function that is not being exercised by the tests. This branch is unreachable and therefore I'm suggesting to remove it.
For your convenience the paper the wnaf algorithm can be found [here (The Width-w NAF Method Provides Small Memory and Fast Elliptic Scalar Multiplications Secure against Side Channel Attacks)](http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.563.1267&rep=rep1&type=pdf). Similarly, unless I'm missing something important, I don't see how their algorithm needs to consider `sign(u[i-1])` unless `d` can be negative - which doesn't make much sense to me either.
ACKs for top commit:
real-or-random:
ACK 37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0 I verified the correctness of the change and claimed invariant by manual inspection. I tested the code, both with 32bit and 64bit scalars.
Tree-SHA512: 9db45f76bd881d00a81923b6d2ae1c3e0f49a82a5d55347f01e1ce4e924d9a3bf55483a0697f25039c327e33edca6796ba3205c068d9f2f99aa5d655e46b15be
The comments with 'XXX' was intended to indicate that the listed
concerns was subject to review and change, but the code with these
comments was merged straight away. This commit replaces comments
with more complete text describing the issues.
This also signifies that the commit that this code was introduced in is
not anymore 'work in progress'.
