Running the RNG is pointless if no seed is available because the key
will be fixed. The computation just wastes time.
Previously, users could avoid this computation at least by asking for
a context without signing capabilities. But since 3b0c218 we always
build an ecmult_gen context, ignoring the context flags. Moreover,
users could never avoid this pointless computation when asking for
the creation of a signing context.
This is a backwards-compatible API change: Before this commit, a context
initialized for signing was required to call functions that rely on
ecmult_gen. After this commit, this is no longer necessary because the
static ecmult_gen table is always present. In practice this means that
the corresponding functions will just work instead of calling the
illegal callback when given a context which is not (officially)
initialized for signing.
This is in line with 6815761, which made the analogous change with
respect to ecmult and contexts initialized for signing. But as opposed
to 681571, which removed the ecmult context entirely, we cannot remove
the ecmult_gen context entirely because it is still used for random
blinding. Moreover, since the secp256k1_context_no_precomp context is
const and cannot meaningfully support random blinding, we refrain (for
now) from changing its API, i.e., the illegal callback will still be
called when trying to use ecmult_gen operations with the static
secp256k1_context_no_precomp context.
There were several places where the code was non-constant time
for invalid secret inputs. These are harmless under sane use
but get in the way of automatic const-time validation.
(Nonce overflow in signing is not addressed, nor is s==0 in
signing)
make ECMULT_GEN_PREC_BITS configurable
ecmult_static_context.h: add compile time config assertion (#3) - Prevents accidentally using a file which was generated with a
different configuration.
README: mention valgrind issue
With --with-ecmult-gen-precision=8, valgrind needs a max stack size
adjustment to not run into a stack switching heuristic:
http://valgrind.org/docs/manual/manual-core.html
> -max-stackframe= [default: 2000000]
> The maximum size of a stack frame. If the stack pointer moves by more than this amount then Valgrind will assume that the program is switching to a different stack.
You may need to use this option if your program has large stack-allocated arrays.
basic-config: undef ECMULT_WINDOW_SIZE before (re-)defining it
Most of the codebase correctly used short-cutting to avoid calling
_is_zero on possibly incompletely initialized elements, but a few
places were missed.
Identifiers starting with an underscore and followed immediately by a capital letter are reserved by the C++ standard.
The only header guards not fixed are those in the headers auto-generated from java.
The side-effects make review somewhat harder because 99.9% of the
time the macro usage has no sideeffects, so they're easily ignored.
The main motivation for avoiding the side effects is so that the
macro can be completely stubbed out for branch coverage analysis
otherwise all the unreachable verify code gets counted against
coverage.
This makes it more clear that a null check is intended. Avoiding the
use of a pointer as a test condition alse increases the type-safety
of the comparisons.
(This is also MISRA C 2012 rules 14.4 and 11.9)
This vastly shrinks the size of the context required for signing on devices with
memory-mapped Flash.
Tables are generated by the new gen_context tool into a header.
* Make secp256k1_gej_add_var and secp256k1_gej_double return the
Z ratio to go from a.z to r.z.
* Use these Z ratios to speed up batch point conversion to affine
coordinates, and to speed up batch conversion of points to a
common Z coordinate.
* Add a point addition function that takes a point with a known
Z inverse.
* Due to secp256k1's endomorphism, all additions in the EC
multiplication code can work on affine coordinate (with an
implicit common Z coordinate), correcting the Z coordinate of
the result afterwards.
Refactoring by Pieter Wuille:
* Move more global-z logic into the group code.
* Separate code for computing the odd multiples from the code to bring it
to either storage or globalz format.
* Rename functions.
* Make all addition operations return Z ratios, and test them.
* Make the zr table format compatible with future batch chaining
(the first entry in zr becomes the ratio between the input and the
first output).
Original idea and code by Peter Dettman.
This computes (n-b)G + bG with random value b, in place of nG in
ecmult_gen() for signing.
This is intended to reduce exposure to potential power/EMI sidechannels
during signing and pubkey generation by blinding the secret value with
another value which is hopefully unknown to the attacker.
It may not be very helpful if the attacker is able to observe the setup
or if even the scalar addition has an unacceptable leak, but it has low
overhead in any case and the security should be purely additive on top
of the existing defenses against sidechannels.
Unbraced statements spanning multiple lines has been shown in many
projects to contribute to the introduction of bugs and a failure
to catch them in review, especially for maintenance on infrequently
modified code.
Most, but not all, of the existing practice in the codebase were not
cases that I would have expected to eventually result in bugs but
applying it as a rule makes it easier for other people to safely
contribute.
I'm not aware of any such evidence for the case with the statement
on a single line, but some people strongly prefer to never do that
and the opposite rule of "_always_ use a single line for single
statement blocks" isn't a reasonable rule for formatting reasons.
Might as well brace all these too, since that's more universally
acceptable.
[In any case, I seem to have introduced the vast majority of the
single-line form (as they're my preference where they fit).]
This also removes a broken test which is no longer needed.
C doesn't include the null in an array initilized from a
string literal if it doesn't fit, in C++ this is invalid.
The vararray style prototypes and init+calc also changed in
this commit are not C89 enough for some tools.
GCC (and clang) supports extensions to annotate functions so that their
results must be used and so that their arguments can't be statically
provable to be null. If a caller violates these requirements they
get a warning, so this helps them write correct code.
I deployed this in libopus a couple years ago with good success, and
the implementation here is basically copied straight from that.
One consideration is that the non-null annotation teaches the optimizer
and will actually compile out runtime non-nullness checks as dead-code.
Since this is usually not whats wanted, the non-null annotations are
disabled when compiling the library itself.
The commit also removes some dead inclusions of assert.h and introduces
compatibility macros for restrict and inline in preparation for some
portability improvements.
