Fix some TODO(3.0) occurrences in ssl/t1_lib.c

One was related to probing for the combination of signature and hash
algorithm together. This is currently not easily possible. The TODO(3.0)
is converted to a normal comment and I've raised the problem as issue
number #14885 as something to resolve post 3.0.

The other TODO was a hard coded limit on the number of groups that could
be registered. This has been amended so that there is no limit.

Fixes #14333

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14886)

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 31873a3fa28cb39d49a32b80fdd7dc12c780bfd1..14c16e355d8473a9eb4a52bdc09ee6e4ad62e316 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -691,13 +691,13 @@ err:
     return 0;
 }
 
-/* TODO(3.0): An arbitrary amount for now. Take another look at this */
-# define MAX_GROUPLIST   40
+# define GROUPLIST_INCREMENT   40
 # define GROUP_NAME_BUFFER_LENGTH 64
 typedef struct {
     SSL_CTX *ctx;
     size_t gidcnt;
-    uint16_t gid_arr[MAX_GROUPLIST];
+    size_t gidmax;
+    uint16_t *gid_arr;
 } gid_cb_st;
 
 static int gid_cb(const char *elem, int len, void *arg)
@@ -709,8 +709,14 @@ static int gid_cb(const char *elem, int len, void *arg)
 
     if (elem == NULL)
         return 0;
-    if (garg->gidcnt == MAX_GROUPLIST)
-        return 0;
+    if (garg->gidcnt == garg->gidmax) {
+        uint16_t *tmp =
+            OPENSSL_realloc(garg->gid_arr, garg->gidmax + GROUPLIST_INCREMENT);
+        if (tmp == NULL)
+            return 0;
+        garg->gidmax += GROUPLIST_INCREMENT;
+        garg->gid_arr = tmp;
+    }
     if (len > (int)(sizeof(etmp) - 1))
         return 0;
     memcpy(etmp, elem, len);
@@ -732,13 +738,20 @@ int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
 {
     gid_cb_st gcb;
     uint16_t *tmparr;
+    int ret = 0;
 
     gcb.gidcnt = 0;
+    gcb.gidmax = GROUPLIST_INCREMENT;
+    gcb.gid_arr = OPENSSL_malloc(gcb.gidmax * sizeof(*gcb.gid_arr));
+    if (gcb.gid_arr == NULL)
+        return 0;
     gcb.ctx = ctx;
     if (!CONF_parse_list(str, ':', 1, gid_cb, &gcb))
-        return 0;
-    if (pext == NULL)
-        return 1;
+        goto end;
+    if (pext == NULL) {
+        ret = 1;
+        goto end;
+    }
 
     /*
      * gid_cb ensurse there are no duplicates so we can just go ahead and set
@@ -746,10 +759,13 @@ int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
      */
     tmparr = OPENSSL_memdup(gcb.gid_arr, gcb.gidcnt * sizeof(*tmparr));
     if (tmparr == NULL)
-        return 0;
+        goto end;
     *pext = tmparr;
     *pextlen = gcb.gidcnt;
-    return 1;
+    ret = 1;
+ end:
+    OPENSSL_free(gcb.gid_arr);
+    return ret;
 }
 
 /* Check a group id matches preferences */
@@ -1142,7 +1158,7 @@ int ssl_setup_sig_algs(SSL_CTX *ctx)
 
         /*
          * Check hash is available.
-         * TODO(3.0): This test is not perfect. A provider could have support
+         * This test is not perfect. A provider could have support
          * for a signature scheme, but not a particular hash. However the hash
          * could be available from some other loaded provider. In that case it
          * could be that the signature is available, and the hash is available

diff --git a/test/tls-provider.c b/test/tls-provider.c
index 482c3aa0daefc55adccdd4f47c96d8f92c7ad4fe..d9d52664b2b337e7e899eeed96ba1d587a503f85 100644 (file)
--- a/test/tls-provider.c
+++ b/test/tls-provider.c
@@ -14,6 +14,7 @@
 #include <openssl/params.h>
 /* For TLS1_3_VERSION */
 #include <openssl/ssl.h>
+#include <internal/nelem.h>
 
 static OSSL_FUNC_keymgmt_import_fn xor_import;
 static OSSL_FUNC_keymgmt_import_types_fn xor_import_types;
@@ -167,16 +168,52 @@ static const OSSL_PARAM xor_kemgroup_params[] = {
     OSSL_PARAM_END
 };
 
+#define NUM_DUMMY_GROUPS 50
+static char *dummy_group_names[NUM_DUMMY_GROUPS];
 
 static int tls_prov_get_capabilities(void *provctx, const char *capability,
                                      OSSL_CALLBACK *cb, void *arg)
 {
-    if (strcmp(capability, "TLS-GROUP") == 0)
-        return cb(xor_group_params, arg)
-            && cb(xor_kemgroup_params, arg);
+    int ret;
+    int i;
+    const char *dummy_base = "dummy";
+    const size_t dummy_name_max_size = strlen(dummy_base) + 3;
+
+    if (strcmp(capability, "TLS-GROUP") != 0) {
+        /* We don't support this capability */
+        return 0;
+    }
+
+    /* Register our 2 groups */
+    ret = cb(xor_group_params, arg);
+    ret &= cb(xor_kemgroup_params, arg);
+
+    /*
+     * Now register some dummy groups > GROUPLIST_INCREMENT (== 40) as defined
+     * in ssl/t1_lib.c, to make sure we exercise the code paths for registering
+     * large numbers of groups.
+     */
+
+    for (i = 0; i < NUM_DUMMY_GROUPS; i++) {
+        OSSL_PARAM dummygroup[OSSL_NELEM(xor_group_params)];
+
+        memcpy(dummygroup, xor_group_params, sizeof(xor_group_params));
 
-    /* We don't support this capability */
-    return 0;
+        /* Give the dummy group a unique name */
+        if (dummy_group_names[i] == NULL) {
+            dummy_group_names[i] = OPENSSL_zalloc(dummy_name_max_size);
+            if (dummy_group_names[i] == NULL)
+                return 0;
+            BIO_snprintf(dummy_group_names[i],
+                         dummy_name_max_size,
+                         "%s%d", dummy_base, i);
+        }
+        dummygroup[0].data = dummy_group_names[i];
+        dummygroup[0].data_size = strlen(dummy_group_names[i]) + 1;
+        ret &= cb(dummygroup, arg);
+    }
+
+    return ret;
 }
 
 /*
@@ -743,9 +780,21 @@ static const OSSL_ALGORITHM *tls_prov_query(void *provctx, int operation_id,
     return NULL;
 }
 
+static void tls_prov_teardown(void *provctx)
+{
+    int i;
+
+    OSSL_LIB_CTX_free(provctx);
+
+    for (i = 0; i < NUM_DUMMY_GROUPS; i++) {
+        OPENSSL_free(dummy_group_names[i]);
+        dummy_group_names[i] = NULL;
+    }
+}
+
 /* Functions we provide to the core */
 static const OSSL_DISPATCH tls_prov_dispatch_table[] = {
-    { OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))OSSL_LIB_CTX_free },
+    { OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))tls_prov_teardown },
     { OSSL_FUNC_PROVIDER_QUERY_OPERATION, (void (*)(void))tls_prov_query },
     { OSSL_FUNC_PROVIDER_GET_CAPABILITIES, (void (*)(void))tls_prov_get_capabilities },
     { 0, NULL }