Long overdue cleanup of X509 policy tree verification

Replace all magic numbers with #defined constants except in boolean
functions that return 0 for failure and 1 for success.  Avoid a
couple memory leaks in error recovery code paths.  Code style
improvements.

Reviewed-by: Dr. Stephen Henson <steve@openssl.org>

3 files changed, 224 insertions, 247 deletions

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 1f3b2b9dab..3438692e57 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1505,12 +1505,12 @@ static int check_policy(X509_STORE_CTX *ctx)
         return 1;
     ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain,
                             ctx->param->policies, ctx->param->flags);
-    if (ret == 0) {
+    if (ret == X509_PCY_TREE_INTERNAL) {
         X509err(X509_F_CHECK_POLICY, ERR_R_MALLOC_FAILURE);
         return 0;
     }
     /* Invalid or inconsistent extensions */
-    if (ret == -1) {
+    if (ret == X509_PCY_TREE_INVALID) {
         /*
          * Locate certificates with bad extensions and notify callback.
          */
@@ -1527,11 +1527,15 @@ static int check_policy(X509_STORE_CTX *ctx)
         }
         return 1;
     }
-    if (ret == -2) {
+    if (ret == X509_PCY_TREE_FAILURE) {
         ctx->current_cert = NULL;
         ctx->error = X509_V_ERR_NO_EXPLICIT_POLICY;
         return ctx->verify_cb(0, ctx);
     }
+    if (ret != X509_PCY_TREE_VALID) {
+        X509err(X509_F_CHECK_POLICY, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
 
     if (ctx->param->flags & X509_V_FLAG_NOTIFY_POLICY) {
         ctx->current_cert = NULL;
diff --git a/crypto/x509v3/pcy_node.c b/crypto/x509v3/pcy_node.c
index e8007c23f9..581c246b74 100644
--- a/crypto/x509v3/pcy_node.c
+++ b/crypto/x509v3/pcy_node.c
@@ -151,8 +151,7 @@ X509_POLICY_NODE *level_add_node(X509_POLICY_LEVEL *level,
 
  node_error:
     policy_node_free(node);
-    return 0;
-
+    return NULL;
 }
 
 void policy_node_free(X509_POLICY_NODE *node)
diff --git a/crypto/x509v3/pcy_tree.c b/crypto/x509v3/pcy_tree.c
index cac2d51dc3..b3995d64e5 100644
--- a/crypto/x509v3/pcy_tree.c
+++ b/crypto/x509v3/pcy_tree.c
@@ -97,24 +97,26 @@ static void expected_print(BIO *err, X509_POLICY_LEVEL *lev,
 static void tree_print(char *str, X509_POLICY_TREE *tree,
                        X509_POLICY_LEVEL *curr)
 {
+    BIO *err = BIO_new_fp(stderr, BIO_NOCLOSE);
     X509_POLICY_LEVEL *plev;
-    X509_POLICY_NODE *node;
-    int i;
-    BIO *err;
-    err = BIO_new_fp(stderr, BIO_NOCLOSE);
+
     if (err == NULL)
         return;
     if (!curr)
         curr = tree->levels + tree->nlevel;
     else
         curr++;
+
     BIO_printf(err, "Level print after %s\n", str);
     BIO_printf(err, "Printing Up to Level %ld\n", curr - tree->levels);
     for (plev = tree->levels; plev != curr; plev++) {
+        int i;
+
         BIO_printf(err, "Level %ld, flags = %x\n",
-                   plev - tree->levels, plev->flags);
+                   (long)(plev - tree->levels), plev->flags);
         for (i = 0; i < sk_X509_POLICY_NODE_num(plev->nodes); i++) {
-            node = sk_X509_POLICY_NODE_value(plev->nodes, i);
+            X509_POLICY_NODE *node = sk_X509_POLICY_NODE_value(plev->nodes, i);
+
             X509_POLICY_NODE_print(err, node, 2);
             expected_print(err, plev, node, 2);
             BIO_printf(err, "  Flags: %x\n", node->data->flags);
@@ -122,26 +124,17 @@ static void tree_print(char *str, X509_POLICY_TREE *tree,
         if (plev->anyPolicy)
             X509_POLICY_NODE_print(err, plev->anyPolicy, 2);
     }
-
     BIO_free(err);
-
 }
-#else
-
-# define tree_print(a,b,c)      /* */
-
 #endif
 
 /*-
- * Initialize policy tree. Return values:
- *  0 Some internal error occurred.
- * -1 Inconsistent or invalid extensions in certificates.
- *  1 Tree initialized OK.
- *  2 Policy tree is empty.
- *  5 Tree OK and requireExplicitPolicy true.
- *  6 Tree empty and requireExplicitPolicy true.
+ * Return value: <= 0 on error, or positive bit mask:
+ *
+ * X509_PCY_TREE_VALID: valid tree
+ * X509_PCY_TREE_EMPTY: empty tree (including bare TA case)
+ * X509_PCY_TREE_EXPLICIT: explicit policy required
  */
-
 static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
                      unsigned int flags)
 {
@@ -149,103 +142,112 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
     X509_POLICY_LEVEL *level;
     const X509_POLICY_CACHE *cache;
     X509_POLICY_DATA *data = NULL;
-    X509 *x;
-    int ret = 1;
-    int i, n;
-    int explicit_policy;
-    int any_skip;
-    int map_skip;
+    int ret = X509_PCY_TREE_VALID;
+    int n = sk_X509_num(certs) - 1; /* RFC5280 paths omit the TA */
+    int explicit_policy = (flags & X509_V_FLAG_EXPLICIT_POLICY) ? 0 : n+1;
+    int any_skip = (flags & X509_V_FLAG_INHIBIT_ANY) ? 0 : n+1;
+    int map_skip = (flags & X509_V_FLAG_INHIBIT_MAP) ? 0 : n+1;
+    int i;
 
     *ptree = NULL;
-    n = sk_X509_num(certs);
 
-    if (flags & X509_V_FLAG_EXPLICIT_POLICY)
-        explicit_policy = 0;
-    else
-        explicit_policy = n + 1;
+    /* Can't do anything with just a trust anchor */
+    if (n == 0)
+        return X509_PCY_TREE_EMPTY;
 
-    if (flags & X509_V_FLAG_INHIBIT_ANY)
-        any_skip = 0;
-    else
-        any_skip = n + 1;
+    /*
+     * First setup the policy cache in all n non-TA certificates, this will be
+     * used in X509_verify_cert() which will invoke the verify callback for all
+     * certificates with invalid policy extensions.
+     */
+    for (i = n - 1; i >= 0; i--) {
+        X509 *x = sk_X509_value(certs, i);
 
-    if (flags & X509_V_FLAG_INHIBIT_MAP)
-        map_skip = 0;
-    else
-        map_skip = n + 1;
+        /* Call for side-effect of computing hash and caching extensions */
+        X509_check_purpose(x, -1, 0);
+
+        /* If cache is NULL, likely ENOMEM: return immediately */
+        if ((cache = policy_cache_set(x)) == NULL)
+            return X509_PCY_TREE_INTERNAL;
+    }
 
-    /* Can't do anything with just a trust anchor */
-    if (n == 1)
-        return 1;
     /*
-     * First setup policy cache in all certificates apart from the trust
-     * anchor. Note any bad cache results on the way. Also can calculate
-     * explicit_policy value at this point.
+     * At this point check for invalid policies and required explicit policy.
+     * Note that the explicit_policy counter is a count-down to zero, with the
+     * requirement kicking in if and once it does that.  The counter is
+     * decremented for every non-self-issued certificate in the path, but may
+     * be further reduced by policy constraints in a non-leaf certificate.
+     *
+     * The ultimate policy set is the interesection of all the policies along
+     * the path, if we hit a certificate with an empty policy set, and explicit
+     * policy is required we're done.
      */
-    for (i = n - 2; i >= 0; i--) {
-        uint32_t ex_flags;
-        x = sk_X509_value(certs, i);
+    for (i = n - 1;
+         i >= 0 && (explicit_policy > 0 || (ret & X509_PCY_TREE_EMPTY) == 0);
+         i--) {
+        X509 *x = sk_X509_value(certs, i);
+        uint32_t ex_flags = X509_get_extension_flags(x);
 
-        /*
-         * Note, this modifies x->ex_flags.  If cache NULL something bad
-         * happened: return immediately
-         */
-        cache = policy_cache_set(x);
-        if (cache == NULL)
-            return 0;
-        /*
-         * If inconsistent extensions keep a note of it but continue
-         */
-        ex_flags = X509_get_extension_flags(x);
+        /* All the policies are already cached, we can return early */
         if (ex_flags & EXFLAG_INVALID_POLICY)
-            ret = -1;
-        /*
-         * Otherwise if we have no data (hence no CertificatePolicies) and
-         * haven't already set an inconsistent code note it.
-         */
-        else if ((ret == 1) && !cache->data)
-            ret = 2;
+            return X509_PCY_TREE_INVALID;
+
+        /* Access the cache which we now know exists */
+        cache = policy_cache_set(x);
+
+        if ((ret & X509_PCY_TREE_VALID) && cache->data == NULL)
+            ret = X509_PCY_TREE_EMPTY;
         if (explicit_policy > 0) {
             if (!(ex_flags & EXFLAG_SI))
                 explicit_policy--;
-            if ((cache->explicit_skip != -1)
+            if ((cache->explicit_skip >= 0)
                 && (cache->explicit_skip < explicit_policy))
                 explicit_policy = cache->explicit_skip;
         }
     }
 
-    if (ret != 1) {
-        if (ret == 2 && !explicit_policy)
-            return 6;
+    if (explicit_policy == 0)
+        ret |= X509_PCY_TREE_EXPLICIT;
+    if ((ret & X509_PCY_TREE_VALID) == 0)
         return ret;
-    }
 
     /* If we get this far initialize the tree */
-    tree = OPENSSL_zalloc(sizeof(*tree));
-    if (tree == NULL)
-        return 0;
-    tree->levels = OPENSSL_zalloc(sizeof(*tree->levels) * n);
-    if (tree->levels == NULL) {
+    if ((tree = OPENSSL_zalloc(sizeof(*tree))) == NULL)
+        return X509_PCY_TREE_INTERNAL;
+
+    /*
+     * http://tools.ietf.org/html/rfc5280#section-6.1.2, figure 3.
+     *
+     * The top level is implicitly for the trust anchor with valid expected
+     * policies of anyPolicy.  (RFC 5280 has the TA at depth 0 and the leaf at
+     * depth n, we have the leaf at depth 0 and the TA at depth n).
+     */
+    if ((tree->levels = OPENSSL_zalloc(sizeof(*tree->levels)*(n+1))) == NULL) {
         OPENSSL_free(tree);
-        return 0;
+        return X509_PCY_TREE_INTERNAL;
     }
-    tree->nlevel = n;
+    tree->nlevel = n+1;
     level = tree->levels;
-
-    /* Root data: initialize to anyPolicy */
-    data = policy_data_new(NULL, OBJ_nid2obj(NID_any_policy), 0);
-
-    if (data == NULL || !level_add_node(level, data, NULL, tree))
+    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) {
+        policy_data_free(data);
+        goto bad_tree;
+    }
 
-    for (i = n - 2; i >= 0; i--) {
-        uint32_t ex_flags;
-        level++;
-        x = sk_X509_value(certs, i);
-        ex_flags = X509_get_extension_flags(x);
+    /*
+     * In this pass initialize all the tree levels and whether anyPolicy and
+     * policy mapping are inhibited at each level.
+     */
+    for (i = n - 1; i >= 0; i--) {
+        X509 *x = sk_X509_value(certs, i);
+        uint32_t ex_flags = X509_get_extension_flags(x);
+
+        /* Access the cache which we now know exists */
         cache = policy_cache_set(x);
+
         X509_up_ref(x);
-        level->cert = x;
+        (++level)->cert = x;
 
         if (!cache->anyPolicy)
             level->flags |= X509_V_FLAG_INHIBIT_ANY;
@@ -253,16 +255,15 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
         /* Determine inhibit any and inhibit map flags */
         if (any_skip == 0) {
             /*
-             * Any matching allowed if certificate is self issued and not the
-             * last in the chain.
+             * Any matching allowed only if certificate is self issued and not
+             * the last in the chain.
              */
             if (!(ex_flags & EXFLAG_SI) || (i == 0))
                 level->flags |= X509_V_FLAG_INHIBIT_ANY;
         } else {
             if (!(ex_flags & EXFLAG_SI))
                 any_skip--;
-            if ((cache->any_skip >= 0)
-                && (cache->any_skip < any_skip))
+            if ((cache->any_skip >= 0) && (cache->any_skip < any_skip))
                 any_skip = cache->any_skip;
         }
 
@@ -271,45 +272,40 @@ static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
         else {
             if (!(ex_flags & EXFLAG_SI))
                 map_skip--;
-            if ((cache->map_skip >= 0)
-                && (cache->map_skip < map_skip))
+            if ((cache->map_skip >= 0) && (cache->map_skip < map_skip))
                 map_skip = cache->map_skip;
         }
-
     }
 
     *ptree = tree;
-
-    if (explicit_policy)
-        return 1;
-    else
-        return 5;
+    return ret;
 
  bad_tree:
-
     X509_policy_tree_free(tree);
-
-    return 0;
-
+    return X509_PCY_TREE_INTERNAL;
 }
 
+/*
+ * Return value: 1 on success, 0 otherwise
+ */
 static int tree_link_matching_nodes(X509_POLICY_LEVEL *curr,
                                     X509_POLICY_DATA *data)
 {
     X509_POLICY_LEVEL *last = curr - 1;
-    X509_POLICY_NODE *node;
     int i, matched = 0;
+
     /* Iterate through all in nodes linking matches */
     for (i = 0; i < sk_X509_POLICY_NODE_num(last->nodes); i++) {
-        node = sk_X509_POLICY_NODE_value(last->nodes, i);
+        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))
+            if (level_add_node(curr, data, node, NULL) == NULL)
                 return 0;
             matched = 1;
         }
     }
     if (!matched && last->anyPolicy) {
-        if (!level_add_node(curr, data, last->anyPolicy, NULL))
+        if (level_add_node(curr, data, last->anyPolicy, NULL) == NULL)
             return 0;
     }
     return 1;
@@ -318,16 +314,17 @@ static int tree_link_matching_nodes(X509_POLICY_LEVEL *curr,
 /*
  * This corresponds to RFC3280 6.1.3(d)(1): link any data from
  * CertificatePolicies onto matching parent or anyPolicy if no match.
+ *
+ * Return value: 1 on success, 0 otherwise.
  */
-
 static int tree_link_nodes(X509_POLICY_LEVEL *curr,
                            const X509_POLICY_CACHE *cache)
 {
     int i;
-    X509_POLICY_DATA *data;
 
     for (i = 0; i < sk_X509_POLICY_DATA_num(cache->data); i++) {
-        data = sk_X509_POLICY_DATA_value(cache->data, i);
+        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))
             return 0;
@@ -338,35 +335,38 @@ static int tree_link_nodes(X509_POLICY_LEVEL *curr,
 /*
  * This corresponds to RFC3280 6.1.3(d)(2): Create new data for any unmatched
  * policies in the parent and link to anyPolicy.
+ *
+ * Return value: 1 on success, 0 otherwise.
  */
-
 static int tree_add_unmatched(X509_POLICY_LEVEL *curr,
                               const X509_POLICY_CACHE *cache,
                               const ASN1_OBJECT *id,
                               X509_POLICY_NODE *node, X509_POLICY_TREE *tree)
 {
     X509_POLICY_DATA *data;
+
     if (id == NULL)
         id = node->data->valid_policy;
     /*
      * Create a new node with qualifiers from anyPolicy and id from unmatched
      * node.
      */
-    data = policy_data_new(NULL, id, node_critical(node));
-
-    if (data == NULL)
+    if ((data = policy_data_new(NULL, id, node_critical(node))) == NULL)
         return 0;
+
     /* 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)) {
+    if (level_add_node(curr, data, node, tree) == NULL) {
         policy_data_free(data);
         return 0;
     }
-
     return 1;
 }
 
+/*
+ * Return value: 1 on success, 0 otherwise.
+ */
 static int tree_link_unmatched(X509_POLICY_LEVEL *curr,
                                const X509_POLICY_CACHE *cache,
                                X509_POLICY_NODE *node, X509_POLICY_TREE *tree)
@@ -397,11 +397,12 @@ static int tree_link_unmatched(X509_POLICY_LEVEL *curr,
         }
 
     }
-
     return 1;
-
 }
 
+/*
+ * Return value: 1 on success, 0 otherwise
+ */
 static int tree_link_any(X509_POLICY_LEVEL *curr,
                          const X509_POLICY_CACHE *cache,
                          X509_POLICY_TREE *tree)
@@ -417,19 +418,22 @@ static int tree_link_any(X509_POLICY_LEVEL *curr,
             return 0;
     }
     /* Finally add link to anyPolicy */
-    if (last->anyPolicy) {
-        if (!level_add_node(curr, cache->anyPolicy, last->anyPolicy, NULL))
-            return 0;
-    }
+    if (last->anyPolicy &&
+        level_add_node(curr, cache->anyPolicy, last->anyPolicy, NULL) == NULL)
+        return 0;
     return 1;
 }
 
-/*
- * Prune the tree: delete any child mapped child data on the current level
- * then proceed up the tree deleting any data with no children. If we ever
- * have no data on a level we can halt because the tree will be empty.
+/*-
+ * Prune the tree: delete any child mapped child data on the current level then
+ * proceed up the tree deleting any data with no children. If we ever have no
+ * data on a level we can halt because the tree will be empty.
+ *
+ * Return value: <= 0 error, otherwise one of:
+ *
+ * X509_PCY_TREE_VALID: valid tree
+ * X509_PCY_TREE_EMPTY: empty tree
  */
-
 static int tree_prune(X509_POLICY_TREE *tree, X509_POLICY_LEVEL *curr)
 {
     STACK_OF(X509_POLICY_NODE) *nodes;
@@ -468,41 +472,43 @@ static int tree_prune(X509_POLICY_TREE *tree, X509_POLICY_LEVEL *curr)
         if (curr == tree->levels) {
             /* If we zapped anyPolicy at top then tree is empty */
             if (!curr->anyPolicy)
-                return 2;
-            return 1;
+                return X509_PCY_TREE_EMPTY;
+            break;
         }
     }
-
-    /* Unreachable */
-
+    return X509_PCY_TREE_VALID;
 }
 
+/*
+ * Return value: 1 on success, 0 otherwise.
+ */
 static int tree_add_auth_node(STACK_OF(X509_POLICY_NODE) **pnodes,
                               X509_POLICY_NODE *pcy)
 {
-    if (*pnodes == NULL) {
-        *pnodes = policy_node_cmp_new();
-        if (*pnodes == NULL)
-            return 0;
-    } else if (sk_X509_POLICY_NODE_find(*pnodes, pcy) != -1)
-        return 1;
-
-    if (!sk_X509_POLICY_NODE_push(*pnodes, pcy))
+    if (*pnodes == NULL &&
+        (*pnodes = policy_node_cmp_new()) == NULL)
         return 0;
-
-    return 1;
-
+    if (sk_X509_POLICY_NODE_find(*pnodes, pcy) != -1)
+        return 1;
+    return sk_X509_POLICY_NODE_push(*pnodes, pcy) != 0;
 }
 
-/*
- * Calculate the authority set based on policy tree. The 'pnodes' parameter
- * is used as a store for the set of policy nodes used to calculate the user
- * set. If the authority set is not anyPolicy then pnodes will just point to
- * the authority set. If however the authority set is anyPolicy then the set
- * of valid policies (other than anyPolicy) is store in pnodes. The return
- * value of '2' is used in this case to indicate that pnodes should be freed.
- */
+#define TREE_CALC_FAILURE 0
+#define TREE_CALC_OK_NOFREE 1
+#define TREE_CALC_OK_DOFREE 2
 
+/*-
+ * Calculate the authority set based on policy tree. The 'pnodes' parameter is
+ * used as a store for the set of policy nodes used to calculate the user set.
+ * If the authority set is not anyPolicy then pnodes will just point to the
+ * authority set. If however the authority set is anyPolicy then the set of
+ * valid policies (other than anyPolicy) is store in pnodes.
+ *
+ * Return value:
+ *  TREE_CALC_FAILURE on failure,
+ *  TREE_CALC_OK_NOFREE on success and pnodes need not be freed,
+ *  TREE_CALC_OK_DOFREE on success and pnodes needs to be freed
+ */
 static int tree_calculate_authority_set(X509_POLICY_TREE *tree,
                                         STACK_OF(X509_POLICY_NODE) **pnodes)
 {
@@ -515,7 +521,7 @@ static int tree_calculate_authority_set(X509_POLICY_TREE *tree,
     /* If last level contains anyPolicy set is anyPolicy */
     if (curr->anyPolicy) {
         if (!tree_add_auth_node(&tree->auth_policies, curr->anyPolicy))
-            return 0;
+            return TREE_CALC_FAILURE;
         addnodes = pnodes;
     } else
         /* Add policies to authority set */
@@ -533,19 +539,25 @@ static int tree_calculate_authority_set(X509_POLICY_TREE *tree,
         for (j = 0; j < sk_X509_POLICY_NODE_num(curr->nodes); j++) {
             node = sk_X509_POLICY_NODE_value(curr->nodes, j);
             if ((node->parent == anyptr)
-                && !tree_add_auth_node(addnodes, node))
-                return 0;
+                && !tree_add_auth_node(addnodes, node)) {
+                if (addnodes == pnodes) {
+                    sk_X509_POLICY_NODE_free(*pnodes);
+                    *pnodes = NULL;
+                }
+                return TREE_CALC_FAILURE;
+            }
         }
     }
-
     if (addnodes == pnodes)
-        return 2;
+        return TREE_CALC_OK_DOFREE;
 
     *pnodes = tree->auth_policies;
-
-    return 1;
+    return TREE_CALC_OK_NOFREE;
 }
 
+/*
+ * Return value: 1 on success, 0 otherwise.
+ */
 static int tree_calculate_user_set(X509_POLICY_TREE *tree,
                                    STACK_OF(ASN1_OBJECT) *policy_oids,
                                    STACK_OF(X509_POLICY_NODE) *auth_nodes)
@@ -553,7 +565,6 @@ static int tree_calculate_user_set(X509_POLICY_TREE *tree,
     int i;
     X509_POLICY_NODE *node;
     ASN1_OBJECT *oid;
-
     X509_POLICY_NODE *anyPolicy;
     X509_POLICY_DATA *extra;
 
@@ -561,7 +572,6 @@ static int tree_calculate_user_set(X509_POLICY_TREE *tree,
      * Check if anyPolicy present in authority constrained policy set: this
      * will happen if it is a leaf node.
      */
-
     if (sk_ASN1_OBJECT_num(policy_oids) <= 0)
         return 1;
 
@@ -602,9 +612,14 @@ static int tree_calculate_user_set(X509_POLICY_TREE *tree,
             return 0;
     }
     return 1;
-
 }
 
+/*-
+ * Return value: <= 0 error, otherwise one of:
+ *  X509_PCY_TREE_VALID: valid tree
+ *  X509_PCY_TREE_EMPTY: empty tree
+ * (see tree_prune()).
+ */
 static int tree_evaluate(X509_POLICY_TREE *tree)
 {
     int ret, i;
@@ -614,19 +629,19 @@ 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))
-            return 0;
+            return X509_PCY_TREE_INTERNAL;
 
         if (!(curr->flags & X509_V_FLAG_INHIBIT_ANY)
             && !tree_link_any(curr, cache, tree))
-            return 0;
+            return X509_PCY_TREE_INTERNAL;
+#ifdef OPENSSL_POLICY_DEBUG
         tree_print("before tree_prune()", tree, curr);
+#endif
         ret = tree_prune(tree, curr);
-        if (ret != 1)
+        if (ret != X509_PCY_TREE_VALID)
             return ret;
     }
-
-    return 1;
-
+    return X509_PCY_TREE_VALID;
 }
 
 static void exnode_free(X509_POLICY_NODE *node)
@@ -661,111 +676,70 @@ void X509_policy_tree_free(X509_POLICY_TREE *tree)
 /*-
  * Application policy checking function.
  * Return codes:
- *  0   Internal Error.
- *  1   Successful.
- * -1   One or more certificates contain invalid or inconsistent extensions
- * -2   User constrained policy set empty and requireExplicit true.
+ *  X509_PCY_TREE_FAILURE:  Failure to satisfy explicit policy
+ *  X509_PCY_TREE_INVALID:  Inconsistent or invalid extensions
+ *  X509_PCY_TREE_INTERNAL: Internal error, most likely malloc
+ *  X509_PCY_TREE_VALID:    Success (null tree if empty or bare TA)
  */
-
 int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
                       STACK_OF(X509) *certs,
                       STACK_OF(ASN1_OBJECT) *policy_oids, unsigned int flags)
 {
+    int init_ret;
     int ret;
     X509_POLICY_TREE *tree = NULL;
     STACK_OF(X509_POLICY_NODE) *nodes, *auth_nodes = NULL;
-    *ptree = NULL;
 
+    *ptree = NULL;
     *pexplicit_policy = 0;
-    ret = tree_init(&tree, certs, flags);
-
-    switch (ret) {
-
-        /* Tree empty requireExplicit False: OK */
-    case 2:
-        return 1;
+    init_ret = tree_init(&tree, certs, flags);
 
-        /* Some internal error */
-    case -1:
-        return -1;
+    if (init_ret <= 0)
+        return init_ret;
 
-        /* Some internal error */
-    case 0:
-        return 0;
-
-        /* Tree empty requireExplicit True: Error */
-
-    case 6:
-        *pexplicit_policy = 1;
-        return -2;
-
-        /* Tree OK requireExplicit True: OK and continue */
-    case 5:
+    if ((init_ret & X509_PCY_TREE_EXPLICIT) == 0) {
+        if (init_ret & X509_PCY_TREE_EMPTY) {
+            X509_policy_tree_free(tree);
+            return X509_PCY_TREE_VALID;
+        }
+    } else {
         *pexplicit_policy = 1;
-        break;
-
-        /* Tree OK: continue */
-
-    case 1:
-        if (!tree)
-            /*
-             * tree_init() returns success and a null tree
-             * if it's just looking at a trust anchor.
-             * I'm not sure that returning success here is
-             * correct, but I'm sure that reporting this
-             * as an internal error which our caller
-             * interprets as a malloc failure is wrong.
-             */
-            return 1;
-        break;
+        /* Tree empty and requireExplicit True: Error */
+        if (init_ret & X509_PCY_TREE_EMPTY)
+            return X509_PCY_TREE_FAILURE;
     }
 
-    if (!tree)
-        goto error;
     ret = tree_evaluate(tree);
-
+#ifdef OPENSSL_POLICY_DEBUG
     tree_print("tree_evaluate()", tree, NULL);
-
+#endif
     if (ret <= 0)
         goto error;
 
-    /* Return value 2 means tree empty */
-    if (ret == 2) {
+    if (ret == X509_PCY_TREE_EMPTY) {
         X509_policy_tree_free(tree);
-        if (*pexplicit_policy)
-            return -2;
-        else
-            return 1;
+        if (init_ret & X509_PCY_TREE_EXPLICIT)
+            return X509_PCY_TREE_FAILURE;
+        return X509_PCY_TREE_VALID;
     }
 
     /* Tree is not empty: continue */
-
-    ret = tree_calculate_authority_set(tree, &auth_nodes);
-
-    if (!ret)
-        goto error;
-
-    if (!tree_calculate_user_set(tree, policy_oids, auth_nodes))
+    if ((ret = tree_calculate_authority_set(tree, &auth_nodes)) == 0 ||
+        !tree_calculate_user_set(tree, policy_oids, auth_nodes))
         goto error;
-
-    if (ret == 2)
+    if (ret == TREE_CALC_OK_DOFREE)
         sk_X509_POLICY_NODE_free(auth_nodes);
 
-    if (tree)
-        *ptree = tree;
+    *ptree = tree;
 
-    if (*pexplicit_policy) {
+    if (init_ret & X509_PCY_TREE_EXPLICIT) {
         nodes = X509_policy_tree_get0_user_policies(tree);
         if (sk_X509_POLICY_NODE_num(nodes) <= 0)
-            return -2;
+            return X509_PCY_TREE_FAILURE;
     }
-
-    return 1;
+    return X509_PCY_TREE_VALID;
 
  error:
-
     X509_policy_tree_free(tree);
-
-    return 0;
-
+    return X509_PCY_TREE_INTERNAL;
 }