Commit Graph

706 Commits

Author SHA1 Message Date
David Benjamin
ac4de241b1 Zero s->packet when releasing the read buffer.
s->packet points into the read buffer. It shouldn't leave a dangling pointer.

Change-Id: Ia7def2f50928ea9fca8cb0b69d614a92f9f47f57
Reviewed-on: https://boringssl-review.googlesource.com/4684
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 18:39:26 +00:00
David Benjamin
aebefed905 Always enable SSL_MODE_RELEASE_BUFFERS.
There's no real need to ever disable it, so this is one fewer configuration to
test. It's still disabled for DTLS, but a follow-up will resolve that.

Change-Id: Ia95ad8c17ae8236ada516b3968a81c684bf37fd9
Reviewed-on: https://boringssl-review.googlesource.com/4683
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 18:39:09 +00:00
David Benjamin
4d2e7ce47b Remove OPENSSL_timeval.
With DTLSv1_get_timeout de-ctrl-ified, the type checker complains about
OPENSSL_timeval. Existing callers all use the real timeval.

Now that OPENSSL_timeval is not included in any public structs, simply
forward-declare timeval itself in ssl.h and pull in winsock2.h in internal
headers.

Change-Id: Ieaf110e141578488048c28cdadb14881301a2ce1
Reviewed-on: https://boringssl-review.googlesource.com/4682
Reviewed-by: Adam Langley <agl@google.com>
2015-05-08 18:03:07 +00:00
David Benjamin
593047fd80 Opaquify DTLS structs.
Nothing ever uses those structs. This to avoid having any structs in the
public header which use struct timeval.

In doing so, move the protocol version constants up to ssl.h so dtls1.h
may be empty. This also removes TLS1_get_version and TLS1_get_client_version
as they're unused and depend on TLS1_VERSION_MAJOR. This still lets tls1.h
be included independently from ssl.h (though I don't think anyone ever includes
it...).

Change-Id: Ieac8b90cf94f7f1e742a88bb75c0ee0aa4b1414c
Reviewed-on: https://boringssl-review.googlesource.com/4681
Reviewed-by: Adam Langley <agl@google.com>
2015-05-08 18:02:02 +00:00
David Benjamin
a24265cfb1 Fix random magic number in ssl3_output_cert_chain.
Per earlier review comment. The number is wrong anyway. (Neither version does
anything since init_buf is initialized to a large size and most functions don't
bother sizing it. Future work should rewrite all of this to use a CBB.)

Change-Id: I3b58672b328396459a34c6403f8bfb77c96efe9c
Reviewed-on: https://boringssl-review.googlesource.com/4650
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 23:25:58 +00:00
David Benjamin
9a10f8fd88 Switch EVP_PKEY_dup calls to EVP_PKEY_up_ref.
Keep internal callers up-to-date with deprecations.

Change-Id: I7ee171afc669592d170f83bd4064857d59332878
Reviewed-on: https://boringssl-review.googlesource.com/4640
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:57:09 +00:00
David Benjamin
6abb37016e Remove ciphers_raw.
With SSL_get0_raw_cipherlist gone, there's no need to hold onto it.

Change-Id: I258f8bfe21cc354211a777660df680df6c49df2a
Reviewed-on: https://boringssl-review.googlesource.com/4616
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:56:31 +00:00
David Benjamin
d6e95eefba Get rid of ssl_undefined_*
The only place using it is export keying material which can do the
version check inline.

Change-Id: I1893966c130aa43fa97a6116d91bb8b04f80c6fb
Reviewed-on: https://boringssl-review.googlesource.com/4615
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:56:02 +00:00
David Benjamin
60da0cd7c6 Fix STACK_OF pointer style.
clang-format got a little confused there.

Change-Id: I46df523e8a7813a2b4e243da3df22851b3393873
Reviewed-on: https://boringssl-review.googlesource.com/4614
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:55:16 +00:00
David Benjamin
605641ed95 Move the NULL case in ssl_add_cert_chain up.
It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)

Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:53:53 +00:00
Adam Langley
e92d24f323 Build fix.
(Semantic no-op.)

Change-Id: I94d3ae12bc82f5080e3cf1405cca79acb316f798
2015-05-06 15:47:17 -07:00
David Benjamin
8eb65e814c Remove dead field from CIPHER_ORDER.
It's unused.

Change-Id: I039ecc40f90cbeed6e95b1dd8414161670ae5b6c
Reviewed-on: https://boringssl-review.googlesource.com/4612
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:36:31 +00:00
David Benjamin
7133d428dd Promote SNI macros to functions.
BUG=404754

Change-Id: I2b2e27f3db0c97f2db65ca5e226c6488d2bee2fc
Reviewed-on: https://boringssl-review.googlesource.com/4570
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:36:19 +00:00
David Benjamin
c2807582fd Promote channel ID macros to proper functions.
BUG=404754

Change-Id: I002d4602720e207f92a985d90f0d58e89562affa
Reviewed-on: https://boringssl-review.googlesource.com/4569
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:33:59 +00:00
David Benjamin
15a3b000cf Promote set_tmp_dh and set_tmp_ecdh to functions.
BUG=404754

Change-Id: I7c75dd88fe9338b1d3b90745f742d15d6b84775a
Reviewed-on: https://boringssl-review.googlesource.com/4568
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:30:22 +00:00
David Benjamin
c045469817 Promote a few more macros.
Next batch. Mostly a bunch of deprecated things. This switches
SSL_CTX_set_tmp_rsa from always failing to always succeeding. The latter
is probably a safer behavior; a consumer may defensively set a temporary
RSA key. We'll successfully "set it" and just never use the result.

Change-Id: Idd3d6bf4fc1a20bc9a26605bb9c77c9f799f993c
Reviewed-on: https://boringssl-review.googlesource.com/4566
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:28:12 +00:00
David Benjamin
9f226a5f51 Always set SSL_OP_SINGLE_DH_USE.
This is an API wart that makes it easy to accidentally reuse the server
DHE half for every handshake. It's much simpler to have only one mode.
This mirrors the change made to the ECDHE code; align with that logic.

Change-Id: I47cccbb354d70127ab458f99a6d390b213e4e515
Reviewed-on: https://boringssl-review.googlesource.com/4565
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:24:53 +00:00
David Benjamin
7cde0dee7c Fold num_renegotiations into total_renegotiations.
The only difference is SSL_clear_num_renegotiations which is never
called.

Change-Id: Id661c71e89d34d834349ad1f1a296e332606e6cc
Reviewed-on: https://boringssl-review.googlesource.com/4564
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:23:04 +00:00
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