Skip to content

Commit

Permalink
stopTLS leverages ExtractedIdentityCert to keep credentials after dro…
Browse files Browse the repository at this point in the history
…pping certificate

Summary:
When dropping down to plain, we currently keep the X509 payload in order to eventually extract identities.

This was attempted to be fixed in D49045451 - however that caused the identities to not be kept and authentication failure ensued.

This diff attempts to:
- Extract identities when dropping down to plain
- Drop the SSL context as soon as we do so

This ensures that we have identities downstream, without needing the X509 payload.

Reviewed By: frqiu

Differential Revision: D49611022

fbshipit-source-id: 7f4246c938c613dd13c973cb07300b53769f36df
  • Loading branch information
Eden Zik authored and facebook-github-bot committed Oct 13, 2023
1 parent 275a7e6 commit d22e9b8
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
16 changes: 10 additions & 6 deletions mcrouter/lib/network/McSSLUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,9 @@ folly::AsyncTransportWrapper::UniquePtr McSSLUtil::moveToPlaintext(
SSL_shutdown(ssl);

// fallback to plaintext
auto selfCert =
folly::ssl::BasicTransportCertificate::create(sock.getSelfCertificate());
auto peerCert =
folly::ssl::BasicTransportCertificate::create(sock.getPeerCertificate());
// Clear X509 Payload, get certificates out
auto [selfCert, peerCert] = dropCertificateX509Payload(sock);

auto evb = sock.getEventBase();
auto zcId = sock.getZeroCopyBufId();
auto fd = sock.detachNetworkSocket();
Expand All @@ -200,13 +199,18 @@ folly::AsyncTransportWrapper::UniquePtr McSSLUtil::moveToKtls(
return nullptr;
}

void McSSLUtil::dropCertificateX509Payload(
McSSLUtil::CertPair McSSLUtil::dropCertificateX509Payload(
folly::AsyncSSLSocket& sock) noexcept {
folly::SharedMutex::ReadHolder rh(getMutex());
auto& func = getDropCertificateX509PayloadFuncRef();
if (func) {
func(sock);
return func(sock);
}
/* By default, initialize a BasicTransportCertificate which is OSS compatible
*/
return std::make_tuple(
folly::ssl::BasicTransportCertificate::create(sock.getSelfCertificate()),
folly::ssl::BasicTransportCertificate::create(sock.getPeerCertificate()));
}

folly::Optional<SecurityTransportStats> McSSLUtil::getKtlsStats(
Expand Down
8 changes: 6 additions & 2 deletions mcrouter/lib/network/McSSLUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ class McSSLUtil {
folly::Function<bool(folly::AsyncSSLSocket*, bool, X509_STORE_CTX*)
const noexcept>;

using CertPair = std::tuple<
std::unique_ptr<folly::AsyncTransportCertificate>,
std::unique_ptr<folly::AsyncTransportCertificate>>;
using DropCertificateX509PayloadFunction =
folly::Function<bool(folly::AsyncSSLSocket&) const noexcept>;
folly::Function<CertPair(folly::AsyncSSLSocket&) const noexcept>;

static const std::string kTlsToPlainProtocolName;

Expand Down Expand Up @@ -105,7 +108,8 @@ class McSSLUtil {
/**
* Drops certificate x509 payload after connection is established
*/
static void dropCertificateX509Payload(folly::AsyncSSLSocket& sock) noexcept;
static CertPair dropCertificateX509Payload(
folly::AsyncSSLSocket& sock) noexcept;

/**
* Move the existing transport to kTLS if possible. Return value of
Expand Down
4 changes: 3 additions & 1 deletion mcrouter/lib/network/McServerSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,9 @@ void McServerSession::onAccepted() {
std::make_unique<const McServerThriftRequestContext>(transport_.get());
/* Trims the certificate memory */
if (auto sock = transport_->getUnderlyingTransport<folly::AsyncSSLSocket>()) {
McSSLUtil::dropCertificateX509Payload(*sock);
auto [selfCert, peerCert] = McSSLUtil::dropCertificateX509Payload(*sock);
sock->setSelfCertificate(std::move(selfCert));
sock->setPeerCertificate(std::move(peerCert));
}
debugFifo_ = getDebugFifo(
options_.debugFifoPath, transport_.get(), onRequest_->name());
Expand Down

0 comments on commit d22e9b8

Please sign in to comment.