commit a05d427b41c21e35c57740951e9b78a8333a488e
author David Benjamin <davidben@google.com> Fri Oct 06 19:34:55 2017 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Oct 17 15:19:01 2017 +0000
tree 1d6fe0b1e2c62875922a2baee6943f97b9da123c
parent 3b777adb611487434b7cb8a135b745589a527e8b

Align dtls_open_record and tls_open_record more closely.

Ultimately the ssl_buffer_* code will be above SSL_PROTOCOL_METHOD, so
having the processing be analogous is simpler. This also means that DTLS
can surface errors out of dtls_open_record without the caller reading an
extra record.

Bug: 206
Change-Id: Ic1cb3a884763c8e875e1129b1cda226f72bc95b7
Reviewed-on: https://boringssl-review.googlesource.com/21364
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>

ssl/d1_pkt.cc

diff --git a/ssl/d1_pkt.cc b/ssl/d1_pkt.cc
index 0f1c407..c2cbd7e 100644
--- a/ssl/d1_pkt.cc
+++ b/ssl/d1_pkt.cc
@@ -140,25 +140,23 @@
       return 0;
   }
 
-  // Read a new packet if there is no unconsumed one.
-  if (ssl_read_buffer(ssl).empty()) {
-    int read_ret = ssl_read_buffer_extend_to(ssl, 0 /* unused */);
-    if (read_ret <= 0) {
-      return read_ret;
-    }
-  }
-  assert(!ssl_read_buffer(ssl).empty());
-
   Span<uint8_t> body;
   uint8_t type, alert;
   size_t consumed;
   enum ssl_open_record_t open_ret = dtls_open_record(
       ssl, &type, &body, &consumed, &alert, ssl_read_buffer(ssl));
-  ssl_read_buffer_consume(ssl, consumed);
+  if (open_ret != ssl_open_record_partial) {
+    ssl_read_buffer_consume(ssl, consumed);
+  }
   switch (open_ret) {
-    case ssl_open_record_partial:
-      // Impossible in DTLS.
-      break;
+    case ssl_open_record_partial: {
+      assert(ssl_read_buffer(ssl).empty());
+      int read_ret = ssl_read_buffer_extend_to(ssl, 0 /* unused */);
+      if (read_ret <= 0) {
+        return read_ret;
+      }
+      goto again;
+    }
 
     case ssl_open_record_success: {
       if (body.size() > 0xffff) {

ssl/dtls_record.cc

diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc
index 71b5e3b..c06a62d 100644
--- a/ssl/dtls_record.cc
+++ b/ssl/dtls_record.cc
@@ -179,6 +179,9 @@
                                         size_t *out_consumed,
                                         uint8_t *out_alert, Span<uint8_t> in) {
   *out_consumed = 0;
+  if (in.empty()) {
+    return ssl_open_record_partial;
+  }
 
   CBS cbs = CBS(in);
 

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index f141923..edbf4eb 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -801,8 +801,9 @@
                                        Span<uint8_t> *out, size_t *out_consumed,
                                        uint8_t *out_alert, Span<uint8_t> in);
 
-// dtls_open_record implements |tls_open_record| for DTLS. It never returns
-// |ssl_open_record_partial| but otherwise behaves analogously.
+// dtls_open_record implements |tls_open_record| for DTLS. It only returns
+// |ssl_open_record_partial| if |in| was empty and sets |*out_consumed| to
+// zero. The caller should read one packet and try again.
 enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
                                         Span<uint8_t> *out,
                                         size_t *out_consumed,