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: NDr. Stephen Henson <steve@openssl.org>

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;

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)

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);