OpenSSL upstream did a bulk reformat. We still have some files that have
the old OpenSSL style and this makes applying patches to them more
manual, and thus more error-prone, than it should be.
This change is the result of running
util/openssl-format-source -v -c .
in the enumerated directories. A few files were in BoringSSL style and
have not been touched.
This change should be formatting only; no semantic difference.
Change-Id: I75ced2970ae22b9facb930a79798350a09c5111e
Reviewed-on: https://boringssl-review.googlesource.com/6904
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The function X509_verify_cert checks the value of |ctx->chain| at the
beginning, and if it is NULL then it initialises it, along with the value
of |ctx->untrusted|. The normal way to use X509_verify_cert() is to first
call X509_STORE_CTX_init(); then set up various parameters etc; then call
X509_verify_cert(); then check the results; and finally call
X509_STORE_CTX_cleanup(). The initial call to X509_STORE_CTX_init() sets
|ctx->chain| to NULL. The only place in the OpenSSL codebase where
|ctx->chain| is set to anything other than a non NULL value is in
X509_verify_cert itself. Therefore the only ways that |ctx->chain| could be
non NULL on entry to X509_verify_cert is if one of the following occurs:
1) An application calls X509_verify_cert() twice without re-initialising
in between.
2) An application reaches inside the X509_STORE_CTX structure and changes
the value of |ctx->chain| directly.
With regards to the second of these, we should discount this - it should
not be supported to allow this.
With regards to the first of these, the documentation is not exactly
crystal clear, but the implication is that you must call
X509_STORE_CTX_init() before each call to X509_verify_cert(). If you fail
to do this then, at best, the results would be undefined.
Calling X509_verify_cert() with |ctx->chain| set to a non NULL value is
likely to have unexpected results, and could be dangerous. This commit
changes the behaviour of X509_verify_cert() so that it causes an error if
|ctx->chain| is anything other than NULL (because this indicates that we
have not been initialised properly). It also clarifies the associated
documentation.
(Imported from upstream's 692f07c3e0c04180b56febc2feb57cd94395a7a2.)
Change-Id: I971f1a305f12bbf9f4ae955313d5557368f0d374
Reviewed-on: https://boringssl-review.googlesource.com/6760
Reviewed-by: Adam Langley <agl@google.com>
This callback is never used. The one caller I've ever seen is in Android
code which isn't built with BoringSSL and it was a no-op.
It also doesn't actually make much sense. A callback cannot reasonably
assume that it sees every, say, SSL_CTX created because the index may be
registered after the first SSL_CTX is created. Nor is there any point in
an EX_DATA consumer in one file knowing about an SSL_CTX created in
completely unrelated code.
Replace all the pointers with a typedef to int*. This will ensure code
which passes NULL or 0 continues to compile while breaking code which
passes an actual function.
This simplifies some object creation functions which now needn't worry
about CRYPTO_new_ex_data failing. (Also avoids bouncing on the lock, but
it's taking a read lock, so this doesn't really matter.)
BUG=391192
Change-Id: I02893883c6fa8693682075b7b130aa538a0a1437
Reviewed-on: https://boringssl-review.googlesource.com/6625
Reviewed-by: Adam Langley <agl@google.com>
There's a few things that will be kind of a nuisance and possibly not worth it
(crypto/asn1 dumps a lot of undeclared things, etc.). But it caught some
mistakes. Even without the warning, making sure to include the externs before
defining a function helps catch type mismatches.
Change-Id: I3dab282aaba6023e7cebc94ed7a767a5d7446b08
Reviewed-on: https://boringssl-review.googlesource.com/6484
Reviewed-by: Adam Langley <agl@google.com>
If get_issuer fails, some of these calls return rather than jumping to common
cleanup code.
Change-Id: Iacd59747fb11e9bfaae86f2eeed88798ee08203e
Reviewed-on: https://boringssl-review.googlesource.com/5711
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 25efcb44ac88ab34f60047e16a96c9462fad39c1 and
56353962e7da7e385c3d577581ccc3015ed6d1dc.)
Change-Id: I2ff22fc9da23868de02e6f31c50a3f1d0c6dec1a
Reviewed-on: https://boringssl-review.googlesource.com/5710
Reviewed-by: Adam Langley <agl@google.com>
In the ancient times, before ex_data and OpenSSL, SSLeay supported a
single app_data slot in various types. Later app_data begat ex_data, and
app_data was replaced by compatibility macros to ex_data index zero.
Today, app_data is still in use, but ex_data never reserved index zero
for app_data. This causes some danger where, if the first ex_data
registration did not use NULL callbacks, the registration's callbacks
would collide with app_data.
Instead, add an option to the types with app_data to reserve index zero.
Also switch SSL_get_ex_data_X509_STORE_CTX_idx to always return zero
rather than allocate a new one. It used to be that you used
X509_STORE_CTX_get_app_data. I only found one consumer that we probably
don't care about, but, to be safe and since it's easy, go with the
conservative option. (Although SSL_get_ex_data_X509_STORE_CTX_idx wasn't
guaranteed to alias app_data, in practice it always did. No consumer
ever calls X509_STORE_CTX_get_ex_new_index.)
Change-Id: Ie75b279d60aefd003ffef103f99021c5d696a5e9
Reviewed-on: https://boringssl-review.googlesource.com/5313
Reviewed-by: Adam Langley <agl@google.com>
Also tighten X509_cmp_time to reject more than three fractional
seconds in the time; and to reject trailing garbage after the offset.
CVE-2015-1789
(Imported from upstream's 9bc3665ac9e3c36f7762acd3691e1115d250b030)
Change-Id: I2091b2d1b691c177d58dc7960e2e7eb4c97b1f69
Reviewed-on: https://boringssl-review.googlesource.com/5124
Reviewed-by: Adam Langley <agl@google.com>
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.
Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
Instead, each module defines a static CRYPTO_EX_DATA_CLASS to hold the values.
This makes CRYPTO_cleanup_all_ex_data a no-op as spreading the
CRYPTO_EX_DATA_CLASSes across modules (and across crypto and ssl) makes cleanup
slightly trickier. We can make it do something if needbe, but it's probably not
worth the trouble.
Change-Id: Ib6f6fd39a51d8ba88649f0fa29c66db540610c76
Reviewed-on: https://boringssl-review.googlesource.com/4375
Reviewed-by: Adam Langley <agl@google.com>
Beyond generally eliminating unnecessary includes, eliminate as many
includes of headers that declare/define particularly error-prone
functionality like strlen, malloc, and free. crypto/err/internal.h was
added to remove the dependency on openssl/thread.h from the public
openssl/err.h header. The include of <stdlib.h> in openssl/mem.h was
retained since it defines OPENSSL_malloc and friends as macros around
the stdlib.h functions. The public x509.h, x509v3.h, and ssl.h headers
were not changed in order to minimize breakage of source compatibility
with external code.
Change-Id: I0d264b73ad0a720587774430b2ab8f8275960329
Reviewed-on: https://boringssl-review.googlesource.com/4220
Reviewed-by: Adam Langley <agl@google.com>
C4701 is "potentially uninitialized local variable 'buf' used". It
sometimes results in false positives, which can now be suppressed
using the macro OPENSSL_SUPPRESS_POTENTIALLY_UNINITIALIZED_WARNINGS.
Change-Id: I15068b5a48e1c704702e7752982b9ead855e7633
Reviewed-on: https://boringssl-review.googlesource.com/3160
Reviewed-by: Adam Langley <agl@google.com>
It's never called in outside code. This too seems to be a remnant of the DSA
PKIX optional parameter stuff. This is confirmed both by a removed comment and
by the brief documentation at http://www.umich.edu/~x509/ssleay/x509_pkey.html
RFC 5480 does not allow ECDSA keys to be missing parameters, so this logic is
incorrect for ECDSA anyway. It was also failing to check
EVP_PKEY_copy_parameters' return value. And that logic looks pretty suspect if
you have a chain made up multiple certificate types.
Change-Id: Id6c60659a0162356c7f3eae5c797047366baae1c
Reviewed-on: https://boringssl-review.googlesource.com/3485
Reviewed-by: Adam Langley <agl@google.com>
Reduces number of silly casts in OpenSSL code and likely most
applications. Consistent with (char *) for "peername" value from
X509_check_host() and X509_VERIFY_PARAM_get0_peername().
(Imported from upstream's e83c913723fac7432a7706812f12394aaa00e8ce.)
Change-Id: Id0fc11773a0cee8933978cd4bdbd8251fd7cfb5f
Pass address of X509_VERIFY_PARAM_ID peername to X509_check_host().
(Imported from upstream's 55fe56837a65ff505b492aa6aee748bf5fa91fec.)
Change-Id: Ic21bfb361b8eb25677c4c2175882fa95ea44fc31
(Imported from upstream's 8abffa4a73fcbf6536e0a42d736ed9211a8204ea,
9624b50d51de25bb2e3a72e81fe45032d80ea5c2 and
41e3ebd5abacfdf98461cdeb6fa97a4175b7aad3.)
Change-Id: Ic9099eb5704b19b4500229e89351371cc6184f9d
(This change is for a future change that increases the range of the
return values.)
(Imported from upstream's 3fc0b1edad0c75d7beb51fa77f63ffe817295e2c.)
Change-Id: I221d4ee0e90586f89f731e01ff4d813058173211
Just store NUL-terminated strings. This works better when we add
support for multiple hostnames.
(Imported from upstream's d93edc0aab98377f42dd19312248597a018a7889.)
Change-Id: Ib3bf8a8c654b829b4432782ba21ba55c3d4a0582
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>
One about a possible uninitialised variable (incorrect, but it's easier
to keep the compiler happy) and one warning about "const static" being
backwards.
Change-Id: Ic5976a5f0b48f32e09682e31b65d8ea1c27e5b88
Reviewed-on: https://boringssl-review.googlesource.com/2632
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.
(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)
This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.
Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
Avoid needing to manually increment the reference count and using the right
lock, both here and in Chromium.
Change-Id: If116ebc224cfb1c4711f7e2c06f1fd2c97af21dd
Reviewed-on: https://boringssl-review.googlesource.com/1415
Reviewed-by: Adam Langley <agl@google.com>
Fixes to host checking wild card support and add support for setting
host checking flags when verifying a certificate chain.
(Imported from upstream's a2219f6be36d12f02b6420dd95f819cf364baf1d)
When a chain is complete and ends in a trusted root checks are also performed
on the TA and the callback notified with ok==1. For consistency do the same for
chains where the TA is not self signed.
(Imported from upstream's b07e4f2f46fc286c306353d5e362cbc22c8547fb)
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
