- Pass in the right ciphertext length to ensure we're indeed testing
ciphertext corruption (and not truncation).
- Only test one mutation per byte to not make the test too slow.
- Add a separate test for truncated ciphertexts.
(Imported from upstream's 5f623eb61655688501cb1817a7ad0592299d894a.)
Change-Id: I425a77668beac9d391387e3afad8d15ae387468f
Reviewed-on: https://boringssl-review.googlesource.com/5945
Reviewed-by: Adam Langley <agl@google.com>
The Android system BoringSSL has a couple of changes:
∙ ChaCha20-Poly1305 is disabled because it's not an offical
cipher suite.
∙ P-521 is offered in the ClientHello.
These changes were carried in the Android BoringSSL repo directly. This
change upstreams them when BORINGSSL_ANDROID_SYSTEM is defined.
Change-Id: If3e787c6694655b56e7701118aca661e97a5f26c
Or at least group them together and make a passing attempt to document
them. The legacy X.509 stack itself remains largely untouched and most
of the parameters have to do with it.
Change-Id: I9e11e2ad1bbeef53478c787344398c0d8d1b3876
Reviewed-on: https://boringssl-review.googlesource.com/5942
Reviewed-by: Adam Langley <agl@google.com>
RSA_PSK is really weird in that it takes a Certificate, but you're not
expected to verify it. It's just a funny way to transmit an RSA key.
(They probably should have used the RSA_EXPORT ServerKeyExchange
spelling.) Some code now already doesn't account for it right around
certificate verification.
Given ECDHE_PSK exists, hopefully there will never be any need to add
this.
Change-Id: Ia64dac28099eaa9021f8d915d45ccbfd62872317
Reviewed-on: https://boringssl-review.googlesource.com/5941
Reviewed-by: Adam Langley <agl@google.com>
Allow configuring digest preferences for the private key. Some
smartcards have limited support for signing digests, notably Windows
CAPI keys and old Estonian smartcards. Chromium used the supports_digest
hook in SSL_PRIVATE_KEY_METHOD to limit such keys to SHA1. However,
detecting those keys was a heuristic, so some SHA256-capable keys
authenticating to SHA256-only servers regressed in the switch to
BoringSSL. Replace this mechanism with an API to configure digest
preference order. This way heuristically-detected SHA1-only keys may be
configured by Chromium as SHA1-preferring rather than SHA1-requiring.
In doing so, clean up the shared_sigalgs machinery somewhat.
BUG=468076
Change-Id: I996a2df213ae4d8b4062f0ab85b15262ca26f3c6
Reviewed-on: https://boringssl-review.googlesource.com/5755
Reviewed-by: Adam Langley <agl@google.com>
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.
This workaround will be removed in six months.
BUG=534766
Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
We wish to be able to detect the use of RC4 so that we can flag it and
investigate before it's disabled.
Change-Id: I6dc3a5d2211b281097531a43fadf08edb5a09646
Reviewed-on: https://boringssl-review.googlesource.com/5930
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
EVP_MD_CTX_copy_ex was implemented with a memcpy, which doesn't work well when
some of the pointers need to be copied, and ssl_verify_cert_chain didn't
account for set_ex_data failing.
Change-Id: Ieb556aeda6ab2e4c810f27012fefb1e65f860023
Reviewed-on: https://boringssl-review.googlesource.com/5911
Reviewed-by: Adam Langley <agl@google.com>
Target date for removal of the workaround is 6 months.
BUG=532048
Change-Id: I402f75e46736936725575559cd8eb194115ab0df
Reviewed-on: https://boringssl-review.googlesource.com/5910
Reviewed-by: Adam Langley <agl@google.com>
Get them out of the way when reading through the header.
Change-Id: Ied3f3601262e74570769cb7f858dcff4eff44813
Reviewed-on: https://boringssl-review.googlesource.com/5898
Reviewed-by: Adam Langley <agl@google.com>
Existing documentation was moved to the header, very slightly tweaked.
Change-Id: Ife3c2351e2d7e6a335854284f996918039414446
Reviewed-on: https://boringssl-review.googlesource.com/5897
Reviewed-by: Adam Langley <agl@google.com>
These were already documented, though some of the documentation was
expanded on slightly.
Change-Id: I04c6276a83a64a03ab9cce9b9c94d4dea9ddf638
Reviewed-on: https://boringssl-review.googlesource.com/5896
Reviewed-by: Adam Langley <agl@google.com>
All these functions were already documented, just not grouped. I put
these above DTLS-SRTP and PSK as they're considerably less niche of
features.
Change-Id: I610892ce9763fe0da4f65ec87e5c7aaecb10388b
Reviewed-on: https://boringssl-review.googlesource.com/5895
Reviewed-by: Adam Langley <agl@google.com>
SCT and OCSP are part of the session data and as such shouldn't be sent
again to the client when resuming.
Change-Id: Iaee3a3c4c167ea34b91504929e38aadee37da572
Reviewed-on: https://boringssl-review.googlesource.com/5900
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
ssl.h should be first. Also two lines after includes and the rest of the
file.
Change-Id: Icb7586e00a3e64170082c96cf3f8bfbb2b7e1611
Reviewed-on: https://boringssl-review.googlesource.com/5892
Reviewed-by: Adam Langley <agl@google.com>
Forgot to fix these when I fixed the headers.
Change-Id: Ie45e624abc993e16e2d5a872ef00dba9029a38df
Reviewed-on: https://boringssl-review.googlesource.com/5891
Reviewed-by: Adam Langley <agl@google.com>
The documentation for |ECDSA_sign| and |ECDSA_verify| says that the
|type| parameter should be zero.
Change-Id: I875d3405455c5443f5a5a5c2960a9a9f486ca5bb
Reviewed-on: https://boringssl-review.googlesource.com/5832
Reviewed-by: Adam Langley <agl@google.com>
Estonian IDs issued between September 2014 to September 2015 are broken and use
negative moduli. They last five years and are common enough that we need to
work around this bug.
Add parallel "buggy" versions of BN_cbs2unsigned and RSA_parse_public_key which
tolerate this mistake, to align with OpenSSL's previous behavior. This code is
currently hooked up to rsa_pub_decode in RSA_ASN1_METHOD so that d2i_X509 is
tolerant. (This isn't a huge deal as the rest of that stack still uses the
legacy ASN.1 code which is overly lenient in many other ways.)
In future, when Chromium isn't using crypto/x509 and has more unified
certificate handling code, we can put client certificates under a slightly
different codepath, so this needn't hold for all certificates forever. Then in
September 2019, when the broken Estonian certificates all expire, we can purge
this codepath altogether.
BUG=532048
Change-Id: Iadb245048c71dba2eec45dd066c4a6e077140751
Reviewed-on: https://boringssl-review.googlesource.com/5894
Reviewed-by: Adam Langley <agl@google.com>
This gets the documentation into the ssl.h documentation, and removes
one of the circularly-dependent headers hanging off ssl.h. Also fixes
some typos; there were a few instances of "SSL *ctx".
Change-Id: I2a41c6f518f4780af84d468ed220fe7b0b8eb0d3
Reviewed-on: https://boringssl-review.googlesource.com/5883
Reviewed-by: Adam Langley <agl@google.com>
Also switch to the new variable names (SSL_CTX *ctx, SSL *ssl,
SSL_SESSION *session) for all documented functions.
Change-Id: I15e15a703b96af1727601108223c7ce3b0691f1d
Reviewed-on: https://boringssl-review.googlesource.com/5882
Reviewed-by: Adam Langley <agl@google.com>
To be consistent with some of the other headers and because SSL_METHOD
no longer has a place to anchor documentation, move the type
documentation up to the corresponding section headers, rather than
attached to a convenient function.
Also document thread-safety properties of SSL and SSL_CTX.
Change-Id: I7109d704d28dda3f5d83c72d86fe31bc302b816e
Reviewed-on: https://boringssl-review.googlesource.com/5876
Reviewed-by: Adam Langley <agl@google.com>
There was a bug in skipPast; it was skipping to the start of the string,
rather than the end of it. But more of an issue is that it would skip if
it was in the middle of the string, which caused problems when
STACK_OF(FOO) was used as a parameter.
At some point, we'll probably need to give this a real C declaration
parser. We still have declarations (like those that return function
pointers) which we can't parse. But for now let's clear the low-hanging
fruit.
Change-Id: Ic2cee452cc8cf6887a6ff1b00cea353cec361955
Reviewed-on: https://boringssl-review.googlesource.com/5875
Reviewed-by: Adam Langley <agl@google.com>
This is arguably more commonly queried connection information than the
tls-unique.
Change-Id: I1f080536153ba9f178af8e92cb43b03df37110b5
Reviewed-on: https://boringssl-review.googlesource.com/5874
Reviewed-by: Adam Langley <agl@google.com>
Just the stuff that has been pulled out into sections already.
Change-Id: I3da6bc61d79ccfe2b18d888075dc32026a656464
Reviewed-on: https://boringssl-review.googlesource.com/5873
Reviewed-by: Adam Langley <agl@google.com>
Unfortunately, these are also some of the worst APIs in the SSL stack.
I've tried to capture all the things they expose to the caller. 0 vs -1
is intentionally left unexpanded on for now. Upstream's documentation
says 0 means transport EOF, which is a nice idea but isn't true. (A lot
of random functions return 0 on error and pass it up to the caller.)
https://crbug.com/466303 tracks fixing that.
SSL_set_bio is intentionally documented to NOT be usable when they're
already configured. The function tries to behave in this case and even
with additional cases when |rbio| and/or |wbio| are unchanged, but this
is buggy. For instance, this will explode:
SSL_set_bio(ssl, bio1, bio1);
SSL_set_bio(ssl, bio2, SSL_get_wbio(ssl));
As will this, though it's less clear this is part of the API contract
due to SSL taking ownership.
SSL_set_bio(ssl, bio1, bio2);
SSL_set_bio(ssl, bio2, bio1);
It also tries to handle ssl->bbio already existing, but I doubt it quite
works. Hopefully we can drop ssl->bbio eventually. (Why is this so
complicated...)
Change-Id: I5f9f3043915bffc67e2ebd282813e04afbe076e6
Reviewed-on: https://boringssl-review.googlesource.com/5872
Reviewed-by: Adam Langley <agl@google.com>
Last set of changes didn't do that.
Change-Id: Iae24e75103529ce4d50099c5cbfbcef0e10ba663
Reviewed-on: https://boringssl-review.googlesource.com/5871
Reviewed-by: Adam Langley <agl@google.com>
clang scan-build found a memory leak if the overflow codepath in
dtls1_hm_fragment is hit. Along the way, tidy up that function.
Change-Id: I3c4b88916ee56ab3ab63f93d4a967ceae381d187
Reviewed-on: https://boringssl-review.googlesource.com/5870
Reviewed-by: Adam Langley <agl@google.com>
We were getting this because of C's defaults, but it's fragile to leave
it like this because someone may add another field at the end in the
future.
Change-Id: I8b2dcbbc7cee8062915d15101f99f5a1aae6ad87
Reviewed-on: https://boringssl-review.googlesource.com/5860
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
SSL_get_client_CA_list is one of those dreaded functions which may query either
configuration state or handshake state. Moreover, it does so based on
|ssl->server|, which may not be configured until later. Also check
|ssl->handshake_func| to make sure |ssl| is not in an indeterminate state.
This also fixes a bug where SSL_get_client_CA_list wouldn't work in DTLS due to
the incorrect |ssl->version| check.
Change-Id: Ie564dbfeecd2c8257fd6bcb148bc5db827390c77
Reviewed-on: https://boringssl-review.googlesource.com/5827
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Ifa44fef160fc9d67771eed165f8fc277f28a0222
Reviewed-on: https://boringssl-review.googlesource.com/5840
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Using numbers is sensitive to moving things around. Instead, use the
names and enforce, for sections, that they are unique. Names would be
enforced too, but there's a table-of-contents bug around #ifdefs to
resolve first.
Change-Id: I8822e8ba8da9ed3ee4984365b8a64932d16d5baf
Reviewed-on: https://boringssl-review.googlesource.com/5826
Reviewed-by: Adam Langley <agl@google.com>
A small handful of functions got a 'Deprecated:' prefix instead in
documentation.
Change-Id: Ic151fb7d797514add66bc6465b6851b666a471bc
Reviewed-on: https://boringssl-review.googlesource.com/5825
Reviewed-by: Adam Langley <agl@google.com>
We had a few duplicate section names.
Change-Id: I0c9b2a1669ac14392fd577097d5ee8dd80f7c73c
Reviewed-on: https://boringssl-review.googlesource.com/5824
Reviewed-by: Adam Langley <agl@google.com>
It was checking algorithm_mac rather than algorithm_enc.
(Coincidentally, it gave the right answer if you compiled out the
ChaCha20 ciphers since SSL_AES128GCM and SSL_AEAD shared a value.)
Change-Id: I17047425ef7fabb98969144965d8db9528ef8497
Reviewed-on: https://boringssl-review.googlesource.com/5850
Reviewed-by: Adam Langley <agl@google.com>
The size of the stack caused by this object is problematic for systems
that have smaller stacks because they expect many threads.
Change-Id: Ib8f03741f9dd96bf474126f001947f879e50a781
Reviewed-on: https://boringssl-review.googlesource.com/5831
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
It didn't do too much and I didn't notice that CRYPTO_sysrand wasn't
OPENSSL_EXPORTed, which makes the test impossible on shared-library
builds.
Change-Id: I38986572aa34fa9c0f30075d562b8ee4e1a0c8b8
