[openssl-commits] [openssl] OpenSSL_1_0_2-stable update

[Matt Caswell]

The branch OpenSSL_1_0_2-stable has been updated
       via  d498e526832bd6c50238f3dc0bcac9375179926e (commit)
      from  fe8199badfce9ffccd0e1d9c245c4cdd8d412ef1 (commit)


- Log -----------------------------------------------------------------
commit d498e526832bd6c50238f3dc0bcac9375179926e
Author: Matt Caswell <matt at openssl.org>
Date:   Mon Jan 29 14:55:44 2018 +0000

    Make sure we check an incoming reneg ClientHello in DTLS
    
    In TLS we have a check to make sure an incoming reneg ClientHello is
    acceptable. The equivalent check is missing in the DTLS code. This means
    that if a client does not signal the ability to handle secure reneg in the
    initial handshake, then a subsequent reneg handshake should be rejected by
    the server. In the DTLS case the reneg was being allowed if the the 2nd
    ClientHello had a renegotiation_info extension. This is incorrect.
    
    While incorrect, this does not represent a security issue because if
    the renegotiation_info extension is present in the second ClientHello it
    also has to be *correct*. Therefore this will only work if both the client
    and server believe they are renegotiating, and both know the previous
    Finished result. This is not the case in an insecure rengotiation attack.
    
    I have also tidied up the check in the TLS code and given a better check
    for determining whether we are renegotiating or not.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5192)

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

Summary of changes:
 ssl/d1_pkt.c | 18 ++++++++++++++++++
 ssl/s3_pkt.c | 25 ++++++++++++-------------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 685d50a..4e92e49 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -1205,6 +1205,24 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
         goto start;
     }
 
+    /*
+     * If we are a server and get a client hello when renegotiation isn't
+     * allowed send back a no renegotiation alert and carry on.
+     */
+    if (s->server
+            && SSL_is_init_finished(s)
+            && !s->s3->send_connection_binding
+            && s->d1->handshake_fragment_len >= DTLS1_HM_HEADER_LENGTH
+            && s->d1->handshake_fragment[0] == SSL3_MT_CLIENT_HELLO
+            && s->s3->previous_client_finished_len != 0
+            && (s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION) == 0) {
+        s->d1->handshake_fragment_len = 0;
+        rr->length = 0;
+        ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
+        goto start;
+    }
+
+
     if (s->d1->alert_fragment_len >= DTLS1_AL_HEADER_LENGTH) {
         int alert_level = s->d1->alert_fragment[0];
         int alert_descr = s->d1->alert_fragment[1];
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index b2373dc..d74a91d 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -1421,26 +1421,25 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
          */
         goto start;
     }
+
     /*
      * If we are a server and get a client hello when renegotiation isn't
-     * allowed send back a no renegotiation alert and carry on. WARNING:
-     * experimental code, needs reviewing (steve)
+     * allowed send back a no renegotiation alert and carry on.
      */
-    if (s->server &&
-        SSL_is_init_finished(s) &&
-        !s->s3->send_connection_binding &&
-        (s->version > SSL3_VERSION) &&
-        (s->s3->handshake_fragment_len >= 4) &&
-        (s->s3->handshake_fragment[0] == SSL3_MT_CLIENT_HELLO) &&
-        (s->session != NULL) && (s->session->cipher != NULL) &&
-        !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) {
-        /*
-         * s->s3->handshake_fragment_len = 0;
-         */
+    if (s->server
+            && SSL_is_init_finished(s)
+            && !s->s3->send_connection_binding
+            && s->version > SSL3_VERSION
+            && s->s3->handshake_fragment_len >= SSL3_HM_HEADER_LENGTH
+            && s->s3->handshake_fragment[0] == SSL3_MT_CLIENT_HELLO
+            && s->s3->previous_client_finished_len != 0
+            && (s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION) == 0) {
+        s->s3->handshake_fragment_len = 0;
         rr->length = 0;
         ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
         goto start;
     }
+
     if (s->s3->alert_fragment_len >= 2) {
         int alert_level = s->s3->alert_fragment[0];
         int alert_descr = s->s3->alert_fragment[1];