|ECDH_compute_key_ex| provides a safer interface with a better
implementation. In particular, the error checking in
|ECDH_compute_key_ex| seems better.
After recent changes, |EC_KEY_set_public_key| is now dead code.
|EC_KEY| is immutable, though note that |EC_KEY| instances reference
|EC_GROUP| instances that aren't immutable. Now operations that take
|EC_KEY*| parameters use the private key, whereas operations that use
only public keys do not take |EC_KEY*| or |const EC_KEY*| parameters.
This gives some test coverage to |ECDSA_verify_signed_digest| and
|ECDSA_do_verify_point|. The new functions do not require the caller to
construct an |EC_KEY| and now there are no callers of
|EC_KEY_set_public_key|.
Importantly, this also removes |ECDSA_verify_signed_digest|'s indirect
dependency on the |CBB_*| code, which reduces code size for
applications that don't have other dependencies on |CBB_*|.
The |EC_KEY_generate_key_ex| API is less error-prone and the
implementation becomes simpler if we don't have to worry about also
implementing |EC_KEY_generate_key|.
This function only does per-group precomputation, not key-specific
precomputation, so using it is no better than using
|EC_GROUP_precompute_mult|. Worse, an unsuspecting caller may not
realize that this is modifying a possibly-shared |EC_GROUP|, so it
seems dangerous.
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.
1. Check for the presence of the private key before allocating or
computing anything.
2. Check the return value of |BN_CTX_get|.
3. Don't bother computing the Y coordinate since it is not used.
4. |OPENSSL_cleanse| the temporary buffer.
5. Remove conditional logic in cleanup section.
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|.
BUF_memdup tries to avoid mallocing zero bytes (and thus unduly
returning an error for a NULL return value) by testing whether the input
buffer is NULL. This goes back to the original OpenSSL code.
However, when |ext_npn_parse_serverhello| tries to use |BUF_memdup| to
copy an NPN value returned by a callback, some callbacks just set the
output /length/ to zero to indicate an empty value. Thus, when
|BUF_memdup| tests the pointer, it's an uninitialised value and MSan
throws an error.
Since passing a NULL pointer to |BUF_memdup| better imply that the
length is zero, while the reverse empirically isn't true, testing the
length seems safer.
Change-Id: I06626f7dfb761de631fd997bda60057b76b8da94
Bazel on Mac requires some alterations to the generated build files.
This change updates generate_build_files.py to emit suitable Bazel
files. This will require some tweaks to projects that build with Bazel.
Change-Id: I3d68ec754b8abaa41a348f86c32434477f2c5e1c
Reviewed-on: https://boringssl-review.googlesource.com/6146
Reviewed-by: Adam Langley <agl@google.com>
This removes the confusion about whether |gcm128_context| copies the
key (it didn't) or whether the caller is responsible for keeping the
key alive for the lifetime of the |gcm128_context| (it was).
The key is never modified through the key pointer member, and the
calling code relies on that fact for maintaining its own
const-correctness.
Change-Id: I63946451aa7c400cd127895a61c30d9a647b1b8c
The test cases for other sizes of nonces were removed from gcm_test.c.
Since the new API doesn't provide any way of providing a non-96-bit
nonce, there's no way of testing those cases at this level. Similar
scenerios are tested in the AES-GCM tests.
Previously a value of 0 would be accepted and intepreted as equivalent
to 1. This contradicts RFC 2898 which defines:
iterationCount INTEGER (1..MAX),
BUG=https://crbug.com/534961
Change-Id: I89623980f99fde3ca3780880d311955d3f6fe0b5
Reviewed-on: https://boringssl-review.googlesource.com/5971
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Some code relies on OpenSSL's behavior where it allowed for NULL. But this time
add a comment so people don't think this is the convention for new functions.
BUG=538292
Change-Id: I66566e0e24566fafe17e05369276248be3b05591
Reviewed-on: https://boringssl-review.googlesource.com/6070
Reviewed-by: Adam Langley <agl@google.com>