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>
tls1_process_sigalgs now only determines the intersection between the peer
algorithms and those configured locally. That list is queried later to
determine the hash algorithm to use when signing CertificateVerify or
ServerKeyExchange.
This is needed to support client auth on Windows where smartcards or CAPI may
not support all hash functions.
As a bonus, this does away with more connection-global state. This avoids the
current situation where digests are chosen before keys are known (for
CertificateVerify) or for slots that don't exist.
Change-Id: Iec3619a103d691291d8ebe08ef77d574f2faf0e8
Reviewed-on: https://boringssl-review.googlesource.com/2280
Reviewed-by: Adam Langley <agl@google.com>
CERT_PKEY_SIGN isn't meaningful since, without strict mode, we always fall back
to SHA-1 anyway. So the digest is never NULL when CERT_PKEY_SIGN is computed.
The entire valid_flags is now back to it's pre-1.0.2 check of seeing if the
certificate and key are configured.
This finally removes the sensitivity between valid_flags and selecting the
digest, so we can defer choosing the digest all we like.
Change-Id: I9f9952498f512d7f0cc799497f7c5b52145a48af
Reviewed-on: https://boringssl-review.googlesource.com/2288
Reviewed-by: Adam Langley <agl@google.com>
It doesn't depend on the cipher now that export ciphers are gone. It need only
be called once. Also remove the valid bit; nothing ever reads it. Its output is
also only used within a function, so make mask_k and mask_a local variables.
So all the configuration-based checks are in one place, change the input
parameter from CERT to SSL and move the PSK and ECDHE checks to the mask
computation. This avoids having to evaluate the temporary EC key for each
cipher.
The remaining uses are on the client which uses them differently (disabled
features rather than enabled ones). Those too may as well be local variables,
so leave a TODO.
Change-Id: Ibcb574341795d4016ea749f0290a793eed798874
Reviewed-on: https://boringssl-review.googlesource.com/2287
Reviewed-by: Adam Langley <agl@google.com>
Many are now unused. Only two are currently considered in cipher selection:
CERT_PKEY_VALID and CERT_PKEY_SIGN. (As per previous commits, this is either
bizarre due to limited slots or redundant with ssl_early_callback_ctx. We can
probably prune this too.)
This also fixes a bug where DTLS 1.0 went through a TLS 1.2 codepath. As the
DTLS code is currently arranged, all version comparisons must be done via
macros like SSL_USE_SIGALGS. (Probably we should add functions to map from DTLS
to TLS versions and slowly move the library to using the TLS version as
in-memory representation.)
Change-Id: I89bcf5b7b9ea5cdecf54f4445156586377328fe0
Reviewed-on: https://boringssl-review.googlesource.com/2286
Reviewed-by: Adam Langley <agl@google.com>
It's new in OpenSSL 1.0.2 so it's never set by existing code. This removes gobs
and gobs of complexity from tls1_check_chain. It only checks the local
certificate, not the peer certificate. The uses appear to be:
- Sanity-check configuration. Not worth the complexity.
- Guide in selecting ciphers based on ClientHello parameters and which
certificates in the CERT_PKEY are compatible. This isn't very useful one its
own since the CERT_PKEY array only stores one slot per type (e.g. you cannot
configure RSA/SHA-1 and RSA/SHA-256).
- For the (currently removed) SSL_check_chain to return more information based
on ClientHello parameters and guide selecting a certificate. This is
potentially useful but, as noted in the commit which removed it, redundant
with ssl_early_callback_ctx.
This CL is largely mechanical removing of dead codepaths. The follow-up will
clean up the now unnecessary parts of this function.
Change-Id: I2ebfa17e4f73e59aa1ee9e4ae7f615af2c6cf590
Reviewed-on: https://boringssl-review.googlesource.com/2285
Reviewed-by: Adam Langley <agl@google.com>
Both of these are newly-exported in OpenSSL 1.0.2, so they cannot be used by
current consumers.
This was added in upstream's 18d7158809c9722f4c6d2a8af7513577274f9b56 to
support custom selection of certificates. The intent seems to be that you
listen to cert_cb and use SSL_check_chain to lean on OpenSSL to process
signature algorithms list for you.
Unfortunately, the implementation is slightly suspect: it uses the same
function as the codepath which mutates and refers to the CERT_PKEY of the
matching type. Some access was guarded by check_flags, but this is too
complex. Part of it is also because the matching digest is selected early and
we intend to connect this to EVP_PKEY_supports_digest so it is no longer a
property of just the key type.
Let's remove the hook for now, to unblock removing a lot of complexity. After
cleaning up this area, a function like this could be cleaner to support, but
we already have a version of this: select_certificate_cb and
ssl_early_callback_ctx.
Change-Id: I3add425b3996e5e32d4a88e14cc607b4fdaa5aec
Reviewed-on: https://boringssl-review.googlesource.com/2283
Reviewed-by: Adam Langley <agl@google.com>
Server-side support was removed in 77a942b7fe,
but client-side support was retained as it appeared NSS supported this.
However, this is not the case: ssl3_HandleServerKeyExchange only allows a
ServerKeyExchange message if hs.ws is in an appropriate state.
ssl3_AuthCertificate only sets it to allow ServerKeyExchange if it is a key
exchange that normally uses it or if is_limited is set. is_limited is only set
for the export cipher suites.
Thus we can safely remove this without waiting on gathering UMA data.
BUG=chromium:400587
Change-Id: I9aefb742dbb2d99c13340ab48017e1ceee04bc2f
Reviewed-on: https://boringssl-review.googlesource.com/2230
Reviewed-by: Adam Langley <agl@google.com>
There's only one caller and it doesn't use that feature. While I'm here, tidy
that function a little. Don't bother passing FALLBACK_SCSV into
ssl3_get_cipher_by_value.
Change-Id: Ie71298aeaaab6e24401e0a6c2c0d2281caa93ba4
Reviewed-on: https://boringssl-review.googlesource.com/2030
Reviewed-by: Adam Langley <agl@google.com>
This avoids the strange optional parameter thing by moving it to the client.
Also document what the functions should do.
Change-Id: I361266acadedfd2bfc4731f0900821fc2c2f954d
Reviewed-on: https://boringssl-review.googlesource.com/1843
Reviewed-by: Adam Langley <agl@google.com>
The TLS-specific hooks have been removed. We aim to no longer perform version
negotiation as a pre-processing step, so ensure the only differences to worry
about are the version, get_method hook, and the enc_data.
BUG=chromium:403378
Change-Id: I628ec6f4c50ceed01d7af8f4110b6dc95cfbe023
Reviewed-on: https://boringssl-review.googlesource.com/1841
Reviewed-by: Adam Langley <agl@google.com>
Still need to convert serializing code to CBB, but the current one is kinda
crazy.
Change-Id: I00e12a812c815bf01c53a26ccbb7c6727ea8c8fc
Reviewed-on: https://boringssl-review.googlesource.com/1840
Reviewed-by: Adam Langley <agl@google.com>
Splitting the strength mask between SSL_EXP_MASK and SSL_STRONG_MASK no longer
does anything. Also remove the SSL_NOT_EXP bit and condense the strength bits.
Change-Id: I9e61acdde008c3ce06bb37f78a72099fc53ed080
Reviewed-on: https://boringssl-review.googlesource.com/1757
Reviewed-by: Adam Langley <agl@google.com>
Simplify all the cipher gathering logic. The set of supported ciphers is known,
so there is no need to determine if some cipher exists but doesn't work.
Change-Id: Idcaae67e7bfc40a3deb925d85ee1a99a931b67e7
Reviewed-on: https://boringssl-review.googlesource.com/1756
Reviewed-by: Adam Langley <agl@google.com>
Configures the SSL stack to log session information to a BIO. The intent is to
support NSS's SSLKEYLOGFILE environment variable. Add support for the same
environment variable to tool/client.cc.
Tested against Wireshark 1.12.0.
BUG=393477
Change-Id: I4c231f9abebf194eb2df4aaeeafa337516774c95
Reviewed-on: https://boringssl-review.googlesource.com/1699
Reviewed-by: Adam Langley <agl@google.com>
Remove the old implementation which was excessively general. This mirrors the
SCT support and adds a single boolean flag to request an OCSP response with no
responder IDs, extensions, or frills. The response, if received, is stored on
the SSL_SESSION so that it is available for (re)validation on session
resumption; Chromium revalidates the saved auth parameters on resume.
Server support is unimplemented for now. This API will also need to be adjusted
in the future if we implement RFC 6961.
Change-Id: I533c029b7f7ea622d814d05f934fdace2da85cb1
Reviewed-on: https://boringssl-review.googlesource.com/1671
Reviewed-by: Adam Langley <agl@google.com>
Get all this stuff out of the way.
- OPENSSL_NO_MD5
- OPENSSL_NO_SHA
- OPENSSL_NO_EC
- OPENSSL_NO_ECDSA
- OPENSSL_NO_ECDH
- OPENSSL_NO_NEXTPROTONEG
- OPENSSL_NO_DH
- OPENSSL_NO_SSL3
- OPENSSL_NO_RC4
- OPENSSL_NO_RSA
Also manually removed a couple instances of OPENSSL_NO_DSA that seemed to be
confused anyway. Did some minor manual cleanup. (Removed a few now-pointless
'if (0)'s.)
Change-Id: Id540ba97ee22ff2309ab20ceb24c7eabe766d4c4
Reviewed-on: https://boringssl-review.googlesource.com/1662
Reviewed-by: Adam Langley <agl@google.com>
This moves CertificateVerify digest processing to the new
SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag. It also refactors it similarly to
ssl3_send_cert_verify and moves that logic to a common ssl3_cert_verify_hash
function to compute the handshake hash.
This removes a large chunk of duplicate (and divergent!) logic between TLS and
DTLS. It also removes TLS1_FLAGS_KEEP_HANDSHAKE.
Change-Id: Ia63c94f7d76d901bc9c4c33454fbfede411adf63
Reviewed-on: https://boringssl-review.googlesource.com/1633
Reviewed-by: Adam Langley <agl@google.com>
This replaces the special-case in ssl3_get_message for Channel ID. Also add
ssl3_hash_current_message to hash the current message, taking TLS vs DTLS
handshake header size into account.
One subtlety with this flag is that a message intended to be processed with
SSL_GET_MESSAGE_DONT_HASH_MESSAGE cannot follow an optional message
(reprocessed with reuse_message, etc.). There is an assertion to that effect.
If need be, we can loosen it to requiring that the preceeding optional message
also pass SSL_GET_MESSAGE_DONT_HASH_MESSAGE and then maintain some state to
perform the more accurate assertion, but this is sufficient for now.
Change-Id: If8c87342b291ac041a35885b9b5ee961aee86eab
Reviewed-on: https://boringssl-review.googlesource.com/1630
Reviewed-by: Adam Langley <agl@google.com>
It doesn't appear to have ever been implemented on the client. The server code
stopped working anyway because it now skips the ssl_get_message call, so we
never cash in on the reuse_message, attempt to reprocess the repeated
ClientHello, and reject it thinking it's a second MS SGC restart.
Change-Id: Id536846e08460143f6fc0a550bdcc1b26b506b04
Reviewed-on: https://boringssl-review.googlesource.com/1580
Reviewed-by: Adam Langley <agl@google.com>
Remove all the logic managing key types that aren't being used anymore.
Change-Id: I101369164588048e64ba1c84a6b8aac8f3a221cd
Reviewed-on: https://boringssl-review.googlesource.com/1567
Reviewed-by: Adam Langley <agl@google.com>
DSA is not connected up to EVP, so it wouldn't work anyway. We shouldn't
advertise a cipher suite we don't support. Chrome UMA data says virtually no
handshakes end up negotiating one of these.
Change-Id: I874d934432da6318f05782ebd149432c1d1e5275
Reviewed-on: https://boringssl-review.googlesource.com/1566
Reviewed-by: Adam Langley <agl@google.com>
These are the variants where the CA signs a Diffie-Hellman keypair. They are
not supported by Chrome on NSS.
Change-Id: I569a7ac58454bd3ed1cd5292d1f98499012cdf01
Reviewed-on: https://boringssl-review.googlesource.com/1564
Reviewed-by: Adam Langley <agl@google.com>
This also removes the 'LOW' strength class.
Change-Id: Iffd2356dadb4a4875c1547a613d51061101358fd
Reviewed-on: https://boringssl-review.googlesource.com/1562
Reviewed-by: Adam Langley <agl@google.com>
NULL, SRP, CAMELLIA, export ciphers, SSLv2, IDEA, and SEED are gone. Unknown
directives are silently ignored in the parser, so there is no need to retain
their masks and entries in the cipher suite aliases.
Change-Id: If43b9cbce56b3e1c401db764b88996940452a300
Reviewed-on: https://boringssl-review.googlesource.com/1561
Reviewed-by: Adam Langley <agl@google.com>
This lets us put the SSL_CIPHER table in the data section. For type-checking,
make STACK_OF(SSL_CIPHER) cast everything to const SSL_CIPHER*.
Note that this will require some changes in consumers which weren't using a
const SSL_CIPHER *.
Change-Id: Iff734ac0e36f9e5c4a0f3c8411c7f727b820469c
Reviewed-on: https://boringssl-review.googlesource.com/1541
Reviewed-by: Adam Langley <agl@google.com>
Of the remaining implementations left, ssl3_, dtls1_, and ssl23_, dtls1_ is
redundant and can be folded into ssl3_. ssl23_ actually isn't; it sets 5
minutes rather than 2 hours. Two hours seems to be what everything else uses
and seems a saner default. Most consumers seem to override it anyway
(SSL_CTX_set_timeout). But it is a behavior change.
The method is called at two points:
- SSL_get_default_timeout
- SSL_CTX_new
Incidentally, the latter call actually makes the former never called internally
and the value it returns a lie. SSL_get_default_timeout returns the default
timeout of the /current/ method, but in ssl_get_new_session, the timeout is
shadowed by session_timeout on the context. That is initialized when
SSL_CTX_new is called. So, unless you go out of your way to
SSL_CTX_set_timeout(0), it always overrides. (And it actually used to a
difference because, for SSL23, the SSL_CTX's method is SSL23, but, when session
creation happens, the SSL's method is the version-specific one.)
Change-Id: I331d3fd69b726242b36492402717b6d0b521c6ee
Reviewed-on: https://boringssl-review.googlesource.com/1521
Reviewed-by: Adam Langley <agl@google.com>
It was added in OpenSSL 1.0.2, so nothing can be depending on it yet. If we
really want a Suite B profile, it seems better to generate a configuration for
the rest of the system rather than pepper the codebase with checks.
Change-Id: I1be3ebed0e87cbfe236ade4174dcf5bbc7e10dd5
Reviewed-on: https://boringssl-review.googlesource.com/1517
Reviewed-by: Adam Langley <agl@google.com>
The protocols are pretty similar; they were all basically redundant. The free
of s->tlsext_session_ticket (more fallout from the EAP-FAST patch) was moved to
SSL_free because that object's attached to s, not s->s3. This is relevant if
SSL_set_ssl_method gets called.
Change-Id: I14a896ba8a6a2c34ab1cb5f65311b117051228da
Reviewed-on: https://boringssl-review.googlesource.com/1509
Reviewed-by: Adam Langley <agl@google.com>
Windows doesn't have ssize_t, sadly. There's SSIZE_T, but defining an
OPENSSL_SSIZE_T seems worse than just using an int.
Change-Id: I09bb5aa03f96da78b619e551f92ed52ce24d9f3f
Reviewed-on: https://boringssl-review.googlesource.com/1352
Reviewed-by: Adam Langley <agl@google.com>
The memory freelist maintained by OpenSSL claims to be a performance
optimization for platforms that have a slow malloc/free
implementation. This should not be the case on modern
linux/glibc. Remove the freelist as it poses a potential security
hazard of buffer-reuse that is of "initialized" memory that will not
be caught be tools such as valgrind.
Change-Id: I3cfa6a05f9bdfbbba7820060bae5a673dee43014
Reviewed-on: https://boringssl-review.googlesource.com/1385
Reviewed-by: Adam Langley <agl@google.com>
The code guarded by PKCS1_CHECK appears to be unhelpful, and the guard
is explicitly undefined in ssl_locl.h Remove both.
Change-Id: I3cd45a744a8f35b02181b1e48fd1ef11af5e6f4a
Reviewed-on: https://boringssl-review.googlesource.com/1383
Reviewed-by: Adam Langley <agl@google.com>
This change marks public symbols as dynamically exported. This means
that it becomes viable to build a shared library of libcrypto and libssl
with -fvisibility=hidden.
On Windows, one not only needs to mark functions for export in a
component, but also for import when using them from a different
component. Because of this we have to build with
|BORINGSSL_IMPLEMENTATION| defined when building the code. Other
components, when including our headers, won't have that defined and then
the |OPENSSL_EXPORT| tag becomes an import tag instead. See the #defines
in base.h
In the asm code, symbols are now hidden by default and those that need
to be exported are wrapped by a C function.
In order to support Chromium, a couple of libssl functions were moved to
ssl.h from ssl_locl.h: ssl_get_new_session and ssl_update_cache.
Change-Id: Ib4b76e2f1983ee066e7806c24721e8626d08a261
Reviewed-on: https://boringssl-review.googlesource.com/1350
Reviewed-by: Adam Langley <agl@google.com>
Now that the flag is set accurately, use it to enforce that the handshake and
CCS synchronization. If EXPECT_CCS is set, enforce that:
(a) No handshake records may be received before ChangeCipherSpec.
(b) There is no pending handshake data at the point EXPECT_CCS is set.
Change-Id: I04b228fe6a7a771cf6600b7d38aa762b2d553f08
Reviewed-on: https://boringssl-review.googlesource.com/1299
Reviewed-by: Adam Langley <agl@google.com>
This removes the last case where the server generates an RSA key for the
ServerKeyExchange. Remove the code for this. Client support to accept them
still remains.
Leave the APIs for now, but they don't do anything anymore.
Change-Id: I84439e034cc575719f5bc9b3e501165e12b62107
Reviewed-on: https://boringssl-review.googlesource.com/1286
Reviewed-by: Adam Langley <agl@google.com>
Slightly cleaner; it means we can use CBS_stow.
Change-Id: I074aa2d73a79648013dea025ee531beeea2af4a2
Reviewed-on: https://boringssl-review.googlesource.com/1287
Reviewed-by: Adam Langley <agl@google.com>
Now the only case where temporary RSA keys are used on the server end is
non-signing keys.
Change-Id: I55f6c206e798dd28548c386fdffd555ccc395477
Reviewed-on: https://boringssl-review.googlesource.com/1285
Reviewed-by: Adam Langley <agl@google.com>
This removes support code for a "stream_mac" mode only used by GOST. Also get
rid of this
/* I should fix this up TLS TLS TLS TLS TLS XXXXXXXX */
comment next to it. It's not actually related to GOST (dates to OpenSSL initial
commit), but isn't especially helpful at this point.
Change-Id: Ib13c6e27e16e0d1fb59ed0142ddf913b9abc20b7
Reviewed-on: https://boringssl-review.googlesource.com/1281
Reviewed-by: Adam Langley <agl@google.com>
Some ssl23 functions that can be folded into ssl3, declarations and macros that
don't exist anymore.
Change-Id: I8057fb0bab8b6fe7e4da7b90a4945f7f22e29cd9
Reviewed-on: https://boringssl-review.googlesource.com/1280
Reviewed-by: Adam Langley <agl@google.com>
Without SSLv2, all cipher suite values are 2 bytes. Represent them as a
uint16_t and make all functions pass those around rather than pointers.
This removes SSL_CIPHER_find as it's unused.
Change-Id: Iea0b75abee4352a8333a4b8e39a161430ae55ea6
Reviewed-on: https://boringssl-review.googlesource.com/1259
Reviewed-by: Adam Langley <agl@google.com>
Don't retain curve IDs in serialized form; serialization only happens when
writing and reading from the wire. The internal representation is a uint16_t
which matches the range of the value and avoids all the checks for the first
byte being 0.
This also fixes a bug in tls1_check_ec_tmp_key's suite B logic; the || should
have been &&, though now it's gone.
This doesn't relieve some of the other assumptions about curve IDs:
tls1_set_curves still assumes that all curve IDs are under 32, and
tls1_ec_curve_id2nid still assumes 0 is not a valid curve ID. Add a
compile-time assert and a comment to document this. We're up to 28 now, so this
may well need to be revised sooner or later.
Remove SSL_get_shared_curve as it's new and unused API, using it in a loop is
O(N^3), and lets us simplify a function.
Change-Id: I82778cb82648d82f7b5de8c5341e0e1febdf5611
Reviewed-on: https://boringssl-review.googlesource.com/1256
Reviewed-by: Adam Langley <agl@google.com>
ssl3_get_new_session_ticket is sensible and fills in a session_id for stateless
sessions, so the resumption will already be detected at this point. Remove the
codepath in ssl3_client_hello which allows for resuming sessions with empty
session_ids. The rest of the code doesn't allow it either.
This removes another codepath where we potentially probe a Finished message
early.
Change-Id: I2749b5c65c7ce98c6f30566d8716360ff1bba24c
Reviewed-on: https://boringssl-review.googlesource.com/1295
Reviewed-by: Adam Langley <agl@google.com>
Also fix some DTLS cookie bugs. rcvd_cookie is never referenced after being
saved (and the length isn't saved, so it couldn't be used anyway), and the
cookie verification failed to check the length.
For convenience, add a CBS_mem_equal helper function. Saves a bit of
repetition.
Change-Id: I187137733b069f0ac8d8b1bf151eeb80d388b971
Reviewed-on: https://boringssl-review.googlesource.com/1174
Reviewed-by: Adam Langley <agl@google.com>
More consistent with ssl3_send_server_key_exchange and the message name.
Change-Id: If0f435a89bdf117297d349099708fff0bd5a6e98
Reviewed-on: https://boringssl-review.googlesource.com/1170
Reviewed-by: Adam Langley <agl@google.com>
This avoids having to do the CBS_skip dance and is better about returning the
right alert.
Change-Id: Id84eba307d7c67269ccbc07a38d9044b6f4f7c6c
Reviewed-on: https://boringssl-review.googlesource.com/1169
Reviewed-by: Adam Langley <agl@google.com>
Also tidy up some variable names and update RSA_verify call for it no longer
returning -1. Add CBS helper functions for dealing with C strings.
Change-Id: Ibc398d27714744f5d99d4f94ae38210cbc89471a
Reviewed-on: https://boringssl-review.googlesource.com/1164
Reviewed-by: Adam Langley <agl@google.com>
