From 6065f753ed0b0a4f4b255c5caab79530daca354e Mon Sep 17 00:00:00 2001 From: jsing Date: Wed, 21 Jul 2021 08:42:14 +0000 Subject: [PATCH] Remove DTLS processed_rcds queue. When DTLS handshake records are received from the next epoch, we will potentially queue them on the unprocessed_rcds queue - this is usually a Finished message that has been received without the ChangeCipherSuite (CCS) message (which may have been dropped or reordered). After the epoch increments (due to the CCS being received), the current code processes all records on the unprocessed queue and immediate queues them on the processed queue, which dtls1_get_record() then pulls from. This form of processing only adds more complexity and another queue. Instead, once the epoch increments, pull a single record from the unprocessed queue and process it, allowing the contents to be consumed by the caller. We repeat this process until the unprocessed queue is empty, at which point we go back to consuming messages from the wire. ok inoguchi@ tb@ --- lib/libssl/d1_lib.c | 10 +------- lib/libssl/d1_pkt.c | 57 ++++++++++++++---------------------------- lib/libssl/dtls_locl.h | 5 ++-- 3 files changed, 22 insertions(+), 50 deletions(-) diff --git a/lib/libssl/d1_lib.c b/lib/libssl/d1_lib.c index 6d9959ff43a..3db5629e234 100644 --- a/lib/libssl/d1_lib.c +++ b/lib/libssl/d1_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_lib.c,v 1.57 2021/07/01 17:53:39 jsing Exp $ */ +/* $OpenBSD: d1_lib.c,v 1.58 2021/07/21 08:42:14 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -88,8 +88,6 @@ dtls1_new(SSL *s) if ((s->d1->internal->unprocessed_rcds.q = pqueue_new()) == NULL) goto err; - if ((s->d1->internal->processed_rcds.q = pqueue_new()) == NULL) - goto err; if ((s->d1->internal->buffered_messages = pqueue_new()) == NULL) goto err; if ((s->d1->sent_messages = pqueue_new()) == NULL) @@ -143,7 +141,6 @@ static void dtls1_clear_queues(SSL *s) { dtls1_drain_records(D1I(s)->unprocessed_rcds.q); - dtls1_drain_records(D1I(s)->processed_rcds.q); dtls1_drain_fragments(D1I(s)->buffered_messages); dtls1_drain_fragments(s->d1->sent_messages); dtls1_drain_records(D1I(s)->buffered_app_data.q); @@ -160,7 +157,6 @@ dtls1_free(SSL *s) dtls1_clear_queues(s); pqueue_free(D1I(s)->unprocessed_rcds.q); - pqueue_free(D1I(s)->processed_rcds.q); pqueue_free(D1I(s)->buffered_messages); pqueue_free(s->d1->sent_messages); pqueue_free(D1I(s)->buffered_app_data.q); @@ -176,7 +172,6 @@ dtls1_clear(SSL *s) { struct dtls1_state_internal_st *internal; pqueue unprocessed_rcds; - pqueue processed_rcds; pqueue buffered_messages; pqueue sent_messages; pqueue buffered_app_data; @@ -184,7 +179,6 @@ dtls1_clear(SSL *s) if (s->d1) { unprocessed_rcds = D1I(s)->unprocessed_rcds.q; - processed_rcds = D1I(s)->processed_rcds.q; buffered_messages = D1I(s)->buffered_messages; sent_messages = s->d1->sent_messages; buffered_app_data = D1I(s)->buffered_app_data.q; @@ -200,7 +194,6 @@ dtls1_clear(SSL *s) D1I(s)->r_epoch = tls12_record_layer_initial_epoch(s->internal->rl); - D1I(s)->processed_rcds.epoch = D1I(s)->r_epoch; D1I(s)->unprocessed_rcds.epoch = D1I(s)->r_epoch + 1; if (s->server) { @@ -212,7 +205,6 @@ dtls1_clear(SSL *s) } D1I(s)->unprocessed_rcds.q = unprocessed_rcds; - D1I(s)->processed_rcds.q = processed_rcds; D1I(s)->buffered_messages = buffered_messages; s->d1->sent_messages = sent_messages; D1I(s)->buffered_app_data.q = buffered_app_data; diff --git a/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c index 4e773a42bb7..0416ee9c593 100644 --- a/lib/libssl/d1_pkt.c +++ b/lib/libssl/d1_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_pkt.c,v 1.102 2021/07/21 07:51:12 jsing Exp $ */ +/* $OpenBSD: d1_pkt.c,v 1.103 2021/07/21 08:42:14 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -274,34 +274,23 @@ dtls1_retrieve_buffered_record(SSL *s, record_pqueue *queue) } static int -dtls1_process_buffered_records(SSL *s) +dtls1_process_buffered_record(SSL *s) { - pitem *item; + /* Check if epoch is current. */ + if (D1I(s)->unprocessed_rcds.epoch != D1I(s)->r_epoch) + return (0); - item = pqueue_peek(D1I(s)->unprocessed_rcds.q); - if (item) { - /* Check if epoch is current. */ - if (D1I(s)->unprocessed_rcds.epoch != D1I(s)->r_epoch) - return (1); - /* Nothing to do. */ - - /* Process all the records. */ - while (pqueue_peek(D1I(s)->unprocessed_rcds.q)) { - if (!dtls1_retrieve_buffered_record((s), - &((D1I(s))->unprocessed_rcds))) - return (0); - if (!dtls1_process_record(s)) - return (0); - if (dtls1_buffer_record(s, &(D1I(s)->processed_rcds), - S3I(s)->rrec.seq_num) < 0) - return (-1); - } + /* Update epoch once all unprocessed records have been processed. */ + if (pqueue_peek(D1I(s)->unprocessed_rcds.q) == NULL) { + D1I(s)->unprocessed_rcds.epoch = D1I(s)->r_epoch + 1; + return (0); } - /* sync epoch numbers once all the unprocessed records - * have been processed */ - D1I(s)->processed_rcds.epoch = D1I(s)->r_epoch; - D1I(s)->unprocessed_rcds.epoch = D1I(s)->r_epoch + 1; + /* Process one of the records. */ + if (!dtls1_retrieve_buffered_record(s, &D1I(s)->unprocessed_rcds)) + return (-1); + if (!dtls1_process_record(s)) + return (-1); return (1); } @@ -365,22 +354,15 @@ dtls1_process_record(SSL *s) int dtls1_get_record(SSL *s) { - SSL3_RECORD_INTERNAL *rr; + SSL3_RECORD_INTERNAL *rr = &(S3I(s)->rrec); unsigned char *p = NULL; DTLS1_BITMAP *bitmap; unsigned int is_next_epoch; - int n; + int ret, n; - rr = &(S3I(s)->rrec); - - /* The epoch may have changed. If so, process all the - * pending records. This is a non-blocking operation. */ - if (dtls1_process_buffered_records(s) < 0) - return (-1); - - /* if we're renegotiating, then there may be buffered records */ - if (dtls1_retrieve_buffered_record((s), &((D1I(s))->processed_rcds))) - return 1; + /* See if there are pending records that can now be processed. */ + if ((ret = dtls1_process_buffered_record(s)) != 0) + return (ret); /* get something from the wire */ if (0) { @@ -1189,7 +1171,6 @@ dtls1_dispatch_alert(SSL *s) return (i); } - static DTLS1_BITMAP * dtls1_get_bitmap(SSL *s, SSL3_RECORD_INTERNAL *rr, unsigned int *is_next_epoch) { diff --git a/lib/libssl/dtls_locl.h b/lib/libssl/dtls_locl.h index 9e0699d0986..bc28ce85599 100644 --- a/lib/libssl/dtls_locl.h +++ b/lib/libssl/dtls_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: dtls_locl.h,v 1.2 2021/07/19 08:42:24 jsing Exp $ */ +/* $OpenBSD: dtls_locl.h,v 1.3 2021/07/21 08:42:14 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -151,9 +151,8 @@ typedef struct dtls1_state_internal_st { unsigned short handshake_read_seq; - /* Received handshake records (processed and unprocessed) */ + /* Received handshake records (unprocessed) */ record_pqueue unprocessed_rcds; - record_pqueue processed_rcds; /* Buffered handshake messages */ struct _pqueue *buffered_messages; -- 2.20.1