Accept session tickets with additional optional parameters

Summary:
Allow server to accept session tickets with additional paramters that won't be used.

A subset of servers can include additional transport parameters in the session ticket that are only useful to them. This allows other servers to still use the ticket as long as it has the fields they expect, ignoring any additional parameters.

Reviewed By: mjoras

Differential Revision: D43131800

fbshipit-source-id: ff4ea503c2e30b9bd0e1caf77b418c3891ffdd7f

quic/QuicConstants.h

@@ -589,7 +589,7 @@ constexpr std::chrono::seconds kTimeToRetainLastCongestionAndRttState = 60s;
 
 constexpr uint16_t kMaxNumMigrationsAllowed = 6;
 
-constexpr auto kExpectedNumOfParamsInTheTicket = 8;
+constexpr auto kMinimumNumOfParamsInTheTicket = 8;
 
 constexpr auto kStatelessResetTokenSecretLength = 32;
 

quic/server/handshake/DefaultAppTokenValidator.cpp

@@ -58,14 +58,12 @@ bool DefaultAppTokenValidator::validate(
 
   auto& params = appToken->transportParams.parameters;
 
-  // TODO T33454954 Simplify ticket transport params. see comments in D9324131
-  // Currenly only initialMaxData, initialMaxStreamData, ackDelayExponent, and
-  // maxRecvPacketSize are written into the ticket. In case new parameters
-  // are added for making early data decision (although not likely), this
-  // validator fails the check if number of parameters is not
-  // kExpectedNumOfParamsInTheTicket.
-  if (params.size() != kExpectedNumOfParamsInTheTicket) {
-    VLOG(10) << "Unexpected number of parameters in the ticket";
+  // Reject tickets that do not have the minimum number of params in the ticket.
+  // This is a minimum to allow sending additional optional params
+  // that can be ignored by servers that don't support them.
+  if (params.size() < kMinimumNumOfParamsInTheTicket) {
+    VLOG(10)
+        << "Number of parameters in the ticket is less than the minimum expected";
     return validated = false;
   }
 

quic/server/handshake/test/AppTokenTest.cpp

@@ -29,9 +29,9 @@ void expectAppTokenEqual(
       decodedAppToken->transportParams.parameters.size(),
       appToken.transportParams.parameters.size());
 
-  EXPECT_EQ(
+  EXPECT_GE(
       decodedAppToken->transportParams.parameters.size(),
-      kExpectedNumOfParamsInTheTicket);
+      kMinimumNumOfParamsInTheTicket);
   // TODO Split out into individual flow control parameters.
   auto maxStreamData = getIntegerParameter(
       TransportParameterId::initial_max_stream_data_bidi_local,

quic/server/handshake/test/DefaultAppTokenValidatorTest.cpp

@@ -57,6 +57,39 @@ TEST(DefaultAppTokenValidatorTest, TestValidParams) {
   EXPECT_TRUE(validator.validate(resState));
 }
 
+TEST(DefaultAppTokenValidatorTest, TestValidOptionalParameter) {
+  QuicServerConnectionState conn(
+      FizzServerQuicHandshakeContext::Builder().build());
+  conn.peerAddress = folly::SocketAddress("1.2.3.4", 443);
+  conn.version = QuicVersion::MVFST;
+  conn.transportSettings.zeroRttSourceTokenMatchingPolicy =
+      ZeroRttSourceTokenMatchingPolicy::LIMIT_IF_NO_EXACT_MATCH;
+  auto quicStats = std::make_shared<MockQuicStats>();
+  conn.statsCallback = quicStats.get();
+
+  AppToken appToken;
+  appToken.transportParams = createTicketTransportParameters(
+      conn.transportSettings.idleTimeout.count(),
+      conn.transportSettings.maxRecvPacketSize,
+      conn.transportSettings.advertisedInitialConnectionWindowSize,
+      conn.transportSettings.advertisedInitialBidiLocalStreamWindowSize,
+      conn.transportSettings.advertisedInitialBidiRemoteStreamWindowSize,
+      conn.transportSettings.advertisedInitialUniStreamWindowSize,
+      conn.transportSettings.advertisedInitialMaxStreamsBidi,
+      conn.transportSettings.advertisedInitialMaxStreamsUni);
+  appToken.transportParams.parameters.push_back(
+      encodeIntegerParameter(TransportParameterId::disable_migration, 1));
+  ResumptionState resState;
+  resState.appToken = encodeAppToken(appToken);
+
+  conn.earlyDataAppParamsValidator = [](const folly::Optional<std::string>&,
+                                        const Buf&) { return true; };
+  DefaultAppTokenValidator validator(&conn);
+  EXPECT_CALL(*quicStats, onZeroRttAccepted()).Times(1);
+  EXPECT_CALL(*quicStats, onZeroRttRejected()).Times(0);
+  EXPECT_TRUE(validator.validate(resState));
+}
+
 TEST(
     DefaultAppTokenValidatorTest,
     TestValidUnequalParamsUpdateTransportSettings) {