[openssl-commits] [openssl] OpenSSL_1_1_0-stable update

[Matt Caswell]

The branch OpenSSL_1_1_0-stable has been updated
       via  329aa3412ea1d8baa4fb09c976b11f9e7c46a762 (commit)
       via  041ddc366b6d18ee3993877a50299257e688c00c (commit)
      from  8e4057768586961942851d89287f43969352127a (commit)


- Log -----------------------------------------------------------------
commit 329aa3412ea1d8baa4fb09c976b11f9e7c46a762
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Jan 16 11:26:50 2018 +0000

    Don't wait for dry at the end of a handshake
    
    For DTLS/SCTP we were waiting for a dry event during the call to
    tls_finish_handshake(). This function just tidies up various internal
    things, and after it completes the handshake is over. I can find no good
    reason for waiting for a dry event here, and nothing in RFC6083 suggests
    to me that we should need to. More importantly though it seems to be
    wrong. It is perfectly possible for a peer to send app data/alerts/new
    handshake while we are still cleaning up our handshake. If this happens
    then we will never get the dry event and so we cannot continue.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5085)

commit 041ddc366b6d18ee3993877a50299257e688c00c
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Jan 16 10:48:01 2018 +0000

    Check for alerts while waiting for a dry event
    
    At a couple of points in a DTLS/SCTP handshake we need to wait for a dry
    event before continuing. However if an alert has been sent by the peer
    then we will never receive that dry event and an infinite loop results.
    
    This commit changes things so that we attempt to read a message if we
    are waiting for a dry event but haven't got one yet. This should never
    succeed, but any alerts will be processed.
    
    Fixes #4763
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5085)

-----------------------------------------------------------------------

Summary of changes:
 include/openssl/ssl.h    |  1 +
 ssl/ssl_err.c            |  1 +
 ssl/statem/statem_dtls.c | 19 +++++++++++++++++++
 ssl/statem/statem_lib.c  |  9 ---------
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index abe4406..d99008d 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2110,6 +2110,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_DTLS_CONSTRUCT_HELLO_VERIFY_REQUEST        385
 # define SSL_F_DTLS_GET_REASSEMBLED_MESSAGE               370
 # define SSL_F_DTLS_PROCESS_HELLO_VERIFY                  386
+# define SSL_F_DTLS_WAIT_FOR_DRY                          592
 # define SSL_F_OPENSSL_INIT_SSL                           342
 # define SSL_F_OSSL_STATEM_CLIENT_READ_TRANSITION         417
 # define SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION         418
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 3c2ebe1..580861e 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -48,6 +48,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_DTLS_GET_REASSEMBLED_MESSAGE),
      "dtls_get_reassembled_message"},
     {ERR_FUNC(SSL_F_DTLS_PROCESS_HELLO_VERIFY), "dtls_process_hello_verify"},
+    {ERR_FUNC(SSL_F_DTLS_WAIT_FOR_DRY), "dtls_wait_for_dry"},
     {ERR_FUNC(SSL_F_OPENSSL_INIT_SSL), "OPENSSL_init_ssl"},
     {ERR_FUNC(SSL_F_OSSL_STATEM_CLIENT_READ_TRANSITION),
      "ossl_statem_client_read_transition"},
diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 22be871..ebc95d3 100644
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -910,9 +910,14 @@ int dtls_construct_change_cipher_spec(SSL *s)
 }
 
 #ifndef OPENSSL_NO_SCTP
+/*
+ * Wait for a dry event. Should only be called at a point in the handshake
+ * where we are not expecting any data from the peer (except possibly an alert).
+ */
 WORK_STATE dtls_wait_for_dry(SSL *s)
 {
     int ret;
+    long len;
 
     /* read app data until dry event */
     ret = BIO_dgram_sctp_wait_for_dry(SSL_get_wbio(s));
@@ -920,6 +925,20 @@ WORK_STATE dtls_wait_for_dry(SSL *s)
         return WORK_ERROR;
 
     if (ret == 0) {
+        /*
+         * We're not expecting any more messages from the peer at this point -
+         * but we could get an alert. If an alert is waiting then we will never
+         * return successfully. Therefore we attempt to read a message. This
+         * should never succeed but will process any waiting alerts.
+         */
+        if (dtls_get_reassembled_message(s, &len)) {
+            /* The call succeeded! This should never happen */
+            SSLerr(SSL_F_DTLS_WAIT_FOR_DRY, SSL_R_UNEXPECTED_MESSAGE);
+            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
+            ossl_statem_set_error(s);
+            return WORK_ERROR;
+        }
+
         s->s3->in_read_app_data = 2;
         s->rwstate = SSL_READING;
         BIO_clear_retry_flags(SSL_get_rbio(s));
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 5702145..4e606d8 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -273,15 +273,6 @@ WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst)
 {
     void (*cb) (const SSL *ssl, int type, int val) = NULL;
 
-#ifndef OPENSSL_NO_SCTP
-    if (SSL_IS_DTLS(s) && BIO_dgram_is_sctp(SSL_get_wbio(s))) {
-        WORK_STATE ret;
-        ret = dtls_wait_for_dry(s);
-        if (ret != WORK_FINISHED_CONTINUE)
-            return ret;
-    }
-#endif
-
     /* clean a few things up */
     ssl3_cleanup_key_block(s);