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>
This conceivably has a use, but NSS doesn't do this buffer either and it still
suffers from the same problems as the other uses of record_pqueue. This removes
the last use of record_pqueue. It also opens the door to removing pqueue
altogether as it isn't the right data structure for either of the remaining
uses either. (It's not clear it was right for record_pqueue either, but I don't
feel like digging into this code.)
Change-Id: If8a43e7332b3cd11a78a516f3e8ebf828052316f
Reviewed-on: https://boringssl-review.googlesource.com/4239
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>
It was only ever enabled for handshake and alert messages. The comments cite
renego as a use case though even then I'm not clear on why. The only use I see
is if, say, the Finished message and ClientKeyExchange came in out-of-order.
DTLS is unreliable so leaning on retransmit seems fine, and usually flights
will be packed into one packet where possible. NSS doesn't have any such
buffer and doesn't seem to have problems.
The buffering mechanism is also rather dubious. It stows away the entire packet
and read buffer---all 16K of it---and there may have been other records in that
packet.
Change-Id: Ic3b7bf817be380dc73102eec62c690ed093e6667
Reviewed-on: https://boringssl-review.googlesource.com/4238
Reviewed-by: Adam Langley <agl@google.com>
At this point, has_version has been set and we may even have a non-null cipher.
Trying to assign meaning to the record-layer version number is not worth making
s->version's semantics even more complicated.
Change-Id: Ia1cf341cf7306eb48d2d11241316dc2116306968
Reviewed-on: https://boringssl-review.googlesource.com/4237
Reviewed-by: Adam Langley <agl@google.com>
Compression is gone, so don't allow for compression overhead. With that fixed,
the second rr->length check in ssl3_get_record matches the length computation
which sizes the read buffer. The first is wrong and doesn't account for the
alignment padding. Move the second to the first.
Change-Id: I3f4f05de9fdf5c645ff24493bbfdf303dcc1aa90
Reviewed-on: https://boringssl-review.googlesource.com/4236
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>
These are redundant with the lower level ones in s3_pkt.c just before BIO_read.
Only the operation which actually failed an operation on the BIO should set
the wait state.
Not all failure paths in ssl3_read_bytes and dtls1_read_bytes set SSL_READING,
but those that don't leave the BIO in a retry state, so SSL_READING doesn't
matter.
Change-Id: I2ae064ecc8b2946cc8ae8f724be09dfe49e077b5
Reviewed-on: https://boringssl-review.googlesource.com/4230
Reviewed-by: Adam Langley <agl@google.com>
Fix up the variable names. Also avoid the messy logic of checking whether the
label and context collide with the normal key expansion ones in the face of
adverserial inputs. Make that the caller's responsibility, just as it's already
the caller's responsibility to ensure that different calls don't overlap. (The
label should be a constant string in an IANA registry anyway.)
Change-Id: I062fadb7b6a18fa946b883be660ea9b3f0f6277c
Reviewed-on: https://boringssl-review.googlesource.com/4216
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>
It's still rather a mess, but this is at least somewhat clearer. The old one
had a lot of remnants of compression, etc.
Change-Id: Iffcb4dd4e8c4ab14f60abf917d22b7af960c93ba
Reviewed-on: https://boringssl-review.googlesource.com/4233
Reviewed-by: Adam Langley <agl@google.com>
If the key is missing, it seems the failure is assumed to be expected.
BUG=473924
Change-Id: I62edd9110fa74bee5e6425fd6786badf5398728c
Reviewed-on: https://boringssl-review.googlesource.com/4231
Reviewed-by: Adam Langley <agl@google.com>
Move the state to TestState rather than passing pointers to them everywhere.
Also move SSL_read and SSL_write retry loops into helper functions so they
aren't repeated everywhere. This also makes the SSL_write calls all
consistently account for partial writes.
Change-Id: I9bc083a03da6a77ab2fc03c29d4028435fc02620
Reviewed-on: https://boringssl-review.googlesource.com/4214
Reviewed-by: Adam Langley <agl@google.com>
Extend the False Start tests to optionally send an alert (thus avoiding
deadlock) before waiting for the out-of-order app data. Based on whether the
peer shuts off the connection before or after sending app data, we can
determine whether the peer False Started by observing purely external effects.
Change-Id: I8b9fecc29668e0b0c34b5fd19d0f239545011bae
Reviewed-on: https://boringssl-review.googlesource.com/4213
Reviewed-by: Adam Langley <agl@google.com>
Based on whether -false-start is passed, we expect SSL_CB_HANDSHAKE_DONE to or
not to fire. Also add a flag that asserts SSL_CB_HANDSHAKE_DONE does *not* fire
in any False Start test where the handshake fails after SSL_connect returns.
Change-Id: I6c5b960fff15e297531e15b16abe0b98be95bec8
Reviewed-on: https://boringssl-review.googlesource.com/4212
Reviewed-by: Adam Langley <agl@google.com>
That's a pretty obvious thing to test. I'm not sure how we forgot that one.
Change-Id: I7e1a7df6c6abbdd587e0f7723117f50d09faa5c4
Reviewed-on: https://boringssl-review.googlesource.com/4211
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>
This fixes the standalone build on Windows and matches Chromium.
Change-Id: I194f53e0a610c5ae9cef53c826b22f7bded5f357
Reviewed-on: https://boringssl-review.googlesource.com/4201
Reviewed-by: Adam Langley <agl@google.com>
There's multiple sets of APIs for selecting the curve. Fold away
SSL_OP_SINGLE_ECDH_USE as failing to set it is either a no-op or a bug. With
that gone, the consumer only needs to control the selection of a curve, with
key generation from then on being uniform. Also clean up the interaction
between the three API modes in s3_srvr.c; they were already mutually exclusive
due to tls1_check_ec_tmp_key.
This also removes all callers of EC_KEY_dup (and thus CRYPTO_dup_ex_data)
within the library.
Change-Id: I477b13bd9e77eb03d944ef631dd521639968dc8c
Reviewed-on: https://boringssl-review.googlesource.com/4200
Reviewed-by: Adam Langley <agl@google.com>
ASan's own malloc interceptor isn't compatible with this mechanism; it doesn't
see calls to __libc_malloc.
Change-Id: Ibac5aa05c6e40f1c72dcee3a2597e96deffca62c
Reviewed-on: https://boringssl-review.googlesource.com/4191
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>
SSL_CIPHER_get_rfc_name still returns an allocated string.
Change-Id: Ie2f14626c1ff22d0ea613b22439b7de5c04c9062
Reviewed-on: https://boringssl-review.googlesource.com/4190
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>
This is in preparation for using RAII in the unit tests. Those tests are built
in Chromium as well, but Chromium does not have C++11 library support across
all its toolchains. Compiler support is available, so add a partial
reimplementation of std::unique_ptr and std::move under crypto/test/. The
scopers for the crypto/ library are also moved there while the ones for ssl/
stay in ssl/test/.
Change-Id: I38f769acbc16a870db34649928575c7314b6e9f6
Reviewed-on: https://boringssl-review.googlesource.com/4120
Reviewed-by: Adam Langley <agl@google.com>
Historically, OpenSSL has used callbacks for anything thread related,
but we don't actually have that many threading libraries to worry about:
just pthreads and Windows (I hope).
That suggests that it's quite reasonable to handle threading ourselves,
and eliminate the need for users to remember to install the thread
callbacks.
The first user of this would be ERR, which currently simulates
thread-local storage using a lock around a hash table keyed by the TID.
(Although I suspect that change will need some CMake work in order that
libpthread is automatically included with libcrypto when linking tests
etc, but not on Windows and without lots of ifs.)
Change-Id: I4dd088e3794506747f875c1f3e92b9bc6700fad2
Reviewed-on: https://boringssl-review.googlesource.com/4010
Reviewed-by: Adam Langley <agl@google.com>
MIPS64 confusingly sets __mips__, but it's not a 32-bit platform. This
change updates the defines in base.h to recognise MIPS64 based on both
__mips__ and __LP64__ being defined.
Change-Id: I220f5d9c8f1cd7d3089cc013348e6f95cdee76d9
Reviewed-on: https://boringssl-review.googlesource.com/4093
Reviewed-by: Adam Langley <agl@google.com>
(system/keymaster is using them now.)
Change-Id: I8fba501005b9318b7d3a76bf1715fb772b23c49d
Reviewed-on: https://boringssl-review.googlesource.com/4092
Reviewed-by: Adam Langley <agl@google.com>
gmtime_s first appeared in MSVCR80, but libmingwex has a helper function
that tries to find the symbol or falls back to an internal
implementation.
(Patch by Kenny Root.)
Change-Id: I96ef9cd7459d7e8202831a4e687dfbc055c9f50b
Reviewed-on: https://boringssl-review.googlesource.com/4091
Reviewed-by: Adam Langley <agl@google.com>
MinGW on Linux needs lowercase include files. On Windows this doesn't
matter since the filesystems are case-insensitive, but building
BoringSSL on Linux with MinGW has case-sensitive filesystems.
Change-Id: Id9c120d819071b041341fbb978352812d6d073bc
Reviewed-on: https://boringssl-review.googlesource.com/4090
Reviewed-by: Adam Langley <agl@google.com>
NEON_FUNCTIONAL is set by default in crypto.c. Chromium calls
|CRYPTO_set_NEON_functional| before |SSL_library_init| and thus the
getauxval path for CPU-feature detection was resetting the functional
flag, even on broken processors.
This change means that, apart from the default, only
|CRYPTO_set_NEON_functional| will change the NEON_FUNCTIONAL flag.
BUG=469511
Change-Id: I3d4dbbd9f4a5e33539f8559f90289e706ad17451
Reviewed-on: https://boringssl-review.googlesource.com/4170
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This affects bignum and sha. Also now that we're passing the SSE2 flag, revert
the change to ghash-x86.pl which unconditionally sets $sse2, just to minimize
upstream divergence. Chromium assumes SSE2 support, so relying on it is okay.
See https://crbug.com/349320.
Note: this change needs to be mirrored in Chromium to take.
bssl speed numbers:
SSE2:
Did 552 RSA 2048 signing operations in 3007814us (183.5 ops/sec)
Did 19003 RSA 2048 verify operations in 3070779us (6188.3 ops/sec)
Did 72 RSA 4096 signing operations in 3055885us (23.6 ops/sec)
Did 4650 RSA 4096 verify operations in 3024926us (1537.2 ops/sec)
Without SSE2:
Did 350 RSA 2048 signing operations in 3042021us (115.1 ops/sec)
Did 11760 RSA 2048 verify operations in 3003197us (3915.8 ops/sec)
Did 46 RSA 4096 signing operations in 3042692us (15.1 ops/sec)
Did 3400 RSA 4096 verify operations in 3083035us (1102.8 ops/sec)
SSE2:
Did 16407000 SHA-1 (16 bytes) operations in 3000141us (5468743.0 ops/sec): 87.5 MB/s
Did 4367000 SHA-1 (256 bytes) operations in 3000436us (1455455.1 ops/sec): 372.6 MB/s
Did 185000 SHA-1 (8192 bytes) operations in 3002666us (61611.9 ops/sec): 504.7 MB/s
Did 9444000 SHA-256 (16 bytes) operations in 3000052us (3147945.4 ops/sec): 50.4 MB/s
Did 2283000 SHA-256 (256 bytes) operations in 3000457us (760884.1 ops/sec): 194.8 MB/s
Did 89000 SHA-256 (8192 bytes) operations in 3016024us (29509.0 ops/sec): 241.7 MB/s
Did 5550000 SHA-512 (16 bytes) operations in 3000350us (1849784.2 ops/sec): 29.6 MB/s
Did 1820000 SHA-512 (256 bytes) operations in 3001039us (606456.6 ops/sec): 155.3 MB/s
Did 93000 SHA-512 (8192 bytes) operations in 3007874us (30918.8 ops/sec): 253.3 MB/s
Without SSE2:
Did 10573000 SHA-1 (16 bytes) operations in 3000261us (3524026.7 ops/sec): 56.4 MB/s
Did 2937000 SHA-1 (256 bytes) operations in 3000621us (978797.4 ops/sec): 250.6 MB/s
Did 123000 SHA-1 (8192 bytes) operations in 3033202us (40551.2 ops/sec): 332.2 MB/s
Did 5846000 SHA-256 (16 bytes) operations in 3000294us (1948475.7 ops/sec): 31.2 MB/s
Did 1377000 SHA-256 (256 bytes) operations in 3000335us (458948.8 ops/sec): 117.5 MB/s
Did 54000 SHA-256 (8192 bytes) operations in 3027962us (17833.8 ops/sec): 146.1 MB/s
Did 2075000 SHA-512 (16 bytes) operations in 3000967us (691443.8 ops/sec): 11.1 MB/s
Did 638000 SHA-512 (256 bytes) operations in 3000576us (212625.8 ops/sec): 54.4 MB/s
Did 30000 SHA-512 (8192 bytes) operations in 3042797us (9859.3 ops/sec): 80.8 MB/s
BUG=430237
Change-Id: I47d1c1ffcd71afe4f4a192272f8cb92af9505ee1
Reviewed-on: https://boringssl-review.googlesource.com/4130
Reviewed-by: Adam Langley <agl@google.com>
- Pull in a trimmed down version of vs_toolchain.py from Chromium.
- Drop in toolchain_vs2013.hash from Chromium to use Chromium's
current toolchain.
- Add a very hacky vs_env.py to pull in Visual Studio. This is
loosely based off a handful of lines of Chromium's
tools/clang/scripts/update.py. This (and vs_toolchain.py) depends
on gyp which is now pulled in via DEPS.
BUG=430237
Change-Id: Ic29cbb15e19a99616cfe778d0778b9a71c45338a
Reviewed-on: https://boringssl-review.googlesource.com/3900
Reviewed-by: Adam Langley <agl@google.com>
Partly inspired by the new state exposed in
dc3da93899, stress this codepath by spamming our
poor shim with warning alerts.
Change-Id: I876c6e52911b6eb57493cf3e1782b37ea96d01f8
Reviewed-on: https://boringssl-review.googlesource.com/4112
Reviewed-by: Adam Langley <agl@google.com>
