external/boringssl: Sync to ee7aa02744a78bf4630913b1c83d0fe36aa45efc.android-o-preview-4

This includes the following changes:

https://boringssl.googlesource.com/boringssl/+log/a93bc1124c00b1ac0a68ea5cb14b158d6c8366e1..ee7aa02744a78bf4630913b1c83d0fe36aa45efc

Test: BoringSSL CTS Presubmits.
Change-Id: I5a05899374e616003f841983b6545f5c90e7c71d

7 files changed, 148 insertions, 45 deletions

diff --git a/src/crypto/cipher_extra/aead_test.cc b/src/crypto/cipher_extra/aead_test.cc
index cce432c5..949c8002 100644
--- a/src/crypto/cipher_extra/aead_test.cc
+++ b/src/crypto/cipher_extra/aead_test.cc
@@ -580,6 +580,31 @@ TEST_P(PerAEADTest, UnalignedInput) {
             Bytes(out + 1, out_len));
 }
 
+TEST_P(PerAEADTest, Overflow) {
+  alignas(64) uint8_t key[EVP_AEAD_MAX_KEY_LENGTH];
+  OPENSSL_memset(key, 'K', sizeof(key));
+
+  bssl::ScopedEVP_AEAD_CTX ctx;
+  const size_t max_tag_len = EVP_AEAD_max_tag_len(aead());
+  ASSERT_TRUE(EVP_AEAD_CTX_init_with_direction(ctx.get(), aead(), key,
+                                               EVP_AEAD_key_length(aead()),
+                                               max_tag_len, evp_aead_seal));
+
+  uint8_t plaintext[1] = {0};
+  uint8_t ciphertext[1024] = {0};
+  size_t ciphertext_len;
+  // The AEAD must not overflow when calculating the ciphertext length.
+  ASSERT_FALSE(EVP_AEAD_CTX_seal(
+      ctx.get(), ciphertext, &ciphertext_len, sizeof(ciphertext), nullptr, 0,
+      plaintext, std::numeric_limits<size_t>::max() - max_tag_len + 1, nullptr,
+      0));
+  ERR_clear_error();
+
+  // (Can't test the scatter interface because it'll attempt to zero the output
+  // buffer on error and the primary output buffer is implicitly the same size
+  // as the input.)
+}
+
 // Test that EVP_aead_aes_128_gcm and EVP_aead_aes_256_gcm reject empty nonces.
 // AES-GCM is not defined for those.
 TEST(AEADTest, AESGCMEmptyNonce) {
diff --git a/src/crypto/evp/evp_tests.txt b/src/crypto/evp/evp_tests.txt
index 52b2a2c4..5e69a4ea 100644
--- a/src/crypto/evp/evp_tests.txt
+++ b/src/crypto/evp/evp_tests.txt
@@ -343,7 +343,7 @@ RSAPadding = PSS
 PSSSaltLength = 223
 Digest = SHA256
 Input = "0123456789ABCDEF0123456789ABCDEF"
-Error = DATA_TOO_LARGE_FOR_KEY_SIZE
+Error = DATA_TOO_LARGE
 
 # The salt length is too large for the modulus (verifying).
 Verify = RSA-2048
@@ -360,14 +360,14 @@ RSAPadding = PSS
 PSSSaltLength = 0
 Digest = SHA512
 Input = "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF"
-Error = DATA_TOO_LARGE_FOR_KEY_SIZE
+Error = DATA_TOO_LARGE
 
 Sign = RSA-512
 RSAPadding = PSS
 PSSSaltLength = -2
 Digest = SHA512
 Input = "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF"
-Error = DATA_TOO_LARGE_FOR_KEY_SIZE
+Error = DATA_TOO_LARGE
 
 # The hash is too large for the modulus (verifying).
 Verify = RSA-512
diff --git a/src/crypto/fipsmodule/FIPS.md b/src/crypto/fipsmodule/FIPS.md
index 06ef9fb5..30c44874 100644
--- a/src/crypto/fipsmodule/FIPS.md
+++ b/src/crypto/fipsmodule/FIPS.md
@@ -1,10 +1,57 @@
+# FIPS 140-2
+
+BoringSSL as a whole is not FIPS validated. However, there is a core library (called BoringCrypto) that is undergoing validation at time of writing. This document contains some notes about the design of the FIPS module and some documentation on performing FIPS-related tasks. This is not a substitute for reading the offical Security Policy (which, at the time of writing, has not yet been published).
+
+## Running CAVP tests
+
+CAVP results are calculated by `fipstools/cavp`, but that binary is almost always run by `fipstools/run_cavp.go`. The latter knows the set of tests to be processed and the flags needed to configure `cavp` for each one. It must be run from the top of a CAVP directory and needs the following options:
+
+1. `-oracle-bin`: points to the location of `fipstools/cavp`
+2. `-no-fax`: this is needed to suppress checking of the FAX files, which are only included in sample sets.
+
+## Breaking power-on and continuous tests
+
+In order to demonstrate failures of the various FIPS 140 tests, BoringSSL can be built in ways that will trigger such failures. This is controlled by passing `-DFIPS_BREAK_TEST=`(test to break) to CMake, where the following tests can be specified:
+
+1. AES\_CBC
+1. AES\_GCM
+1. DES
+1. SHA\_1
+1. SHA\_256
+1. SHA\_512
+1. RSA\_SIG
+1. ECDSA\_SIG
+1. DRBG
+1. RSA\_PWCT
+1. ECDSA\_PWCT
+
+## Breaking the integrity test
+
+The utility in `util/fipstools/break-hash.go` can be used to corrupt the FIPS module inside a binary and thus trigger a failure of the integrity test. Note that the binary must not be stripped, otherwise the utility will not be able to find the FIPS module.
+
+## RNG design
+
+FIPS 140-2 requires that one of its PRNGs be used (which they call DRBGs). In BoringCrypto, we use CTR-DRBG with AES-256 exclusively and `RAND_bytes` (the primary interface for the rest of the system to get random data) takes its output from there.
+
+The DRBG state is kept in a thread-local structure and is seeded from one of the following entropy sources in preference order: RDRAND (on Intel chips), `getrandom`, and `/dev/urandom`. In the case of `/dev/urandom`, in order to ensure that the system has a minimum level of entropy, BoringCrypto polls the kernel until the estimated entropy is at least 256 bits. This is a poor man's version of `getrandom` and we strongly recommend using a kernel recent enough to support the real thing.
+
+In FIPS mode, each of those entropy sources is subject to a 10× overread. That is, when *n* bytes of entropy are needed, *10n* bytes will be read from the entropy source and XORed down to *n* bytes. Reads from the entropy source are also processed in blocks of 16 bytes and if two consecutive chunks are equal the process will abort.
+
+The CTR-DRBG is reseeded every 4096 calls to `RAND_bytes`. Thus the process will randomly crash about every 2¹³⁵ calls.
+
+The FIPS PRNGs allow “additional input” to be fed into a given call. We use this feature to be as robust as possible to state duplication from process forks and VM copies: for every call we read 32 bytes of “additional data” from the entropy source (without overread) which means that cloned states will diverge at the next call to `RAND_bytes`. This is called “prediction resistance” by FIPS, but we do *not* claim this property in a FIPS context because we don't implement it the way they want.
+
+There is a second interface to the RNG which allows the caller to supply bytes that will be XORed into the generated additional data (`RAND_bytes_with_additional_data`). This is used in the ECDSA code to include the message and private key in the generation of *k*, the ECDSA nonce. This allows ECDSA to be robust to entropy failures while still following the FIPS rules.
+
+FIPS requires that RNG state be zeroed when the process exits. In order to implement this, all per-thread RNG states are tracked in a linked list and a destructor function is included which clears them. In order for this to be safe in the presence of threads, a lock is used to stop all other threads from using the RNG once this process has begun. Thus the main thread exiting may cause other threads to deadlock, and drawing on entropy in a destructor function may also deadlock.
+
 ## Integrity Test
 
 FIPS-140 mandates that a module calculate an HMAC of its own code in a constructor function and compare the result to a known-good value. Typical code produced by a C compiler includes large numbers of relocations: places in the machine code where the linker needs to resolve and inject the final value of a symbolic expression. These relocations mean that the bytes that make up any specific bit of code generally aren't known until the final link has completed.
 
 Additionally, because of shared libraries and ASLR, some relocations can only be resolved at run-time, and thus targets of those relocations vary even after the final link.
 
-BoringSSL is linked (often statically) into a large number of binaries. It would be a significant cost if each of these binaries had to be post-processed in order to calculate the known-good HMAC value. We would much prefer if the value could be calculated, once, when BoringSSL itself is compiled.
+BoringCrypto is linked (often statically) into a large number of binaries. It would be a significant cost if each of these binaries had to be post-processed in order to calculate the known-good HMAC value. We would much prefer if the value could be calculated, once, when BoringCrypto itself is compiled.
 
 In order for the value to be calculated before the final link, there can be no relocations in the hashed code and data. This document describes how we build C and assembly code in order to produce an object file containing all the code and data for the FIPS module without that code having any relocations.
 
@@ -32,7 +79,7 @@ One special case is `rel.ro` data, which is data that contains function pointers
 
 Mutable data is a problem. It cannot be in the text segment because the text segment is mapped read-only. If it's in a different segment then the code cannot reference it with a known, IP-relative offset because the segment layout is only fixed during the final link.
 
-Thankfully, mutable data is very rare in our cryptographic code and I hope that we can get it down to just a few variables. In order to allow this we use a similar design to the redirector functions: the code references a symbol that's in the text segment, but out of the module and thus not hashed. A relocation record is emitted to instruct the linker to poke the final offset to the variable in that location. Thus the only change needed is an extra indirection when loading the value.
+In order to allow this we use a similar design to the redirector functions: the code references a symbol that's in the text segment, but out of the module and thus not hashed. A relocation record is emitted to instruct the linker to poke the final offset to the variable in that location. Thus the only change needed is an extra indirection when loading the value.
 
 ##### Other transforms
 
@@ -42,8 +89,7 @@ The script performs a number of other transformations which are worth noting but
 1.  Various sections, notably `.rodata`, are moved to the `.text` section, inside the module, so module code may reference it without relocations.
 1.  For each BSS symbol, it generates a function named after that symbol but with `_bss_get` appended, which returns its address.
 1.  It inserts the labels that delimit the module's code and data (called `module_start` and `module_end` in the diagram above).
-1.  It adds a 32-byte, read-only array outside of the module to contain the known-good HMAC value.
-1.  It rewrites some "dummy" references to point to those labels and that array. In order to get the C compiler to emit the correct code it's necessary to make it think that it's referencing static functions. This compensates for that trick.
+1.  It adds a 64-byte, read-only array outside of the module to contain the known-good HMAC value.
 
 ##### Integrity testing
 
@@ -59,17 +105,16 @@ Initially the known-good value will be incorrect. Another script (`inject-hash.g
 
 OpenSSL's solution to this problem is broadly similar but has a number of differences:
 
-1.  OpenSSL deals with run-time relocations by not hashing parts of the module's data. BoringSSL will eliminate run-time relocations instead and hash everything.
-1.  OpenSSL uses `ld -r` (the partial linking mode) to merge a number of object files into their `fipscanister.o`. For BoringSSL, we propose to merge all the C source files by building a single C file that #includes all the others, then we propose to merge the assembly sources by concatenating them to the assembly output from the C compiler.
-1.  OpenSSL depends on the link order and inserts two object files, `fips_start.o` and `fips_end.o`, in order to establish the `module_start` and `module_end` values. BoringSSL proposes to simply add labels at the correct places in the assembly.
-1.  OpenSSL calculates the hash after the final link and either injects it into the binary or recompiles with the value of the hash passed in as a #define. BoringSSL calculates it prior to the final link and injects it into the object file.
-1.  OpenSSL references read-write data directly, since it can know the offsets to it. BoringSSL indirects these loads and stores.
-1.  OpenSSL doesn't run the power-on test until `FIPS_module_mode_set` is called, BoringSSL plans to do it in a constructor function. Failure of the test is non-fatal in OpenSSL, BoringSSL plans to crash.
-1.  Since the contents of OpenSSL's module change between compilation and use, OpenSSL generates `fipscanister.o.sha1` to check that the compiled object doesn't change before linking. Since BoringSSL's module is fixed after compilation, the final integrity check is unbroken through the linking process.
+1.  OpenSSL deals with run-time relocations by not hashing parts of the module's data.
+1.  OpenSSL uses `ld -r` (the partial linking mode) to merge a number of object files into their `fipscanister.o`. For BoringCrypto, we merge all the C source files by building a single C file that #includes all the others, and we merge the assembly sources by appending them to the assembly output from the C compiler.
+1.  OpenSSL depends on the link order and inserts two object files, `fips_start.o` and `fips_end.o`, in order to establish the `module_start` and `module_end` values. BoringCrypto adds labels at the correct places in the assembly.
+1.  OpenSSL calculates the hash after the final link and either injects it into the binary or recompiles with the value of the hash passed in as a #define. BoringCrypto calculates it prior to the final link and injects it into the object file.
+1.  OpenSSL references read-write data directly, since it can know the offsets to it. BoringCrypto indirects these loads and stores.
+1.  OpenSSL doesn't run the power-on test until `FIPS_module_mode_set` is called. BoringCrypto does it in a constructor function. Failure of the test is non-fatal in OpenSSL, BoringCrypto will crash.
+1.  Since the contents of OpenSSL's module change between compilation and use, OpenSSL generates `fipscanister.o.sha1` to check that the compiled object doesn't change before linking. Since BoringCrypto's module is fixed after compilation, the final integrity check is unbroken through the linking process.
 
 Some of the similarities are worth noting:
 
 1.  OpenSSL has all out-calls from the module indirecting via the PLT, which is equivalent to the redirector functions described above.
 
-
 ![OpenSSL build process](/crypto/fipsmodule/intcheck3.png)
diff --git a/src/crypto/fipsmodule/rsa/padding.c b/src/crypto/fipsmodule/rsa/padding.c
index 69bd9ad6..9f002d24 100644
--- a/src/crypto/fipsmodule/rsa/padding.c
+++ b/src/crypto/fipsmodule/rsa/padding.c
@@ -170,7 +170,7 @@ int RSA_padding_add_PKCS1_type_2(uint8_t *to, size_t to_len,
   }
 
   if (from_len > to_len - RSA_PKCS1_PADDING_SIZE) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     return 0;
   }
 
@@ -254,12 +254,12 @@ int RSA_padding_check_PKCS1_type_2(uint8_t *out, size_t *out_len,
 int RSA_padding_add_none(uint8_t *to, size_t to_len, const uint8_t *from,
                          size_t from_len) {
   if (from_len > to_len) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     return 0;
   }
 
   if (from_len < to_len) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_SMALL_FOR_KEY_SIZE);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_SMALL);
     return 0;
   }
 
@@ -330,7 +330,7 @@ int RSA_padding_add_PKCS1_OAEP_mgf1(uint8_t *to, size_t to_len,
 
   size_t emlen = to_len - 1;
   if (from_len > emlen - 2 * mdlen - 1) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     return 0;
   }
 
@@ -608,7 +608,7 @@ int RSA_padding_add_PKCS1_PSS_mgf1(RSA *rsa, unsigned char *EM,
   }
 
   if (emLen < hLen + 2) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     goto err;
   }
 
@@ -629,7 +629,7 @@ int RSA_padding_add_PKCS1_PSS_mgf1(RSA *rsa, unsigned char *EM,
   }
 
   if (emLen - hLen - 2 < sLen) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     goto err;
   }
 
diff --git a/src/crypto/fipsmodule/rsa/rsa_impl.c b/src/crypto/fipsmodule/rsa/rsa_impl.c
index c79f8649..b126164d 100644
--- a/src/crypto/fipsmodule/rsa/rsa_impl.c
+++ b/src/crypto/fipsmodule/rsa/rsa_impl.c
@@ -176,7 +176,7 @@ int RSA_encrypt(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
 
   if (BN_ucmp(f, rsa->n) >= 0) {
     /* usually the padding functions would catch this */
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_MODULUS);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     goto err;
   }
 
@@ -489,7 +489,7 @@ int RSA_verify_raw(RSA *rsa, size_t *out_len, uint8_t *out, size_t max_out,
   }
 
   if (BN_ucmp(f, rsa->n) >= 0) {
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_MODULUS);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     goto err;
   }
 
@@ -563,7 +563,7 @@ int rsa_default_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in,
 
   if (BN_ucmp(f, rsa->n) >= 0) {
     /* Usually the padding functions would catch this. */
-    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE_FOR_MODULUS);
+    OPENSSL_PUT_ERROR(RSA, RSA_R_DATA_TOO_LARGE);
     goto err;
   }
 
diff --git a/src/crypto/x509/asn1_gen.c b/src/crypto/x509/asn1_gen.c
index c52a1ac0..5b74cd1e 100644
--- a/src/crypto/x509/asn1_gen.c
+++ b/src/crypto/x509/asn1_gen.c
@@ -84,6 +84,8 @@
 #define ASN1_GEN_STR(str,val)   {str, sizeof(str) - 1, val}
 
 #define ASN1_FLAG_EXP_MAX       20
+/* Maximum number of nested sequences */
+#define ASN1_GEN_SEQ_MAX_DEPTH  50
 
 /* Input formats */
 
@@ -120,13 +122,16 @@ typedef struct {
     int exp_count;
 } tag_exp_arg;
 
+static ASN1_TYPE *generate_v3(char *str, X509V3_CTX *cnf, int depth,
+                              int *perr);
 static int bitstr_cb(const char *elem, int len, void *bitstr);
 static int asn1_cb(const char *elem, int len, void *bitstr);
 static int append_exp(tag_exp_arg *arg, int exp_tag, int exp_class,
                       int exp_constructed, int exp_pad, int imp_ok);
 static int parse_tagging(const char *vstart, int vlen, int *ptag,
                          int *pclass);
-static ASN1_TYPE *asn1_multi(int utype, const char *section, X509V3_CTX *cnf);
+static ASN1_TYPE *asn1_multi(int utype, const char *section, X509V3_CTX *cnf,
+                             int depth, int *perr);
 static ASN1_TYPE *asn1_str2type(const char *str, int format, int utype);
 static int asn1_str2tag(const char *tagstr, int len);
 
@@ -143,6 +148,16 @@ ASN1_TYPE *ASN1_generate_nconf(char *str, CONF *nconf)
 
 ASN1_TYPE *ASN1_generate_v3(char *str, X509V3_CTX *cnf)
 {
+    int err = 0;
+    ASN1_TYPE *ret = generate_v3(str, cnf, 0, &err);
+    if (err)
+        OPENSSL_PUT_ERROR(ASN1, err);
+    return ret;
+}
+
+static ASN1_TYPE *generate_v3(char *str, X509V3_CTX *cnf, int depth,
+                              int *perr)
+{
     ASN1_TYPE *ret;
     tag_exp_arg asn1_tags;
     tag_exp_type *etmp;
@@ -162,16 +177,22 @@ ASN1_TYPE *ASN1_generate_v3(char *str, X509V3_CTX *cnf)
     asn1_tags.imp_class = -1;
     asn1_tags.format = ASN1_GEN_FORMAT_ASCII;
     asn1_tags.exp_count = 0;
-    if (CONF_parse_list(str, ',', 1, asn1_cb, &asn1_tags) != 0)
+    if (CONF_parse_list(str, ',', 1, asn1_cb, &asn1_tags) != 0) {
+        *perr = ASN1_R_UNKNOWN_TAG;
         return NULL;
+    }
 
     if ((asn1_tags.utype == V_ASN1_SEQUENCE)
         || (asn1_tags.utype == V_ASN1_SET)) {
         if (!cnf) {
-            OPENSSL_PUT_ERROR(ASN1, ASN1_R_SEQUENCE_OR_SET_NEEDS_CONFIG);
+            *perr = ASN1_R_SEQUENCE_OR_SET_NEEDS_CONFIG;
             return NULL;
         }
-        ret = asn1_multi(asn1_tags.utype, asn1_tags.str, cnf);
+        if (depth >= ASN1_GEN_SEQ_MAX_DEPTH) {
+            *perr = ASN1_R_ILLEGAL_NESTED_TAGGING;
+            return NULL;
+        }
+        ret = asn1_multi(asn1_tags.utype, asn1_tags.str, cnf, depth, perr);
     } else
         ret = asn1_str2type(asn1_tags.str, asn1_tags.format, asn1_tags.utype);
 
@@ -289,7 +310,7 @@ static int asn1_cb(const char *elem, int len, void *bitstr)
     int tmp_tag, tmp_class;
 
     if (elem == NULL)
-        return 0;
+        return -1;
 
     for (i = 0, p = elem; i < len; p++, i++) {
         /* Look for the ':' in name value pairs */
@@ -444,7 +465,8 @@ static int parse_tagging(const char *vstart, int vlen, int *ptag, int *pclass)
 
 /* Handle multiple types: SET and SEQUENCE */
 
-static ASN1_TYPE *asn1_multi(int utype, const char *section, X509V3_CTX *cnf)
+static ASN1_TYPE *asn1_multi(int utype, const char *section, X509V3_CTX *cnf,
+                             int depth, int *perr)
 {
     ASN1_TYPE *ret = NULL;
     STACK_OF(ASN1_TYPE) *sk = NULL;
@@ -463,7 +485,8 @@ static ASN1_TYPE *asn1_multi(int utype, const char *section, X509V3_CTX *cnf)
             goto bad;
         for (i = 0; i < sk_CONF_VALUE_num(sect); i++) {
             ASN1_TYPE *typ =
-                ASN1_generate_v3(sk_CONF_VALUE_value(sect, i)->value, cnf);
+                generate_v3(sk_CONF_VALUE_value(sect, i)->value, cnf,
+                            depth + 1, perr);
             if (!typ)
                 goto bad;
             if (!sk_ASN1_TYPE_push(sk, typ))
diff --git a/src/crypto/x509v3/pcy_tree.c b/src/crypto/x509v3/pcy_tree.c
index 71f9a6dc..256fe88e 100644
--- a/src/crypto/x509v3/pcy_tree.c
+++ b/src/crypto/x509v3/pcy_tree.c
@@ -136,11 +136,14 @@ static void tree_print(char *str, X509_POLICY_TREE *tree,
 
 #endif
 
-/*
- * Initialize policy tree. Return values: 0 Some internal error occured. -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.
+/*-
+ * 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.
  */
 
 static int tree_init(X509_POLICY_TREE **ptree, STACK_OF(X509) *certs,
@@ -720,10 +723,13 @@ 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.
+/*-
+ * 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.
  */
 
 int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
@@ -731,6 +737,7 @@ int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
                       STACK_OF(ASN1_OBJECT) *policy_oids, unsigned int flags)
 {
     int ret;
+    int calc_ret;
     X509_POLICY_TREE *tree = NULL;
     STACK_OF(X509_POLICY_NODE) *nodes, *auth_nodes = NULL;
     *ptree = NULL;
@@ -799,17 +806,20 @@ int X509_policy_check(X509_POLICY_TREE **ptree, int *pexplicit_policy,
 
     /* Tree is not empty: continue */
 
-    ret = tree_calculate_authority_set(tree, &auth_nodes);
+    calc_ret = tree_calculate_authority_set(tree, &auth_nodes);
 
-    if (!ret)
+    if (!calc_ret)
         goto error;
 
-    if (!tree_calculate_user_set(tree, policy_oids, auth_nodes))
-        goto error;
+    ret = tree_calculate_user_set(tree, policy_oids, auth_nodes);
 
-    if (ret == 2)
+    if (calc_ret == 2)
         sk_X509_POLICY_NODE_free(auth_nodes);
 
+    if (!ret)
+        goto error;
+
+
     if (tree)
         *ptree = tree;