Support X509 objects with extern ASN1_ITEM helpers

For lower-level components, it's preferable to implement a "parse into"
hook so we can parse into an embedded object and avoid an allocation.
Other times, it's too much of a mess to reset cached state and we'd
rather return a newly-allocated object.

Change-Id: I9690b2ad4a4718eb227ea69ba57fc978cbfe85d0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/97156
Reviewed-by: Rudolf Polzer <rpolzer@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h
index 6013057..d774c9a 100644
--- a/crypto/asn1/internal.h
+++ b/crypto/asn1/internal.h
@@ -312,8 +312,10 @@
   ASN1_ex_i2d *asn1_ex_i2d;
 } ASN1_EXTERN_FUNCS;
 
-#define IMPLEMENT_EXTERN_ASN1_SIMPLE(name, new_func, free_func, tag,           \
-                                     parse_func, i2d_func)                     \
+// IMPLEMENT_EXTERN_ASN1_PARSE_NEW implements an `ASN1_ITEM` for a type whose
+// parse function returns a new object.
+#define IMPLEMENT_EXTERN_ASN1_PARSE_NEW(name, new_func, free_func, tag,        \
+                                        parse_func, i2d_func)                  \
   static int name##_new_cb(ASN1_VALUE **pval, const ASN1_ITEM *it) {           \
     *pval = (ASN1_VALUE *)new_func();                                          \
     return *pval != nullptr;                                                   \
@@ -330,10 +332,12 @@
       return 1;                                                                \
     }                                                                          \
                                                                                \
-    if ((*pval == nullptr && !name##_new_cb(pval, it)) ||                      \
-        !parse_func(cbs, (name *)*pval)) {                                     \
+    bssl::UniquePtr<name> ret = parse_func(cbs);                               \
+    if (ret == nullptr) {                                                      \
       return 0;                                                                \
     }                                                                          \
+    name##_free_cb(pval, it);                                                  \
+    *pval = (ASN1_VALUE *)ret.release();                                       \
     return 1;                                                                  \
   }                                                                            \
                                                                                \
@@ -347,6 +351,22 @@
                                                                                \
   IMPLEMENT_EXTERN_ASN1(name, name##_extern_funcs)
 
+// IMPLEMENT_EXTERN_ASN1_PARSE_INTO implements an `ASN1_ITEM` for a type whose
+// parse function writes into an existing object.
+#define IMPLEMENT_EXTERN_ASN1_PARSE_INTO(name, new_func, free_func, tag, \
+                                         parse_func, i2d_func)           \
+                                                                         \
+  static bssl::UniquePtr<name> name##_parse_new(CBS *cbs) {              \
+    bssl::UniquePtr<name> ret(new_func());                               \
+    if (ret == nullptr || !parse_func(cbs, ret.get())) {                 \
+      return 0;                                                          \
+    }                                                                    \
+    return ret;                                                          \
+  }                                                                      \
+                                                                         \
+  IMPLEMENT_EXTERN_ASN1_PARSE_NEW(name, new_func, free_func, tag,        \
+                                  name##_parse_new, i2d_func)
+
 // ASN1_TIME is an `ASN1_ITEM` whose ASN.1 type is X.509 Time (RFC 5280) and C
 // type is `ASN1_TIME*`.
 DECLARE_ASN1_ITEM(ASN1_TIME)
diff --git a/crypto/x509/x_algor.cc b/crypto/x509/x_algor.cc
index 35dd6da..2d19237 100644
--- a/crypto/x509/x_algor.cc
+++ b/crypto/x509/x_algor.cc
@@ -112,9 +112,9 @@
   });
 }
 
-IMPLEMENT_EXTERN_ASN1_SIMPLE(X509_ALGOR, X509_ALGOR_new, X509_ALGOR_free,
-                             CBS_ASN1_SEQUENCE, x509_parse_algorithm,
-                             i2d_X509_ALGOR)
+IMPLEMENT_EXTERN_ASN1_PARSE_INTO(X509_ALGOR, X509_ALGOR_new, X509_ALGOR_free,
+                                 CBS_ASN1_SEQUENCE, x509_parse_algorithm,
+                                 i2d_X509_ALGOR)
 
 X509_ALGOR *X509_ALGOR_dup(const X509_ALGOR *alg) {
   UniquePtr<X509_ALGOR> copy(X509_ALGOR_new());
diff --git a/crypto/x509/x_name.cc b/crypto/x509/x_name.cc
index 9b2afc9..1bde6e1 100644
--- a/crypto/x509/x_name.cc
+++ b/crypto/x509/x_name.cc
@@ -101,9 +101,9 @@
 
 BSSL_NAMESPACE_BEGIN
 
-IMPLEMENT_EXTERN_ASN1_SIMPLE(X509_NAME_ENTRY, X509_NAME_ENTRY_new,
-                             X509_NAME_ENTRY_free, CBS_ASN1_SEQUENCE,
-                             x509_parse_name_entry, i2d_x509_name_entry)
+IMPLEMENT_EXTERN_ASN1_PARSE_INTO(X509_NAME_ENTRY, X509_NAME_ENTRY_new,
+                                 X509_NAME_ENTRY_free, CBS_ASN1_SEQUENCE,
+                                 x509_parse_name_entry, i2d_x509_name_entry)
 
 BSSL_NAMESPACE_END
 
@@ -327,8 +327,9 @@
   return len;
 }
 
-IMPLEMENT_EXTERN_ASN1_SIMPLE(X509_NAME, X509_NAME_new, X509_NAME_free,
-                             CBS_ASN1_SEQUENCE, x509_parse_name, i2d_X509_NAME)
+IMPLEMENT_EXTERN_ASN1_PARSE_INTO(X509_NAME, X509_NAME_new, X509_NAME_free,
+                                 CBS_ASN1_SEQUENCE, x509_parse_name,
+                                 i2d_X509_NAME)
 
 static int asn1_marshal_string_canon(CBB *cbb, const ASN1_STRING *in) {
   int (*decode_func)(CBS *, uint32_t *);
diff --git a/crypto/x509/x_pubkey.cc b/crypto/x509/x_pubkey.cc
index 8cf2846..4dd56e1 100644
--- a/crypto/x509/x_pubkey.cc
+++ b/crypto/x509/x_pubkey.cc
@@ -132,9 +132,9 @@
 
 // TODO(crbug.com/42290417): Remove this when `X509` and `X509_REQ` no longer
 // depend on the tables.
-IMPLEMENT_EXTERN_ASN1_SIMPLE(X509_PUBKEY, X509_PUBKEY_new, X509_PUBKEY_free,
-                             CBS_ASN1_SEQUENCE, x509_parse_public_key_default,
-                             i2d_X509_PUBKEY)
+IMPLEMENT_EXTERN_ASN1_PARSE_INTO(X509_PUBKEY, X509_PUBKEY_new, X509_PUBKEY_free,
+                                 CBS_ASN1_SEQUENCE,
+                                 x509_parse_public_key_default, i2d_X509_PUBKEY)
 
 BSSL_NAMESPACE_END
 
diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index a7b7e4c..91b69d1 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -304,40 +304,8 @@
       [&](CBB *cbb) -> bool { return x509_marshal(cbb, x509); });
 }
 
-static int x509_new_cb(ASN1_VALUE **pval, const ASN1_ITEM *it) {
-  *pval = (ASN1_VALUE *)X509_new();
-  return *pval != nullptr;
-}
-
-static void x509_free_cb(ASN1_VALUE **pval, const ASN1_ITEM *it) {
-  X509_free((X509 *)*pval);
-  *pval = nullptr;
-}
-
-static int x509_parse_cb(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it,
-                         int opt) {
-  if (opt && !CBS_peek_asn1_tag(cbs, CBS_ASN1_SEQUENCE)) {
-    return 1;
-  }
-
-  UniquePtr<X509> ret = x509_parse(cbs);
-  if (ret == nullptr) {
-    return 0;
-  }
-
-  X509_free((X509 *)*pval);
-  *pval = (ASN1_VALUE *)ret.release();
-  return 1;
-}
-
-static int x509_i2d_cb(ASN1_VALUE **pval, unsigned char **out,
-                       const ASN1_ITEM *it) {
-  return i2d_X509((X509 *)*pval, out);
-}
-
-static const ASN1_EXTERN_FUNCS x509_extern_funcs = {x509_new_cb, x509_free_cb,
-                                                    x509_parse_cb, x509_i2d_cb};
-IMPLEMENT_EXTERN_ASN1(X509, x509_extern_funcs)
+IMPLEMENT_EXTERN_ASN1_PARSE_NEW(X509, X509_new, X509_free, CBS_ASN1_SEQUENCE,
+                                x509_parse, i2d_X509)
 
 X509 *X509_dup(const X509 *x509) {
   uint8_t *der = nullptr;