Commit 9d022380 authored by Viswanath Sivakumar's avatar Viswanath Sivakumar

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
parent df2469a3
......@@ -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);
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment