Call init and finalization functions per extension message
authorTatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Tue, 18 Apr 2017 14:59:39 +0000 (23:59 +0900)
committerMatt Caswell <matt@openssl.org>
Wed, 26 Apr 2017 15:56:35 +0000 (16:56 +0100)
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@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3244)

ssl/statem/extensions.c
ssl/statem/statem_clnt.c
ssl/statem/statem_locl.h
ssl/statem/statem_srvr.c

index c8ed722..54075ae 100644 (file)
@@ -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
  * 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.
  *
  * 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,
  * 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;
 {
     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
-                && extension_is_relevant(s, thisexd->context, context)
-                && !thisexd->init(s, context)) {
-            *al = SSL_AD_INTERNAL_ERROR;
-            goto err;
+    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;
+            }
         }
     }
 
         }
     }
 
@@ -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
 
 /*
  * 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,
  */
 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;
 {
     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;
     }
 
             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
-                && !thisexd->final(s, context, exts[i].present, al))
-            return 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 1;
     }
 
     return 1;
index ab77ba0..0b4931d 100644 (file)
@@ -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;
 
     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;
         goto f_err;
 
     s->hit = 0;
@@ -1525,7 +1525,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     }
 #endif
 
     }
 #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
         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,
     }
 
     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,
             || !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);
         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,
             }
             if (!tls_collect_extensions(s, &extensions,
                                         SSL_EXT_TLS1_3_CERTIFICATE, &rawexts,
-                                        &al, NULL)
+                                        &al, NULL, chainidx == 0)
                     || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
                     || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
-                                                 rawexts, x, chainidx, &al)) {
+                                                 rawexts, x, chainidx, &al,
+                                                 !PACKET_remaining(pkt))) {
                 OPENSSL_free(rawexts);
                 goto f_err;
             }
                 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,
         }
         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,
             || !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;
         }
             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,
         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,
                 || !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;
         }
             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,
 
     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,
             || !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);
         goto err;
 
     OPENSSL_free(rawexts);
index 2352c6a..3b9311e 100644 (file)
@@ -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,
 __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,
 __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,
 __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,
index d751502..f6ecbf7 100644 (file)
@@ -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,
     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;
     }
         /* 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,
 
     /* 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;
     }
         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,
             }
             if (!tls_collect_extensions(s, &extensions,
                                         SSL_EXT_TLS1_3_CERTIFICATE, &rawexts,
-                                        &al, NULL)
+                                        &al, NULL, chainidx == 0)
                     || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
                     || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
-                                                 rawexts, x, chainidx, &al)) {
+                                                 rawexts, x, chainidx, &al,
+                                                 !PACKET_remaining(&spkt))) {
                 OPENSSL_free(rawexts);
                 goto f_err;
             }
                 OPENSSL_free(rawexts);
                 goto f_err;
             }