Use scopers for embedded ASN1_STRING and X509_ALGOR Ideally we'd do this with the opaque/private struct split, but these structs are public. Change-Id: I38b8fa6a16a4526975c6c451f84d5252422952af Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/97155 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 186651e..f3dd9db 100644
--- a/crypto/asn1/internal.h
+++ b/crypto/asn1/internal.h

@@ -107,6 +107,26 @@
 // freeing `str` itself.
 void asn1_string_cleanup(ASN1_STRING *str);
 
+// A ScopedASN1String is a stack-allocatable `ASN1_STRING` with managed
+// lifetime.
+// TODO(crbug.com/443769299): Once `ASN1_STRING` is no longer public, this can
+// instead use `DECLARE_OPAQUE_STRUCT`.
+class ScopedASN1String {
+ public:
+  explicit ScopedASN1String(int type) { asn1_string_init(&str_, type); }
+  ScopedASN1String(const ScopedASN1String &) = delete;
+  ScopedASN1String &operator=(const ScopedASN1String &) = delete;
+  ~ScopedASN1String() { asn1_string_cleanup(&str_); }
+
+  ASN1_STRING *get() { return &str_; }
+  const ASN1_STRING *get() const { return &str_; }
+  ASN1_STRING *operator->() { return &str_; }
+  const ASN1_STRING *operator->() const { return &str_; }
+
+ private:
+  ASN1_STRING str_;
+};
+
 // asn1_parse_string_unchecked parses a DER-encoded string of type `str_type`,
 // tagged with `tag`. It does not check the contents.
 int asn1_parse_string_unchecked(CBS *cbs, ASN1_STRING *out, int str_type,

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 1f3bba7..f9a09d3 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h

@@ -34,13 +34,28 @@
 
 BSSL_NAMESPACE_BEGIN
 
+void x509_algor_init(X509_ALGOR *alg);
+void x509_algor_cleanup(X509_ALGOR *alg);
+
+// A ScopedX509Algor is a stack-allocatable `X509_ALGOR` with managed lifetime.
+// This cannot use `DECLARE_OPAQUE_STRUCT` because `X509_ALGOR` is a public
+// struct.
+using ScopedX509Algor =
+    internal::StackAllocated<X509_ALGOR, void, x509_algor_init,
+                             x509_algor_cleanup>;
+
+// x509_parse_algorithm parses a DER-encoded, AlgorithmIdentifier from `cbs` and
+// writes the result to `*out`. It returns one on success and zero on error.
+int x509_parse_algorithm(CBS *cbs, X509_ALGOR *out);
+
+// x509_marshal_algorithm marshals `in` as a DER-encoded, AlgorithmIdentifier
+// and writes the result to `out`. It returns one on success and zero on error.
+int x509_marshal_algorithm(CBB *out, const X509_ALGOR *in);
+
 class X509Pubkey : public X509_pubkey_st {
  public:
-  X509Pubkey();
-  ~X509Pubkey();
-
-  X509_ALGOR algor;
-  ASN1_BIT_STRING public_key;
+  ScopedX509Algor algor;
+  ScopedASN1String public_key{V_ASN1_BIT_STRING};
   UniquePtr<EVP_PKEY> pkey;
 };
 
@@ -59,10 +74,9 @@
  public:
   static constexpr bool kAllowUniquePtr = true;
   X509NameEntry();
-  ~X509NameEntry();
 
   UniquePtr<ASN1_OBJECT> object;
-  ASN1_STRING value;
+  ScopedASN1String value{-1};
   int set = 0;
 };
 
@@ -137,19 +151,19 @@
 
   // TBSCertificate fields:
   uint8_t version = X509_VERSION_1;  // One of the `X509_VERSION_*` constants.
-  ASN1_INTEGER serialNumber;
-  X509_ALGOR tbs_sig_alg;
+  ScopedASN1String serialNumber{V_ASN1_INTEGER};
+  ScopedX509Algor tbs_sig_alg;
   X509Name issuer;
-  ASN1_TIME notBefore;
-  ASN1_TIME notAfter;
+  ScopedASN1String notBefore{-1};
+  ScopedASN1String notAfter{-1};
   X509Name subject;
   X509Pubkey key;
   UniquePtr<ASN1_BIT_STRING> issuerUID;            // [ 1 ] optional in v2
   UniquePtr<ASN1_BIT_STRING> subjectUID;           // [ 2 ] optional in v2
   STACK_OF(X509_EXTENSION) *extensions = nullptr;  // [ 3 ] optional in v3
   // Certificate fields:
-  X509_ALGOR sig_alg;
-  ASN1_BIT_STRING signature;
+  ScopedX509Algor sig_alg;
+  ScopedASN1String signature{V_ASN1_BIT_STRING};
   // Other state:
   // buf, if not nullptr, contains a copy of the serialized Certificate.
   // TODO(davidben): Now every parsed `X509` has an underlying `CRYPTO_BUFFER`,
@@ -634,17 +648,6 @@
 
 int x509_name_copy(X509_NAME *dst, const X509_NAME *src);
 
-void x509_algor_init(X509_ALGOR *alg);
-void x509_algor_cleanup(X509_ALGOR *alg);
-
-// x509_parse_algorithm parses a DER-encoded, AlgorithmIdentifier from `cbs` and
-// writes the result to `*out`. It returns one on success and zero on error.
-int x509_parse_algorithm(CBS *cbs, X509_ALGOR *out);
-
-// x509_marshal_algorithm marshals `in` as a DER-encoded, AlgorithmIdentifier
-// and writes the result to `out`. It returns one on success and zero on error.
-int x509_marshal_algorithm(CBB *out, const X509_ALGOR *in);
-
 
 // Standard extensions.
 

diff --git a/crypto/x509/t_req.cc b/crypto/x509/t_req.cc
index 17db341..6096f47 100644
--- a/crypto/x509/t_req.cc
+++ b/crypto/x509/t_req.cc

@@ -82,7 +82,7 @@
   if (!(cflag & X509_FLAG_NO_PUBKEY)) {
     if (BIO_write(bio, "        Subject Public Key Info:\n", 33) <= 0 ||
         BIO_printf(bio, "%12sPublic Key Algorithm: ", "") <= 0 ||
-        i2a_ASN1_OBJECT(bio, FromOpaque(ri->pubkey)->algor.algorithm) <= 0 ||
+        i2a_ASN1_OBJECT(bio, FromOpaque(ri->pubkey)->algor->algorithm) <= 0 ||
         BIO_puts(bio, "\n") <= 0) {
       goto err;
     }

diff --git a/crypto/x509/t_x509.cc b/crypto/x509/t_x509.cc
index a68a7a8..a14ec59 100644
--- a/crypto/x509/t_x509.cc
+++ b/crypto/x509/t_x509.cc

@@ -110,7 +110,7 @@
   }
 
   if (!(cflag & X509_FLAG_NO_SIGNAME)) {
-    if (X509_signature_print(bp, &impl->tbs_sig_alg, nullptr) <= 0) {
+    if (X509_signature_print(bp, impl->tbs_sig_alg.get(), nullptr) <= 0) {
       return 0;
     }
   }
@@ -166,7 +166,7 @@
     if (BIO_printf(bp, "%12sPublic Key Algorithm: ", "") <= 0) {
       return 0;
     }
-    if (i2a_ASN1_OBJECT(bp, impl->key.algor.algorithm) <= 0) {
+    if (i2a_ASN1_OBJECT(bp, impl->key.algor->algorithm) <= 0) {
       return 0;
     }
     if (BIO_puts(bp, "\n") <= 0) {
@@ -207,7 +207,8 @@
   }
 
   if (!(cflag & X509_FLAG_NO_SIGDUMP)) {
-    if (X509_signature_print(bp, &impl->sig_alg, &impl->signature) <= 0) {
+    if (X509_signature_print(bp, impl->sig_alg.get(), impl->signature.get()) <=
+        0) {
       return 0;
     }
   }

diff --git a/crypto/x509/v3_skey.cc b/crypto/x509/v3_skey.cc
index 446e327..3eed1cc 100644
--- a/crypto/x509/v3_skey.cc
+++ b/crypto/x509/v3_skey.cc

@@ -89,7 +89,7 @@
   }
 
   if (ctx->subject_req) {
-    pk = &FromOpaque(ctx->subject_req->req_info->pubkey)->public_key;
+    pk = FromOpaque(ctx->subject_req->req_info->pubkey)->public_key.get();
   } else {
     pk = X509_get0_pubkey_bitstr(ctx->subject_cert);
   }

diff --git a/crypto/x509/x509_cmp.cc b/crypto/x509/x509_cmp.cc
index f30f030..4dea236 100644
--- a/crypto/x509/x509_cmp.cc
+++ b/crypto/x509/x509_cmp.cc

@@ -72,14 +72,12 @@
   return const_cast<X509Name *>(&impl->subject);
 }
 
-ASN1_INTEGER *X509_get_serialNumber(X509 *a) {
-  auto *impl = FromOpaque(a);
-  return &impl->serialNumber;
+ASN1_INTEGER *X509_get_serialNumber(X509 *x509) {
+  return FromOpaque(x509)->serialNumber.get();
 }
 
 const ASN1_INTEGER *X509_get0_serialNumber(const X509 *x509) {
-  const auto *impl = FromOpaque(x509);
-  return &impl->serialNumber;
+  return FromOpaque(x509)->serialNumber.get();
 }
 
 uint32_t X509_subject_name_hash(const X509 *x) {
@@ -216,7 +214,7 @@
   }
   // This function is not const-correct for OpenSSL compatibility.
   auto *impl = FromOpaque(x);
-  return const_cast<ASN1_BIT_STRING *>(&impl->key.public_key);
+  return const_cast<ASN1_BIT_STRING *>(impl->key.public_key.get());
 }
 
 int X509_check_private_key(const X509 *x, const EVP_PKEY *k) {

diff --git a/crypto/x509/x509_set.cc b/crypto/x509/x509_set.cc
index 970d2e6..ae45063 100644
--- a/crypto/x509/x509_set.cc
+++ b/crypto/x509/x509_set.cc

@@ -53,7 +53,7 @@
   }
 
   auto *impl = FromOpaque(x);
-  return ASN1_STRING_copy(&impl->serialNumber, serial);
+  return ASN1_STRING_copy(impl->serialNumber.get(), serial);
 }
 
 int X509_set_issuer_name(X509 *x, const X509_NAME *name) {
@@ -75,7 +75,7 @@
 int X509_set1_notBefore(X509 *x, const ASN1_TIME *tm) {
   // TODO(crbug.com/42290309): Check that `tm->type` is correct.
   auto *impl = FromOpaque(x);
-  return ASN1_STRING_copy(&impl->notBefore, tm);
+  return ASN1_STRING_copy(impl->notBefore.get(), tm);
 }
 
 int X509_set_notBefore(X509 *x, const ASN1_TIME *tm) {
@@ -84,7 +84,7 @@
 
 const ASN1_TIME *X509_get0_notBefore(const X509 *x) {
   auto *impl = FromOpaque(x);
-  return &impl->notBefore;
+  return impl->notBefore.get();
 }
 
 ASN1_TIME *X509_getm_notBefore(X509 *x) {
@@ -92,7 +92,7 @@
   // non-const as this allows mutating `x`. If it comes up for compatibility,
   // we can relax this.
   auto *impl = FromOpaque(x);
-  return &impl->notBefore;
+  return impl->notBefore.get();
 }
 
 ASN1_TIME *X509_get_notBefore(const X509 *x509) {
@@ -100,13 +100,13 @@
   // `X509_getm_notBefore` is const-correct. `X509_get_notBefore` was
   // originally a macro, so it needs to capture both get0 and getm use cases.
   const auto *impl = FromOpaque(x509);
-  return const_cast<ASN1_TIME *>(&impl->notBefore);
+  return const_cast<ASN1_TIME *>(impl->notBefore.get());
 }
 
 int X509_set1_notAfter(X509 *x, const ASN1_TIME *tm) {
   // TODO(crbug.com/42290309): Check that `tm->type` is correct.
   auto *impl = FromOpaque(x);
-  return ASN1_STRING_copy(&impl->notAfter, tm);
+  return ASN1_STRING_copy(impl->notAfter.get(), tm);
 }
 
 int X509_set_notAfter(X509 *x, const ASN1_TIME *tm) {
@@ -114,16 +114,14 @@
 }
 
 const ASN1_TIME *X509_get0_notAfter(const X509 *x) {
-  auto *impl = FromOpaque(x);
-  return &impl->notAfter;
+  return FromOpaque(x)->notAfter.get();
 }
 
 ASN1_TIME *X509_getm_notAfter(X509 *x) {
   // Note this function takes a const `X509` pointer in OpenSSL. We require
   // non-const as this allows mutating `x`. If it comes up for compatibility,
   // we can relax this.
-  auto *impl = FromOpaque(x);
-  return &impl->notAfter;
+  return FromOpaque(x)->notAfter.get();
 }
 
 ASN1_TIME *X509_get_notAfter(const X509 *x509) {
@@ -131,7 +129,7 @@
   // `X509_getm_notAfter` is const-correct. `X509_get_notAfter` was
   // originally a macro, so it needs to capture both get0 and getm use cases.
   const auto *impl = FromOpaque(x509);
-  return const_cast<ASN1_TIME *>(&impl->notAfter);
+  return const_cast<ASN1_TIME *>(impl->notAfter.get());
 }
 
 void X509_get0_uids(const X509 *x509, const ASN1_BIT_STRING **out_issuer_uid,
@@ -154,13 +152,11 @@
 }
 
 const STACK_OF(X509_EXTENSION) *X509_get0_extensions(const X509 *x) {
-  auto *impl = FromOpaque(x);
-  return impl->extensions;
+  return FromOpaque(x)->extensions;
 }
 
 const X509_ALGOR *X509_get0_tbs_sigalg(const X509 *x) {
-  auto *impl = FromOpaque(x);
-  return &impl->tbs_sig_alg;
+  return FromOpaque(x)->tbs_sig_alg.get();
 }
 
 X509_PUBKEY *X509_get_X509_PUBKEY(const X509 *x509) {

diff --git a/crypto/x509/x509name.cc b/crypto/x509/x509name.cc
index 5f8841b..c2b4302 100644
--- a/crypto/x509/x509name.cc
+++ b/crypto/x509/x509name.cc

@@ -344,7 +344,7 @@
     return 0;
   }
   if ((type > 0) && (type & MBSTRING_FLAG)) {
-    ASN1_STRING *dst = &ne_impl->value;
+    ASN1_STRING *dst = ne_impl->value.get();
     return ASN1_STRING_set_by_NID(&dst, bytes, len, type,
                                   OBJ_obj2nid(ne_impl->object.get())) !=
            nullptr;
@@ -352,11 +352,11 @@
   if (len < 0) {
     len = strlen((const char *)bytes);
   }
-  if (!ASN1_STRING_set(&ne_impl->value, bytes, len)) {
+  if (!ASN1_STRING_set(ne_impl->value.get(), bytes, len)) {
     return 0;
   }
   if (type != V_ASN1_UNDEF) {
-    ne_impl->value.type = type;
+    ne_impl->value->type = type;
   }
   return 1;
 }
@@ -373,5 +373,5 @@
     return nullptr;
   }
   // This function is not const-correct for OpenSSL compatibility.
-  return const_cast<ASN1_STRING*>(&FromOpaque(ne)->value);
+  return const_cast<ASN1_STRING*>(FromOpaque(ne)->value.get());
 }

diff --git a/crypto/x509/x_all.cc b/crypto/x509/x_all.cc
index 415d0fc..7a50617 100644
--- a/crypto/x509/x_all.cc
+++ b/crypto/x509/x_all.cc

@@ -37,7 +37,7 @@
 
 int X509_verify(const X509 *x509, EVP_PKEY *pkey) {
   auto *impl = FromOpaque(x509);
-  if (X509_ALGOR_cmp(&impl->sig_alg, &impl->tbs_sig_alg)) {
+  if (X509_ALGOR_cmp(impl->sig_alg.get(), impl->tbs_sig_alg.get())) {
     OPENSSL_PUT_ERROR(X509, X509_R_SIGNATURE_ALGORITHM_MISMATCH);
     return 0;
   }
@@ -46,7 +46,7 @@
   if (!CBB_init(cbb.get(), 128) || !x509_marshal_tbs_cert(cbb.get(), x509)) {
     return 0;
   }
-  return x509_verify_signature(&impl->sig_alg, &impl->signature,
+  return x509_verify_signature(impl->sig_alg.get(), impl->signature.get(),
                                CBBAsSpan(cbb.get()), pkey);
 }
 
@@ -72,8 +72,8 @@
 
   // Fill in the two copies of AlgorithmIdentifier. Note one of these modifies
   // the TBSCertificate.
-  if (!x509_digest_sign_algorithm(ctx, &impl->tbs_sig_alg) ||
-      !x509_digest_sign_algorithm(ctx, &impl->sig_alg)) {
+  if (!x509_digest_sign_algorithm(ctx, impl->tbs_sig_alg.get()) ||
+      !x509_digest_sign_algorithm(ctx, impl->sig_alg.get())) {
     return 0;
   }
 
@@ -84,7 +84,8 @@
   if (!CBB_init(cbb.get(), 128) || !x509_marshal_tbs_cert(cbb.get(), x)) {
     return 0;
   }
-  return x509_sign_to_bit_string(ctx, &impl->signature, CBBAsSpan(cbb.get()));
+  return x509_sign_to_bit_string(ctx, impl->signature.get(),
+                                 CBBAsSpan(cbb.get()));
 }
 
 int X509_REQ_sign(X509_REQ *x, EVP_PKEY *pkey, const EVP_MD *md) {

diff --git a/crypto/x509/x_name.cc b/crypto/x509/x_name.cc
index 7f9ff54..ac1a8cc 100644
--- a/crypto/x509/x_name.cc
+++ b/crypto/x509/x_name.cc

@@ -44,11 +44,8 @@
 
 bssl::X509NameEntry::X509NameEntry() {
   object.reset(const_cast<ASN1_OBJECT *>(OBJ_get_undef()));
-  asn1_string_init(&value, -1);
 }
 
-bssl::X509NameEntry::~X509NameEntry() { asn1_string_cleanup(&value); }
-
 X509_NAME_ENTRY *X509_NAME_ENTRY_new() { return New<X509NameEntry>(); }
 
 void X509_NAME_ENTRY_free(X509_NAME_ENTRY *entry) { Delete(FromOpaque(entry)); }
@@ -62,7 +59,7 @@
   }
   out_impl->object.reset(asn1_parse_object(&seq, /*tag=*/0));
   if (out_impl->object == nullptr ||                        //
-      !asn1_parse_any_as_string(&seq, &out_impl->value) ||  //
+      !asn1_parse_any_as_string(&seq, out_impl->value.get()) ||  //
       CBS_len(&seq) != 0) {
     OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
     return 0;
@@ -78,8 +75,9 @@
       !asn1_marshal_object(&seq, entry_impl->object.get(), /*tag=*/0)) {
     return 0;
   }
-  int ok = canonicalize ? asn1_marshal_string_canon(&seq, &entry_impl->value)
-                        : asn1_marshal_any_string(&seq, &entry_impl->value);
+  int ok = canonicalize
+               ? asn1_marshal_string_canon(&seq, entry_impl->value.get())
+               : asn1_marshal_any_string(&seq, entry_impl->value.get());
   if (!ok) {
     return 0;
   }

diff --git a/crypto/x509/x_pubkey.cc b/crypto/x509/x_pubkey.cc
index 953ad31..4862f1f 100644
--- a/crypto/x509/x_pubkey.cc
+++ b/crypto/x509/x_pubkey.cc

@@ -37,16 +37,6 @@
 
 using namespace bssl;
 
-bssl::X509Pubkey::X509Pubkey() {
-  x509_algor_init(&algor);
-  asn1_string_init(&public_key, V_ASN1_BIT_STRING);
-}
-
-bssl::X509Pubkey::~X509Pubkey() {
-  x509_algor_cleanup(&algor);
-  asn1_string_cleanup(&public_key);
-}
-
 X509_PUBKEY *X509_PUBKEY_new() { return New<X509Pubkey>(); }
 
 void X509_PUBKEY_free(X509_PUBKEY *key) { Delete(FromOpaque(key)); }
@@ -78,8 +68,8 @@
   auto *out_impl = FromOpaque(out);
   CBS seq;
   if (!CBS_get_asn1(cbs, &seq, CBS_ASN1_SEQUENCE) ||
-      !x509_parse_algorithm(&seq, &out_impl->algor) ||
-      !asn1_parse_bit_string(&seq, &out_impl->public_key, /*tag=*/0) ||
+      !x509_parse_algorithm(&seq, out_impl->algor.get()) ||
+      !asn1_parse_bit_string(&seq, out_impl->public_key.get(), /*tag=*/0) ||
       CBS_len(&seq) != 0) {
     OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
     return 0;
@@ -96,8 +86,8 @@
   auto *in_impl = FromOpaque(in);
   CBB seq;
   return CBB_add_asn1(cbb, &seq, CBS_ASN1_SEQUENCE) &&
-         x509_marshal_algorithm(&seq, &in_impl->algor) &&
-         asn1_marshal_bit_string(&seq, &in_impl->public_key, /*tag=*/0) &&
+         x509_marshal_algorithm(&seq, in_impl->algor.get()) &&
+         asn1_marshal_bit_string(&seq, in_impl->public_key.get(), /*tag=*/0) &&
          CBB_flush(cbb);
 }
 
@@ -178,11 +168,11 @@
 int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *obj, int param_type,
                            void *param_value, uint8_t *key, int key_len) {
   auto *pub_impl = FromOpaque(pub);
-  if (!X509_ALGOR_set0(&pub_impl->algor, obj, param_type, param_value)) {
+  if (!X509_ALGOR_set0(pub_impl->algor.get(), obj, param_type, param_value)) {
     return 0;
   }
 
-  ASN1_STRING_set0(&pub_impl->public_key, key, key_len);
+  ASN1_STRING_set0(pub_impl->public_key.get(), key, key_len);
   x509_pubkey_changed(pub_impl, GetDefaultEVPAlgorithms());
   return 1;
 }
@@ -192,18 +182,18 @@
                            X509_PUBKEY *pub) {
   auto *pub_impl = FromOpaque(pub);
   if (out_obj != nullptr) {
-    *out_obj = pub_impl->algor.algorithm;
+    *out_obj = pub_impl->algor->algorithm;
   }
   if (out_key != nullptr) {
-    *out_key = pub_impl->public_key.data;
-    *out_key_len = pub_impl->public_key.length;
+    *out_key = pub_impl->public_key->data;
+    *out_key_len = pub_impl->public_key->length;
   }
   if (out_alg != nullptr) {
-    *out_alg = &pub_impl->algor;
+    *out_alg = pub_impl->algor.get();
   }
   return 1;
 }
 
 const ASN1_BIT_STRING *X509_PUBKEY_get0_public_key(const X509_PUBKEY *pub) {
-  return &FromOpaque(pub)->public_key;
+  return FromOpaque(pub)->public_key.get();
 }

diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index 740124b..21f5380 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc

@@ -45,12 +45,6 @@
     CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 3;
 
 X509Impl::X509Impl() : RefCounted(CheckSubClass()) {
-  asn1_string_init(&serialNumber, V_ASN1_INTEGER);
-  x509_algor_init(&tbs_sig_alg);
-  asn1_string_init(&notBefore, -1);
-  asn1_string_init(&notAfter, -1);
-  x509_algor_init(&sig_alg);
-  asn1_string_init(&signature, V_ASN1_BIT_STRING);
   CRYPTO_new_ex_data(&ex_data);
 }
 
@@ -59,13 +53,7 @@
 X509Impl::~X509Impl() {
   CRYPTO_free_ex_data(&g_ex_data_class, &ex_data);
 
-  asn1_string_cleanup(&serialNumber);
-  x509_algor_cleanup(&tbs_sig_alg);
-  asn1_string_cleanup(&notBefore);
-  asn1_string_cleanup(&notAfter);
   sk_X509_EXTENSION_pop_free(extensions, X509_EXTENSION_free);
-  x509_algor_cleanup(&sig_alg);
-  asn1_string_cleanup(&signature);
   X509_CERT_AUX_free(aux);
 }
 
@@ -97,13 +85,13 @@
       // module often omit overflow checks.
       CBS_len(&cert) > INT_MAX / 2 ||
       !CBS_get_asn1(&cert, &tbs, CBS_ASN1_SEQUENCE) ||
-      !x509_parse_algorithm(&cert, &ret->sig_alg) ||
+      !x509_parse_algorithm(&cert, ret->sig_alg.get()) ||
       // For just the signature field, we accept non-minimal BER lengths, though
       // not indefinite-length encoding. See b/18228011.
       //
       // TODO(crbug.com/boringssl/354): Switch the affected callers to convert
       // the certificate before parsing and then remove this workaround.
-      !asn1_parse_bit_string_with_bad_length(&cert, &ret->signature) ||
+      !asn1_parse_bit_string_with_bad_length(&cert, ret->signature.get()) ||
       CBS_len(&cert) != 0) {
     OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
     return nullptr;
@@ -132,13 +120,13 @@
     ret->version = X509_VERSION_1;
   }
   CBS validity;
-  if (!asn1_parse_integer(&tbs, &ret->serialNumber, /*tag=*/0) ||
-      !x509_parse_algorithm(&tbs, &ret->tbs_sig_alg) ||
+  if (!asn1_parse_integer(&tbs, ret->serialNumber.get(), /*tag=*/0) ||
+      !x509_parse_algorithm(&tbs, ret->tbs_sig_alg.get()) ||
       !x509_parse_name(&tbs, &ret->issuer) ||
       !CBS_get_asn1(&tbs, &validity, CBS_ASN1_SEQUENCE) ||
-      !asn1_parse_time(&validity, &ret->notBefore,
+      !asn1_parse_time(&validity, ret->notBefore.get(),
                        /*allow_utc_timezone_offset=*/1) ||
-      !asn1_parse_time(&validity, &ret->notAfter,
+      !asn1_parse_time(&validity, ret->notAfter.get(),
                        /*allow_utc_timezone_offset=*/1) ||
       CBS_len(&validity) != 0 ||  //
       !x509_parse_name(&tbs, &ret->subject) ||
@@ -238,12 +226,12 @@
       return 0;
     }
   }
-  if (!asn1_marshal_integer(&tbs, &impl->serialNumber, /*tag=*/0) ||
-      !x509_marshal_algorithm(&tbs, &impl->tbs_sig_alg) ||
+  if (!asn1_marshal_integer(&tbs, impl->serialNumber.get(), /*tag=*/0) ||
+      !x509_marshal_algorithm(&tbs, impl->tbs_sig_alg.get()) ||
       !x509_marshal_name(&tbs, &impl->issuer) ||
       !CBB_add_asn1(&tbs, &validity, CBS_ASN1_SEQUENCE) ||
-      !asn1_marshal_time(&validity, &impl->notBefore) ||
-      !asn1_marshal_time(&validity, &impl->notAfter) ||
+      !asn1_marshal_time(&validity, impl->notBefore.get()) ||
+      !asn1_marshal_time(&validity, impl->notAfter.get()) ||
       !x509_marshal_name(&tbs, &impl->subject) ||
       !x509_marshal_public_key(&tbs, &impl->key) ||
       (impl->issuerUID != nullptr &&
@@ -271,8 +259,8 @@
   auto *impl = FromOpaque(x509);
   return CBB_add_asn1(cbb, &cert, CBS_ASN1_SEQUENCE) &&
          x509_marshal_tbs_cert(&cert, x509) &&
-         x509_marshal_algorithm(&cert, &impl->sig_alg) &&
-         asn1_marshal_bit_string(&cert, &impl->signature, /*tag=*/0) &&
+         x509_marshal_algorithm(&cert, impl->sig_alg.get()) &&
+         asn1_marshal_bit_string(&cert, impl->signature.get(), /*tag=*/0) &&
          CBB_flush(cbb);
 }
 
@@ -456,26 +444,26 @@
 
 int X509_set1_signature_algo(X509 *x509, const X509_ALGOR *algo) {
   auto *impl = FromOpaque(x509);
-  return X509_ALGOR_copy(&impl->sig_alg, algo) &&
-         X509_ALGOR_copy(&impl->tbs_sig_alg, algo);
+  return X509_ALGOR_copy(impl->sig_alg.get(), algo) &&
+         X509_ALGOR_copy(impl->tbs_sig_alg.get(), algo);
 }
 
 int X509_set1_signature_value(X509 *x509, const uint8_t *sig, size_t sig_len) {
-  return ASN1_STRING_set(&FromOpaque(x509)->signature, sig, sig_len);
+  return ASN1_STRING_set(FromOpaque(x509)->signature.get(), sig, sig_len);
 }
 
 void X509_get0_signature(const ASN1_BIT_STRING **psig, const X509_ALGOR **palg,
                          const X509 *x) {
   const auto *impl = FromOpaque(x);
   if (psig) {
-    *psig = &impl->signature;
+    *psig = impl->signature.get();
   }
   if (palg) {
-    *palg = &impl->sig_alg;
+    *palg = impl->sig_alg.get();
   }
 }
 
 int X509_get_signature_nid(const X509 *x) {
   const auto *impl = FromOpaque(x);
-  return OBJ_obj2nid(impl->sig_alg.algorithm);
+  return OBJ_obj2nid(impl->sig_alg->algorithm);
 }