From 8c68b18df3fe43a8ffede2f62efbc6ca5823d43f Mon Sep 17 00:00:00 2001 From: Kyle Nekritz Date: Wed, 29 Nov 2023 18:06:03 -0800 Subject: [PATCH] Unconditionally defer control of writing session tickets to quic layer. Summary: There is no reason to let the TLS layer control this. Reviewed By: jbeshay Differential Revision: D51267586 fbshipit-source-id: 6f70b7f6ba51a9a022195f7e2c1683b5323fde7a --- quic/fizz/server/handshake/FizzServerHandshake.cpp | 2 +- quic/server/QuicServerTransport.cpp | 2 +- quic/server/test/QuicServerTransportTest.cpp | 9 +++++---- quic/server/test/QuicServerTransportTestUtil.h | 8 ++------ 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/quic/fizz/server/handshake/FizzServerHandshake.cpp b/quic/fizz/server/handshake/FizzServerHandshake.cpp index 44805e1d0..cb9a1aa46 100644 --- a/quic/fizz/server/handshake/FizzServerHandshake.cpp +++ b/quic/fizz/server/handshake/FizzServerHandshake.cpp @@ -59,8 +59,8 @@ void FizzServerHandshake::initializeImpl( context->setFactory(cryptoFactory_->getFizzFactory()); context->setSupportedCiphers({{fizz::CipherSuite::TLS_AES_128_GCM_SHA256}}); context->setVersionFallbackEnabled(false); - // Since Draft-17, client won't sent EOED context->setOmitEarlyRecordLayer(true); + context->setSendNewSessionTicket(false); state_.context() = std::move(context); callback_ = callback; diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index 8a826b276..1e882caa3 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -564,7 +564,7 @@ bool QuicServerTransport::shouldWriteNewSessionTicket() { } void QuicServerTransport::maybeWriteNewSessionTicket() { - if (shouldWriteNewSessionTicket() && !ctx_->getSendNewSessionTicket() && + if (shouldWriteNewSessionTicket() && serverConn_->serverHandshakeLayer->isHandshakeDone()) { if (conn_->qLogger) { conn_->qLogger->addTransportStateUpdate(kWriteNst); diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index d4fee8e1a..04f19727d 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -4043,13 +4043,14 @@ TEST_F( std::vector indices = getQLogEventIndices(QLogEventType::TransportStateUpdate, qLogger); - EXPECT_EQ(indices.size(), 4); - std::array<::std::string, 4> updateArray = { + EXPECT_EQ(indices.size(), 5); + std::array<::std::string, 5> updateArray = { kDerivedZeroRttReadCipher, kDerivedOneRttWriteCipher, kTransportReady, - kDerivedOneRttReadCipher}; - for (int i = 0; i < 4; ++i) { + kDerivedOneRttReadCipher, + kWriteNst}; + for (int i = 0; i < 5; ++i) { auto tmp = std::move(qLogger->logs[indices[i]]); auto event = dynamic_cast(tmp.get()); EXPECT_EQ(event->update, updateArray[i]); diff --git a/quic/server/test/QuicServerTransportTestUtil.h b/quic/server/test/QuicServerTransportTestUtil.h index 0eb4e8d43..58e90763f 100644 --- a/quic/server/test/QuicServerTransportTestUtil.h +++ b/quic/server/test/QuicServerTransportTestUtil.h @@ -350,13 +350,9 @@ class QuicServerTransportTestBase : public virtual testing::Test { virtual void expectWriteNewSessionTicket() { server->setEarlyDataAppParamsFunctions( [](const folly::Optional&, const Buf&) { return false; }, - []() -> Buf { - // This function shouldn't be called - EXPECT_TRUE(false); - return nullptr; - }); + []() -> Buf { return nullptr; }); EXPECT_CALL(*getFakeHandshakeLayer(), writeNewSessionTicket(testing::_)) - .Times(0); + .Times(1); } virtual void setupConnection() {