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>
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>
Got one of the conditions flipped.
Change-Id: I327a9c13e42865459e8d69a431b0d3a2bc6b54a5
Reviewed-on: https://boringssl-review.googlesource.com/1210
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
