Commit Graph

688 Commits

Author SHA1 Message Date
David Benjamin
71f7d3d2e1 Promote everything directly in SSL_CTX_ctrl.
BUG=404754

Change-Id: I0e4af2f341fcef5d01c855d97e981b8597d08b63
Reviewed-on: https://boringssl-review.googlesource.com/4563
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:22:36 +00:00
David Benjamin
cb9cf796a2 Promote everything directly in SSL_ctrl.
BUG=404754

Change-Id: I0b0a1c35f3dc81e81deb34d409ae18a1d248669e
Reviewed-on: https://boringssl-review.googlesource.com/4561
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:15:27 +00:00
David Benjamin
2844b5eb1e Remove SSL_get0_raw_cipherlist.
The API is unused and rather awkward (mixes output parameters with
return values, special-case for NULL).

Change-Id: I4396f98534bf1271e53642f255e235cf82c7615a
Reviewed-on: https://boringssl-review.googlesource.com/4560
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:14:15 +00:00
David Benjamin
1d0a194cc1 Promote max_cert_list and max_send_fragment to functions.
Also size them based on the limits in the quantities they control (after
checking bounds at the API boundary).

BUG=404754

Change-Id: Id56ba45465a473a1a793244904310ef747f29b63
Reviewed-on: https://boringssl-review.googlesource.com/4559
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:14:07 +00:00
David Benjamin
da881e9a15 Promote deprecated statistics macros to functions.
Not going to bother adding the compatibility macros. If they get ifdef'd
out, all the better.

BUG=404754

Change-Id: I26414d2fb84ee1f0b15a3b96c871949fe2bb7fb1
Reviewed-on: https://boringssl-review.googlesource.com/4558
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:13:13 +00:00
David Benjamin
61ecccfa3e Promote read_ahead, mode and option macros to functions.
BUG=404754

Change-Id: Ieeeb538bd25854d5664e33fe3fa79bd686c26704
Reviewed-on: https://boringssl-review.googlesource.com/4557
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:12:48 +00:00
David Benjamin
123a8fdb98 Switch options and mode bits to uint32_t from unsigned long.
This is a bitmask, so the number of bits available should be the same
across all platforms.

Change-Id: I98e8d375fc7d042aeae1270174bc8fc63fba5dfc
Reviewed-on: https://boringssl-review.googlesource.com/4556
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:11:15 +00:00
David Benjamin
8c24980d83 Promote all dtls1_ctrl hooks to functions.
BUG=404754

Change-Id: I5f11485fbafa07cddcf2612e2f616f90bf7c722d
Reviewed-on: https://boringssl-review.googlesource.com/4554
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:11:05 +00:00
David Benjamin
59015c365b Promote all SSL callback ctrl hooks to proper functions.
Document them while I'm here. This adds a new 'preprocessor
compatibility section' to avoid breaking #ifdefs. The CTRL values
themselves are defined to 'doesnt_exist' to catch anything calling
SSL_ctrl directly until that function can be unexported completely.

BUG=404754

Change-Id: Ia157490ea8efe0215d4079556a0c7643273e7601
Reviewed-on: https://boringssl-review.googlesource.com/4553
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:10:47 +00:00
Adam Langley
3457cd943a Cast to size_t to avoid MSVC warning.
Previously:
..\ssl\t1_lib.c(2160) : warning C4018: '<' : signed/unsigned mismatch

Change-Id: I342e2ac1de71e9335659f5e5ed13423b1c7860d7
2015-05-05 12:05:01 -07:00
David Benjamin
9362b6e235 Errors are uint32_t, not unsigned long.
Change-Id: Ic2339b771d949a555b8d05a3b24dc2e990b9d8d3
Reviewed-on: https://boringssl-review.googlesource.com/4555
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:48:01 +00:00
David Benjamin
adcc39560e Tidy up ticket length checks.
When tlsext_ticket_key_cb is used, the full bounds aren't known until
after the callback has returned.

Change-Id: I9e89ffae6944c74c4ca04e6aa28afd3ec80aa1d4
Reviewed-on: https://boringssl-review.googlesource.com/4552
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:39:51 +00:00
David Benjamin
f32d6b292e Remove remnants of old OCSP stapling API.
Probably we'll want some simpler server-side API later. But, as things
stand, all consumers of these functions are #ifdef'd out and have to be
because the requisite OCSP_RESPONSE types are gone.

Change-Id: Ic82b2ab3feca14c56656da3ceb3651819e3eb377
Reviewed-on: https://boringssl-review.googlesource.com/4551
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:31:15 +00:00
David Benjamin
9a727c21c1 Remove SSL_set_tlsext_debug_callback.
It's unused, but for some old #ifdef branch in wpa_supplicant's EAP-FAST
hack, before SSL_set_session_ticket_ext_cb existed.

Change-Id: Ifc11fea2f6434354f756e04e5fc3ed5f1692025e
Reviewed-on: https://boringssl-review.googlesource.com/4550
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:30:52 +00:00
Adam Langley
97e8ba8d1d Rename ECDHE-PSK-WITH-AES-128-GCM-SHA256 to follow the naming conventions.
“ECDHE-PSK-WITH-AES-128-GCM-SHA256” doesn't follow the standard naming
for OpenSSL: it was “-WITH-” in it and has a hyphen between “AES” and
“128”. This change fixes that.

Change-Id: I7465b1ec83e7d5b9a60d8ca589808aeee10c174e
Reviewed-on: https://boringssl-review.googlesource.com/4601
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 00:33:32 +00:00
Adam Langley
f868409124 Fix SSL_get0_chain_certs.
SSL_get0_chain_certs calls a ctrl function with
SSL_CTRL_GET_CHAIN_CERTS. The switch failed to set a positive return
value and so the call always appeared to fail.

Change-Id: If40ca7840197a9748fd69b761fd905f44bb79835
Reviewed-on: https://boringssl-review.googlesource.com/4521
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 00:28:44 +00:00
David Benjamin
687937304b Revert "Temporarily break a handful of tests."
This reverts commit a921d550d0.
2015-05-04 20:21:32 -04:00
David Benjamin
a921d550d0 Temporarily break a handful of tests.
This will be reverted in a minute. The bots should run both suites of tests and
report the names of all failing tests in the summary.

Change-Id: Ibe351017dfa8ccfd182b3c88eee413cd2cbdeaf0
2015-05-04 20:17:28 -04:00
David Benjamin
2755a3eda3 Remove unnecessary NULL checks, part 5.
Finally, the ssl stack.

Change-Id: Iea10e302825947da36ad46eaf3e8e2bce060fde2
Reviewed-on: https://boringssl-review.googlesource.com/4518
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:16:19 +00:00
David Benjamin
5d1ec73b0f Require that FOO_free functions do nothing on NULL.
This is consistent with C's free function and upstream's convention.

Change-Id: I83f6e2f5824e28f69a9916e580dc2d8cb3b94234
Reviewed-on: https://boringssl-review.googlesource.com/4512
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:58:13 +00:00
David Benjamin
4fcc2e2031 Make a few variable names saner.
Change-Id: I6790dc9651dc400992fc59a4c900210edeb2520c
Reviewed-on: https://boringssl-review.googlesource.com/4511
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:58:05 +00:00
David Benjamin
ed8fbad170 Remove SSL cert_flags.
These are never used and no flags are defined anyway.

Change-Id: I206dc2838c5f68d87559a702dcb299b208cc7e1e
Reviewed-on: https://boringssl-review.googlesource.com/4493
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:48:13 +00:00
David Benjamin
67be048e1a Convert ec_test to C++
Change-Id: I5e25ddbc87370b58d9b6fc410f51e259947df8dd
Reviewed-on: https://boringssl-review.googlesource.com/4468
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 21:00:34 +00:00
David Benjamin
90da8c8817 Test that the server picks a non-ECC cipher when no curves are supported.
Change-Id: I9cd788998345ad877f73dd1341ccff68dbb8d124
Reviewed-on: https://boringssl-review.googlesource.com/4465
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 20:55:09 +00:00
David Benjamin
dd978784d7 Always enable ecdh_auto.
This is a really dumb API wart. Now that we have a limited set of curves that
are all reasonable, the automatic logic should just always kick in. This makes
set_ecdh_auto a no-op and, instead of making it the first choice, uses it as
the fallback behavior should none of the older curve selection APIs be used.

Currently, by default, server sockets can only use the plain RSA key exchange.

BUG=481139

Change-Id: Iaabc82de766cd00968844a71aaac29bd59841cd4
Reviewed-on: https://boringssl-review.googlesource.com/4531
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 20:51:05 +00:00
David Benjamin
55a436497f Handle empty curve preferences from the client.
See upstream's bd891f098bdfcaa285c073ce556d0f5e27ec3a10. It honestly seems
kinda dumb for a client to do this, but apparently the spec allows this.
Judging by code inspection, OpenSSL 1.0.1 also allowed this, so this avoids a
behavior change when switching from 1.0.1 to BoringSSL.

Add a test for this, which revealed that, unlike upstream's version, this
actually works with ecdh_auto since tls1_get_shared_curve also needs updating.
(To be mentioned in newsletter.)

Change-Id: Ie622700f17835965457034393b90f346740cfca8
Reviewed-on: https://boringssl-review.googlesource.com/4464
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 20:44:01 +00:00
David Benjamin
1c722b7781 Add DTLS to SSL_get_version
(Imported from upstream's 504e643e0996fb842ac183023c3a6b9049af50ea)

Change-Id: I94f4fea105b6eeb001d376ca3ffef77d1432aa88
Reviewed-on: https://boringssl-review.googlesource.com/4462
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 20:38:45 +00:00
David Benjamin
dcd979f1a4 CertificateStatus is optional.
Because RFC 6066 is obnoxious like that and IIS servers actually do this
when OCSP-stapling is configured, but the OCSP server cannot be reached.

BUG=478947

Change-Id: I3d34c1497e0b6b02d706278dcea5ceb684ff60ae
Reviewed-on: https://boringssl-review.googlesource.com/4461
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 20:36:57 +00:00
David Benjamin
83f182c6c5 Revert "Temporary introduce a memory leak in ssl_test"
This reverts commit 88d0f91952.
2015-04-21 06:32:24 -04:00
David Benjamin
88d0f91952 Temporary introduce a memory leak in ssl_test
Making sure the ASan bot notices. This will be reverted in a minute.

Change-Id: Ib8acf21948e3638d54ab09f3df4c339c4ea3d649
2015-04-21 06:30:59 -04:00
David Benjamin
c574f4114d Test that client curve preferences are enforced.
Change-Id: Idc8ac43bd59607641ac2ad0b7179b2f942c0b0ce
Reviewed-on: https://boringssl-review.googlesource.com/4403
Reviewed-by: Adam Langley <agl@google.com>
2015-04-20 18:59:15 +00:00
David Benjamin
93de5e5c11 Reject empty cipher suite lists early.
See upstream's 3ae91cfb327c9ed689b9aaf7bca01a3f5a0657cb.

I misread that code and thought it was allowing empty cipher suites when there
*is* a session ID, but it was allowing them when there isn't. Which doesn't
make much sense because it'll get rejected later anyway. (Verified by toying
with handshake_client.go.)

Change-Id: Ia870a1518bca36fce6f3018892254f53ab49f460
Reviewed-on: https://boringssl-review.googlesource.com/4401
Reviewed-by: Adam Langley <agl@google.com>
2015-04-20 18:55:20 +00:00
David Benjamin
3fa27774b4 Fix some unsigned long cipher masks.
107db58047 missed a few.

Change-Id: Ib1c7e85e7de7e26888be17d3b644d856b134f76e
Reviewed-on: https://boringssl-review.googlesource.com/4400
Reviewed-by: Adam Langley <agl@google.com>
2015-04-20 18:54:37 +00:00
Adam Langley
caf6b09598 runner: fix a couple of nits from govet.
Change-Id: I489d00bc4ee22a5ecad75dc1eb84776f044566e5
Reviewed-on: https://boringssl-review.googlesource.com/4391
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-04-17 21:45:50 +00:00
Matt Braithwaite
7eaf4015fa (d2i_SSL_SESSION): Fix a memory leak that occurs on error.
Change-Id: I5424617f0b12937f092698d35a99316eb73a6c35
Reviewed-on: https://boringssl-review.googlesource.com/4390
Reviewed-by: Adam Langley <agl@google.com>
2015-04-17 17:39:38 +00:00
David Benjamin
9f33fc63c6 Remove hash table lookups from ex_data.
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>
2015-04-15 23:59:35 +00:00
David Benjamin
546f1a59ef Unexpose the generic ex_data functions.
Callers are required to use the wrappers now. They still need OPENSSL_EXPORT
since crypto and ssl get built separately in the standalone shared library
build.

Change-Id: I61186964e6099b9b589c4cd45b8314dcb2210c89
Reviewed-on: https://boringssl-review.googlesource.com/4372
Reviewed-by: Adam Langley <agl@google.com>
2015-04-15 23:27:22 +00:00
David Benjamin
25f0846316 Revert "Temporarily break a test on purpose."
This reverts commit cbbe020894.
2015-04-15 16:13:49 -04:00
David Benjamin
cbbe020894 Temporarily break a test on purpose.
This is to make sure emails get sent to the right place. This will be reverted
in a minute.

Change-Id: I657e8c32034deb2231b76c1a418bdc5dcf6be8bd
2015-04-15 15:59:07 -04:00
David Benjamin
b16346b0ad Add SSL_set_reject_peer_renegotiations.
This causes any unexpected handshake records to be met with a fatal
no_renegotiation alert.

In addition, restore the redundant version sanity-checks in the handshake state
machines. Some code would zero the version field as a hacky way to break the
handshake on renego. Those will be removed when switching to this API.

The spec allows for a non-fatal no_renegotiation alert, but ssl3_read_bytes
makes it difficult to find the end of a ClientHello and skip it entirely. Given
that OpenSSL goes out of its way to map non-fatal no_renegotiation alerts to
fatal ones, this seems probably fine. This avoids needing to account for
another source of the library consuming an unbounded number of bytes without
returning data up.

Change-Id: Ie5050d9c9350c29cfe32d03a3c991bdc1da9e0e4
Reviewed-on: https://boringssl-review.googlesource.com/4300
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:38:58 +00:00
David Benjamin
2bf1a79654 Prune some unused constants from ssl/internal.h.
Change-Id: Iae9e064261cf7cb2968520812e2f242d7f643ecc
Reviewed-on: https://boringssl-review.googlesource.com/4293
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:07:38 +00:00
David Benjamin
71f0794d34 Document everything in ssl_ciph.c, now ssl_cipher.c.
Just about everything depends on SSL_CIPHER. Move it to the top as the first
section in ssl.h. Match the header order and the source file order and document
everything. Also make a couple of minor style guide tweaks.

Change-Id: I6a810dbe79238278ac480e5ced1447055715a79f
Reviewed-on: https://boringssl-review.googlesource.com/4290
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:06:55 +00:00
David Benjamin
5c366d62e8 Remove dead code.
That block is slightly unreachable.

Change-Id: I1b4b2d8b1cd4bb7137ce0aac4b65079545cd9264
Reviewed-on: https://boringssl-review.googlesource.com/4286
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:06:00 +00:00
David Benjamin
6b38086341 Remove SSL_CIPHER::valid.
It's no longer needed to distinguish ciphers from fake ciphers.

Change-Id: I1ad4990ba936b1059eb48f3d2f309eb832dd1cb5
Reviewed-on: https://boringssl-review.googlesource.com/4285
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:05:41 +00:00
David Benjamin
0344dafb71 Tidy cipher rule processing.
Rather than shoehorn real ciphers and cipher aliases into the same type (that's
what cipher->valid is used for), treat them separately. Make
ssl_cipher_apply_rule match ciphers by cipher_id (the parameter was ignored and
we assumed that masks uniquely identify a cipher) and remove the special cases
around zero for all the masks. This requires us to remember which fields
default to 0 and which default to ~0u, but the logic is much clearer.

Finally, now that ciphers and cipher aliases are different, don't process rules
which sum together an actual cipher with cipher aliases. This would AND
together the masks for the alias with the values in the cipher and do something
weird around alg_ssl. (alg_ssl is just weird in general, as everyone trying to
disable SSLv3 in OpenSSL recently discovered.)

With all that, we can finally remove cipher->valid which was always one.

Change-Id: Iefcfe159bd6c22dbaea3a5f1517bd82f756dcfe1
Reviewed-on: https://boringssl-review.googlesource.com/4284
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:05:10 +00:00
Brian Smith
054e682675 Eliminate unnecessary includes from low-level crypto modules.
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>
2015-04-13 20:49:18 +00:00
Brian Smith
83a82981dc Rename BIO_print_errors_fp back to ERR_print_errors_fp & refactor it.
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>
2015-04-13 20:23:29 +00:00
David Benjamin
107db58047 Switch cipher masks to uint32_t.
These are all masks of some sort (except id which is a combined version and
cipher), so they should use fixed-size unsigned integers.

Change-Id: I058dd8ad231ee747df4b4fb17d9c1e2cbee21918
Reviewed-on: https://boringssl-review.googlesource.com/4283
Reviewed-by: Adam Langley <agl@google.com>
2015-04-10 22:16:05 +00:00
David Benjamin
f0ae170021 Include-what-you-use ssl/internal.h.
The rest of ssl/ still includes things everywhere, but this at least fixes the
includes that were implicit from ssl/internal.h.

Change-Id: I7ed22590aca0fe78af84fd99a3e557f4b05f6782
Reviewed-on: https://boringssl-review.googlesource.com/4281
Reviewed-by: Adam Langley <agl@google.com>
2015-04-10 22:15:02 +00:00
David Benjamin
2ee94aabf5 Rename ssl_locl.h to internal.h
Match the other internal headers.

Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa
Reviewed-on: https://boringssl-review.googlesource.com/4280
Reviewed-by: Adam Langley <agl@google.com>
2015-04-10 22:14:09 +00:00