Simplify purpose checks

The check_ca calls are purpose-independent and redundant. Extract them
to common code. This makes it clearer that the purpose checks (almost)
all have the exact same structure.

Change-Id: I16218da1d994f59afc730424e9ad63f28cae14db
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65210
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index 2985dce..5419f06 100644
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -74,13 +74,13 @@
 #define xku_reject(x, usage) \
   (((x)->ex_flags & EXFLAG_XKUSAGE) && !((x)->ex_xkusage & (usage)))
 
+static int check_ca(const X509 *x);
 static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
                                     int ca);
 static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x,
                                     int ca);
 static int check_purpose_ns_ssl_server(const X509_PURPOSE *xp, const X509 *x,
                                        int ca);
-static int purpose_smime(const X509 *x, int ca);
 static int check_purpose_smime_sign(const X509_PURPOSE *xp, const X509 *x,
                                     int ca);
 static int check_purpose_smime_encrypt(const X509_PURPOSE *xp, const X509 *x,
@@ -90,7 +90,6 @@
 static int check_purpose_timestamp_sign(const X509_PURPOSE *xp, const X509 *x,
                                         int ca);
 static int no_check(const X509_PURPOSE *xp, const X509 *x, int ca);
-static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca);
 
 static const X509_PURPOSE xstandard[] = {
     {X509_PURPOSE_SSL_CLIENT, X509_TRUST_SSL_CLIENT, 0,
@@ -109,7 +108,9 @@
      (char *)"CRL signing", (char *)"crlsign", NULL},
     {X509_PURPOSE_ANY, X509_TRUST_DEFAULT, 0, no_check, (char *)"Any Purpose",
      (char *)"any", NULL},
-    {X509_PURPOSE_OCSP_HELPER, X509_TRUST_COMPAT, 0, ocsp_helper,
+    // |X509_PURPOSE_OCSP_HELPER| performs no actual checks. OpenSSL's OCSP
+    // implementation relied on the caller performing EKU and KU checks.
+    {X509_PURPOSE_OCSP_HELPER, X509_TRUST_COMPAT, 0, no_check,
      (char *)"OCSP helper", (char *)"ocsphelper", NULL},
     {X509_PURPOSE_TIMESTAMP_SIGN, X509_TRUST_TSA, 0,
      check_purpose_timestamp_sign, (char *)"Time Stamp signing",
@@ -130,6 +131,13 @@
   if (idx == -1) {
     return 0;
   }
+  // Historically, |check_purpose| implementations other than |X509_PURPOSE_ANY|
+  // called |check_ca|. This is redundant with the |X509_V_ERR_INVALID_CA|
+  // logic, but |X509_check_purpose| is public API, so we preserve this
+  // behavior.
+  if (ca && id != X509_PURPOSE_ANY && !check_ca(x)) {
+    return 0;
+  }
   const X509_PURPOSE *pt = X509_PURPOSE_get0(idx);
   return pt->check_purpose(pt, x, ca);
 }
@@ -411,23 +419,28 @@
   return check_ca(x);
 }
 
+// check_purpose returns one if |x| is a valid part of a certificate path for
+// extended key usage |required_xku| and at least one of key usages in
+// |required_kus|. |ca| indicates whether |x| is a CA or end-entity certificate.
+static int check_purpose(const X509 *x, int ca, int required_xku,
+                         int required_kus) {
+  // Check extended key usage on the entire chain.
+  if (required_xku != 0 && xku_reject(x, required_xku)) {
+    return 0;
+  }
+
+  // Check key usages only on the end-entity certificate.
+  return ca || !ku_reject(x, required_kus);
+}
+
 static int check_purpose_ssl_client(const X509_PURPOSE *xp, const X509 *x,
                                     int ca) {
-  if (xku_reject(x, XKU_SSL_CLIENT)) {
-    return 0;
-  }
-  if (ca) {
-    // TODO(davidben): Move the various |check_ca| calls out of the
-    // |check_purpose| callbacks. Those checks are purpose-independent. They are
-    // also redundant when called from |X509_verify_cert|, though
-    // not when |X509_check_purpose| is called directly.
-    return check_ca(x);
-  }
-  // We need to do digital signatures or key agreement
-  if (ku_reject(x, X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_KEY_AGREEMENT)) {
-    return 0;
-  }
-  return 1;
+  // We need to do digital signatures or key agreement.
+  //
+  // TODO(davidben): We do not implement any TLS client certificate modes based
+  // on key agreement.
+  return check_purpose(x, ca, XKU_SSL_CLIENT,
+                       X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_KEY_AGREEMENT);
 }
 
 // Key usage needed for TLS/SSL server: digital signature, encipherment or
@@ -439,96 +452,35 @@
 
 static int check_purpose_ssl_server(const X509_PURPOSE *xp, const X509 *x,
                                     int ca) {
-  if (xku_reject(x, XKU_SSL_SERVER)) {
-    return 0;
-  }
-  if (ca) {
-    return check_ca(x);
-  }
-
-  if (ku_reject(x, X509v3_KU_TLS)) {
-    return 0;
-  }
-
-  return 1;
+  return check_purpose(x, ca, XKU_SSL_SERVER, X509v3_KU_TLS);
 }
 
 static int check_purpose_ns_ssl_server(const X509_PURPOSE *xp, const X509 *x,
                                        int ca) {
-  int ret = check_purpose_ssl_server(xp, x, ca);
-  if (!ret || ca) {
-    return ret;
-  }
-  // We need to encipher or Netscape complains
-  if (ku_reject(x, X509v3_KU_KEY_ENCIPHERMENT)) {
-    return 0;
-  }
-  return ret;
-}
-
-// purpose_smime returns one if |x| is a valid S/MIME leaf (|ca| is zero) or CA
-// (|ca| is one) certificate, and zero otherwise.
-static int purpose_smime(const X509 *x, int ca) {
-  if (xku_reject(x, XKU_SMIME)) {
-    return 0;
-  }
-  if (ca) {
-    return check_ca(x);
-  }
-  return 1;
+  // We need to encipher or Netscape complains.
+  return check_purpose(x, ca, XKU_SSL_SERVER, X509v3_KU_KEY_ENCIPHERMENT);
 }
 
 static int check_purpose_smime_sign(const X509_PURPOSE *xp, const X509 *x,
                                     int ca) {
-  int ret = purpose_smime(x, ca);
-  if (!ret || ca) {
-    return ret;
-  }
-  if (ku_reject(x, X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_NON_REPUDIATION)) {
-    return 0;
-  }
-  return ret;
+  return check_purpose(x, ca, XKU_SMIME,
+                       X509v3_KU_DIGITAL_SIGNATURE | X509v3_KU_NON_REPUDIATION);
 }
 
 static int check_purpose_smime_encrypt(const X509_PURPOSE *xp, const X509 *x,
                                        int ca) {
-  int ret = purpose_smime(x, ca);
-  if (!ret || ca) {
-    return ret;
-  }
-  if (ku_reject(x, X509v3_KU_KEY_ENCIPHERMENT)) {
-    return 0;
-  }
-  return ret;
+  return check_purpose(x, ca, XKU_SMIME, X509v3_KU_KEY_ENCIPHERMENT);
 }
 
 static int check_purpose_crl_sign(const X509_PURPOSE *xp, const X509 *x,
                                   int ca) {
-  if (ca) {
-    return check_ca(x);
-  }
-  if (ku_reject(x, X509v3_KU_CRL_SIGN)) {
-    return 0;
-  }
-  return 1;
-}
-
-// OCSP helper: this is *not* a full OCSP check. It just checks that each CA
-// is valid. Additional checks must be made on the chain.
-
-static int ocsp_helper(const X509_PURPOSE *xp, const X509 *x, int ca) {
-  if (ca) {
-    return check_ca(x);
-  }
-  // leaf certificate is checked in OCSP_verify()
-  return 1;
+  return check_purpose(x, ca, /*required_xku=*/0, X509v3_KU_CRL_SIGN);
 }
 
 static int check_purpose_timestamp_sign(const X509_PURPOSE *xp, const X509 *x,
                                         int ca) {
-  // If ca is true we must return if this is a valid CA certificate.
   if (ca) {
-    return check_ca(x);
+    return 1;
   }
 
   // Check the optional key usage field:
@@ -544,6 +496,8 @@
   }
 
   // Only time stamp key usage is permitted and it's required.
+  //
+  // TODO(davidben): Should we check EKUs up the chain like the other cases?
   if (!(x->ex_flags & EXFLAG_XKUSAGE) || x->ex_xkusage != XKU_TIMESTAMP) {
     return 0;
   }