From: Viswanath Sivakumar <viswanath@fb.com>
Date: Mon, 5 Jan 2015 18:46:07 +0000 (-0800)
Subject: Fix use of SSL session TransportInfo after txn is detached
X-Git-Tag: v0.22.0~51
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=9d0223804c0b12ac620cd9ed213ad6c303d1a2e0;p=folly.git

Fix use of SSL session TransportInfo after txn is detached

Summary:
De-couple TransportInfo fields from SSL session structs to avoid
dangling pointers.

Facebook:
We sometimes lazily copy TransportInfo in handler after
detachClientTransaction for logging. If the socket is closed, then this
creates dangling pointers to some SSL structs. This is an attempt to fix
that.

This is similar to what @ajitb did in
https://phabricator.fb.com/D1666951 which had to be abandoned because of
memory overhead. Here, instead of copying the relevant fields per
transaction, we are only doing it once per session (shared_ptr), so the
memory overhead should be negligible.

Test Plan: Unit tests pass. Will canary

Reviewed By: afrind@fb.com

Subscribers: fugalh, bmatheny, ssl-diffs@, folly-diffs@, ajitb

FB internal diff: D1757318

Tasks: 5865651, 5879508

Signature: t1:1757318:1420482488:9f5144b499eb2086cf2a80243328db5715b48f88
---

diff --git a/folly/wangle/acceptor/Acceptor.cpp b/folly/wangle/acceptor/Acceptor.cpp
index d02ad248..15807809 100644
--- a/folly/wangle/acceptor/Acceptor.cpp
+++ b/folly/wangle/acceptor/Acceptor.cpp
@@ -15,14 +15,12 @@
 #include <boost/cast.hpp>
 #include <fcntl.h>
 #include <folly/ScopeGuard.h>
-#include <folly/wangle/acceptor/ManagedConnection.h>
 #include <folly/io/async/EventBase.h>
 #include <fstream>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <folly/io/async/AsyncSSLSocket.h>
 #include <folly/io/async/AsyncSocket.h>
-#include <folly/io/async/EventBase.h>
 #include <unistd.h>
 
 using folly::wangle::ConnectionManager;
@@ -113,8 +111,10 @@ class AcceptorHandshakeHelper :
     tinfo.sslSetupTime = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - acceptTime_);
     tinfo.sslSetupBytesRead = sock->getRawBytesReceived();
     tinfo.sslSetupBytesWritten = sock->getRawBytesWritten();
-    tinfo.sslServerName = sock->getSSLServerName();
-    tinfo.sslCipher = sock->getNegotiatedCipherName();
+    tinfo.sslServerName = sock->getSSLServerName() ?
+      std::make_shared<std::string>(sock->getSSLServerName()) : nullptr;
+    tinfo.sslCipher = sock->getNegotiatedCipherName() ?
+      std::make_shared<std::string>(sock->getNegotiatedCipherName()) : nullptr;
     tinfo.sslVersion = sock->getSSLVersion();
     tinfo.sslCertSize = sock->getSSLCertSize();
     tinfo.sslResume = SSLUtil::getResumeState(sock);
diff --git a/folly/wangle/acceptor/TransportInfo.h b/folly/wangle/acceptor/TransportInfo.h
index e11021b1..831e2517 100644
--- a/folly/wangle/acceptor/TransportInfo.h
+++ b/folly/wangle/acceptor/TransportInfo.h
@@ -75,13 +75,13 @@ struct TransportInfo {
    * The name of the SSL ciphersuite used by the transaction's
    * transport.  Returns null if the transport is not SSL.
    */
-  const char* sslCipher{nullptr};
+  std::shared_ptr<std::string> sslCipher{nullptr};
 
   /*
    * The SSL server name used by the transaction's
    * transport.  Returns null if the transport is not SSL.
    */
-  const char* sslServerName{nullptr};
+  std::shared_ptr<std::string> sslServerName{nullptr};
 
   /*
    * list of ciphers sent by the client