Fix DTLS unprocessed records bug

During a DTLS handshake we may get records destined for the next epoch
arrive before we have processed the CCS. In that case we can't decrypt or
verify the record yet, so we buffer it for later use. When we do receive
the CCS we work through the queue of unprocessed records and process them.

Unfortunately the act of processing wipes out any existing packet data
that we were still working through. This includes any records from the new
epoch that were in the same packet as the CCS. We should only process the
buffered records if we've not got any data left.

Reviewed-by: Richard Levitte <levitte@openssl.org>
This commit is contained in:
Matt Caswell 2016-07-05 09:51:08 +01:00
parent 6fc1748ec6
commit 738ad946dd
2 changed files with 21 additions and 2 deletions

View file

@ -229,6 +229,7 @@ int dtls1_retrieve_buffered_record(SSL *s, record_pqueue *queue)
int dtls1_process_buffered_records(SSL *s) int dtls1_process_buffered_records(SSL *s)
{ {
pitem *item; pitem *item;
SSL3_BUFFER *rb;
item = pqueue_peek(s->rlayer.d->unprocessed_rcds.q); item = pqueue_peek(s->rlayer.d->unprocessed_rcds.q);
if (item) { if (item) {
@ -236,6 +237,18 @@ int dtls1_process_buffered_records(SSL *s)
if (s->rlayer.d->unprocessed_rcds.epoch != s->rlayer.d->r_epoch) if (s->rlayer.d->unprocessed_rcds.epoch != s->rlayer.d->r_epoch)
return (1); /* Nothing to do. */ return (1); /* Nothing to do. */
rb = RECORD_LAYER_get_rbuf(&s->rlayer);
if (SSL3_BUFFER_get_left(rb) > 0) {
/*
* We've still got data from the current packet to read. There could
* be a record from the new epoch in it - so don't overwrite it
* with the unprocessed records yet (we'll do it when we've
* finished reading the current packet).
*/
return 1;
}
/* Process all the records. */ /* Process all the records. */
while (pqueue_peek(s->rlayer.d->unprocessed_rcds.q)) { while (pqueue_peek(s->rlayer.d->unprocessed_rcds.q)) {
dtls1_get_unprocessed_record(s); dtls1_get_unprocessed_record(s);
@ -1115,8 +1128,13 @@ DTLS1_BITMAP *dtls1_get_bitmap(SSL *s, SSL3_RECORD *rr,
if (rr->epoch == s->rlayer.d->r_epoch) if (rr->epoch == s->rlayer.d->r_epoch)
return &s->rlayer.d->bitmap; return &s->rlayer.d->bitmap;
/* Only HM and ALERT messages can be from the next epoch */ /*
* Only HM and ALERT messages can be from the next epoch and only if we
* have already processed all of the unprocessed records from the last
* epoch
*/
else if (rr->epoch == (unsigned long)(s->rlayer.d->r_epoch + 1) && else if (rr->epoch == (unsigned long)(s->rlayer.d->r_epoch + 1) &&
s->rlayer.d->unprocessed_rcds.epoch != s->rlayer.d->r_epoch &&
(rr->type == SSL3_RT_HANDSHAKE || rr->type == SSL3_RT_ALERT)) { (rr->type == SSL3_RT_HANDSHAKE || rr->type == SSL3_RT_ALERT)) {
*is_next_epoch = 1; *is_next_epoch = 1;
return &s->rlayer.d->next_bitmap; return &s->rlayer.d->next_bitmap;

View file

@ -1467,6 +1467,7 @@ int dtls1_get_record(SSL *s)
rr = RECORD_LAYER_get_rrec(&s->rlayer); rr = RECORD_LAYER_get_rrec(&s->rlayer);
again:
/* /*
* The epoch may have changed. If so, process all the pending records. * The epoch may have changed. If so, process all the pending records.
* This is a non-blocking operation. * This is a non-blocking operation.
@ -1479,7 +1480,7 @@ int dtls1_get_record(SSL *s)
return 1; return 1;
/* get something from the wire */ /* get something from the wire */
again:
/* check if we have the header */ /* check if we have the header */
if ((RECORD_LAYER_get_rstate(&s->rlayer) != SSL_ST_READ_BODY) || if ((RECORD_LAYER_get_rstate(&s->rlayer) != SSL_ST_READ_BODY) ||
(RECORD_LAYER_get_packet_length(&s->rlayer) < DTLS1_RT_HEADER_LENGTH)) { (RECORD_LAYER_get_packet_length(&s->rlayer) < DTLS1_RT_HEADER_LENGTH)) {