[openssl-commits] [openssl] master update
Viktor Dukhovni
viktor at openssl.org
Fri Feb 5 16:13:26 UTC 2016
The branch master has been updated
via 3921ded79a8cd24fc8b333cb35298b01612bb38c (commit)
via 895c2f84a6a083fc8b9f69f962ed19da12ce3b40 (commit)
from a0474357743b5cc4db1b5428ac3db85b1168d3a9 (commit)
- Log -----------------------------------------------------------------
commit 3921ded79a8cd24fc8b333cb35298b01612bb38c
Author: Viktor Dukhovni <openssl-users at dukhovni.org>
Date: Sun Jan 31 21:48:00 2016 -0500
Ensure correct chain depth for policy checks with DANE bare key TA
Reviewed-by: Dr. Stephen Henson <steve at openssl.org>
commit 895c2f84a6a083fc8b9f69f962ed19da12ce3b40
Author: Viktor Dukhovni <openssl-users at dukhovni.org>
Date: Sun Jan 31 21:14:51 2016 -0500
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 at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
crypto/x509/x509_vfy.c | 29 ++-
crypto/x509v3/pcy_node.c | 3 +-
crypto/x509v3/pcy_tree.c | 458 +++++++++++++++++++++------------------------
include/openssl/x509_vfy.h | 28 ++-
4 files changed, 263 insertions(+), 255 deletions(-)
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 1f3b2b9..f16be8a 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1501,16 +1501,35 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x)
static int check_policy(X509_STORE_CTX *ctx)
{
int ret;
+
if (ctx->parent)
return 1;
+ /*
+ * With DANE, the trust anchor might be a bare public key, not a
+ * certificate! In that case our chain does not have the trust anchor
+ * certificate as a top-most element. This comports well with RFC5280
+ * chain verification, since there too, the trust anchor is not part of the
+ * chain to be verified. In particular, X509_policy_check() does not look
+ * at the TA cert, but assumes that it is present as the top-most chain
+ * element. We therefore temporarily push a NULL cert onto the chain if it
+ * was verified via a bare public key, and pop it off right after the
+ * X509_policy_check() call.
+ */
+ if (ctx->bare_ta_signed && !sk_X509_push(ctx->chain, NULL)) {
+ X509err(X509_F_CHECK_POLICY, ERR_R_MALLOC_FAILURE);
+ return 0;
+ }
ret = X509_policy_check(&ctx->tree, &ctx->explicit_policy, ctx->chain,
ctx->param->policies, ctx->param->flags);
- if (ret == 0) {
+ if (ctx->bare_ta_signed)
+ sk_X509_pop(ctx->chain);
+
+ 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 +1546,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 e8007c2..581c246 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 cac2d51..b3995d6 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;
}
diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index ef54208..4e458d2 100644
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -55,17 +55,16 @@
* [including the GNU Public Licence.]
*/
-#ifndef HEADER_X509_H
-# include <openssl/x509.h>
-/*
- * openssl/x509.h ends up #include-ing this file at about the only
- * appropriate moment.
- */
-#endif
-
#ifndef HEADER_X509_VFY_H
# define HEADER_X509_VFY_H
+/*
+ * Protect against recursion, x509.h and x509_vfy.h each include the other.
+ */
+# ifndef HEADER_X509_H
+# include <openssl/x509.h>
+# endif
+
# include <openssl/opensslconf.h>
# include <openssl/lhash.h>
# include <openssl/bio.h>
@@ -583,6 +582,19 @@ const X509_VERIFY_PARAM *X509_VERIFY_PARAM_get0(int id);
const X509_VERIFY_PARAM *X509_VERIFY_PARAM_lookup(const char *name);
void X509_VERIFY_PARAM_table_cleanup(void);
+/* Non positive return values are errors */
+#define X509_PCY_TREE_FAILURE -2 /* Failure to satisfy explicit policy */
+#define X509_PCY_TREE_INVALID -1 /* Inconsistent or invalid extensions */
+#define X509_PCY_TREE_INTERNAL 0 /* Internal error, most likely malloc */
+
+/*
+ * Positive return values form a bit mask, all but the first are internal to
+ * the library and don't appear in results from X509_policy_check().
+ */
+#define X509_PCY_TREE_VALID 1 /* The policy tree is valid */
+#define X509_PCY_TREE_EMPTY 2 /* The policy tree is empty */
+#define X509_PCY_TREE_EXPLICIT 4 /* Explicit policy required */
+
int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
STACK_OF(X509) *certs,
STACK_OF(ASN1_OBJECT) *policy_oids, unsigned int flags);
More information about the openssl-commits
mailing list