| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
| |
This is another bit of indirection that makes this code so hard to follow.
ok jsing
|
| |
|
| |
|
|
|
|
|
|
|
| |
This is usually method specific, so remove the indirection and call the
appropriate blinding function directly.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is only used by ec_points_make_affine(), which is only used by the
wNAF multiplication, which is only used by ECDSA. We can afford computing
that one once per ECDSA verification given the cost of the rest of this.
Thus, the field_set_to_one() member disappears from the EC_METHOD and the
mont_one member disappears from EC_GROUP and with it all the complications
when setting/copying/freeing the group.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
That the BN-driven EC code uses Jacobian projective coordinates as an
optimization is an implementation detail. As such this should never have
leaked out of the library as part of the public API. No consumer should
ever care and if they do they're doing it wrong. The only port that cares
is one of those stupid little perl modules that expose all the things and
transform terrible OpenSSL regress tests into similarly horrible Perl.
In practice, only affine coordinates matter (perhaps in compressed form).
This prunes two more function pointers from EC_GROUP and prepares the
removal of the field_set_to_one() method which is now only used in
ec_points_make_affine().
ok jsing sthen
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The only way to get an EC_GROUP or an EC_POINT is by calling the relevant
_new() function and to get rid of it, something must call _free(). Thus we
can establish the invariant that every group has Weierstrass coefficients
p, a, b as well as order and cofactor hanging off it. Similarly, Every
point has allocated BIGNUMs for its Jacobian projective coordinates.
Unfortunately, a group has the generator as an optional component in
addition to seed and montgomery context/one (where optionality makes
more sense).
This is a mostly mechanical diff and only drops a few silly comments and
a couple of unnecessary NULL checks since in our part of the wrold the
word invariant has a meaning.
This should also appease Coverity who likes to throw fits at calling
BN_free() for BIGNUM on the stack (yes, this is actually a thing).
ok jsing
|
|
|
|
|
|
|
| |
There is only one caller, EC_GROUP_free(), so inline the relevant free
calls there and dispose of a few layers of indirection.
ok jsing
|
|
|
|
|
|
|
|
| |
For both in-tree methods these are just complicated ways of zeroing part
of the group object. The group is allocated with calloc(), so it's all
entirely pointless.
ok jsing
|
| |
|
|
|
|
| |
ok jsing
|
|
|
|
|
| |
The shift is between 0 and 5 bits, so it doesn't matter, but VS is short
for very st...ubborn as are its users when it comes to reporting non-issues
|
|
|
|
|
|
|
|
|
|
|
| |
This had an extra dance to allow a NULL output buffer. The plan was to
use this in i2o_ECPublicKey() to preserve the behavior of avoiding an
allocation if out == NULL. However, when I rewrote the latter I punted
on preserving that complication, as it was already batshit crazy enough.
Thus, remove said dance and make ec_point_to_octets() cleaner.
ok jsing
|
| |
|
|
|
|
|
|
| |
Now that we only do curves over GF(p) fields, there's no need to use a
weird, confusing name for what we usually call p. Adjust some comments
in the vicinity as well.
|
| |
|
| |
|
|
|
|
| |
discussed with jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This refactors the wNAF multiplication further and introduces a small API
that manages the wNAF digits for bn and the multiples of digit * point in
a single struct that is initialized and freed in two API calls in the main
function, ec_wNAF_mul(). This way the main algorithm is no longer cluttered
with logic to keep various arrays in sync, helper functions calculating the
wNAF splitting of bn and multiples of the point do not need to deal with
memory management, and a pair of accessors obviates previously missing
bounds checking.
At this point we have reached a relatively clean and straightforward wNAF
implementation that fits precisely the purpose needed in libcrypto, i.e.,
ECDSA verification instead of being generalized and optimized to the max
for no good reason apart from endowing the author with an academic degree.
Popper's famous maxim "if you can't say it clearly, keep quiet, and keep
working until you can" very much applies to code as well. In other words,
shut up and hack (and don't pour too much energy into commit messages, tb).
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We match curve parameters against the builtin curves and only accept
them if they're encoding a curve known to us. After getting rid of the
wtls curves, some of which used to coincide with secp curves (sometimes
the wrong ones), the nid is unambiguous. Setting the nid has no direct
implications on the encoding.
This helps ssh avoid doing ugly computations during the key exchange
for PEM keys using this encoding.
ok djm joshua jsing
|
|
|
|
|
|
|
| |
This should really have been using SECP 160R2, not SECP 160R1. Of course
this means in particular that nobody ever used this curve, at least not
against another implementation than OpenSSL. Quasi-monocultures are
poisonous whether the monopolist is benevolent and competent or not.
|
|
|
|
|
|
|
|
|
| |
Like most of the "group" methods these are shared between Montgomery
curves and simple curves. There's no point in five methods hanging off
the EC_METHODS struct whne they can just as well be inlined in the
public API. It makes all files involved shorter...
ok jsing
|
|
|
|
|
|
|
|
| |
While there likely won't be enough BNs already available in the ctx, and
thus it won't greatly reduce the amount of allocated BNs, it simplifies
the exit path quite a bit.
review feedback from jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It is unclear how the original code was supposed to work. It clearly
missed a few corner cases (like handling points at infinity correctly)
and the badly mangled comment that was supposed to display a binary
search tree didn't help at all.
Instead do something much more straightforward: multiply all the non-zero
Z coordinates of the points not at infinity together, keeping track of the
intermediate products. Then do a single expensive modular inversion before
working backwards to compute all the inverses. Then the transformation from
Jacobian coordinates to affine coordiantes (x, y, z) -> (x/z^2, y/z^3, 1)
becomes cheap. A little bit of care has to be taken for Montgomery curves
but that's very simple compared to the mess that was there before.
ok jsing
This is a cleaned up version of:
commit 0fe73d6c3641cb175871463bdddbbea3ee0b62ae
Author: Bodo Moeller <bodo@openssl.org>
Date: Fri Aug 1 17:18:14 2014 +0200
Simplify and fix ec_GFp_simple_points_make_affine
(which didn't always handle value 0 correctly).
Reviewed-by: emilia@openssl.org
|
|
|
|
|
|
|
|
|
|
|
| |
There are only two flag values that libcrypto understands and the default
value is 1 while, helpfully, the undesirable non-default is 0. The few
existing callers set OPENSSL_EC_NAMED_CURVE or OPENSSL_EC_EXPLICIT_CURVE.
Nevertheless, the flag should be checked properly as a flag. The recent
upstream checks for EC_GROUP_get_asn1_flag(group) == OPENSSL_EC_NAMED_CURVE
don't look right either...
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This disables all the curves over fields < 224 bits and a few others.
Specifically:
SECG: 112r1 112r2 128r1 128r2 160k1 160r1 160r2 192k1 192r1 192v{1,2,3}
WTLS: 6 7 8 9 12
Brainpool: P160r1 P160t1 P192r1 P192t1
These are below or at the limit of what is acceptable nowadays. This is
less aggressive than what some enterprise linux distributions are using
in their patched OpenSSL versions where everything over fields < 256 bits
is disabled with the exception of P-224, so interoperability should not
be a problem.
The curves are left in the tree for now and can be re-enabled by compiling
libcrypto with -DENABLE_SMALL_CURVES. They will be fully removed later.
One nice benefit of doing this is that the incorrect parameters for WTLS 7
are fixed (obviously nobody uses this one) and now all the builtin curves
have a unique corresponding OID (nid).
Something like this was suggested a while back by beck, makes sense to sthen
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The big change is that the "rows" are no longer slices of val[] but
that they actually own the points they contain. The price for this
is an extra allocation for val[] and to piece it together from the
two rows. That's ugly, but less ugly than before.
Add a helper for freeing a row of points. It can deal with a NULL
row so, we can remove a couple of complications.
The second change is that the logic for preparing the rows is pulled
back into ec_wNAF_mul[]. This way the m * G + n * P logic is in the
one function that needs to know about it, the rest just deals with
a pair of a point and a scalar.
This starts resembling actual code...
ok jsing
|
|
|
|
|
|
|
|
| |
This is a corner case that isn't really of interest. We're making a few
calculations that don't really hurt, but it's super cheap, so one more
complication bites the dust.
ok jsing
|
|
|
|
|
|
|
|
| |
We can now turn the for loop into a proper for loop for which there is
obviously no out of bounds access. The length can be determined up front
and it's easier to explain what's going on, so expand a few comments.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
| |
This is another micro optimization that introduces needless complications
for the sake of saving a few cycles. Specifically, by ditching the rule
defining the wNAF representation (at most one of w+1 consecutive digits
is non-zero) for the topmost digits, one can sometimes save a few digits
at the cost of crazy loop conditions and other weirdness. That's not worth
it.
ok jsing
|
| |
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
| |
It's still horrible, but slightly less so...
ok jsing
|
|
|
|
|
|
|
|
| |
This streamlines this mess and adapts the API better to its only caller.
Nothing much going on here, except that we drop confusing checks and
unhelpful comment, thereby making the algorithm more cleanly visible.
ok jsing
|
|
|
|
|
|
| |
This matches the ec_wNAF_mul() API better
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As its name indicates, the first, ec_compute_odd_multiples(), fills
point, 3 * point, 5 * point, ..., (2 * len - 1) * point into row[].
In fact, it first computes doubled = 2 * point and then goes on to
set row[i] = row[i - 1] + doubled. That's straightforward enough. One
change here is that this helper allocates row[i] on the fly rather
than preallocating the entire array of points up front.
The second piece is the actual precomputation, ec_wNAF_precompute().
It first computes the wNAF digits of the two scalars n and m (in this
order for now) with appropriate window size and length. Then the above
mentioned val[] array is allocated and populated with odd multiples
of point and generator. Finally, all points in val[] are made affine
in a single step, which means we only need one modular inversion, and
this then allows us to take fast paths in all the computations in the
one remaining loop in ec_wNAF_mul().
ok jsing
|
|
|
|
|
|
| |
This used to be the case until they were given a 'more meaningful name'
about 20 years ago. We cant fix the public API, but I'm tired of being
confused by this nonsense.
|
| |
|
| |
|
|
|
|
|
|
|
| |
Again, we know their sizes (always 2), so we can avoid allocating and
freeing them. Also remove the extra "pivot" element. It's not needed.
ok djm
|
|
|
|
| |
pointed out by jsing
|
|
|
|
| |
ok djm
|
|
|
|
|
|
| |
This makes the mess a bit more readable.
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
All the EC_POINT_* API has a fast path for the point at infinity. So we're
not gaining more than a few cycles by making this terrible mess even more
terrible than it already is by avoding calls ot it (it's also incorrect as
it is since we don't know that the point is no longer at infinity when it
is unset). Simplify and add a comment explaining what this mess is doing.
ok jsing
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Use better variable names (cf. https://jmilne.org/math/tips.html#4) and
avoid the weird style of assigning to r (what does r stand for anyway?)
and short circuiting subsequent tests using if (r || ...). Also, do not
reuse the variables for order and cofactor that were previously used for
the curve coefficients.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The only caller passes in num = 1 and is itself called in a path that
ensures that the multiplier of the generator is != NULL. Consequently
we don't need to deal with an array of points and an array of scalars
so rename them accordingly.
In addition, the change implies that numblocks and num_scalar are now
always 1, so inline this information and take a first step towards
disentangling this gordian knot.
ok jsing
|
| |
|