commit bfdd1a93084f55d01448648e98a3fb3e97e0e8c1
author David Benjamin <davidben@google.com> Fri Jun 29 16:26:38 2018 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Jul 03 22:57:56 2018 +0000
tree cd0d3f7444c46919baf569f074f9a88186d3efd4
parent 58150ed59b69c2e6c35379d96f4f43c5f5838d03

Give SSL_SESSION a destructor.

Previously we'd partially attempted the ssl_st / bssl::SSLConnection
subclassing split, but that gets messy when we actually try to add a
destructor, because CRYPTO_EX_DATA's cleanup function needs an ssl_st*,
not a bssl::SSLConnection*. Downcasting is technically undefined at this
point and will likely offend some CFI-like check.

Moreover, it appears that even with today's subclassing split,
New<SSL>() emits symbols like:

W ssl_st*& std::forward<ssl_st*&>(std::remove_reference<ssl_st*&>::type&)

The compiler does not bother emitting them in optimized builds, but it
does suggest we can't really avoid claiming the ssl_st type name at the
symbol level, short of doing reinterpret_casts at all API boundaries.
And, of course, we've already long claimed it at the #include level.

So I've just left this defining directly on ssl_session_st. The cost is
we need to write some silly "bssl::" prefixes in the headers, but so it
goes. In the likely event we change our minds again, we can always
revise this.

Change-Id: Ieb429e8eaabe7c2961ef7f8d9234fb71f19a5e2a
Reviewed-on: https://boringssl-review.googlesource.com/29587
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>

ssl/tls13_client.cc

diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 317b4d3..1f1d4b4 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -429,20 +429,14 @@
   }
 
   // Store the negotiated ALPN in the session.
-  if (!ssl->s3->alpn_selected.empty()) {
-    hs->new_session->early_alpn = (uint8_t *)BUF_memdup(
-        ssl->s3->alpn_selected.data(), ssl->s3->alpn_selected.size());
-    if (hs->new_session->early_alpn == NULL) {
-      ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
-      return ssl_hs_error;
-    }
-    hs->new_session->early_alpn_len = ssl->s3->alpn_selected.size();
+  if (!hs->new_session->early_alpn.CopyFrom(ssl->s3->alpn_selected)) {
+    ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+    return ssl_hs_error;
   }
 
   if (ssl->s3->early_data_accepted) {
     if (hs->early_session->cipher != hs->new_session->cipher ||
-        MakeConstSpan(hs->early_session->early_alpn,
-                      hs->early_session->early_alpn_len) !=
+        MakeConstSpan(hs->early_session->early_alpn) !=
             ssl->s3->alpn_selected) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_ALPN_MISMATCH_ON_EARLY_DATA);
       return ssl_hs_error;
@@ -849,7 +843,7 @@
       !CBS_get_u32(&body, &session->ticket_age_add) ||
       !CBS_get_u8_length_prefixed(&body, &ticket_nonce) ||
       !CBS_get_u16_length_prefixed(&body, &ticket) ||
-      !CBS_stow(&ticket, &session->tlsext_tick, &session->tlsext_ticklen) ||
+      !session->ticket.CopyFrom(ticket) ||
       !CBS_get_u16_length_prefixed(&body, &extensions) ||
       CBS_len(&body) != 0) {
     ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);