It takes ownership of the buffer, so it's not actually const. The
const-ness gets dropped once it transits through EVP_PKEY_CTX_ctrl.
Also compare against INT_MAX explicitly for the overflow check. I'm not sure
whether the casting version is undefined, but comparing against INT_MAX matches
the rest of the codebase when transiting in and out of signed ints.
Change-Id: I131165a4b5f0ebe02c6db3e7e3e0d1af5b771710
Reviewed-on: https://boringssl-review.googlesource.com/6850
Reviewed-by: Adam Langley <alangley@gmail.com>
It's never used. It's not clear why one would want such a thing.
EVP_PKEY_CTX has no way for callers to register callbacks, which means
there shouldn't be a way for the library to present you an EVP_PKEY_CTX
out-of-context. (Whereas app_data/ex_data makes sense on SSL because of
its numerous callbacks or RSA because of RSA_METHOD.)
Change-Id: I55af537ab101682677af34f6ac1f2c27b5899a89
Reviewed-on: https://boringssl-review.googlesource.com/6849
Reviewed-by: Adam Langley <alangley@gmail.com>
This removes the last caller of EVP_PKEY_copy_parameters within the
library.
Change-Id: I6af138d364973b18f52baf55c36c50a24a56bd44
Reviewed-on: https://boringssl-review.googlesource.com/6848
Reviewed-by: Adam Langley <alangley@gmail.com>
foo_init hooks are never implemented. Even upstream never uses them. The
flags member is also never used. We also don't expose paramgen, so
remove it.
Change-Id: I51d9439316c5163520ab7168693c457f33e59417
Reviewed-on: https://boringssl-review.googlesource.com/6846
Reviewed-by: Adam Langley <alangley@gmail.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>
There's many ways to serialize a BIGNUM, so not including asn1 in the name is
confusing (and collides with BN_bn2cbb_padded). Since BN_asn12bn looks
ridiculous, match the parse/marshal naming scheme of other modules instead.
Change-Id: I53d22ae0537a98e223ed943e943c48cb0743cf51
Reviewed-on: https://boringssl-review.googlesource.com/6822
Reviewed-by: Adam Langley <alangley@gmail.com>
The code was using `#define INLINE` instead, but we have `inline` so
use it.
Change-Id: Id05eaec4720061c5d9a7278e20127c2bebcb2495
Reviewed-on: https://boringssl-review.googlesource.com/6976
Reviewed-by: David Benjamin <davidben@google.com>
Commit 75a64c08fc63ee9619214be5607a48ae9c97fdcc missed one case where
the GCC syntax should have been replaced with |alignas|.
Change-Id: Iebdaa9c9a2c0aff171f0b5d4daac607e351a4b7e
Reviewed-on: https://boringssl-review.googlesource.com/6974
Reviewed-by: David Benjamin <davidben@google.com>
The uses of |memcpy| to cast pointer-to-function to pointer-to-data and
back again did not have well-defined semantics. Use a union instead to
avoid the need for such a conversion get well-defined semantics.
Change-Id: I8ee54a83ba75440f7bc78c194eb55e2cf09b05d8
Reviewed-on: https://boringssl-review.googlesource.com/6972
Reviewed-by: David Benjamin <davidben@google.com>
Casting a pointer-to-non-volatile to pointer-to-volatile can be a no-op
as the compiler only requires volatile semantics when the pointed-to
object is a volatile object and there are no pointers-to-non-volatile
involved. This probably doesn't matter unless building with the MSVC
-volatile:iso flag, and maybe not even then, but it is good practice
anyway.
Change-Id: I94900d3dc61de3b8ce2ddecab2811907a9a7adbf
Reviewed-on: https://boringssl-review.googlesource.com/6973
Reviewed-by: David Benjamin <davidben@google.com>
Division isn't constant-time on Intel chips so the code was adding a
large multiple of md_size to try and force the operation to always take
the maximum amount of time.
I'm less convinced, these days, that compilers aren't going to get smart
enough to optimise that away so use Barrett reduction instead.
Change-Id: Ib8c514192682a2fcb4b1fb7e7c6dd1301d9888d0
Reviewed-on: https://boringssl-review.googlesource.com/6906
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
OpenSSL 1.1.0 doesn't seem to have these two, so this isn't based on anything.
Have them return uint64_t in preparation for switching the internal
representation to uint64_t so ssl_record_sequence_update can go away.
Change-Id: I21d55e9a29861c992f409ed293e0930a7aaef7a3
Reviewed-on: https://boringssl-review.googlesource.com/6941
Reviewed-by: Adam Langley <alangley@gmail.com>
We have the hook on the SSL_CTX, but it should be possible to set it without
reaching into SSL_CTX.
Change-Id: I93db070c7c944be374543442a8de3ce655a28928
Reviewed-on: https://boringssl-review.googlesource.com/6880
Reviewed-by: Adam Langley <alangley@gmail.com>
They should use the same P-256 check.
Change-Id: I66dd63663e638cba35b8f70f9cf119c718af4aec
Reviewed-on: https://boringssl-review.googlesource.com/6845
Reviewed-by: Adam Langley <alangley@gmail.com>
Move it into ssl->s3 so it automatically behaves correctly on SSL_clear.
ssl->version is still a mess though.
Change-Id: I17a692a04a845886ec4f8de229fa6cf99fa7e24a
Reviewed-on: https://boringssl-review.googlesource.com/6844
Reviewed-by: Adam Langley <alangley@gmail.com>
For TLS, this machinery only exists to swallow no_certificate alerts
which only get sent in an SSL 3.0 codepath anyway. It's much less a
no-op for SSL 3.0 which, strictly speaking, has only a subset of TLS's
alerts.
This gets messy around version negotiation because of the complex
relationship between enc_method, have_version, and version which all get
set at different times. Given that SSL 3.0 is nearly dead and all these
alerts are fatal to the connection anyway, this doesn't seem worth
carrying around. (It doesn't work very well anyway. An SSLv3-only server
may still send a record_overflow alert before version negotiation.)
This removes the last place enc_method is accessed prior to version
negotiation.
Change-Id: I79a704259fca69e4df76bd5a6846c9373f46f5a9
Reviewed-on: https://boringssl-review.googlesource.com/6843
Reviewed-by: Adam Langley <alangley@gmail.com>
This removes the various non-PRF checks from SSL3_ENC_METHOD so that can
have a clearer purpose. It also makes TLS 1.0 through 1.2's
SSL3_ENC_METHOD tables identical and gives us an assert to ensure
nothing accesses the version bits before version negotiation.
Accordingly, ssl_needs_record_splitting was reordered slightly so we
don't rely on enc_method being initialized to TLS 1.2
pre-version-negotiation.
This leaves alert_value as the only part of SSL3_ENC_METHOD which may be
accessed before version negotiation.
Change-Id: If9e299e2ef5511b5fa442b2af654eed054c3e675
Reviewed-on: https://boringssl-review.googlesource.com/6842
Reviewed-by: Adam Langley <alangley@gmail.com>
The default implementation of |from_be_u64| was badly broken. It
obviously was never tested and we don't have an easy way to test it.
For now, just assume |bswap_u64| is provided unless/until we encounter
a platform where that isn't true.
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.
Changes:
∙ Support verify_recover. This is very obscure and the motivation
appears to be https://github.com/nodejs/node/issues/477 – where it's
not clear that anyone understands what it means :(
∙ Add a few, no-op #defines
∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
reach into these structs in the future.
∙ Add EC_get_builtin_curves.
∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
decrepit.
Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
In code, structs that happened to have a '(' somewhere in their body
would cause the parser to go wrong. This change fixes that and updates
the comments on a number of structs.
Change-Id: Ia76ead266615a3d5875b64a0857a0177fec2bd00
Reviewed-on: https://boringssl-review.googlesource.com/6970
Reviewed-by: Adam Langley <agl@google.com>
We haven't had problems with this, but make sure it stays that way.
Bogus signature algorithms are already covered.
Change-Id: I085350d89d79741dba3f30fc7c9f92de16bf242a
Reviewed-on: https://boringssl-review.googlesource.com/6910
Reviewed-by: David Benjamin <davidben@google.com>
Conscrypt needs to, in the certificate verification callback, know the key
exchange + auth method of the current cipher suite to pass into
X509TrustManager.checkServerTrusted. Currently it reaches into the struct to
get it. Add an API for this.
Change-Id: Ib4e0a1fbf1d9ea24e0114f760b7524e1f7bafe33
Reviewed-on: https://boringssl-review.googlesource.com/6881
Reviewed-by: David Benjamin <davidben@google.com>
Apparently OpenSSL's API is made entirely of initialization functions.
Some external libraries like to initialize with OPENSSL_config instead.
Change-Id: I28efe97fc5eb21309f560c84112b80e947f8bb17
Reviewed-on: https://boringssl-review.googlesource.com/6981
Reviewed-by: Adam Langley <agl@google.com>
With these stubs, cURL should not need any BoringSSL #ifdefs at all,
except for their OCSP #ifdefs (which can switch to the more generally
useful OPENSSL_NO_OCSP) and the workaround for wincrypt.h macro
collisions. That we intentionally leave to the consumer rather than add
a partial hack that makes the build sensitive to include order.
(I'll send them a patch upstream once this cycles in.)
Change-Id: I815fe67e51e80e9aafa9b91ae68867ca1ff1d623
Reviewed-on: https://boringssl-review.googlesource.com/6980
Reviewed-by: Adam Langley <agl@google.com>
Since the error string logic was rewritten, this hasn't done anything.
Change-Id: Icb73dca65e852bb3c7d04c260d591906ec72c15f
Reviewed-on: https://boringssl-review.googlesource.com/6961
Reviewed-by: Adam Langley <agl@google.com>
MSVC doesn't have stdalign.h and so doesn't support |alignas| in C
code. Define |alignas(x)| as a synonym for |__decltype(align(x))|
instead for it.
This also fixes -Wcast-qual warnings in rsaz_exp.c.
Change-Id: Ifce9031724cb93f5a4aa1f567e7af61b272df9d5
Reviewed-on: https://boringssl-review.googlesource.com/6924
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
After its initial assignment, |e| is immediately reassigned another
value and so the initial assignment from |BN_CTX_get| is useless. If
that were not the case, then the |BN_free(e)| at the end of the
function would be very bad.
Change-Id: Id63a172073501c8ac157db9188a22f55ee36b205
Reviewed-on: https://boringssl-review.googlesource.com/6951
Reviewed-by: David Benjamin <davidben@google.com>
Also remove the now-redundant `bssl_` part of the names of the things
inside the module.
This should help avoid confusion in the event we create a ring-ffi
crate.