Skip to content

Commit 559eb32

Browse files
Ankit Kumarmeta-codesync[bot]
authored andcommitted
Add a security interface slot to FastThriftServer
Summary: Adds `FastThriftServer::setSecurityInterface`, the fast_thrift counterpart of `ThriftServer::setSecurityInterface`, wiring a `SecurityServerInterface` marker as a fourth auxiliary composite child after monitoring/status/debug. Reviewed By: robertroeser Differential Revision: D116959429 fbshipit-source-id: 635ec1fb97c964c5f2395c99922d105b35ca0dbe
1 parent e765577 commit 559eb32

7 files changed

Lines changed: 427 additions & 3 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#pragma once
18+
19+
#include <thrift/lib/cpp2/fast_thrift/thrift/server/adapter/ThriftServerAppAdapterFactory.h>
20+
21+
namespace apache::thrift::fast_thrift {
22+
23+
/**
24+
* A marker base class for a fast_thrift service intended for reading security
25+
* metadata (the fast_thrift counterpart of
26+
* apache::thrift::SecurityServerInterface from
27+
* thrift/lib/cpp2/server/SecurityServerInterface.h).
28+
*
29+
* Security-metadata introspection RPCs — "who am I to this server", which
30+
* interface owns a method, what the server's enforcement posture is — are
31+
* answered on this interface. Marker exists purely as a type-system
32+
* guardrail: passing a user-facing handler to setSecurityInterface is a
33+
* compile error.
34+
*
35+
* Unlike the monitoring / status / debug slots, no IDL ships alongside this
36+
* header: the canonical security-metadata service depends on Meta-internal
37+
* types that cannot live in this tree. The embedder supplies both the IDL
38+
* and the FastServiceHandler that implements it.
39+
*
40+
* Naming contract for that service: authorization gates exempt these RPCs by
41+
* matching the `Service.method` name against a fixed allowlist of interface
42+
* prefixes (facebook::services::isInternalMethod). Two consequences for an
43+
* embedder. The service name must be one the allowlist already recognizes,
44+
* and — because a fast_thrift method name on the wire is bare — a gate must
45+
* qualify it with the service name before matching. An unqualified name never
46+
* matches, which denies the request rather than exempting it.
47+
*
48+
* DO NOT inherit this type if the ThriftServerAppAdapter returned by your
49+
* class handles non-security methods.
50+
*/
51+
class SecurityServerInterface
52+
: public virtual thrift::ThriftServerAppAdapterFactory {};
53+
54+
} // namespace apache::thrift::fast_thrift

third-party/thrift/src/thrift/lib/cpp2/fast_thrift/thrift/server/FastThriftServer.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,19 @@ void FastThriftServer::setDebugInterface(
9797
auxInterfaces_.debugHandler = std::move(handler);
9898
}
9999

100+
void FastThriftServer::setSecurityInterface(
101+
std::shared_ptr<fast_thrift::SecurityServerInterface> handler) {
102+
std::lock_guard<std::mutex> lock(lifecycleMutex_);
103+
CHECK(state_ == State::kNotStarted)
104+
<< "FastThriftServer::setSecurityInterface must be called before "
105+
"start()/serve()";
106+
CHECK(handler)
107+
<< "FastThriftServer::setSecurityInterface requires a non-null handler";
108+
CHECK(!auxInterfaces_.securityHandler)
109+
<< "FastThriftServer::setSecurityInterface called more than once";
110+
auxInterfaces_.securityHandler = std::move(handler);
111+
}
112+
100113
void FastThriftServer::setStats(std::shared_ptr<ServerStats> stats) {
101114
std::lock_guard<std::mutex> lock(lifecycleMutex_);
102115
CHECK(state_ == State::kNotStarted)
@@ -293,6 +306,9 @@ void FastThriftServer::start() {
293306
if (auxInterfaces_.debugHandler) {
294307
auxInterfaces_.debugHandler->getServiceMetadata(*resp);
295308
}
309+
if (auxInterfaces_.securityHandler) {
310+
auxInterfaces_.securityHandler->getServiceMetadata(*resp);
311+
}
296312
handler_->getServiceMetadata(*resp);
297313
metadataResponse_ = std::move(resp);
298314
}
@@ -354,6 +370,7 @@ void FastThriftServer::start() {
354370
.monitoringHandler = auxInterfaces_.monitoringHandler,
355371
.statusHandler = auxInterfaces_.statusHandler,
356372
.debugHandler = auxInterfaces_.debugHandler,
373+
.securityHandler = auxInterfaces_.securityHandler,
357374
.metadataResponse = metadataResponse_,
358375
.zeroCopyThreshold = config_.zeroCopyThreshold,
359376
.enableRequestContext = config_.enableRequestContext,

third-party/thrift/src/thrift/lib/cpp2/fast_thrift/thrift/server/FastThriftServer.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include <thrift/lib/cpp2/fast_thrift/connection/SocketOptions.h>
3636
#include <thrift/lib/cpp2/fast_thrift/interface/debug/DebugServerInterface.h>
3737
#include <thrift/lib/cpp2/fast_thrift/interface/monitor/MonitoringServerInterface.h>
38+
#include <thrift/lib/cpp2/fast_thrift/interface/security/SecurityServerInterface.h>
3839
#include <thrift/lib/cpp2/fast_thrift/interface/status/StatusServerInterface.h>
3940
#include <thrift/lib/cpp2/fast_thrift/security/FizzServerCertConfig.h>
4041
#include <thrift/lib/cpp2/fast_thrift/security/ThriftTlsConfig.h>
@@ -134,6 +135,22 @@ class FastThriftServer {
134135
void setDebugInterface(
135136
std::shared_ptr<fast_thrift::DebugServerInterface> handler);
136137

138+
/**
139+
* Attach a Security handler. Methods on the security handler are dispatched
140+
* on the same connection as the user handler; routing is by method name
141+
* with the user handler winning on conflict (mirrors
142+
* ThriftServer::setSecurityInterface). Must be called before
143+
* start()/serve().
144+
*
145+
* Security-metadata introspection clients call into this interface.
146+
*
147+
* The handler must derive from fast_thrift::SecurityServerInterface — a
148+
* marker base that exists purely as a type-system guardrail. See that
149+
* header for the service-naming contract an authorization gate relies on.
150+
*/
151+
void setSecurityInterface(
152+
std::shared_ptr<fast_thrift::SecurityServerInterface> handler);
153+
137154
/**
138155
* Attach server counters. Wires the rocket- and thrift-layer metrics
139156
* handlers into every connection built after this point; leaving it unset
@@ -352,6 +369,9 @@ class FastThriftServer {
352369
bool hasDebugHandler() const noexcept {
353370
return static_cast<bool>(auxInterfaces_.debugHandler);
354371
}
372+
bool hasSecurityHandler() const noexcept {
373+
return static_cast<bool>(auxInterfaces_.securityHandler);
374+
}
355375

356376
private:
357377
// Lifecycle states. Transitions are linear: kNotStarted → kRunning →
@@ -373,6 +393,8 @@ class FastThriftServer {
373393
nullptr};
374394
std::shared_ptr<fast_thrift::StatusServerInterface> statusHandler{nullptr};
375395
std::shared_ptr<fast_thrift::DebugServerInterface> debugHandler{nullptr};
396+
std::shared_ptr<fast_thrift::SecurityServerInterface> securityHandler{
397+
nullptr};
376398
};
377399

378400
const FastThriftServerConfig config_;

third-party/thrift/src/thrift/lib/cpp2/fast_thrift/thrift/server/ThriftServerConnectionFactory.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ ThriftServerConnectionFactory::ThriftServerConnectionFactory(
8787
static_cast<bool>(config_.monitoringHandler) ||
8888
static_cast<bool>(config_.statusHandler) ||
8989
static_cast<bool>(config_.debugHandler) ||
90+
static_cast<bool>(config_.securityHandler) ||
9091
static_cast<bool>(config_.metadataResponse)) {
9192
CHECK(config_.handler)
9293
<< "ThriftServerConnectionFactory requires a non-null handler";
@@ -169,6 +170,11 @@ ThriftServerConnection ThriftServerConnectionFactory::buildCompositeConnection(
169170
config_.debugHandler->getAppAdapter(config_.debugHandler));
170171
attachCPUExecutor(*tail.children.back());
171172
}
173+
if (config_.securityHandler) {
174+
tail.children.push_back(
175+
config_.securityHandler->getAppAdapter(config_.securityHandler));
176+
attachCPUExecutor(*tail.children.back());
177+
}
172178
// Deliberately not offloaded: MetadataAppAdapter is hand-written rather
173179
// than generated, so it never consults cpuExecutor() and completes through
174180
// the EventBase-only writeResponse overload. Attaching an executor would

third-party/thrift/src/thrift/lib/cpp2/fast_thrift/thrift/server/ThriftServerConnectionFactory.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include <thrift/lib/cpp2/fast_thrift/frame/write/IntervalBatchingHandlerConfig.h>
3636
#include <thrift/lib/cpp2/fast_thrift/interface/debug/DebugServerInterface.h>
3737
#include <thrift/lib/cpp2/fast_thrift/interface/monitor/MonitoringServerInterface.h>
38+
#include <thrift/lib/cpp2/fast_thrift/interface/security/SecurityServerInterface.h>
3839
#include <thrift/lib/cpp2/fast_thrift/interface/status/StatusServerInterface.h>
3940
#include <thrift/lib/cpp2/fast_thrift/rocket/server/adapter/RocketServerAppAdapter.h>
4041
#include <thrift/lib/cpp2/fast_thrift/rocket/server/handler/RocketServerSetupFrameHandler.h>
@@ -57,9 +58,10 @@ struct ThriftServerConnectionFactoryConfig {
5758
// Executor that handler methods are dispatched to. Null keeps dispatch
5859
// inline on the connection's EventBase.
5960
//
60-
// Applied to the user handler and to the monitoring / status / debug aux
61-
// interfaces alike. Methods that must stay on the EventBase — the liveness
62-
// probe and the counter scrapes — are pinned per-method in their IDLs with
61+
// Applied to the user handler and to the monitoring / status / debug /
62+
// security aux interfaces alike. Methods that must stay on the EventBase —
63+
// the liveness probe and the counter scrapes — are pinned per-method in
64+
// their IDLs with
6365
// @cpp.ProcessInEbThreadUnsafe, mirroring what the legacy stack pins in
6466
// common/thrift/thrift/status.thrift and fb303/thrift/fb303_core.thrift.
6567
//
@@ -70,6 +72,7 @@ struct ThriftServerConnectionFactoryConfig {
7072
std::shared_ptr<fast_thrift::MonitoringServerInterface> monitoringHandler;
7173
std::shared_ptr<fast_thrift::StatusServerInterface> statusHandler;
7274
std::shared_ptr<fast_thrift::DebugServerInterface> debugHandler;
75+
std::shared_ptr<fast_thrift::SecurityServerInterface> securityHandler;
7376
std::shared_ptr<const apache::thrift::metadata::ThriftServiceMetadataResponse>
7477
metadataResponse;
7578
// Per-connection MSG_ZEROCOPY threshold; 0 disables zero-copy.

0 commit comments

Comments
 (0)