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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
(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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
