Commit Graph

242 Commits

Author SHA1 Message Date
David Benjamin
b398d16c1d Remove SSL_check_chain and unexport CERT_PKEY flags.
Both of these are newly-exported in OpenSSL 1.0.2, so they cannot be used by
current consumers.

This was added in upstream's 18d7158809c9722f4c6d2a8af7513577274f9b56 to
support custom selection of certificates. The intent seems to be that you
listen to cert_cb and use SSL_check_chain to lean on OpenSSL to process
signature algorithms list for you.

Unfortunately, the implementation is slightly suspect: it uses the same
function as the codepath which mutates and refers to the CERT_PKEY of the
matching type.  Some access was guarded by check_flags, but this is too
complex. Part of it is also because the matching digest is selected early and
we intend to connect this to EVP_PKEY_supports_digest so it is no longer a
property of just the key type.

Let's remove the hook for now, to unblock removing a lot of complexity. After
cleaning up this area, a function like this could be cleaner to support, but
we already have a version of this: select_certificate_cb and
ssl_early_callback_ctx.

Change-Id: I3add425b3996e5e32d4a88e14cc607b4fdaa5aec
Reviewed-on: https://boringssl-review.googlesource.com/2283
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:19:24 +00:00
David Benjamin
bdf5e72f50 Don't resume sessions if the negotiated version doesn't match.
All of NSS, upstream OpenSSL, SChannel, and Secure Transport require, on the
client, that the ServerHello version match the session's version on resumption.
OpenSSL's current behavior is incompatible with all of these. Fall back to a
full handshake on the server instead of mismatch.

Add a comment on the client for why we are, as of
30ddb434bf, not currently enforcing the same in
the client.

Change-Id: I60aec972d81368c4ec30e2fd515dabd69401d175
Reviewed-on: https://boringssl-review.googlesource.com/2244
Reviewed-by: Adam Langley <agl@google.com>
2014-11-13 22:05:12 +00:00
David Benjamin
688d8dfe48 Remove psk_identity_hint from SSL_SESSION.
There's not much point in retaining the identity hint in the SSL_SESSION. This
avoids the complexity around setting psk_identity hint on either the SSL or the
SSL_SESSION. Introduce a peer_psk_identity_hint for the client to store the one
received from the server.

This changes the semantics of SSL_get_psk_identity_hint; it now only returns
the value configured for the server. The client learns the hint through the
callback. This is compatible with the one use of this API in conscrypt (it
pulls the hint back out to pass to a callback).

Change-Id: I6d9131636b47f13ac5800b4451436a057021054a
Reviewed-on: https://boringssl-review.googlesource.com/2213
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:59:47 +00:00
David Benjamin
e1b20a0136 Remove SSL3_FLAGS_POP_BUFFER.
This is an experimental flag that dates back to SSLeay 0.8.1b or earlier. It's
never set internally and never set in consumers.

Change-Id: I922583635c9f3d8d93f08f1707531ad22a26ae6a
Reviewed-on: https://boringssl-review.googlesource.com/2214
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:59:13 +00:00
David Benjamin
1f10d9c8e1 Remove redundant PSK length check.
If psk_len were 0, it would already have been an error earlier. The PSK cipher
suites don't lose the other_secret || psk construction if the PSK happens to be
empty.

Change-Id: I1917236720d0862658562bc8f014cb827ee9aed5
Reviewed-on: https://boringssl-review.googlesource.com/2233
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:02:11 +00:00
David Benjamin
1df112448b Fix memory leak in ssl3_send_client_key_exchange error handling.
Change-Id: I0f0d7a3d4cb6448582ae4945e732611bb9bf5d9f
Reviewed-on: https://boringssl-review.googlesource.com/2231
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:01:07 +00:00
David Benjamin
525a0fe315 Remove client-side support for ServerKeyExchange in the RSA key exchange.
Server-side support was removed in 77a942b7fe,
but client-side support was retained as it appeared NSS supported this.
However, this is not the case: ssl3_HandleServerKeyExchange only allows a
ServerKeyExchange message if hs.ws is in an appropriate state.
ssl3_AuthCertificate only sets it to allow ServerKeyExchange if it is a key
exchange that normally uses it or if is_limited is set. is_limited is only set
for the export cipher suites.

Thus we can safely remove this without waiting on gathering UMA data.

BUG=chromium:400587

Change-Id: I9aefb742dbb2d99c13340ab48017e1ceee04bc2f
Reviewed-on: https://boringssl-review.googlesource.com/2230
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:00:09 +00:00
David Benjamin
f2b32a2de2 Switch ssl3_send_channel_id to BN_bn2bin_padded.
Check the return value while we're here. This avoids some arithmetic and
appease scan-build's dead assignment flagger.

Change-Id: If3615076e091eb44b9e3e9d50cd64f80e645337e
Reviewed-on: https://boringssl-review.googlesource.com/2204
Reviewed-by: Adam Langley <agl@google.com>
2014-11-06 01:32:27 +00:00
David Benjamin
fd617a5030 Port ssl3_{get,send}_server_key_exchange to EVP_Digest{Verify,Sign}*.
Minor change, but they're the users of the old API left within
BoringSSL.

Change-Id: Ic24e0d006c97fa5265abc3373d3f98aa8d2f8b1e
Reviewed-on: https://boringssl-review.googlesource.com/2100
Reviewed-by: Adam Langley <agl@google.com>
2014-10-31 21:59:49 +00:00
David Benjamin
93d67d36c5 Refactor ssl3_send_client_key_exchange slightly.
Like ssl3_get_client_key_exchange, it is split into three parts:
- If PSK, query the PSK and write out the PSK identity.
- Compute the base pre-master secret.
- If PSK, compute the final pre-master secret.

This also fixes some double-frees on malloc failures in the ECDHE case. And it
avoids using the handshake output buffer to start the premaster secret.

Change-Id: I8631ee33c1e9c19604b3dcce2c676c83893c308d
Reviewed-on: https://boringssl-review.googlesource.com/2062
Reviewed-by: Adam Langley <agl@google.com>
2014-10-29 20:34:07 +00:00
David Benjamin
491956c866 Fix ECDHE_PSK key exchange.
The current implementation switches the order of other_secret and psk;
other_secret is first. Fix it and rewrite with CBB instead. The server half got
fixed on accident in a prior refactor.

Change-Id: Ib52a756aadd66e4bf22c66794447f71f4772da09
Reviewed-on: https://boringssl-review.googlesource.com/2052
Reviewed-by: Adam Langley <agl@google.com>
2014-10-29 20:32:45 +00:00
Adam Langley
7571292eac Extended master secret support.
This change implements support for the extended master secret. See
https://tools.ietf.org/html/draft-ietf-tls-session-hash-01
https://secure-resumption.com/

Change-Id: Ifc7327763149ab0894b4f1d48cdc35e0f1093b93
Reviewed-on: https://boringssl-review.googlesource.com/1930
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-10-24 21:19:44 +00:00
David Benjamin
5b33a5e0dd Merge the get_ssl_method hooks between TLS and SSLv3.
Remove one more difference to worry about switching between TLS and SSLv3
method tables.

Although this does change the get_ssl_method hook for the version-specific
tables (before TLS and SSLv3 would be somewhat partitioned), it does not appear
to do anything. get_ssl_method is only ever called in SSL_set_session for
client session resumption. Either you're using the version-specific method
tables and don't know about other versions anyway or you're using SSLv23 and
don't partition TLS vs SSL3 anyway.

BUG=chromium:403378

Change-Id: I8cbdf02847653a01b04dbbcaf61fcb3fa4753a99
Reviewed-on: https://boringssl-review.googlesource.com/1842
Reviewed-by: Adam Langley <agl@google.com>
2014-09-30 22:58:59 +00:00
David Benjamin
00075b80ca Merge IMPLEMENT_tls_meth_func and IMPLEMENT_ssl3_meth_func.
The TLS-specific hooks have been removed. We aim to no longer perform version
negotiation as a pre-processing step, so ensure the only differences to worry
about are the version, get_method hook, and the enc_data.

BUG=chromium:403378

Change-Id: I628ec6f4c50ceed01d7af8f4110b6dc95cfbe023
Reviewed-on: https://boringssl-review.googlesource.com/1841
Reviewed-by: Adam Langley <agl@google.com>
2014-09-30 22:56:47 +00:00
Adam Langley
5d0c163b37 Also clean the last byte of the PSK identity.
Patch by Alex Kljubin.

Change-Id: Ieec830dce11b501aaa82f03c82ff04c3cdde41e1
Reviewed-on: https://boringssl-review.googlesource.com/1831
Reviewed-by: Adam Langley <agl@google.com>
2014-09-26 22:10:53 +00:00
David Benjamin
859ec3cc09 Add SSL_CTX_set_keylog_bio.
Configures the SSL stack to log session information to a BIO. The intent is to
support NSS's SSLKEYLOGFILE environment variable. Add support for the same
environment variable to tool/client.cc.

Tested against Wireshark 1.12.0.

BUG=393477

Change-Id: I4c231f9abebf194eb2df4aaeeafa337516774c95
Reviewed-on: https://boringssl-review.googlesource.com/1699
Reviewed-by: Adam Langley <agl@google.com>
2014-09-03 20:15:55 +00:00
David Benjamin
39ebf53dd3 Check the server did not use a TLS 1.2 cipher suite pre-TLS 1.2.
This check got refactored in OpenSSL 1.0.2 and broke in the process. Fix this
and add a test. Otherwise things like client auth can get slightly confused; it
will try to sign the MD5/SHA-1 hash, but the TLS 1.2 cipher suite may not use
SSL_HANDSHAKE_MAC_DEFAULT, so those digests won't be available.

Based on upstream's 226751ae4a1f3e00021c43399d7bb51a99c22c17.

Change-Id: I5b864d3a696f3187b849c53b872c24fb7df27924
Reviewed-on: https://boringssl-review.googlesource.com/1696
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 23:41:34 +00:00
Adam Langley
b2cb0ece76 Fix minor issues found by Clang's analysis.
Thanks to Denis Denisov for running the analysis.

Change-Id: I80810261e013423e746fd8d8afefb3581cffccc0
Reviewed-on: https://boringssl-review.googlesource.com/1701
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 22:39:41 +00:00
David Benjamin
6c7aed048c Client-side OCSP stapling support.
Remove the old implementation which was excessively general. This mirrors the
SCT support and adds a single boolean flag to request an OCSP response with no
responder IDs, extensions, or frills. The response, if received, is stored on
the SSL_SESSION so that it is available for (re)validation on session
resumption; Chromium revalidates the saved auth parameters on resume.

Server support is unimplemented for now. This API will also need to be adjusted
in the future if we implement RFC 6961.

Change-Id: I533c029b7f7ea622d814d05f934fdace2da85cb1
Reviewed-on: https://boringssl-review.googlesource.com/1671
Reviewed-by: Adam Langley <agl@google.com>
2014-08-29 00:39:33 +00:00
David Benjamin
457112e197 unifdef a bunch of OPENSSL_NO_* ifdefs.
Get all this stuff out of the way.

- OPENSSL_NO_MD5
- OPENSSL_NO_SHA
- OPENSSL_NO_EC
- OPENSSL_NO_ECDSA
- OPENSSL_NO_ECDH
- OPENSSL_NO_NEXTPROTONEG
- OPENSSL_NO_DH
- OPENSSL_NO_SSL3
- OPENSSL_NO_RC4
- OPENSSL_NO_RSA

Also manually removed a couple instances of OPENSSL_NO_DSA that seemed to be
confused anyway. Did some minor manual cleanup. (Removed a few now-pointless
'if (0)'s.)

Change-Id: Id540ba97ee22ff2309ab20ceb24c7eabe766d4c4
Reviewed-on: https://boringssl-review.googlesource.com/1662
Reviewed-by: Adam Langley <agl@google.com>
2014-08-28 00:41:34 +00:00
David Benjamin
854dd654d1 Refactor server-side CertificateVerify handling.
This moves CertificateVerify digest processing to the new
SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag. It also refactors it similarly to
ssl3_send_cert_verify and moves that logic to a common ssl3_cert_verify_hash
function to compute the handshake hash.

This removes a large chunk of duplicate (and divergent!) logic between TLS and
DTLS. It also removes TLS1_FLAGS_KEEP_HANDSHAKE.

Change-Id: Ia63c94f7d76d901bc9c4c33454fbfede411adf63
Reviewed-on: https://boringssl-review.googlesource.com/1633
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:55:27 +00:00
David Benjamin
590cbe970c Introduce a hash_message parameter to ssl_get_message.
This replaces the special-case in ssl3_get_message for Channel ID. Also add
ssl3_hash_current_message to hash the current message, taking TLS vs DTLS
handshake header size into account.

One subtlety with this flag is that a message intended to be processed with
SSL_GET_MESSAGE_DONT_HASH_MESSAGE cannot follow an optional message
(reprocessed with reuse_message, etc.).  There is an assertion to that effect.
If need be, we can loosen it to requiring that the preceeding optional message
also pass SSL_GET_MESSAGE_DONT_HASH_MESSAGE and then maintain some state to
perform the more accurate assertion, but this is sufficient for now.

Change-Id: If8c87342b291ac041a35885b9b5ee961aee86eab
Reviewed-on: https://boringssl-review.googlesource.com/1630
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:54:50 +00:00
David Benjamin
8da990677b Rename some message functions for consistency.
Make the get/send functions match.

ssl3_client_hello -> ssl3_send_client_hello.
ssl3_send_newsession_ticket -> ssl3_send_new_session_ticket.
ssl3_send_client_verify -> ssl3_send_cert_verify

Change-Id: Iea5579479b8a8f392167b8fb3b7e9fe961d0f007
Reviewed-on: https://boringssl-review.googlesource.com/1613
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 21:09:40 +00:00
David Benjamin
422d3a4015 Remove some unused state and code.
Change-Id: I1f917d1fe70ef5046eeea8fed27cc402bcd674b9
Reviewed-on: https://boringssl-review.googlesource.com/1568
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:04:32 +00:00
David Benjamin
a7d1363fcb Prune removed key types from SSL_PKEY_*.
Remove all the logic managing key types that aren't being used anymore.

Change-Id: I101369164588048e64ba1c84a6b8aac8f3a221cd
Reviewed-on: https://boringssl-review.googlesource.com/1567
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:15:32 +00:00
David Benjamin
ef2116d33c Remove DSA-based cipher suites and client auth.
DSA is not connected up to EVP, so it wouldn't work anyway. We shouldn't
advertise a cipher suite we don't support. Chrome UMA data says virtually no
handshakes end up negotiating one of these.

Change-Id: I874d934432da6318f05782ebd149432c1d1e5275
Reviewed-on: https://boringssl-review.googlesource.com/1566
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:14:40 +00:00
David Benjamin
0da0e18a60 Remove ECDH_RSA, ECDH_ECDSA, DH_RSA, and DH_DSS.
These are the variants where the CA signs a Diffie-Hellman keypair. They are
not supported by Chrome on NSS.

Change-Id: I569a7ac58454bd3ed1cd5292d1f98499012cdf01
Reviewed-on: https://boringssl-review.googlesource.com/1564
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:12:38 +00:00
David Benjamin
bd30f8e34a Remove support on both sides for *_fixed_(ec)dh client auth.
In the fixed_ecdh case, it wasn't even implemented, but there was stub code for
it. It complicates the ClientKeyExchange (the client parameters become implicit
in the certificate) and isn't used.

Change-Id: I3627a37042539c90e05e59cd0cb3cd6c56225561
Reviewed-on: https://boringssl-review.googlesource.com/1563
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:07:00 +00:00
David Benjamin
e712778a7b Remove rsa_md5, md5, and sha1 fields from SSL_CTX.
Just use the normal API for them.

Change-Id: Ibb5988611a86e8d39abda1e02087523d98defb51
Reviewed-on: https://boringssl-review.googlesource.com/1555
Reviewed-by: Adam Langley <agl@google.com>
2014-08-19 18:18:36 +00:00
David Benjamin
f2fedefdca Simplify HelloVerifyRequest processing.
Rather than switching the order of the ServerHello and HelloVerifyRequest
states and processing each twice, have the states follow the protocol order.
HelloVerifyRequest reading is optional and ServerHello is strict. Use the
send_cookie bit to determine whether we're expecting a cookie or not.

Fix the dtls1_stop_timer call in these states to consistently hit the end of a
server flight; the previous flight should not be cleared from the retransmit
buffer until the entire next flight is received. That said, OpenSSL doesn't
appear to implement the part where, on receipt of the previous peer flight, the
buffered flight is retransmitted. (With the exception of a SSL3_MT_FINISHED
special-case in dtls1_read_bytes.) So if the peer is also OpenSSL, this doesn't
do anything.

Also fix the DTLS test which wasn't actually asserting that the ClientHello
matched.

Change-Id: Ia542190972dbffabb837d32c9d453a243caa90b2
Reviewed-on: https://boringssl-review.googlesource.com/1551
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 18:05:10 +00:00
David Benjamin
6f2600199c Mark all SSL_CIPHERs as const.
This lets us put the SSL_CIPHER table in the data section. For type-checking,
make STACK_OF(SSL_CIPHER) cast everything to const SSL_CIPHER*.

Note that this will require some changes in consumers which weren't using a
const SSL_CIPHER *.

Change-Id: Iff734ac0e36f9e5c4a0f3c8411c7f727b820469c
Reviewed-on: https://boringssl-review.googlesource.com/1541
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:55:05 +00:00
David Benjamin
335d10d201 Remove Suite B mode.
It was added in OpenSSL 1.0.2, so nothing can be depending on it yet. If we
really want a Suite B profile, it seems better to generate a configuration for
the rest of the system rather than pepper the codebase with checks.

Change-Id: I1be3ebed0e87cbfe236ade4174dcf5bbc7e10dd5
Reviewed-on: https://boringssl-review.googlesource.com/1517
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 22:00:16 +00:00
Håvard Molland
16c623b83d Fix of handling weak dh server keys.
https://crbug.com/353579

Align behavior with NSS and report SSL_R_BAD_DH_P_LENGTH error
when size of the server's dh group is less than 512 bits.

Change-Id: I09f1828482f40b2283f7c6a69425819379399815
Reviewed-on: https://boringssl-review.googlesource.com/1480
Reviewed-by: Adam Langley <agl@google.com>
2014-08-13 17:59:59 +00:00
Håvard Molland
90974e7081 Fixed reporting wrong function to OPENSSL_PUT_ERROR.
ssl3_send_client_key_exchange were wrongly reported
by ssl3_send_client_certificate() and
ssl3_check_cert_and_algorithm()

Change-Id: I244d3d871b6b4f75a188fd386d52ffc4335d1f9b
Reviewed-on: https://boringssl-review.googlesource.com/1460
Reviewed-by: Adam Langley <agl@google.com>
2014-08-11 18:46:56 +00:00
Emilia Käsper
c3d79605ab Fix DTLS anonymous EC(DH) denial of service
(This change originally applied to 1.0.1. In the switch to 1.0.2, the
DTLS specific client processing was removed and now the s3_clnt.c
functions are used. This caused most of the patch to be moot. What
remains is still useful however. For the original patch, see the change
against 1.0.1: 88ae012c8092852f03c50f6461175271104b4c8a)

CVE-2014-3510

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>

(Imported from upstream's 1d7d0ed9c21403d79d602b6c7d76fdecf5e737da)

Change-Id: I666f9c48d603f2366cab821ae446a57360c3026b
Reviewed-on: https://boringssl-review.googlesource.com/1439
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 21:13:45 +00:00
David Benjamin
150c617cfc Add X509_up_ref and use it internally.
Avoid needing to manually increment the reference count and using the right
lock, both here and in Chromium.

Change-Id: If116ebc224cfb1c4711f7e2c06f1fd2c97af21dd
Reviewed-on: https://boringssl-review.googlesource.com/1415
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:06:34 +00:00
Alex Chernyakhovsky
75d1589f60 Remove PKCS1_CHECK
The code guarded by PKCS1_CHECK appears to be unhelpful, and the guard
is explicitly undefined in ssl_locl.h Remove both.

Change-Id: I3cd45a744a8f35b02181b1e48fd1ef11af5e6f4a
Reviewed-on: https://boringssl-review.googlesource.com/1383
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:14:21 +00:00
Alex Chernyakhovsky
69877b6119 Remove OPENSSL_NO_SHA256
Changes were made by running

find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_SHA256
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_SHA256

Change-Id: Ia2fda4b1a3a4aad286ced86cf936358fc7a30b15
Reviewed-on: https://boringssl-review.googlesource.com/1378
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 20:13:46 +00:00
David Benjamin
8f0ceb6f27 Remove OPENSSL_MAX_TLS1_2_CIPHER_LENGTH.
It's not built. The problem is worked around by the padding extension now.

Change-Id: If577efdae57d1bca4e0a626486fc0502c3567ebb
Reviewed-on: https://boringssl-review.googlesource.com/1374
Reviewed-by: Adam Langley <agl@google.com>
2014-08-04 19:00:30 +00:00
David Benjamin
7bbeead507 A bunch of dead assignments.
Caught by clang scan-build.

Change-Id: I4f10c879dc137d4a14a7a395764d28e5caa033ff
Reviewed-on: https://boringssl-review.googlesource.com/1342
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:44:03 +00:00
David Benjamin
7e3305eebd Fix False Start without session tickets.
One of the state transitions wasn't rewritten to CR_CHANGE. Add a test to
exercise this codepath. Also SSL_cutthrough_complete references the state.

Change-Id: Ib2f7ac5ac3f0348864efa93cf13cfd87454572f0
Reviewed-on: https://boringssl-review.googlesource.com/1337
Reviewed-by: Adam Langley <agl@google.com>
2014-07-28 20:31:56 +00:00
David Benjamin
86271ee9f8 Change CCS_OK to EXPECT_CCS.
Now that the flag is set accurately, use it to enforce that the handshake and
CCS synchronization. If EXPECT_CCS is set, enforce that:

(a) No handshake records may be received before ChangeCipherSpec.

(b) There is no pending handshake data at the point EXPECT_CCS is set.

Change-Id: I04b228fe6a7a771cf6600b7d38aa762b2d553f08
Reviewed-on: https://boringssl-review.googlesource.com/1299
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 17:58:58 +00:00
David Benjamin
cb5abad717 Consolidate CCS_OK paths in s3_clnt.c.
Introduce a CR_CHANGE state just before entering CR_FINISHED_A. This replaces
the CCS_OK in the CR_FINISHED_A/CR_FINISHED_B case which otherwise would get
applied after partial reads of Finished. The other CCS_OK settings are
redundant with this one.

The copy in tls_secret_session_cb codepath is made unnecessary with
9eaeef81fa.

The copy in the normal session resumption case is unnecessary with
6444287806. Before that commit, OpenSSL would
potentially read Finished a state early. Now that we are strict (and get the
book-keeping correct) for expecting the NewSessionTicket message it too is
redundant.

Of particular note is the one after ssl3_send_finished. That was added in
response to upstream's PR#3400. I've reproduced the bug and concluded it was
actually a bug around expecting a NewSessionTicket message. That has been fixed
properly in 6444287806 by resetting
tlsext_expect_ticket on renegotiations.

Change-Id: I6a928386994fcd5efff26a5f0efb12b65bf7f299
Reviewed-on: https://boringssl-review.googlesource.com/1298
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 17:49:12 +00:00
David Benjamin
cd9969434c Pass parameters to tls1_process_sigalgs as a CBS.
Slightly cleaner; it means we can use CBS_stow.

Change-Id: I074aa2d73a79648013dea025ee531beeea2af4a2
Reviewed-on: https://boringssl-review.googlesource.com/1287
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:18:38 +00:00
David Benjamin
060d9d2c56 Remove support code for export cipher suites.
Now the only case where temporary RSA keys are used on the server end is
non-signing keys.

Change-Id: I55f6c206e798dd28548c386fdffd555ccc395477
Reviewed-on: https://boringssl-review.googlesource.com/1285
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:14:08 +00:00
David Benjamin
ff175b4a46 unifdef OPENSSL_NO_PSK.
Get those out of the way.

Change-Id: I4cc8c34cf637379ad734c43623f76ae72f22014e
Reviewed-on: https://boringssl-review.googlesource.com/1282
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:11:05 +00:00
David Benjamin
019c3cc64a Remove last remnants of GOST support.
This removes support code for a "stream_mac" mode only used by GOST. Also get
rid of this

   /* I should fix this up TLS TLS TLS TLS TLS XXXXXXXX */

comment next to it. It's not actually related to GOST (dates to OpenSSL initial
commit), but isn't especially helpful at this point.

Change-Id: Ib13c6e27e16e0d1fb59ed0142ddf913b9abc20b7
Reviewed-on: https://boringssl-review.googlesource.com/1281
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:10:53 +00:00
David Benjamin
39482a13ac Remove get_cipher_by_char and put_cipher_by_char.
Without SSLv2, all cipher suite values are 2 bytes. Represent them as a
uint16_t and make all functions pass those around rather than pointers.

This removes SSL_CIPHER_find as it's unused.

Change-Id: Iea0b75abee4352a8333a4b8e39a161430ae55ea6
Reviewed-on: https://boringssl-review.googlesource.com/1259
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:10:29 +00:00
David Benjamin
6444287806 Do not allow a Finished message when NewSessionTicket is expected.
Per spec, the server sends it iff it sends the extension in ServerHello. There
is no need to probe for whether Finished is or isn't sent. NSS is strict about
this (wait_new_session_ticket never transitions to wait_change_cipher without a
NewSessionTicket message), so this is safe.

Reset tlsext_ticket_expected in ssl_scan_serverhello_tlsext to ensure state
from the initial handshake doesn't confuse renegotiation.  This is another one
of those per-handshake states that should be systematically reset on each
handshake. For now, reset it properly at least.

Change-Id: I7d16439ce632b9abf42f62d5d8e1303cb6f0be1f
Reviewed-on: https://boringssl-review.googlesource.com/1296
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:53:29 +00:00
David Benjamin
407886f589 Remove ssl3_check_finished.
ssl3_get_new_session_ticket is sensible and fills in a session_id for stateless
sessions, so the resumption will already be detected at this point. Remove the
codepath in ssl3_client_hello which allows for resuming sessions with empty
session_ids. The rest of the code doesn't allow it either.

This removes another codepath where we potentially probe a Finished message
early.

Change-Id: I2749b5c65c7ce98c6f30566d8716360ff1bba24c
Reviewed-on: https://boringssl-review.googlesource.com/1295
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:52:38 +00:00
David Benjamin
9eaeef81fa Set s->hit when using tls_session_secret_cb.
tls_session_secret_cb is used for EAP-FAST which computes the master secret
externally and enters the abbreviated handshake. It appears to only have been
working because ssl3_check_finished would drive it into the appropriate state
afterwards. That, in turn, only has been working because EAP-FAST misuses the
session ticket extension for some other field, so ssl3_check_finished isn't a
no-op.

Instead, set s->hit so it follows the abbreviated state machine directly.

If we ever build wpa_supplicant with BoringSSL, this will require some testing.
(And, if not, this API should be removed.)

Change-Id: Ie2992a9bba049f7120522b996f739906e38a070e
Reviewed-on: https://boringssl-review.googlesource.com/1294
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:50:29 +00:00
Adam Langley
37a623cf29 Have BIO_get_mem_data return a size_t and uint8_t*
Change-Id: I883f9c3527b572a2140ae4899cf4409cdc25c6dc
Reviewed-on: https://boringssl-review.googlesource.com/1261
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 17:22:05 +00:00
Adam Langley
045cc5590a Don't delay CKX and Finished for False Start.
Android never did this - they patched out the point in the code that set
the SSL3_FLAGS_DELAY_CLIENT_FINISHED flag when doing False Start.
Also, from the unittests it appears that NSS doesn't do this either.
Thus this change brings BoringSSL into line with existing behaviour.

SSL3_FLAGS_DELAY_CLIENT_FINISHED wasn't introduced with False Start,
it's an option in vanilla OpenSSL. But I can't find anything that uses
it and, since it's going to be untested, I've removed it completely in
this change.

Change-Id: I910537bfa35e74ab88778b83612cf5607d485969
Reviewed-on: https://boringssl-review.googlesource.com/1221
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 18:59:38 +00:00
David Benjamin
407a10cf43 Fix parsing of CertificateRequests.
Got one of the conditions flipped.

Change-Id: I327a9c13e42865459e8d69a431b0d3a2bc6b54a5
Reviewed-on: https://boringssl-review.googlesource.com/1210
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 17:07:15 +00:00
David Benjamin
22f9bccde5 Port ssl3_get_client_hello to CBS.
Also fix some DTLS cookie bugs. rcvd_cookie is never referenced after being
saved (and the length isn't saved, so it couldn't be used anyway), and the
cookie verification failed to check the length.

For convenience, add a CBS_mem_equal helper function. Saves a bit of
repetition.

Change-Id: I187137733b069f0ac8d8b1bf151eeb80d388b971
Reviewed-on: https://boringssl-review.googlesource.com/1174
Reviewed-by: Adam Langley <agl@google.com>
2014-07-15 18:30:09 +00:00
David Benjamin
14c83e7d00 Refactor PSK logic in ssl3_get_client_key_exchange.
This avoids duplicating the code to build the final premaster in PSK and
ECDHE_PSK. It also ports it to CBB for an initial trial of the API. Computing
the premaster secret now proceeds in four steps:

1. If a PSK key exchange (alg_a), look up the pre-shared key.
2. Compute the premaster secret based on alg_k. If PSK, it's all zeros.
3. If a PSK key exchange (alg_a), wrap the premaster in a struct with the
   pre-shared key.
4. Use the possibly modified premaster to compute the master secret.

Change-Id: Ib511dd2724cbed42c82b82a676f641114cec5470
Reviewed-on: https://boringssl-review.googlesource.com/1173
Reviewed-by: Adam Langley <agl@google.com>
2014-07-15 00:02:14 +00:00
David Benjamin
8f8040dd4f Rename ssl3_get_key_exchange to ssl3_get_server_key_exchange.
More consistent with ssl3_send_server_key_exchange and the message name.

Change-Id: If0f435a89bdf117297d349099708fff0bd5a6e98
Reviewed-on: https://boringssl-review.googlesource.com/1170
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:55:35 +00:00
David Benjamin
05da6e1641 Port tls12_check_peer_sigalg to CBS.
This avoids having to do the CBS_skip dance and is better about returning the
right alert.

Change-Id: Id84eba307d7c67269ccbc07a38d9044b6f4f7c6c
Reviewed-on: https://boringssl-review.googlesource.com/1169
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:55:01 +00:00
David Benjamin
a03d95d156 Port ssl3_get_server_hello to CBS.
Change-Id: I8ec531cf327653b4779ff6a29cac62e240502269
Reviewed-on: https://boringssl-review.googlesource.com/1167
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:52:52 +00:00
David Benjamin
46062681ab Port ssl3_get_cert_status to CBS.
Change-Id: I18b68f32fceb0f9273f2d86ec201ebf9350103df
Reviewed-on: https://boringssl-review.googlesource.com/1166
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:42:28 +00:00
David Benjamin
e044c3d8a2 Port ssl3_get_new_session_ticket to CBS.
Change-Id: Iabca923c9be48d001abd3b12b8c6898e604aa85a
Reviewed-on: https://boringssl-review.googlesource.com/1165
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:38:16 +00:00
David Benjamin
ed43958853 Port ssl3_get_key_exchange to CBS.
Also tidy up some variable names and update RSA_verify call for it no longer
returning -1. Add CBS helper functions for dealing with C strings.

Change-Id: Ibc398d27714744f5d99d4f94ae38210cbc89471a
Reviewed-on: https://boringssl-review.googlesource.com/1164
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:37:29 +00:00
David Benjamin
51b1f7427b Make init_msg a uint8_t*.
It's current a void* and gets explicitly cast everywhere. Make it a uint8_t and
only add the casts when converting it come init_buf, which internally stores a
char*.

Change-Id: I28bed129e46ed37ee1ce378d5c3bd0738fc1177f
Reviewed-on: https://boringssl-review.googlesource.com/1163
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:43:20 +00:00
David Benjamin
9c651c9ef6 Be strict about requiring ServerKeyExchange.
Missing ServerKeyExchange is handled, but only because it hits an
ERR_R_INTERNAL_ERROR in ssl3_send_client_key_exchange in trying to find the
server ECDH parameters. Be strict about requiring it for ECDHE.

Change-Id: Ifce5b73c8bd14746b8a2185f479d550e9e3f84df
Reviewed-on: https://boringssl-review.googlesource.com/1157
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:42:30 +00:00
David Benjamin
e8f3d666cc Be strict about expecting a server Certificate message.
Introduce a ssl_cipher_has_server_public_key to save the repeated
NULL/PSK/RSA_PSK[*] check. Don't allow skipping to ServerKeyExchange when
expecting Certificate; the messages expected are determined by the cipher
suite. The ssl3_get_server_public_key call is already guarded.

As the previous test demonstrates, this is safe because of the
ssl3_check_cert_and_algorithm call, but avoid the looseness in the parsing
there.

[*] NB: we don't implement RSA_PSK, and OpenSSL has never implemented it.

Change-Id: I0571e6bcbeb8eb883f77878bdc98d1aa3a287cf3
Reviewed-on: https://boringssl-review.googlesource.com/1156
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:50:30 +00:00
David Benjamin
d26aea6c96 Remove remnants of KRB5 support.
This drops the bits of logic that allowed Certificate messages to be optional
for a KRB5 cipher suite.

Change-Id: I2a71b7c13d7e76f4f5542d4074169f80f3617240
Reviewed-on: https://boringssl-review.googlesource.com/1154
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:57 +00:00
David Benjamin
0c49ec97f4 Fix potential memory leak.
This can't happen because we don't implement RSA_PSK, but we probably should
check here.

Probably |sess_cert| shouldn't be attached to SSL_SESSION anyway; it's only
relevant when initializing the session and if it's accessed afterwards, it'll
be shared and cause problems.

Change-Id: Id868e523195f33c22e057f9b89dc02fe68e9b554
Reviewed-on: https://boringssl-review.googlesource.com/1153
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:39 +00:00
David Benjamin
5235f74bc2 Fix algorithm_auth and SSL_k* mismatch.
Fixes bug introduced in c26c802a89. Only one of
the two halves got flipped.

Change-Id: I0b3905ab22b0f83f093e1720af85594b1a970a7f
Reviewed-on: https://boringssl-review.googlesource.com/1152
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:26 +00:00
David Benjamin
0e2908a806 Unify signing codepath of ssl3_send_client_verify.
Instead of, in the pre-TLS-1.2 case, reaching into the EVP and manually
signing, compute the digest separately from signing. Then use EVP_PKEY_sign.
This will make it easier to implement https://crbug.com/347404 by having only
one signing codepath as well as make that logic simpler.

Also add a bounds check while we're here, although the buffer is too large to
actually matter.

runner.go client auth tests should cover code changes.

Change-Id: I7d87181bbcc5a761660412452e508d24c4725327
Reviewed-on: https://boringssl-review.googlesource.com/1122
Reviewed-by: Adam Langley <agl@google.com>
2014-07-10 00:05:14 +00:00
David Benjamin
676d1e780e Separate client and server certificate_types.
This is the first of reorganizing state between connection state and handshake
state. The existing set are retained in cert_st for the server; they are server
configuration. The client gets a copy in s->s3->tmp alongside other handshake
state.

With other handshake state moved there, hopefully we can reset that state in
one go and possibly not even maintain it when there is no handshake in
progress.  Rather than currently where we sometimes confused connection state
and handshake state and have to reset as appropriate on renegotiate.

While I'm here, document the fields and name them something more useful than
'ctypes'.

Change-Id: Ib927579f0004fc5c6854fce2127625df669b2b6d
Reviewed-on: https://boringssl-review.googlesource.com/1113
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 19:51:08 +00:00
David Benjamin
6dbd73db5d Remove OPENSSL_NO_TLSEXT compilation option.
Mostly done with unifdef.

Change-Id: I876f79f9e96d77628d696b09694363d07aee6b74
Reviewed-on: https://boringssl-review.googlesource.com/1096
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:31:37 +00:00
Alex Chernyakhovsky
31955f91dc Port Certificate Request parsing to crypto/bytestring
Along the way, clean up the certificate types code to not have the
hard-coded fixed-size array.

Change-Id: If3e5978f7c5099478a3dfa37a0a7059072f5454a
Reviewed-on: https://boringssl-review.googlesource.com/1103
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:27:04 +00:00
Alex Chernyakhovsky
9c890d4b70 Remove SSL_OP_NETSCAPE_CA_DN_BUG
SSL_OP_NETSCAPE_CA_DN_BUG is not included in SSL_OP_ALL.

Change-Id: I1635ad2721ed2742b1dff189d68bfc67a1c840a6
Reviewed-on: https://boringssl-review.googlesource.com/1102
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:21:59 +00:00
Alex Chernyakhovsky
3c5034e97c Remove OPENSSL_NO_RSA
Building without RSA support is unreasonable. Changes were made by
running

find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_RSA
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_RSA

using unifdef 2.10 and some newlines were removed manually.

Change-Id: Iea559e2d4b3d1053f28a4a9cc2f7a3d1f6cabd61
Reviewed-on: https://boringssl-review.googlesource.com/1095
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:20:53 +00:00
Alex Chernyakhovsky
c6318e349a Port Server Certificate parsing to crypto/bytestring
Change-Id: I79e5475be4d8d1fe6fcce6ae7ab5b43e0dc98efa
Reviewed-on: https://boringssl-review.googlesource.com/1094
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:20:19 +00:00
David Benjamin
2b0aeecabf Remove authz extension (RFC5878)
Found no users of the functions which control the feature. (Also I don't
particularly want to port all of that to CBS...)

Change-Id: I55da42c44d57252bd47bdcb30431be5e6e90dc56
Reviewed-on: https://boringssl-review.googlesource.com/1061
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 20:48:38 +00:00
David Benjamin
13ab3e3ce1 Remove heartbeat extension.
Change-Id: I0273a31e49c5367b89b9899553e3ebe13ec50687
Reviewed-on: https://boringssl-review.googlesource.com/1050
Reviewed-by: Adam Langley <agl@google.com>
2014-06-26 20:48:19 +00:00
David Benjamin
03973096f4 Port ServerHello extension parsing to CBS.
This gives us systematic bounds-checking on all the parses. Also adds a
convenience function, CBS_memdup, for saving the current contents of a CBS.

Change-Id: I17dad74575f03121aee3f771037b8806ff99d0c3
Reviewed-on: https://boringssl-review.googlesource.com/1031
Reviewed-by: Adam Langley <agl@google.com>
2014-06-26 20:42:27 +00:00
David Benjamin
3f6fa3db62 Remove more remnants of compression.
Change-Id: I721914594fc92a66d95c7ec2088f13b68e964103
2014-06-24 18:43:57 -04:00
Adam Langley
3213bed728 Remove GOST support code from ssl/
Also remove related quirk, SSL_OP_CRYPTOPRO_TLSEXT_BUG. Glue code is
left in for now.

Change-Id: Ic09593dabf7da6ba3904fffe59f322a7c7cb74f4
2014-06-23 12:26:07 -07:00
Adam Langley
a433cbdc4f Accept CCS after sending finished.
Allow CCS after finished has been sent by client: at this point
keys have been correctly set up so it is OK to accept CCS from
server. Without this renegotiation can sometimes fail.

PR#3400

(Imported from upstream's 90d94ce39ecc2fad7fb2b8eb6bde0c669a65ee81)
2014-06-20 13:17:43 -07:00
Adam Langley
6acf476eb6 Make tls_session_secret_cb work with CVE-2014-0224 fix.
If application uses tls_session_secret_cb for session resumption set the
CCS_OK flag.

(Imported from upstream's a21f350a76b34b66dcaf9c1676baec945f32e980)
2014-06-20 13:17:41 -07:00
Adam Langley
ce7f9caa98 Fix for CVE-2014-0224
Only accept change cipher spec when it is expected instead of at any
time. This prevents premature setting of session keys before the master
secret is determined which an attacker could use as a MITM attack.

Thanks to KIKUCHI Masashi (Lepidum Co. Ltd.) for reporting this issue
and providing the initial fix this patch is based on.

(Imported from upstream's 77719aefb8f549ccc7f04222174889615d62057b)
2014-06-20 13:17:41 -07:00
Adam Langley
d06eddd15c Fix CVE-2014-3470
Check session_cert is not NULL before dereferencing it.

(Imported from upstream's e5f706590c7b1f19ca34415593aebdb6cbef355b)
2014-06-20 13:17:41 -07:00
Adam Langley
01797e309f psk_client_callback, 128-byte id bug.
Fix a bug in handling of 128 byte long PSK identity in
psk_client_callback.

OpenSSL supports PSK identities of up to (and including) 128 bytes in
length. PSK identity is obtained via the psk_client_callback,
implementors of which are expected to provide a NULL-terminated
identity. However, the callback is invoked with only 128 bytes of
storage thus making it impossible to return a 128 byte long identity and
the required additional NULL byte.

This CL fixes the issue by passing in a 129 byte long buffer into the
psk_client_callback. As a safety precaution, this CL also zeroes out the
buffer before passing it into the callback, uses strnlen for obtaining
the length of the identity returned by the callback, and aborts the
handshake if the identity (without the NULL terminator) is longer than
128 bytes.
2014-06-20 13:17:37 -07:00
Adam Langley
aed2306b9d Refactor ssl3_send_client_verify.
The original logic was a confusing spaghetti and mixed up initialization
for all the different cases together.
2014-06-20 13:17:37 -07:00
Adam Langley
0289c73051 Fix TLS-PSK identity hint implementation issues.
PSK identity hint can be stored in SSL_CTX and in SSL/SSL_SESSION,
similar to other TLS parameters, with the value in SSL/SSL_SESSION
taking precedence over the one in SSL_CTX. The value in SSL_CTX is
shared (used as the default) between all SSL instances associated
with that SSL_CTX, whereas the value in SSL/SSL_SESSION is confined
to that particular TLS/SSL connection/session.

The existing implementation of TLS-PSK does not correctly distinguish
between PSK identity hint in SSL_CTX and in SSL/SSL_SESSION. This
change fixes these issues:
1. SSL_use_psk_identity_hint does nothing and returns "success" when
   the SSL object does not have an associated SSL_SESSION.
2. On the client, the hint in SSL_CTX (which is shared between
   multiple SSL instances) is overwritten with the hint received from
   server or reset to NULL if no hint was received.
3. On the client, psk_client_callback is invoked with the hint from
   SSL_CTX rather than from current SSL/SSL_SESSION (i.e., the one
   received from the server). Issue #2 above masks this issue.
4. On the server, the hint in SSL/SSL_SESSION is ignored and the hint
   from SSL_CTX is sent to the client.
5. On the server, the hint in SSL/SSL_SESSION is reset to the one in
   SSL_CTX after the ClientKeyExchange message step.

This change fixes the issues by:
* Adding storage for the hint in the SSL object. The idea being that
  the hint in the associated SSL_SESSION takes precedence.
* Reading the hint during the handshake only from the associated
  SSL_SESSION object.
* Initializing the hint in SSL object with the one from the SSL_CTX
  object.
* Initializing the hint in SSL_SESSION object with the one from the
  SSL object.
* Making SSL_use_psk_identity_hint and SSL_get_psk_identity_hint
  set/get the hint to/from SSL_SESSION associated with the provided
  SSL object, or, if no SSL_SESSION is available, set/get the hint
  to/from the provided SSL object.
* Removing code which resets the hint during handshake.
2014-06-20 13:17:36 -07:00
Adam Langley
b0c235ed36 TLS extension limit check fixes.
Fix limit checks in ssl_add_clienthello_tlsext and
ssl_add_serverhello_tlsext.

Some of the limit checks reference p rather than ret. p is the original
buffer position, not the current one. Fix those and rename p to orig so
it's clearer.
2014-06-20 13:17:36 -07:00
Adam Langley
c26c802a89 Implement ECDHE-PSK-WITH-AES-128-GCM-SHA256. 2014-06-20 13:17:35 -07:00
Adam Langley
1258b6a756 ChannelID support.
Implement ChannelID as both a client and server.
2014-06-20 13:17:33 -07:00
Adam Langley
adb739e1e5 False Start support.
(Called "cut through" for historical reasons in this patch.)

Enables SSL3+ clients to send application data immediately following the
Finished message even when negotiating full-handshakes.  With this
patch, clients can negotiate SSL connections in 1-RTT even when
performing full-handshakes.
2014-06-20 13:17:32 -07:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00