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_server.cc

diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 481dc54..a113504 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -395,8 +395,7 @@
           // Custom extensions is incompatible with 0-RTT.
           hs->custom_extensions.received == 0 &&
           // The negotiated ALPN must match the one in the ticket.
-          ssl->s3->alpn_selected ==
-              MakeConstSpan(session->early_alpn, session->early_alpn_len)) {
+          MakeConstSpan(ssl->s3->alpn_selected) == session->early_alpn) {
         ssl->s3->early_data_accepted = true;
       }
 
@@ -425,14 +424,9 @@
   hs->new_session->cipher = hs->new_cipher;
 
   // Store the initial 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->ctx->dos_protection_cb != NULL &&
@@ -824,7 +818,7 @@
 static enum ssl_hs_wait_t do_read_client_certificate_verify(
     SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
-  if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) == 0) {
+  if (sk_CRYPTO_BUFFER_num(hs->new_session->certs.get()) == 0) {
     // Skip this state.
     hs->tls13_state = state_read_channel_id;
     return ssl_hs_ok;