Commit Graph

917 Commits

Author SHA1 Message Date
Doug Hogan
ecdf7f9986 Minor whitespace update after running go fmt with 1.4.2.
Change-Id: I20d0a43942359ac18afec670cf7fd38f56b369b1
Reviewed-on: https://boringssl-review.googlesource.com/5400
Reviewed-by: Adam Langley <agl@google.com>
2015-07-10 01:50:26 +00:00
Adam Langley
97dfcbfc7c Convert the NPN extension to the new system
Change-Id: I92d76aef5a55d5fdef1b9fa24dd7db8047e56df0
Reviewed-on: https://boringssl-review.googlesource.com/5364
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 23:13:52 +00:00
Adam Langley
bb0bd04d15 Convert the status_request (OCSP stapling) extension to the new system
Change-Id: I31dd9e9f523aee3700bb2f07a1624d124b157d3e
Reviewed-on: https://boringssl-review.googlesource.com/5363
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 23:13:24 +00:00
Adam Langley
2e857bdad3 Convert the signature algorithms extension to the new system
Change-Id: Ia53b434acd11e9d2b0151b967387d86745ae441f
Reviewed-on: https://boringssl-review.googlesource.com/5362
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 23:12:51 +00:00
Adam Langley
9b05bc5caf Convert the session ticket extension to the new system.
Change-Id: I3893db92e87c0c041fe4ab489e867706902f1c43
Reviewed-on: https://boringssl-review.googlesource.com/5361
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 23:06:38 +00:00
Adam Langley
0a05671f76 Switch EMS over to the new extensions system.
Change-Id: I2a368a43bd05a75e37f180048c45c48de140b1eb
Reviewed-on: https://boringssl-review.googlesource.com/5360
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 23:05:24 +00:00
Adam Langley
efb0e16ee5 Reject empty ALPN protocols.
https://tools.ietf.org/html/rfc7301#section-3.1 specifies that a
ProtocolName may not be empty. This change enforces this in ClientHello
and ServerHello messages.

Thanks to Doug Hogan for reporting this.

Change-Id: Iab879c83145007799b94d2725201ede1a39e4596
Reviewed-on: https://boringssl-review.googlesource.com/5390
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 22:47:14 +00:00
David Benjamin
d822ed811a Make CBB_len return a length, not remaining.
It switched from CBB_remaining to CBB_len partway through review, but
the semantics are still CBB_remaining. Using CBB_len allows the
len_before/len_after logic to continue working even if, in the future,
handshake messages are built on a non-fixed CBB.

Change-Id: Id466bb341a14dbbafcdb26e4c940a04181f2787d
Reviewed-on: https://boringssl-review.googlesource.com/5371
Reviewed-by: Adam Langley <agl@google.com>
2015-07-09 19:20:09 +00:00
Adam Langley
b558c4c504 Add a test case for renegotation with False Start enabled.
Historically we had a bug around this and this change implements a test
that we were previously carrying internally.

Change-Id: Id181fedf66b2b385b54131ac91d74a31f86f0205
Reviewed-on: https://boringssl-review.googlesource.com/5380
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-07-08 20:30:55 +00:00
David Benjamin
74f711083d Parse RSAPrivateKey with CBS.
This removes the version field from RSA and instead handles versioning
as part of parsing. (As a bonus, we now correctly limit multi-prime RSA
to version 1 keys.)

Most consumers are also converted. old_rsa_priv_{de,en}code are left
alone for now. Those hooks are passed in parameters which match the old
d2i/i2d pattern (they're only used in d2i_PrivateKey and
i2d_PrivateKey).

Include a test which, among other things, checks that public keys being
serialized as private keys are handled properly.

BUG=499653

Change-Id: Icdd5f0382c4a84f9c8867024f29756e1a306ba08
Reviewed-on: https://boringssl-review.googlesource.com/5273
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 22:50:53 +00:00
Adam Langley
6df1ac9092 Fix Windows build.
Now that 11c0f8e54c has landed, none of
the cases of the switch in |ssl3_ctrl| ever break and so the “return 1”
at the end of the function is unreachable. MSVC is unhappy about that.

Change-Id: I001dc63831ba60d93b622ac095297e2febc5f078
2015-07-06 19:01:48 -07:00
David Benjamin
71d2e54099 Clear key_method in ssl_cert_clear_certs.
Since it resets leaf, private key, and chain, it makes sense to also
clear custom key method tables.

Change-Id: If511b8f15a44674c31d068d36984e9189c5a9071
Reviewed-on: https://boringssl-review.googlesource.com/5356
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:56:11 +00:00
David Benjamin
11c0f8e54c Promote certificate-related ctrl macros to functions.
Also document them in the process. Almost done!

BUG=404754

Change-Id: I3333c7e9ea6b4a4844f1cfd02bff8b5161b16143
Reviewed-on: https://boringssl-review.googlesource.com/5355
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:55:39 +00:00
David Benjamin
7481d39bf7 Document APIs relating to configuring certificates and private keys.
The APIs that are CTRL macros will be documented (and converted to
functions) in a follow-up.

Change-Id: I7d086db1768aa3c16e8d7775b0c818b72918f4c2
Reviewed-on: https://boringssl-review.googlesource.com/5354
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:52:39 +00:00
David Benjamin
b2a9d6ab78 Remove SSL_build_cert_chain.
This is unused. It seems to be distinct from the automatic chain
building and was added in 1.0.2. Seems to be an awful lot of machinery
that consumers ought to configure anyway.

BUG=486295

Change-Id: If3d4a2761f61c5b2252b37d4692089112fc0ec21
Reviewed-on: https://boringssl-review.googlesource.com/5353
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:23:18 +00:00
David Benjamin
4462809623 Remove SSL_CTX_select_current_cert.
Without certificate slots this function doesn't do anything. It's new in
1.02 and thus unused, so get rid of it rather than maintain a
compatibility stub.

BUG=486295

Change-Id: I798fce7e4307724756ad4e14046f1abac74f53ed
Reviewed-on: https://boringssl-review.googlesource.com/5352
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:22:32 +00:00
David Benjamin
d1d8078025 Fold away certificate slots mechanism.
This allows us to remove the confusing EVP_PKEY argument to the
SSL_PRIVATE_KEY_METHOD wrapper functions. It also simplifies some of the
book-keeping around the CERT structure, as well as the API for
configuring certificates themselves. The current one is a little odd as
some functions automatically route to the slot while others affect the
most recently touched slot. Others still (extra_certs) apply to all
slots, making them not terribly useful.

Consumers with complex needs should use cert_cb or the early callback
(select_certificate_cb) to configure whatever they like based on the
ClientHello.

BUG=486295

Change-Id: Ice29ffeb867fa4959898b70dfc50fc00137f01f3
Reviewed-on: https://boringssl-review.googlesource.com/5351
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:22:13 +00:00
David Benjamin
570364800c Remove SSL_CTX_get_extra_chain_certs_only.
This is in preparation for folding away certificate slots. extra_certs
and the slot-specific certificate chain will be the same.
SSL_CTX_get_extra_chain_certs already falls back to the slot-specific
chain if missing. SSL_CTX_get_extra_chain_certs_only is similar but
never falls back. This isn't very useful and is confusing with them
merged, so remove it.

BUG=486295

Change-Id: Ic708105bcf453dfe4e1969353d7eb7547ed2981b
Reviewed-on: https://boringssl-review.googlesource.com/5350
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:16:20 +00:00
David Benjamin
bb20f52383 Merge the RSA_ENC and RSA_SIGN certificate slots.
The distinction was not well-enforced in the code. In fact, it wasn't
even possible to use the RSA_SIGN slot because ssl_set_pkey and
ssl_set_cert would always use the RSA_ENC slot.

A follow-up will fold away the mechanism altogether, but this is an easy
initial simplfication.

BUG=486295

Change-Id: I66b5bf3e6dc243dac7c75924c1c1983538e49060
Reviewed-on: https://boringssl-review.googlesource.com/5349
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:15:41 +00:00
David Benjamin
0fc431a0d7 Prune NIDs from TLS_SIGALGS.
There's no need to store more than the TLS values.

Change-Id: I1a93c7c6aa3254caf7cc09969da52713e6f8acf4
Reviewed-on: https://boringssl-review.googlesource.com/5348
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:14:40 +00:00
David Benjamin
ba16a1e405 Remove SSL_get_(shared_)sigalgs.
These are new as of 1.0.2, not terribly useful of APIs, and are the only
reason we have to retain so many NIDs in the TLS_SIGALGS structure.

Change-Id: I7237becca09acc2ec2be441ca17364f062253893
Reviewed-on: https://boringssl-review.googlesource.com/5347
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:12:24 +00:00
David Benjamin
7d97fd3a91 Remove SSL_get_server_tmp_key.
It's never used and is partially broken right now; EVP_PKEY_DH doesn't
work.

Change-Id: Id6262cd868153ef731e3f4d679b2ca308cfb12a3
Reviewed-on: https://boringssl-review.googlesource.com/5343
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:10 +00:00
David Benjamin
396a441421 ssl3_cert_verify_hash should take the EVP_PKEY type.
After the custom key method support, the EVP_PKEY parameter is somewhat
confusing (to be resolved with the certificate slots removal) as it must
always refer to a private key. ssl3_cert_verify_hash is sometimes used
with the peer's public key. If custom keys were supported on the server,
this would break.

Fix this by passing a pkey_type parameter and letting the caller decide
whether this uses SSL_PRIVATE_KEY_METHOD or not.

Change-Id: I673b92579a84b4561f28026ec0b1c78a6bfee440
Reviewed-on: https://boringssl-review.googlesource.com/5341
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:10:35 +00:00
David Benjamin
67dc83851b Fix ssl3_send_client_certificate state machine.
If ssl_do_write takes more than one iteration, ssl3_output_cert_chain
would be called an extra time. This is very unlikely in practice because
of the buffer BIO.

Change-Id: Ic1ae9752a8837bb404429fc60306c659208c6185
Reviewed-on: https://boringssl-review.googlesource.com/5340
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:09:45 +00:00
David Benjamin
dfa4069a16 Fix MSVC build some more.
Missed that there were two of them.

Change-Id: Ibab169ef1f75be9c5ad1ffa0f232629e76a4512d
Reviewed-on: https://boringssl-review.googlesource.com/5332
Reviewed-by: Adam Langley <agl@google.com>
2015-07-02 00:02:27 +00:00
David Benjamin
3e536334be Fix Windows build.
It (incorrectly) thinks some variables are uninitialized. It also gets confused
about some const parameters.

Change-Id: Ie2b3a5336692e7293cf03d6a4cd5345d30b628b3
Reviewed-on: https://boringssl-review.googlesource.com/5330
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 23:01:05 +00:00
Matt Braithwaite
6a1275b627 typedef |COMP_METHOD| to void, for compatibility.
Change-Id: I2965c0d698e25f4eb08ddd9fc000b0e4adf6f2c0
Reviewed-on: https://boringssl-review.googlesource.com/5240
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 21:49:06 +00:00
Håvard Molland
047d6f0f4d Remove ssl2.h and ssl23.h.
The SSL23_ST_foo macros are only used in ssl_stat.c.
However, these states are never set and can be removed.

Move the two remaining SSLv2 client hello record macros to ssl3.h

Change-Id: I76055405a9050cf873b4d1cbc689e54dd3490b8a
Reviewed-on: https://boringssl-review.googlesource.com/4160
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 21:47:01 +00:00
David Benjamin
a8653208ec Add CBB_zero to set a CBB to the zero state.
One tedious thing about using CBB is that you can't safely CBB_cleanup
until CBB_init is successful, which breaks the general 'goto err' style
of cleanup. This makes it possible:

  CBB_zero ~ EVP_MD_CTX_init
  CBB_init ~ EVP_DigestInit
  CBB_cleanup ~ EVP_MD_CTX_cleanup

Change-Id: I085ecc4405715368886dc4de02285a47e7fc4c52
Reviewed-on: https://boringssl-review.googlesource.com/5267
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 19:45:43 +00:00
David Benjamin
961ad6ad2c Sign channel IDs with ECDSA_do_sign.
Rather than parse with d2i_ECDSA_SIG and reserialize, this is cleaner.
It's also clearer that i2d_PublicKey isn't being used for DER.

Change-Id: Iac57fb6badd1dfed1e66984e95a31f609b1538a4
Reviewed-on: https://boringssl-review.googlesource.com/5263
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 19:38:04 +00:00
David Benjamin
e3aa1d9dd4 Cleanup ticket processing and session lookup.
Use more sensible variable names. Also move some work between the helpers and
s3_srvr.c a little; the session lookup functions now only return a new session.
Whether to send a ticket is now an additional output to avoid the enum
explosion around renewal. The actual SSL state is not modified.

This is somewhat cleaner as s3_srvr.c may still reject a session for other
reasons, so we avoid setting ssl->session and ssl->verify_result to a session
that wouldn't be used. (They get fixed up in ssl_get_new_session, so it didn't
actually matter.)

Change-Id: Ib52fabbe993b5e2b7408395a02cdea3dee66df7b
Reviewed-on: https://boringssl-review.googlesource.com/5235
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 19:33:23 +00:00
Adam Langley
5021b223d8 Convert the renegotiation extension to the new system.
This change also switches the behaviour of the client. Previously the
client would send the SCSV rather than the extension, but now it'll only
do that for SSLv3 connections.

Change-Id: I67a04b8abbef2234747c0dac450458deb6b0cd0a
Reviewed-on: https://boringssl-review.googlesource.com/5143
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 19:30:53 +00:00
Adam Langley
4cfa96bd26 Move compile-time asserts to the top-level.
It's still the case that we have many old compilers that can't cope with
anything else ☹.

Change-Id: Ie5a1987cd5164bdbde0c17effaa62aecb7d12352
Reviewed-on: https://boringssl-review.googlesource.com/5320
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 18:57:57 +00:00
Adam Langley
614c66a2f8 Add infrastructure for better extension handling.
Rather than four massive functions that handle every extension,
organise the code by extension with four smaller functions for each.

Change-Id: I876b31dacb05aca9884ed3ae7c48462e6ffe3b49
Reviewed-on: https://boringssl-review.googlesource.com/5142
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 18:25:28 +00:00
Adam Langley
be9eda4a88 Fix Renegotiate-Client-NoExt test.
This test shouldn't trigger a renegotiation: the test is trying to
assert that without the legacy-server flag set, a server that doesn't
echo the renegotiation extension can't be connected to.

Change-Id: I1368d15ebc8f296f3ff07040c0e6c48fdb49e56f
Reviewed-on: https://boringssl-review.googlesource.com/5141
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 17:56:19 +00:00
David Benjamin
6cacac033b Promote SSL_CTX_[gs]et_tlsext_ticket_keys to functions.
BUG=404754

Change-Id: Iae75a7ab24d4aa3b30edf578cbfc1058aeadd863
Reviewed-on: https://boringssl-review.googlesource.com/5233
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 22:39:36 +00:00
David Benjamin
bed63548d1 Increment the reference count of sessions inside the lock.
Otherwise another thread may cause the session to be destroyed first.

Change-Id: I2084a28ece11540e1b8f289553161d99395e2d1f
Reviewed-on: https://boringssl-review.googlesource.com/5231
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 22:35:51 +00:00
David Benjamin
d98452d2db Add a test for the ticket callback.
Change-Id: I7b2a4f617bd8d49c86fdaaf45bf67e0170bbd44f
Reviewed-on: https://boringssl-review.googlesource.com/5230
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 22:34:11 +00:00
Adam Langley
bc94929290 bssl_shim: move large buffer to heap.
This change reduces the amount of stack needed by bssl_shim by moving a
large buffer to the heap.

Change-Id: I3a4bcf119218d98046ff15320433a1012be1615d
2015-06-18 21:32:44 -07:00
David Benjamin
ba4594aee6 Don't put sessions from renegotiations in the cache.
Rather than rely on Chromium to query SSL_initial_handshake_complete in the
callback (which didn't work anyway because the callback is called afterwards),
move the logic into BoringSSL. BoringSSL already enforces that clients never
offer resumptions on renegotiation (it wouldn't work well anyway as client
session cache lookup is external), so it's reasonable to also implement
in-library that sessions established on a renegotiation are not cached.

Add a bunch of tests that new_session_cb is called when expected.

BUG=501418

Change-Id: I42d44c82b043af72b60a0f8fdb57799e20f13ed5
Reviewed-on: https://boringssl-review.googlesource.com/5171
Reviewed-by: Adam Langley <agl@google.com>
2015-06-18 23:40:51 +00:00
David Benjamin
91eab5c9df Move all the bssl_shim handshake checks to their own function.
DoExchange is getting unwieldy.

Change-Id: I4eae6eb7471d1bb53b5305191dd9c52bb097a24f
Reviewed-on: https://boringssl-review.googlesource.com/5172
Reviewed-by: Adam Langley <agl@google.com>
2015-06-18 23:34:59 +00:00
Adam Langley
288d8d5ada runner: prepend the resource directory for async-signing tests.
b4d65fda70 was written concurrently with
my updating runner to handle -resource-dir (in
7c803a65d5) and thus it didn't include the
needed change for the test that it added to handle it.

This change fixes that added test so that it can run with -resource-dir.

Change-Id: I06b0adfb3fcf3f11c061fe1c8332a45cd7cd2dbc
2015-06-18 16:24:31 -07:00
David Benjamin
b4d65fda70 Implement asynchronous private key operations for client auth.
This adds a new API, SSL_set_private_key_method, which allows the consumer to
customize private key operations. For simplicity, it is incompatible with the
multiple slots feature (which will hopefully go away) but does not, for now,
break it.

The new method is only routed up for the client for now. The server will
require a decrypt hook as well for the plain RSA key exchange.

BUG=347404

Change-Id: I35d69095c29134c34c2af88c613ad557d6957614
Reviewed-on: https://boringssl-review.googlesource.com/5049
Reviewed-by: Adam Langley <agl@google.com>
2015-06-18 22:14:51 +00:00
David Benjamin
680ca961f9 Preserve session->sess_cert on ticket renewal.
Turns out the safer/simpler method still wasn't quite right. :-)
session->sess_cert isn't serialized and deserialized, which is poor. Duplicate
it manually for now. Leave a TODO to get rid of that field altogether as it's
not especially helpful. The certificate-related fields should be in the
session. The others probably have no reason to be preserved on resumptions at
all.

Test by making bssl_shim.cc assert the peer cert chain is there or not as
expected.

BUG=501220

Change-Id: I44034167629720d6e2b7b0b938d58bcab3ab0abe
Reviewed-on: https://boringssl-review.googlesource.com/5170
Reviewed-by: Adam Langley <agl@google.com>
2015-06-18 17:53:57 +00:00
David Benjamin
d65bb78c6c Add SSL_initial_handshake_complete.
To account for the changes in ticket renewal, Chromium will need to listen for
new_session_cb to determine whether the handshake produced a new session.
Chromium currently never caches sessions produced on a renegotiation. To retain
that behavior, it'll need to know whether new_session_cb is initial or not.
Rather than maintain duplicate state and listen for SSL_HANDSHAKE_DONE, it's
simpler to just let it query ssl->s3->initial_handshake_complete.

BUG=501418

Change-Id: Ib2f2541460bd09cf16106388e9cfdf3662e02681
Reviewed-on: https://boringssl-review.googlesource.com/5126
Reviewed-by: Adam Langley <agl@google.com>
2015-06-18 17:46:33 +00:00
David Benjamin
184494dfcc Raise SIGTRAP rather than abort on failure.
If gdb is attached, it's convenient to be able to continue running.

Change-Id: I3bbb2634d05a08f6bad5425f71da2210dbb80cfe
Reviewed-on: https://boringssl-review.googlesource.com/5125
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:25:30 +00:00
Adam Langley
7c803a65d5 Allow runner to run from anywhere.
This change adds flags to runner to allow it to be sufficiently
configured that it can run from any directory.

Change-Id: I82c08da4ffd26c5b11637480b0a79eaba0904d38
Reviewed-on: https://boringssl-review.googlesource.com/5130
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:24:36 +00:00
David Benjamin
11fc66a04c DTLS fragments may not be split across two records.
See also upstream's 9dcab127e14467733523ff7626da8906e67eedd6. The root problem
is dtls1_read_bytes is wrong, but we can get the right behavior now and add a
regression test for it before cleaning it up.

Change-Id: I4e5c39ab254a872d9f64242c9b77b020bdded6e6
Reviewed-on: https://boringssl-review.googlesource.com/5123
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:20:56 +00:00
David Benjamin
95d3182576 Duplicate SSL_SESSIONs when renewing them.
See also upstream's 27c76b9b8010b536687318739c6f631ce4194688, CVE-2015-1791.
Rather than write a dup function, serializing and deserializing the object is
simpler. It also fixes a bug in the original fix where it never calls
new_session_cb to store the new session (for clients which use that callback;
how clients should handle the session cache is much less clear).

The old session isn't pruned as we haven't processed the Finished message yet.
RFC 5077 says:

   The server MUST NOT assume that the client actually received the updated
   ticket until it successfully verifies the client's Finished message.

Moreover, because network messages are asynchronous, a new SSL connection may
have began just before the client received the new ticket, so any such servers
are broken regardless.

Change-Id: I13b3dc986dc58ea2ce66659dbb29e14cd02a641b
Reviewed-on: https://boringssl-review.googlesource.com/5122
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:15:12 +00:00
David Benjamin
fd67aa8c95 Add SSL_SESSION_from_bytes.
Mirrors SSL_SESSION_to_bytes. It avoids having to deal with object-reuse, the
non-size_t length parameter, and trailing data. Both it and the object-reuse
variant back onto an unexposed SSL_SESSION_parse which reads a CBS.

Note that this changes the object reuse story slightly. It's now merely an
optional output pointer that frees its old contents. No d2i_SSL_SESSION
consumer in Google that's built does reuse, much less reuse with the assumption
that the top-level object won't be overridden.

Change-Id: I5cb8522f96909bb222cab0f342423f2dd7814282
Reviewed-on: https://boringssl-review.googlesource.com/5121
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:12:39 +00:00
Adam Langley
85bc5601ee Add ECDHE-PSK-AES{128,256}-SHA cipher suites.
If we're going to have PSK and use standard cipher suites, this might be
the best that we can do for the moment.

Change-Id: I35d9831b2991dc5b23c9e24d98cdc0db95919d39
Reviewed-on: https://boringssl-review.googlesource.com/5052
Reviewed-by: Adam Langley <agl@google.com>
2015-06-09 18:10:42 +00:00
Adam Langley
1feb42a2fb Drop ECDHE-PSK-AES-128-GCM.
This is the best PSK cipher suite, but it's non-standard and nobody is
using it. Trivial to bring back in the future if we have need of it.

Change-Id: Ie78790f102027c67d1c9b19994bfb10a2095ba92
Reviewed-on: https://boringssl-review.googlesource.com/5051
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-06-09 18:08:52 +00:00
David Benjamin
d1c1c8e0a1 Remove EVP_PKEY_missing_parameters in ssl3_check_certificate_for_cipher.
This is a remnant of DSA support. It's not possible to parse out an incomplete
public key for the more reasonable X.509 key types.

Change-Id: I4f4c7b9d3795f5f0635f80a4cec9ca4c778e6c69
Reviewed-on: https://boringssl-review.googlesource.com/5050
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:43:36 +00:00
David Benjamin
436bf82ee8 Prune ssl3_check_cert_and_algorithm.
Most of the logic was redundant with checks already made in
ssl3_get_server_certificate. The DHE check was missing an ECDHE half
(and was impossible). The ECDSA check allowed an ECDSA certificate for
RSA. The only non-redundant check was a key usage check which,
strangely, is only done for ECDSA ciphers.

(Although this function called X509_certificate_type and checked sign
bits, those bits in X509_certificate_type are purely a function of the
key type and don't do anything.)

Change-Id: I8df7eccc0ffff49e4cfd778bd91058eb253b13cb
Reviewed-on: https://boringssl-review.googlesource.com/5047
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:27:12 +00:00
David Benjamin
8923c0bc53 Explicitly check for empty certificate list.
The NULL checks later on notice, but failing with
SSL_R_UNABLE_TO_FIND_PUBLIC_KEY_PARAMETERS on accident is confusing.
Require that the message be non-empty.

Change-Id: Iddfac6a3ae6e6dc66c3de41d3bb26e133c0c6e1d
Reviewed-on: https://boringssl-review.googlesource.com/5046
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:19:00 +00:00
David Benjamin
24f346d77b Limit the number of warning alerts silently consumed.
Per review comments on
https://boringssl-review.googlesource.com/#/c/4112/.

Change-Id: I82cacf67c6882e64f6637015ac41945522699797
Reviewed-on: https://boringssl-review.googlesource.com/5041
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:16:14 +00:00
David Benjamin
f4958e7678 Slightly simplify ServerKeyExchange handling.
The current logic requires each key exchange extract the key. It also
leaves handling X509_get_pubkey failure to the anonymous cipher suite
case which has an escape hatch where it goes back to check
ssl3_check_cert_and_algorithm.

Instead, get the key iff we know we have a signature to check.

Change-Id: If7154c7156aad3b89489defe4c1d951eeebf0089
Reviewed-on: https://boringssl-review.googlesource.com/5045
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:14:53 +00:00
David Benjamin
b31040d0d8 Get rid of CERT_PKEY slots in SESS_CERT.
This doesn't even change behavior. Unlike local configuration, the peer
can never have multiple certificates anyway. (Even with a renego, the
SESS_CERT is created anew.)

This does lose the implicit certificate type check, but the certificate
type is already checked in ssl3_get_server_certificate and later checked
post-facto in ssl3_check_cert_and_algorithm (except that one seems to
have some bugs like it accepts ECDSA certificates for RSA cipher suites,
to be cleaned up in a follow-up). Either way, we have the certificate
mismatch tests for this.

BUG=486295

Change-Id: I437bb723bb310ad54ee4150eda67c1cfe43377b3
Reviewed-on: https://boringssl-review.googlesource.com/5044
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:13:45 +00:00
David Benjamin
a8ebe2261f Add tests for empty record limit and make it work in the async case.
We shouldn't have protocol constraints that are sensitive to whether
data is returned synchronously or not.

Per https://boringssl-review.googlesource.com/#/c/4112/, the original
limitation was to avoid OpenSSL ABI changes. This is no longer a
concern.

Add tests for the sync and async case. Send the empty records in two
batches to ensure the count is reset correctly.

Change-Id: I3fee839438527e71adb83d437879bb0d49ca5c07
Reviewed-on: https://boringssl-review.googlesource.com/5040
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 21:45:21 +00:00
Adam Langley
af0e32cb84 Add SSL_get_tls_unique.
SSL_get_tls_unique returns the tls-unique channel-binding value as
defined in https://tools.ietf.org/html/rfc5929#section-3.1.

Change-Id: Id9644328a7db8a91cf3ff0deee9dd6ce0d3e00ba
Reviewed-on: https://boringssl-review.googlesource.com/4984
Reviewed-by: Adam Langley <agl@google.com>
2015-06-04 22:10:22 +00:00
Adam Langley
691992b0ea Minor typo fix in comment.
Change-Id: I55dc3d87a9571901abd2bbaf268871a482cf3bc5
Reviewed-on: https://boringssl-review.googlesource.com/4983
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-06-04 21:59:45 +00:00
Adam Langley
ba5934b77f Tighten up EMS resumption behaviour.
The client and server both have to decide on behaviour when resuming a
session where the EMS state of the session doesn't match the EMS state
as exchanged in the handshake.

                        Original handshake
      |  No                                         Yes
------+--------------------------------------------------------------
      |
R     |  Server: ok [1]                     Server: abort [3]
e  No |  Client: ok [2]                     Client: abort [4]
s     |
u     |
m     |
e     |
  Yes |  Server: don't resume                   No problem
      |  Client: abort; server
      |    shouldn't have resumed

[1] Servers want to accept legacy clients. The draft[5] says that
resumptions SHOULD be rejected so that Triple-Handshake can't be done,
but we'll rather enforce that EMS was used when using tls-unique etc.

[2] The draft[5] says that even the initial handshake should be aborted
if the server doesn't support EMS, but we need to be able to talk to the
world.

[3] This is a very weird case where a client has regressed without
flushing the session cache. Hopefully we can be strict and reject these.

[4] This can happen when a server-farm shares a session cache but
frontends are not all updated at once. If Chrome is strict here then
hopefully we can prevent any servers from existing that will try to
resume an EMS session that they don't understand. OpenSSL appears to be
ok here: https://www.ietf.org/mail-archive/web/tls/current/msg16570.html

[5] https://tools.ietf.org/html/draft-ietf-tls-session-hash-05#section-5.2

BUG=492200

Change-Id: Ie1225a3960d49117b05eefa5a36263d8e556e467
Reviewed-on: https://boringssl-review.googlesource.com/4981
Reviewed-by: Adam Langley <agl@google.com>
2015-06-03 22:05:50 +00:00
Adam Langley
b0eef0aee9 runner: minor tidyups.
Add expectResumeRejected to note cases where we expect a resumption
handshake to be rejected. (This was previously done by adding a flag,
which is a little less clear.)

Also, save the result of crypto/tls.Conn.ConnectionState() rather than
repeat that a lot.

Change-Id: I963945eda5ce1f3040b655e2441174b918b216b3
Reviewed-on: https://boringssl-review.googlesource.com/4980
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-06-03 22:03:07 +00:00
David Benjamin
a1c90a5ce1 Further tidy up cipher logic.
With SSL2 gone, there's no need for this split between the abstract
cipher framework and ciphers. Put the cipher suite table in ssl_cipher.c
and move other SSL_CIPHER logic there. With that gone, prune the
cipher-related hooks in SSL_PROTOCOL_METHOD.

BUG=468889

Change-Id: I48579de8bc4c0ea52781ba1b7b57bc5b4919d21c
Reviewed-on: https://boringssl-review.googlesource.com/4961
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:48:30 +00:00
David Benjamin
0fa4012331 Add a test that DTLS does not support RC4.
Make sure we don't break that on accident.

Change-Id: I22d58d35170d43375622fe61e4a588d1d626a054
Reviewed-on: https://boringssl-review.googlesource.com/4960
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:43:34 +00:00
David Benjamin
9a980abaee Fold TLS1_PRF_* into SSL_HANDSHAKE_MAC_*
They're redundant with each other.

Change-Id: I17e7ff8c4e0b1486986dd866fd99673fa2aaa494
Reviewed-on: https://boringssl-review.googlesource.com/4959
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:43:06 +00:00
David Benjamin
29864b5bb8 Remove SSL_CIPHER_ALGORITHM2_AEAD.
All ciphers are implemented by an EVP_AEAD.

Change-Id: Ifa754599a34e16bf97e1a4b84a271c6d45462c7c
Reviewed-on: https://boringssl-review.googlesource.com/4958
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:30:00 +00:00
David Benjamin
904dc72b32 Fold away SSL_PROTOCOL_METHOD hooks shared between TLS and DTLS.
The ctrl hooks are left alone since they should just go away.
Simplifying the cipher story will happen in the next CL.

BUG=468889

Change-Id: I979971c90f59c55cd5d17554f1253158b114f18b
Reviewed-on: https://boringssl-review.googlesource.com/4957
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:29:46 +00:00
David Benjamin
a6022771b3 Split ssl_read_bytes hook into app_data and close_notify hooks.
This still needs significant work, especially the close_notify half, but
clarify the interface and get *_read_bytes out of SSL_PROTOCOL_METHOD.
read_bytes is an implementation detail of those two and get_message
rather than both an implementation detail of get_message for handshake
and a (wholly inappropriate) exposed interface for the other two.

BUG=468889

Change-Id: I7dd23869e0b7c3532ceb2e9dd31ca25ea31128e7
Reviewed-on: https://boringssl-review.googlesource.com/4956
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:27:55 +00:00
David Benjamin
c933a47e6f Switch the ssl_write_bytes hook to ssl_write_app_data.
The SSL_PROTOCOL_METHOD table needs work, but this makes it clearer
exactly what the shared interface between the upper later and TLS/DTLS
is.

BUG=468889

Change-Id: I38931c484aa4ab3f77964d708d38bfd349fac293
Reviewed-on: https://boringssl-review.googlesource.com/4955
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:18:06 +00:00
David Benjamin
338e06771c Reject sessions with the wrong structure version.
The old upstream logic actually didn't do this, but 1.1.0's new code does.
Given that the version has never changed and even unknown fields were rejected
by the old code, this seems a safe and prudent thing to do.

Change-Id: I09071585e5183993b358c10ad36fc206f8bceeda
Reviewed-on: https://boringssl-review.googlesource.com/4942
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 20:30:02 +00:00
David Benjamin
f297e021f1 Reject unknown fields in d2i_SSL_SESSION.
The original OpenSSL implementation did the same. M_ASN1_D2I_Finish checks
this. Forwards compatibility with future sessions with unknown fields is
probably not desirable.

Change-Id: I116a8c482cbcc47c3fcc31515c4a3718f66cf268
Reviewed-on: https://boringssl-review.googlesource.com/4941
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 20:29:07 +00:00
David Benjamin
bd15a8e748 Fix DTLS handling of multiple records in a packet.
9a41d1b946 broke handling of multiple records in
a single packet. If |extend| is true, not all of the previous packet should be
consumed, only up to the record length.

Add a test which stresses the DTLS stack's handling of multiple handshake
fragments in a handshake record and multiple handshake records in a packet.

Change-Id: I96571098ad9001e96440501c4730325227b155b8
Reviewed-on: https://boringssl-review.googlesource.com/4950
Reviewed-by: Adam Langley <agl@google.com>
2015-05-29 22:59:38 +00:00
David Benjamin
e76ccae68b Release handshake buffer when sending no certificate.
See also upstream's dab18ab596acb35eff2545643e25757e4f9cd777. This allows us to
add an assertion to the finished computation that the handshake buffer has
already been released.

BUG=492371

Change-Id: I8f15c618c8b2c70bfe583c81644d9dbea95519d4
Reviewed-on: https://boringssl-review.googlesource.com/4887
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 21:54:30 +00:00
David Benjamin
5f04b6521d Release the handshake buffer on the client for abbreviated handshakes.
Another missing case.

BUG=492371

Change-Id: Iaabe43517b8581969431a20f7ba7094787b954aa
Reviewed-on: https://boringssl-review.googlesource.com/4886
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 21:53:52 +00:00
David Benjamin
5c1ce2925d Decide whether or not to request client certificates early.
This allows us to merge two of the ssl3_digest_cached_records calls which were
almost, but not completely, redundant. Also catches a missing case: the buffer
may be discarded if doing session resumption but otherwise enabling client
authentication.

BUG=492371

Change-Id: I78e9a4a9cca665e89899ef97b815454c6f5c7e02
Reviewed-on: https://boringssl-review.googlesource.com/4885
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 21:53:16 +00:00
David Benjamin
4b30b28def Remove server-side renego session resumption check.
Servers can no longer renegotiate.

Change-Id: Id79d5753562e29d2872871f4f571552a019215fa
Reviewed-on: https://boringssl-review.googlesource.com/4884
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 21:51:19 +00:00
David Benjamin
5aea93e604 Deprecate and no-op SSL_VERIFY_CLIENT_ONCE.
This is documented as "Only request a client certificate on the initial TLS/SSL
handshake. Do not ask for a client certificate again in case of a
renegotiation." Server-side renegotiation is gone.

I'm not sure this flag has ever worked anyway, dating all the way back to
SSLeay 0.8.1b. ssl_get_new_session overwrites s->session, so the old
session->peer is lost.

Change-Id: Ie173243e189c63272c368a55167b8596494fd59c
Reviewed-on: https://boringssl-review.googlesource.com/4883
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 21:50:24 +00:00
David Benjamin
74d8bc2503 Don't make SSL_MODE_*HELLO_TIME configurable.
Never send the time as a client. Always send it as a server.

Change-Id: I20c55078cfe199d53dc002f6ee5dd57060b086d5
Reviewed-on: https://boringssl-review.googlesource.com/4829
Reviewed-by: Adam Langley <agl@google.com>
2015-05-27 21:47:59 +00:00
David Benjamin
ece089c3a3 Deprecate and no-op SSL_set_state.
Yes, OpenSSL lets you randomly change its internal state. This is used
as part of server-side renegotiation. Server-side renegotiation is gone.

BUG=429450

Change-Id: Ic1b013705734357acf64e8bf89a051b2b7521c64
Reviewed-on: https://boringssl-review.googlesource.com/4828
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 20:52:05 +00:00
David Benjamin
be05c63bf8 Remove compatibility s->version checks.
They were added to avoid accidentally enabling renego for a consumer which set
them to zero to break the handshake on renego. Now that renego is off by
default, we can get rid of them again.

Change-Id: I2cc3bf567c55c6562352446a36f2b5af37f519ba
Reviewed-on: https://boringssl-review.googlesource.com/4827
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 20:51:39 +00:00
David Benjamin
8ec88108d4 Remove SSL_in_before and SSL_ST_BEFORE.
It's never called and the state is meaningless now.

Change-Id: I5429ec3eb7dc2b789c0584ea88323f0ff18920ae
Reviewed-on: https://boringssl-review.googlesource.com/4826
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 20:51:06 +00:00
David Benjamin
cd90f3a241 Remove renegotiation deferral logic.
When the peer or caller requests a renegotiation, OpenSSL doesn't
renegotiate immediately. It sets a flag to begin a renegotiation as soon
as record-layer read and write buffers are clear. One reason is that
OpenSSL's record layer cannot write a handshake record while an
application data record is being written. The buffer consistency checks
around partial writes will break.

None of these cases are relevant for the client auth hack. We already
require that renego come in at a quiescent part of the application
protocol by forbidding handshake/app_data interleave.

The new behavior is now: when a HelloRequest comes in, if the record
layer is not idle, the renegotiation is rejected as if
SSL_set_reject_peer_renegotiations were set. Otherwise we immediately
begin the new handshake. The server may not send any application data
between HelloRequest and completing the handshake. The HelloRequest may
not be consumed if an SSL_write is pending.

Note this does require that Chromium's HTTP stack not attempt to read
the HTTP response until the request has been written, but the
renegotiation logic already assumes it. Were Chromium to drive the
SSL_read state machine early and the server, say, sent a HelloRequest
after reading the request headers but before we've sent the whole POST
body, the SSL state machine may racily enter renegotiate early, block
writing the POST body on the new handshake, which would break Chromium's
ERR_SSL_CLIENT_AUTH_CERT_NEEDED plumbing.

BUG=429450

Change-Id: I6278240c3bceb5d2e1a2195bdb62dd9e0f4df718
Reviewed-on: https://boringssl-review.googlesource.com/4825
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 20:50:43 +00:00
David Benjamin
44d3eed2bb Forbid caller-initiated renegotiations and all renego as a servers.
The only case where renego is supported is if we are a client and the
server sends a HelloRequest. That is still needed to support the renego
+ client auth hack in Chrome. Beyond that, no other forms of renego will
work.

The messy logic where the handshake loop is repurposed to send
HelloRequest and the extremely confusing tri-state s->renegotiate (which
makes SSL_renegotiate_pending a lie during the initial handshake as a
server) are now gone. The next change will further simplify things by
removing ssl->s3->renegotiate and the renego deferral logic. There's
also some server-only renegotiation checks that can go now.

Also clean up ssl3_read_bytes' HelloRequest handling. The old logic relied on
the handshake state machine to reject bad HelloRequests which... actually that
code probably lets you initiate renego by sending the first four bytes of a
ServerHello and expecting the peer to read it later.

BUG=429450

Change-Id: Ie0f87d0c2b94e13811fe8e22e810ab2ffc8efa6c
Reviewed-on: https://boringssl-review.googlesource.com/4824
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 20:43:56 +00:00
David Benjamin
5f387e38fc Remove s->renegotiate check in SSL_clear.
This dates back to upstream's a2a0158959e597188c10fbfeaf61888b2df2e587.
It seems to be a remnant of those SSL_clear calls in the handshake state
machine which... were also bizarre and since gone.

Since SSL_clear is to drop the current connection but retain the
configuration, it doesn't really make sense to forbid it while you're
mid-handshake.

This removes another consumer of s->renegotiate.

BUG=429450

Change-Id: Ifac6bf11644447fd5571262bed7421684739bc39
Reviewed-on: https://boringssl-review.googlesource.com/4823
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:32:26 +00:00
David Benjamin
20f6e97c7e Switch three more renegotiate checks to initial_handshake_complete.
ssl_cipher_list_to_bytes is client-only, so s->renegotiate worked, but
the only reason the other two worked is because s->renegotiate isn't a
lie on the server before ServerHello.

BUG=429450

Change-Id: If68a986c6ec4a0f16e57a6187238e05b50ecedfc
Reviewed-on: https://boringssl-review.googlesource.com/4822
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:31:55 +00:00
David Benjamin
d23d5a5a8b Remove remnants of DTLS renegotiate.
BUG=429450

Change-Id: I94846d1fd377bc07044f916d0bb1880e219416df
Reviewed-on: https://boringssl-review.googlesource.com/4821
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:31:07 +00:00
David Benjamin
9a41d1b946 Deprecate SSL_*_read_ahead and enforce DTLS packet boundaries.
Now that WebRTC honors packet boundaries (https://crbug.com/447431), we
can start enforcing them correctly. Configuring read-ahead now does
nothing. Instead DTLS will always set "read-ahead" and also correctly
enforce packet boundaries when reading records. Add tests to ensure that
badly fragmented packets are ignored. Because such packets don't fail
the handshake, the tests work by injecting an alert in the front of the
handshake stream and ensuring the DTLS implementation ignores them.

ssl3_read_n can be be considerably unraveled now, but leave that for
future cleanup. For now, make it correct.

BUG=468889

Change-Id: I800cfabe06615af31c2ccece436ca52aed9fe899
Reviewed-on: https://boringssl-review.googlesource.com/4820
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:29:34 +00:00
David Benjamin
76e48c51d0 Fix Windows mode.
MSVC hates unsigned unary minus.

Change-Id: I777f792f19868bfc4572c383a723b10ea091c0ca
Reviewed-on: https://boringssl-review.googlesource.com/4840
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:25:32 +00:00
David Benjamin
3fa65f0f05 Fix some malloc test crashs.
This isn't exhaustive. There are still failures in some tests which probably
ought to get C++'d first.

Change-Id: Iac58df9d98cdfd94603d54374a531b2559df64c3
Reviewed-on: https://boringssl-review.googlesource.com/4795
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:00:10 +00:00
David Benjamin
0b635c52b2 Add malloc test support to unit tests.
Currently far from passing and I haven't even tried with a leak checker yet.
Also bn_test is slow.

Change-Id: I4fe2783aa5f7897839ca846062ae7e4a367d2469
Reviewed-on: https://boringssl-review.googlesource.com/4794
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 17:59:48 +00:00
David Benjamin
3e3090dc50 Pass a dtls1_use_epoch enum down to dtls1_seal_record.
This is considerably less scary than swapping out connection state. It also
fixes a minor bug where, if dtls1_do_write had an alert to dispatch and we
happened to retry during a rexmit, it would use the wrong epoch.

BUG=468889

Change-Id: I754b0d46bfd02f797f4c3f7cfde28d3e5f30c52b
Reviewed-on: https://boringssl-review.googlesource.com/4793
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 17:59:36 +00:00
David Benjamin
31a07798a5 Factor SSL_AEAD_CTX into a dedicated type.
tls1_enc is now SSL_AEAD_CTX_{open,seal}. This starts tidying up a bit
of the record-layer logic. This removes rr->input, as encrypting and
decrypting records no longer refers to various globals. It also removes
wrec altogether. SSL3_RECORD is now only used to maintain state about
the current incoming record. Outgoing records go straight to the write
buffer.

This also removes the outgoing alignment memcpy and simply calls
SSL_AEAD_CTX_seal with the parameters as appropriate. From bssl speed
tests, this seems to be faster on non-ARM and a bit of a wash on ARM.

Later it may be worth recasting these open/seal functions to write into
a CBB (tweaked so it can be malloc-averse), but for now they take an
out/out_len/max_out trio like their EVP_AEAD counterparts.

BUG=468889

Change-Id: Ie9266a818cc053f695d35ef611fd74c5d4def6c3
Reviewed-on: https://boringssl-review.googlesource.com/4792
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 17:59:15 +00:00
David Benjamin
7ef9fff53d Remove ssl_ok.
This is never used.

Change-Id: I560f04c0a6f140298ca42b8a0913ce954a2fdf7d
Reviewed-on: https://boringssl-review.googlesource.com/4789
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 21:41:38 +00:00
David Benjamin
afc9ecddb6 Unexport ssl_get_new_session and ssl_update_cache.
Chromium's session cache has since been rewritten and no longer needs to
muck with those functions in tests.

Change-Id: I2defad81513210dca5e105757e04cbb677583251
Reviewed-on: https://boringssl-review.googlesource.com/4788
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 21:41:13 +00:00
David Benjamin
4831c3328c Document some core SSL_CTX and SSL methods.
Current thought is to organize this by:

- Core SSL_CTX APIs (creating, destroying)
- Core SSL APIs (creating destroying, maybe handshake, read, write as
  well)
- APIs to configure SSL_CTX/SSL, roughly grouped by feature. Probably
  options and modes are the first two sections. SSL_TXT_* constants can
  be part of documenting cipher suite configuration.
- APIs to query state from SSL_CTX/SSL, roughly grouped by feature. (Or
  perhaps these should be folded into the configuration sections?)

The functions themselves aren't reordered or reorganized to match the
eventual header order yet. Though I did do the s -> ssl rename on the
ones I've touched.

Also formally deprecate SSL_clear. It would be a core SSL API
except it's horrible.

Change-Id: Ia7e4fdcb7bad4e9ccdee8cf8c3136dc63aaaa772
Reviewed-on: https://boringssl-review.googlesource.com/4784
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 21:32:42 +00:00
David Benjamin
760b1ddcdb Tidy up state machine coverage tests.
Rather than duplicate all the various modifiers, which is quite
error-prone, write all the tests to a temporary array and then apply
modifiers afterwards.

Change-Id: I19bfeb83b722ed34e973f17906c5e071471a926a
Reviewed-on: https://boringssl-review.googlesource.com/4782
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 21:12:58 +00:00
David Benjamin
3629c7b016 Add client peer-initiated renego to the state machine tests.
We should be testing asynchronous renego.

BUG=429450

Change-Id: Ib7a5d42f2ac728f9ea0d80158eef63ad77cd77a4
Reviewed-on: https://boringssl-review.googlesource.com/4781
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 21:11:55 +00:00
David Benjamin
cff0b90cbb Add client-side tests for renegotiation_info enforcement.
Since we hope to eventually lose server-side renegotiation support
altogether, get the client-side version of those tests. We should have
had those anyway to test that the default is to allow it.

BUG=429450

Change-Id: I4a18f339b55f3f07d77e22e823141e10a12bc9ff
Reviewed-on: https://boringssl-review.googlesource.com/4780
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 21:10:14 +00:00
Adam Langley
4bdb6e43fa Remove remaining calls to the old lock functions.
|SSL_CTX| and |X509_STORE| have grown their own locks. Several static
locks have been added to hack around not being able to use a
|CRYPTO_once_t| in public headers. Lastly, support for calling
|SSL_CTX_set_generate_session_id| concurrently with active connections
has been removed. No other property of an |SSL_CTX| works like that.

Change-Id: Iff5fe3ee3fdd6ea9c9daee96f850b107ad8a6bca
Reviewed-on: https://boringssl-review.googlesource.com/4775
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:18:13 +00:00
Adam Langley
0b5e3908cf Convert reference counts in ssl/
Convert reference counts in ssl/ to use |CRYPTO_refcount_t|.

Change-Id: I5d60f641b0c89b1ddfe38bfbd9d7285c60377f4c
Reviewed-on: https://boringssl-review.googlesource.com/4773
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:15:47 +00:00
Adam Langley
0da323a8b8 Convert reference counts in crypto/
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.

Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:15:26 +00:00
Adam Langley
a7997f12be Set minimum DH group size to 1024 bits.
DH groups less than 1024 bits are clearly not very safe. Ideally servers
would switch to ECDHE because 1024 isn't great either, but this will
serve for the short term.

BUG=490240

Change-Id: Ic9aac714cdcdcbfae319b5eb1410675d3b903a69
Reviewed-on: https://boringssl-review.googlesource.com/4813
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 18:35:31 +00:00
David Benjamin
81091d55e1 Don't use uninitialized memory in RAND_bytes.
We can't actually catch this with MSan because it requires all code be
instrumented, so it needs a NO_ASM build which no disables that code. valgrind
doesn't notice either, possibly because there's some computation being done on
it. Still, we shouldn't use uninitialized memory.

Also get us closer to being instrumentable by MSan, but the runner tests will
need to build against an instrumented STL and I haven't tried that yet.

Change-Id: I2d65697a3269b5b022899f361730a85c51ecaa12
Reviewed-on: https://boringssl-review.googlesource.com/4760
Reviewed-by: Adam Langley <agl@google.com>
2015-05-15 20:31:27 +00:00
David Benjamin
a07c0fc8f2 Fix SSL_get_current_cipher.
SSL_get_current_cipher is documented by upstream to return the cipher actually
being used. However, because it reads s->session, it returns information
pertaining to the session to be offered if queried before ServerHello or early
in an abbreviated handshake.

Logic around s->session needs more comprehensive cleanup but for just this
function, defining it to be the current outgoing cipher is close to the current
semantics but for fixing the initial state (s->session->cipher is populated
when sending CCS). Store it in the SSL_AEAD_CTX which seems a natural place to
associate state pertaining to a connection half.

BUG=484744

Change-Id: Ife8db27a16615d0dbb2aec65359537243e08af7c
Reviewed-on: https://boringssl-review.googlesource.com/4733
Reviewed-by: Adam Langley <agl@google.com>
2015-05-14 23:02:16 +00:00
David Benjamin
4b27d9f8bd Never resume sessions on renegotiations.
This cuts down on one config knob as well as one case in the renego
combinatorial explosion. Since the only case we care about with renego
is the client auth hack, there's no reason to ever do resumption.
Especially since, no matter what's in the session cache:

- OpenSSL will only ever offer the session it just established,
  whether or not a newer one with client auth was since established.

- Chrome will never cache sessions created on a renegotiation, so
  such a session would never make it to the session cache.

- The new_session + SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
  logic had a bug where it would unconditionally never offer tickets
  (but would advertise support) on renego, so any server doing renego
  resumption against an OpenSSL-derived client must not support
  session tickets.

This also gets rid of s->new_session which is now pointless.

BUG=429450

Change-Id: I884bdcdc80bff45935b2c429b4bbc9c16b2288f8
Reviewed-on: https://boringssl-review.googlesource.com/4732
Reviewed-by: Adam Langley <agl@google.com>
2015-05-14 22:53:21 +00:00
David Benjamin
e6df054a75 Add s->s3->initial_handshake_complete.
There's multiple different versions of this check, between
s->s3->have_version (only works at some points), s->new_session (really
weird and not actually right), s->renegotiate (fails on the server
because it's always 2 after ClientHello), and s->s3->tmp.finish_md_len
(super confusing). Add an explicit bit with clear meaning. We'll prune
some of the others later; notably s->renegotiate can go away when
initiating renegotiation is removed.

This also tidies up the extensions to be consistent about whether
they're allowed during renego:

- ALPN failed to condition when accepting from the server, so even
  if the client didn't advertise, the server could.

- SCTs now *are* allowed during renego. I think forbidding it was a
  stray copy-paste. It wasn't consistently enforced in both ClientHello
  and ServerHello, so the server could still supply it. Moreover, SCTs
  are part of the certificate, so we should accept it wherever we accept
  certificates, otherwise that session's state becomes incomplete. This
  matches OCSP stapling. (NB: Chrome will never insert a session created
  on renego into the session cache and won't accept a certificate
  change, so this is moot anyway.)

Change-Id: Ic9bd1ebe2a2dbe75930ed0213bf3c8ed8170e251
Reviewed-on: https://boringssl-review.googlesource.com/4730
Reviewed-by: Adam Langley <agl@google.com>
2015-05-13 17:11:31 +00:00
David Benjamin
897e5e0013 Default renegotiations to off.
As of crbug.com/484543, Chromium's SSLClientSocket is not sensitive to whether
renegotiation is enabled or not. Disable it by default and require consumers to
opt into enabling this protocol mistake.

BUG=429450

Change-Id: I2329068284dbb851da010ff1fd398df3d663bcc3
Reviewed-on: https://boringssl-review.googlesource.com/4723
Reviewed-by: Adam Langley <agl@google.com>
2015-05-13 17:02:14 +00:00
David Benjamin
6a08da2cf8 Remove redundant setup buffer calls.
Nothing should call ssl3_setup_read_buffer or ssl3_setup_write_buffer unless it
intends to write into the buffer. This way buffer management can later be an
implementation detail of the record layer.

Change-Id: Idb0effba00e77c6169764843793f40ec37868b61
Reviewed-on: https://boringssl-review.googlesource.com/4687
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 21:31:59 +00:00
David Benjamin
b1f5bca538 Remove max parameter to ssl3_read_n.
It's completely redundant with the extend bit. If extend is 0, we're reading a
new record, and rbuf.len is passed. Then it needs to get clamped by ssl3_read_n
post alignment anyway. If extend is 1, we're reading the rest of the current
record and max is always n. (For TLS, we actually could just read more, but not
for DTLS. Basically no one sets it on the TLS side of things, so instead, after
WebRTC's broken DTLS handling is fixed, read_ahead can go away altogether and
DTLS/TLS record layers can be separated.)

This removes ssl3_read_n's callers' dependency on ssl3_setup_read_buffer
setting up rbuf.len.

Change-Id: Iaf11535d01017507a52a33b19240f42984d6cf52
Reviewed-on: https://boringssl-review.googlesource.com/4686
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 18:41:41 +00:00
David Benjamin
9417b7649f Remove DTLS special-cases in buffer releasing.
They date to https://rt.openssl.org/Ticket/Display.html?id=2533, but no
particularly good justification was given for them. It seems it was just a
bandaid because d1_pkt.c forgot to initialize the buffer. I went through
codesearch for all accesses to SSL3_BUFFER::buf and SSL::packet. They seem
appropriately guarded but for this one.

Change-Id: Ife4e7afdb7a7c137d6be4791542eb5de6dd5b1b6
Reviewed-on: https://boringssl-review.googlesource.com/4685
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 18:40:04 +00:00
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