commit 15aa895a0b6bb6d8f409cff8f1d6f47a069cd4b9
author David Benjamin <davidben@google.com> Tue May 10 20:51:34 2016 -0400
committer David Benjamin <davidben@google.com> Wed Jun 08 18:13:42 2016 +0000
tree abcccb85b801aeb54e3edae040862dc6bd72828a
parent 585d7a4987126192d90d5e6d33fc400da00a863b

Tidy up the DTLS code's blocking-mode retransmits.

Move this logic out of dtls1_read_bytes and into dtls1_get_record. Only trigger
it when reading from the buffer fails. The other one shouldn't be necessary.
This exists to handle the blocking BIO case when the
BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT signal triggers, so we only need to do it when
timeouts actually trigger.

There also doesn't seem to be a need for most of the machinery. The
BIO_set_flags call seems to be working around a deficiency in the underlying
BIO. There also shouldn't be a need to check the handshake state as there
wouldn't be a timer to restart otherwise.

Change-Id: Ic901ccfb5b82aeb409d16a9d32c04741410ad6d7
Reviewed-on: https://boringssl-review.googlesource.com/8122
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

ssl/d1_both.c

diff --git a/ssl/d1_both.c b/ssl/d1_both.c
index 8d8784b..e101a4f 100644
--- a/ssl/d1_both.c
+++ b/ssl/d1_both.c
@@ -658,27 +658,6 @@
   return -1;
 }
 
-int dtls1_read_failed(SSL *ssl, int code) {
-  if (code > 0) {
-    assert(0);
-    return 1;
-  }
-
-  if (!dtls1_is_timer_expired(ssl)) {
-    /* not a timeout, none of our business, let higher layers handle this. In
-     * fact, it's probably an error */
-    return code;
-  }
-
-  if (!SSL_in_init(ssl)) {
-    /* done, no need to send a retransmit */
-    BIO_set_flags(ssl->rbio, BIO_FLAGS_READ);
-    return code;
-  }
-
-  return DTLSv1_handle_timeout(ssl);
-}
-
 static uint16_t dtls1_get_queue_priority(uint16_t seq, int is_ccs) {
   assert(seq * 2 >= seq);
 

ssl/d1_pkt.c

diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 34eeddb..2a1017d 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -114,6 +114,7 @@
 #include <assert.h>
 #include <string.h>
 
+#include <openssl/bio.h>
 #include <openssl/buf.h>
 #include <openssl/mem.h>
 #include <openssl/evp.h>
@@ -144,6 +145,14 @@
   /* Read a new packet if there is no unconsumed one. */
   if (ssl_read_buffer_len(ssl) == 0) {
     int ret = ssl_read_buffer_extend_to(ssl, 0 /* unused */);
+    if (ret < 0 && dtls1_is_timer_expired(ssl)) {
+      /* For blocking BIOs, retransmits must be handled internally. */
+      int timeout_ret = DTLSv1_handle_timeout(ssl);
+      if (timeout_ret <= 0) {
+        return timeout_ret;
+      }
+      goto again;
+    }
     if (ret <= 0) {
       return ret;
     }
@@ -264,22 +273,11 @@
    * ssl->s3->rrec.length   - number of bytes. */
   rr = &ssl->s3->rrec;
 
-  /* Check for timeout */
-  if (DTLSv1_handle_timeout(ssl) > 0) {
-    goto start;
-  }
-
   /* get new packet if necessary */
   if (rr->length == 0) {
     ret = dtls1_get_record(ssl);
     if (ret <= 0) {
-      ret = dtls1_read_failed(ssl, ret);
-      /* anything other than a timeout is an error */
-      if (ret <= 0) {
-        return ret;
-      } else {
-        goto start;
-      }
+      return ret;
     }
   }
 

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 3b56e78..20cf1cb 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1121,7 +1121,6 @@
 
 int dtls1_send_change_cipher_spec(SSL *ssl, int a, int b);
 int dtls1_send_finished(SSL *ssl, int a, int b, const char *sender, int slen);
-int dtls1_read_failed(SSL *ssl, int code);
 int dtls1_buffer_message(SSL *ssl);
 int dtls1_retransmit_buffered_messages(SSL *ssl);
 void dtls1_clear_record_buffer(SSL *ssl);