Reference-count CRYPTO_BUFFER_POOLs

SSL_CTX_set0_buffer_pool was a difficult API to use correctly, if you
tried to free your buffer pools. (It was envisioned as being mostly used
with global pools.)

You had to ensure the pool outlives the SSL_CTX, but the SSL_CTX is
reference-counted, so it is hard to be sure when the SSL_CTX will be
destroyed. SSL objects, in particular, extend the lifetime.

Instead, just reference-count the pools. Note that buffers still do not
own references to their pools. That is still a weak reference. We
probably could promote it to a strong reference now, but no need to keep
the hash table around when we don't need to.

With that, deprecated SSL_CTX_set0_buffer_pool in favor of
SSL_CTX_set1_buffer_pool, though SSL_CTX_set0_buffer_pool now internally
takes the refcount to and becomes safer automatically. (This should be
backwards-compatible.) I've switched calls within the library to the
set1 spelling but left bssl-tls alone for now, since bssl-tls can
probably take other simplifications at the same time.

Change-Id: I1421b44cc129c7f2d7090538ac595a5530688f2f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/94467
Reviewed-by: Xiangfei Ding <xfding@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Xiangfei Ding <xfding@google.com>
Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com>

PORTING.md

@@ -293,7 +293,7 @@ With the standard OpenSSL APIs, when making many TLS connections, the certificat
 
 A [`CRYPTO_BUFFER`](https://commondatastorage.googleapis.com/chromium-boringssl-docs/pool.h.html) is just an opaque byte string. A `CRYPTO_BUFFER_POOL` is an intern table for these buffers, i.e. it ensures that only a single copy of any given byte string is kept for each pool.
 
-The function `TLS_with_buffers_method` returns an `SSL_METHOD` that avoids creating `X509` objects for certificates. Additionally, `SSL_CTX_set0_buffer_pool` can be used to install a pool on an `SSL_CTX` so that certificates can be deduplicated across connections and across `SSL_CTX`s.
+The function `TLS_with_buffers_method` returns an `SSL_METHOD` that avoids creating `X509` objects for certificates. Additionally, `SSL_CTX_set1_buffer_pool` can be used to install a pool on an `SSL_CTX` so that certificates can be deduplicated across connections and across `SSL_CTX`s.
 
 When using these functions, the application also needs to ensure that it doesn't call other functions that deal with `X509` or `X509_NAME` objects. For example, `SSL_get_peer_certificate` or `SSL_get_peer_cert_chain`. Doing so will trigger an assert in debug mode and will result in NULLs in release mode. Instead, call the buffer-based alternatives such as `SSL_get0_peer_certificates`. (See [ssl.h](https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html) for functions taking or returning `CRYPTO_BUFFER`.) The buffer-based alternative functions will work even when not using `TLS_with_buffers_method`, thus application code can transition gradually.
 

crypto/pool/internal.h

@@ -68,11 +68,10 @@ class CryptoBuffer : public crypto_buffer_st {
 
 DEFINE_LHASH_OF(CryptoBuffer)
 
-class CryptoBufferPool : public crypto_buffer_pool_st {
+class CryptoBufferPool : public crypto_buffer_pool_st,
+                         public RefCounted<CryptoBufferPool> {
  public:
-  static constexpr bool kAllowUniquePtr = true;
   CryptoBufferPool();
-  ~CryptoBufferPool();
 
   // Hash returns the hash of |data|.
   uint32_t Hash(Span<const uint8_t> data) const;
@@ -85,6 +84,10 @@ class CryptoBufferPool : public crypto_buffer_pool_st {
   UniquePtr<CryptoBufferPoolHandle> handle_;
   LHASH_OF(CryptoBuffer) *bufs_ = nullptr;
   uint64_t hash_key_[2];
+
+ private:
+  friend RefCounted;
+  ~CryptoBufferPool();
 };
 
 BSSL_NAMESPACE_END

crypto/pool/pool.cc

@@ -42,7 +42,7 @@ static int CRYPTO_BUFFER_cmp(const CryptoBuffer *a, const CryptoBuffer *b) {
   return a->span() == b->span() ? 0 : 1;
 }
 
-CryptoBufferPool::CryptoBufferPool() {
+CryptoBufferPool::CryptoBufferPool() : RefCounted(CheckSubClass()) {
   RAND_bytes(reinterpret_cast<uint8_t *>(&hash_key_), sizeof(hash_key_));
 }
 
@@ -85,7 +85,14 @@ CRYPTO_BUFFER_POOL *CRYPTO_BUFFER_POOL_new() {
 }
 
 void CRYPTO_BUFFER_POOL_free(CRYPTO_BUFFER_POOL *pool) {
-  Delete(FromOpaque(pool));
+  if (pool != nullptr) {
+    FromOpaque(pool)->DecRefInternal();
+  }
+}
+
+int CRYPTO_BUFFER_POOL_up_ref(CRYPTO_BUFFER_POOL *pool) {
+  FromOpaque(pool)->UpRefInternal();
+  return 1;
 }
 
 void CryptoBuffer::UpRefInternal() {

crypto/pool/pool_test.cc

@@ -14,6 +14,7 @@
 
 #include <gtest/gtest.h>
 
+#include <openssl/base.h>
 #include <openssl/pool.h>
 
 #include "internal.h"
@@ -125,6 +126,9 @@ TEST(PoolTest, Pooled) {
   UniquePtr<CRYPTO_BUFFER> buf7(
       CRYPTO_BUFFER_new(kData1, sizeof(kData1), pool.get()));
   EXPECT_EQ(buf7.get(), buf6.get());
+
+  // Pools are reference-counted.
+  UniquePtr<CRYPTO_BUFFER_POOL> pool2 = UpRef(pool);
 }
 
 // Buffers are allowed to outlive pools.

include/openssl/pool.h

@@ -50,9 +50,15 @@ DEFINE_STACK_OF(CRYPTO_BUFFER)
 // NULL on error.
 OPENSSL_EXPORT CRYPTO_BUFFER_POOL* CRYPTO_BUFFER_POOL_new(void);
 
-// CRYPTO_BUFFER_POOL_free frees |pool|, which must be empty.
+// CRYPTO_BUFFER_POOL_free decrements the reference count of |pool| and frees it
+// if the reference count drops to zero.
 OPENSSL_EXPORT void CRYPTO_BUFFER_POOL_free(CRYPTO_BUFFER_POOL *pool);
 
+// CRYPTO_BUFFER_POOL_up_ref increments the reference count of |pool| and
+// returns one. It does not mutate |pool| for thread-safety purposes and may be
+// used concurrently.
+OPENSSL_EXPORT int CRYPTO_BUFFER_POOL_up_ref(CRYPTO_BUFFER_POOL *pool);
+
 // CRYPTO_BUFFER_new returns a |CRYPTO_BUFFER| containing a copy of |data|, or
 // else NULL on error. If |pool| is not NULL then the returned value may be a
 // reference to a previously existing |CRYPTO_BUFFER| that contained the same
@@ -113,6 +119,7 @@ extern "C++" {
 BSSL_NAMESPACE_BEGIN
 
 BORINGSSL_MAKE_DELETER(CRYPTO_BUFFER_POOL, CRYPTO_BUFFER_POOL_free)
+BORINGSSL_MAKE_UP_REF(CRYPTO_BUFFER_POOL, CRYPTO_BUFFER_POOL_up_ref)
 BORINGSSL_MAKE_DELETER(CRYPTO_BUFFER, CRYPTO_BUFFER_free)
 BORINGSSL_MAKE_UP_REF(CRYPTO_BUFFER, CRYPTO_BUFFER_up_ref)
 

include/openssl/prefix_symbols.h

@@ -613,6 +613,7 @@
 #pragma redefine_extname CRL_DIST_POINTS_new BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRL_DIST_POINTS_new)
 #pragma redefine_extname CRYPTO_BUFFER_POOL_free BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRYPTO_BUFFER_POOL_free)
 #pragma redefine_extname CRYPTO_BUFFER_POOL_new BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRYPTO_BUFFER_POOL_new)
+#pragma redefine_extname CRYPTO_BUFFER_POOL_up_ref BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRYPTO_BUFFER_POOL_up_ref)
 #pragma redefine_extname CRYPTO_BUFFER_alloc BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRYPTO_BUFFER_alloc)
 #pragma redefine_extname CRYPTO_BUFFER_data BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRYPTO_BUFFER_data)
 #pragma redefine_extname CRYPTO_BUFFER_free BORINGSSL_ADD_USER_LABEL_AND_PREFIX(CRYPTO_BUFFER_free)
@@ -1969,6 +1970,7 @@
 #pragma redefine_extname SSL_CTX_set0_verify_cert_store BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set0_verify_cert_store)
 #pragma redefine_extname SSL_CTX_set1_accepted_peer_cert_types BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set1_accepted_peer_cert_types)
 #pragma redefine_extname SSL_CTX_set1_available_client_cert_types BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set1_available_client_cert_types)
+#pragma redefine_extname SSL_CTX_set1_buffer_pool BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set1_buffer_pool)
 #pragma redefine_extname SSL_CTX_set1_chain BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set1_chain)
 #pragma redefine_extname SSL_CTX_set1_curves BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set1_curves)
 #pragma redefine_extname SSL_CTX_set1_curves_list BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set1_curves_list)
@@ -3714,6 +3716,7 @@
 #define CRL_DIST_POINTS_new BORINGSSL_ADD_PREFIX(CRL_DIST_POINTS_new)
 #define CRYPTO_BUFFER_POOL_free BORINGSSL_ADD_PREFIX(CRYPTO_BUFFER_POOL_free)
 #define CRYPTO_BUFFER_POOL_new BORINGSSL_ADD_PREFIX(CRYPTO_BUFFER_POOL_new)
+#define CRYPTO_BUFFER_POOL_up_ref BORINGSSL_ADD_PREFIX(CRYPTO_BUFFER_POOL_up_ref)
 #define CRYPTO_BUFFER_alloc BORINGSSL_ADD_PREFIX(CRYPTO_BUFFER_alloc)
 #define CRYPTO_BUFFER_data BORINGSSL_ADD_PREFIX(CRYPTO_BUFFER_data)
 #define CRYPTO_BUFFER_free BORINGSSL_ADD_PREFIX(CRYPTO_BUFFER_free)
@@ -5070,6 +5073,7 @@
 #define SSL_CTX_set0_verify_cert_store BORINGSSL_ADD_PREFIX(SSL_CTX_set0_verify_cert_store)
 #define SSL_CTX_set1_accepted_peer_cert_types BORINGSSL_ADD_PREFIX(SSL_CTX_set1_accepted_peer_cert_types)
 #define SSL_CTX_set1_available_client_cert_types BORINGSSL_ADD_PREFIX(SSL_CTX_set1_available_client_cert_types)
+#define SSL_CTX_set1_buffer_pool BORINGSSL_ADD_PREFIX(SSL_CTX_set1_buffer_pool)
 #define SSL_CTX_set1_chain BORINGSSL_ADD_PREFIX(SSL_CTX_set1_chain)
 #define SSL_CTX_set1_curves BORINGSSL_ADD_PREFIX(SSL_CTX_set1_curves)
 #define SSL_CTX_set1_curves_list BORINGSSL_ADD_PREFIX(SSL_CTX_set1_curves_list)

include/openssl/ssl.h

@@ -730,17 +730,10 @@ OPENSSL_EXPORT uint32_t SSL_clear_mode(SSL *ssl, uint32_t mode);
 // modes enabled for |ssl|.
 OPENSSL_EXPORT uint32_t SSL_get_mode(const SSL *ssl);
 
-// SSL_CTX_set0_buffer_pool sets a |CRYPTO_BUFFER_POOL| that will be used to
+// SSL_CTX_set1_buffer_pool sets a |CRYPTO_BUFFER_POOL| that will be used to
 // store certificates. This can allow multiple connections to share
 // certificates and thus save memory.
-//
-// |ctx| does not take ownership of |pool|. The caller must ensure that |pool|
-// outlives |ctx|, as well as any |SSL| objects referencing |ctx|. (|SSL|
-// objects will increment an |SSL_CTX|'s reference count.) It is not necessary
-// for |pool| to outlive any |CRYPTO_BUFFER|s derived from it, so there is no
-// lifetime constraint on |X509| or |SSL_SESSION| objects created from the
-// connection.
-OPENSSL_EXPORT void SSL_CTX_set0_buffer_pool(SSL_CTX *ctx,
+OPENSSL_EXPORT void SSL_CTX_set1_buffer_pool(SSL_CTX *ctx,
                                              CRYPTO_BUFFER_POOL *pool);
 
 
@@ -6236,6 +6229,19 @@ OPENSSL_EXPORT int SSL_check_private_key(const SSL *ssl);
 // security levels mechanism is not used.
 OPENSSL_EXPORT int SSL_CTX_get_security_level(const SSL_CTX *ctx);
 
+// SSL_CTX_set0_buffer_pool calls |SSL_CTX_set1_buffer_pool|. Use
+// |SSL_CTX_set1_buffer_pool| instead.
+//
+// WARNING: Despite being named set0, this function does not adopt the caller's
+// reference to |pool| and instead increments its own reference like a set1
+// function. Historically, |CRYPTO_BUFFER_POOL| was not reference-counted and
+// this function saved a non-owning pointer, expecting the caller to maintain a
+// lifetime relationship between the two objects. Now that pools are
+// reference-counted, the compatible behavior is to treat it as set0 rather than
+// ownership-transfering.
+OPENSSL_EXPORT void SSL_CTX_set0_buffer_pool(SSL_CTX *ctx,
+                                             CRYPTO_BUFFER_POOL *pool);
+
 
 // Compliance policy configurations
 //

ssl/extensions.cc

@@ -1311,7 +1311,7 @@ static bool ext_sct_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
   // TODO(davidben): Enforce this anyway.
   if (!ssl->s3->session_reused) {
     hs->new_session->signed_cert_timestamp_list.reset(
-        CRYPTO_BUFFER_new_from_CBS(contents, ssl->ctx->pool));
+        CRYPTO_BUFFER_new_from_CBS(contents, ssl->ctx->pool.get()));
     if (hs->new_session->signed_cert_timestamp_list == nullptr) {
       *out_alert = SSL_AD_INTERNAL_ERROR;
       return false;

ssl/handoff.cc

@@ -546,11 +546,11 @@ bool SSL_apply_handback(SSL *ssl, Span<const uint8_t> handback) {
   SSL_HANDSHAKE *const hs = s3->hs.get();
   if (!session_reused || type == handback_tls13) {
     hs->new_session =
-        SSL_SESSION_parse(&seq, ssl->ctx->x509_method, ssl->ctx->pool);
+        SSL_SESSION_parse(&seq, ssl->ctx->x509_method, ssl->ctx->pool.get());
     session = hs->new_session.get();
   } else {
     ssl->session =
-        SSL_SESSION_parse(&seq, ssl->ctx->x509_method, ssl->ctx->pool);
+        SSL_SESSION_parse(&seq, ssl->ctx->x509_method, ssl->ctx->pool.get());
     session = ssl->session.get();
   }
 

ssl/handshake_client.cc

@@ -871,7 +871,7 @@ static enum ssl_hs_wait_t do_read_server_certificate(SSL_HANDSHAKE *hs) {
     hs->new_session->peer_cert_type = TLSEXT_cert_type_x509;
     parse_ok =
         ssl_parse_cert_chain(&alert, &hs->new_session->certs, &hs->peer_pubkey,
-                             nullptr, &body, ssl->ctx->pool);
+                             nullptr, &body, ssl->ctx->pool.get());
   }
 
   if (!parse_ok) {
@@ -937,7 +937,7 @@ static enum ssl_hs_wait_t do_read_certificate_status(SSL_HANDSHAKE *hs) {
   }
 
   hs->new_session->ocsp_response.reset(
-      CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool));
+      CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool.get()));
   if (hs->new_session->ocsp_response == nullptr) {
     ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
     return ssl_hs_error;

ssl/handshake_server.cc

@@ -1251,9 +1251,9 @@ static enum ssl_hs_wait_t do_read_client_certificate(SSL_HANDSHAKE *hs) {
   } else {
     assert(hs->peer_cert_type == TLSEXT_cert_type_x509);
     hs->new_session->peer_cert_type = TLSEXT_cert_type_x509;
-    parse_ok =
-        ssl_parse_cert_chain(&alert, &hs->new_session->certs, &hs->peer_pubkey,
-                             out_sha256, &certificate_msg, ssl->ctx->pool);
+    parse_ok = ssl_parse_cert_chain(&alert, &hs->new_session->certs,
+                                    &hs->peer_pubkey, out_sha256,
+                                    &certificate_msg, ssl->ctx->pool.get());
   }
 
   if (!parse_ok) {

ssl/internal.h

@@ -4097,7 +4097,7 @@ struct ssl_ctx_st : public bssl::RefCounted<ssl_ctx_st> {
 
   // pool is used for all |CRYPTO_BUFFER|s in case we wish to share certificate
   // memory.
-  CRYPTO_BUFFER_POOL *pool = nullptr;
+  bssl::UniquePtr<CRYPTO_BUFFER_POOL> pool;
 
   // ticket_aead_method contains function pointers for opening and sealing
   // session tickets.

ssl/ssl_asn1.cc

@@ -838,7 +838,7 @@ SSL_SESSION *SSL_SESSION_from_bytes(const uint8_t *in, size_t in_len,
   CBS cbs;
   CBS_init(&cbs, in, in_len);
   UniquePtr<SSL_SESSION> ret =
-      SSL_SESSION_parse(&cbs, ctx->x509_method, ctx->pool);
+      SSL_SESSION_parse(&cbs, ctx->x509_method, ctx->pool.get());
   if (!ret) {
     return nullptr;
   }

ssl/ssl_cert.cc

@@ -406,7 +406,7 @@ bool ssl_cert_check_key_usage(const CBS *in, enum ssl_key_usage_t bit) {
 UniquePtr<STACK_OF(CRYPTO_BUFFER)> SSL_parse_CA_list(SSL *ssl,
                                                      uint8_t *out_alert,
                                                      CBS *cbs) {
-  CRYPTO_BUFFER_POOL *const pool = ssl->ctx->pool;
+  CRYPTO_BUFFER_POOL *const pool = ssl->ctx->pool.get();
 
   UniquePtr<STACK_OF(CRYPTO_BUFFER)> ret(sk_CRYPTO_BUFFER_new_null());
   if (!ret) {

ssl/ssl_lib.cc

@@ -1368,8 +1368,17 @@ uint32_t SSL_clear_mode(SSL *ssl, uint32_t mode) {
 
 uint32_t SSL_get_mode(const SSL *ssl) { return ssl->mode; }
 
+void SSL_CTX_set1_buffer_pool(SSL_CTX *ctx, CRYPTO_BUFFER_POOL *pool) {
+  ctx->pool = UpRef(pool);
+}
+
 void SSL_CTX_set0_buffer_pool(SSL_CTX *ctx, CRYPTO_BUFFER_POOL *pool) {
-  ctx->pool = pool;
+  // Historically, |CRYPTO_BUFFER_POOL| was not reference-counted and this
+  // function saved a non-owning pointer, expecting the caller to maintain a
+  // lifetime relationship between the two objects. Now that pools are
+  // reference-counted, the compatible behavior is to treat it as set0 rather
+  // than ownership-transfering.
+  return SSL_CTX_set1_buffer_pool(ctx, pool);
 }
 
 int SSL_get_tls_unique(const SSL *ssl, uint8_t *out, size_t *out_len,

ssl/ssl_x509.cc

@@ -866,14 +866,14 @@ void SSL_set_client_CA_list(SSL *ssl, STACK_OF(X509_NAME) *name_list) {
     return;
   }
   ssl->ctx->x509_method->ssl_flush_cached_client_CA(ssl->config.get());
-  set_client_CA_list(&ssl->config->client_CA, name_list, ssl->ctx->pool);
+  set_client_CA_list(&ssl->config->client_CA, name_list, ssl->ctx->pool.get());
   sk_X509_NAME_pop_free(name_list, X509_NAME_free);
 }
 
 void SSL_CTX_set_client_CA_list(SSL_CTX *ctx, STACK_OF(X509_NAME) *name_list) {
   check_ssl_ctx_x509_method(ctx);
   ctx->x509_method->ssl_ctx_flush_cached_client_CA(ctx);
-  set_client_CA_list(&ctx->client_CA, name_list, ctx->pool);
+  set_client_CA_list(&ctx->client_CA, name_list, ctx->pool.get());
   sk_X509_NAME_pop_free(name_list, X509_NAME_free);
 }
 
@@ -988,7 +988,7 @@ int SSL_add_client_CA(SSL *ssl, X509 *x509) {
   if (!ssl->config) {
     return 0;
   }
-  if (!add_client_CA(&ssl->config->client_CA, x509, ssl->ctx->pool)) {
+  if (!add_client_CA(&ssl->config->client_CA, x509, ssl->ctx->pool.get())) {
     return 0;
   }
 
@@ -998,7 +998,7 @@ int SSL_add_client_CA(SSL *ssl, X509 *x509) {
 
 int SSL_CTX_add_client_CA(SSL_CTX *ctx, X509 *x509) {
   check_ssl_ctx_x509_method(ctx);
-  if (!add_client_CA(&ctx->client_CA, x509, ctx->pool)) {
+  if (!add_client_CA(&ctx->client_CA, x509, ctx->pool.get())) {
     return 0;
   }
 

ssl/test/test_config.cc

@@ -2027,7 +2027,7 @@ bssl::UniquePtr<SSL_CTX> TestConfig::SetupCtx(SSL_CTX *old_ctx) const {
     return nullptr;
   }
 
-  SSL_CTX_set0_buffer_pool(ssl_ctx.get(), BufferPool());
+  SSL_CTX_set1_buffer_pool(ssl_ctx.get(), BufferPool());
 
   std::string cipher_list = "ALL";
   // Explicitly add deprecated ciphers that are otherwise not included.

ssl/test/test_state.cc

@@ -121,7 +121,7 @@ bool DeserializeContextState(CBS *cbs, SSL_CTX *ctx) {
   }
   while (CBS_len(&sessions)) {
     UniquePtr<SSL_SESSION> session =
-        SSL_SESSION_parse(&sessions, ctx->x509_method, ctx->pool);
+        SSL_SESSION_parse(&sessions, ctx->x509_method, ctx->pool.get());
     if (!session) {
       return false;
     }
@@ -166,8 +166,8 @@ std::unique_ptr<TestState> TestState::Deserialize(CBS *cbs, SSL_CTX *ctx) {
     return nullptr;
   }
   if (CBS_len(&pending_session)) {
-    state->pending_session = SSL_SESSION_parse(
-        &pending_session, ctx->x509_method, ctx->pool);
+    state->pending_session =
+        SSL_SESSION_parse(&pending_session, ctx->x509_method, ctx->pool.get());
     if (!state->pending_session) {
       return nullptr;
     }

ssl/tls13_both.cc

@@ -244,7 +244,7 @@ bool tls13_process_certificate(SSL_HANDSHAKE *hs, const SSLMessage &msg,
 
     if (hs->peer_cert_type == TLSEXT_cert_type_x509) {
       UniquePtr<CRYPTO_BUFFER> buf(
-          CRYPTO_BUFFER_new_from_CBS(&certificate, ssl->ctx->pool));
+          CRYPTO_BUFFER_new_from_CBS(&certificate, ssl->ctx->pool.get()));
       if (!buf ||  //
           !PushToStack(certs.get(), std::move(buf))) {
         ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
@@ -291,7 +291,7 @@ bool tls13_process_certificate(SSL_HANDSHAKE *hs, const SSLMessage &msg,
 
       if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) {
         hs->new_session->ocsp_response.reset(
-            CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool));
+            CRYPTO_BUFFER_new_from_CBS(&ocsp_response, ssl->ctx->pool.get()));
         if (hs->new_session->ocsp_response == nullptr) {
           ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
           return false;
@@ -308,7 +308,7 @@ bool tls13_process_certificate(SSL_HANDSHAKE *hs, const SSLMessage &msg,
 
       if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) {
         hs->new_session->signed_cert_timestamp_list.reset(
-            CRYPTO_BUFFER_new_from_CBS(&sct.data, ssl->ctx->pool));
+            CRYPTO_BUFFER_new_from_CBS(&sct.data, ssl->ctx->pool.get()));
         if (hs->new_session->signed_cert_timestamp_list == nullptr) {
           ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
           return false;