Merge X509_PURPOSE/X509_TRUST IDs and indices

OpenSSL's API uses this weird "index" intermediate integer
representation, which is the same as the ID but offset bit. Just use the
IDs throughout. Also document and deprecate the string-based APIs that
rust-openssl uses.

As a bonus, we remove some int/size_t casts.

Change-Id: I3ffd2ab59bf3c9d96014a028b667b0bd3288b16b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65789
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index d35c7f5..b2b6ad9 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -588,11 +588,8 @@
 // |X509_NAME| issue is resolved.
 int X509_check_akid(X509 *issuer, const AUTHORITY_KEYID *akid);
 
-int X509_TRUST_set(int *t, int trust);
-int X509_TRUST_get_by_id(int id);
+int X509_is_valid_trust_id(int trust);
 
-int X509_PURPOSE_set(int *p, int purpose);
-int X509_PURPOSE_get_by_id(int id);
 int X509_PURPOSE_get_trust(const X509_PURPOSE *xp);
 
 
diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c
index fe6cb0f..78681c5 100644
--- a/crypto/x509/v3_purp.c
+++ b/crypto/x509/v3_purp.c
@@ -54,8 +54,6 @@
  * (eay@cryptsoft.com).  This product includes software written by Tim
  * Hudson (tjh@cryptsoft.com). */
 
-#include <assert.h>
-#include <limits.h>
 #include <string.h>
 
 #include <openssl/digest.h>
@@ -135,8 +133,8 @@
   if (id == -1) {
     return 1;
   }
-  int idx = X509_PURPOSE_get_by_id(id);
-  if (idx == -1) {
+  const X509_PURPOSE *pt = X509_PURPOSE_get0(id);
+  if (pt == NULL) {
     return 0;
   }
   // Historically, |check_purpose| implementations other than |X509_PURPOSE_ANY|
@@ -146,42 +144,22 @@
   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);
 }
 
-int X509_PURPOSE_set(int *p, int purpose) {
-  if (X509_PURPOSE_get_by_id(purpose) == -1) {
-    OPENSSL_PUT_ERROR(X509V3, X509V3_R_INVALID_PURPOSE);
-    return 0;
+const X509_PURPOSE *X509_PURPOSE_get0(int id) {
+  for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(xstandard); i++) {
+    if (xstandard[i].purpose == id) {
+      return &xstandard[i];
+    }
   }
-  *p = purpose;
-  return 1;
-}
-
-const X509_PURPOSE *X509_PURPOSE_get0(int idx) {
-  if (idx < 0 || (size_t)idx >= OPENSSL_ARRAY_SIZE(xstandard)) {
-    return NULL;
-  }
-  return xstandard + idx;
+  return NULL;
 }
 
 int X509_PURPOSE_get_by_sname(const char *sname) {
-  for (int i = 0; i < (int)OPENSSL_ARRAY_SIZE(xstandard); i++) {
-    const X509_PURPOSE *xptmp = X509_PURPOSE_get0(i);
-    if (!strcmp(xptmp->sname, sname)) {
-      return i;
-    }
-  }
-  return -1;
-}
-
-int X509_PURPOSE_get_by_id(int purpose) {
-  for (size_t i = 0; i <OPENSSL_ARRAY_SIZE(xstandard); i++) {
-    if (xstandard[i].purpose == purpose) {
-      static_assert(OPENSSL_ARRAY_SIZE(xstandard) <= INT_MAX,
-                    "indices must fit in int");
-      return (int)i;
+  for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(xstandard); i++) {
+    if (strcmp(xstandard[i].sname, sname) == 0) {
+      return xstandard[i].purpose;
     }
   }
   return -1;
diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c
index a626c88..87137b1 100644
--- a/crypto/x509/x509_trs.c
+++ b/crypto/x509/x509_trs.c
@@ -54,9 +54,6 @@
  * (eay@cryptsoft.com).  This product includes software written by Tim
  * Hudson (tjh@cryptsoft.com). */
 
-#include <assert.h>
-#include <limits.h>
-
 #include <openssl/err.h>
 #include <openssl/mem.h>
 #include <openssl/obj.h>
@@ -74,8 +71,6 @@
   int nid;
 } /* X509_TRUST */;
 
-static const X509_TRUST *X509_TRUST_get0(int idx);
-
 static int trust_1oidany(const X509_TRUST *trust, X509 *x, int flags);
 static int trust_compat(const X509_TRUST *trust, X509 *x, int flags);
 
@@ -89,6 +84,15 @@
     {X509_TRUST_OBJECT_SIGN, trust_1oidany, NID_code_sign},
     {X509_TRUST_TSA, trust_1oidany, NID_time_stamp}};
 
+static const X509_TRUST *X509_TRUST_get0(int id) {
+  for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(trstandard); i++) {
+    if (trstandard[i].trust == id) {
+      return &trstandard[i];
+    }
+  }
+  return NULL;
+}
+
 int X509_check_trust(X509 *x, int id, int flags) {
   if (id == -1) {
     return X509_TRUST_TRUSTED;
@@ -101,39 +105,18 @@
     }
     return trust_compat(NULL, x, 0);
   }
-  int idx = X509_TRUST_get_by_id(id);
-  if (idx == -1) {
+  const X509_TRUST *pt = X509_TRUST_get0(id);
+  if (pt == NULL) {
+    // Unknown trust IDs are silently reintrepreted as NIDs. This is unreachable
+    // from the certificate verifier itself, but wpa_supplicant relies on it.
+    // Note this relies on commonly-used NIDs and trust IDs not colliding.
     return obj_trust(id, x, flags);
   }
-  const X509_TRUST *pt = X509_TRUST_get0(idx);
   return pt->check_trust(pt, x, flags);
 }
 
-static const X509_TRUST *X509_TRUST_get0(int idx) {
-  if (idx < 0 || (size_t)idx >= OPENSSL_ARRAY_SIZE(trstandard)) {
-    return NULL;
-  }
-  return trstandard + idx;
-}
-
-int X509_TRUST_get_by_id(int id) {
-  for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(trstandard); i++) {
-    if (trstandard[i].trust == id) {
-      static_assert(OPENSSL_ARRAY_SIZE(trstandard) <= INT_MAX,
-                    "indices must fit in int");
-      return (int)i;
-    }
-  }
-  return -1;
-}
-
-int X509_TRUST_set(int *t, int trust) {
-  if (X509_TRUST_get_by_id(trust) == -1) {
-    OPENSSL_PUT_ERROR(X509, X509_R_INVALID_TRUST);
-    return 0;
-  }
-  *t = trust;
-  return 1;
+int X509_is_valid_trust_id(int trust) {
+  return X509_TRUST_get0(trust) != NULL;
 }
 
 static int trust_1oidany(const X509_TRUST *trust, X509 *x, int flags) {
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index a6f6215..81e413e 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1485,13 +1485,13 @@
     return 1;
   }
 
-  int idx = X509_PURPOSE_get_by_id(purpose);
-  if (idx == -1) {
+  const X509_PURPOSE *pobj = X509_PURPOSE_get0(purpose);
+  if (pobj == NULL) {
     OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_PURPOSE_ID);
     return 0;
   }
 
-  int trust = X509_PURPOSE_get_trust(X509_PURPOSE_get0(idx));
+  int trust = X509_PURPOSE_get_trust(pobj);
   if (!X509_STORE_CTX_set_trust(ctx, trust)) {
     return 0;
   }
@@ -1508,7 +1508,7 @@
     return 1;
   }
 
-  if (X509_TRUST_get_by_id(trust) == -1) {
+  if (!X509_is_valid_trust_id(trust)) {
     OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_TRUST_ID);
     return 0;
   }
diff --git a/crypto/x509/x509_vpm.c b/crypto/x509/x509_vpm.c
index 7314c44..0640fd0 100644
--- a/crypto/x509/x509_vpm.c
+++ b/crypto/x509/x509_vpm.c
@@ -269,11 +269,22 @@
 }
 
 int X509_VERIFY_PARAM_set_purpose(X509_VERIFY_PARAM *param, int purpose) {
-  return X509_PURPOSE_set(&param->purpose, purpose);
+  if (X509_PURPOSE_get0(purpose) == NULL) {
+    OPENSSL_PUT_ERROR(X509V3, X509V3_R_INVALID_PURPOSE);
+    return 0;
+  }
+  param->purpose = purpose;
+  return 1;
 }
 
 int X509_VERIFY_PARAM_set_trust(X509_VERIFY_PARAM *param, int trust) {
-  return X509_TRUST_set(&param->trust, trust);
+  if (!X509_is_valid_trust_id(trust)) {
+    OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_TRUST_ID);
+    return 0;
+  }
+
+  param->trust = trust;
+  return 1;
 }
 
 void X509_VERIFY_PARAM_set_depth(X509_VERIFY_PARAM *param, int depth) {
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 60d1835..23d643b 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -4313,6 +4313,31 @@
 OPENSSL_EXPORT STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(
     X509_STORE *store);
 
+// X509_PURPOSE_get_by_sname returns the |X509_PURPOSE_*| constant corresponding
+// a short name |sname|, or -1 if |sname| was not recognized.
+//
+// Use |X509_PURPOSE_*| constants directly instead. The short names used by this
+// function look like "sslserver" or "smimeencrypt", so they do not make
+// especially good APIs.
+//
+// This function differs from OpenSSL, which returns an "index" to be passed to
+// |X509_PURPOSE_get0|, followed by |X509_PURPOSE_get_id|, to finally obtain an
+// |X509_PURPOSE_*| value suitable for use with |X509_VERIFY_PARAM_set_purpose|.
+OPENSSL_EXPORT int X509_PURPOSE_get_by_sname(const char *sname);
+
+// X509_PURPOSE_get0 returns the |X509_PURPOSE| object corresponding to |id|,
+// which should be one of the |X509_PURPOSE_*| constants, or NULL if none
+// exists.
+//
+// This function differs from OpenSSL, which takes an "index", returned from
+// |X509_PURPOSE_get_by_sname|. In BoringSSL, indices and |X509_PURPOSE_*| IDs
+// are the same.
+OPENSSL_EXPORT const X509_PURPOSE *X509_PURPOSE_get0(int id);
+
+// X509_PURPOSE_get_id returns |purpose|'s ID. This will be one of the
+// |X509_PURPOSE_*| constants.
+OPENSSL_EXPORT int X509_PURPOSE_get_id(const X509_PURPOSE *purpose);
+
 
 // Private structures.
 
@@ -4859,10 +4884,6 @@
 OPENSSL_EXPORT int X509V3_add1_i2d(STACK_OF(X509_EXTENSION) **x, int nid,
                                    void *value, int crit, unsigned long flags);
 
-OPENSSL_EXPORT int X509_PURPOSE_get_by_sname(const char *sname);
-OPENSSL_EXPORT const X509_PURPOSE *X509_PURPOSE_get0(int idx);
-OPENSSL_EXPORT int X509_PURPOSE_get_id(const X509_PURPOSE *purpose);
-
 
 #if defined(__cplusplus)
 }  // extern C