Splitting finish_message to finish_message/queue_message.
This is to allow for PSK binders to be munged into the ClientHello as part of
draft 18.
BUG=112
Change-Id: Ic4fd3b70fa45669389b6aaf55e61d5839f296748
Reviewed-on: https://boringssl-review.googlesource.com/12228
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/d1_both.c b/ssl/d1_both.c
index eae657c..82ee58c 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -736,21 +736,22 @@
return 1;
}
-int dtls1_finish_message(SSL *ssl, CBB *cbb) {
- uint8_t *msg = NULL;
- size_t len;
- if (!CBB_finish(cbb, &msg, &len) ||
- len > 0xffffffffu ||
- len < DTLS1_HM_HEADER_LENGTH) {
+int dtls1_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg,
+ size_t *out_len) {
+ if (!CBB_finish(cbb, out_msg, out_len) ||
+ *out_len < DTLS1_HM_HEADER_LENGTH) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- OPENSSL_free(msg);
+ OPENSSL_free(*out_msg);
return 0;
}
/* Fix up the header. Copy the fragment length into the total message
* length. */
- memcpy(msg + 1, msg + DTLS1_HM_HEADER_LENGTH - 3, 3);
+ memcpy(*out_msg + 1, *out_msg + DTLS1_HM_HEADER_LENGTH - 3, 3);
+ return 1;
+}
+int dtls1_queue_message(SSL *ssl, uint8_t *msg, size_t len) {
ssl3_update_handshake_hash(ssl, msg, len);
ssl->d1->handshake_write_seq++;
diff --git a/ssl/dtls_method.c b/ssl/dtls_method.c
index 9d791b5..8026579 100644
--- a/ssl/dtls_method.c
+++ b/ssl/dtls_method.c
@@ -142,6 +142,7 @@
dtls1_supports_cipher,
dtls1_init_message,
dtls1_finish_message,
+ dtls1_queue_message,
dtls1_write_message,
dtls1_send_change_cipher_spec,
dtls1_expect_flight,
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 64d3941..3583be4 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -764,7 +764,7 @@
CBB body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO) ||
!ssl_add_client_hello_body(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -1676,7 +1676,7 @@
/* The message must be added to the finished hash before calculating the
* master secret. */
- if (!ssl->method->finish_message(ssl, &cbb)) {
+ if (!ssl_complete_message(ssl, &cbb)) {
goto err;
}
ssl->state = SSL3_ST_CW_KEY_EXCH_B;
@@ -1793,7 +1793,7 @@
}
if (!CBB_did_write(&child, sig_len) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -1822,7 +1822,7 @@
ssl->s3->next_proto_negotiated_len) ||
!CBB_add_u8_length_prefixed(&body, &child) ||
!CBB_add_bytes(&child, kZero, padding_len) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return -1;
@@ -1851,7 +1851,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) ||
!tls1_write_channel_id(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return -1;
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index f0469d4..e9da90e 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -964,7 +964,7 @@
!CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) ||
!CBB_add_u8(&body, 0 /* no compression */) ||
!ssl_add_serverhello_tlsext(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return -1;
@@ -1003,7 +1003,7 @@
!CBB_add_u24_length_prefixed(&body, &ocsp_response) ||
!CBB_add_bytes(&ocsp_response, ssl->ctx->ocsp_response,
ssl->ctx->ocsp_response_length) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return -1;
@@ -1184,7 +1184,7 @@
}
}
- if (!ssl->method->finish_message(ssl, &cbb)) {
+ if (!ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -1266,7 +1266,7 @@
}
if (!ssl_add_client_CA_list(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -1286,7 +1286,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO_DONE) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return -1;
@@ -1849,7 +1849,7 @@
CBB_add_u32(&body, session->timeout) &&
CBB_add_u16_length_prefixed(&body, &ticket) &&
ssl_encrypt_ticket(ssl, &ticket, session) &&
- ssl->method->finish_message(ssl, &cbb);
+ ssl_complete_message(ssl, &cbb);
SSL_SESSION_free(session_copy);
CBB_cleanup(&cbb);
diff --git a/ssl/internal.h b/ssl/internal.h
index 6b57971..64cc597 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1268,9 +1268,15 @@
* root CBB to be passed into |finish_message|. |*body| is set to a child CBB
* the caller should write to. It returns one on success and zero on error. */
int (*init_message)(SSL *ssl, CBB *cbb, CBB *body, uint8_t type);
- /* finish_message finishes a handshake message and prepares it to be
- * written. It returns one on success and zero on error. */
- int (*finish_message)(SSL *ssl, CBB *cbb);
+ /* finish_message finishes a handshake message. It sets |*out_msg| to a
+ * newly-allocated buffer with the serialized message. The caller must
+ * release it with |OPENSSL_free| when done. It returns one on success and
+ * zero on error. */
+ int (*finish_message)(SSL *ssl, CBB *cbb, uint8_t **out_msg, size_t *out_len);
+ /* queue_message queues a handshake message and prepares it to be written. It
+ * takes ownership of |msg| and releases it with |OPENSSL_free| when done. It
+ * returns one on success and zero on error. */
+ int (*queue_message)(SSL *ssl, uint8_t *msg, size_t len);
/* write_message writes the next message to the transport. It returns one on
* success and <= 0 on error. */
int (*write_message)(SSL *ssl);
@@ -1720,16 +1726,23 @@
int ssl3_connect(SSL *ssl);
int ssl3_init_message(SSL *ssl, CBB *cbb, CBB *body, uint8_t type);
-int ssl3_finish_message(SSL *ssl, CBB *cbb);
+int ssl3_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg, size_t *out_len);
+int ssl3_queue_message(SSL *ssl, uint8_t *msg, size_t len);
int ssl3_write_message(SSL *ssl);
void ssl3_expect_flight(SSL *ssl);
void ssl3_received_flight(SSL *ssl);
int dtls1_init_message(SSL *ssl, CBB *cbb, CBB *body, uint8_t type);
-int dtls1_finish_message(SSL *ssl, CBB *cbb);
+int dtls1_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg,
+ size_t *out_len);
+int dtls1_queue_message(SSL *ssl, uint8_t *msg, size_t len);
int dtls1_write_message(SSL *ssl);
+/* ssl_complete_message calls |finish_message| and |queue_message| on |cbb| to
+ * queue the message for writing. */
+int ssl_complete_message(SSL *ssl, CBB *cbb);
+
/* dtls1_get_record reads a new input record. On success, it places it in
* |ssl->s3->rrec| and returns one. Otherwise it returns <= 0 on error or if
* more data is needed. */
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 11aeed0..4cb37bc 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -205,18 +205,21 @@
return 1;
}
-int ssl3_finish_message(SSL *ssl, CBB *cbb) {
- if (ssl->s3->pending_message != NULL) {
+int ssl3_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg,
+ size_t *out_len) {
+ if (!CBB_finish(cbb, out_msg, out_len)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ OPENSSL_free(*out_msg);
return 0;
}
+ return 1;
+}
- uint8_t *msg = NULL;
- size_t len;
- if (!CBB_finish(cbb, &msg, &len) ||
+int ssl3_queue_message(SSL *ssl, uint8_t *msg, size_t len) {
+ if (ssl->s3->pending_message != NULL ||
len > 0xffffffffu) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
OPENSSL_free(msg);
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return 0;
}
@@ -227,6 +230,17 @@
return 1;
}
+int ssl_complete_message(SSL *ssl, CBB *cbb) {
+ uint8_t *msg = NULL;
+ size_t len;
+ if (!ssl->method->finish_message(ssl, cbb, &msg, &len) ||
+ !ssl->method->queue_message(ssl, msg, len)) {
+ return 0;
+ }
+
+ return 1;
+}
+
int ssl3_write_message(SSL *ssl) {
if (ssl->s3->pending_message == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
@@ -284,7 +298,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_FINISHED) ||
!CBB_add_bytes(&body, finished, finished_len) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return -1;
@@ -352,7 +366,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CERTIFICATE) ||
!ssl_add_cert_chain(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
CBB_cleanup(&cbb);
return 0;
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 75be849..56ca9fc 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -329,7 +329,7 @@
/* The request context is always empty in the handshake. */
!CBB_add_u8(&body, 0) ||
!ssl_add_cert_chain(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
CBB_cleanup(&cbb);
return 0;
}
@@ -387,7 +387,7 @@
}
if (!CBB_did_write(&child, sig_len) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -412,7 +412,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_FINISHED) ||
!CBB_add_bytes(&body, verify_data, verify_data_len) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
CBB_cleanup(&cbb);
return 0;
}
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index fd60bd3..4b0b56b 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -168,7 +168,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO) ||
!ssl_add_client_hello_body(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
CBB_cleanup(&cbb);
return ssl_hs_error;
}
@@ -602,7 +602,7 @@
CBB cbb, body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) ||
!tls1_write_channel_id(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
CBB_cleanup(&cbb);
return ssl_hs_error;
}
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 49f8c00..446dd1c 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -310,7 +310,7 @@
!CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, group_id) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
CBB_cleanup(&cbb);
return ssl_hs_error;
}
@@ -378,7 +378,7 @@
}
}
- if (!ssl->method->finish_message(ssl, &cbb)) {
+ if (!ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -400,7 +400,7 @@
if (!ssl->method->init_message(ssl, &cbb, &body,
SSL3_MT_ENCRYPTED_EXTENSIONS) ||
!ssl_add_serverhello_tlsext(ssl, &body) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
CBB_cleanup(&cbb);
return ssl_hs_error;
}
@@ -445,7 +445,7 @@
if (!ssl_add_client_CA_list(ssl, &body) ||
!CBB_add_u16(&body, 0 /* empty certificate_extensions. */) ||
- !ssl->method->finish_message(ssl, &cbb)) {
+ !ssl_complete_message(ssl, &cbb)) {
goto err;
}
@@ -637,7 +637,7 @@
goto err;
}
- if (!ssl->method->finish_message(ssl, &cbb)) {
+ if (!ssl_complete_message(ssl, &cbb)) {
goto err;
}
diff --git a/ssl/tls_method.c b/ssl/tls_method.c
index 8bcdf8f..0c7a952 100644
--- a/ssl/tls_method.c
+++ b/ssl/tls_method.c
@@ -140,6 +140,7 @@
ssl3_supports_cipher,
ssl3_init_message,
ssl3_finish_message,
+ ssl3_queue_message,
ssl3_write_message,
ssl3_send_change_cipher_spec,
ssl3_expect_flight,
