Add X509_OBJECT_new and X509_OBJECT_free

This is a bit of a mess. The immediate motivation here is that there is
no legitimate reason to ever call X509_OBJECT_free_contents outside of
the library. Unsurprisingly, this means rust-openssl uses it.

rust-openssl uses it because they want to be able to free X509_OBJECTs.
Add OpenSSL 1.1.x's X509_OBJECT_free, which is what they should be using
it instead. As it turns out, they don't *actually* need to free
X509_OBJECTs. This is just some design mistake that cause them to need
free functions for types they never free. On top of that, the only
reason rust-openssl references X509_OBJECT is for
X509_STORE_get0_objects, but their use of that API is a Rust safety
violation anyway. It's all a mess.

As for whether freeing it ever makes sense, the question is whether
X509_STORE_get_by_subject needs to be a public API. In so far as it is
public, callers would need to create empty X509_OBJECTs as an output,
now that X509_OBJECT is opaque. There are also other users of
X509_STORE_get0_objects that might benefit from an
X509_STORE_get1_objects, in which case X509_OBJECT_free will be useful.

For now just to unblock fixing the more immediate rust-openssl mistake
(rather than the underlying mistake), add the APIs that
X509_STORE_get_by_subject callers would need if they existed.

There's quite a bit to clean up around X509_OBJECT, but start by adding
these APIs.

As part of this, since rust-openssl prevents us from removing
X509_OBJECT_free_contents, deprecate it and fix it to leave the
X509_OBJECT in a self-consistent state. (This is moot because
rust-openssl will never call it, but still.)

Change-Id: I78708f2d2464eb9a18844fef0d62cb0a727b9f47
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64129
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/x509_lu.c

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 814f0ce..d113cee 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -205,21 +205,6 @@
   return 1;
 }
 
-static void cleanup(X509_OBJECT *a) {
-  if (a == NULL) {
-    return;
-  }
-  if (a->type == X509_LU_X509) {
-    X509_free(a->data.x509);
-  } else if (a->type == X509_LU_CRL) {
-    X509_CRL_free(a->data.crl);
-  } else {
-    // abort();
-  }
-
-  OPENSSL_free(a);
-}
-
 void X509_STORE_free(X509_STORE *vfy) {
   size_t j;
   STACK_OF(X509_LOOKUP) *sk;
@@ -242,7 +227,7 @@
     X509_LOOKUP_free(lu);
   }
   sk_X509_LOOKUP_free(sk);
-  sk_X509_OBJECT_pop_free(vfy->objs, cleanup);
+  sk_X509_OBJECT_pop_free(vfy->objs, X509_OBJECT_free);
 
   if (vfy->param) {
     X509_VERIFY_PARAM_free(vfy->param);
@@ -316,7 +301,7 @@
     return 0;
   }
 
-  X509_OBJECT *const obj = (X509_OBJECT *)OPENSSL_malloc(sizeof(X509_OBJECT));
+  X509_OBJECT *const obj = X509_OBJECT_new();
   if (obj == NULL) {
     return 0;
   }
@@ -342,8 +327,7 @@
   CRYPTO_MUTEX_unlock_write(&ctx->objs_lock);
 
   if (!added) {
-    X509_OBJECT_free_contents(obj);
-    OPENSSL_free(obj);
+    X509_OBJECT_free(obj);
   }
 
   return ret;
@@ -357,6 +341,18 @@
   return x509_store_add(ctx, x, /*is_crl=*/1);
 }
 
+X509_OBJECT *X509_OBJECT_new(void) {
+  return OPENSSL_zalloc(sizeof(X509_OBJECT));
+}
+
+void X509_OBJECT_free(X509_OBJECT *obj) {
+  if (obj == NULL) {
+    return;
+  }
+  X509_OBJECT_free_contents(obj);
+  OPENSSL_free(obj);
+}
+
 int X509_OBJECT_up_ref_count(X509_OBJECT *a) {
   switch (a->type) {
     case X509_LU_X509:
@@ -378,6 +374,8 @@
       X509_CRL_free(a->data.crl);
       break;
   }
+
+  OPENSSL_memset(a, 0, sizeof(X509_OBJECT));
 }
 
 int X509_OBJECT_get_type(const X509_OBJECT *a) { return a->type; }

include/openssl/x509.h

diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index df3dd58..dfe66c5 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -2582,6 +2582,13 @@
 OPENSSL_EXPORT X509_STORE_CTX *X509_STORE_CTX_get0_parent_ctx(
     X509_STORE_CTX *ctx);
 
+// X509_OBJECT_free_contents sets |obj| to the empty object, freeing any values
+// that were previously there.
+//
+// TODO(davidben): Unexport this function after rust-openssl is fixed to no
+// longer call it.
+OPENSSL_EXPORT void X509_OBJECT_free_contents(X509_OBJECT *obj);
+
 
 // Private structures.
 
@@ -2723,6 +2730,7 @@
 certificate chain.
 */
 
+#define X509_LU_NONE 0
 #define X509_LU_X509 1
 #define X509_LU_CRL 2
 #define X509_LU_PKEY 3
@@ -2900,10 +2908,23 @@
     STACK_OF(X509_OBJECT) *h, int type, X509_NAME *name);
 OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
                                                        X509_OBJECT *x);
+
+// X509_OBJECT_new returns a newly-allocated, empty |X509_OBJECT| or NULL on
+// error.
+OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_new(void);
+
+// X509_OBJECT_free releases memory associated with |obj|.
+OPENSSL_EXPORT void X509_OBJECT_free(X509_OBJECT *obj);
+
+// X509_OBJECT_get_type returns the type of |obj|, which will be one of the
+// |X509_LU_*| constants.
+OPENSSL_EXPORT int X509_OBJECT_get_type(const X509_OBJECT *obj);
+
+// X509_OBJECT_get0_X509 returns |obj| as a certificate, or NULL if |obj| is not
+// a certificate.
+OPENSSL_EXPORT X509 *X509_OBJECT_get0_X509(const X509_OBJECT *obj);
+
 OPENSSL_EXPORT int X509_OBJECT_up_ref_count(X509_OBJECT *a);
-OPENSSL_EXPORT void X509_OBJECT_free_contents(X509_OBJECT *a);
-OPENSSL_EXPORT int X509_OBJECT_get_type(const X509_OBJECT *a);
-OPENSSL_EXPORT X509 *X509_OBJECT_get0_X509(const X509_OBJECT *a);
 OPENSSL_EXPORT X509_STORE *X509_STORE_new(void);
 OPENSSL_EXPORT int X509_STORE_up_ref(X509_STORE *store);
 OPENSSL_EXPORT void X509_STORE_free(X509_STORE *v);