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 d02ad248f7319b87dc56d83ce60983b941e32c94..15807809a7c5d8175ebffdcd2160c9ca94e880a8 100644 (file)
--- 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 e11021b1a1d4f06159b5fcd817877c468c06f94d..831e251735e12ad15a9ded7014b6ad6f7c703f24 100644 (file)
--- 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