Switch finish_handshake to release_current_message.
With the previous DTLS change, the dispatch layer only cares about the
end of the handshake to know when to drop the current message. TLS 1.3
post-handshake messages will need a similar hook, so convert it to this
lower-level one.
BUG=83
Change-Id: I4c8c3ba55ba793afa065bf261a7bccac8816c348
Reviewed-on: https://boringssl-review.googlesource.com/8989
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@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 1d9cd75..712c63b 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -262,17 +262,6 @@
return frag != NULL && frag->reassembly == NULL;
}
-/* dtls1_pop_message removes the current handshake message, which must be
- * complete, and advances to the next one. */
-static void dtls1_pop_message(SSL *ssl) {
- assert(dtls1_is_current_message_complete(ssl));
-
- size_t index = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
- dtls1_hm_fragment_free(ssl->d1->incoming_messages[index]);
- ssl->d1->incoming_messages[index] = NULL;
- ssl->d1->handshake_read_seq++;
-}
-
/* dtls1_get_incoming_message returns the incoming message corresponding to
* |msg_hdr|. If none exists, it creates a new one and inserts it in the
* queue. Otherwise, it checks |msg_hdr| is consistent with the existing one. It
@@ -412,12 +401,12 @@
* ssl_dont_hash_message would have to have been applied to the previous
* call. */
assert(hash_message == ssl_hash_message);
- assert(dtls1_is_current_message_complete(ssl));
+ assert(ssl->init_msg != NULL);
ssl->s3->tmp.reuse_message = 0;
hash_message = ssl_dont_hash_message;
- } else if (dtls1_is_current_message_complete(ssl)) {
- dtls1_pop_message(ssl);
+ } else {
+ dtls1_release_current_message(ssl, 0 /* don't free buffer */);
}
/* Process handshake records until the current message is ready. */
@@ -463,6 +452,21 @@
DTLS1_HM_HEADER_LENGTH + frag->msg_len);
}
+void dtls1_release_current_message(SSL *ssl, int free_buffer) {
+ if (ssl->init_msg == NULL) {
+ return;
+ }
+
+ assert(dtls1_is_current_message_complete(ssl));
+ size_t index = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
+ dtls1_hm_fragment_free(ssl->d1->incoming_messages[index]);
+ ssl->d1->incoming_messages[index] = NULL;
+ ssl->d1->handshake_read_seq++;
+
+ ssl->init_msg = NULL;
+ ssl->init_num = 0;
+}
+
void dtls_clear_incoming_messages(SSL *ssl) {
for (size_t i = 0; i < SSL_MAX_HANDSHAKE_FLIGHT; i++) {
dtls1_hm_fragment_free(ssl->d1->incoming_messages[i]);
@@ -471,13 +475,14 @@
}
int dtls_has_incoming_messages(const SSL *ssl) {
- /* This function may not be called if there is a pending |dtls1_get_message|
- * operation. */
- assert(dtls1_is_current_message_complete(ssl));
-
size_t current = ssl->d1->handshake_read_seq % SSL_MAX_HANDSHAKE_FLIGHT;
for (size_t i = 0; i < SSL_MAX_HANDSHAKE_FLIGHT; i++) {
- if (i != current && ssl->d1->incoming_messages[i] != NULL) {
+ /* Skip the current message. */
+ if (ssl->init_msg != NULL && i == current) {
+ assert(dtls1_is_current_message_complete(ssl));
+ continue;
+ }
+ if (ssl->d1->incoming_messages[i] != NULL) {
return 1;
}
}
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index ceb6e17..ab9b911 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -224,7 +224,7 @@
}
if (msg_hdr.type == SSL3_MT_FINISHED &&
- msg_hdr.seq == ssl->d1->handshake_read_seq) {
+ msg_hdr.seq == ssl->d1->handshake_read_seq - 1) {
if (msg_hdr.frag_off == 0) {
/* Retransmit our last flight of messages. If the peer sends the second
* Finished, they may not have received ours. Only do this for the
diff --git a/ssl/dtls_method.c b/ssl/dtls_method.c
index 09bd455..e2e1726 100644
--- a/ssl/dtls_method.c
+++ b/ssl/dtls_method.c
@@ -92,12 +92,6 @@
return ~(version - 0x0201);
}
-static void dtls1_finish_handshake(SSL *ssl) {
- dtls_clear_incoming_messages(ssl);
- ssl->init_msg = NULL;
- ssl->init_num = 0;
-}
-
static int dtls1_set_read_state(SSL *ssl, SSL_AEAD_CTX *aead_ctx) {
/* Cipher changes are illegal when there are buffered incoming messages. */
if (dtls_has_incoming_messages(ssl)) {
@@ -135,9 +129,9 @@
dtls1_version_to_wire,
dtls1_new,
dtls1_free,
- dtls1_finish_handshake,
dtls1_get_message,
dtls1_hash_current_message,
+ dtls1_release_current_message,
dtls1_read_app_data,
dtls1_read_change_cipher_spec,
dtls1_read_close_notify,
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index fc4318e..5fe706c 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -502,10 +502,9 @@
break;
case SSL_ST_OK:
- /* clean a few things up */
+ /* Clean a few things up. */
ssl3_cleanup_key_block(ssl);
-
- ssl->method->finish_handshake(ssl);
+ ssl->method->release_current_message(ssl, 1 /* free_buffer */);
/* Remove write buffering now. */
ssl_free_wbio_buffer(ssl);
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 4708dd0..9f4aae3 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -471,10 +471,9 @@
break;
case SSL_ST_OK:
- /* clean a few things up */
+ /* Clean a few things up. */
ssl3_cleanup_key_block(ssl);
-
- ssl->method->finish_handshake(ssl);
+ ssl->method->release_current_message(ssl, 1 /* free_buffer */);
/* remove buffering on output */
ssl_free_wbio_buffer(ssl);
diff --git a/ssl/internal.h b/ssl/internal.h
index 2fb1492..1b3f28a 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1039,8 +1039,6 @@
uint16_t (*version_to_wire)(uint16_t version);
int (*ssl_new)(SSL *ssl);
void (*ssl_free)(SSL *ssl);
- /* finish_handshake is called when a handshake completes. */
- void (*finish_handshake)(SSL *ssl);
/* ssl_get_message reads the next handshake message. If |msg_type| is not -1,
* the message must have the specified type. On success, it returns one and
* sets |ssl->s3->tmp.message_type|, |ssl->init_msg|, and |ssl->init_num|.
@@ -1051,6 +1049,9 @@
* handshake hash. It returns one on success and zero on allocation
* failure. */
int (*hash_current_message)(SSL *ssl);
+ /* release_current_message is called to release the current handshake message.
+ * If |free_buffer| is one, buffers will also be released. */
+ void (*release_current_message)(SSL *ssl, int free_buffer);
int (*read_app_data)(SSL *ssl, uint8_t *buf, int len, int peek);
int (*read_change_cipher_spec)(SSL *ssl);
void (*read_close_notify)(SSL *ssl);
@@ -1242,6 +1243,7 @@
int ssl3_get_message(SSL *ssl, int msg_type,
enum ssl_hash_message_t hash_message);
int ssl3_hash_current_message(SSL *ssl);
+void ssl3_release_current_message(SSL *ssl, int free_buffer);
/* ssl3_cert_verify_hash writes the SSL 3.0 CertificateVerify hash into the
* bytes pointed to by |out| and writes the number of bytes to |*out_len|. |out|
@@ -1321,6 +1323,7 @@
int dtls1_get_message(SSL *ssl, int mt, enum ssl_hash_message_t hash_message);
int dtls1_hash_current_message(SSL *ssl);
+void dtls1_release_current_message(SSL *ssl, int free_buffer);
int dtls1_dispatch_alert(SSL *ssl);
/* ssl_is_wbio_buffered returns one if |ssl|'s write BIO is buffered and zero
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 11555dd..7608c34 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -550,14 +550,8 @@
ssl->s3->tmp.reuse_message = 0;
hash_message = ssl_dont_hash_message;
- } else if (ssl->init_msg != NULL) {
- /* |init_buf| never contains data beyond the current message. */
- assert(SSL3_HM_HEADER_LENGTH + ssl->init_num == ssl->init_buf->length);
-
- /* Clear the current message. */
- ssl->init_msg = NULL;
- ssl->init_num = 0;
- ssl->init_buf->length = 0;
+ } else {
+ ssl3_release_current_message(ssl, 0 /* don't free buffer */);
}
/* Read the message header, if we haven't yet. */
@@ -618,6 +612,23 @@
ssl->init_buf->length);
}
+void ssl3_release_current_message(SSL *ssl, int free_buffer) {
+ if (ssl->init_msg != NULL) {
+ /* |init_buf| never contains data beyond the current message. */
+ assert(SSL3_HM_HEADER_LENGTH + ssl->init_num == ssl->init_buf->length);
+
+ /* Clear the current message. */
+ ssl->init_msg = NULL;
+ ssl->init_num = 0;
+ ssl->init_buf->length = 0;
+ }
+
+ if (free_buffer) {
+ BUF_MEM_free(ssl->init_buf);
+ ssl->init_buf = NULL;
+ }
+}
+
int ssl_verify_alarm_type(long type) {
int al;
diff --git a/ssl/tls_method.c b/ssl/tls_method.c
index c11c452..923541b 100644
--- a/ssl/tls_method.c
+++ b/ssl/tls_method.c
@@ -69,14 +69,6 @@
static uint16_t ssl3_version_to_wire(uint16_t version) { return version; }
-static void ssl3_finish_handshake(SSL *ssl) {
- BUF_MEM_free(ssl->init_buf);
- ssl->init_buf = NULL;
-
- ssl->init_msg = NULL;
- ssl->init_num = 0;
-}
-
static int ssl3_set_read_state(SSL *ssl, SSL_AEAD_CTX *aead_ctx) {
if (ssl->s3->rrec.length != 0) {
/* There may not be unprocessed record data at a cipher change. */
@@ -109,9 +101,9 @@
ssl3_version_to_wire,
ssl3_new,
ssl3_free,
- ssl3_finish_handshake,
ssl3_get_message,
ssl3_hash_current_message,
+ ssl3_release_current_message,
ssl3_read_app_data,
ssl3_read_change_cipher_spec,
ssl3_read_close_notify,
