Add extensions construction support

Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>

4 files changed, 55 insertions, 32 deletions

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index b59da2b263..b5b9857ebd 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -275,6 +275,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
      "tls_construct_client_verify"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_ENCRYPTED_EXTENSIONS),
      "tls_construct_encrypted_extensions"},
+    {ERR_FUNC(SSL_F_TLS_CONSTRUCT_EXTENSIONS), "tls_construct_extensions"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_FINISHED), "tls_construct_finished"},
     {ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_REQUEST),
      "tls_construct_hello_request"},
@@ -292,6 +293,10 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_GET_MESSAGE_HEADER), "tls_get_message_header"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_KEY_SHARE),
      "tls_parse_clienthello_key_share"},
+    {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE),
+     "tls_parse_clienthello_renegotiate"},
+    {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT),
+     "tls_parse_clienthello_tlsext"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_USE_SRTP),
      "tls_parse_clienthello_use_srtp"},
     {ERR_FUNC(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO),
@@ -336,6 +341,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_PSK_PREAMBLE),
      "tls_process_ske_psk_preamble"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_SRP), "tls_process_ske_srp"},
+    {ERR_FUNC(SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT),
+     "tls_scan_clienthello_tlsext"},
     {ERR_FUNC(SSL_F_USE_CERTIFICATE_CHAIN_FILE),
      "use_certificate_chain_file"},
     {0, NULL}
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 82f1565a95..c98a2055c2 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -45,6 +45,11 @@ typedef struct {
     unsigned int context;
 } EXTENSION_DEFINITION;
 
+/*
+ * TODO(TLS1.3): Temporarily modified the definitions below to put all TLS1.3
+ * extensions in the ServerHello for now. That needs to be put back to correct
+ * setting once encrypted extensions is working properly.
+ */
 static const EXTENSION_DEFINITION ext_defs[] = {
     {
         TLSEXT_TYPE_renegotiate,
@@ -62,7 +67,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         NULL,
         NULL,
         EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO
-        | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+        | /*EXT_TLS1_3_ENCRYPTED_EXTENSIONS*/EXT_TLS1_3_SERVER_HELLO
     },
 #ifndef OPENSSL_NO_SRP
     {
@@ -89,7 +94,8 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         NULL,
         NULL,
         NULL,
-        EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+        EXT_CLIENT_HELLO
+        | /*EXT_TLS1_3_ENCRYPTED_EXTENSIONS*/EXT_TLS1_3_SERVER_HELLO
     },
 #endif
     {
@@ -114,7 +120,8 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         NULL,
         NULL,
         NULL,
-        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_3_CERTIFICATE
+        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO
+        | /*EXT_TLS1_3_CERTIFICATE*/EXT_TLS1_3_SERVER_HELLO
     },
 #ifndef OPENSSL_NO_NEXTPROTONEG
     {
@@ -133,7 +140,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         NULL,
         NULL,
         EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO
-        | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+        | /*EXT_TLS1_3_ENCRYPTED_EXTENSIONS*/EXT_TLS1_3_SERVER_HELLO
     },
     {
         TLSEXT_TYPE_use_srtp,
@@ -163,12 +170,15 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         NULL,
         NULL,
         NULL,
-        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_3_CERTIFICATE
+        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO
+        | /*EXT_TLS1_3_CERTIFICATE*/EXT_TLS1_3_SERVER_HELLO
     },
     {
         TLSEXT_TYPE_extended_master_secret,
         tls_parse_clienthello_ems,
         NULL,
+        NULL,
+        NULL,
         EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_2_AND_BELOW_ONLY
     },
     {
@@ -365,8 +375,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
     return 0;
 }
 
-int tls_parse_all_extensions(SSL *s, RAW_EXTENSION *exts, size_t numexts,
-                             int *al)
+int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts,
+                             size_t numexts, int *al)
 {
     size_t loop;
 
@@ -437,15 +447,15 @@ int tls_parse_all_extensions(SSL *s, RAW_EXTENSION *exts, size_t numexts,
  * failure. If a failure has occurred then |*al| will also be set to the alert
  * to be sent.
  */
-int tls_parse_extension(SSL *s, int type,  RAW_EXTENSION *exts, size_t numexts,
-                        int *al)
+int tls_parse_extension(SSL *s, int type, int context, RAW_EXTENSION *exts,
+                        size_t numexts, int *al)
 {
     RAW_EXTENSION *ext = tls_get_extension_by_type(exts, numexts, type);
 
     if (ext == NULL)
         return 1;
 
-    return tls_parse_all_extensions(s, ext, 1, al);
+    return tls_parse_all_extensions(s, context, ext, 1, al);
 }
 
 int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
@@ -468,23 +478,27 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
     }
 
     for (loop = 0; loop < OSSL_NELEM(ext_defs); loop++) {
+        int (*construct)(SSL *s, WPACKET *pkt, int *al);
+
         /* Skip if not relevant for our context */
         if ((ext_defs[loop].context & context) == 0)
             continue;
 
-        construct = s->server ? extdef->server_construct
-                              : extdef->client_construct;
+        construct = s->server ? ext_defs[loop].server_construct
+                              : ext_defs[loop].client_construct;
 
         /* Check if this extension is defined for our protocol. If not, skip */
         if ((SSL_IS_DTLS(s)
-                    && (extdef->context & EXT_TLS_IMPLEMENTATION_ONLY) != 0)
+                    && (ext_defs[loop].context & EXT_TLS_IMPLEMENTATION_ONLY)
+                       != 0)
                 || (s->version == SSL3_VERSION
-                        && (extdef->context & EXT_SSL3_ALLOWED) == 0)
+                        && (ext_defs[loop].context & EXT_SSL3_ALLOWED) == 0)
                 || (SSL_IS_TLS13(s)
-                    && (extdef->context & EXT_TLS1_2_AND_BELOW_ONLY) != 0)
+                    && (ext_defs[loop].context & EXT_TLS1_2_AND_BELOW_ONLY)
+                       != 0)
                 || (!SSL_IS_TLS13(s)
-                    && ((extdef->context & EXT_TLS1_3_ONLY) != 0
-                        || (context & EXT_CLIENT_HELLO) != 0))
+                    && (ext_defs[loop].context & EXT_TLS1_3_ONLY) != 0
+                    && (context & EXT_CLIENT_HELLO) == 0)
                 || construct == NULL)
             continue;
 
@@ -492,12 +506,11 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
             return 0;
     }
 
-
     /* Add custom extensions */
     if ((context & EXT_CLIENT_HELLO) != 0) {
         custom_ext_init(&s->cert->cli_ext);
         addcustom = 1;
-    } else if (context & (EXT_TLS1_2_SERVER_HELLO) {
+    } else if ((context & EXT_TLS1_2_SERVER_HELLO) != 0) {
         /*
          * We already initialised the custom extensions during ClientHello
          * parsing.
@@ -508,13 +521,14 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
          */
         addcustom = 1;
     }
+
     if (addcustom && !custom_ext_add(s, s->server, pkt, al)) {
-        SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+        SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);
         return 0;
     }
 
     if (!WPACKET_close(pkt)) {
-        *sl = SSL_AD_INTERNAL_ERROR;
+        *al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);
         return 0;
     }
@@ -522,7 +536,6 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
     return 1;
 }
 
-
 /*
  * Parse the client's renegotiation binding and abort if it's not right
  */
@@ -534,7 +547,7 @@ static int tls_parse_clienthello_renegotiate(SSL *s, PACKET *pkt, int *al)
     /* Parse the length byte */
     if (!PACKET_get_1(pkt, &ilen)
         || !PACKET_get_bytes(pkt, &data, ilen)) {
-        SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,
+        SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE,
                SSL_R_RENEGOTIATION_ENCODING_ERR);
         *al = SSL_AD_ILLEGAL_PARAMETER;
         return 0;
@@ -542,7 +555,7 @@ static int tls_parse_clienthello_renegotiate(SSL *s, PACKET *pkt, int *al)
 
     /* Check that the extension matches */
     if (ilen != s->s3->previous_client_finished_len) {
-        SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,
+        SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE,
                SSL_R_RENEGOTIATION_MISMATCH);
         *al = SSL_AD_HANDSHAKE_FAILURE;
         return 0;
@@ -550,7 +563,7 @@ static int tls_parse_clienthello_renegotiate(SSL *s, PACKET *pkt, int *al)
 
     if (memcmp(data, s->s3->previous_client_finished,
                s->s3->previous_client_finished_len)) {
-        SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,
+        SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE,
                SSL_R_RENEGOTIATION_MISMATCH);
         *al = SSL_AD_HANDSHAKE_FAILURE;
         return 0;
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 42d5a8c5c6..481336d47e 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -149,7 +149,9 @@ __owur MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt);
 #endif
 __owur int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt);
 
-__owur int tls_parse_all_extensions(SSL *s, RAW_EXTENSION *exts, size_t numexts,
-                                    int *al);
-__owur int tls_parse_extension(SSL *s, int type,  RAW_EXTENSION *exts,
+__owur int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts,
+                                    size_t numexts, int *al);
+__owur int tls_parse_extension(SSL *s, int type, int context, RAW_EXTENSION *exts,
                                size_t numexts, int *al);
+__owur int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
+                                    int *al);
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 694f83e157..773e732642 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1173,7 +1173,7 @@ static int tls_scan_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello, int *al)
      * We process the supported_groups extension first so that is done before
      * we get to key_share which needs to use the information in it.
      */
-    if (!tls_parse_extension(s, TLSEXT_TYPE_supported_groups,
+    if (!tls_parse_extension(s, TLSEXT_TYPE_supported_groups, EXT_CLIENT_HELLO,
                              hello->pre_proc_exts, hello->num_extensions, al)) {
         return 0;
     }
@@ -1185,12 +1185,12 @@ static int tls_scan_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello, int *al)
                                          hello->num_extensions,
                                          TLSEXT_TYPE_renegotiate) == NULL) {
         *al = SSL_AD_HANDSHAKE_FAILURE;
-        SSLerr(SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT,
+        SSLerr(SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT,
                SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
         return 0;
     }
 
-    return tls_parse_all_extensions(s, hello->pre_proc_exts,
+    return tls_parse_all_extensions(s, EXT_CLIENT_HELLO, hello->pre_proc_exts,
                                     hello->num_extensions, al);
 }
 
@@ -1245,7 +1245,7 @@ static int tls_parse_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello)
     }
 
     if (!tls_check_clienthello_tlsext(s)) {
-        SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT, SSL_R_CLIENTHELLO_TLSEXT);
+        SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT, SSL_R_CLIENTHELLO_TLSEXT);
         return 0;
     }
 
@@ -1527,6 +1527,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
 
     /* We need to do this before getting the session */
     if (!tls_parse_extension(s, TLSEXT_TYPE_extended_master_secret,
+                             EXT_CLIENT_HELLO,
                              clienthello.pre_proc_exts,
                              clienthello.num_extensions, &al)) {
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT);