Remove buffered_app_data as well.

This conceivably has a use, but NSS doesn't do this buffer either and it still
suffers from the same problems as the other uses of record_pqueue. This removes
the last use of record_pqueue. It also opens the door to removing pqueue
altogether as it isn't the right data structure for either of the remaining
uses either. (It's not clear it was right for record_pqueue either, but I don't
feel like digging into this code.)

Change-Id: If8a43e7332b3cd11a78a516f3e8ebf828052316f
Reviewed-on: https://boringssl-review.googlesource.com/4239
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 61a862d..fc59e33 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -98,19 +98,14 @@
 
   d1->buffered_messages = pqueue_new();
   d1->sent_messages = pqueue_new();
-  d1->buffered_app_data.q = pqueue_new();
 
-  if (!d1->buffered_messages || !d1->sent_messages ||
-      !d1->buffered_app_data.q) {
+  if (!d1->buffered_messages || !d1->sent_messages) {
     if (d1->buffered_messages) {
       pqueue_free(d1->buffered_messages);
     }
     if (d1->sent_messages) {
       pqueue_free(d1->sent_messages);
     }
-    if (d1->buffered_app_data.q) {
-      pqueue_free(d1->buffered_app_data.q);
-    }
     OPENSSL_free(d1);
     ssl3_free(s);
     return 0;
@@ -130,7 +125,6 @@
 static void dtls1_clear_queues(SSL *s) {
   pitem *item = NULL;
   hm_fragment *frag = NULL;
-  DTLS1_RECORD_DATA *rdata;
 
   while ((item = pqueue_pop(s->d1->buffered_messages)) != NULL) {
     frag = (hm_fragment *)item->data;
@@ -143,15 +137,6 @@
     dtls1_hm_fragment_free(frag);
     pitem_free(item);
   }
-
-  while ((item = pqueue_pop(s->d1->buffered_app_data.q)) != NULL) {
-    rdata = (DTLS1_RECORD_DATA *)item->data;
-    if (rdata->rbuf.buf) {
-      OPENSSL_free(rdata->rbuf.buf);
-    }
-    OPENSSL_free(item->data);
-    pitem_free(item);
-  }
 }
 
 void dtls1_free(SSL *s) {
@@ -165,7 +150,6 @@
 
   pqueue_free(s->d1->buffered_messages);
   pqueue_free(s->d1->sent_messages);
-  pqueue_free(s->d1->buffered_app_data.q);
 
   OPENSSL_free(s->d1);
   s->d1 = NULL;
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 56bdae0..7b2cab1 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -183,90 +183,10 @@
 
 static int dtls1_record_replay_check(SSL *s, DTLS1_BITMAP *bitmap);
 static void dtls1_record_bitmap_update(SSL *s, DTLS1_BITMAP *bitmap);
-static int dtls1_buffer_record(SSL *s, record_pqueue *q,
-                               uint8_t *priority);
 static int dtls1_process_record(SSL *s);
 static int do_dtls1_write(SSL *s, int type, const uint8_t *buf,
                           unsigned int len);
 
-/* copy buffered record into SSL structure */
-static int dtls1_copy_record(SSL *s, pitem *item) {
-  DTLS1_RECORD_DATA *rdata;
-
-  rdata = (DTLS1_RECORD_DATA *)item->data;
-
-  if (s->s3->rbuf.buf != NULL) {
-    OPENSSL_free(s->s3->rbuf.buf);
-  }
-
-  s->packet = rdata->packet;
-  s->packet_length = rdata->packet_length;
-  memcpy(&(s->s3->rbuf), &(rdata->rbuf), sizeof(SSL3_BUFFER));
-  memcpy(&(s->s3->rrec), &(rdata->rrec), sizeof(SSL3_RECORD));
-
-  /* Set proper sequence number for mac calculation */
-  memcpy(&(s->s3->read_sequence[2]), &(rdata->packet[5]), 6);
-
-  return 1;
-}
-
-static int dtls1_buffer_record(SSL *s, record_pqueue *queue,
-                               uint8_t *priority) {
-  DTLS1_RECORD_DATA *rdata;
-  pitem *item;
-
-  /* Limit the size of the queue to prevent DOS attacks */
-  if (pqueue_size(queue->q) >= 100) {
-    return 0;
-  }
-
-  rdata = OPENSSL_malloc(sizeof(DTLS1_RECORD_DATA));
-  item = pitem_new(priority, rdata);
-  if (rdata == NULL || item == NULL) {
-    if (rdata != NULL) {
-      OPENSSL_free(rdata);
-    }
-    if (item != NULL) {
-      pitem_free(item);
-    }
-
-    OPENSSL_PUT_ERROR(SSL, dtls1_buffer_record, ERR_R_INTERNAL_ERROR);
-    return -1;
-  }
-
-  rdata->packet = s->packet;
-  rdata->packet_length = s->packet_length;
-  memcpy(&(rdata->rbuf), &(s->s3->rbuf), sizeof(SSL3_BUFFER));
-  memcpy(&(rdata->rrec), &(s->s3->rrec), sizeof(SSL3_RECORD));
-
-  item->data = rdata;
-
-  s->packet = NULL;
-  s->packet_length = 0;
-  memset(&(s->s3->rbuf), 0, sizeof(SSL3_BUFFER));
-  memset(&(s->s3->rrec), 0, sizeof(SSL3_RECORD));
-
-  if (!ssl3_setup_buffers(s)) {
-    goto internal_error;
-  }
-
-  /* insert should not fail, since duplicates are dropped */
-  if (pqueue_insert(queue->q, item) == NULL) {
-    goto internal_error;
-  }
-
-  return 1;
-
-internal_error:
-  OPENSSL_PUT_ERROR(SSL, dtls1_buffer_record, ERR_R_INTERNAL_ERROR);
-  if (rdata->rbuf.buf != NULL) {
-    OPENSSL_free(rdata->rbuf.buf);
-  }
-  OPENSSL_free(rdata);
-  pitem_free(item);
-  return -1;
-}
-
 static int dtls1_process_record(SSL *s) {
   int al;
   SSL3_RECORD *rr;
@@ -540,21 +460,6 @@
    * s->s3->rrec.length   - number of bytes. */
   rr = &s->s3->rrec;
 
-  /* We are not handshaking and have no data yet,
-   * so process data buffered during the last handshake
-   * in advance, if any.
-   */
-  if (s->state == SSL_ST_OK && rr->length == 0) {
-    pitem *item;
-    item = pqueue_pop(s->d1->buffered_app_data.q);
-    if (item) {
-      dtls1_copy_record(s, item);
-
-      OPENSSL_free(item->data);
-      pitem_free(item);
-    }
-  }
-
   /* Check for timeout */
   if (dtls1_handle_timeout(s) > 0) {
     goto start;
@@ -581,12 +486,8 @@
   if (s->s3->change_cipher_spec && rr->type != SSL3_RT_HANDSHAKE &&
       rr->type != SSL3_RT_ALERT) {
     /* We now have an unexpected record between CCS and Finished. Most likely
-     * the packets were reordered on their way, so buffer the application data
-     * for later processing rather than dropping the connection. */
-    if (dtls1_buffer_record(s, &(s->d1->buffered_app_data), rr->seq_num) < 0) {
-      OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, ERR_R_INTERNAL_ERROR);
-      return -1;
-    }
+     * the packets were reordered on their way. DTLS is unreliable, so drop the
+     * packet and expect the peer to retransmit. */
     rr->length = 0;
     goto start;
   }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 084ae46..6e80f94 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -695,6 +695,8 @@
 				AppDataAfterChangeCipherSpec: []byte("TEST MESSAGE"),
 			},
 		},
+		// BoringSSL's DTLS implementation will drop the out-of-order
+		// application data.
 	},
 	{
 		name: "AlertAfterChangeCipherSpec",
@@ -1174,21 +1176,14 @@
 		return err
 	}
 
-	var testMessage []byte
-	if config.Bugs.AppDataAfterChangeCipherSpec != nil {
-		// We've already sent a message. Expect the shim to echo it
-		// back.
-		testMessage = config.Bugs.AppDataAfterChangeCipherSpec
-	} else {
-		if messageLen == 0 {
-			messageLen = 32
-		}
-		testMessage = make([]byte, messageLen)
-		for i := range testMessage {
-			testMessage[i] = 0x42
-		}
-		tlsConn.Write(testMessage)
+	if messageLen == 0 {
+		messageLen = 32
 	}
+	testMessage := make([]byte, messageLen)
+	for i := range testMessage {
+		testMessage[i] = 0x42
+	}
+	tlsConn.Write(testMessage)
 
 	buf := make([]byte, len(testMessage))
 	if test.protocol == dtls {