Avoid possible memleak in X509_policy_check()
[openssl.git] / crypto / x509v3 / pcy_tree.c
index 8d02092cdee2fa99012a6df5aadb549178594e64..b3d1983f9e050093ce8c3c24233d691b89494cc4 100644 (file)
@@ -1,63 +1,13 @@
-/* pcy_tree.c */
 /*
- * Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL project
- * 2004.
- */
-/* ====================================================================
- * Copyright (c) 2004 The OpenSSL Project.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- *
- * 3. All advertising materials mentioning features or use of this
- *    software must display the following acknowledgment:
- *    "This product includes software developed by the OpenSSL Project
- *    for use in the OpenSSL Toolkit. (http://www.OpenSSL.org/)"
- *
- * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
- *    endorse or promote products derived from this software without
- *    prior written permission. For written permission, please contact
- *    licensing@OpenSSL.org.
- *
- * 5. Products derived from this software may not be called "OpenSSL"
- *    nor may "OpenSSL" appear in their names without prior written
- *    permission of the OpenSSL Project.
- *
- * 6. Redistributions of any form whatsoever must retain the following
- *    acknowledgment:
- *    "This product includes software developed by the OpenSSL Project
- *    for use in the OpenSSL Toolkit (http://www.OpenSSL.org/)"
- *
- * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
- * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE OpenSSL PROJECT OR
- * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
- * OF THE POSSIBILITY OF SUCH DAMAGE.
- * ====================================================================
- *
- * This product includes cryptographic software written by Eric Young
- * (eay@cryptsoft.com).  This product includes software written by Tim
- * Hudson (tjh@cryptsoft.com).
+ * Copyright 2004-2016 The OpenSSL Project Authors. All Rights Reserved.
  *
+ * Licensed under the OpenSSL license (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
  */
 
-#include "cryptlib.h"
+#include "internal/cryptlib.h"
 #include <openssl/x509.h>
 #include <openssl/x509v3.h>
 
@@ -98,24 +48,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);
@@ -123,26 +75,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)
 {
@@ -150,114 +93,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 0
-    /* Disable policy mapping for now... */
-    flags |= X509_V_FLAG_INHIBIT_MAP;
-#endif
+    /* Can't do anything with just a trust anchor */
+    if (n == 0)
+        return X509_PCY_TREE_EMPTY;
 
-    if (flags & X509_V_FLAG_EXPLICIT_POLICY)
-        explicit_policy = 0;
-    else
-        explicit_policy = 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_ANY)
-        any_skip = 0;
-    else
-        any_skip = n + 1;
+        /* Call for side-effect of computing hash and caching extensions */
+        X509_check_purpose(x, -1, 0);
 
-    if (flags & X509_V_FLAG_INHIBIT_MAP)
-        map_skip = 0;
-    else
-        map_skip = n + 1;
+        /* If cache is NULL, likely ENOMEM: return immediately */
+        if (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 intersection 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--) {
-        x = sk_X509_value(certs, i);
-        X509_check_purpose(x, -1, -1);
+    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);
+
+        /* All the policies are already cached, we can return early */
+        if (ex_flags & EXFLAG_INVALID_POLICY)
+            return X509_PCY_TREE_INVALID;
+
+        /* Access the cache which we now know exists */
         cache = policy_cache_set(x);
-        /* If cache NULL something bad happened: return immediately */
-        if (cache == NULL)
-            return 0;
-        /*
-         * If inconsistent extensions keep a note of it but continue
-         */
-        if (x->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;
+
+        if ((ret & X509_PCY_TREE_VALID) && cache->data == NULL)
+            ret = X509_PCY_TREE_EMPTY;
         if (explicit_policy > 0) {
-            if (!(x->ex_flags & EXFLAG_SI))
+            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 */
+    if ((tree = OPENSSL_zalloc(sizeof(*tree))) == NULL)
+        return X509_PCY_TREE_INTERNAL;
 
-    tree = OPENSSL_malloc(sizeof(X509_POLICY_TREE));
-
-    if (!tree)
-        return 0;
-
-    tree->flags = 0;
-    tree->levels = OPENSSL_malloc(sizeof(X509_POLICY_LEVEL) * n);
-    tree->nlevel = 0;
-    tree->extra_data = NULL;
-    tree->auth_policies = NULL;
-    tree->user_policies = NULL;
-
-    if (!tree->levels) {
+    /*
+     * 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;
     }
-
-    memset(tree->levels, 0, n * sizeof(X509_POLICY_LEVEL));
-
-    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 || !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--) {
-        level++;
-        x = sk_X509_value(certs, i);
+    /*
+     * 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);
-        CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509);
-        level->cert = x;
+
+        X509_up_ref(x);
+        (++level)->cert = x;
 
         if (!cache->anyPolicy)
             level->flags |= X509_V_FLAG_INHIBIT_ANY;
@@ -265,63 +206,57 @@ 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 (!(x->ex_flags & EXFLAG_SI) || (i == 0))
+            if (!(ex_flags & EXFLAG_SI) || (i == 0))
                 level->flags |= X509_V_FLAG_INHIBIT_ANY;
         } else {
-            if (!(x->ex_flags & EXFLAG_SI))
+            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;
         }
 
         if (map_skip == 0)
             level->flags |= X509_V_FLAG_INHIBIT_MAP;
         else {
-            if (!(x->ex_flags & EXFLAG_SI))
+            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,
-                                    const X509_POLICY_DATA *data)
+                                    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;
@@ -330,29 +265,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);
-        /*
-         * If a node is mapped any it doesn't have a corresponding
-         * CertificatePolicies entry. However such an identical node would
-         * be created if anyPolicy matching is enabled because there would be
-         * no match with the parent valid_policy_set. So we create link
-         * because then it will have the mapping flags right and we can prune
-         * it later.
-         */
-#if 0
-        if ((data->flags & POLICY_DATA_FLAG_MAPPED_ANY)
-            && !(curr->flags & X509_V_FLAG_INHIBIT_ANY))
-            continue;
-#endif
+        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;
@@ -363,35 +286,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)
@@ -422,19 +348,17 @@ 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)
 {
     int i;
-    /*
-     * X509_POLICY_DATA *data;
-     */
     X509_POLICY_NODE *node;
     X509_POLICY_LEVEL *last = curr - 1;
 
@@ -443,50 +367,24 @@ static int tree_link_any(X509_POLICY_LEVEL *curr,
 
         if (!tree_link_unmatched(curr, cache, node, tree))
             return 0;
-
-#if 0
-
-        /*
-         * Skip any node with any children: we only want unmathced nodes.
-         * Note: need something better for policy mapping because each node
-         * may have multiple children
-         */
-        if (node->nchild)
-            continue;
-
-        /*
-         * Create a new node with qualifiers from anyPolicy and id from
-         * unmatched node.
-         */
-        data = policy_data_new(NULL, node->data->valid_policy,
-                               node_critical(node));
-
-        if (data == 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)) {
-            policy_data_free(data);
-            return 0;
-        }
-#endif
-
     }
     /* 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;
@@ -525,41 +423,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;
         }
     }
-
-    return 1;
-
+    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) {
-        *pnodes = policy_node_cmp_new();
-        if (!*pnodes)
-            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)
 {
@@ -572,7 +472,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 */
@@ -584,25 +484,31 @@ static int tree_calculate_authority_set(X509_POLICY_TREE *tree,
          * If no anyPolicy node on this this level it can't appear on lower
          * levels so end search.
          */
-        if (!(anyptr = curr->anyPolicy))
+        if ((anyptr = curr->anyPolicy) == NULL)
             break;
         curr++;
         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)
@@ -610,7 +516,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;
 
@@ -618,7 +523,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;
 
@@ -643,7 +547,7 @@ static int tree_calculate_user_set(X509_POLICY_TREE *tree,
              * from anyPolicy.
              */
             extra = policy_data_new(NULL, oid, node_critical(anyPolicy));
-            if (!extra)
+            if (extra == NULL)
                 return 0;
             extra->qualifier_set = anyPolicy->data->qualifier_set;
             extra->flags = POLICY_DATA_FLAG_SHARED_QUALIFIERS
@@ -659,9 +563,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;
@@ -671,19 +580,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)
@@ -704,17 +613,12 @@ void X509_policy_tree_free(X509_POLICY_TREE *tree)
     sk_X509_POLICY_NODE_pop_free(tree->user_policies, exnode_free);
 
     for (i = 0, curr = tree->levels; i < tree->nlevel; i++, curr++) {
-        if (curr->cert)
-            X509_free(curr->cert);
-        if (curr->nodes)
-            sk_X509_POLICY_NODE_pop_free(curr->nodes, policy_node_free);
-        if (curr->anyPolicy)
-            policy_node_free(curr->anyPolicy);
+        X509_free(curr->cert);
+        sk_X509_POLICY_NODE_pop_free(curr->nodes, policy_node_free);
+        policy_node_free(curr->anyPolicy);
     }
 
-    if (tree->extra_data)
-        sk_X509_POLICY_DATA_pop_free(tree->extra_data, policy_data_free);
-
+    sk_X509_POLICY_DATA_pop_free(tree->extra_data, policy_data_free);
     OPENSSL_free(tree->levels);
     OPENSSL_free(tree);
 
@@ -723,111 +627,74 @@ 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;
+    int calc_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;
-
-        /* Some internal error */
-    case -1:
-        return -1;
-
-        /* Some internal error */
-    case 0:
-        return 0;
-
-        /* Tree empty requireExplicit True: Error */
+    init_ret = tree_init(&tree, certs, flags);
 
-    case 6:
-        *pexplicit_policy = 1;
-        return -2;
+    if (init_ret <= 0)
+        return init_ret;
 
-        /* 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 ((calc_ret = tree_calculate_authority_set(tree, &auth_nodes)) == 0)
         goto error;
-
-    if (ret == 2)
+    ret = tree_calculate_user_set(tree, policy_oids, auth_nodes);
+    if (calc_ret == TREE_CALC_OK_DOFREE)
         sk_X509_POLICY_NODE_free(auth_nodes);
+    if (!ret)
+        goto error;
 
-    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;
 }