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 <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

(cherry picked from commit c2b7df5850398dc7c73146c07f6eed95dd363a48)

Author: davidben

crypto/x509/x509_lu.c

@@ -217,21 +217,6 @@ int X509_STORE_up_ref(X509_STORE *store) {
   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;
@@ -254,7 +239,7 @@ void X509_STORE_free(X509_STORE *vfy) {
     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);
@@ -328,7 +313,7 @@ static int x509_store_add(X509_STORE *ctx, void *x, int is_crl) {
     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;
   }
@@ -354,8 +339,7 @@ static int x509_store_add(X509_STORE *ctx, void *x, int is_crl) {
   CRYPTO_MUTEX_unlock_write(&ctx->objs_lock);
 
   if (!added) {
-    X509_OBJECT_free_contents(obj);
-    OPENSSL_free(obj);
+    X509_OBJECT_free(obj);
   }
 
   return ret;
@@ -370,11 +354,15 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) {
 }
 
 X509_OBJECT *X509_OBJECT_new(void) {
-  X509_OBJECT *ret = OPENSSL_zalloc(sizeof(X509_OBJECT));
-  if (ret == NULL) {
-    return NULL;
+  return OPENSSL_zalloc(sizeof(X509_OBJECT));
+}
+
+void X509_OBJECT_free(X509_OBJECT *obj) {
+  if (obj == NULL) {
+    return;
   }
-  return ret;
+  X509_OBJECT_free_contents(obj);
+  OPENSSL_free(obj);
 }
 
 int X509_OBJECT_up_ref_count(X509_OBJECT *a) {
@@ -398,11 +386,8 @@ void X509_OBJECT_free_contents(X509_OBJECT *a) {
       X509_CRL_free(a->data.crl);
       break;
   }
-}
 
-void X509_OBJECT_free(X509_OBJECT *a) {
-  X509_OBJECT_free_contents(a);
-  OPENSSL_free(a);
+  OPENSSL_memset(a, 0, sizeof(X509_OBJECT));
 }
 
 int X509_OBJECT_get_type(const X509_OBJECT *a) { return a->type; }

include/openssl/x509.h

@@ -2820,6 +2820,7 @@ The X509_STORE then calls a function to actually verify the
 certificate chain.
 */
 
+#define X509_LU_NONE 0
 #define X509_LU_X509 1
 #define X509_LU_CRL 2
 #define X509_LU_PKEY 3
@@ -3016,6 +3017,22 @@ OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_retrieve_by_subject(
     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 int X509_OBJECT_get_type(const X509_OBJECT *a);
 OPENSSL_EXPORT X509 *X509_OBJECT_get0_X509(const X509_OBJECT *a);