Skip to content

Commit d78e00b

Browse files
Huilin Chenmeta-codesync[bot]
authored andcommitted
Remove AES-OCB cipher preference usage
Summary: - The incentive to clean up mcrouter OCB cipher usage is from D113609692 (next diff in stack), which removes OCB from the CipherSuite enum. - We already removed ocb from SRConfig, Thrift, and Servicerouter: D53867041, D55032082, and D53207446 back in 2024. - The OCB cipher is not being used in prod according [thrift connection events table](https://fburl.com/scuba/thrift_connection_events/q9r1kmj6), so this change should be safe. Reviewed By: alikhtarov Differential Revision: D113842492 fbshipit-source-id: 35e73e35381dabed1b830db23b3d670837c024c7
1 parent 742a06b commit d78e00b

12 files changed

Lines changed: 6 additions & 77 deletions

mcrouter/ProxyDestination-inl.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,6 @@ void ProxyDestination<Transport>::initializeTransport() {
324324
options.securityOpts.sslAuthorizationEnforce =
325325
opts.ssl_service_identity_authorization_enforce;
326326
options.securityOpts.tfoEnabledForSsl = opts.enable_ssl_tfo;
327-
options.securityOpts.tlsPreferOcbCipher = opts.tls_prefer_ocb_cipher;
328327
}
329328

330329
auto client = std::unique_ptr<Transport, typename Transport::Destructor>(

mcrouter/lib/network/AsyncMcServer.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,8 +433,7 @@ class McServerThread {
433433
opts.pemKeyPath,
434434
opts.pemCaPath,
435435
opts.sslRequirePeerCerts,
436-
std::move(ticketKeySeeds),
437-
opts.tlsPreferOcbCipher);
436+
std::move(ticketKeySeeds));
438437

439438
if (contextPair.first) {
440439
mcServerThread_->worker_.addSecureClientSocket(

mcrouter/lib/network/AsyncMcServer.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,6 @@ class AsyncMcServer {
100100
*/
101101
bool sslRequirePeerCerts{false};
102102

103-
/**
104-
* Prefer AES-OCB cipher suite if available.
105-
*/
106-
bool tlsPreferOcbCipher{false};
107-
108103
/**
109104
* Path to JSON file containing old, current, and new seeds used for TLS
110105
* ticket key generation.

mcrouter/lib/network/FizzContextProvider.cpp

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ constexpr size_t kHandshakeValidity = 604800;
3333
FizzContextAndVerifier createClientFizzContextAndVerifier(
3434
std::string certData,
3535
std::string keyData,
36-
folly::StringPiece pemCaPath,
37-
bool preferOcbCipher) {
36+
folly::StringPiece pemCaPath) {
3837
// global session cache
3938
static auto SESSION_CACHE =
4039
std::make_shared<fizz::client::SynchronizedLruPskCache>(100);
@@ -68,16 +67,6 @@ FizzContextAndVerifier createClientFizzContextAndVerifier(
6867
verifier = std::move(verifierUniq);
6968
}
7069

71-
if (preferOcbCipher) {
72-
#if !defined(OPENSSL_NO_OCB)
73-
auto ciphers = folly::copy(ctx->getSupportedCiphers());
74-
ciphers.insert(
75-
ciphers.begin(),
76-
fizz::CipherSuite::TLS_AES_128_OCB_SHA256_EXPERIMENTAL);
77-
ctx->setSupportedCiphers(std::move(ciphers));
78-
#endif
79-
}
80-
8170
return FizzContextAndVerifier(std::move(ctx), std::move(verifier));
8271
}
8372

@@ -88,7 +77,6 @@ std::shared_ptr<fizz::server::FizzServerContext> createFizzServerContext(
8877
folly::StringPiece keyData,
8978
folly::StringPiece pemCaPath,
9079
bool requireClientVerification,
91-
bool preferOcbCipher,
9280
wangle::TLSTicketKeySeeds* ticketKeySeeds) {
9381
auto certMgr = std::make_shared<fizz::server::DefaultCertManager>();
9482
try {
@@ -130,18 +118,6 @@ std::shared_ptr<fizz::server::FizzServerContext> createFizzServerContext(
130118
if (requireClientVerification) {
131119
ctx->setClientAuthMode(fizz::server::ClientAuthMode::Required);
132120
}
133-
if (preferOcbCipher) {
134-
#if !defined(OPENSSL_NO_OCB)
135-
auto serverCiphers = folly::copy(ctx->getSupportedCiphers());
136-
serverCiphers.insert(
137-
serverCiphers.begin(),
138-
{
139-
fizz::CipherSuite::TLS_AES_128_OCB_SHA256_EXPERIMENTAL,
140-
});
141-
ctx->setSupportedCiphers(std::move(serverCiphers));
142-
#endif
143-
}
144-
145121
// set ticket seeds
146122
if (ticketKeySeeds) {
147123
std::vector<folly::ByteRange> ticketSecrets;

mcrouter/lib/network/FizzContextProvider.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ using FizzContextAndVerifier = std::pair<
3232
FizzContextAndVerifier createClientFizzContextAndVerifier(
3333
std::string certData,
3434
std::string keyData,
35-
folly::StringPiece pemCaPath,
36-
bool preferOcbCipher);
35+
folly::StringPiece pemCaPath);
3736

3837
std::shared_ptr<fizz::server::FizzServerContext> createFizzServerContext(
3938
folly::StringPiece pemCertPath,
@@ -42,7 +41,6 @@ std::shared_ptr<fizz::server::FizzServerContext> createFizzServerContext(
4241
folly::StringPiece keyData,
4342
folly::StringPiece pemCaPath,
4443
bool requireClientVerification,
45-
bool preferOcbCipher,
4644
wangle::TLSTicketKeySeeds* ticketKeySeeds);
4745
} // namespace memcache
4846
} // namespace facebook

mcrouter/lib/network/SecurityOptions.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,6 @@ struct SecurityOptions {
8383
* Whether TFO is enabled for SSL connections
8484
*/
8585
bool tfoEnabledForSsl{false};
86-
87-
/**
88-
* Client side to prefer AES-OCB cipher suite if supported.
89-
*/
90-
bool tlsPreferOcbCipher{false};
9186
};
9287

9388
} // namespace memcache

mcrouter/lib/network/ThreadLocalSSLContextProvider.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -452,10 +452,7 @@ FizzContextAndVerifier getFizzClientConfig(
452452
auto certData = readFile(opts.sslPemCertPath);
453453
auto keyData = readFile(opts.sslPemKeyPath);
454454
auto fizzData = createClientFizzContextAndVerifier(
455-
std::move(certData),
456-
std::move(keyData),
457-
opts.sslPemCaPath,
458-
opts.tlsPreferOcbCipher);
455+
std::move(certData), std::move(keyData), opts.sslPemCaPath);
459456
info.setFizzData(std::move(fizzData), now);
460457
}
461458
return info.fizzData;
@@ -488,8 +485,7 @@ ServerContextPair getServerContexts(
488485
folly::StringPiece pemKeyPath,
489486
folly::StringPiece pemCaPath,
490487
bool requireClientCerts,
491-
folly::Optional<wangle::TLSTicketKeySeeds> seeds,
492-
bool preferOcbCipher) {
488+
folly::Optional<wangle::TLSTicketKeySeeds> seeds) {
493489
auto& info = getServerContextInfo(
494490
evb, pemCertPath, pemKeyPath, pemCaPath, requireClientCerts);
495491
auto now = std::chrono::steady_clock::now();
@@ -511,7 +507,6 @@ ServerContextPair getServerContexts(
511507
keyData,
512508
pemCaPath,
513509
requireClientCerts,
514-
preferOcbCipher,
515510
seeds.get_pointer());
516511
info.setContexts(std::move(ctx), std::move(fizzCtx), now);
517512
}

mcrouter/lib/network/ThreadLocalSSLContextProvider.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ ServerContextPair getServerContexts(
8989
folly::StringPiece pemKeyPath,
9090
folly::StringPiece pemCaPath,
9191
bool requireClientCerts,
92-
folly::Optional<wangle::TLSTicketKeySeeds> seeds,
93-
bool preferOcbCipher = false);
92+
folly::Optional<wangle::TLSTicketKeySeeds> seeds);
9493

9594
} // namespace memcache
9695
} // namespace facebook

mcrouter/lib/network/test/AsyncMcClientTestSync.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,6 @@ folly::Optional<SSLTestPaths> getFizzSSL() {
4646
return res;
4747
}
4848

49-
folly::Optional<SSLTestPaths> getFizzSSLWithOCB() {
50-
auto res = getFizzSSL();
51-
res->useOcbCipher = true;
52-
return res;
53-
}
54-
5549
folly::Optional<SSLTestPaths> getKtlsSSL() {
5650
auto res = validClientSsl();
5751
res.mech = SecurityMech::KTLS12;
@@ -67,9 +61,6 @@ class AsyncMcClientSimpleTest
6761
const folly::Optional<SSLTestPaths>& ssl,
6862
TestServer::Config& config) {
6963
config.useSsl = ssl.has_value();
70-
if (config.useSsl) {
71-
config.tlsPreferOcbCipher = ssl->useOcbCipher;
72-
}
7364
}
7465
};
7566

@@ -112,12 +103,6 @@ TEST_P(AsyncMcClientSimpleTest, serverShutdownTest) {
112103
auto fizzTransport =
113104
transport->getUnderlyingTransport<fizz::client::AsyncFizzClient>();
114105
EXPECT_NE(fizzTransport, nullptr);
115-
if (ssl->useOcbCipher) {
116-
const auto cipher = fizzTransport->getCipher();
117-
EXPECT_TRUE(cipher.has_value());
118-
EXPECT_EQ(
119-
fizz::CipherSuite::TLS_AES_128_OCB_SHA256_EXPERIMENTAL, *cipher);
120-
}
121106
}
122107

123108
server->join();
@@ -271,7 +256,6 @@ INSTANTIATE_TEST_CASE_P(
271256
validClientSsl(),
272257
getTlsToPtSSL(),
273258
getFizzSSL(),
274-
getFizzSSLWithOCB(),
275259
getKtlsSSL()));
276260

277261
void testCerts(

mcrouter/lib/network/test/TestClientServerUtil.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ TestServer::TestServer(Config config)
163163
opts_.pemCertPath = config.certPath;
164164
opts_.pemCaPath = config.caPath;
165165
opts_.sslRequirePeerCerts = config.requirePeerCerts;
166-
opts_.tlsPreferOcbCipher = config.tlsPreferOcbCipher;
167166
if (config.tfoEnabled) {
168167
opts_.tfoEnabledForSsl = true;
169168
opts_.tfoQueueSize = 100000;
@@ -262,7 +261,6 @@ TestClient::TestClient(
262261
opts.securityOpts.sslServiceIdentity = serviceIdentity;
263262
opts.securityOpts.tfoEnabledForSsl = enableTfo;
264263
opts.securityOpts.sslHandshakeOffload = offloadHandshakes;
265-
opts.securityOpts.tlsPreferOcbCipher = ssl->useOcbCipher;
266264
}
267265
if (qosClass != 0 || qosPath != 0) {
268266
opts.enableQoS = true;

0 commit comments

Comments
 (0)