From 7b72427c6d4694085c8b140eaa741126e04466a4 Mon Sep 17 00:00:00 2001 From: Benjamin Kaduk Date: Mon, 31 Aug 2020 12:27:33 -0700 Subject: [PATCH 23/43] QUIC: Buffer all provided quic data Make all data supplied via SSL_provide_quic_data() pass through an internal buffer, so that we can handle data supplied with arbitrary framing and only parse complete TLS records onto the list of QUIC_DATA managed by quic_input_data_head/quic_input_data_tail. This lets us remove the concept of "incomplete" QUIC_DATA structures, and the 'offset' field needed to support them. However, we've already moved the provided data onto the buffer by the time we can check for KeyUpdate messages, so defer that check to quic_get_message() (where it is adjacent to the preexisting ChangeCipherSpec check). To avoid extra memory copies, we also make the QUIC_DATA structures just store offsets into the consolidated buffer instead of having copies of the TLS handshake messages themselves. --- ssl/ssl_lib.c | 1 + ssl/ssl_local.h | 5 +-- ssl/ssl_quic.c | 75 +++++++++++++++++++--------------------- ssl/statem/statem_quic.c | 11 ++++-- 4 files changed, 49 insertions(+), 43 deletions(-) --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1260,6 +1260,7 @@ void SSL_free(SSL *s) #ifndef OPENSSL_NO_QUIC OPENSSL_free(s->ext.quic_transport_params); OPENSSL_free(s->ext.peer_quic_transport_params); + BUF_MEM_free(s->quic_buf); while (s->quic_input_data_head != NULL) { QUIC_DATA *qd; --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -1225,9 +1225,8 @@ typedef struct cert_pkey_st CERT_PKEY; struct quic_data_st { struct quic_data_st *next; OSSL_ENCRYPTION_LEVEL level; - size_t offset; + size_t start; /* offset into quic_buf->data */ size_t length; - /* char data[]; should be here but C90 VLAs not allowed here */ }; typedef struct quic_data_st QUIC_DATA; int quic_set_encryption_secrets(SSL *ssl, OSSL_ENCRYPTION_LEVEL level); @@ -1721,8 +1720,10 @@ struct ssl_st { #ifndef OPENSSL_NO_QUIC OSSL_ENCRYPTION_LEVEL quic_read_level; OSSL_ENCRYPTION_LEVEL quic_write_level; + BUF_MEM *quic_buf; /* buffer incoming handshake messages */ QUIC_DATA *quic_input_data_head; QUIC_DATA *quic_input_data_tail; + size_t quic_next_record_start; const SSL_QUIC_METHOD *quic_method; #endif /* --- a/ssl/ssl_quic.c +++ b/ssl/ssl_quic.c @@ -91,8 +91,7 @@ OSSL_ENCRYPTION_LEVEL SSL_quic_write_lev int SSL_provide_quic_data(SSL *ssl, OSSL_ENCRYPTION_LEVEL level, const uint8_t *data, size_t len) { - size_t l; - uint8_t mt; + size_t l, offset; if (!SSL_IS_QUIC(ssl)) { SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); @@ -106,42 +105,46 @@ int SSL_provide_quic_data(SSL *ssl, OSSL return 0; } - /* Split on handshake message boundaries, if necessary */ - while (len > 0) { - QUIC_DATA *qd; - const uint8_t *p; - - /* Check for an incomplete block */ - qd = ssl->quic_input_data_tail; - if (qd != NULL) { - l = qd->length - qd->offset; - if (l != 0) { - /* we still need to copy `l` bytes into the last data block */ - if (l > len) - l = len; - memcpy((char*)(qd+1) + qd->offset, data, l); - qd->offset += l; - len -= l; - data += l; - continue; - } + if (ssl->quic_buf == NULL) { + BUF_MEM *buf; + if ((buf = BUF_MEM_new()) == NULL) { + SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, ERR_R_INTERNAL_ERROR); + return 0; } - - if (len < SSL3_HM_HEADER_LENGTH) { - SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, SSL_R_BAD_LENGTH); + if (!BUF_MEM_grow(buf, SSL3_RT_MAX_PLAIN_LENGTH)) { + SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, ERR_R_INTERNAL_ERROR); + BUF_MEM_free(buf); return 0; } + ssl->quic_buf = buf; + /* We preallocated storage, but there's still no *data*. */ + ssl->quic_buf->length = 0; + buf = NULL; + } + + offset = ssl->quic_buf->length; + if (!BUF_MEM_grow(ssl->quic_buf, offset + len)) { + SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, ERR_R_INTERNAL_ERROR); + return 0; + } + memcpy(ssl->quic_buf->data + offset, data, len); + + /* Split on handshake message boundaries */ + while (ssl->quic_buf->length > ssl->quic_next_record_start + + SSL3_HM_HEADER_LENGTH) { + QUIC_DATA *qd; + const uint8_t *p; + /* TLS Handshake message header has 1-byte type and 3-byte length */ - mt = *data; - p = data + 1; + p = (const uint8_t *)ssl->quic_buf->data + + ssl->quic_next_record_start + 1; n2l3(p, l); l += SSL3_HM_HEADER_LENGTH; - if (mt == SSL3_MT_KEY_UPDATE) { - SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, SSL_R_UNEXPECTED_MESSAGE); - return 0; - } + /* Don't allocate a QUIC_DATA if we don't have a full record */ + if (l > ssl->quic_buf->length - ssl->quic_next_record_start) + break; - qd = OPENSSL_zalloc(sizeof(QUIC_DATA) + l); + qd = OPENSSL_zalloc(sizeof(*qd)); if (qd == NULL) { SSLerr(SSL_F_SSL_PROVIDE_QUIC_DATA, ERR_R_INTERNAL_ERROR); return 0; @@ -149,21 +152,15 @@ int SSL_provide_quic_data(SSL *ssl, OSSL qd->next = NULL; qd->length = l; + qd->start = ssl->quic_next_record_start; qd->level = level; - /* partial data received? */ - if (l > len) - l = len; - qd->offset = l; - memcpy((void*)(qd + 1), data, l); if (ssl->quic_input_data_tail != NULL) ssl->quic_input_data_tail->next = qd; else ssl->quic_input_data_head = qd; ssl->quic_input_data_tail = qd; - - data += l; - len -= l; + ssl->quic_next_record_start += l; } return 1; --- a/ssl/statem/statem_quic.c +++ b/ssl/statem/statem_quic.c @@ -17,7 +17,7 @@ int quic_get_message(SSL *s, int *mt, si QUIC_DATA *qd = s->quic_input_data_head; uint8_t *p; - if (qd == NULL || (qd->length - qd->offset) != 0) { + if (qd == NULL) { s->rwstate = SSL_READING; *mt = *len = 0; return 0; @@ -43,7 +43,7 @@ int quic_get_message(SSL *s, int *mt, si } /* Copy buffered data */ - memcpy(s->init_buf->data, (void*)(qd + 1), qd->length); + memcpy(s->init_buf->data, s->quic_buf->data + qd->start, qd->length); s->init_buf->length = qd->length; s->quic_input_data_head = qd->next; if (s->quic_input_data_head == NULL) @@ -62,6 +62,13 @@ int quic_get_message(SSL *s, int *mt, si *len = 0; return 0; } + /* No KeyUpdate in QUIC */ + if (*mt == SSL3_MT_KEY_UPDATE) { + SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_R_UNEXPECTED_MESSAGE); + *len = 0; + return 0; + } + /* * If receiving Finished, record MAC of prior handshake messages for