!101 fix CVE-2023-0464 CVE-2023-0465 CVE-2023-0466

Merge pull request !101 from code4lala/OpenHarmony-3.2-Release

CHANGES

@@ -8,6 +8,10 @@
  release branch.
 
  Changes between 1.1.1s and 1.1.1t [xx XXX xxxx]
+  *) Corrected documentation of X509_VERIFY_PARAM_add0_policy() to mention
+     that it does not enable policy checking. Thanks to
+     David Benjamin for discovering this issue. (CVE-2023-0466)
+     [Tomas Mraz]
 
   *) Fixed a type confusion vulnerability relating to X.400 address processing
      inside an X.509 GeneralName. X.400 addresses were parsed as an ASN1_STRING

NEWS

@@ -4,6 +4,7 @@
 
   This file gives a brief overview of the major changes between each OpenSSL
   release. For more details please read the CHANGES file.
+      o Fixed documentation of X509_VERIFY_PARAM_add0_policy() (CVE-2023-0466)
 
   Major changes between OpenSSL 1.1.1m and OpenSSL 1.1.1n [15 Mar 2022]
 

crypto/x509/x509_vfy.c

@@ -1649,18 +1649,25 @@ static int check_policy(X509_STORE_CTX *ctx)
     }
     /* Invalid or inconsistent extensions */
     if (ret == X509_PCY_TREE_INVALID) {
-        int i;
+        int i, cbcalled = 0;
 
         /* Locate certificates with bad extensions and notify callback. */
-        for (i = 1; i < sk_X509_num(ctx->chain); i++) {
+        for (i = 0; i < sk_X509_num(ctx->chain); i++) {
             X509 *x = sk_X509_value(ctx->chain, i);
 
             if (!(x->ex_flags & EXFLAG_INVALID_POLICY))
                 continue;
+            cbcalled = 1;
             if (!verify_cb_cert(ctx, x, i,
                                 X509_V_ERR_INVALID_POLICY_EXTENSION))
                 return 0;
         }
+        if (!cbcalled) {
+            /* Should not be able to get here */
+            X509err(X509_F_CHECK_POLICY, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
+        /* The callback ignored the error so we return success */
         return 1;
     }
     if (ret == X509_PCY_TREE_FAILURE) {

crypto/x509v3/pcy_local.h

@@ -111,6 +111,11 @@ struct X509_POLICY_LEVEL_st {
 };
 
 struct X509_POLICY_TREE_st {
+    /* The number of nodes in the tree */
+    size_t node_count;
+    /* The maximum number of nodes in the tree */
+    size_t node_maximum;
+
     /* This is the tree 'level' data */
     X509_POLICY_LEVEL *levels;
     int nlevel;
@@ -159,7 +164,8 @@ X509_POLICY_NODE *tree_find_sk(STACK_OF(X509_POLICY_NODE) *sk,
 X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level,
                                  X509_POLICY_DATA *data,
                                  X509_POLICY_NODE *parent,
-                                 X509_POLICY_TREE *tree);
+                                 X509_POLICY_TREE *tree,
+                                 int extra_data);
 void policy_node_free(X509_POLICY_NODE *node);
 int policy_node_match(const X509_POLICY_LEVEL *lvl,
                       const X509_POLICY_NODE *node, const ASN1_OBJECT *oid);

crypto/x509v3/pcy_node.c

@@ -59,10 +59,15 @@ X509_POLICY_NODE *level_find_node(const X509_POLICY_LEVEL *level,
 X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level,
                                  X509_POLICY_DATA *data,
                                  X509_POLICY_NODE *parent,
-                                 X509_POLICY_TREE *tree)
+                                 X509_POLICY_TREE *tree,
+                                 int extra_data)
 {
     X509_POLICY_NODE *node;
 
+    /* Verify that the tree isn't too large.  This mitigates CVE-2023-0464 */
+    if (tree->node_maximum > 0 && tree->node_count >= tree->node_maximum)
+        return NULL;
+
     node = OPENSSL_zalloc(sizeof(*node));
     if (node == NULL) {
         X509V3err(X509V3_F_LEVEL_ADD_NODE, ERR_R_MALLOC_FAILURE);
@@ -70,7 +75,7 @@ X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level,
     }
     node->data = data;
     node->parent = parent;
-    if (level) {
+    if (level != NULL) {
         if (OBJ_obj2nid(data->valid_policy) == NID_any_policy) {
             if (level->anyPolicy)
                 goto node_error;
@@ -90,7 +95,7 @@ X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level,
         }
     }
 
-    if (tree) {
+    if (extra_data) {
         if (tree->extra_data == NULL)
             tree->extra_data = sk_X509_POLICY_DATA_new_null();
         if (tree->extra_data == NULL){
@@ -103,6 +108,7 @@ X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level,
         }
     }
 
+    tree->node_count++;
     if (parent)
         parent->nchild++;
 

crypto/x509v3/pcy_tree.c

@@ -13,6 +13,18 @@
 
 #include "pcy_local.h"
 
+/*
+ * If the maximum number of nodes in the policy tree isn't defined, set it to
+ * a generous default of 1000 nodes.
+ *
+ * Defining this to be zero means unlimited policy tree growth which opens the
+ * door on CVE-2023-0464.
+ */
+
+#ifndef OPENSSL_POLICY_TREE_NODES_MAX
+# define OPENSSL_POLICY_TREE_NODES_MAX 1000
+#endif
+
 /*
  * Enable this to print out the complete policy tree at various point during
  * evaluation.
@@ -168,6 +180,9 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
         return X509_PCY_TREE_INTERNAL;
     }
 
+    /* Limit the growth of the tree to mitigate CVE-2023-0464 */
+    tree->node_maximum = OPENSSL_POLICY_TREE_NODES_MAX;
+
     /*
      * http://tools.ietf.org/html/rfc5280#section-6.1.2, figure 3.
      *
@@ -184,7 +199,7 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
     level = tree->levels;
     if ((data = policy_data_new(NULL, OBJ_nid2obj(NID_any_policy), 0)) == NULL)
         goto bad_tree;
-    if (level_add_node(level, data, NULL, tree) == NULL) {
+    if (level_add_node(level, data, NULL, tree, 1) == NULL) {
         policy_data_free(data);
         goto bad_tree;
     }
@@ -243,7 +258,8 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
  * Return value: 1 on success, 0 otherwise
  */
 static int tree_link_matching_nodes(X509_POLICY_LEVEL *curr,
-                                    X509_POLICY_DATA *data)
+                                    X509_POLICY_DATA *data,
+                                    X509_POLICY_TREE *tree)
 {
     X509_POLICY_LEVEL *last = curr - 1;
     int i, matched = 0;
@@ -253,13 +269,13 @@ static int tree_link_matching_nodes(X509_POLICY_LEVEL *curr,
         X509_POLICY_NODE *node = sk_X509_POLICY_NODE_value(last->nodes, i);
 
         if (policy_node_match(last, node, data->valid_policy)) {
-            if (level_add_node(curr, data, node, NULL) == NULL)
+            if (level_add_node(curr, data, node, tree, 0) == NULL)
                 return 0;
             matched = 1;
         }
     }
     if (!matched && last->anyPolicy) {
-        if (level_add_node(curr, data, last->anyPolicy, NULL) == NULL)
+        if (level_add_node(curr, data, last->anyPolicy, tree, 0) == NULL)
             return 0;
     }
     return 1;
@@ -272,7 +288,8 @@ static int tree_link_matching_nodes(X509_POLICY_LEVEL *curr,
  * Return value: 1 on success, 0 otherwise.
  */
 static int tree_link_nodes(X509_POLICY_LEVEL *curr,
-                           const X509_POLICY_CACHE *cache)
+                           const X509_POLICY_CACHE *cache,
+                           X509_POLICY_TREE *tree)
 {
     int i;
 
@@ -280,7 +297,7 @@ static int tree_link_nodes(X509_POLICY_LEVEL *curr,
         X509_POLICY_DATA *data = sk_X509_POLICY_DATA_value(cache->data, i);
 
         /* Look for matching nodes in previous level */
-        if (!tree_link_matching_nodes(curr, data))
+        if (!tree_link_matching_nodes(curr, data, tree))
             return 0;
     }
     return 1;
@@ -311,7 +328,7 @@ static int tree_add_unmatched(X509_POLICY_LEVEL *curr,
     /* Curr may not have anyPolicy */
     data->qualifier_set = cache->anyPolicy->qualifier_set;
     data->flags |= POLICY_DATA_FLAG_SHARED_QUALIFIERS;
-    if (level_add_node(curr, data, node, tree) == NULL) {
+    if (level_add_node(curr, data, node, tree, 1) == NULL) {
         policy_data_free(data);
         return 0;
     }
@@ -373,7 +390,7 @@ static int tree_link_any(X509_POLICY_LEVEL *curr,
     }
     /* Finally add link to anyPolicy */
     if (last->anyPolicy &&
-        level_add_node(curr, cache->anyPolicy, last->anyPolicy, NULL) == NULL)
+        level_add_node(curr, cache->anyPolicy, last->anyPolicy, tree, 0) == NULL)
         return 0;
     return 1;
 }
@@ -555,7 +572,7 @@ static int tree_calculate_user_set(X509_POLICY_TREE *tree,
             extra->qualifier_set = anyPolicy->data->qualifier_set;
             extra->flags = POLICY_DATA_FLAG_SHARED_QUALIFIERS
                 | POLICY_DATA_FLAG_EXTRA_NODE;
-            node = level_add_node(NULL, extra, anyPolicy->parent, tree);
+            node = level_add_node(NULL, extra, anyPolicy->parent, tree, 1);
         }
         if (!tree->user_policies) {
             tree->user_policies = sk_X509_POLICY_NODE_new_null();
@@ -582,7 +599,7 @@ static int tree_evaluate(X509_POLICY_TREE *tree)
 
     for (i = 1; i < tree->nlevel; i++, curr++) {
         cache = policy_cache_set(curr->cert);
-        if (!tree_link_nodes(curr, cache))
+        if (!tree_link_nodes(curr, cache, tree))
             return X509_PCY_TREE_INTERNAL;
 
         if (!(curr->flags & X509_V_FLAG_INHIBIT_ANY)

doc/man3/X509_VERIFY_PARAM_set_flags.pod

@@ -92,8 +92,9 @@ B<trust>.
 X509_VERIFY_PARAM_set_time() sets the verification time in B<param> to
 B<t>. Normally the current time is used.
 
-X509_VERIFY_PARAM_add0_policy() enables policy checking (it is disabled
-by default) and adds B<policy> to the acceptable policy set.
+X509_VERIFY_PARAM_add0_policy() adds B<policy> to the acceptable policy set.
+Contrary to preexisting documentation of this function it does not enable
+policy checking.
 
 X509_VERIFY_PARAM_set1_policies() enables policy checking (it is disabled
 by default) and sets the acceptable policy set to B<policies>. Any existing
@@ -377,6 +378,10 @@ and has no effect.
 
 The X509_VERIFY_PARAM_get_hostflags() function was added in OpenSSL 1.1.0i.
 
+The function X509_VERIFY_PARAM_add0_policy() was historically documented as
+enabling policy checking however the implementation has never done this.
+The documentation was changed to align with the implementation.
+
 =head1 COPYRIGHT
 
 Copyright 2009-2020 The OpenSSL Project Authors. All Rights Reserved.