Commit Graph

209 Commits

Author SHA1 Message Date
David Benjamin
59937045ec Document certificate verification functions in SSL.
Or at least group them together and make a passing attempt to document
them. The legacy X.509 stack itself remains largely untouched and most
of the parameters have to do with it.

Change-Id: I9e11e2ad1bbeef53478c787344398c0d8d1b3876
Reviewed-on: https://boringssl-review.googlesource.com/5942
Reviewed-by: Adam Langley <agl@google.com>
2015-09-23 23:31:18 +00:00
David Benjamin
42fea37227 Document Channel ID functions and a few random extras.
Change-Id: I34a6352d265615ee08bb18fd537acf70a558f014
Reviewed-on: https://boringssl-review.googlesource.com/5940
Reviewed-by: Adam Langley <agl@google.com>
2015-09-23 23:08:59 +00:00
David Benjamin
977547b6f9 Group NPN functions in their own section.
Existing documentation was moved to the header, very slightly tweaked.

Change-Id: Ife3c2351e2d7e6a335854284f996918039414446
Reviewed-on: https://boringssl-review.googlesource.com/5897
Reviewed-by: Adam Langley <agl@google.com>
2015-09-18 18:39:53 +00:00
David Benjamin
8984f1f6e5 Group ALPN functions into their own section.
These were already documented, though some of the documentation was
expanded on slightly.

Change-Id: I04c6276a83a64a03ab9cce9b9c94d4dea9ddf638
Reviewed-on: https://boringssl-review.googlesource.com/5896
Reviewed-by: Adam Langley <agl@google.com>
2015-09-17 22:19:58 +00:00
Matt Braithwaite
cd6f54b193 Add method to query Extended Master Secret support
Change-Id: I4c285f4b3dd77f8fc7249c7504b5e4eb9b62959f
Reviewed-on: https://boringssl-review.googlesource.com/5920
Reviewed-by: Adam Langley <agl@google.com>
2015-09-17 21:18:28 +00:00
David Benjamin
e8814df699 Document PSK functions in ssl.h.
Change-Id: Iac58d28570d429236de1d2bc40b631e252b8e710
Reviewed-on: https://boringssl-review.googlesource.com/5893
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 23:34:12 +00:00
David Benjamin
9e4e01ee14 Align the SSL stack on #include style.
ssl.h should be first. Also two lines after includes and the rest of the
file.

Change-Id: Icb7586e00a3e64170082c96cf3f8bfbb2b7e1611
Reviewed-on: https://boringssl-review.googlesource.com/5892
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 23:32:07 +00:00
David Benjamin
1ac08feed6 Fix SSL and SSL_CTX ex_data variable names.
Forgot to fix these when I fixed the headers.

Change-Id: Ie45e624abc993e16e2d5a872ef00dba9029a38df
Reviewed-on: https://boringssl-review.googlesource.com/5891
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 23:22:07 +00:00
David Benjamin
dafbdd49c7 Document session cache functions.
Also switch to the new variable names (SSL_CTX *ctx, SSL *ssl,
SSL_SESSION *session) for all documented functions.

Change-Id: I15e15a703b96af1727601108223c7ce3b0691f1d
Reviewed-on: https://boringssl-review.googlesource.com/5882
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:58:48 +00:00
David Benjamin
a6b8cdfbe9 Document functions acting on an SSL_SESSION.
Change-Id: I0b4386d1972af0ac10db397716de8161810a87f4
Reviewed-on: https://boringssl-review.googlesource.com/5877
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:37:42 +00:00
David Benjamin
ee0c82789a Move SSL_get_peer_* to Connection information.
This is arguably more commonly queried connection information than the
tls-unique.

Change-Id: I1f080536153ba9f178af8e92cb43b03df37110b5
Reviewed-on: https://boringssl-review.googlesource.com/5874
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:30:54 +00:00
David Benjamin
27bbae4682 Reorder functions in ssl_lib.c to match ssl.h.
Just the stuff that has been pulled out into sections already.

Change-Id: I3da6bc61d79ccfe2b18d888075dc32026a656464
Reviewed-on: https://boringssl-review.googlesource.com/5873
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:30:23 +00:00
David Benjamin
3c1ccc017c Document a bunch of core functions in ssl.h.
Unfortunately, these are also some of the worst APIs in the SSL stack.
I've tried to capture all the things they expose to the caller. 0 vs -1
is intentionally left unexpanded on for now. Upstream's documentation
says 0 means transport EOF, which is a nice idea but isn't true. (A lot
of random functions return 0 on error and pass it up to the caller.)
https://crbug.com/466303 tracks fixing that.

SSL_set_bio is intentionally documented to NOT be usable when they're
already configured. The function tries to behave in this case and even
with additional cases when |rbio| and/or |wbio| are unchanged, but this
is buggy. For instance, this will explode:

     SSL_set_bio(ssl, bio1, bio1);
     SSL_set_bio(ssl, bio2, SSL_get_wbio(ssl));

As will this, though it's less clear this is part of the API contract
due to SSL taking ownership.

     SSL_set_bio(ssl, bio1, bio2);
     SSL_set_bio(ssl, bio2, bio1);

It also tries to handle ssl->bbio already existing, but I doubt it quite
works. Hopefully we can drop ssl->bbio eventually. (Why is this so
complicated...)

Change-Id: I5f9f3043915bffc67e2ebd282813e04afbe076e6
Reviewed-on: https://boringssl-review.googlesource.com/5872
Reviewed-by: Adam Langley <agl@google.com>
2015-09-14 23:29:18 +00:00
Paul Lietar
4fac72e638 Add server-side support for Signed Certificate Timestamps.
Change-Id: Ifa44fef160fc9d67771eed165f8fc277f28a0222
Reviewed-on: https://boringssl-review.googlesource.com/5840
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-09-11 21:52:26 +00:00
David Benjamin
7c6d09b9e5 Fold ssl_algs.c into ssl_lib.c.
There's not enough in that file to really justify its own file now.

Change-Id: I6130cfce6c40fe9d46aa83dd83e6f38d87fdcf64
Reviewed-on: https://boringssl-review.googlesource.com/5823
Reviewed-by: Adam Langley <agl@google.com>
2015-09-09 01:05:25 +00:00
Adam Langley
8748920ca7 Don't forget to free the TLS 1.0 ciphers.
Change-Id: I38517e0b13c9fc75b3964bc980d16daee9aed5b5
2015-09-03 15:52:29 -07:00
Adam Langley
cef7583633 Add cipher suite settings for TLS ≥ 1.0.
This change adds the ability to configure ciphers specifically for
TLS ≥ 1.0. This compliments the existing ability to specify ciphers
for TLS ≥ 1.1.

This is useful because TLS 1.0 is the first version not to suffer from
POODLE. (Assuming that it's implemented correctly[1].) Thus one might
wish to reserve RC4 solely for SSLv3.

[1] https://www.imperialviolet.org/2014/12/08/poodleagain.html

Change-Id: I774d5336fead48f03d8a0a3cf80c369692ee60df
Reviewed-on: https://boringssl-review.googlesource.com/5793
Reviewed-by: Adam Langley <agl@google.com>
2015-09-03 22:44:36 +00:00
David Benjamin
c9d40ba5b0 Remove unnecessary field initializations.
SSL_CTX gets memset to zero, so many of the values needn't be explicitly
initialized.

Change-Id: I0e707a0dcc052cd6f0a5dc8d037400170bd75594
Reviewed-on: https://boringssl-review.googlesource.com/5812
Reviewed-by: Adam Langley <agl@google.com>
2015-09-03 18:39:54 +00:00
David Benjamin
76c2efc0e9 Forbid a server from negotiating both ALPN and NPN.
If the two extensions select different next protocols (quite possible since one
is server-selected and the other is client-selected), things will break. This
matches the behavior of NSS (Firefox) and Go.

Change-Id: Ie1da97bf062b91a370c85c12bc61423220a22f36
Reviewed-on: https://boringssl-review.googlesource.com/5780
Reviewed-by: Adam Langley <agl@google.com>
2015-09-01 20:46:42 +00:00
David Benjamin
26416e9dde Remove the last of SESS_CERT.
Move cert_chain to the SSL_SESSION. Now everything on an SSL_SESSION is
properly serialized. The cert_chain field is, unfortunately, messed up
since it means different things between client and server.

There exists code which calls SSL_get_peer_cert_chain as both client and
server and assumes the existing semantics for each. Since that function
doesn't return a newly-allocated STACK_OF(X509), normalizing between the
two formats is a nuisance (we'd either need to store both cert_chain and
cert_chain_full on the SSL_SESSION or create one of the two variants
on-demand and stash it into the SSL).

This CL does not resolve this and retains the client/server difference
in SSL_SESSION. The SSL_SESSION serialization is a little inefficient
(two copies of the leaf certificate) for a client, but clients don't
typically serialize sessions. Should we wish to resolve it in the
future, we can use a different tag number. Because this was historically
unserialized, existing code must already allow for cert_chain not being
preserved across i2d/d2i.

In keeping with the semantics of retain_only_sha256_of_client_certs,
cert_chain is not retained when that flag is set.

Change-Id: Ieb72fc62c3076dd59750219e550902f1ad039651
Reviewed-on: https://boringssl-review.googlesource.com/5759
Reviewed-by: Adam Langley <agl@google.com>
2015-08-28 22:45:59 +00:00
David Benjamin
b8d28cf532 Factor out the buffering and low-level record code.
This begins decoupling the transport from the SSL state machine. The buffering
logic is hidden behind an opaque API. Fields like ssl->packet and
ssl->packet_length are gone.

ssl3_get_record and dtls1_get_record now call low-level tls_open_record and
dtls_open_record functions that unpack a single record independent of who owns
the buffer. Both may be called in-place. This removes ssl->rstate which was
redundant with the buffer length.

Future work will push the buffer up the stack until it is above the handshake.
Then we can expose SSL_open and SSL_seal APIs which act like *_open_record but
return a slightly larger enum due to other events being possible. Likewise the
handshake state machine will be detached from its buffer. The existing
SSL_read, SSL_write, etc., APIs will be implemented on top of SSL_open, etc.,
combined with ssl_read_buffer_* and ssl_write_buffer_*. (Which is why
ssl_read_buffer_extend still tries to abstract between TLS's and DTLS's fairly
different needs.)

The new buffering logic does not support read-ahead (removed previously) since
it lacks a memmove on ssl_read_buffer_discard for TLS, but this could be added
if desired. The old buffering logic wasn't quite right anyway; it tried to
avoid the memmove in some cases and could get stuck too far into the buffer and
not accept records. (The only time the memmove is optional is in DTLS or if
enough of the record header is available to know that the entire next record
would fit in the buffer.)

The new logic also now actually decrypts the ciphertext in-place again, rather
than almost in-place when there's an explicit nonce/IV. (That accidentally
switched in https://boringssl-review.googlesource.com/#/c/4792/; see
3d59e04bce96474099ba76786a2337e99ae14505.)

BUG=468889

Change-Id: I403c1626253c46897f47c7ae93aeab1064b767b2
Reviewed-on: https://boringssl-review.googlesource.com/5715
Reviewed-by: Adam Langley <agl@google.com>
2015-08-28 22:01:02 +00:00
Matt Braithwaite
4838d8aa90 Add do-nothing function |ERR_load_SSL_strings|.
This benefits pyOpenSSL.

Change-Id: Ie7a35d6f9e9a1f0c7fe42ba105c68c327f7d0b0e
Reviewed-on: https://boringssl-review.googlesource.com/5740
Reviewed-by: Adam Langley <agl@google.com>
2015-08-20 21:33:50 +00:00
Paul Lietar
aeeff2ceee Server-side OCSP stapling support.
This is a simpler implementation than OpenSSL's, lacking responder IDs
and request extensions support. This mirrors the client implementation
already present.

Change-Id: I54592b60e0a708bfb003d491c9250401403c9e69
Reviewed-on: https://boringssl-review.googlesource.com/5700
Reviewed-by: Adam Langley <agl@google.com>
2015-08-20 17:55:31 +00:00
David Benjamin
651b3d9284 ssl->cert and ctx->cert cannot be NULL.
They get initialized in SSL_new and SSL_CTX_new, respectively.

Change-Id: Ib484108987a99f654d1a77fc473103f5cb393bd7
Reviewed-on: https://boringssl-review.googlesource.com/5676
Reviewed-by: Adam Langley <agl@google.com>
2015-08-18 22:14:31 +00:00
David Benjamin
3dd9016a51 Remove signature algorithm configuration hooks and SSL_ctrl.
They're not called (new in 1.0.2). We actually may well need to
configure these later to strike ECDSA from the list on Chrome/XP
depending on what TLS 1.3 does, but for now striking it from the cipher
suite list is both necessary and sufficient. I think we're better off
removing these for now and adding new APIs later if we need them.

(This API is weird. You pass in an array of NIDs that must be even
length and alternating between hash and signature NID. We'd also need a
way to query the configured set of sigalgs to filter away. Those used to
exist but were removed in
https://boringssl-review.googlesource.com/#/c/5347/. SSL_get_sigalgs is
an even uglier API and doesn't act on the SSL_CTX.)

And with that, SSL_ctrl and SSL_CTX_ctrl can *finally* be dropped. Don't
leave no-op wrappers; anything calling SSL_ctrl and SSL_CTX_ctrl should
instead switch to the wrapper macros.

BUG=404754

Change-Id: I5d465cd27eef30d108eeb6de075330c9ef5c05e8
Reviewed-on: https://boringssl-review.googlesource.com/5675
Reviewed-by: Adam Langley <agl@google.com>
2015-08-18 22:13:20 +00:00
Sigbjorn Vik
2b23d24c38 Make key exchange strength available.
This change stores the size of the group/modulus (for RSA/DHE) or curve
ID (for ECDHE) in the |SSL_SESSION|. This makes it available for UIs
where desired.

Change-Id: I354141da432a08f71704c9683f298b361362483d
Reviewed-on: https://boringssl-review.googlesource.com/5280
Reviewed-by: Adam Langley <agl@google.com>
2015-08-18 22:09:27 +00:00
David Benjamin
97760d5254 Slightly simplify V2ClientHello sniffing.
Rather than sniff for ClientHello, just fall through to standard logic
once weird cases are resolved.

This means that garbage will now read as WRONG_VERSION rather than
UNKNOWN_PROTOCOL, but the rules here were slightly odd anyway. This also
means we'll now accept empty records before the ClientHello (up to the
empty record limit), and process records of the wrong type with the
usual codepath during the handshake.

This shouldn't be any more risk as it just makes the ClientHello more
consistent with the rest of the protocol. A TLS implementation that
doesn't parse V2ClientHello would do the same unless it still
special-cased the first record. All newly-exposed states are reachable
by fragmenting ClientHello by one byte and then sending the record in
question.

BUG=468889

Change-Id: Ib701ae5d8adb663e158c391639b232a9d9cd1c6e
Reviewed-on: https://boringssl-review.googlesource.com/5712
Reviewed-by: Adam Langley <agl@google.com>
2015-08-17 20:48:06 +00:00
David Benjamin
fc05994e24 Fold away EC point format negotiation.
The only point format that we ever support is uncompressed, which the
RFC says implementations MUST support. The TLS 1.3 and Curve25519
forecast is that point format negotiation is gone. Each curve has just
one point format and it's labeled, for historial reasons, as
"uncompressed".

Change-Id: I8ffc8556bed1127cf288d2a29671abe3c9b3c585
Reviewed-on: https://boringssl-review.googlesource.com/5542
Reviewed-by: Adam Langley <agl@google.com>
2015-07-31 22:46:36 +00:00
nagendra modadugu
601448aa13 Add server-side support for asynchronous signing.
The RSA key exchange needs decryption and is still unsupported.

Change-Id: I8c13b74e25a5424356afbe6e97b5f700a56de41f
Reviewed-on: https://boringssl-review.googlesource.com/5467
Reviewed-by: Adam Langley <agl@google.com>
2015-07-31 01:14:29 +00:00
Adam Langley
0950563a9b Implement custom extensions.
This change mirrors upstream's custom extension API because we have some
internal users that depend on it.

Change-Id: I408e442de0a55df7b05c872c953ff048cd406513
Reviewed-on: https://boringssl-review.googlesource.com/5471
Reviewed-by: Adam Langley <agl@google.com>
2015-07-31 01:12:00 +00:00
Adam Langley
33ad2b59da Tidy up extensions stuff and drop fastradio support.
Fastradio was a trick where the ClientHello was padding to at least 1024
bytes in order to trick some mobile radios into entering high-power mode
immediately. After experimentation, the feature is being dropped.

This change also tidies up a bit of the extensions code now that
everything is using the new system.

Change-Id: Icf7892e0ac1fbe5d66a5d7b405ec455c6850a41c
Reviewed-on: https://boringssl-review.googlesource.com/5466
Reviewed-by: Adam Langley <agl@google.com>
2015-07-21 21:44:55 +00:00
David Benjamin
aa58513f40 Reserve ex_data index zero for app_data.
In the ancient times, before ex_data and OpenSSL, SSLeay supported a
single app_data slot in various types. Later app_data begat ex_data, and
app_data was replaced by compatibility macros to ex_data index zero.

Today, app_data is still in use, but ex_data never reserved index zero
for app_data. This causes some danger where, if the first ex_data
registration did not use NULL callbacks, the registration's callbacks
would collide with app_data.

Instead, add an option to the types with app_data to reserve index zero.
Also switch SSL_get_ex_data_X509_STORE_CTX_idx to always return zero
rather than allocate a new one. It used to be that you used
X509_STORE_CTX_get_app_data. I only found one consumer that we probably
don't care about, but, to be safe and since it's easy, go with the
conservative option. (Although SSL_get_ex_data_X509_STORE_CTX_idx wasn't
guaranteed to alias app_data, in practice it always did. No consumer
ever calls X509_STORE_CTX_get_ex_new_index.)

Change-Id: Ie75b279d60aefd003ffef103f99021c5d696a5e9
Reviewed-on: https://boringssl-review.googlesource.com/5313
Reviewed-by: Adam Langley <agl@google.com>
2015-07-20 16:56:34 +00:00
David Benjamin
3570d73bf1 Remove the func parameter to OPENSSL_PUT_ERROR.
Much of this was done automatically with
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/'
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+,  ([a-zA-Z_0-9]+\);)/\1\2/'

BUG=468039

Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8
Reviewed-on: https://boringssl-review.googlesource.com/5276
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:37 +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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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