diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index 3290a6896..d091916fb 100644 --- a/quic/QuicConstants.h +++ b/quic/QuicConstants.h @@ -373,6 +373,8 @@ constexpr auto kExpectedNumOfParamsInTheTicket = 8; constexpr auto kStatelessResetTokenSecretLength = 32; +constexpr uint64_t kMinNumAvailableConnIds = 8; + // default capability of QUIC partial reliability constexpr TransportPartialReliabilitySetting kDefaultPartialReliability = false; diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index cc71ed9fc..8f99527eb 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -11,6 +11,7 @@ #include #include #include +#include namespace quic { @@ -115,6 +116,7 @@ void QuicServerTransport::onReadData( } maybeWriteNewSessionTicket(); maybeNotifyConnectionIdBound(); + maybeIssueConnectionIds(); maybeNotifyTransportReady(); } @@ -298,6 +300,7 @@ void QuicServerTransport::onCryptoEventAvailable() noexcept { } maybeWriteNewSessionTicket(); maybeNotifyConnectionIdBound(); + maybeIssueConnectionIds(); writeSocketData(); maybeNotifyTransportReady(); } catch (const QuicTransportException& ex) { @@ -404,6 +407,35 @@ void QuicServerTransport::maybeNotifyConnectionIdBound() { } } +void QuicServerTransport::maybeIssueConnectionIds() { + if (!conn_->transportSettings.disableMigration && !connectionIdsIssued_ && + serverConn_->serverHandshakeLayer->isHandshakeDone()) { + connectionIdsIssued_ = true; + + CHECK(conn_->transportSettings.statelessResetTokenSecret.hasValue()); + + const uint64_t maximumIdsToIssue = std::min( + conn_->peerActiveConnectionIdLimit, kMinNumAvailableConnIds - 1); + for (size_t i = 0; i < maximumIdsToIssue; i++) { + auto newConnIdData = serverConn_->createAndAddNewSelfConnId(); + if (!newConnIdData.hasValue()) { + return; + } + + CHECK(routingCb_); + routingCb_->onConnectionIdAvailable( + shared_from_this(), newConnIdData->connId); + + NewConnectionIdFrame frame( + newConnIdData->sequenceNumber, + 0, + newConnIdData->connId, + *newConnIdData->token); + sendSimpleFrame(*conn_, std::move(frame)); + } + } +} + void QuicServerTransport::maybeNotifyTransportReady() { if (!transportReadyNotified_ && connCallback_ && hasWriteCipher()) { if (conn_->qLogger) { diff --git a/quic/server/QuicServerTransport.h b/quic/server/QuicServerTransport.h index ab57faacf..67d9c64fa 100644 --- a/quic/server/QuicServerTransport.h +++ b/quic/server/QuicServerTransport.h @@ -117,9 +117,10 @@ class QuicServerTransport private: void processPendingData(bool async); - void maybeWriteNewSessionTicket(); - void maybeNotifyConnectionIdBound(); void maybeNotifyTransportReady(); + void maybeNotifyConnectionIdBound(); + void maybeWriteNewSessionTicket(); + void maybeIssueConnectionIds(); private: RoutingCallback* routingCb_{nullptr}; @@ -128,6 +129,7 @@ class QuicServerTransport bool notifiedConnIdBound_{false}; bool newSessionTicketWritten_{false}; bool shedConnection_{false}; + bool connectionIdsIssued_{false}; QuicServerConnectionState* serverConn_; }; } // namespace quic diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index cda50c5ed..cb0d0d7ae 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -42,11 +42,13 @@ class FakeServerHandshake : public ServerHandshake { explicit FakeServerHandshake( QuicServerConnectionState& conn, bool chloSync = false, - bool cfinSync = false) + bool cfinSync = false, + folly::Optional clientActiveConnectionIdLimit = folly::none) : ServerHandshake(*conn.cryptoState), conn_(conn), chloSync_(chloSync), - cfinSync_(cfinSync) {} + cfinSync_(cfinSync), + clientActiveConnectionIdLimit_(clientActiveConnectionIdLimit) {} void accept(std::shared_ptr) override {} @@ -131,6 +133,12 @@ class FakeServerHandshake : public ServerHandshake { TransportParameterId::idle_timeout, kDefaultIdleTimeout.count())); transportParams.push_back(encodeIntegerParameter( TransportParameterId::max_packet_size, maxRecvPacketSize)); + if (clientActiveConnectionIdLimit_) { + transportParams.push_back(encodeIntegerParameter( + TransportParameterId::active_connection_id_limit, + *clientActiveConnectionIdLimit_)); + } + return ClientTransportParameters{QuicVersion::MVFST, std::move(transportParams)}; } @@ -178,6 +186,7 @@ class FakeServerHandshake : public ServerHandshake { uint64_t maxRecvPacketSize{2 * 1024}; bool allowZeroRttKeys_{false}; std::vector sourceAddrs_; + folly::Optional clientActiveConnectionIdLimit_; }; bool verifyFramePresent( @@ -202,6 +211,10 @@ bool verifyFramePresent( return false; } +struct MigrationParam { + folly::Optional clientSentActiveConnIdTransportParam; +}; + class TestingQuicServerTransport : public QuicServerTransport { public: TestingQuicServerTransport( @@ -268,7 +281,7 @@ class QuicServerTransportTest : public Test { public: void SetUp() override { clientAddr = folly::SocketAddress("127.0.0.1", 1000); - auto fakeServerAddr = folly::SocketAddress("1.2.3.4", 8080); + serverAddr = folly::SocketAddress("1.2.3.4", 8080); clientConnectionId = getTestConnectionId(); initialDestinationConnectionId = clientConnectionId; // change the initialDestinationConnectionId to be different @@ -284,7 +297,7 @@ class QuicServerTransportTest : public Test { serverWrites.push_back(buf->clone()); return buf->computeChainDataLength(); })); - EXPECT_CALL(*sock, address()).WillRepeatedly(ReturnRef(fakeServerAddr)); + EXPECT_CALL(*sock, address()).WillRepeatedly(ReturnRef(serverAddr)); supportedVersions = {QuicVersion::MVFST, QuicVersion::QUIC_DRAFT}; serverCtx = createServerCtx(); connIdAlgo_ = std::make_unique(); @@ -305,6 +318,8 @@ class QuicServerTransportTest : public Test { server->getNonConstConn().serverHandshakeLayer = fakeHandshake; // Allow ignoring path mtu for testing negotiation. server->getNonConstConn().transportSettings.canIgnorePathMTU = true; + server->getNonConstConn().transportSettings.disableMigration = + getDisableMigration(); server->setConnectionIdAlgo(connIdAlgo_.get()); server->setClientConnectionId(*clientConnectionId); VLOG(20) << __func__ << " client connId=" << clientConnectionId->hex() @@ -325,6 +340,10 @@ class QuicServerTransportTest : public Test { fakeHandshake = new FakeServerHandshake(server->getNonConstConn()); } + virtual bool getDisableMigration() { + return true; + } + std::unique_ptr getInitialCipher() { FizzCryptoFactory cryptoFactory; return cryptoFactory.getClientInitialCipher( @@ -471,10 +490,47 @@ class QuicServerTransportTest : public Test { .WillOnce(Invoke([&, clientAddr = clientAddr](auto transport) { EXPECT_EQ(clientAddr, transport->getOriginalPeerAddress()); })); + + EXPECT_TRUE(server->getConn().pendingEvents.frames.empty()); + EXPECT_EQ(server->getConn().nextSelfConnectionIdSequence, 1); recvClientFinished(); // We need an extra pump here for some reason. loopForWrites(); + + // Issue (kMinNumAvailableConnIds - 1) more connection ids on handshake + // complete + auto numNewConnIdFrames = 0; + for (const auto& packet : server->getConn().outstandingPackets) { + for (const auto& frame : packet.packet.frames) { + switch (frame.type()) { + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + const auto writeFrame = frame.asQuicSimpleFrame(); + if (writeFrame->type() == + QuicSimpleFrame::Type::NewConnectionIdFrame_E) { + ++numNewConnIdFrames; + } + break; + } + default: + break; + } + } + } + uint64_t connIdsToIssue = std::min( + server->getConn().peerActiveConnectionIdLimit, + kMinNumAvailableConnIds - 1); + + if (server->getConn().transportSettings.disableMigration || + (connIdsToIssue == 0)) { + EXPECT_EQ(numNewConnIdFrames, 0); + EXPECT_EQ(server->getConn().nextSelfConnectionIdSequence, 1); + } else { + EXPECT_EQ(numNewConnIdFrames, connIdsToIssue); + EXPECT_EQ( + server->getConn().nextSelfConnectionIdSequence, connIdsToIssue + 1); + } + EXPECT_NE(server->getConn().readCodec, nullptr); EXPECT_NE(server->getConn().oneRttWriteCipher, nullptr); EXPECT_NE(server->getConn().oneRttWriteHeaderCipher, nullptr); @@ -613,6 +669,7 @@ class QuicServerTransportTest : public Test { } EventBase evb; + SocketAddress serverAddr; SocketAddress clientAddr; MockConnectionCallback connCallback; MockRoutingCallback routingCallback; @@ -813,7 +870,9 @@ TEST_F(QuicServerTransportTest, IdleTimeoutExpired) { EXPECT_TRUE(server->isClosed()); auto serverReadCodec = makeClientEncryptedCodec(); EXPECT_FALSE(verifyFramePresent( - serverWrites, *serverReadCodec, QuicFrame::Type::ApplicationCloseFrame_E)); + serverWrites, + *serverReadCodec, + QuicFrame::Type::ApplicationCloseFrame_E)); EXPECT_FALSE(verifyFramePresent( serverWrites, *serverReadCodec, QuicFrame::Type::ConnectionCloseFrame_E)); } @@ -1975,7 +2034,35 @@ TEST_F( EXPECT_EQ(server->getConn().streamManager->streamCount(), 0); } -TEST_F(QuicServerTransportTest, ReceiveProbingPacketFromChangedPeerAddress) { +class QuicServerTransportAllowMigrationTest + : public QuicServerTransportTest, + public WithParamInterface { + public: + bool getDisableMigration() override { + return false; + } + + virtual void initializeServerHandshake() override { + fakeHandshake = new FakeServerHandshake( + server->getNonConstConn(), + false, + false, + GetParam().clientSentActiveConnIdTransportParam); + } +}; + +INSTANTIATE_TEST_CASE_P( + QuicServerTransportMigrationTests, + QuicServerTransportAllowMigrationTest, + Values( + MigrationParam{folly::none}, + MigrationParam{0}, + MigrationParam{4}, + MigrationParam{9})); + +TEST_P( + QuicServerTransportAllowMigrationTest, + ReceiveProbingPacketFromChangedPeerAddress) { auto qLogger = std::make_shared(VantagePoint::SERVER); server->getNonConstConn().qLogger = qLogger; server->getNonConstConn().transportSettings.disableMigration = false; @@ -2017,8 +2104,9 @@ TEST_F(QuicServerTransportTest, ReceiveProbingPacketFromChangedPeerAddress) { PacketDropReason::PEER_ADDRESS_CHANGE)); } -TEST_F(QuicServerTransportTest, ReceiveReorderedDataFromChangedPeerAddress) { - server->getNonConstConn().transportSettings.disableMigration = false; +TEST_P( + QuicServerTransportAllowMigrationTest, + ReceiveReorderedDataFromChangedPeerAddress) { auto data = IOBuf::copyBuffer("bad data"); auto firstPacket = packetToBuf(createStreamPacket( *clientConnectionId, @@ -2050,8 +2138,7 @@ TEST_F(QuicServerTransportTest, ReceiveReorderedDataFromChangedPeerAddress) { EXPECT_EQ(server->getConn().peerAddress, peerAddress); } -TEST_F(QuicServerTransportTest, MigrateToUnvalidatedPeer) { - server->getNonConstConn().transportSettings.disableMigration = false; +TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) { auto data = IOBuf::copyBuffer("bad data"); auto packetData = packetToBuf(createStreamPacket( *clientConnectionId, @@ -2128,8 +2215,7 @@ TEST_F(QuicServerTransportTest, MigrateToUnvalidatedPeer) { EXPECT_FALSE(server->getConn().writableBytesLimit); } -TEST_F(QuicServerTransportTest, IgnoreInvalidPathResponse) { - server->getNonConstConn().transportSettings.disableMigration = false; +TEST_P(QuicServerTransportAllowMigrationTest, IgnoreInvalidPathResponse) { auto data = IOBuf::copyBuffer("bad data"); auto packetData = packetToBuf(createStreamPacket( *clientConnectionId, @@ -2187,8 +2273,9 @@ TEST_F(QuicServerTransportTest, IgnoreInvalidPathResponse) { EXPECT_TRUE(server->getConn().writableBytesLimit); } -TEST_F(QuicServerTransportTest, ReceivePathResponseFromDifferentPeerAddress) { - server->getNonConstConn().transportSettings.disableMigration = false; +TEST_P( + QuicServerTransportAllowMigrationTest, + ReceivePathResponseFromDifferentPeerAddress) { auto data = IOBuf::copyBuffer("bad data"); auto packetData = packetToBuf(createStreamPacket( *clientConnectionId, @@ -2259,6 +2346,7 @@ TEST_F(QuicServerTransportTest, TooManyMigrations) { auto qLogger = std::make_shared(VantagePoint::SERVER); server->getNonConstConn().qLogger = qLogger; server->getNonConstConn().transportSettings.disableMigration = false; + auto data = IOBuf::copyBuffer("bad data"); auto packetData = packetToBuf(createStreamPacket( *clientConnectionId, @@ -2296,8 +2384,7 @@ TEST_F(QuicServerTransportTest, TooManyMigrations) { PacketDropReason::PEER_ADDRESS_CHANGE)); } -TEST_F(QuicServerTransportTest, MigrateToValidatedPeer) { - server->getNonConstConn().transportSettings.disableMigration = false; +TEST_P(QuicServerTransportAllowMigrationTest, MigrateToValidatedPeer) { folly::SocketAddress newPeer("100.101.102.103", 23456); server->getNonConstConn().migrationState.previousPeerAddresses.push_back( newPeer); @@ -2365,10 +2452,9 @@ TEST_F(QuicServerTransportTest, MigrateToValidatedPeer) { server->getConn().migrationState.lastCongestionAndRtt->rttvar, rttvar); } -TEST_F( - QuicServerTransportTest, +TEST_P( + QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeerOverwritesCachedRttState) { - server->getNonConstConn().transportSettings.disableMigration = false; folly::SocketAddress newPeer("100.101.102.103", 23456); server->getNonConstConn().migrationState.previousPeerAddresses.push_back( newPeer); @@ -2432,8 +2518,7 @@ TEST_F( server->getConn().migrationState.lastCongestionAndRtt->rttvar, rttvar); } -TEST_F(QuicServerTransportTest, MigrateToStaleValidatedPeer) { - server->getNonConstConn().transportSettings.disableMigration = false; +TEST_P(QuicServerTransportAllowMigrationTest, MigrateToStaleValidatedPeer) { folly::SocketAddress newPeer("100.101.102.103", 23456); server->getNonConstConn().migrationState.previousPeerAddresses.push_back( newPeer); diff --git a/quic/state/StateData.h b/quic/state/StateData.h index eb819b3f3..ce62f1ff1 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -528,8 +528,8 @@ struct QuicConnectionStateBase { // Time at which the connection started. TimePoint connectionTime; - uint64_t peerActiveConnectionIdLimit{kDefaultConnectionIdLimit}; - + // uint64_t peerActiveConnectionIdLimit{kDefaultConnectionIdLimit}; + uint64_t peerActiveConnectionIdLimit{8}; // The current connection id. This will eventually be negotiated // with the peer. folly::Optional clientConnectionId; @@ -543,6 +543,10 @@ struct QuicConnectionStateBase { // Connection ids issued by peer - to be used as destination ids. std::vector peerConnectionIds; + // ConnectionIdAlgo implementation to encode and decode ConnectionId with + // various info, such as routing related info. + ConnectionIdAlgo* connIdAlgo{nullptr}; + // Negotiated version. folly::Optional version;