Replace init_msg/init_num with a get_message hook.
Rather than init_msg/init_num, there is a get_message function which
either returns success or try again. This function does not advance the
current message (see the previous preparatory change). It only completes
the current one if necessary.
Being idempotent means it may be freely placed at the top of states
which otherwise have other asychronous operations. It also eases
converting the TLS 1.2 state machine. See
https://docs.google.com/a/google.com/document/d/11n7LHsT3GwE34LAJIe3EFs4165TI4UR_3CqiM9LJVpI/edit?usp=sharing
for details.
The read_message hook (later to be replaced by something which doesn't
depend on BIO) intentionally does not finish the handshake, only "makes
progress". A follow-up change will align both TLS and DTLS on consuming
one handshake record and always consuming the entire record (so init_buf
may contain trailing data). In a few places I've gone ahead and
accounted for that case because it was more natural to do so.
This change also removes a couple pointers of redundant state from every
socket.
Bug: 128
Change-Id: I89d8f3622d3b53147d69ee3ac34bb654ed044a71
Reviewed-on: https://boringssl-review.googlesource.com/18806
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index c43bda3..aa1524b 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -766,20 +766,19 @@
static int dtls1_get_hello_verify_request(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- CBS hello_verify_request, cookie;
- uint16_t server_version;
-
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
return ret;
}
- if (ssl->s3->tmp.message_type != DTLS1_MT_HELLO_VERIFY_REQUEST) {
+ if (msg.type != DTLS1_MT_HELLO_VERIFY_REQUEST) {
ssl->d1->send_cookie = false;
return 1;
}
- CBS_init(&hello_verify_request, ssl->init_msg, ssl->init_num);
+ CBS hello_verify_request = msg.body, cookie;
+ uint16_t server_version;
if (!CBS_get_u16(&hello_verify_request, &server_version) ||
!CBS_get_u8_length_prefixed(&hello_verify_request, &cookie) ||
CBS_len(&cookie) > sizeof(ssl->d1->cookie) ||
@@ -797,17 +796,17 @@
return 1;
}
-static int parse_server_version(SSL_HANDSHAKE *hs, uint16_t *out) {
+static int parse_server_version(SSL_HANDSHAKE *hs, uint16_t *out,
+ const SSLMessage &msg) {
SSL *const ssl = hs->ssl;
- if (ssl->s3->tmp.message_type != SSL3_MT_SERVER_HELLO &&
- ssl->s3->tmp.message_type != SSL3_MT_HELLO_RETRY_REQUEST) {
+ if (msg.type != SSL3_MT_SERVER_HELLO &&
+ msg.type != SSL3_MT_HELLO_RETRY_REQUEST) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
return 0;
}
- CBS server_hello;
- CBS_init(&server_hello, ssl->init_msg, ssl->init_num);
+ CBS server_hello = msg.body;
if (!CBS_get_u16(&server_hello, out)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
@@ -816,8 +815,7 @@
/* The server version may also be in the supported_versions extension if
* applicable. */
- if (ssl->s3->tmp.message_type != SSL3_MT_SERVER_HELLO ||
- *out != TLS1_2_VERSION) {
+ if (msg.type != SSL3_MT_SERVER_HELLO || *out != TLS1_2_VERSION) {
return 1;
}
@@ -871,7 +869,8 @@
static int ssl3_get_server_hello(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
uint32_t err = ERR_peek_error();
if (ERR_GET_LIB(err) == ERR_LIB_SSL &&
@@ -888,7 +887,7 @@
}
uint16_t server_version;
- if (!parse_server_version(hs, &server_version)) {
+ if (!parse_server_version(hs, &server_version, msg)) {
return -1;
}
@@ -924,14 +923,13 @@
ssl_clear_tls13_state(hs);
- if (!ssl_check_message_type(ssl, SSL3_MT_SERVER_HELLO)) {
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_SERVER_HELLO)) {
return -1;
}
- CBS server_hello, server_random, session_id;
+ CBS server_hello = msg.body, server_random, session_id;
uint16_t cipher_suite;
uint8_t compression_method;
- CBS_init(&server_hello, ssl->init_msg, ssl->init_num);
if (!CBS_skip(&server_hello, 2 /* version */) ||
!CBS_get_bytes(&server_hello, &server_random, SSL3_RANDOM_SIZE) ||
!CBS_get_u8_length_prefixed(&server_hello, &session_id) ||
@@ -1015,7 +1013,7 @@
/* Now that the cipher is known, initialize the handshake hash and hash the
* ServerHello. */
if (!hs->transcript.InitHash(ssl3_protocol_version(ssl), c->algorithm_prf) ||
- !ssl_hash_current_message(hs)) {
+ !ssl_hash_message(hs, msg)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return -1;
}
@@ -1066,22 +1064,21 @@
static int ssl3_get_server_certificate(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
return ret;
}
- if (!ssl_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
- !ssl_hash_current_message(hs)) {
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_CERTIFICATE) ||
+ !ssl_hash_message(hs, msg)) {
return -1;
}
- CBS cbs;
- CBS_init(&cbs, ssl->init_msg, ssl->init_num);
-
+ CBS body = msg.body;
uint8_t alert = SSL_AD_DECODE_ERROR;
UniquePtr<STACK_OF(CRYPTO_BUFFER)> chain;
- if (!ssl_parse_cert_chain(&alert, &chain, &hs->peer_pubkey, NULL, &cbs,
+ if (!ssl_parse_cert_chain(&alert, &chain, &hs->peer_pubkey, NULL, &body,
ssl->ctx->pool)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
return -1;
@@ -1090,7 +1087,7 @@
hs->new_session->certs = chain.release();
if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) == 0 ||
- CBS_len(&cbs) != 0 ||
+ CBS_len(&body) != 0 ||
!ssl->ctx->x509_method->session_cache_objects(hs->new_session.get())) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
@@ -1137,24 +1134,24 @@
static int ssl3_get_cert_status(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
return ret;
}
- if (ssl->s3->tmp.message_type != SSL3_MT_CERTIFICATE_STATUS) {
+ if (msg.type != SSL3_MT_CERTIFICATE_STATUS) {
/* A server may send status_request in ServerHello and then change
* its mind about sending CertificateStatus. */
return 1;
}
- if (!ssl_hash_current_message(hs)) {
+ if (!ssl_hash_message(hs, msg)) {
return -1;
}
- CBS certificate_status, ocsp_response;
+ CBS certificate_status = msg.body, ocsp_response;
uint8_t status_type;
- CBS_init(&certificate_status, ssl->init_msg, ssl->init_num);
if (!CBS_get_u8(&certificate_status, &status_type) ||
status_type != TLSEXT_STATUSTYPE_ocsp ||
!CBS_get_u24_length_prefixed(&certificate_status, &ocsp_response) ||
@@ -1178,12 +1175,13 @@
static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
return ret;
}
- if (ssl->s3->tmp.message_type != SSL3_MT_SERVER_KEY_EXCHANGE) {
+ if (msg.type != SSL3_MT_SERVER_KEY_EXCHANGE) {
/* Some ciphers (pure PSK) have an optional ServerKeyExchange message. */
if (ssl_cipher_requires_server_key_exchange(hs->new_cipher)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
@@ -1194,18 +1192,13 @@
return 1;
}
- if (!ssl_hash_current_message(hs)) {
+ if (!ssl_hash_message(hs, msg)) {
return -1;
}
- /* Retain a copy of the original CBS to compute the signature over. */
- CBS server_key_exchange;
- CBS_init(&server_key_exchange, ssl->init_msg, ssl->init_num);
- CBS server_key_exchange_orig = server_key_exchange;
-
uint32_t alg_k = hs->new_cipher->algorithm_mkey;
uint32_t alg_a = hs->new_cipher->algorithm_auth;
-
+ CBS server_key_exchange = msg.body;
if (alg_a & SSL_aPSK) {
CBS psk_identity_hint;
@@ -1281,11 +1274,11 @@
}
/* At this point, |server_key_exchange| contains the signature, if any, while
- * |server_key_exchange_orig| contains the entire message. From that, derive
- * a CBS containing just the parameter. */
+ * |msg.body| contains the entire message. From that, derive a CBS containing
+ * just the parameter. */
CBS parameter;
- CBS_init(¶meter, CBS_data(&server_key_exchange_orig),
- CBS_len(&server_key_exchange_orig) - CBS_len(&server_key_exchange));
+ CBS_init(¶meter, CBS_data(&msg.body),
+ CBS_len(&msg.body) - CBS_len(&server_key_exchange));
/* ServerKeyExchange should be signed by the server's public key. */
if (ssl_cipher_uses_certificate_auth(hs->new_cipher)) {
@@ -1367,29 +1360,27 @@
static int ssl3_get_certificate_request(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int msg_ret = ssl->method->ssl_get_message(ssl);
- if (msg_ret <= 0) {
- return msg_ret;
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
+ if (ret <= 0) {
+ return ret;
}
- if (ssl->s3->tmp.message_type == SSL3_MT_SERVER_HELLO_DONE) {
+ if (msg.type == SSL3_MT_SERVER_HELLO_DONE) {
/* If we get here we don't need the handshake buffer as we won't be doing
* client auth. */
hs->transcript.FreeBuffer();
return 1;
}
- if (!ssl_check_message_type(ssl, SSL3_MT_CERTIFICATE_REQUEST) ||
- !ssl_hash_current_message(hs)) {
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_CERTIFICATE_REQUEST) ||
+ !ssl_hash_message(hs, msg)) {
return -1;
}
- CBS cbs;
- CBS_init(&cbs, ssl->init_msg, ssl->init_num);
-
/* Get the certificate types. */
- CBS certificate_types;
- if (!CBS_get_u8_length_prefixed(&cbs, &certificate_types)) {
+ CBS body = msg.body, certificate_types;
+ if (!CBS_get_u8_length_prefixed(&body, &certificate_types)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return -1;
@@ -1403,7 +1394,7 @@
if (ssl3_protocol_version(ssl) >= TLS1_2_VERSION) {
CBS supported_signature_algorithms;
- if (!CBS_get_u16_length_prefixed(&cbs, &supported_signature_algorithms) ||
+ if (!CBS_get_u16_length_prefixed(&body, &supported_signature_algorithms) ||
!tls1_parse_peer_sigalgs(hs, &supported_signature_algorithms)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
@@ -1413,13 +1404,13 @@
uint8_t alert = SSL_AD_DECODE_ERROR;
UniquePtr<STACK_OF(CRYPTO_BUFFER)> ca_names =
- ssl_parse_client_CA_list(ssl, &alert, &cbs);
+ ssl_parse_client_CA_list(ssl, &alert, &body);
if (!ca_names) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
return -1;
}
- if (CBS_len(&cbs) != 0) {
+ if (CBS_len(&body) != 0) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return -1;
@@ -1434,18 +1425,19 @@
static int ssl3_get_server_hello_done(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
return ret;
}
- if (!ssl_check_message_type(ssl, SSL3_MT_SERVER_HELLO_DONE) ||
- !ssl_hash_current_message(hs)) {
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_SERVER_HELLO_DONE) ||
+ !ssl_hash_message(hs, msg)) {
return -1;
}
/* ServerHelloDone is empty. */
- if (ssl->init_num > 0) {
+ if (CBS_len(&msg.body) != 0) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return -1;
@@ -1796,19 +1788,19 @@
static int ssl3_get_new_session_ticket(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- int ret = ssl->method->ssl_get_message(ssl);
+ SSLMessage msg;
+ int ret = ssl_read_message(ssl, &msg);
if (ret <= 0) {
return ret;
}
- if (!ssl_check_message_type(ssl, SSL3_MT_NEW_SESSION_TICKET) ||
- !ssl_hash_current_message(hs)) {
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_NEW_SESSION_TICKET) ||
+ !ssl_hash_message(hs, msg)) {
return -1;
}
- CBS new_session_ticket, ticket;
+ CBS new_session_ticket = msg.body, ticket;
uint32_t tlsext_tick_lifetime_hint;
- CBS_init(&new_session_ticket, ssl->init_msg, ssl->init_num);
if (!CBS_get_u32(&new_session_ticket, &tlsext_tick_lifetime_hint) ||
!CBS_get_u16_length_prefixed(&new_session_ticket, &ticket) ||
CBS_len(&new_session_ticket) != 0) {