Skip to content

Commit 6065f75

Browse files
committed
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@
1 parent 90aa0eb commit 6065f75

File tree

3 files changed

+22
-50
lines changed

3 files changed

+22
-50
lines changed

lib/libssl/d1_lib.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: d1_lib.c,v 1.57 2021/07/01 17:53:39 jsing Exp $ */
1+
/* $OpenBSD: d1_lib.c,v 1.58 2021/07/21 08:42:14 jsing Exp $ */
22
/*
33
* DTLS implementation written by Nagendra Modadugu
44
* (nagendra@cs.stanford.edu) for the OpenSSL project 2005.
@@ -88,8 +88,6 @@ dtls1_new(SSL *s)
8888

8989
if ((s->d1->internal->unprocessed_rcds.q = pqueue_new()) == NULL)
9090
goto err;
91-
if ((s->d1->internal->processed_rcds.q = pqueue_new()) == NULL)
92-
goto err;
9391
if ((s->d1->internal->buffered_messages = pqueue_new()) == NULL)
9492
goto err;
9593
if ((s->d1->sent_messages = pqueue_new()) == NULL)
@@ -143,7 +141,6 @@ static void
143141
dtls1_clear_queues(SSL *s)
144142
{
145143
dtls1_drain_records(D1I(s)->unprocessed_rcds.q);
146-
dtls1_drain_records(D1I(s)->processed_rcds.q);
147144
dtls1_drain_fragments(D1I(s)->buffered_messages);
148145
dtls1_drain_fragments(s->d1->sent_messages);
149146
dtls1_drain_records(D1I(s)->buffered_app_data.q);
@@ -160,7 +157,6 @@ dtls1_free(SSL *s)
160157
dtls1_clear_queues(s);
161158

162159
pqueue_free(D1I(s)->unprocessed_rcds.q);
163-
pqueue_free(D1I(s)->processed_rcds.q);
164160
pqueue_free(D1I(s)->buffered_messages);
165161
pqueue_free(s->d1->sent_messages);
166162
pqueue_free(D1I(s)->buffered_app_data.q);
@@ -176,15 +172,13 @@ dtls1_clear(SSL *s)
176172
{
177173
struct dtls1_state_internal_st *internal;
178174
pqueue unprocessed_rcds;
179-
pqueue processed_rcds;
180175
pqueue buffered_messages;
181176
pqueue sent_messages;
182177
pqueue buffered_app_data;
183178
unsigned int mtu;
184179

185180
if (s->d1) {
186181
unprocessed_rcds = D1I(s)->unprocessed_rcds.q;
187-
processed_rcds = D1I(s)->processed_rcds.q;
188182
buffered_messages = D1I(s)->buffered_messages;
189183
sent_messages = s->d1->sent_messages;
190184
buffered_app_data = D1I(s)->buffered_app_data.q;
@@ -200,7 +194,6 @@ dtls1_clear(SSL *s)
200194
D1I(s)->r_epoch =
201195
tls12_record_layer_initial_epoch(s->internal->rl);
202196

203-
D1I(s)->processed_rcds.epoch = D1I(s)->r_epoch;
204197
D1I(s)->unprocessed_rcds.epoch = D1I(s)->r_epoch + 1;
205198

206199
if (s->server) {
@@ -212,7 +205,6 @@ dtls1_clear(SSL *s)
212205
}
213206

214207
D1I(s)->unprocessed_rcds.q = unprocessed_rcds;
215-
D1I(s)->processed_rcds.q = processed_rcds;
216208
D1I(s)->buffered_messages = buffered_messages;
217209
s->d1->sent_messages = sent_messages;
218210
D1I(s)->buffered_app_data.q = buffered_app_data;

lib/libssl/d1_pkt.c

Lines changed: 19 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: d1_pkt.c,v 1.102 2021/07/21 07:51:12 jsing Exp $ */
1+
/* $OpenBSD: d1_pkt.c,v 1.103 2021/07/21 08:42:14 jsing Exp $ */
22
/*
33
* DTLS implementation written by Nagendra Modadugu
44
* (nagendra@cs.stanford.edu) for the OpenSSL project 2005.
@@ -274,34 +274,23 @@ dtls1_retrieve_buffered_record(SSL *s, record_pqueue *queue)
274274
}
275275

276276
static int
277-
dtls1_process_buffered_records(SSL *s)
277+
dtls1_process_buffered_record(SSL *s)
278278
{
279-
pitem *item;
279+
/* Check if epoch is current. */
280+
if (D1I(s)->unprocessed_rcds.epoch != D1I(s)->r_epoch)
281+
return (0);
280282

281-
item = pqueue_peek(D1I(s)->unprocessed_rcds.q);
282-
if (item) {
283-
/* Check if epoch is current. */
284-
if (D1I(s)->unprocessed_rcds.epoch != D1I(s)->r_epoch)
285-
return (1);
286-
/* Nothing to do. */
287-
288-
/* Process all the records. */
289-
while (pqueue_peek(D1I(s)->unprocessed_rcds.q)) {
290-
if (!dtls1_retrieve_buffered_record((s),
291-
&((D1I(s))->unprocessed_rcds)))
292-
return (0);
293-
if (!dtls1_process_record(s))
294-
return (0);
295-
if (dtls1_buffer_record(s, &(D1I(s)->processed_rcds),
296-
S3I(s)->rrec.seq_num) < 0)
297-
return (-1);
298-
}
283+
/* Update epoch once all unprocessed records have been processed. */
284+
if (pqueue_peek(D1I(s)->unprocessed_rcds.q) == NULL) {
285+
D1I(s)->unprocessed_rcds.epoch = D1I(s)->r_epoch + 1;
286+
return (0);
299287
}
300288

301-
/* sync epoch numbers once all the unprocessed records
302-
* have been processed */
303-
D1I(s)->processed_rcds.epoch = D1I(s)->r_epoch;
304-
D1I(s)->unprocessed_rcds.epoch = D1I(s)->r_epoch + 1;
289+
/* Process one of the records. */
290+
if (!dtls1_retrieve_buffered_record(s, &D1I(s)->unprocessed_rcds))
291+
return (-1);
292+
if (!dtls1_process_record(s))
293+
return (-1);
305294

306295
return (1);
307296
}
@@ -365,22 +354,15 @@ dtls1_process_record(SSL *s)
365354
int
366355
dtls1_get_record(SSL *s)
367356
{
368-
SSL3_RECORD_INTERNAL *rr;
357+
SSL3_RECORD_INTERNAL *rr = &(S3I(s)->rrec);
369358
unsigned char *p = NULL;
370359
DTLS1_BITMAP *bitmap;
371360
unsigned int is_next_epoch;
372-
int n;
361+
int ret, n;
373362

374-
rr = &(S3I(s)->rrec);
375-
376-
/* The epoch may have changed. If so, process all the
377-
* pending records. This is a non-blocking operation. */
378-
if (dtls1_process_buffered_records(s) < 0)
379-
return (-1);
380-
381-
/* if we're renegotiating, then there may be buffered records */
382-
if (dtls1_retrieve_buffered_record((s), &((D1I(s))->processed_rcds)))
383-
return 1;
363+
/* See if there are pending records that can now be processed. */
364+
if ((ret = dtls1_process_buffered_record(s)) != 0)
365+
return (ret);
384366

385367
/* get something from the wire */
386368
if (0) {
@@ -1189,7 +1171,6 @@ dtls1_dispatch_alert(SSL *s)
11891171
return (i);
11901172
}
11911173

1192-
11931174
static DTLS1_BITMAP *
11941175
dtls1_get_bitmap(SSL *s, SSL3_RECORD_INTERNAL *rr, unsigned int *is_next_epoch)
11951176
{

lib/libssl/dtls_locl.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: dtls_locl.h,v 1.2 2021/07/19 08:42:24 jsing Exp $ */
1+
/* $OpenBSD: dtls_locl.h,v 1.3 2021/07/21 08:42:14 jsing Exp $ */
22
/*
33
* DTLS implementation written by Nagendra Modadugu
44
* (nagendra@cs.stanford.edu) for the OpenSSL project 2005.
@@ -151,9 +151,8 @@ typedef struct dtls1_state_internal_st {
151151

152152
unsigned short handshake_read_seq;
153153

154-
/* Received handshake records (processed and unprocessed) */
154+
/* Received handshake records (unprocessed) */
155155
record_pqueue unprocessed_rcds;
156-
record_pqueue processed_rcds;
157156

158157
/* Buffered handshake messages */
159158
struct _pqueue *buffered_messages;

0 commit comments

Comments
 (0)