Avoid repeatedly scanning the list of extensions

Because extensions were keyed by type which is sparse, we were continually
scanning the list to find the one we wanted. The way we stored them also
had the side effect that we were running initialisers/finalisers in a
different oder to the parsers. In this commit we change things so that we
instead key on an index value for each extension.

Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz
Reviewed-by: NRich Salz <rsalz@openssl.org>
Reviewed-by: NRichard Levitte <levitte@openssl.org>

ssl/ssl_locl.h

@@ -1636,14 +1636,39 @@ typedef struct ssl3_comp_st {
 # endif
 
 typedef struct raw_extension_st {
-    /* The type of the extension */
-    unsigned int type;
     /* Raw packet data for the extension */
     PACKET data;
+    /* Set to 1 if the extension is present or 0 otherwise */
+    int present;
     /* Set to 1 if we have already parsed the extension or 0 otherwise */
     int parsed;
+    /* The type of this extension */
+    unsigned int type;
 } RAW_EXTENSION;
 
+/*
+ * Extension index values NOTE: Any updates to these defines should be mirrored
+ * with equivalent updates to ext_defs in extensions.c
+ */
+#define TLSEXT_IDX_renegotiate                                   0
+#define TLSEXT_IDX_server_name                                   1
+#define TLSEXT_IDX_srp                                           2
+#define TLSEXT_IDX_ec_point_formats                              3
+#define TLSEXT_IDX_supported_groups                              4
+#define TLSEXT_IDX_session_ticket                                5
+#define TLSEXT_IDX_signature_algorithms                          6
+#define TLSEXT_IDX_status_request                                7
+#define TLSEXT_IDX_next_proto_neg                                8
+#define TLSEXT_IDX_application_layer_protocol_negotiation        9
+#define TLSEXT_IDX_use_srtp                                     10
+#define TLSEXT_IDX_encrypt_then_mac                             11
+#define TLSEXT_IDX_signed_certificate_timestamp                 12
+#define TLSEXT_IDX_extended_master_secret                       13
+#define TLSEXT_IDX_supported_versions                           14
+#define TLSEXT_IDX_key_share                                    15
+#define TLSEXT_IDX_cryptopro_bug                                16
+#define TLSEXT_IDX_padding                                      17
+
 #define MAX_COMPRESSIONS_SIZE   255
 
 typedef struct {
@@ -1658,7 +1683,6 @@ typedef struct {
     size_t compressions_len;
     unsigned char compressions[MAX_COMPRESSIONS_SIZE];
     PACKET extensions;
-    size_t num_extensions;
     RAW_EXTENSION *pre_proc_exts;
 } CLIENTHELLO_MSG;
 

ssl/statem/extensions_clnt.c

@@ -1036,7 +1036,6 @@ int tls_parse_server_key_share(SSL *s, PACKET *pkt, int *al)
 
 static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
 {
-    size_t num_extensions = 0;
     RAW_EXTENSION *extensions = NULL;
     PACKET extpkt;
 
@@ -1071,7 +1070,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
                                             | EXT_TLS1_3_SERVER_HELLO
                                             | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
                                             | EXT_TLS1_3_CERTIFICATE,
-                                &extensions, &num_extensions, al))
+                                &extensions, al))
         return 0;
 
     /*
@@ -1083,8 +1082,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
      */
     if (!(s->options & SSL_OP_LEGACY_SERVER_CONNECT)
             && !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)
-            && tls_get_extension_by_type(extensions, num_extensions,
-                                         TLSEXT_TYPE_renegotiate) == NULL) {
+            && !extensions[TLSEXT_IDX_renegotiate].present) {
         *al = SSL_AD_HANDSHAKE_FAILURE;
         SSLerr(SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT,
                SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
@@ -1095,7 +1093,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
                                      | EXT_TLS1_3_SERVER_HELLO
                                      | EXT_TLS1_3_ENCRYPTED_EXTENSIONS
                                      | EXT_TLS1_3_CERTIFICATE,
-                                  extensions, num_extensions, al))
+                                  extensions,al))
         return 0;
 
     if (s->hit) {

ssl/statem/extensions_srvr.c

@@ -522,9 +522,7 @@ int tls_parse_client_key_share(SSL *s, PACKET *pkt, int *al)
     }
 
     /*
-     * Get the clients list of supported curves. Note: We rely on the fact
-     * extension parsing happens in order of type. supported_groups has a lower
-     * type than key_share so will have been processed first.
+     * Get the clients list of supported curves.
      * TODO(TLS1.3): We should validate that we actually received
      * supported_groups!
      */

ssl/statem/statem_lib.c

@@ -948,11 +948,9 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello)
         break;
     }
 
-    suppversions = tls_get_extension_by_type(hello->pre_proc_exts,
-                                             hello->num_extensions,
-                                             TLSEXT_TYPE_supported_versions);
+    suppversions = &hello->pre_proc_exts[TLSEXT_IDX_supported_versions];
 
-    if (suppversions != NULL && !SSL_IS_DTLS(s)) {
+    if (suppversions->present && !SSL_IS_DTLS(s)) {
         unsigned int candidate_vers = 0;
         unsigned int best_vers = 0;
         const SSL_METHOD *best_method = NULL;

ssl/statem/statem_locl.h

@@ -27,13 +27,17 @@
 #define FINISHED_MAX_LENGTH             64
 
 /* Extension context codes */
+/* This extension is only allowed in TLS */
 #define EXT_TLS_ONLY                        0x0001
+/* This extension is only allowed in DTLS */
 #define EXT_DTLS_ONLY                       0x0002
 /* Some extensions may be allowed in DTLS but we don't implement them for it */
 #define EXT_TLS_IMPLEMENTATION_ONLY         0x0004
 /* Most extensions are not defined for SSLv3 but EXT_TYPE_renegotiate is */
 #define EXT_SSL3_ALLOWED                    0x0008
+/* Extension is only defined for TLS1.2 and above */
 #define EXT_TLS1_2_AND_BELOW_ONLY           0x0010
+/* Extension is only defined for TLS1.3 and above */
 #define EXT_TLS1_3_ONLY                     0x0020
 #define EXT_CLIENT_HELLO                    0x0040
 /* Really means TLS1.2 or below */
@@ -106,9 +110,6 @@ __owur int tls_construct_finished(SSL *s, WPACKET *pkt);
 __owur WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst);
 __owur WORK_STATE dtls_wait_for_dry(SSL *s);
 
-int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
-                           RAW_EXTENSION **res, size_t *numfound, int *ad);
-
 /* some client-only functions */
 __owur int tls_construct_client_hello(SSL *s, WPACKET *pkt);
 __owur MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt);
@@ -149,10 +150,15 @@ __owur MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt);
 #endif
 __owur int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt);
 
+
+/* Extension processing */
+
+__owur int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
+                                  RAW_EXTENSION **res, int *al);
+__owur int tls_parse_extension(SSL *s, unsigned int idx, int context,
+                               RAW_EXTENSION *exts, int *al);
 __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);
+                                    int *al);
 __owur int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
                                     int *al);
 

ssl/statem/statem_srvr.c

@@ -1315,8 +1315,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     /* Preserve the raw extensions PACKET for later use */
     extensions = clienthello.extensions;
     if (!tls_collect_extensions(s, &extensions, EXT_CLIENT_HELLO,
-                                &clienthello.pre_proc_exts,
-                                &clienthello.num_extensions, &al)) {
+                                &clienthello.pre_proc_exts, &al)) {
         /* SSLerr already been called */
         goto f_err;
     }
@@ -1401,10 +1400,9 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     s->hit = 0;
 
     /* We need to do this before getting the session */
-    if (!tls_parse_extension(s, TLSEXT_TYPE_extended_master_secret,
+    if (!tls_parse_extension(s, TLSEXT_IDX_extended_master_secret,
                              EXT_CLIENT_HELLO,
-                             clienthello.pre_proc_exts,
-                             clienthello.num_extensions, &al)) {
+                             clienthello.pre_proc_exts, &al)) {
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT);
         goto f_err;
     }
@@ -1508,8 +1506,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
 
     /* TLS extensions */
     if (!tls_parse_all_extensions(s, EXT_CLIENT_HELLO,
-                                  clienthello.pre_proc_exts,
-                                  clienthello.num_extensions, &al)) {
+                                  clienthello.pre_proc_exts, &al)) {
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_PARSE_TLSEXT);
         goto f_err;
     }

ssl/t1_lib.c

@@ -1062,7 +1062,6 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
                                SSL_SESSION **ret)
 {
     int retv;
-    const unsigned char *etick;
     size_t size;
     RAW_EXTENSION *ticketext;
 
@@ -1077,14 +1076,10 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
     if (s->version <= SSL3_VERSION || !tls_use_ticket(s))
         return 0;
 
-    ticketext = tls_get_extension_by_type(hello->pre_proc_exts,
-                                          hello->num_extensions,
-                                          TLSEXT_TYPE_session_ticket);
-    if (ticketext == NULL)
+    ticketext = &hello->pre_proc_exts[TLSEXT_IDX_session_ticket];
+    if (!ticketext->present)
         return 0;
 
-    ticketext->parsed = 1;
-
     size = PACKET_remaining(&ticketext->data);
     if (size == 0) {
         /*
@@ -1103,12 +1098,9 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
          */
         return 2;
     }
-    if (!PACKET_get_bytes(&ticketext->data, &etick, size)) {
-        /* Shouldn't ever happen */
-        return -1;
-    }
-    retv = tls_decrypt_ticket(s, etick, size, hello->session_id,
-                           hello->session_id_len, ret);
+
+    retv = tls_decrypt_ticket(s, PACKET_data(&ticketext->data), size,
+                              hello->session_id, hello->session_id_len, ret);
     switch (retv) {
     case 2:            /* ticket couldn't be decrypted */
         s->tlsext_ticket_expected = 1;

util/TLSProxy/Message.pm

@@ -67,9 +67,10 @@ use constant {
     EXT_SESSION_TICKET => 35,
     EXT_SUPPORTED_VERSIONS => 43,
     EXT_KEY_SHARE => 40,
-    # This extension does not exist and isn't recognised by OpenSSL.
-    # We use it to test handling of duplicate extensions.
-    EXT_DUPLICATE_EXTENSION => 1234
+    # This extension is an unofficial extension only ever written by OpenSSL
+    # (i.e. not read), and even then only when enabled. We use it to test
+    # handling of duplicate extensions.
+    EXT_DUPLICATE_EXTENSION => 0xfde8
 };
 
 my $payload = "";