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) {
