Ensure verify error is set when X509_verify_cert() fails.

Set ctx->error = X509_V_ERR_OUT_OF_MEM when verification cannot
continue due to malloc failure.  Similarly for issuer lookup failures
and caller errors (bad parameters or invalid state).

Also, when X509_verify_cert() returns <= 0 make sure that the
verification status does not remain X509_V_OK, as a last resort set
it it to X509_V_ERR_UNSPECIFIED, just in case some code path returns
an error without setting an appropriate value of ctx->error.

Add new and some missing error codes to X509 error -> SSL alert switch.

(Imported from upstream's 5553a12735e11bc9aa28727afe721e7236788aab.)

Change-Id: I3231a6b2e72a3914cb9316b8e90ebaee009a1c5f
Reviewed-on: https://boringssl-review.googlesource.com/8170
Reviewed-by: David Benjamin <davidben@google.com>

2 files changed, 36 insertions, 4 deletions

diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c
index 86af3fec..17e6cdb9 100644
--- a/crypto/x509/x509_txt.c
+++ b/crypto/x509/x509_txt.c
@@ -199,6 +199,11 @@ const char *X509_verify_cert_error_string(long n)
     case X509_V_ERR_IP_ADDRESS_MISMATCH:
         return ("IP address mismatch");
 
+    case X509_V_ERR_INVALID_CALL:
+        return ("Invalid certificate verification context");
+    case X509_V_ERR_STORE_LOOKUP:
+        return ("Issuer certificate lookup error");
+
     default:
         BIO_snprintf(buf, sizeof buf, "error number %ld", n);
         return (buf);
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 2ed2f03e..520408fe 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -198,6 +198,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
     STACK_OF(X509) *sktmp = NULL;
     if (ctx->cert == NULL) {
         OPENSSL_PUT_ERROR(X509, X509_R_NO_CERT_SET_FOR_US_TO_VERIFY);
+        ctx->error = X509_V_ERR_INVALID_CALL;
         return -1;
     }
     if (ctx->chain != NULL) {
@@ -206,6 +207,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
          * cannot do another one.
          */
         OPENSSL_PUT_ERROR(X509, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+        ctx->error = X509_V_ERR_INVALID_CALL;
         return -1;
     }
 
@@ -218,6 +220,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
     ctx->chain = sk_X509_new_null();
     if (ctx->chain == NULL || !sk_X509_push(ctx->chain, ctx->cert)) {
         OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
+        ctx->error = X509_V_ERR_OUT_OF_MEM;
         goto end;
     }
     X509_up_ref(ctx->cert);
@@ -227,6 +230,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
     if (ctx->untrusted != NULL
         && (sktmp = sk_X509_dup(ctx->untrusted)) == NULL) {
         OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
+        ctx->error = X509_V_ERR_OUT_OF_MEM;
         goto end;
     }
 
@@ -250,8 +254,10 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
          */
         if (ctx->param->flags & X509_V_FLAG_TRUSTED_FIRST) {
             ok = ctx->get_issuer(&xtmp, ctx, x);
-            if (ok < 0)
+            if (ok < 0) {
+                ctx->error = X509_V_ERR_STORE_LOOKUP;
                 goto end;
+            }
             /*
              * If successful for now free up cert so it will be picked up
              * again later.
@@ -268,6 +274,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
             if (xtmp != NULL) {
                 if (!sk_X509_push(ctx->chain, xtmp)) {
                     OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
+                    ctx->error = X509_V_ERR_OUT_OF_MEM;
                     ok = 0;
                     goto end;
                 }
@@ -349,14 +356,17 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
                 break;
             ok = ctx->get_issuer(&xtmp, ctx, x);
 
-            if (ok < 0)
+            if (ok < 0) {
+                ctx->error = X509_V_ERR_STORE_LOOKUP;
                 goto end;
+            }
             if (ok == 0)
                 break;
             x = xtmp;
             if (!sk_X509_push(ctx->chain, x)) {
                 X509_free(xtmp);
                 OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
+                ctx->error = X509_V_ERR_OUT_OF_MEM;
                 ok = 0;
                 goto end;
             }
@@ -493,6 +503,10 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
         sk_X509_free(sktmp);
     if (chain_ss != NULL)
         X509_free(chain_ss);
+
+    /* Safety net, error returns must set ctx->error */
+    if (ok <= 0 && ctx->error == X509_V_OK)
+        ctx->error = X509_V_ERR_UNSPECIFIED;
     return ok;
 }
 
@@ -709,12 +723,19 @@ static int check_name_constraints(X509_STORE_CTX *ctx)
             NAME_CONSTRAINTS *nc = sk_X509_value(ctx->chain, j)->nc;
             if (nc) {
                 rv = NAME_CONSTRAINTS_check(x, nc);
-                if (rv != X509_V_OK) {
+                switch (rv) {
+                case X509_V_OK:
+                    continue;
+                case X509_V_ERR_OUT_OF_MEM:
+                    ctx->error = rv;
+                    return 0;
+                default:
                     ctx->error = rv;
                     ctx->error_depth = i;
                     ctx->current_cert = x;
                     if (!ctx->verify_cb(0, ctx))
                         return 0;
+                    break;
                 }
             }
         }
@@ -1605,6 +1626,7 @@ static int check_policy(X509_STORE_CTX *ctx)
                             ctx->param->policies, ctx->param->flags);
     if (ret == 0) {
         OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE);
+        ctx->error = X509_V_ERR_OUT_OF_MEM;
         return 0;
     }
     /* Invalid or inconsistent extensions */
@@ -1633,7 +1655,12 @@ static int check_policy(X509_STORE_CTX *ctx)
 
     if (ctx->param->flags & X509_V_FLAG_NOTIFY_POLICY) {
         ctx->current_cert = NULL;
-        ctx->error = X509_V_OK;
+        /*
+         * Verification errors need to be "sticky", a callback may have allowed
+         * an SSL handshake to continue despite an error, and we must then
+         * remain in an error state.  Therefore, we MUST NOT clear earlier
+         * verification errors by setting the error to X509_V_OK.
+         */
         if (!ctx->verify_cb(2, ctx))
             return 0;
     }