Fold x509_pubkey_{init,cleanup} into the class As part of this, make X509_PUBKEY an opaque type. Change-Id: I867ed6df39f5a185e3face317c0a37f40096db6c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/97152 Reviewed-by: Rudolf Polzer <rpolzer@google.com> Commit-Queue: David Benjamin <davidben@google.com>

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 05ccd69..2d5febc 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h

@@ -29,17 +29,19 @@
 DECLARE_OPAQUE_STRUCT(x509_st, X509Impl)
 DECLARE_OPAQUE_STRUCT(x509_store_st, X509Store)
 DECLARE_OPAQUE_STRUCT(X509_name_st, X509Name)
-
-struct X509_pubkey_st {
-  X509_ALGOR algor;
-  ASN1_BIT_STRING public_key;
-  EVP_PKEY *pkey;
-} /* X509_PUBKEY */;
+DECLARE_OPAQUE_STRUCT(X509_pubkey_st, X509Pubkey)
 
 BSSL_NAMESPACE_BEGIN
 
-void x509_pubkey_init(X509_PUBKEY *key);
-void x509_pubkey_cleanup(X509_PUBKEY *key);
+class X509Pubkey : public X509_pubkey_st {
+ public:
+  X509Pubkey();
+  ~X509Pubkey();
+
+  X509_ALGOR algor;
+  ASN1_BIT_STRING public_key;
+  UniquePtr<EVP_PKEY> pkey;
+};
 
 int x509_parse_public_key(CBS *cbs, X509_PUBKEY *out,
                           Span<const EVP_PKEY_ALG *const> algs);
@@ -139,7 +141,7 @@
   ASN1_TIME notBefore;
   ASN1_TIME notAfter;
   X509Name subject;
-  X509_PUBKEY key;
+  X509Pubkey key;
   ASN1_BIT_STRING *issuerUID = nullptr;            // [ 1 ] optional in v2
   ASN1_BIT_STRING *subjectUID = nullptr;           // [ 2 ] optional in v2
   STACK_OF(X509_EXTENSION) *extensions = nullptr;  // [ 3 ] optional in v3

diff --git a/crypto/x509/t_req.cc b/crypto/x509/t_req.cc
index 012dd89..17db341 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, 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/v3_skey.cc b/crypto/x509/v3_skey.cc
index e99fdaa..446e327 100644
--- a/crypto/x509/v3_skey.cc
+++ b/crypto/x509/v3_skey.cc

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

diff --git a/crypto/x509/x509_set.cc b/crypto/x509/x509_set.cc
index 26abf2d..e55829f 100644
--- a/crypto/x509/x509_set.cc
+++ b/crypto/x509/x509_set.cc

@@ -166,5 +166,5 @@
 X509_PUBKEY *X509_get_X509_PUBKEY(const X509 *x509) {
   // This function is not const-correct for OpenSSL compatibility.
   const auto *impl = FromOpaque(x509);
-  return const_cast<X509_PUBKEY *>(&impl->key);
+  return const_cast<X509Pubkey *>(&impl->key);
 }

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

@@ -16,6 +16,8 @@
 
 #include <limits.h>
 
+#include <utility>
+
 #include <openssl/asn1.h>
 #include <openssl/asn1t.h>
 #include <openssl/bytestring.h>
@@ -35,37 +37,22 @@
 
 using namespace bssl;
 
-void bssl::x509_pubkey_init(X509_PUBKEY *key) {
-  OPENSSL_memset(key, 0, sizeof(X509_PUBKEY));
-  x509_algor_init(&key->algor);
-  asn1_string_init(&key->public_key, V_ASN1_BIT_STRING);
+bssl::X509Pubkey::X509Pubkey() {
+  x509_algor_init(&algor);
+  asn1_string_init(&public_key, V_ASN1_BIT_STRING);
 }
 
-X509_PUBKEY *X509_PUBKEY_new() {
-  UniquePtr<X509_PUBKEY> ret = MakeUnique<X509_PUBKEY>();
-  if (ret == nullptr) {
-    return nullptr;
-  }
-  x509_pubkey_init(ret.get());
-  return ret.release();
+bssl::X509Pubkey::~X509Pubkey() {
+  x509_algor_cleanup(&algor);
+  asn1_string_cleanup(&public_key);
 }
 
-void bssl::x509_pubkey_cleanup(X509_PUBKEY *key) {
-  x509_algor_cleanup(&key->algor);
-  asn1_string_cleanup(&key->public_key);
-  EVP_PKEY_free(key->pkey);
-}
+X509_PUBKEY *X509_PUBKEY_new() { return New<X509Pubkey>(); }
 
-void X509_PUBKEY_free(X509_PUBKEY *key) {
-  if (key != nullptr) {
-    x509_pubkey_cleanup(key);
-    OPENSSL_free(key);
-  }
-}
+void X509_PUBKEY_free(X509_PUBKEY *key) { Delete(FromOpaque(key)); }
 
-static void x509_pubkey_changed(X509_PUBKEY *pub,
+static void x509_pubkey_changed(X509Pubkey *pub,
                                 Span<const EVP_PKEY_ALG *const> algs) {
-  EVP_PKEY_free(pub->pkey);
   pub->pkey = nullptr;
 
   // Re-encode the `X509_PUBKEY` to DER and parse it with EVP's APIs. If the
@@ -83,32 +70,34 @@
     return;
   }
 
-  pub->pkey = pkey.release();
+  pub->pkey = std::move(pkey);
 }
 
 int bssl::x509_parse_public_key(CBS *cbs, X509_PUBKEY *out,
                                 Span<const EVP_PKEY_ALG *const> algs) {
+  auto *out_impl = FromOpaque(out);
   CBS seq;
   if (!CBS_get_asn1(cbs, &seq, CBS_ASN1_SEQUENCE) ||
-      !x509_parse_algorithm(&seq, &out->algor) ||
-      !asn1_parse_bit_string(&seq, &out->public_key, /*tag=*/0) ||
+      !x509_parse_algorithm(&seq, &out_impl->algor) ||
+      !asn1_parse_bit_string(&seq, &out_impl->public_key, /*tag=*/0) ||
       CBS_len(&seq) != 0) {
     OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
     return 0;
   }
-  x509_pubkey_changed(out, algs);
+  x509_pubkey_changed(out_impl, algs);
   return 1;
 }
 
 static int x509_parse_public_key_default(CBS *cbs, X509_PUBKEY *out) {
-  return x509_parse_public_key(cbs, out, GetDefaultEVPAlgorithms());
+  return x509_parse_public_key(cbs, FromOpaque(out), GetDefaultEVPAlgorithms());
 }
 
 int bssl::x509_marshal_public_key(CBB *cbb, const X509_PUBKEY *in) {
+  auto *in_impl = FromOpaque(in);
   CBB seq;
   return CBB_add_asn1(cbb, &seq, CBS_ASN1_SEQUENCE) &&
-         x509_marshal_algorithm(&seq, &in->algor) &&
-         asn1_marshal_bit_string(&seq, &in->public_key, /*tag=*/0) &&
+         x509_marshal_algorithm(&seq, &in_impl->algor) &&
+         asn1_marshal_bit_string(&seq, &in_impl->public_key, /*tag=*/0) &&
          CBB_flush(cbb);
 }
 
@@ -124,7 +113,7 @@
 
 int i2d_X509_PUBKEY(const X509_PUBKEY *key, uint8_t **outp) {
   return I2DFromCBB(/*initial_capacity=*/32, outp, [&](CBB *cbb) -> bool {
-    return x509_marshal_public_key(cbb, key);
+    return x509_marshal_public_key(cbb, FromOpaque(key));
   });
 }
 
@@ -156,7 +145,8 @@
 
 int X509_PUBKEY_set(X509_PUBKEY **x, EVP_PKEY *pkey) {
   UniquePtr<X509_PUBKEY> new_key(X509_PUBKEY_new());
-  if (new_key == nullptr || !x509_pubkey_set1(new_key.get(), pkey)) {
+  if (new_key == nullptr ||
+      !x509_pubkey_set1(FromOpaque(new_key.get()), pkey)) {
     return 0;
   }
   X509_PUBKEY_free(*x);
@@ -169,12 +159,12 @@
     return nullptr;
   }
 
-  if (key->pkey == nullptr) {
+  if (FromOpaque(key)->pkey == nullptr) {
     OPENSSL_PUT_ERROR(X509, X509_R_PUBLIC_KEY_DECODE_ERROR);
     return nullptr;
   }
 
-  return key->pkey;
+  return FromOpaque(key)->pkey.get();
 }
 
 EVP_PKEY *X509_PUBKEY_get(const X509_PUBKEY *key) {
@@ -187,31 +177,33 @@
 
 int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *obj, int param_type,
                            void *param_value, uint8_t *key, int key_len) {
-  if (!X509_ALGOR_set0(&pub->algor, obj, param_type, param_value)) {
+  auto *pub_impl = FromOpaque(pub);
+  if (!X509_ALGOR_set0(&pub_impl->algor, obj, param_type, param_value)) {
     return 0;
   }
 
-  ASN1_STRING_set0(&pub->public_key, key, key_len);
-  x509_pubkey_changed(pub, GetDefaultEVPAlgorithms());
+  ASN1_STRING_set0(&pub_impl->public_key, key, key_len);
+  x509_pubkey_changed(pub_impl, GetDefaultEVPAlgorithms());
   return 1;
 }
 
 int X509_PUBKEY_get0_param(ASN1_OBJECT **out_obj, const uint8_t **out_key,
                            int *out_key_len, X509_ALGOR **out_alg,
                            X509_PUBKEY *pub) {
+  auto *pub_impl = FromOpaque(pub);
   if (out_obj != nullptr) {
-    *out_obj = pub->algor.algorithm;
+    *out_obj = pub_impl->algor.algorithm;
   }
   if (out_key != nullptr) {
-    *out_key = pub->public_key.data;
-    *out_key_len = pub->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->algor;
+    *out_alg = &pub_impl->algor;
   }
   return 1;
 }
 
 const ASN1_BIT_STRING *X509_PUBKEY_get0_public_key(const X509_PUBKEY *pub) {
-  return &pub->public_key;
+  return &FromOpaque(pub)->public_key;
 }

diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index 088a2ce..54c3a8c 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc

@@ -49,7 +49,6 @@
   x509_algor_init(&tbs_sig_alg);
   asn1_string_init(&notBefore, -1);
   asn1_string_init(&notAfter, -1);
-  x509_pubkey_init(&key);
   x509_algor_init(&sig_alg);
   asn1_string_init(&signature, V_ASN1_BIT_STRING);
   CRYPTO_new_ex_data(&ex_data);
@@ -64,7 +63,6 @@
   x509_algor_cleanup(&tbs_sig_alg);
   asn1_string_cleanup(&notBefore);
   asn1_string_cleanup(&notAfter);
-  x509_pubkey_cleanup(&key);
   ASN1_BIT_STRING_free(issuerUID);
   ASN1_BIT_STRING_free(subjectUID);
   sk_X509_EXTENSION_pop_free(extensions, X509_EXTENSION_free);