MSVC doesn't like |const size_t len| in a function definition where the
declaration was just |size_t len| without the |const|. Also, MSVC needs
declarations of parameterless functions to have a |void| parameter list.
Change-Id: I91e01a12aca657b2ee1d653926f09cc52da2faed
Reviewed-on: https://boringssl-review.googlesource.com/4329
Reviewed-by: Adam Langley <agl@google.com>
OpenSSH, especially, does some terrible things that mean that it needs
the EVP_CIPHER structure to be exposed ☹. Damian is open to a better API
to replace this, but only if OpenSSL agree too. Either way, it won't be
happening soon.
Change-Id: I393b7a6af6694d4d2fe9ebcccd40286eff4029bd
Reviewed-on: https://boringssl-review.googlesource.com/4330
Reviewed-by: Adam Langley <agl@google.com>
This introduces a per-RSA/DSA/DH lock. This is good for lock contention,
although pthread locks are depressingly bloated.
Change-Id: I07c4d1606fc35135fc141ebe6ba904a28c8f8a0c
Reviewed-on: https://boringssl-review.googlesource.com/4324
Reviewed-by: Adam Langley <agl@google.com>
Prior to this, BoringSSL was using OpenSSL's technique of having users
register a callback for locking operation. This change adds native mutex
support.
Since mutexes often need to be in objects that are exposed via public
headers, the non-static mutexes are defined in thread.h. However, on
Windows we don't want to #include windows.h for CRITICAL_SECTION and, on
Linux, pthread.h doesn't define pthread_rwlock_t unless the feature
flags are set correctly—something that we can't control in general
for public header files. Thus, on both platforms, the mutex is defined
as a uint8_t[] of equal or greater size and we depend on static asserts
to ensure that everything works out ok.
Change-Id: Iafec17ae7e3422325e587878a5384107ec6647ab
Reviewed-on: https://boringssl-review.googlesource.com/4321
Reviewed-by: Adam Langley <agl@google.com>
It appears that this reference “count” is set to one at creation and
never touched after that.
Change-Id: I3238a6d3dd702953771b8ec725c1c5712c648fba
Reviewed-on: https://boringssl-review.googlesource.com/4320
Reviewed-by: Adam Langley <agl@google.com>
Avoiding superflous references to MD5 makes it easier to audit the code
to find unsafe uses of it. It also avoids subtly encouraging users to
choose MD5 instead of a better alternative.
Change-Id: Ic78eb5dfbf44aac39e4e4eb29050e3337c4445cc
Reviewed-on: https://boringssl-review.googlesource.com/3926
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>
`cmake -GNinja .. -DCMAKE_BUILD_TYPE=Release` fails without this
patch, when building using MSVC 2013.
MSVC will detect (in release builds only, it seems) that functions that
call abort will never return, and then warn that any code after a call
to one of them is unreachable. Since we treat warnings as errors when
building, this breaks the build. While this is usually desirable, it
isn't desirable in this case.
Change-Id: Ie5f24b1beb60fd2b33582a2ceef4c378ad0678fb
Reviewed-on: https://boringssl-review.googlesource.com/3960
Reviewed-by: Adam Langley <agl@google.com>
The only dependency the low-level crypto modules have on code in
crypto/obj is their use of OBJ_nid2sn, which is trivial to avoid.
This facilitates future simplification of crypto/obj, including
possibly the removal of functions like OBJ_nid2sn and the complex
build infrastructure that supports them.
This change also removes EVP_CIPHER_name and EVP_MD_name.
Change-Id: I34ce7dc7e58d5c08b52f95d25eba3963590cf2f7
Reviewed-on: https://boringssl-review.googlesource.com/3932
Reviewed-by: Adam Langley <agl@google.com>
Limiting uses of crypto/bio to code that really need to it by avoiding
the use of BIO just to write to stdout/stderr.
Change-Id: I34e0f773161aeec073691e439ac353fb7b1785f3
Reviewed-on: https://boringssl-review.googlesource.com/3930
Reviewed-by: Adam Langley <agl@google.com>
A previous change in BoringSSL renamed ERR_print_errors_fp to
BIO_print_errors_fp as part of refactoring the code to improve the
layering of modules within BoringSSL. Rename it back for better
compatibility with code that was using the function under the original
name. Move its definition back to crypto/err using an implementation
that avoids depending on crypto/bio.
Change-Id: Iee7703bb1eb4a3d640aff6485712bea71d7c1052
Reviewed-on: https://boringssl-review.googlesource.com/4310
Reviewed-by: Adam Langley <agl@google.com>
Avoiding superflous references to RC4 makes it easier to audit the code
to find unsafe uses of it. It also avoids subtly encouraging users to
choose RC4 instead of a better alternative.
Change-Id: Ia27d7f4cd465e143d30a28b36c7871f7c30411ea
Reviewed-on: https://boringssl-review.googlesource.com/3990
Reviewed-by: Adam Langley <agl@google.com>
Prior to this change, when EC_GROUP_get0_generator fails, BN_CTX_end
would get called even though BN_CTX_start hadn't been called yet, in
the case where the caller-supplied |ctx| is not NULL.
Change-Id: I6f728e74f0167193891cdb6f122b20b0770283dc
Reviewed-on: https://boringssl-review.googlesource.com/4271
Reviewed-by: Adam Langley <agl@google.com>
Now that ERR is using thread-local storage, there's very little that the
THREADID code is doing and it can be turned into stub functions.
Change-Id: I668613fec39b26c894d029b10a8173c3055f6019
Since ERR will soon have thread-local storage, we don't need to worry
about high-performance implementations and thus don't need to be able to
switch two different implementations at run-time.
Change-Id: I0598054ee8a8b499ac686ea635a96f5d03c754e0
Amazingly, asn1_GetSequence isn't completely unused? Keep that around for now
and ditch everything else. This lets us enable C4311 in MSVC which is actually
a pretty reasonable warning.
Change-Id: I43bb9206b1745e8a68224f3a435713d2a74e04ea
Reviewed-on: https://boringssl-review.googlesource.com/4256
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: If15f2f0e2b4627318c9cdfbc76d5ca56a6894e3f
Reviewed-on: https://boringssl-review.googlesource.com/4270
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
all_tests.go will still complain if tab_test is missing.
Change-Id: I97c3684a4397caa55aaae2ec6555b16ee8366233
Reviewed-on: https://boringssl-review.googlesource.com/4250
Reviewed-by: Adam Langley <agl@google.com>
Implement ECDSA_SIG_new and ECDSA_SIG_free manually in preparation for
removing all crypto/asn1 dependencies from ECDSA signature verification.
Change-Id: I0e84d74fa8e757af0cfb09daef03d59f428143cc
Reviewed-on: https://boringssl-review.googlesource.com/4153
Reviewed-by: Adam Langley <agl@google.com>
Sadly, it turns out that we have need of this, at least for now. The
code is taken from upstream and changed only as much as needed.
This only imports keys and doesn't know how to actually perform
operations on them for now.
Change-Id: I0db70fb938186cb7a91d03f068b386c59ed90b84
After sharding the session cache for fallbacks, the numbers have been pretty
good; 0.03% on dev and 0.02% on canary. Stable is at 0.06% but does not have
the sharded session cache. Before sharding, stable, beta, and dev had been
fairly closely aligned. Between 0.03% being low and the fallback saving us in
all but extremely contrived cases, I think this should be fairly safe.
Add tests for both the cipher suite and protocol version mismatch checks.
BUG=441456
Change-Id: I2374bf64d0aee0119f293d207d45319c274d89ab
Reviewed-on: https://boringssl-review.googlesource.com/3972
Reviewed-by: Adam Langley <agl@google.com>
The encoding of an INTEGER should not have leading zeros, except to pad for the
sign bit.
Change-Id: I80d22818cf1d2ca9d27e215620392e1725372aa5
Reviewed-on: https://boringssl-review.googlesource.com/4218
Reviewed-by: Adam Langley <agl@google.com>
Also check for overflow, although it really shouldn't happen.
Change-Id: I34dfe8eaf635aeaa8bef2656fda3cd0bad7e1268
Reviewed-on: https://boringssl-review.googlesource.com/4235
Reviewed-by: Adam Langley <agl@google.com>
Separate actually writing the fragment to the network from assembling it so
there is no need for is_fragment. record_split_done also needn't be a global;
as of 7fdeaf1101, it is always reset to 0 whether
or not SSL3_WANT_WRITE occurred, despite the comment.
I believe this is sound, but the pre-7fdeaf1 logic wasn't quiiite right;
ssl3_write_pending allows a retry to supply *additional* data, so not all
plaintext had been commited to before the IV was randomized. We could fix this
by tracking how many bytes were committed to the last time we fragmented, but
this is purely an optimization and doesn't seem worth the complexity.
This also fixes the alignment computation in the record-splitting case. The
extra byte was wrong, as demonstrated by the assert.
Change-Id: Ia087a45a6622f4faad32e501942cc910eca1237b
Reviewed-on: https://boringssl-review.googlesource.com/4234
Reviewed-by: Adam Langley <agl@google.com>
This fixes the Windows build. Otherwise this collides with the symbol in
Chromium's //base. (The 'base' suffix is the name of the library, not some
Windows-ism.)
Change-Id: I65d755f08991978bd2040d53c401082b2fee65fa
Reviewed-on: https://boringssl-review.googlesource.com/4217
Reviewed-by: Adam Langley <agl@google.com>
It seems Android's inttypes.h refuses to define those macros on C++ unless
__STDC_FORMAT_MACROS is set. This unbreaks the roll on Android.
Change-Id: Iad6c971b4789f0302534d9e5022534c6124e0ff0
Reviewed-on: https://boringssl-review.googlesource.com/4202
Reviewed-by: Adam Langley <agl@google.com>
Not actually much of a change, but consistency.
Change-Id: If2ef7a8b698a229f5c494822d870767e1a61476e
Reviewed-on: https://boringssl-review.googlesource.com/4127
Reviewed-by: Adam Langley <agl@google.com>
Along the way, fix a host of missing failure checks. This will save some
headache when it comes time to run these under the malloc failure tests.
Change-Id: I3fd589bd094178723398e793d6bc578884e99b67
Reviewed-on: https://boringssl-review.googlesource.com/4126
Reviewed-by: Adam Langley <agl@google.com>
We don't really gain much from this one, but consistency.
Change-Id: I3f830c6d1ad65263bd1cc09372a5b810a8f690c0
Reviewed-on: https://boringssl-review.googlesource.com/4124
Reviewed-by: Adam Langley <agl@google.com>
