Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html
Add OPENSSL_memcpy, etc., wrappers which avoid this problem.
BUG=23
Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left
everything in crypto/x509 and friends alone.
There's some instances in gcm.c that are non-trivial and pulled into a
separate CL for ease of review.
Change-Id: I6515804e3097f7e90855f1e7610868ee87117223
Reviewed-on: https://boringssl-review.googlesource.com/10801
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
BN_mod_inverse_odd was always being used on 64-bit platforms and was being used
for all curves with an order of 450 bits or smaller (basically, everything but
P-521). We generally don't care much about minor differences in the speed of
verifying signatures using curves other than P-256 and P-384. It is better to
always use the same algorithm.
This also allows |bn_mod_inverse_general|, |bn_mod_inverse_no_branch|, and
|BN_mod_inverse| to be dropped from programs that can somehow avoid linking in
the RSA key generation and RSA CRT recovery code.
Change-Id: I79b94bff23d2b07d5e0c704f7d44538797f8c7a0
Reviewed-on: https://boringssl-review.googlesource.com/9103
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>
It is only used in ec_montgomery.c, so move it there.
Change-Id: Ib189d5579d6363bdc1da89b775ad3df824129758
Reviewed-on: https://boringssl-review.googlesource.com/7584
Reviewed-by: David Benjamin <davidben@google.com>
The (internal) constant-time callers of this function already do a
constant-time reduction before calling. And, nobody should be calling
this function with out-of-range coordinates anyway. So, just require
valid coordinates as input.
Further, this function is rarely called, so don't bother with the
optimization to avoid encoding Montgomery encoding of 1 for the Z
coordinate.
Change-Id: I637ffaf4d39135ca17214915b9a8582ea052eea8
Reviewed-on: https://boringssl-review.googlesource.com/7441
Reviewed-by: David Benjamin <davidben@google.com>
Having |Z_is_one| be out of sync with |Z| could potentially be a very
bad thing, and in the past there have been multiple bugs of this sort,
including one currently in p256-x86_64.c (type confusion: Montgomery-
encoded vs unencoded). Avoid the issue entirely by getting rid of
|Z_is_one|.
Change-Id: Icb5aa0342df41d6bc443f15f952734295d0ee4ba
Reviewed-on: https://boringssl-review.googlesource.com/6576
Reviewed-by: David Benjamin <davidben@google.com>
Callers of this function are not checking for the -1 result. Change
the semantics to match their expectations and to match the common
semantics of most other parts of BoringSSL.
Change-Id: I4ec537d7619e20e8ddfee80c72125e4c02cfaac1
Reviewed-on: https://boringssl-review.googlesource.com/7125
Reviewed-by: David Benjamin <davidben@google.com>
These are never called. Group parameters are not secret anyway. This is
a remnant of upstream's EC_GROUP_clear_free.
Change-Id: I23a4076eae8e4561abddbe74d0ba72641532f229
Reviewed-on: https://boringssl-review.googlesource.com/6823
Reviewed-by: Adam Langley <alangley@gmail.com>
These functions ultimately return the result of |BN_num_bits|, and that
function's return type is |unsigned|. Thus, these functions' return
type should also be |unsigned|.
Change-Id: I2cef63e6f75425857bac71f7c5517ef22ab2296b
Reviewed-on: https://boringssl-review.googlesource.com/6170
Reviewed-by: Adam Langley <alangley@gmail.com>
If the application is only using the P-256 implementation in p256-64.c,
then the WNAF code would all be dead code. The change reorganizes the
code so that all modern toolchains should be able to recognize that
fact and eliminate the WNAF-based code when it is unused.
Change-Id: I9f94bd934ca7d2292de4c29bb89e17c940c7cd2a
Reviewed-on: https://boringssl-review.googlesource.com/6173
Reviewed-by: Adam Langley <alangley@gmail.com>
None of these methods vary per group. Factoring these out of
|EC_METHOD| should help some toolchains to do a better job optimizing
the code for size.
Change-Id: Ibd22a52992b4d549f12a8d22bddfdb3051aaa891
Reviewed-on: https://boringssl-review.googlesource.com/6172
Reviewed-by: Adam Langley <alangley@gmail.com>
The documentation for |BN_CTX_get| states: "Once |BN_CTX_get| has
returned NULL, all future calls will also return NULL until
|BN_CTX_end| is called." Some code takes advantage of that guarantee
by only checking the return value of the last call to |BN_CTX_get| in a
series of calls. That is correct and the most efficient way of doing
it. However, that pattern is inconsistent with most of the other uses
of |BN_CTX_get|. Also, static analysis tools like Coverity cannot
understand that pattern. This commit removes the instances of that
pattern that Coverity complained about when scanning *ring*.
Change-Id: Ie36d0223ea1caee460c7979547cf5bfd5fb16f93
Reviewed-on: https://boringssl-review.googlesource.com/5611
Reviewed-by: Adam Langley <agl@google.com>
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.
This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.
Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 267e6f3cc0ef78dea4e5cf93907a71556a45f008)
Change-Id: I99cfd0196b9625449c50494562c44f57f09fed17
Reviewed-on: https://boringssl-review.googlesource.com/2167
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Replace the tree-like structure by a linear approach, with fewer special
cases to handle value 0.
(Imported from upstream's d5213519c0ed87c71136084e7e843a4125ecc024.)
Change-Id: Icdd4815066bdbab0d2c0020db6a8cacc49b3d82a
Reviewed-on: https://boringssl-review.googlesource.com/1400
Reviewed-by: Adam Langley <agl@google.com>
Fix handling of points at infinity in ec_GFp_simple_points_make_affine.
When inverting an array of Z coordinates, the algorithm is supposed to
treat any 0 essentially like a 1 to remain in the multiplicative group;
however, for one of the cases, we incorrectly multiplied by 0 and thus
ended up with garbage.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
