[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Wed Apr 26 16:02:09 UTC 2017


The branch master has been updated
       via  8e1634ec36c9a125f3aa1c48b743ff864c6fcbd3 (commit)
       via  bf5c84f5d17ed68076ba1faa4a9eaaf4f51e4bf1 (commit)
       via  735d5b59df341236a6c9bb51ebdfebf9119ebeab (commit)
      from  b89646684d920d3014979f8a73b96aecb61c7b1f (commit)


- Log -----------------------------------------------------------------
commit 8e1634ec36c9a125f3aa1c48b743ff864c6fcbd3
Author: Tatsuhiro Tsujikawa <tatsuhiro.t at gmail.com>
Date:   Fri Apr 21 22:10:32 2017 +0900

    Don't treat PACKET_remaining() as boolean
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3244)

commit bf5c84f5d17ed68076ba1faa4a9eaaf4f51e4bf1
Author: Tatsuhiro Tsujikawa <tatsuhiro.t at gmail.com>
Date:   Wed Apr 19 21:12:34 2017 +0900

    Break before && operator
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3244)

commit 735d5b59df341236a6c9bb51ebdfebf9119ebeab
Author: Tatsuhiro Tsujikawa <tatsuhiro.t at gmail.com>
Date:   Tue Apr 18 23:59:39 2017 +0900

    Call init and finalization functions per extension message
    
    Previously, init and finalization function for extensions are called
    per extension block, rather than per message.  This commit changes
    that behaviour, and now they are called per message.  The parse
    function is still called per extension block.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3244)

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

Summary of changes:
 ssl/statem/extensions.c  | 57 +++++++++++++++++++++++++++---------------------
 ssl/statem/statem_clnt.c | 27 ++++++++++++-----------
 ssl/statem/statem_locl.h |  5 +++--
 ssl/statem/statem_srvr.c | 11 +++++-----
 4 files changed, 55 insertions(+), 45 deletions(-)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index c8ed722..f892675 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -421,8 +421,9 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx)
  * stored in |*res| on success. In the event of an error the alert type to use
  * is stored in |*al|. We don't actually process the content of the extensions
  * yet, except to check their types. This function also runs the initialiser
- * functions for all known extensions (whether we have collected them or not).
- * If successful the caller is responsible for freeing the contents of |*res|.
+ * functions for all known extensions if |init| is nonzero (whether we have
+ * collected them or not). If successful the caller is responsible for freeing
+ * the contents of |*res|.
  *
  * Per http://tools.ietf.org/html/rfc5246#section-7.4.1.4, there may not be
  * more than one extension of the same type in a ClientHello or ServerHello.
@@ -432,7 +433,8 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx)
  * extensions that we know about. We ignore others.
  */
 int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
-                           RAW_EXTENSION **res, int *al, size_t *len)
+                           RAW_EXTENSION **res, int *al, size_t *len,
+                           int init)
 {
     PACKET extensions = *packet;
     size_t i = 0;
@@ -490,16 +492,19 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
         }
     }
 
-    /*
-     * Initialise all known extensions relevant to this context, whether we have
-     * found them or not
-     */
-    for (thisexd = ext_defs, i = 0; i < OSSL_NELEM(ext_defs); i++, thisexd++) {
-        if(thisexd->init != NULL && (thisexd->context & context) != 0
+    if (init) {
+        /*
+         * Initialise all known extensions relevant to this context,
+         * whether we have found them or not
+         */
+        for (thisexd = ext_defs, i = 0; i < OSSL_NELEM(ext_defs);
+             i++, thisexd++) {
+            if (thisexd->init != NULL && (thisexd->context & context) != 0
                 && extension_is_relevant(s, thisexd->context, context)
                 && !thisexd->init(s, context)) {
-            *al = SSL_AD_INTERNAL_ERROR;
-            goto err;
+                *al = SSL_AD_INTERNAL_ERROR;
+                goto err;
+            }
         }
     }
 
@@ -578,14 +583,14 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
 
 /*
  * Parse all remaining extensions that have not yet been parsed. Also calls the
- * finalisation for all extensions at the end, whether we collected them or not.
- * Returns 1 for success or 0 for failure. If we are working on a Certificate
- * message then we also pass the Certificate |x| and its position in the
- * |chainidx|, with 0 being the first certificate. On failure, |*al| is
- * populated with a suitable alert code.
+ * finalisation for all extensions at the end if |fin| is nonzero, whether we
+ * collected them or not. Returns 1 for success or 0 for failure. If we are
+ * working on a Certificate message then we also pass the Certificate |x| and
+ * its position in the |chainidx|, with 0 being the first certificate. On
+ * failure, |*al| is populated with a suitable alert code.
  */
 int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,
-                             size_t chainidx, int *al)
+                             size_t chainidx, int *al, int fin)
 {
     size_t i, numexts = OSSL_NELEM(ext_defs);
     const EXTENSION_DEFINITION *thisexd;
@@ -599,15 +604,17 @@ int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,
             return 0;
     }
 
-    /*
-     * Finalise all known extensions relevant to this context, whether we have
-     * found them or not
-     */
-    for (i = 0, thisexd = ext_defs; i < OSSL_NELEM(ext_defs); i++, thisexd++) {
-        if(thisexd->final != NULL
-                && (thisexd->context & context) != 0
+    if (fin) {
+        /*
+         * Finalise all known extensions relevant to this context,
+         * whether we have found them or not
+         */
+        for (i = 0, thisexd = ext_defs; i < OSSL_NELEM(ext_defs);
+             i++, thisexd++) {
+            if (thisexd->final != NULL && (thisexd->context & context) != 0
                 && !thisexd->final(s, context, exts[i].present, al))
-            return 0;
+                return 0;
+        }
     }
 
     return 1;
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index ab77ba0..a66dd40 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1373,7 +1373,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
 
     context = SSL_IS_TLS13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO
                               : SSL_EXT_TLS1_2_SERVER_HELLO;
-    if (!tls_collect_extensions(s, &extpkt, context, &extensions, &al, NULL))
+    if (!tls_collect_extensions(s, &extpkt, context, &extensions, &al, NULL, 1))
         goto f_err;
 
     s->hit = 0;
@@ -1525,7 +1525,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     }
 #endif
 
-    if (!tls_parse_all_extensions(s, context, extensions, NULL, 0, &al))
+    if (!tls_parse_all_extensions(s, context, extensions, NULL, 0, &al, 1))
         goto f_err;
 
 #ifndef OPENSSL_NO_SCTP
@@ -1616,9 +1616,9 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
     }
 
     if (!tls_collect_extensions(s, &extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
-                                &extensions, &al, NULL)
+                                &extensions, &al, NULL, 1)
             || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
-                                         extensions, NULL, 0, &al))
+                                         extensions, NULL, 0, &al, 1))
         goto f_err;
 
     OPENSSL_free(extensions);
@@ -1711,9 +1711,10 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt)
             }
             if (!tls_collect_extensions(s, &extensions,
                                         SSL_EXT_TLS1_3_CERTIFICATE, &rawexts,
-                                        &al, NULL)
-                    || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
-                                                 rawexts, x, chainidx, &al)) {
+                                        &al, NULL, chainidx == 0)
+                || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
+                                             rawexts, x, chainidx, &al,
+                                             PACKET_remaining(pkt) == 0)) {
                 OPENSSL_free(rawexts);
                 goto f_err;
             }
@@ -2340,9 +2341,9 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt)
         }
         if (!tls_collect_extensions(s, &extensions,
                                     SSL_EXT_TLS1_3_CERTIFICATE_REQUEST,
-                                    &rawexts, &al, NULL)
+                                    &rawexts, &al, NULL, 1)
             || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE_REQUEST,
-                                         rawexts, NULL, 0, &al)) {
+                                         rawexts, NULL, 0, &al, 1)) {
             OPENSSL_free(rawexts);
             goto err;
         }
@@ -2501,10 +2502,10 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
         if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
                 || !tls_collect_extensions(s, &extpkt,
                                            SSL_EXT_TLS1_3_NEW_SESSION_TICKET,
-                                           &exts, &al, NULL)
+                                           &exts, &al, NULL, 1)
                 || !tls_parse_all_extensions(s,
                                              SSL_EXT_TLS1_3_NEW_SESSION_TICKET,
-                                             exts, NULL, 0, &al)) {
+                                             exts, NULL, 0, &al, 1)) {
             SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, SSL_R_BAD_EXTENSION);
             goto f_err;
         }
@@ -3464,9 +3465,9 @@ static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt)
 
     if (!tls_collect_extensions(s, &extensions,
                                 SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS, &rawexts,
-                                &al, NULL)
+                                &al, NULL, 1)
             || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
-                                         rawexts, NULL, 0, &al))
+                                         rawexts, NULL, 0, &al, 1))
         goto err;
 
     OPENSSL_free(rawexts);
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 2352c6a..3b9311e 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -156,12 +156,13 @@ MSG_PROCESS_RETURN tls_process_end_of_early_data(SSL *s, PACKET *pkt);
 __owur int extension_is_relevant(SSL *s, unsigned int extctx,
                                  unsigned int thisctx);
 __owur int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
-                                  RAW_EXTENSION **res, int *al, size_t *len);
+                                  RAW_EXTENSION **res, int *al, size_t *len,
+                                  int init);
 __owur int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
                                RAW_EXTENSION *exts,  X509 *x, size_t chainidx,
                                int *al);
 __owur int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts,
-                                    X509 *x, size_t chainidx, int *al);
+                                    X509 *x, size_t chainidx, int *al, int fin);
 __owur int should_add_extension(SSL *s, unsigned int extctx,
                                 unsigned int thisctx, int max_version);
 __owur int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index d751502..9dfdbe5 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1426,7 +1426,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     extensions = clienthello->extensions;
     if (!tls_collect_extensions(s, &extensions, SSL_EXT_CLIENT_HELLO,
                                 &clienthello->pre_proc_exts, &al,
-                                &clienthello->pre_proc_exts_len)) {
+                                &clienthello->pre_proc_exts_len, 1)) {
         /* SSLerr already been called */
         goto f_err;
     }
@@ -1690,7 +1690,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
 
     /* TLS extensions */
     if (!tls_parse_all_extensions(s, SSL_EXT_CLIENT_HELLO,
-                                  clienthello->pre_proc_exts, NULL, 0, &al)) {
+                                  clienthello->pre_proc_exts, NULL, 0, &al, 1)) {
         SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_PARSE_TLSEXT);
         goto err;
     }
@@ -3217,9 +3217,10 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
             }
             if (!tls_collect_extensions(s, &extensions,
                                         SSL_EXT_TLS1_3_CERTIFICATE, &rawexts,
-                                        &al, NULL)
-                    || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
-                                                 rawexts, x, chainidx, &al)) {
+                                        &al, NULL, chainidx == 0)
+                || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
+                                             rawexts, x, chainidx, &al,
+                                             PACKET_remaining(&spkt) == 0)) {
                 OPENSSL_free(rawexts);
                 goto f_err;
             }


More information about the openssl-commits mailing list