I'm not sure why I separated "fixed" and "quick_ctx" names. That's
annoying and doesn't generalize well to, say, adding a bn_div_consttime
function for RSA keygen.
Change-Id: I751d52b30e079de2f0d37a952de380fbf2c1e6b7
Reviewed-on: https://boringssl-review.googlesource.com/26364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
As the EC code will ultimately want to use these in "words" form by way
of EC_FELEM, and because it's much easier, I've implement these as
low-level words-based functions that require all inputs have the same
width. The BIGNUM versions which RSA and, for now, EC calls are
implemented on top of that.
Unfortunately, doing such things in constant-time and accounting for
undersized inputs requires some scratch space, and these functions don't
take BN_CTX. So I've added internal bn_mod_*_quick_ctx functions that
take a BN_CTX and the old functions now allocate a bit unnecessarily.
RSA only needs lshift (for BN_MONT_CTX) and sub (for CRT), but the
generic EC code wants add as well.
The generic EC code isn't even remotely constant-time, and I hope to
ultimately use stack-allocated EC_FELEMs, so I've made the actual
implementations here implemented in "words", which is much simpler
anyway due to not having to take care of widths.
I've also gone ahead and switched the EC code to these functions,
largely as a test of their performance (an earlier iteration made the EC
code noticeably slower). These operations are otherwise not
performance-critical in RSA.
The conversion from BIGNUM to BIGNUM+BN_CTX should be dropped by the
static linker already, and the unused BIGNUM+BN_CTX functions will fall
off when EC_FELEM happens.
Update-Note: BN_mod_*_quick bounce on malloc a bit now, but they're not
really used externally. The one caller I found was wpa_supplicant
which bounces on malloc already. They appear to be implementing
compressed coordinates by hand? We may be able to convince them to
call EC_POINT_set_compressed_coordinates_GFp.
Bug: 233, 236
Change-Id: I2bf361e9c089e0211b97d95523dbc06f1168e12b
Reviewed-on: https://boringssl-review.googlesource.com/25261
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This has no behavior change, but it has a semantic one. This CL is an
assertion that all BIGNUM functions tolerate non-minimal BIGNUMs now.
Specifically:
- Functions that do not touch top/width are assumed to not care.
- Functions that do touch top/width will be changed by this CL. These
should be checked in review that they tolerate non-minimal BIGNUMs.
Subsequent CLs will start adjusting the widths that BIGNUM functions
output, to fix timing leaks.
Bug: 232
Change-Id: I3a2b41b071f2174452f8d3801bce5c78947bb8f7
Reviewed-on: https://boringssl-review.googlesource.com/25257
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The order (and later the field) are used to size stack-allocated fixed
width word arrays. They're also entirely public, so this is fine.
Bug: 232
Change-Id: Ie98869cdbbdfea92dcad64a300f7e0b47bef6bf2
Reviewed-on: https://boringssl-review.googlesource.com/25256
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
p224-64.c can just write straight into the EC_POINT, as the other files
do, which saves the mess around BN_CTX. It's also more correct.
ec_point_set_Jprojective_coordinates_GFp abstracts out field_encode, but
then we would want to abstract out field_decode too when reading.
That then allows us to inline ec_point_set_Jprojective_coordinates_GFp
into ec_GFp_simple_point_set_affine_coordinates and get rid of an
unnecessary tower of helper functions. Also we can use the precomputed
value of one rather than recompute it each time.
Change-Id: I8282dc66a4a437f5a3b6a1a59cc39be4cb71ccf9
Reviewed-on: https://boringssl-review.googlesource.com/24687
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Id12ab478b6ba441fb1b6f4c2f9479384fc3fbdb6
Reviewed-on: https://boringssl-review.googlesource.com/23144
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Currently we only check that the underlying EC_METHODs match, which
avoids the points being in different forms, but not that the points are
on the same curves. (We fixed the APIs early on so off-curve EC_POINTs
cannot be created.)
In particular, this comes up with folks implementating Java's crypto
APIs with ECDH_compute_key. These APIs are both unfortunate and should
not be mimicked, as they allow folks to mismatch the groups on the two
multiple EC_POINTs. Instead, ECDH APIs should take the public value as a
byte string.
Thanks also to Java's poor crypto APIs, we must support custom curves,
which makes this particularly gnarly. This CL makes EC_GROUP_cmp work
with custom curves and adds an additional subtle requirement to
EC_GROUP_set_generator.
Annoyingly, this change is additionally subtle because we now have a
reference cycle to hack around.
Change-Id: I2efbc4bd5cb65fee5f66527bd6ccad6b9d5120b9
Reviewed-on: https://boringssl-review.googlesource.com/22245
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I really need to resurrect the CL to make them entirely static
(https://crbug.com/boringssl/20), but, in the meantime, to make
replacing the EC_METHOD pointer in EC_POINT with EC_GROUP not
*completely* insane, make them refcounted.
OpenSSL did not do this because their EC_GROUPs are mutable
(EC_GROUP_set_asn1_flag and EC_GROUP_set_point_conversion_form). Ours
are immutable but for the two-function dance around custom curves (more
of OpenSSL's habit of making their objects too complex), which is good
enough to refcount.
Change-Id: I3650993737a97da0ddcf0e5fb7a15876e724cadc
Reviewed-on: https://boringssl-review.googlesource.com/22244
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.
Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The names in the P-224 code collided with the P-256 code and thus many
of the functions and constants in the P-224 code have been prefixed.
Change-Id: I6bcd304640c539d0483d129d5eaf1702894929a8
Reviewed-on: https://boringssl-review.googlesource.com/15847
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
