Convert ssl3_send_server_hello to CBB. BUG=468889 Change-Id: I899d67addbff01c64175f47b19ca2b688626405b Reviewed-on: https://boringssl-review.googlesource.com/6191 Reviewed-by: Adam Langley <alangley@gmail.com>
diff --git a/ssl/internal.h b/ssl/internal.h index 7f13ebc..fe6ee24 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1123,7 +1123,7 @@ int ssl3_get_initial_bytes(SSL *s); int ssl3_get_v2_client_hello(SSL *s); int ssl3_get_client_hello(SSL *s); -int ssl3_send_server_hello(SSL *s); +int ssl3_send_server_hello(SSL *ssl); int ssl3_send_server_key_exchange(SSL *s); int ssl3_send_certificate_request(SSL *s); int ssl3_send_server_done(SSL *s); @@ -1214,8 +1214,7 @@ * length. (It does not include the record header.) */ int ssl_add_clienthello_tlsext(SSL *ssl, CBB *out, size_t header_len); -uint8_t *ssl_add_serverhello_tlsext(SSL *s, uint8_t *const buf, - uint8_t *const limit); +int ssl_add_serverhello_tlsext(SSL *ssl, CBB *out); int ssl_parse_clienthello_tlsext(SSL *s, CBS *cbs); int ssl_parse_serverhello_tlsext(SSL *s, CBS *cbs);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 935edae..25ab802 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c
@@ -944,8 +944,15 @@ session = NULL; s->verify_result = s->session->verify_result; - } else if (!ssl_get_new_session(s, 1)) { - goto err; + } else { + if (!ssl_get_new_session(s, 1)) { + goto err; + } + + /* Clear the session ID if we want the session to be single-use. */ + if (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)) { + s->session->session_id_length = 0; + } } if (s->ctx->dos_protection_cb != NULL && s->ctx->dos_protection_cb(&early_ctx) == 0) { @@ -1106,90 +1113,55 @@ return ret; } -int ssl3_send_server_hello(SSL *s) { - uint8_t *buf; - uint8_t *p, *d; - int sl; - unsigned long l; - - if (s->state == SSL3_ST_SW_SRVR_HELLO_A) { - /* We only accept ChannelIDs on connections with ECDHE in order to avoid a - * known attack while we fix ChannelID itself. */ - if (s->s3->tlsext_channel_id_valid && - (s->s3->tmp.new_cipher->algorithm_mkey & SSL_kECDHE) == 0) { - s->s3->tlsext_channel_id_valid = 0; - } - - /* If this is a resumption and the original handshake didn't support - * ChannelID then we didn't record the original handshake hashes in the - * session and so cannot resume with ChannelIDs. */ - if (s->hit && s->session->original_handshake_hash_len == 0) { - s->s3->tlsext_channel_id_valid = 0; - } - - buf = (uint8_t *)s->init_buf->data; - /* Do the message type and length last */ - d = p = ssl_handshake_start(s); - - *(p++) = s->version >> 8; - *(p++) = s->version & 0xff; - - /* Random stuff */ - if (!ssl_fill_hello_random(s->s3->server_random, SSL3_RANDOM_SIZE, - 1 /* server */)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return -1; - } - memcpy(p, s->s3->server_random, SSL3_RANDOM_SIZE); - p += SSL3_RANDOM_SIZE; - - /* There are several cases for the session ID to send - * back in the server hello: - * - For session reuse from the session cache, we send back the old session - * ID. - * - If stateless session reuse (using a session ticket) is successful, we - * send back the client's "session ID" (which doesn't actually identify - * the session). - * - If it is a new session, we send back the new session ID. - * - However, if we want the new session to be single-use, we send back a - * 0-length session ID. - * s->hit is non-zero in either case of session reuse, so the following - * won't overwrite an ID that we're supposed to send back. */ - if (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER) && !s->hit) { - s->session->session_id_length = 0; - } - - sl = s->session->session_id_length; - if (sl > (int)sizeof(s->session->session_id)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return -1; - } - *(p++) = sl; - memcpy(p, s->session->session_id, sl); - p += sl; - - /* put the cipher */ - s2n(ssl_cipher_get_value(s->s3->tmp.new_cipher), p); - - /* put the compression method */ - *(p++) = 0; - - p = ssl_add_serverhello_tlsext(s, p, buf + SSL3_RT_MAX_PLAIN_LENGTH); - if (p == NULL) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return -1; - } - - /* do the header */ - l = (p - d); - if (!ssl_set_handshake_header(s, SSL3_MT_SERVER_HELLO, l)) { - return -1; - } - s->state = SSL3_ST_SW_SRVR_HELLO_B; +int ssl3_send_server_hello(SSL *ssl) { + if (ssl->state == SSL3_ST_SW_SRVR_HELLO_B) { + return ssl_do_write(ssl); } - /* SSL3_ST_SW_SRVR_HELLO_B */ - return ssl_do_write(s); + assert(ssl->state == SSL3_ST_SW_SRVR_HELLO_A); + + /* We only accept ChannelIDs on connections with ECDHE in order to avoid a + * known attack while we fix ChannelID itself. */ + if (ssl->s3->tlsext_channel_id_valid && + (ssl->s3->tmp.new_cipher->algorithm_mkey & SSL_kECDHE) == 0) { + ssl->s3->tlsext_channel_id_valid = 0; + } + + /* If this is a resumption and the original handshake didn't support + * ChannelID then we didn't record the original handshake hashes in the + * session and so cannot resume with ChannelIDs. */ + if (ssl->hit && ssl->session->original_handshake_hash_len == 0) { + ssl->s3->tlsext_channel_id_valid = 0; + } + + if (!ssl_fill_hello_random(ssl->s3->server_random, SSL3_RANDOM_SIZE, + 1 /* server */)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return -1; + } + + CBB cbb, session_id; + size_t length; + CBB_zero(&cbb); + if (!CBB_init_fixed(&cbb, ssl_handshake_start(ssl), + ssl->init_buf->max - SSL_HM_HEADER_LENGTH(ssl)) || + !CBB_add_u16(&cbb, ssl->version) || + !CBB_add_bytes(&cbb, ssl->s3->server_random, SSL3_RANDOM_SIZE) || + !CBB_add_u8_length_prefixed(&cbb, &session_id) || + !CBB_add_bytes(&session_id, ssl->session->session_id, + ssl->session->session_id_length) || + !CBB_add_u16(&cbb, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) || + !CBB_add_u8(&cbb, 0 /* no compression */) || + !ssl_add_serverhello_tlsext(ssl, &cbb) || + !CBB_finish(&cbb, NULL, &length) || + !ssl_set_handshake_header(ssl, SSL3_MT_SERVER_HELLO, length)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + CBB_cleanup(&cbb); + return -1; + } + + ssl->state = SSL3_ST_SW_SRVR_HELLO_B; + return ssl_do_write(ssl); } int ssl3_send_certificate_status(SSL *ssl) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index b521d06..c133e51 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c
@@ -2311,53 +2311,44 @@ return 0; } -uint8_t *ssl_add_serverhello_tlsext(SSL *s, uint8_t *const buf, - uint8_t *const limit) { - CBB cbb, extensions; - CBB_zero(&cbb); - if (!CBB_init_fixed(&cbb, buf, limit - buf) || - !CBB_add_u16_length_prefixed(&cbb, &extensions)) { +int ssl_add_serverhello_tlsext(SSL *ssl, CBB *out) { + const size_t orig_len = CBB_len(out); + + CBB extensions; + if (!CBB_add_u16_length_prefixed(out, &extensions)) { goto err; } unsigned i; for (i = 0; i < kNumExtensions; i++) { - if (!(s->s3->tmp.extensions.received & (1u << i))) { + if (!(ssl->s3->tmp.extensions.received & (1u << i))) { /* Don't send extensions that were not received. */ continue; } - if (!kExtensions[i].add_serverhello(s, &extensions)) { + if (!kExtensions[i].add_serverhello(ssl, &extensions)) { OPENSSL_PUT_ERROR(SSL, SSL_R_ERROR_ADDING_EXTENSION); ERR_add_error_dataf("extension: %u", (unsigned)kExtensions[i].value); goto err; } } - if (!custom_ext_add_serverhello(s, &extensions)) { + if (!custom_ext_add_serverhello(ssl, &extensions)) { goto err; } - if (!CBB_flush(&cbb)) { - goto err; - } - - uint8_t *ret = buf; - const size_t cbb_len = CBB_len(&cbb); /* If only two bytes have been written then the extensions are actually empty * and those two bytes are the zero length. In that case, we don't bother * sending the extensions length. */ - if (cbb_len > 2) { - ret += cbb_len; + if (CBB_len(&extensions) - orig_len == 2) { + CBB_discard_child(out); } - CBB_cleanup(&cbb); - return ret; + return CBB_flush(out); err: - CBB_cleanup(&cbb); OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return NULL; + return 0; } static int ssl_scan_clienthello_tlsext(SSL *s, CBS *cbs, int *out_alert) {