commit e9cb2ec832bc3225f617551c5bb2ba9170c74abc
author David Benjamin <davidben@chromium.org> Sat Aug 22 11:31:33 2015 -0400
committer Adam Langley <agl@google.com> Mon Aug 31 19:08:06 2015 +0000
tree cceed812bcfd707655dca9ef55b907ff86b24a07
parent 30789da28e0124c0523adc0b7bfb83737015dba6

Don't support bidirectional shutdown over DTLS.

Bidirectional shutdown doesn't make sense over DTLS; you can't reuse the
underlying channel after receiving close_notify because the channel is
unordered. This removes one caller of dtls1_read_bytes.

Really close_notify makes no sense in DTLS. It can't even protect
against some kind of truncation because it's all unordered. But continue
to send it in case anything is (unreliably since the channel is lossy)
relying on close_notify to signal some kind of session end. This only
makes SSL_shutdown stop trying to wait for one once we've already
decided to shut down the connection.

BUG=526437

Change-Id: I6afad7cb7209c4aba0b96f9246b04c81d90987a9
Reviewed-on: https://boringssl-review.googlesource.com/5770
Reviewed-by: Adam Langley <agl@google.com>

ssl/d1_pkt.c

diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 81e9b0a..3306368 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -189,7 +189,11 @@
 }
 
 void dtls1_read_close_notify(SSL *ssl) {
-  dtls1_read_bytes(ssl, 0, NULL, 0, 0);
+  /* Bidirectional shutdown doesn't make sense for an unordered transport. DTLS
+   * alerts also aren't delivered reliably, so we may even time out because the
+   * peer never received our close_notify. Report to the caller that the channel
+   * has fully shut down. */
+  ssl->shutdown |= SSL_RECEIVED_SHUTDOWN;
 }
 
 /* Return up to 'len' payload bytes received in 'type' records.
@@ -197,7 +201,6 @@
  *
  *   -  SSL3_RT_HANDSHAKE (when ssl3_get_message calls us)
  *   -  SSL3_RT_APPLICATION_DATA (when ssl3_read calls us)
- *   -  0 (during a shutdown, no data has to be returned)
  *
  * If we don't have stored data to work from, read a SSL/TLS record first
  * (possibly multiple records if we still don't have anything to return).
@@ -225,10 +228,8 @@
   SSL3_RECORD *rr;
   void (*cb)(const SSL *ssl, int type2, int val) = NULL;
 
-  /* XXX: check what the second '&& type' is about */
-  if ((type && (type != SSL3_RT_APPLICATION_DATA) &&
-       (type != SSL3_RT_HANDSHAKE) && type) ||
-      (peek && (type != SSL3_RT_APPLICATION_DATA))) {
+  if ((type != SSL3_RT_APPLICATION_DATA && type != SSL3_RT_HANDSHAKE) ||
+      (peek && type != SSL3_RT_APPLICATION_DATA)) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
     return -1;
   }
@@ -393,13 +394,6 @@
     goto start;
   }
 
-  if (s->shutdown & SSL_SENT_SHUTDOWN) {
-    /* but we have not received a shutdown */
-    s->rwstate = SSL_NOTHING;
-    rr->length = 0;
-    return 0;
-  }
-
   if (rr->type == SSL3_RT_CHANGE_CIPHER_SPEC) {
     /* 'Change Cipher Spec' is just a single byte, so we know exactly what the
      * record payload has to look like */